From fe972b8f2f0efc261906b78cc087c77a57e05b06 Mon Sep 17 00:00:00 2001 From: Jefftree Date: Wed, 23 Sep 2020 13:28:06 -0700 Subject: [PATCH 1/2] Remove dependency on behaviors for conformance generation --- test/conformance/testdata/conformance.yaml | 5 --- test/conformance/walk.go | 49 ++++++++++++---------- test/conformance/walk_test.go | 26 ++++-------- test/e2e/common/pods.go | 3 -- test/e2e/node/pods.go | 2 - 5 files changed, 36 insertions(+), 49 deletions(-) diff --git a/test/conformance/testdata/conformance.yaml b/test/conformance/testdata/conformance.yaml index f2b1e25a164..7f86b58ae60 100755 --- a/test/conformance/testdata/conformance.yaml +++ b/test/conformance/testdata/conformance.yaml @@ -244,9 +244,6 @@ Query for the Pod with the new value for the label MUST be successful. release: v1.9 file: test/e2e/common/pods.go - behaviors: - - pod/spec/label/create - - pod/spec/label/patch - testname: Pods, service environment variables codename: '[k8s.io] Pods should contain environment variables for services [NodeConformance] [Conformance]' @@ -484,8 +481,6 @@ have QOSClass set to PodQOSGuaranteed. release: v1.9 file: test/e2e/node/pods.go - behaviors: - - pod/spec/container/resources - testname: Pods, prestop hook codename: '[k8s.io] [sig-node] PreStop should call prestop when killing a pod [Conformance]' description: Create a server pod with a rest endpoint '/write' that changes state.Received diff --git a/test/conformance/walk.go b/test/conformance/walk.go index 99edc6cb06f..c44a43f76dc 100644 --- a/test/conformance/walk.go +++ b/test/conformance/walk.go @@ -23,7 +23,6 @@ import ( "go/ast" "go/parser" "go/token" - "gopkg.in/yaml.v2" "io" "log" "os" @@ -33,11 +32,27 @@ import ( "strings" "text/template" - "github.com/onsi/ginkgo/types" + "gopkg.in/yaml.v2" - "k8s.io/kubernetes/test/conformance/behaviors" + "github.com/onsi/ginkgo/types" ) +// ConformanceData describes the structure of the conformance.yaml file +type ConformanceData struct { + // 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 +} + var ( 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") @@ -80,7 +95,7 @@ func main() { seenLines = map[string]struct{}{} dec := json.NewDecoder(f) - testInfos := []*behaviors.ConformanceData{} + testInfos := []*ConformanceData{} for { var spec *types.SpecSummary if err := dec.Decode(&spec); err == io.EOF { @@ -109,8 +124,8 @@ func isConformance(spec *types.SpecSummary) bool { return strings.Contains(getTestName(spec), "[Conformance]") } -func getTestInfo(spec *types.SpecSummary) *behaviors.ConformanceData { - var c *behaviors.ConformanceData +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 @@ -140,7 +155,7 @@ func getTestName(spec *types.SpecSummary) string { return strings.Join(spec.ComponentTexts[1:], " ") } -func saveAllTestInfo(dataSet []*behaviors.ConformanceData) { +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") @@ -171,7 +186,7 @@ func saveAllTestInfo(dataSet []*behaviors.ConformanceData) { fmt.Println(string(b)) } -func getConformanceDataFromStackTrace(fullstackstrace string) (*behaviors.ConformanceData, error) { +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. @@ -225,7 +240,7 @@ func getConformanceDataFromStackTrace(fullstackstrace string) (*behaviors.Confor // 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) (*behaviors.ConformanceData, error) { +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 { @@ -251,7 +266,7 @@ func validateTestName(s string) error { return nil } -func tryCommentGroupAndFrame(fset *token.FileSet, cg *ast.CommentGroup, f frame) *behaviors.ConformanceData { +func tryCommentGroupAndFrame(fset *token.FileSet, cg *ast.CommentGroup, f frame) *ConformanceData { if !shouldProcessCommentGroup(fset, cg, f) { return nil } @@ -275,10 +290,10 @@ func shouldProcessCommentGroup(fset *token.FileSet, cg *ast.CommentGroup, f fram return lineDiff > 0 && lineDiff <= conformanceCommentsLineWindow } -func commentToConformanceData(comment string) *behaviors.ConformanceData { +func commentToConformanceData(comment string) *ConformanceData { lines := strings.Split(comment, "\n") descLines := []string{} - cd := &behaviors.ConformanceData{} + cd := &ConformanceData{} var curLine string for _, line := range lines { line = strings.TrimSpace(line) @@ -300,17 +315,9 @@ func commentToConformanceData(comment string) *behaviors.ConformanceData { descLines = append(descLines, sline[1]) continue } - if sline := regexp.MustCompile("^Behaviors\\s*:\\s*").Split(line, -1); len(sline) == 2 { - curLine = "Behaviors" - continue - } // Line has no header - if curLine == "Behaviors" { - if sline := regexp.MustCompile("^-\\s").Split(line, -1); len(sline) == 2 { - cd.Behaviors = append(cd.Behaviors, sline[1]) - } - } else if curLine == "Description" { + if curLine == "Description" { descLines = append(descLines, line) } } diff --git a/test/conformance/walk_test.go b/test/conformance/walk_test.go index f1b4d3ec430..7d1f105168a 100644 --- a/test/conformance/walk_test.go +++ b/test/conformance/walk_test.go @@ -20,8 +20,6 @@ import ( "fmt" "reflect" "testing" - - "k8s.io/kubernetes/test/conformance/behaviors" ) func TestConformance(t *testing.T) { @@ -30,7 +28,7 @@ func TestConformance(t *testing.T) { filename string code string targetFrame frame - output *behaviors.ConformanceData + output *ConformanceData }{ { desc: "Grabs comment above test", @@ -47,7 +45,7 @@ func TestConformance(t *testing.T) { */ framework.ConformanceIt("validates describe with ConformanceIt", func() {}) })`, - output: &behaviors.ConformanceData{ + output: &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.`, @@ -67,7 +65,7 @@ func TestConformance(t *testing.T) { framework.ConformanceIt("should work", func() {}) }) })`, - output: &behaviors.ConformanceData{ + output: &ConformanceData{ URL: "https://github.com/kubernetes/kubernetes/tree/master/e2e/foo.go#L8", TestName: "Test with spaces", Description: `Should pick up testname even if it is not within 3 spaces even when executed from memory.`, @@ -91,7 +89,7 @@ func TestConformance(t *testing.T) { framework.ConformanceIt("should work", func() {}) }) })`, - output: &behaviors.ConformanceData{ + output: &ConformanceData{ URL: "https://github.com/kubernetes/kubernetes/tree/master/e2e/foo.go#L13", TestName: "Second test", Description: `Should target the correct test/comment based on the line numbers`, @@ -114,7 +112,7 @@ func TestConformance(t *testing.T) { framework.ConformanceIt("should work", func() {}) }) })`, - output: &behaviors.ConformanceData{ + output: &ConformanceData{ URL: "https://github.com/kubernetes/kubernetes/tree/master/e2e/foo.go#L8", TestName: "First test", Description: `Should target the correct test/comment based on the line numbers`, @@ -141,7 +139,7 @@ func TestCommentToConformanceData(t *testing.T) { tcs := []struct { desc string input string - expected *behaviors.ConformanceData + expected *ConformanceData }{ { desc: "Empty comment leads to nil", @@ -154,19 +152,11 @@ func TestCommentToConformanceData(t *testing.T) { }, { desc: "Testname but no Release does not result in nil", input: "Testname: mytest\nDescription: foo", - expected: &behaviors.ConformanceData{TestName: "mytest", Description: "foo"}, + expected: &ConformanceData{TestName: "mytest", Description: "foo"}, }, { desc: "All fields parsed and newlines and whitespace removed from description", input: "Release: v1.1\n\t\tTestname: mytest\n\t\tDescription: foo\n\t\tbar\ndone", - expected: &behaviors.ConformanceData{TestName: "mytest", Release: "v1.1", Description: "foo bar done"}, - }, { - desc: "Behaviors are read", - input: "Testname: behaviors\nBehaviors:\n- should behave\n- second behavior", - expected: &behaviors.ConformanceData{TestName: "behaviors", Behaviors: []string{"should behave", "second behavior"}}, - }, { - desc: "Multiple behaviors are parsed", - input: "Testname: behaviors2\nBehaviors:\n- first behavior\n- second behavior", - expected: &behaviors.ConformanceData{TestName: "behaviors2", Behaviors: []string{"first behavior", "second behavior"}}, + expected: &ConformanceData{TestName: "mytest", Release: "v1.1", Description: "foo bar done"}, }, } diff --git a/test/e2e/common/pods.go b/test/e2e/common/pods.go index 07b4ee36aef..e377a77722c 100644 --- a/test/e2e/common/pods.go +++ b/test/e2e/common/pods.go @@ -339,9 +339,6 @@ var _ = framework.KubeDescribe("Pods", func() { Release: v1.9 Testname: Pods, update Description: Create a Pod with a unique label. Query for the Pod with the label as selector MUST be successful. Update the pod to change the value of the Label. Query for the Pod with the new value for the label MUST be successful. - Behaviors: - - pod/spec/label/create - - pod/spec/label/patch */ framework.ConformanceIt("should be updated [NodeConformance]", func() { ginkgo.By("creating the pod") diff --git a/test/e2e/node/pods.go b/test/e2e/node/pods.go index 23c5e82e1da..93661f75b3f 100644 --- a/test/e2e/node/pods.go +++ b/test/e2e/node/pods.go @@ -168,8 +168,6 @@ var _ = SIGDescribe("Pods Extended", func() { Release: v1.9 Testname: Pods, QOS Description: Create a Pod with CPU and Memory request and limits. Pod status MUST have QOSClass set to PodQOSGuaranteed. - Behaviors: - - pod/spec/container/resources */ framework.ConformanceIt("should be set on Pods with matching resource requests and limits for memory and cpu", func() { ginkgo.By("creating the pod") From 6b8c8e44d8a7fb459631f242fcfa14e63d19f755 Mon Sep 17 00:00:00 2001 From: Jefftree Date: Wed, 23 Sep 2020 14:27:49 -0700 Subject: [PATCH 2/2] Update bazel --- test/conformance/BUILD | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/conformance/BUILD b/test/conformance/BUILD index bb9003038c0..a387ab2c366 100644 --- a/test/conformance/BUILD +++ b/test/conformance/BUILD @@ -9,7 +9,6 @@ go_library( importpath = "k8s.io/kubernetes/test/conformance", visibility = ["//visibility:private"], deps = [ - "//test/conformance/behaviors:go_default_library", "//vendor/github.com/onsi/ginkgo/types:go_default_library", "//vendor/gopkg.in/yaml.v2:go_default_library", ], @@ -79,7 +78,6 @@ go_test( srcs = ["walk_test.go"], data = glob(["testdata/**"]), embed = [":go_default_library"], - deps = ["//test/conformance/behaviors:go_default_library"], ) genrule(