github/kubernetes
4
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Update the conformance list and doc generation logic

Browse Source 
The existing walk.go and conformance.txt have a few shortcomings
which we'd like to resolve:
 - difficult to get the full test name due to test context nesting
 - complicated AST logic and understanding necessary due to the
different ways a test can be invoked and written

This changes the AST parsing logic to be much more simple and simply
looks for the comments at/around a specific line. This file/line
information (and the full test name) is gathered by a custom ginkgo
reporter which dumps the SpecSummary data to a file.

Also, the SpecSummary dump can, itself, be potentially useful for
other post-processing and debugging tasks.

Signed-off-by: John Schnake <jschnake@vmware.com>
This commit is contained in:
John Schnake
2019-11-06 13:57:11 -06:00
committed by Jefftree
parent 4e79344501
commit 2683b1065c
14 changed files with 3097 additions and 773 deletions
Download Patch File Download Diff File Expand all files Collapse all files

613
test/conformance/walk.go
View File

@@ -1,5 +1,5 @@
/*
Copyright 2017 The Kubernetes Authors.
Copyright 2019 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.
@@ -14,84 +14,289 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
// Package main provides a tool that scans kubernetes e2e test source code
// looking for conformance test declarations, which it emits on stdout. It
// also looks for legacy, manually added "[Conformance]" tags and reports an
// error if it finds any.
//
// This approach is not air tight, but it will serve our purpose as a
// pre-submit check.
package main
import (
"encoding/json"
"flag"
"fmt"
"go/ast"
"go/parser"
"go/token"
"gopkg.in/yaml.v2"
"io"
"log"
"os"
"path/filepath"
"regexp"
"sort"
"strconv"
"strings"
"text/template"
"github.com/onsi/ginkgo/types"
)
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")
version = flag.String("version", "v1.9", "version of this conformance document")
totalConfTests, totalLegacyTests, missingComments int
baseURL = flag.String("url", "https://github.com/kubernetes/kubernetes/tree/master/", "location of the current source")
k8sPath = flag.String("source", "", "location of the current source on the current machine")
confDoc = flag.Bool("docs", false, "write a conformance document")
version = flag.String("version", "v1.9", "version of this conformance document")
// If a test name contains any of these tags, it is ineligble for promotion to conformance
regexIneligibleTags = regexp.MustCompile(`\[(Alpha|Feature:[^\]]+|Flaky)\]`)
// Conformance comments should be within this number of lines to the call itself.
// Allowing for more than one in case a spare comment or two is below it.
conformanceCommentsLineWindow = 5
seenLines map[string]struct{}
)
const regexDescribe = "Describe|KubeDescribe|SIGDescribe"
const regexContext = "^Context$"
type frame struct {
Function string
type visitor struct {
FileSet *token.FileSet
describes []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 {
rparen token.Pos
text string
lastContext context
}
//context contain the text associated with the Context clause
type context struct {
text string
// File and Line are the file name and line number of the
// location in this frame. For non-leaf frames, this will be
// the location of a call. These may be the empty string and
// zero, respectively, if not known.
File string
Line int
}
type conformanceData struct {
// A URL to the line of code in the kube src repo for the test
URL string
// A URL to the line of code in the kube src repo for the test. Omitted from the YAML to avoid exposing line number.
URL string `yaml:"-"`
// Extracted from the "Testname:" comment before the test
TestName string
// CodeName is taken from the actual ginkgo descriptions, e.g. `[sig-apps] Foo should bar [Conformance]`
CodeName string
// Extracted from the "Description:" comment before the test
Description string
// Version when this test is added or modified ex: v1.12, v1.13
Release string
// File is the filename where the test is defined. We intentionally don't save the line here to avoid meaningless changes.
File string
}
func (v *visitor) convertToConformanceData(at *ast.BasicLit) {
cd := conformanceData{}
func main() {
flag.Parse()
comment := v.comment(at)
pos := v.FileSet.Position(at.Pos())
cd.URL = fmt.Sprintf("%s%s#L%d", *baseURL, pos.Filename, pos.Line)
if len(flag.Args()) < 1 {
log.Fatalln("Requires the name of the test details file as first and only argument.")
}
testDetailsFile := flag.Args()[0]
f, err := os.Open(testDetailsFile)
if err != nil {
log.Fatalf("Failed to open file %v: %v", testDetailsFile, err)
}
defer f.Close()
seenLines = map[string]struct{}{}
dec := json.NewDecoder(f)
testInfos := []*conformanceData{}
for {
var spec *types.SpecSummary
if err := dec.Decode(&spec); err == io.EOF {
break
} else if err != nil {
log.Fatal(err)
}
if isConformance(spec) {
testInfo := getTestInfo(spec)
if testInfo != nil {
testInfos = append(testInfos, testInfo)
}
if err := validateTestName(testInfo.CodeName); err != nil {
log.Fatal(err)
}
}
}
sort.Slice(testInfos, func(i, j int) bool { return testInfos[i].CodeName < testInfos[j].CodeName })
saveAllTestInfo(testInfos)
}
func isConformance(spec *types.SpecSummary) bool {
return strings.Contains(getTestName(spec), "[Conformance]")
}
func getTestInfo(spec *types.SpecSummary) *conformanceData {
var c *conformanceData
var err error
// The key to this working is that we don't need to parse every file or walk
// every componentCodeLocation. The last componentCodeLocation is going to typically start
// with the ConformanceIt(...) call and the next call in that callstack will be the
// ast.Node which is attached to the comment that we want.
for i := len(spec.ComponentCodeLocations) - 1; i > 0; i-- {
fullstacktrace := spec.ComponentCodeLocations[i].FullStackTrace
c, err = getConformanceDataFromStackTrace(fullstacktrace)
if err != nil {
log.Printf("Error looking for conformance data: %v", err)
}
if c != nil {
break
}
}
if c == nil {
log.Printf("Did not find test info for spec: %#v\n", getTestName(spec))
return nil
}
c.CodeName = getTestName(spec)
return c
}
func getTestName(spec *types.SpecSummary) string {
return strings.Join(spec.ComponentTexts[1:], " ")
}
func saveAllTestInfo(dataSet []*conformanceData) {
if *confDoc {
// Note: this assumes that you're running from the root of the kube src repo
templ, err := template.ParseFiles("./test/conformance/cf_header.md")
if err != nil {
fmt.Printf("Error reading the Header file information: %s\n\n", err)
}
data := struct {
Version string
}{
Version: *version,
}
templ.Execute(os.Stdout, data)
for _, data := range dataSet {
fmt.Printf("## [%s](%s)\n\n", data.TestName, data.URL)
fmt.Printf("- Added to conformance in release %s\n", data.Release)
fmt.Printf("- Defined in code as: %s\n\n", data.CodeName)
fmt.Printf("%s\n\n", data.Description)
}
return
}
// Serialize the list as a whole. Generally meant to end up as conformance.txt which tracks the set of tests.
b, err := yaml.Marshal(dataSet)
if err != nil {
log.Printf("Error marshalling data into YAML: %v", err)
}
fmt.Println(string(b))
}
func getConformanceDataFromStackTrace(fullstackstrace string) (*conformanceData, error) {
// The full stacktrace to parse from ginkgo is of the form:
// k8s.io/kubernetes/test/e2e/storage/utils.SIGDescribe(0x51f4c4f, 0xf, 0x53a0dd8, 0xc000ab6e01)\n\ttest/e2e/storage/utils/framework.go:23 +0x75\n ... ...
// So we need to split it into lines, remove whitespace, and then grab the files/lines.
stack := strings.Replace(fullstackstrace, "\t", "", -1)
calls := strings.Split(stack, "\n")
frames := []frame{}
i := 0
for i < len(calls) {
fileLine := strings.Split(calls[i+1], " ")
lineinfo := strings.Split(fileLine[0], ":")
line, err := strconv.Atoi(lineinfo[1])
if err != nil {
panic(err)
}
frames = append(frames, frame{
Function: calls[i],
File: lineinfo[0],
Line: line,
})
i += 2
}
// filenames have `/go/src/k8s.io` prefix which dont exist locally
for i, v := range frames {
frames[i].File = strings.Replace(v.File,
"/go/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes",
*k8sPath, 1)
}
for _, curFrame := range frames {
if _, seen := seenLines[fmt.Sprintf("%v:%v", curFrame.File, curFrame.Line)]; seen {
continue
}
freader, err := os.Open(curFrame.File)
if err != nil {
return nil, err
}
defer freader.Close()
cd, err := scanFileForFrame(curFrame.File, freader, curFrame)
if err != nil {
return nil, err
}
if cd != nil {
return cd, nil
}
}
return nil, nil
}
// scanFileForFrame will scan the target and look for a conformance comment attached to the function
// described by the target frame. If the comment can't be found then nil, nil is returned.
func scanFileForFrame(filename string, src interface{}, targetFrame frame) (*conformanceData, error) {
fset := token.NewFileSet() // positions are relative to fset
f, err := parser.ParseFile(fset, filename, src, parser.ParseComments)
if err != nil {
return nil, err
}
cmap := ast.NewCommentMap(fset, f, f.Comments)
for _, cs := range cmap {
for _, c := range cs {
if cd := tryCommentGroupAndFrame(fset, c, targetFrame); cd != nil {
return cd, nil
}
}
}
return nil, nil
}
func validateTestName(s string) error {
matches := regexIneligibleTags.FindAllString(s, -1)
if matches != nil {
return fmt.Errorf("'%s' cannot have invalid tags %v", s, strings.Join(matches, ","))
}
return nil
}
func tryCommentGroupAndFrame(fset *token.FileSet, cg *ast.CommentGroup, f frame) *conformanceData {
if !shouldProcessCommentGroup(fset, cg, f) {
return nil
}
// Each file/line will either be some helper function (not a conformance comment) or apply to just a single test. Don't revisit.
if seenLines != nil {
seenLines[fmt.Sprintf("%v:%v", f.File, f.Line)] = struct{}{}
}
cd := commentToConformanceData(cg.Text())
if cd == nil {
return nil
}
cd.URL = fmt.Sprintf("%s%s#L%d", *baseURL, f.File, f.Line)
cd.File = f.File
return cd
}
func shouldProcessCommentGroup(fset *token.FileSet, cg *ast.CommentGroup, f frame) bool {
lineDiff := f.Line - fset.Position(cg.End()).Line
return lineDiff > 0 && lineDiff <= conformanceCommentsLineWindow
}
func commentToConformanceData(comment string) *conformanceData {
lines := strings.Split(comment, "\n")
cd.Description = ""
descLines := []string{}
cd := &conformanceData{}
for _, line := range lines {
line = strings.TrimSpace(line)
if len(line) == 0 {
continue
}
if sline := regexp.MustCompile("^Testname\\s*:\\s*").Split(line, -1); len(sline) == 2 {
cd.TestName = sline[1]
continue
@@ -103,326 +308,12 @@ func (v *visitor) convertToConformanceData(at *ast.BasicLit) {
if sline := regexp.MustCompile("^Description\\s*:\\s*").Split(line, -1); len(sline) == 2 {
line = sline[1]
}
cd.Description += line + "\n"
descLines = append(descLines, line)
}
if cd.Release == "" && cd.TestName == "" {
return nil
}
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 {
return &visitor{
FileSet: token.NewFileSet(),
}
}
func (v *visitor) isConformanceCall(call *ast.CallExpr) bool {
switch fun := call.Fun.(type) {
case *ast.SelectorExpr:
if fun.Sel != nil {
return fun.Sel.Name == "ConformanceIt"
}
}
return false
}
func (v *visitor) isLegacyItCall(call *ast.CallExpr) bool {
switch fun := call.Fun.(type) {
case *ast.Ident:
if fun.Name != "It" {
return false
}
if len(call.Args) < 1 {
v.failf(call, "Not enough arguments to It()")
}
default:
return false
}
switch arg := call.Args[0].(type) {
case *ast.BasicLit:
if arg.Kind != token.STRING {
v.failf(arg, "Unexpected non-string argument to It()")
}
if strings.Contains(arg.Value, "[Conformance]") {
return true
}
default:
// non-literal argument to It()... we just ignore these even though they could be a way to "sneak in" a conformance test
}
return false
}
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)
}
func (v *visitor) comment(x *ast.BasicLit) string {
for _, comm := range v.cMap.Comments() {
testOffset := int(x.Pos()-comm.End()) - len("framework.ConformanceIt(\"")
//Cannot assume the offset is within three or four tabs from the test block itself.
//It is better to trim the newlines, tabs, etc and then we if the comment is followed
//by the test block itself so that we can associate the comment with it properly.
if 0 <= testOffset && testOffset <= 10 {
b1 := make([]byte, x.Pos()-comm.End())
//if we fail to open the file to compare the content we just assume the
//proximity of the comment and apply it.
myf, err := os.Open(v.FileSet.File(x.Pos()).Name())
if err == nil {
defer myf.Close()
if _, err := myf.Seek(int64(comm.End()), 0); err == nil {
if _, err := myf.Read(b1); err == nil {
if strings.Compare(strings.Trim(string(b1), "\t \r\n"), "framework.ConformanceIt(\"") == 0 {
return comm.Text()
}
}
}
} else {
//comment section's end is noticed within 10 characters from framework.ConformanceIt block
return comm.Text()
}
}
}
return ""
}
func (v *visitor) emit(arg ast.Expr) {
switch at := arg.(type) {
case *ast.BasicLit:
if at.Kind != token.STRING {
v.failf(at, "framework.ConformanceIt() called with non-string argument")
return
}
description := v.getDescription(at.Value)
err := validateTestName(description)
if err != nil {
v.failf(at, err.Error())
return
}
at.Value = normalizeTestName(at.Value)
if *confDoc {
v.convertToConformanceData(at)
} else {
fmt.Printf("%s: %q\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 {
tokens := []string{}
for _, describe := range v.describes {
tokens = append(tokens, describe.text)
if len(describe.lastContext.text) > 0 {
tokens = append(tokens, describe.lastContext.text)
}
}
tokens = append(tokens, value)
trimmed := []string{}
for _, token := range tokens {
trimmed = append(trimmed, strings.Trim(token, "\""))
}
return strings.Join(trimmed, " ")
}
var (
regexTag = regexp.MustCompile(`(\[[a-zA-Z0-9:-]+\])`)
)
// normalizeTestName removes tags (e.g., [Feature:Foo]), double quotes and trim
// the spaces to normalize the test name.
func normalizeTestName(s string) string {
r := regexTag.ReplaceAllString(s, "")
r = strings.Trim(r, "\"")
return strings.TrimSpace(r)
}
func validateTestName(s string) error {
matches := regexIneligibleTags.FindAllString(s, -1)
if matches != nil {
return fmt.Errorf("'%s' cannot have invalid tags %v", s, strings.Join(matches, ","))
}
return nil
}
// 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
// about.
func (v *visitor) Visit(node ast.Node) (w ast.Visitor) {
lastDescribe := len(v.describes) - 1
switch t := node.(type) {
case *ast.CallExpr:
if name := v.matchFuncName(t, regexDescribe); name != "" && len(t.Args) >= 2 {
v.describes = append(v.describes, describe{text: name, rparen: t.Rparen})
} else if name := v.matchFuncName(t, regexContext); name != "" && len(t.Args) >= 2 {
if lastDescribe > -1 {
v.describes[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
}
}
// If we're past the position of the last describe's rparen, pop the describe off
if lastDescribe > -1 && node != nil {
if node.Pos() > v.describes[lastDescribe].rparen {
v.describes = v.describes[:lastDescribe]
}
}
return v
}
func scanfile(path string, src interface{}) []conformanceData {
v := newVisitor()
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() {
flag.Parse()
if len(flag.Args()) < 1 {
fmt.Fprintf(os.Stderr, "USAGE: %s <DIR or FILE> [...]\n", os.Args[0])
os.Exit(64)
}
if *confDoc {
// Note: this assumes that you're running from the root of the kube src repo
templ, err := template.ParseFiles("test/conformance/cf_header.md")
if err != nil {
fmt.Printf("Error reading the Header file information: %s\n\n", err)
}
data := struct {
Version string
}{
Version: *version,
}
templ.Execute(os.Stdout, data)
}
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 strings.HasSuffix(path, ".go") {
tests := scanfile(path, nil)
for _, cd := range tests {
fmt.Printf("## [%s](%s)\n\n", cd.TestName, cd.URL)
fmt.Printf("### Release %s\n", cd.Release)
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)
}
cd.Description = strings.Join(descLines, " ")
return cd
}