Merge pull request #52863 from brahmaroutu/conformance_doc

Automatic merge from submit-queue (batch tested with PRs 58411, 58407, 52863). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Create Conformance document to display all tests that belong to Confo…

…rmance suite



**What this PR does / why we need it**:

**Which issue this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close that issue when PR gets merged)*: fixes #

**Special notes for your reviewer**:

**Release note**:

```release-note
NONE
```

test/conformance/BUILD

@@ -1,4 +1,4 @@
-load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library")
+load("@io_bazel_rules_go//go:def.bzl", "go_binary", "go_library", "go_test")
 
 go_library(
     name = "go_default_library",
@@ -48,3 +48,11 @@ sh_test(
         ":list_conformance_tests",
     ],
 )
+
+go_test(
+    name = "go_default_test",
+    srcs = ["walk_test.go"],
+    data = glob(["testdata/**"]),
+    embed = [":go_default_library"],
+    importpath = "k8s.io/kubernetes/test/conformance",
+)

test/conformance/cf_header.md

@@ -0,0 +1,41 @@
+# Kubernetes Conformance Test Suite - v1.9
+
+## **Summary**
+This document provides a summary of the tests included in the Kubernetes conformance test suite.
+Each test lists a set of formal requirements that a platform that meets conformance requirements  must adhere to.
+
+The tests are a subset of the "e2e" tests that make up the Kubernetes testing infrastructure.
+Each test is identified by the presence of the `[Conformance]` keyword in the ginkgo descriptive function calls.
+The contents of this document are extracted from comments preceding those `[Conformance]` keywords
+and those comments are expected to include a descriptive overview of what the test is validating using
+RFC2119 keywords. This will provide a clear distinction between which bits of code in the tests are
+there for the purposes of validating the platform rather than simply infrastructure logic used to setup, or
+clean up the tests.
+
+Example:
+```
+/*
+  Testname: Kubelet-OutputToLogs
+  Description: By default the stdout and stderr from the process
+           being executed in a pod MUST be sent to the pod's logs.
+*/
+// Note this test needs to be fixed to also test for stderr
+It("it should print the output to logs [Conformance]", func() {
+```
+
+would generate the following documentation for the test. Note that the "TestName" from the Documentation above will
+be used to document the test which make it more human readable. The "Description" field will be used as the
+documentation for that test.
+
+### **Output:**
+## [Kubelet-OutputToLogs](https://github.com/kubernetes/kubernetes/blob/release-1.9/test/e2e_node/kubelet_test.go#L42)
+
+By default the stdout and stderr from the process
+being executed in a pod MUST be sent to the pod's logs.
+Note this test needs to be fixed to also test for stderr
+
+Notational Conventions when documenting the tests with the key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" are to be interpreted as described in [RFC 2119](https://tools.ietf.org/html/rfc2119).
+
+Note: Please see the Summary at the end of this document to find the number of tests documented for conformance.
+
+## **List of Tests**

test/conformance/walk.go

@@ -24,17 +24,89 @@ limitations under the License.
 package main
 
 import (
+	"flag"
 	"fmt"
 	"go/ast"
 	"go/parser"
 	"go/token"
+	"io/ioutil"
 	"os"
 	"path/filepath"
+	"regexp"
+	"strconv"
 	"strings"
 )
 
+var (
+	baseURL                                           = flag.String("url", "https://github.com/kubernetes/kubernetes/tree/master/", "location of the current source")
+	confDoc                                           = flag.Bool("conformance", false, "write a conformance document")
+	totalConfTests, totalLegacyTests, missingComments int
+)
+
+const regexDescribe = "Describe|KubeDescribe|SIGDescribe"
+const regexContext = "Context"
+
 type visitor struct {
-	FileSet *token.FileSet
+	FileSet      *token.FileSet
+	lastDescribe describe
+	cMap         ast.CommentMap
+	//list of all the conformance tests in the path
+	tests []conformanceData
+}
+
+//describe contains text associated with ginkgo describe container
+type describe struct {
+	text        string
+	lastContext context
+}
+
+//context contain the text associated with the Context clause
+type context struct {
+	text string
+}
+
+type conformanceData struct {
+	// A URL to the line of code in the kube src repo for the test
+	URL string
+	// Extracted from the "Testname:" comment before the test
+	TestName string
+	// Extracted from the "Description:" comment before the test
+	Description string
+}
+
+func (v *visitor) convertToConformanceData(at *ast.BasicLit) {
+	cd := conformanceData{}
+
+	comment := v.comment(at)
+	pos := v.FileSet.Position(at.Pos())
+	cd.URL = fmt.Sprintf("%s%s#L%d", *baseURL, pos.Filename, pos.Line)
+
+	lines := strings.Split(comment, "\n")
+	cd.Description = ""
+	for _, line := range lines {
+		line = strings.TrimSpace(line)
+		if strings.HasPrefix(line, "Testname:") {
+			line = strings.TrimSpace(line[9:])
+			cd.TestName = line
+			continue
+		}
+		if strings.HasPrefix(line, "Description:") {
+			line = strings.TrimSpace(line[12:])
+		}
+		cd.Description += line + "\n"
+	}
+
+	if cd.TestName == "" {
+		testName := v.getDescription(at.Value)
+		i := strings.Index(testName, "[Conformance]")
+		if i > 0 {
+			cd.TestName = strings.TrimSpace(testName[:i])
+		} else {
+			cd.TestName = testName
+		}
+	}
+
+	v.tests = append(v.tests, cd)
 }
 
 func newVisitor() *visitor {
@@ -84,7 +156,16 @@ func (v *visitor) isLegacyItCall(call *ast.CallExpr) bool {
 func (v *visitor) failf(expr ast.Expr, format string, a ...interface{}) {
 	msg := fmt.Sprintf(format, a...)
 	fmt.Fprintf(os.Stderr, "ERROR at %v: %s\n", v.FileSet.Position(expr.Pos()), msg)
-	os.Exit(65)
+}
+
+func (v *visitor) comment(x *ast.BasicLit) string {
+	for _, comm := range v.cMap.Comments() {
+		testOffset := int(x.Pos()-comm.End()) - len("framework.ConformanceIt(\"")
+		if 0 < testOffset && testOffset < 3 {
+			return comm.Text()
+		}
+	}
+	return ""
 }
 
 func (v *visitor) emit(arg ast.Expr) {
@@ -94,13 +175,94 @@ func (v *visitor) emit(arg ast.Expr) {
 			v.failf(at, "framework.ConformanceIt() called with non-string argument")
 			return
 		}
-		fmt.Printf("%s: %s\n", v.FileSet.Position(at.Pos()).Filename, at.Value)
+
+		if *confDoc {
+			v.convertToConformanceData(at)
+		} else {
+			fmt.Printf("%s: %s\n", v.FileSet.Position(at.Pos()).Filename, at.Value)
+		}
 	default:
 		v.failf(at, "framework.ConformanceIt() called with non-literal argument")
 		fmt.Fprintf(os.Stderr, "ERROR: non-literal argument %v at %v\n", arg, v.FileSet.Position(arg.Pos()))
 	}
 }
 
+func (v *visitor) getDescription(value string) string {
+	if len(v.lastDescribe.lastContext.text) > 0 {
+		return strings.Trim(v.lastDescribe.text, "\"") +
+			" " + strings.Trim(v.lastDescribe.lastContext.text, "\"") +
+			" " + strings.Trim(value, "\"")
+	}
+	return strings.Trim(v.lastDescribe.text, "\"") +
+		" " + strings.Trim(value, "\"")
+}
+
+// funcName converts a selectorExpr with two idents into a string,
+// x.y -> "x.y"
+func funcName(n ast.Expr) string {
+	if sel, ok := n.(*ast.SelectorExpr); ok {
+		if x, ok := sel.X.(*ast.Ident); ok {
+			return x.String() + "." + sel.Sel.String()
+		}
+	}
+	return ""
+}
+
+// isSprintf returns whether the given node is a call to fmt.Sprintf
+func isSprintf(n ast.Expr) bool {
+	call, ok := n.(*ast.CallExpr)
+	return ok && funcName(call.Fun) == "fmt.Sprintf" && len(call.Args) != 0
+}
+
+// firstArg attempts to statically determine the value of the first
+// argument. It only handles strings, and converts any unknown values
+// (fmt.Sprintf interpolations) into *.
+func (v *visitor) firstArg(n *ast.CallExpr) string {
+	if len(n.Args) == 0 {
+		return ""
+	}
+	var lit *ast.BasicLit
+	if isSprintf(n.Args[0]) {
+		return v.firstArg(n.Args[0].(*ast.CallExpr))
+	}
+	lit, ok := n.Args[0].(*ast.BasicLit)
+	if ok && lit.Kind == token.STRING {
+		val, err := strconv.Unquote(lit.Value)
+		if err != nil {
+			panic(err)
+		}
+		if strings.Contains(val, "%") {
+			val = strings.Replace(val, "%d", "*", -1)
+			val = strings.Replace(val, "%v", "*", -1)
+			val = strings.Replace(val, "%s", "*", -1)
+		}
+		return val
+	}
+	if ident, ok := n.Args[0].(*ast.Ident); ok {
+		return ident.String()
+	}
+	return "*"
+}
+
+// matchFuncName returns the first argument of a function if it's
+// a Ginkgo-relevant function (Describe/KubeDescribe/Context),
+// and the empty string otherwise.
+func (v *visitor) matchFuncName(n *ast.CallExpr, pattern string) string {
+	switch x := n.Fun.(type) {
+	case *ast.SelectorExpr:
+		if match, err := regexp.MatchString(pattern, x.Sel.Name); err == nil && match {
+			return v.firstArg(n)
+		}
+	case *ast.Ident:
+		if match, err := regexp.MatchString(pattern, x.Name); err == nil && match {
+			return v.firstArg(n)
+		}
+	default:
+		return ""
+	}
+	return ""
+}
+
 // Visit visits each node looking for either calls to framework.ConformanceIt,
 // which it will emit in its list of conformance tests, or legacy calls to
 // It() with a manually embedded [Conformance] tag, which it will complain
@@ -108,9 +270,16 @@ func (v *visitor) emit(arg ast.Expr) {
 func (v *visitor) Visit(node ast.Node) (w ast.Visitor) {
 	switch t := node.(type) {
 	case *ast.CallExpr:
-		if v.isConformanceCall(t) {
+		if name := v.matchFuncName(t, regexDescribe); name != "" && len(t.Args) >= 2 {
+			v.lastDescribe = describe{text: name}
+		} else if name := v.matchFuncName(t, regexContext); name != "" && len(t.Args) >= 2 {
+			v.lastDescribe.lastContext = context{text: name}
+		} else if v.isConformanceCall(t) {
+			totalConfTests++
 			v.emit(t.Args[0])
+			return nil
 		} else if v.isLegacyItCall(t) {
+			totalLegacyTests++
 			v.failf(t, "Using It() with manual [Conformance] tag is no longer allowed.  Use framework.ConformanceIt() instead.")
 			return nil
 		}
@@ -120,7 +289,7 @@ func (v *visitor) Visit(node ast.Node) (w ast.Visitor) {
 
 func scandir(dir string) {
 	v := newVisitor()
-	pkg, err := parser.ParseDir(v.FileSet, dir, nil, 0)
+	pkg, err := parser.ParseDir(v.FileSet, dir, nil, parser.ParseComments)
 	if err != nil {
 		panic(err)
 	}
@@ -130,37 +299,58 @@ func scandir(dir string) {
 	}
 }
 
-func scanfile(path string) {
+func scanfile(path string, src interface{}) []conformanceData {
 	v := newVisitor()
-	file, err := parser.ParseFile(v.FileSet, path, nil, 0)
+	file, err := parser.ParseFile(v.FileSet, path, src, parser.ParseComments)
 	if err != nil {
 		panic(err)
 	}
 
+	v.cMap = ast.NewCommentMap(v.FileSet, file, file.Comments)
+
 	ast.Walk(v, file)
+	return v.tests
 }
 
 func main() {
-	args := os.Args[1:]
-	if len(args) < 1 {
+	flag.Parse()
+
+	if len(flag.Args()) < 1 {
 		fmt.Fprintf(os.Stderr, "USAGE: %s <DIR or FILE> [...]\n", os.Args[0])
 		os.Exit(64)
 	}
 
-	for _, arg := range args {
+	if *confDoc {
+		// Note: this assumes that you're running from the root of the kube src repo
+		header, err := ioutil.ReadFile("test/conformance/cf_header.md")
+		if err == nil {
+			fmt.Printf("%s\n\n", header)
+		}
+	}
+
+	totalConfTests = 0
+	totalLegacyTests = 0
+	missingComments = 0
+	for _, arg := range flag.Args() {
 		filepath.Walk(arg, func(path string, info os.FileInfo, err error) error {
 			if err != nil {
 				return err
 			}
-			if info.IsDir() {
-				scandir(path)
-			} else {
-				// TODO(mml): Remove this once we have all-go-srcs build rules.  See https://github.com/kubernetes/repo-infra/pull/45
-				if strings.HasSuffix(path, ".go") {
-					scanfile(path)
+			if strings.HasSuffix(path, ".go") {
+				tests := scanfile(path, nil)
+				for _, cd := range tests {
+					fmt.Printf("## [%s](%s)\n\n", cd.TestName, cd.URL)
+					fmt.Printf("%s\n\n", cd.Description)
+					if len(cd.Description) < 10 {
+						missingComments++
+					}
 				}
 			}
 			return nil
 		})
 	}
+	if *confDoc {
+		fmt.Println("\n## **Summary**")
+		fmt.Printf("\nTotal Conformance Tests: %d, total legacy tests that need conversion: %d, while total tests that need comment sections: %d\n\n", totalConfTests, totalLegacyTests, missingComments)
+	}
 }

test/conformance/walk_test.go

@@ -0,0 +1,95 @@
+/*
+Copyright 2016 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package main
+
+import (
+	"reflect"
+	"testing"
+)
+
+var conformanceCases = []struct {
+	filename string
+	code     string
+	output   []conformanceData
+}{
+	// Go unit test
+	{"test/list/main_test.go", `
+var num = 3
+func Helper(x int) { return x / 0 }
+var _ = Describe("Feature", func() {
+/*
+   Testname: Kubelet-OutputToLogs
+   Description: By default the stdout and stderr from the process
+   being executed in a pod MUST be sent to the pod's logs.
+*/
+ framework.ConformanceIt("validates describe with ConformanceIt", func() {})
+})`, []conformanceData{{URL: "https://github.com/kubernetes/kubernetes/tree/master/test/list/main_test.go#L11", TestName: "Kubelet-OutputToLogs",
+		Description: `By default the stdout and stderr from the process
+being executed in a pod MUST be sent to the pod's logs.` + "\n\n"}},
+	},
+	// Describe + It
+	{"e2e/foo.go", `
+var _ = Describe("Feature", func() {
+	//It should have comment
+	framework.ConformanceIt("should work properly", func() {})
+})`, []conformanceData{{URL: "https://github.com/kubernetes/kubernetes/tree/master/e2e/foo.go#L5", TestName: "Feature should work properly", Description: "It should have comment\n\n"}},
+	},
+	// KubeDescribe + It
+	{"e2e/foo.go", `
+var _ = framework.KubeDescribe("Feature", func() {
+	/*It should have comment*/
+	framework.ConformanceIt("should work properly", func() {})
+})`, []conformanceData{{URL: "https://github.com/kubernetes/kubernetes/tree/master/e2e/foo.go#L5", TestName: "Feature should work properly", Description: "It should have comment\n\n"}},
+	},
+	// KubeDescribe + Context + It
+	{"e2e/foo.go", `
+var _ = framework.KubeDescribe("Feature", func() {
+	Context("when offline", func() {
+		//Testname: Kubelet-OutputToLogs
+		//Description: By default the stdout and stderr from the process
+		//being executed in a pod MUST be sent to the pod's logs.
+		framework.ConformanceIt("should work", func() {})
+	})
+})`, []conformanceData{{URL: "https://github.com/kubernetes/kubernetes/tree/master/e2e/foo.go#L8", TestName: "Kubelet-OutputToLogs",
+		Description: `By default the stdout and stderr from the process
+being executed in a pod MUST be sent to the pod's logs.` + "\n\n"}},
+	},
+	// KubeDescribe + Context + It
+	{"e2e/foo.go", `
+var _ = framework.KubeDescribe("Feature", func() {
+	Context("with context", func() {
+		//Description: By default the stdout and stderr from the process
+		//being executed in a pod MUST be sent to the pod's logs.
+		framework.ConformanceIt("should work", func() {})
+	})
+})`, []conformanceData{{URL: "https://github.com/kubernetes/kubernetes/tree/master/e2e/foo.go#L7", TestName: "Feature with context should work",
+		Description: `By default the stdout and stderr from the process
+being executed in a pod MUST be sent to the pod's logs.` + "\n\n"}},
+	},
+}
+
+func TestConformance(t *testing.T) {
+	for _, test := range conformanceCases {
+		code := "package test\n" + test.code
+		*confDoc = true
+		tests := scanfile(test.filename, code)
+		if !reflect.DeepEqual(tests, test.output) {
+			t.Errorf("code:\n%s\ngot  %v\nwant %v",
+				code, tests, test.output)
+		}
+	}
+}