Merge pull request #135774 from pohly/e2e-framework-ginkgo-wrappers

E2E framework: make usage of Ginkgo wrappers optional

test/e2e/auth/projected_clustertrustbundle.go

@@ -162,7 +162,7 @@ var _ = SIGDescribe(framework.WithFeatureGate(features.ClusterTrustBundle), fram
 		}
 	})
 
-	ginkgo.Describe("should prevent a pod from starting if: ", func() {
+	ginkgo.Describe("should prevent a pod from starting if:", func() {
 
 		for _, tt := range []struct {
 			name string

test/e2e/common/node/image_credential_provider.go

@@ -48,7 +48,7 @@ var _ = SIGDescribe("ImageCredentialProvider", feature.KubeletCredentialProvider
 		Testname: Test kubelet image pull with external credential provider plugins
 		Description: Create Pod with an image from a private registry. This test assumes that the kubelet credential provider plugin is enabled for the registry hosting imageutils.AgnhostPrivate.
 	*/
-	ginkgo.It("should be able to create pod with image credentials fetched from external credential provider ", func(ctx context.Context) {
+	ginkgo.It("should be able to create pod with image credentials fetched from external credential provider", func(ctx context.Context) {
 		privateimage := imageutils.GetConfig(imageutils.AgnhostPrivate)
 		name := "pod-auth-image-" + string(uuid.NewUUID())
 

test/e2e/framework/ginkgowrapper.go

@@ -128,10 +128,12 @@ func AnnotatedLocationWithOffset(annotation string, offset int) types.CodeLocati
 // the SIG name as annotation. The parameter should be lowercase with
 // no spaces and no sig- or SIG- prefix.
 func SIGDescribe(sig string) func(...interface{}) bool {
+	ginkgo.GinkgoHelper()
 	if !sigRE.MatchString(sig) || strings.HasPrefix(sig, "sig-") {
-		RecordBug(NewBug(fmt.Sprintf("SIG label must be lowercase, no spaces and no sig- prefix, got instead: %q", sig), 1))
+		RecordBug(NewBug(fmt.Sprintf("SIG label must be lowercase, no spaces and no sig- prefix, got instead: %q", sig), 0))
 	}
 	return func(args ...interface{}) bool {
+		ginkgo.GinkgoHelper()
 		args = append([]interface{}{WithLabel("sig-" + sig)}, args...)
 		return registerInSuite(ginkgo.Describe, args)
 	}
@@ -145,58 +147,127 @@ func ConformanceIt(args ...interface{}) bool {
 	return It(args...)
 }
 
-// It is a wrapper around [ginkgo.It] which supports framework With* labels as
-// optional arguments in addition to those already supported by ginkgo itself,
-// like [ginkgo.Label] and [ginkgo.Offset].
-//
+// It is a wrapper around [ginkgo.It] which removes the requirement
+// to start parameters with a text string.
 // 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(args ...interface{}) bool {
+	ginkgo.GinkgoHelper()
 	return registerInSuite(ginkgo.It, args)
 }
 
 // It is a shorthand for the corresponding package function.
 func (f *Framework) It(args ...interface{}) bool {
+	ginkgo.GinkgoHelper()
 	return registerInSuite(ginkgo.It, args)
 }
 
-// Describe is a wrapper around [ginkgo.Describe] which supports framework
-// With* labels as optional arguments in addition to those already supported by
-// ginkgo itself, like [ginkgo.Label] and [ginkgo.Offset].
-//
+// Describe is a wrapper around [ginkgo.Describe] which removes the requirement
+// to start parameters with a text string.
 // 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(args ...interface{}) bool {
+	ginkgo.GinkgoHelper()
 	return registerInSuite(ginkgo.Describe, args)
 }
 
 // Describe is a shorthand for the corresponding package function.
 func (f *Framework) Describe(args ...interface{}) bool {
+	ginkgo.GinkgoHelper()
 	return registerInSuite(ginkgo.Describe, args)
 }
 
-// Context is a wrapper around [ginkgo.Context] which supports framework With*
-// labels as optional arguments in addition to those already supported by
-// ginkgo itself, like [ginkgo.Label] and [ginkgo.Offset].
-//
+// Context is a wrapper around [ginkgo.Context] which removes the requirement
+// to start parameters with a text string.
 // 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(args ...interface{}) bool {
+	ginkgo.GinkgoHelper()
 	return registerInSuite(ginkgo.Context, args)
 }
 
 // Context is a shorthand for the corresponding package function.
 func (f *Framework) Context(args ...interface{}) bool {
+	ginkgo.GinkgoHelper()
 	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(string, ...interface{}) bool, args []interface{}) bool {
-	var ginkgoArgs []interface{}
 	var offset ginkgo.Offset
+	for arg := range allArgs(args) {
+		if o, ok := arg.(ginkgo.Offset); ok {
+			offset = o
+		}
+	}
+	offset += 2 // This function and the top-level wrapper.
+	return ginkgoCall("", args, offset)
+}
+
+// allArgs produces an iterator which handles nesting without flattening the slices.
+func allArgs(args []any) func(yield func(arg any) bool) {
+	return func(yield func(arg any) bool) {
+		iterArgs(args, yield)
+	}
+}
+
+// iterArgs descends recursively into []any and calls yield for all other arguments.
+func iterArgs(args []any, yield func(arg any) bool) bool {
+	for _, arg := range args {
+		switch arg := arg.(type) {
+		case []any:
+			if !iterArgs(arg, yield) {
+				return false
+			}
+		default:
+			if !yield(arg) {
+				return false
+			}
+		}
+	}
+	return true
+}
+
+// If the framework is used, the user might also use our special test
+// arguments.  To ensure that test registration works we inject our code into
+// the test tree construction. Therefore it doesn't matter anymore whether
+// ginkgo.It or framework.It is used.
+//
+// Code flow is as follows:
+//   - init registers transformGinkgoNodeArgs.
+//     From now on, all ginkgo.Describe/Context/It invocations invoke
+//     transformGinkgoNodeArgs. transformGinkgoNodeArgs replaces
+//     our special arguments with something that Ginkgo can handle.
+//   - Top-level Describe calls register callbacks with more test definitions.
+//     Callbacks are not invoked yet.
+//   - Argument parsing.
+//   - Ginkgo invokes the callbacks recursively to get a complete tree of tests.
+//   - Ginkgo runs those tests.
+//
+// This init is guaranteed to happen before our special argument functions
+// can be called because this is the init code of their package.
+// Code which does not use the framework might call ginkgo.Describe before
+// transformGinkgoNodeArgs is registered. This is not a problem because none of the parameters can be special.
+func init() {
+	ginkgo.AddTreeConstructionNodeArgsTransformer(transformGinkgoNodeArgs)
+}
+
+func transformGinkgoNodeArgs(nodeType types.NodeType, offset ginkgo.Offset, text string, args []any) (string, []any, []error) {
+	text, args = expandGinkgoArgs(offset+1, text, args)
+	return text, args, nil
+}
+
+// expandGinkgoArgs concatenates all strings and translates our custom
+// arguments into something that Ginkgo can handle.
+func expandGinkgoArgs(offset ginkgo.Offset, text string, args []any) (string, []any) {
+	var ginkgoArgs []interface{}
 	var texts []string
 
+	if text != "" {
+		texts = append(texts, text)
+	}
+
 	addLabel := func(label string) {
 		texts = append(texts, fmt.Sprintf("[%s]", label))
 		ginkgoArgs = append(ginkgoArgs, ginkgo.Label(label))
@@ -230,8 +301,6 @@ func registerInSuite(ginkgoCall func(string, ...interface{}) bool, args []interf
 				// run and then make the run longer overall.
 				ginkgoArgs = append(ginkgoArgs, ginkgo.SpecPriority(1))
 			}
-		case ginkgo.Offset:
-			offset = arg
 		case string:
 			if arg == "" {
 				haveEmptyStrings = true
@@ -241,9 +310,7 @@ func registerInSuite(ginkgoCall func(string, ...interface{}) bool, args []interf
 			ginkgoArgs = append(ginkgoArgs, arg)
 		}
 	}
-	offset += 2 // This function and its direct caller.
 
-	// Now that we have the final offset, we can record bugs.
 	if haveEmptyStrings {
 		RecordBug(NewBug("empty strings as separators are unnecessary and need to be removed", int(offset)))
 	}
@@ -256,9 +323,8 @@ func registerInSuite(ginkgoCall func(string, ...interface{}) bool, args []interf
 		}
 	}
 
-	ginkgoArgs = append(ginkgoArgs, offset)
-	text := strings.Join(texts, " ")
-	return ginkgoCall(text, ginkgoArgs...)
+	text = strings.Join(texts, " ")
+	return text, ginkgoArgs
 }
 
 var (
@@ -381,8 +447,8 @@ func recordTextBug(location types.CodeLocation, message string) {
 }
 
 // WithFeature specifies that a certain test or group of tests only works
-// with a feature available. The return value must be passed as additional
-// argument to [framework.It], [framework.Describe], [framework.Context].
+// with a feature available. The return value may be passed as additional
+// argument to the framework wrappers and the Ginkgo functions directly.
 //
 // The feature must be listed in ValidFeatures.
 func WithFeature(name Feature) interface{} {
@@ -403,8 +469,8 @@ func withFeature(name Feature) interface{} {
 
 // WithFeatureGate specifies that a certain test or group of tests depends on a
 // feature gate and the corresponding API group (if there is one)
-// being enabled. The return value must be passed as additional
-// argument to [framework.It], [framework.Describe], [framework.Context].
+// being enabled. The return value may be passed as additional
+// argument to the framework wrappers and the Ginkgo functions directly.
 //
 // The feature gate must be listed in
 // [k8s.io/apiserver/pkg/util/feature.DefaultMutableFeatureGate]. Once a
@@ -461,8 +527,8 @@ func withFeatureGate(featureGate featuregate.Feature) interface{} {
 }
 
 // WithEnvironment specifies that a certain test or group of tests only works
-// in a certain environment. The return value must be passed as additional
-// argument to [framework.It], [framework.Describe], [framework.Context].
+// in a certain environment. The return value may be passed as additional
+// argument to the framework wrappers and the Ginkgo functions directly.
 //
 // The environment must be listed in ValidEnvironments.
 func WithEnvironment(name Environment) interface{} {
@@ -482,9 +548,8 @@ func withEnvironment(name Environment) interface{} {
 }
 
 // WithConformance specifies that a certain test or group of tests must pass in
-// all conformant Kubernetes clusters. The return value must be passed as
-// additional argument to [framework.It], [framework.Describe],
-// [framework.Context].
+// all conformant Kubernetes clusters. The return value may be passed as additional
+// argument to the framework wrappers and the Ginkgo functions directly.
 func WithConformance() interface{} {
 	return withConformance()
 }
@@ -500,8 +565,8 @@ func withConformance() interface{} {
 
 // WithNodeConformance specifies that a certain test or group of tests for node
 // functionality that does not depend on runtime or Kubernetes distro specific
-// behavior. The return value must be passed as additional argument to
-// [framework.It], [framework.Describe], [framework.Context].
+// behavior. The return value may be passed as additional
+// argument to the framework wrappers and the Ginkgo functions directly.
 func WithNodeConformance() interface{} {
 	return withNodeConformance()
 }
@@ -533,8 +598,8 @@ func withDisruptive() interface{} {
 }
 
 // WithSerial specifies that a certain test or group of tests must not run in
-// parallel with other tests. The return value must be passed as additional
-// argument to [framework.It], [framework.Describe], [framework.Context].
+// parallel with other tests. The return value may be passed as additional
+// argument to the framework wrappers and the Ginkgo functions directly.
 //
 // Starting with ginkgo v2, serial and parallel tests can be executed in the
 // same invocation. Ginkgo itself will ensure that the serial tests run
@@ -554,8 +619,8 @@ func withSerial() interface{} {
 
 // WithSlow specifies that a certain test, or each test within a group of
 // tests, is slow (is expected to take longer than 5 minutes to run in CI).
-// The return value must be passed as additional argument to [framework.It],
-// [framework.Describe], [framework.Context].
+// The return value may be passed as additional
+// argument to the framework wrappers and the Ginkgo functions directly.
 func WithSlow() interface{} {
 	return withSlow()
 }

test/e2e/network/proxy.go

@@ -84,13 +84,13 @@ var _ = common.SIGDescribe("Proxy", func() {
 			Test for Proxy, logs port endpoint
 			Select any node in the cluster to invoke /proxy/nodes/<nodeip>:10250/logs endpoint. This endpoint MUST be reachable.
 		*/
-		ginkgo.It("should proxy logs on node with explicit kubelet port using proxy subresource ", func(ctx context.Context) { nodeProxyTest(ctx, f, prefix+"/nodes/", ":10250/proxy/logs/") })
+		ginkgo.It("should proxy logs on node with explicit kubelet port using proxy subresource", func(ctx context.Context) { nodeProxyTest(ctx, f, prefix+"/nodes/", ":10250/proxy/logs/") })
 
 		/*
 			Test for Proxy, logs endpoint
 			Select any node in the cluster to invoke /proxy/nodes/<nodeip>//logs endpoint. This endpoint MUST be reachable.
 		*/
-		ginkgo.It("should proxy logs on node using proxy subresource ", func(ctx context.Context) { nodeProxyTest(ctx, f, prefix+"/nodes/", "/proxy/logs/") })
+		ginkgo.It("should proxy logs on node using proxy subresource", func(ctx context.Context) { nodeProxyTest(ctx, f, prefix+"/nodes/", "/proxy/logs/") })
 
 		/*
 			Release: v1.9

test/e2e/network/service.go

@@ -903,7 +903,7 @@ var _ = common.SIGDescribe("Services", func() {
 		framework.ExpectNoError(err)
 	})
 
-	ginkgo.It("should be updated after adding or deleting ports ", func(ctx context.Context) {
+	ginkgo.It("should be updated after adding or deleting ports", func(ctx context.Context) {
 		serviceName := "edit-port-test"
 		ns := f.Namespace.Name
 		jig := e2eservice.NewTestJig(cs, ns, serviceName)
@@ -3698,7 +3698,7 @@ var _ = common.SIGDescribe("Services", func() {
 		Testname: Service, same ports with different protocols on a Load Balancer Service
 		Description: Create a LoadBalancer service with two ports that have the same value but use different protocols. Add a Pod that listens on both ports. The Pod must be reachable via the ClusterIP and both ports
 	*/
-	ginkgo.It("should serve endpoints on same port and different protocol for internal traffic on Type LoadBalancer ", func(ctx context.Context) {
+	ginkgo.It("should serve endpoints on same port and different protocol for internal traffic on Type LoadBalancer", func(ctx context.Context) {
 		serviceName := "multiprotocol-lb-test"
 		ns := f.Namespace.Name
 		jig := e2eservice.NewTestJig(cs, ns, serviceName)

test/e2e/node/events.go

@@ -40,7 +40,7 @@ var _ = SIGDescribe("Events", func() {
 	f := framework.NewDefaultFramework("events")
 	f.NamespacePodSecurityLevel = admissionapi.LevelBaseline
 
-	ginkgo.It("should be sent by kubelets and the scheduler about pods scheduling and running ", func(ctx context.Context) {
+	ginkgo.It("should be sent by kubelets and the scheduler about pods scheduling and running", func(ctx context.Context) {
 
 		podClient := f.ClientSet.CoreV1().Pods(f.Namespace.Name)
 

test/e2e/node/kubelet.go

@@ -492,7 +492,7 @@ var _ = SIGDescribe("kubelet", func() {
 			returns the kubelet logs
 		*/
 
-		ginkgo.It("should return the kubelet logs ", func(ctx context.Context) {
+		ginkgo.It("should return the kubelet logs", func(ctx context.Context) {
 			ginkgo.By("Starting the command")
 			tk := e2ekubectl.NewTestKubeconfig(framework.TestContext.CertDir, framework.TestContext.Host, framework.TestContext.KubeConfig, framework.TestContext.KubeContext, framework.TestContext.KubectlPath, ns)
 

test/e2e/node/pods.go

@@ -1136,7 +1136,7 @@ var _ = SIGDescribe("Pod Extended (RestartAllContainers)", framework.WithFeature
 			framework.ExpectNoError(e2epod.WaitForContainerRunning(ctx, f.ClientSet, f.Namespace.Name, podName, "regular", 3*time.Minute))
 		})
 
-		ginkgo.It("should restart all containers on a previously restarted regular container exit ", func(ctx context.Context) {
+		ginkgo.It("should restart all containers on a previously restarted regular container exit", func(ctx context.Context) {
 			podName := "restart-rules-exit-code-" + string(uuid.NewUUID())
 			pod := &v1.Pod{
 				ObjectMeta: metav1.ObjectMeta{

test/e2e/node/runtimeclass.go

@@ -128,7 +128,7 @@ var _ = SIGDescribe("RuntimeClass", func() {
 		gomega.Expect(pod.Spec.Tolerations).To(gomega.ContainElement(tolerations[0]))
 	})
 
-	ginkgo.It("should run a Pod requesting a RuntimeClass with scheduling without taints ", func(ctx context.Context) {
+	ginkgo.It("should run a Pod requesting a RuntimeClass with scheduling without taints", func(ctx context.Context) {
 		if err := e2eruntimeclass.NodeSupportsPreconfiguredRuntimeClassHandler(ctx, f); err != nil {
 			e2eskipper.Skipf("Skipping test as node does not have E2E runtime class handler preconfigured in container runtime config: %v", err)
 		}

test/e2e/storage/non_graceful_node_shutdown.go

@@ -78,7 +78,7 @@ var _ = utils.SIGDescribe(framework.WithDisruptive(), "[LinuxOnly] NonGracefulNo
 	})
 
 	ginkgo.Describe("[NonGracefulNodeShutdown] pod that uses a persistent volume via gce pd driver", func() {
-		ginkgo.It("should get immediately rescheduled to a different node after non graceful node shutdown ", func(ctx context.Context) {
+		ginkgo.It("should get immediately rescheduled to a different node after non graceful node shutdown", func(ctx context.Context) {
 			// Install gce pd csi driver
 			ginkgo.By("deploying csi gce-pd driver")
 			driver := drivers.InitGcePDCSIDriver()

test/e2e/storage/persistent_volumes.go

@@ -180,7 +180,7 @@ var _ = utils.SIGDescribe("PersistentVolumes", func() {
 			// Create an nfs PV, then a claim that matches the PV, and a pod that
 			// contains the claim. Verify that the PV and PVC bind correctly, and
 			// that the pod can write to the nfs volume.
-			ginkgo.It("should create a non-pre-bound PV and PVC: test write access ", func(ctx context.Context) {
+			ginkgo.It("should create a non-pre-bound PV and PVC: test write access", func(ctx context.Context) {
 				pv, pvc, err = e2epv.CreatePVPVC(ctx, c, f.Timeouts, pvConfig, pvcConfig, ns, false)
 				framework.ExpectNoError(err)
 				completeTest(ctx, f, c, ns, pv, pvc)

test/e2e_node/hugepages_test.go

@@ -387,7 +387,7 @@ var _ = SIGDescribe("HugePages", framework.WithSerial(), feature.HugePages, func
 			waitForHugepages(f, ctx, hugepages)
 		})
 
-		ginkgo.Context("with the resources requests that contain only one hugepages resource ", func() {
+		ginkgo.Context("with the resources requests that contain only one hugepages resource", func() {
 			ginkgo.Context("with the backward compatible API", func() {
 				ginkgo.BeforeEach(func() {
 					expectedHugepageLimits = v1.ResourceList{
@@ -461,7 +461,7 @@ var _ = SIGDescribe("HugePages", framework.WithSerial(), feature.HugePages, func
 			})
 		})
 
-		ginkgo.Context("with the resources requests that contain multiple hugepages resources ", func() {
+		ginkgo.Context("with the resources requests that contain multiple hugepages resources", func() {
 			ginkgo.BeforeEach(func() {
 				hugepages = map[string]int{
 					hugepagesResourceName2Mi: 5,

test/e2e_node/mirror_pod_grace_period_test.go

@@ -42,7 +42,7 @@ import (
 var _ = SIGDescribe("MirrorPodWithGracePeriod", func() {
 	f := framework.NewDefaultFramework("mirror-pod-with-grace-period")
 	f.NamespacePodSecurityLevel = admissionapi.LevelBaseline
-	ginkgo.Context("when create a mirror pod ", func() {
+	ginkgo.Context("when create a mirror pod", func() {
 		var ns, podPath, staticPodName, mirrorPodName string
 		ginkgo.BeforeEach(func(ctx context.Context) {
 			ns = f.Namespace.Name

test/e2e_node/mirror_pod_test.go

@@ -51,7 +51,7 @@ import (
 var _ = SIGDescribe("MirrorPod", func() {
 	f := framework.NewDefaultFramework("mirror-pod")
 	f.NamespacePodSecurityLevel = admissionapi.LevelPrivileged
-	ginkgo.Context("when create a mirror pod ", func() {
+	ginkgo.Context("when create a mirror pod", func() {
 		var ns, podPath, staticPodName, mirrorPodName string
 		ginkgo.BeforeEach(func(ctx context.Context) {
 			ns = f.Namespace.Name
@@ -148,7 +148,7 @@ var _ = SIGDescribe("MirrorPod", func() {
 			}, 2*time.Minute, time.Second*4).Should(gomega.BeNil())
 		})
 	})
-	ginkgo.Context("when create a mirror pod without changes ", func() {
+	ginkgo.Context("when create a mirror pod without changes", func() {
 		var ns, podPath, staticPodName, mirrorPodName string
 		ginkgo.BeforeEach(func() {
 		})