From 47d02070ba29585dcb5b50f8130d0fa25c74ab68 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Sat, 18 Oct 2025 22:47:34 +0200 Subject: [PATCH 1/2] E2E: remove unnecessary trailing spaces in test names The spaces are unnecessary because Ginkgo adds spaces automatically. This was detected before only for tests using the wrapper functions, now it also gets detected for ginkgo methods. --- test/e2e/auth/projected_clustertrustbundle.go | 2 +- test/e2e/common/node/image_credential_provider.go | 2 +- test/e2e/network/proxy.go | 4 ++-- test/e2e/network/service.go | 4 ++-- test/e2e/node/events.go | 2 +- test/e2e/node/kubelet.go | 2 +- test/e2e/node/pods.go | 2 +- test/e2e/node/runtimeclass.go | 2 +- test/e2e/storage/non_graceful_node_shutdown.go | 2 +- test/e2e/storage/persistent_volumes.go | 2 +- test/e2e_node/hugepages_test.go | 4 ++-- test/e2e_node/mirror_pod_grace_period_test.go | 2 +- test/e2e_node/mirror_pod_test.go | 4 ++-- 13 files changed, 17 insertions(+), 17 deletions(-) diff --git a/test/e2e/auth/projected_clustertrustbundle.go b/test/e2e/auth/projected_clustertrustbundle.go index 8b684cf1d85..634390b8f73 100644 --- a/test/e2e/auth/projected_clustertrustbundle.go +++ b/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 diff --git a/test/e2e/common/node/image_credential_provider.go b/test/e2e/common/node/image_credential_provider.go index e6c43afb39d..615356bca4e 100644 --- a/test/e2e/common/node/image_credential_provider.go +++ b/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()) diff --git a/test/e2e/network/proxy.go b/test/e2e/network/proxy.go index 5506dcea9be..0897d71a291 100644 --- a/test/e2e/network/proxy.go +++ b/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/: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///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 diff --git a/test/e2e/network/service.go b/test/e2e/network/service.go index 58708f11eb8..4a783645505 100644 --- a/test/e2e/network/service.go +++ b/test/e2e/network/service.go @@ -902,7 +902,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) @@ -3675,7 +3675,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) diff --git a/test/e2e/node/events.go b/test/e2e/node/events.go index cc1c3e70d87..cd3cb49c9b2 100644 --- a/test/e2e/node/events.go +++ b/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) diff --git a/test/e2e/node/kubelet.go b/test/e2e/node/kubelet.go index d02fa56bcc0..23c699187f1 100644 --- a/test/e2e/node/kubelet.go +++ b/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) diff --git a/test/e2e/node/pods.go b/test/e2e/node/pods.go index 2cbf75c6542..de5039b1fea 100644 --- a/test/e2e/node/pods.go +++ b/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{ diff --git a/test/e2e/node/runtimeclass.go b/test/e2e/node/runtimeclass.go index bf65dacae43..18360cef37b 100644 --- a/test/e2e/node/runtimeclass.go +++ b/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) } diff --git a/test/e2e/storage/non_graceful_node_shutdown.go b/test/e2e/storage/non_graceful_node_shutdown.go index aebc009d730..ef5e73c2a01 100644 --- a/test/e2e/storage/non_graceful_node_shutdown.go +++ b/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() diff --git a/test/e2e/storage/persistent_volumes.go b/test/e2e/storage/persistent_volumes.go index 44761f6df07..f6dd3c73e6b 100644 --- a/test/e2e/storage/persistent_volumes.go +++ b/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) diff --git a/test/e2e_node/hugepages_test.go b/test/e2e_node/hugepages_test.go index 1e344d2b26d..156ae11e3e5 100644 --- a/test/e2e_node/hugepages_test.go +++ b/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, diff --git a/test/e2e_node/mirror_pod_grace_period_test.go b/test/e2e_node/mirror_pod_grace_period_test.go index 7934b4b9486..5ac4c57d802 100644 --- a/test/e2e_node/mirror_pod_grace_period_test.go +++ b/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 diff --git a/test/e2e_node/mirror_pod_test.go b/test/e2e_node/mirror_pod_test.go index 3f598c243f1..445aea98312 100644 --- a/test/e2e_node/mirror_pod_test.go +++ b/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() { }) From e4ab523161c8ae3c0b9a22010e07f135164fd24f Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Sat, 18 Oct 2025 16:24:57 +0200 Subject: [PATCH 2/2] E2E framework: make usage of Ginkgo wrappers optional Previously it was necessary to use the Ginkgo wrappers when using any of the custom arguments like WithSlow(). Now the hook within Ginkgo for modifying arguments is used such that e.g. the original ginkgo.It also works. --- test/e2e/framework/ginkgowrapper.go | 137 ++++++++++++++++++++-------- 1 file changed, 101 insertions(+), 36 deletions(-) diff --git a/test/e2e/framework/ginkgowrapper.go b/test/e2e/framework/ginkgowrapper.go index 9d82e4e4883..fb3e1cbd2d1 100644 --- a/test/e2e/framework/ginkgowrapper.go +++ b/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() }