From 81b960efefebbc2cbcb8fd6770d0ef7af30be318 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Tue, 17 Oct 2023 16:51:41 +0200 Subject: [PATCH] e2e framework: allow random ordering of tags and text, fix some functions There are some tests which want to insert a tag before the main Describe text, for example: sigDescribe("[Feature:Windows] Cpu Resources [Serial]", skipUnlessWindows(func() { ... }) In order to support this without change existing test names, it must be possible to do this instead: sigDescribe(feature.Windows, "Cpu Resources", framework.WithSerial(), skipUnlessWindows(func() { ... }) There are similar examples for the other functions. While at it, replace one left-over panic with ReportBug and add the missing `NodeFeature:` prefix. --- test/e2e/framework/ginkgowrapper.go | 56 ++++++++----------- .../framework/internal/unittests/bugs/bugs.go | 27 +++++---- 2 files changed, 39 insertions(+), 44 deletions(-) diff --git a/test/e2e/framework/ginkgowrapper.go b/test/e2e/framework/ginkgowrapper.go index ed7b9432f05..f783e1778c9 100644 --- a/test/e2e/framework/ginkgowrapper.go +++ b/test/e2e/framework/ginkgowrapper.go @@ -127,27 +127,22 @@ func AnnotatedLocationWithOffset(annotation string, offset int) types.CodeLocati // SIGDescribe returns a wrapper function for ginkgo.Describe which injects // the SIG name as annotation. The parameter should be lowercase with // no spaces and no sig- or SIG- prefix. -func SIGDescribe(sig string) func(string, ...interface{}) bool { +func SIGDescribe(sig string) func(...interface{}) bool { if !sigRE.MatchString(sig) || strings.HasPrefix(sig, "sig-") { - panic(fmt.Sprintf("SIG label must be lowercase, no spaces and no sig- prefix, got instead: %q", sig)) + RecordBug(NewBug(fmt.Sprintf("SIG label must be lowercase, no spaces and no sig- prefix, got instead: %q", sig), 1)) } - return func(text string, args ...interface{}) bool { - args = append(args, ginkgo.Label("sig-"+sig)) - if text == "" { - text = fmt.Sprintf("[sig-%s]", sig) - } else { - text = fmt.Sprintf("[sig-%s] %s", sig, text) - } - return registerInSuite(ginkgo.Describe, text, args) + return func(args ...interface{}) bool { + args = append([]interface{}{WithLabel("sig-" + sig)}, args...) + return registerInSuite(ginkgo.Describe, args) } } var sigRE = regexp.MustCompile(`^[a-z]+(-[a-z]+)*$`) // ConformanceIt is wrapper function for ginkgo It. Adds "[Conformance]" tag and makes static analysis easier. -func ConformanceIt(text string, args ...interface{}) bool { +func ConformanceIt(args ...interface{}) bool { args = append(args, ginkgo.Offset(1), WithConformance()) - return It(text, args...) + return It(args...) } // It is a wrapper around [ginkgo.It] which supports framework With* labels as @@ -156,13 +151,13 @@ func ConformanceIt(text string, args ...interface{}) bool { // // Text and arguments may be mixed. The final text is a concatenation // of the text arguments and special tags from the With functions. -func It(text string, args ...interface{}) bool { - return registerInSuite(ginkgo.It, text, args) +func It(args ...interface{}) bool { + return registerInSuite(ginkgo.It, args) } // It is a shorthand for the corresponding package function. -func (f *Framework) It(text string, args ...interface{}) bool { - return registerInSuite(ginkgo.It, text, args) +func (f *Framework) It(args ...interface{}) bool { + return registerInSuite(ginkgo.It, args) } // Describe is a wrapper around [ginkgo.Describe] which supports framework @@ -171,13 +166,13 @@ func (f *Framework) It(text string, args ...interface{}) bool { // // Text and arguments may be mixed. The final text is a concatenation // of the text arguments and special tags from the With functions. -func Describe(text string, args ...interface{}) bool { - return registerInSuite(ginkgo.Describe, text, args) +func Describe(args ...interface{}) bool { + return registerInSuite(ginkgo.Describe, args) } // Describe is a shorthand for the corresponding package function. -func (f *Framework) Describe(text string, args ...interface{}) bool { - return registerInSuite(ginkgo.Describe, text, args) +func (f *Framework) Describe(args ...interface{}) bool { + return registerInSuite(ginkgo.Describe, args) } // Context is a wrapper around [ginkgo.Context] which supports framework With* @@ -186,24 +181,21 @@ func (f *Framework) Describe(text string, args ...interface{}) bool { // // Text and arguments may be mixed. The final text is a concatenation // of the text arguments and special tags from the With functions. -func Context(text string, args ...interface{}) bool { - return registerInSuite(ginkgo.Context, text, args) +func Context(args ...interface{}) bool { + return registerInSuite(ginkgo.Context, args) } // Context is a shorthand for the corresponding package function. -func (f *Framework) Context(text string, args ...interface{}) bool { - return registerInSuite(ginkgo.Context, text, args) +func (f *Framework) Context(args ...interface{}) bool { + return registerInSuite(ginkgo.Context, args) } // registerInSuite is the common implementation of all wrapper functions. It // expects to be called through one intermediate wrapper. -func registerInSuite(ginkgoCall func(text string, args ...interface{}) bool, text string, args []interface{}) bool { +func registerInSuite(ginkgoCall func(string, ...interface{}) bool, args []interface{}) bool { var ginkgoArgs []interface{} var offset ginkgo.Offset var texts []string - if text != "" { - texts = append(texts, text) - } addLabel := func(label string) { texts = append(texts, fmt.Sprintf("[%s]", label)) @@ -214,7 +206,7 @@ func registerInSuite(ginkgoCall func(text string, args ...interface{}) bool, tex for _, arg := range args { switch arg := arg.(type) { case label: - fullLabel := strings.Join(arg.parts, ": ") + fullLabel := strings.Join(arg.parts, ":") addLabel(fullLabel) if arg.extra != "" { addLabel(arg.extra) @@ -249,7 +241,7 @@ func registerInSuite(ginkgoCall func(text string, args ...interface{}) bool, tex } ginkgoArgs = append(ginkgoArgs, offset) - text = strings.Join(texts, " ") + text := strings.Join(texts, " ") return ginkgoCall(text, ginkgoArgs...) } @@ -349,7 +341,7 @@ func withNodeFeature(name NodeFeature) interface{} { if !ValidNodeFeatures.items.Has(name) { RecordBug(NewBug(fmt.Sprintf("WithNodeFeature: unknown environment %q", name), 2)) } - return newLabel(string(name)) + return newLabel("NodeFeature", string(name)) } // WithConformace specifies that a certain test or group of tests must pass in @@ -456,7 +448,7 @@ func withLabel(label string) interface{} { } type label struct { - // parts get concatenated with ": " to build the full label. + // parts get concatenated with ":" to build the full label. parts []string // extra is an optional fully-formed extra label. extra string diff --git a/test/e2e/framework/internal/unittests/bugs/bugs.go b/test/e2e/framework/internal/unittests/bugs/bugs.go index e84d04d638d..dac80ca9a03 100644 --- a/test/e2e/framework/internal/unittests/bugs/bugs.go +++ b/test/e2e/framework/internal/unittests/bugs/bugs.go @@ -105,6 +105,8 @@ func Describe() { }) }, ) + + framework.SIGDescribe("123") } const ( @@ -118,13 +120,14 @@ ERROR: bugs.go:77: WithFeature: unknown feature "no-such-feature" ERROR: bugs.go:79: WithEnvironment: unknown environment "no-such-env" ERROR: bugs.go:81: WithNodeFeature: unknown environment "no-such-node-env" ERROR: bugs.go:83: WithFeatureGate: the feature gate "no-such-feature-gate" is unknown +ERROR: bugs.go:109: SIG label must be lowercase, no spaces and no sig- prefix, got instead: "123" ERROR: buggy/buggy.go:100: hello world ERROR: some/relative/path/buggy.go:200: with spaces ` // Used by unittests/list-tests. It's sorted by test name, not source code location. ListTestsOutput = `The following spec names can be used with 'ginkgo run --focus/skip': - ../bugs/bugs.go:103: [sig-testing] abc space1 space2 [Feature: no-such-feature] [Feature: feature-foo] [Environment: no-such-env] [Environment: Linux] [no-such-node-env] [node-feature-foo] [FeatureGate: no-such-feature-gate] [FeatureGate: TestAlphaFeature] [Alpha] [FeatureGate: TestBetaFeature] [Beta] [FeatureGate: TestGAFeature] [Conformance] [NodeConformance] [Slow] [Serial] [Disruptive] [custom-label] xyz x [foo] should [bar] - ../bugs/bugs.go:98: [sig-testing] abc space1 space2 [Feature: no-such-feature] [Feature: feature-foo] [Environment: no-such-env] [Environment: Linux] [no-such-node-env] [node-feature-foo] [FeatureGate: no-such-feature-gate] [FeatureGate: TestAlphaFeature] [Alpha] [FeatureGate: TestBetaFeature] [Beta] [FeatureGate: TestGAFeature] [Conformance] [NodeConformance] [Slow] [Serial] [Disruptive] [custom-label] xyz y [foo] should [bar] + ../bugs/bugs.go:103: [sig-testing] abc space1 space2 [Feature:no-such-feature] [Feature:feature-foo] [Environment:no-such-env] [Environment:Linux] [NodeFeature:no-such-node-env] [NodeFeature:node-feature-foo] [FeatureGate:no-such-feature-gate] [FeatureGate:TestAlphaFeature] [Alpha] [FeatureGate:TestBetaFeature] [Beta] [FeatureGate:TestGAFeature] [Conformance] [NodeConformance] [Slow] [Serial] [Disruptive] [custom-label] xyz x [foo] should [bar] + ../bugs/bugs.go:98: [sig-testing] abc space1 space2 [Feature:no-such-feature] [Feature:feature-foo] [Environment:no-such-env] [Environment:Linux] [NodeFeature:no-such-node-env] [NodeFeature:node-feature-foo] [FeatureGate:no-such-feature-gate] [FeatureGate:TestAlphaFeature] [Alpha] [FeatureGate:TestBetaFeature] [Beta] [FeatureGate:TestGAFeature] [Conformance] [NodeConformance] [Slow] [Serial] [Disruptive] [custom-label] xyz y [foo] should [bar] ` @@ -134,22 +137,22 @@ ERROR: some/relative/path/buggy.go:200: with spaces Beta Conformance Disruptive - Environment: Linux - Environment: no-such-env - Feature: feature-foo - Feature: no-such-feature - FeatureGate: TestAlphaFeature - FeatureGate: TestBetaFeature - FeatureGate: TestGAFeature - FeatureGate: no-such-feature-gate + Environment:Linux + Environment:no-such-env + Feature:feature-foo + Feature:no-such-feature + FeatureGate:TestAlphaFeature + FeatureGate:TestBetaFeature + FeatureGate:TestGAFeature + FeatureGate:no-such-feature-gate NodeConformance + NodeFeature:no-such-node-env + NodeFeature:node-feature-foo Serial Slow bar custom-label foo - no-such-node-env - node-feature-foo sig-testing `