From 7805661a934c097a14b3b19f97756f259a83b1d0 Mon Sep 17 00:00:00 2001 From: vr4manta Date: Tue, 1 Apr 2025 11:57:36 -0400 Subject: [PATCH] Update tests-extensions to pick up new vendor filtering --- go.mod | 2 +- go.sum | 4 +- .../pkg/extension/extensiontests/spec.go | 68 +++++++++++++++++-- .../pkg/extension/extensiontests/types.go | 3 + .../pkg/ginkgo/util.go | 40 ++++++++--- vendor/modules.txt | 2 +- 6 files changed, 101 insertions(+), 18 deletions(-) diff --git a/go.mod b/go.mod index 168f173ed6..b3c41f92e6 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( github.com/google/uuid v1.6.0 github.com/onsi/ginkgo/v2 v2.22.2 github.com/onsi/gomega v1.36.2 - github.com/openshift-eng/openshift-tests-extension v0.0.0-20250220212757-b9c4d98a0c45 + github.com/openshift-eng/openshift-tests-extension v0.0.0-20250401163437-6cbf1d70a450 github.com/openshift/api v0.0.0-20250305122440-3e04d3af8c3e github.com/openshift/client-go v0.0.0-20250131180035-f7ec47e2d87a github.com/openshift/cluster-api-actuator-pkg/testutils v0.0.0-20250122171707-86066d47a264 diff --git a/go.sum b/go.sum index 8e41d64428..b90b79bbf3 100644 --- a/go.sum +++ b/go.sum @@ -455,8 +455,8 @@ github.com/opencontainers/runtime-spec v1.2.0 h1:z97+pHb3uELt/yiAWD691HNHQIF07bE github.com/opencontainers/runtime-spec v1.2.0/go.mod h1:jwyrGlmzljRJv/Fgzds9SsS/C5hL+LL3ko9hs6T5lQ0= github.com/opencontainers/selinux v1.11.1 h1:nHFvthhM0qY8/m+vfhJylliSshm8G1jJ2jDMcgULaH8= github.com/opencontainers/selinux v1.11.1/go.mod h1:E5dMC3VPuVvVHDYmi78qvhJp8+M586T4DlDRYpFkyec= -github.com/openshift-eng/openshift-tests-extension v0.0.0-20250220212757-b9c4d98a0c45 h1:hXpbYtP3iTh8oy/RKwKkcMziwchY3fIk95ciczf7cOA= -github.com/openshift-eng/openshift-tests-extension v0.0.0-20250220212757-b9c4d98a0c45/go.mod h1:6gkP5f2HL0meusT0Aim8icAspcD1cG055xxBZ9yC68M= +github.com/openshift-eng/openshift-tests-extension v0.0.0-20250401163437-6cbf1d70a450 h1:F3D3ly8ZFEVoUTwB1/RYkTevtrCuPKBXsLCHW/73BNE= +github.com/openshift-eng/openshift-tests-extension v0.0.0-20250401163437-6cbf1d70a450/go.mod h1:6gkP5f2HL0meusT0Aim8icAspcD1cG055xxBZ9yC68M= github.com/openshift/api v0.0.0-20250305122440-3e04d3af8c3e h1:bEcCutNr5RLU/DudWNs/nlOLBqMxrfsHtVuMttkxvWE= github.com/openshift/api v0.0.0-20250305122440-3e04d3af8c3e/go.mod h1:yk60tHAmHhtVpJQo3TwVYq2zpuP70iJIFDCmeKMIzPw= github.com/openshift/client-go v0.0.0-20250131180035-f7ec47e2d87a h1:duO3JMrUOqVx50QhzxvDeOYIwTNOB8/EEuRLPyvAMBg= diff --git a/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/spec.go b/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/spec.go index 08bae75832..3fc42395d3 100644 --- a/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/spec.go +++ b/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/spec.go @@ -10,9 +10,9 @@ import ( "github.com/google/cel-go/cel" "github.com/google/cel-go/checker/decls" "github.com/google/cel-go/common/types" - "github.com/openshift-eng/openshift-tests-extension/pkg/flags" "github.com/openshift-eng/openshift-tests-extension/pkg/dbtime" + "github.com/openshift-eng/openshift-tests-extension/pkg/flags" ) // Walk iterates over all test specs, and executions the function provided. The test spec can be mutated. @@ -38,6 +38,17 @@ func (specs ExtensionTestSpecs) Select(selectFn SelectFunction) ExtensionTestSpe return filtered } +// MustSelect filters the ExtensionTestSpecs to only those that match the provided SelectFunction. +// if no specs are selected, it will throw an error +func (specs ExtensionTestSpecs) MustSelect(selectFn SelectFunction) (ExtensionTestSpecs, error) { + filtered := specs.Select(selectFn) + if len(filtered) == 0 { + return filtered, fmt.Errorf("no specs selected with specified SelectFunctions") + } + + return filtered, nil +} + // SelectAny filters the ExtensionTestSpecs to only those that match any of the provided SelectFunctions func (specs ExtensionTestSpecs) SelectAny(selectFns []SelectFunction) ExtensionTestSpecs { filtered := ExtensionTestSpecs{} @@ -53,6 +64,17 @@ func (specs ExtensionTestSpecs) SelectAny(selectFns []SelectFunction) ExtensionT return filtered } +// MustSelectAny filters the ExtensionTestSpecs to only those that match any of the provided SelectFunctions. +// if no specs are selected, it will throw an error +func (specs ExtensionTestSpecs) MustSelectAny(selectFns []SelectFunction) (ExtensionTestSpecs, error) { + filtered := specs.SelectAny(selectFns) + if len(filtered) == 0 { + return filtered, fmt.Errorf("no specs selected with specified SelectFunctions") + } + + return filtered, nil +} + // SelectAll filters the ExtensionTestSpecs to only those that match all the provided SelectFunctions func (specs ExtensionTestSpecs) SelectAll(selectFns []SelectFunction) ExtensionTestSpecs { filtered := ExtensionTestSpecs{} @@ -72,6 +94,38 @@ func (specs ExtensionTestSpecs) SelectAll(selectFns []SelectFunction) ExtensionT return filtered } +// MustSelectAll filters the ExtensionTestSpecs to only those that match all the provided SelectFunctions. +// if no specs are selected, it will throw an error +func (specs ExtensionTestSpecs) MustSelectAll(selectFns []SelectFunction) (ExtensionTestSpecs, error) { + filtered := specs.SelectAll(selectFns) + if len(filtered) == 0 { + return filtered, fmt.Errorf("no specs selected with specified SelectFunctions") + } + + return filtered, nil +} + +// ModuleTestsOnly ensures that ginkgo tests from vendored sources aren't selected. +func ModuleTestsOnly() SelectFunction { + return func(spec *ExtensionTestSpec) bool { + for _, cl := range spec.CodeLocations { + if strings.Contains(cl, "/vendor/") { + return false + } + } + + return true + } +} + +// AllTestsIncludingVendored is an alternative to ModuleTestsOnly, which would explicitly opt-in +// to including vendored tests. +func AllTestsIncludingVendored() SelectFunction { + return func(spec *ExtensionTestSpec) bool { + return true + } +} + // NameContains returns a function that selects specs whose name contains the provided string func NameContains(name string) SelectFunction { return func(spec *ExtensionTestSpec) bool { @@ -251,6 +305,7 @@ func (specs ExtensionTestSpecs) Filter(celExprs []string) (ExtensionTestSpecs, e decls.NewVar("name", decls.String), decls.NewVar("originalName", decls.String), decls.NewVar("labels", decls.NewListType(decls.String)), + decls.NewVar("codeLocations", decls.NewListType(decls.String)), decls.NewVar("tags", decls.NewMapType(decls.String, decls.String)), ), ) @@ -267,11 +322,12 @@ func (specs ExtensionTestSpecs) Filter(celExprs []string) (ExtensionTestSpecs, e return nil, err } out, _, err := prg.Eval(map[string]interface{}{ - "name": spec.Name, - "source": spec.Source, - "originalName": spec.OriginalName, - "labels": spec.Labels.UnsortedList(), - "tags": spec.Tags, + "name": spec.Name, + "source": spec.Source, + "originalName": spec.OriginalName, + "labels": spec.Labels.UnsortedList(), + "codeLocations": spec.CodeLocations, + "tags": spec.Tags, }) if err != nil { return nil, fmt.Errorf("error evaluating CEL expression: %v", err) diff --git a/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/types.go b/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/types.go index 445e2d5a86..2ec0444b68 100644 --- a/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/types.go +++ b/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests/types.go @@ -31,6 +31,9 @@ type ExtensionTestSpec struct { // Source is the origin of the test. Source string `json:"source"` + // CodeLocations are the files where the spec originates from. + CodeLocations []string `json:"codeLocations,omitempty"` + // Lifecycle informs the executor whether the test is informing only, and should not cause the // overall job run to fail, or if it's blocking where a failure of the test is fatal. // Informing lifecycle tests can be used temporarily to gather information about a test's stability. diff --git a/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/ginkgo/util.go b/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/ginkgo/util.go index 7984c924d0..2b7f5aa6d1 100644 --- a/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/ginkgo/util.go +++ b/vendor/github.com/openshift-eng/openshift-tests-extension/pkg/ginkgo/util.go @@ -10,9 +10,10 @@ import ( "github.com/onsi/ginkgo/v2" "github.com/onsi/ginkgo/v2/types" "github.com/onsi/gomega" - "github.com/openshift-eng/openshift-tests-extension/pkg/util/sets" "github.com/pkg/errors" + "github.com/openshift-eng/openshift-tests-extension/pkg/util/sets" + ext "github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests" ) @@ -40,8 +41,12 @@ func configureGinkgo() (*types.SuiteConfig, *types.ReporterConfig, error) { return &suiteConfig, &reporterConfig, nil } -func BuildExtensionTestSpecsFromOpenShiftGinkgoSuite() (ext.ExtensionTestSpecs, error) { - var tests []*ext.ExtensionTestSpec +// BuildExtensionTestSpecsFromOpenShiftGinkgoSuite generates OTE specs for Gingko tests. While OTE isn't limited to +// calling ginkgo tests, anything that implements the ExtensionTestSpec interface can be used, it's the most common +// course of action. The typical use case is to omit selectFns, but if provided, these will filter the returned list +// of specs, applied in the order provided. +func BuildExtensionTestSpecsFromOpenShiftGinkgoSuite(selectFns ...ext.SelectFunction) (ext.ExtensionTestSpecs, error) { + var specs ext.ExtensionTestSpecs var enforceSerialExecutionForGinkgo sync.Mutex // in-process parallelization for ginkgo is impossible so far if _, _, err := configureGinkgo(); err != nil { @@ -54,10 +59,16 @@ func BuildExtensionTestSpecsFromOpenShiftGinkgoSuite() (ext.ExtensionTestSpecs, } ginkgo.GetSuite().WalkTests(func(name string, spec types.TestSpec) { + var codeLocations []string + for _, cl := range spec.CodeLocations() { + codeLocations = append(codeLocations, cl.String()) + } + testCase := &ext.ExtensionTestSpec{ - Name: spec.Text(), - Labels: sets.New[string](spec.Labels()...), - Lifecycle: GetLifecycle(spec.Labels()), + Name: spec.Text(), + Labels: sets.New[string](spec.Labels()...), + CodeLocations: codeLocations, + Lifecycle: GetLifecycle(spec.Labels()), Run: func() *ext.ExtensionTestResult { enforceSerialExecutionForGinkgo.Lock() defer enforceSerialExecutionForGinkgo.Unlock() @@ -103,10 +114,23 @@ func BuildExtensionTestSpecsFromOpenShiftGinkgoSuite() (ext.ExtensionTestSpecs, return result }, } - tests = append(tests, testCase) + specs = append(specs, testCase) }) - return tests, nil + // Default select function is to exclude vendored specs. When relying on Kubernetes test framework for its helpers, + // it also unfortunately ends up importing *all* Gingko specs. This is unsafe: it would potentially override the + // kube specs already present in origin. The best course of action is enforce this behavior on everyone. If for + // some reason, you must include vendored specs, you can opt-in directly by supplying your own SelectFunctions or using + // AllTestsIncludedVendored(). + if len(selectFns) == 0 { + selectFns = []ext.SelectFunction{ext.ModuleTestsOnly()} + } + + for _, selectFn := range selectFns { + specs = specs.Select(selectFn) + } + + return specs, nil } func Informing() ginkgo.Labels { diff --git a/vendor/modules.txt b/vendor/modules.txt index 378bf4f3ac..ae7e6b985b 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -985,7 +985,7 @@ github.com/opencontainers/runtime-spec/specs-go github.com/opencontainers/selinux/go-selinux github.com/opencontainers/selinux/go-selinux/label github.com/opencontainers/selinux/pkg/pwalkdir -# github.com/openshift-eng/openshift-tests-extension v0.0.0-20250220212757-b9c4d98a0c45 +# github.com/openshift-eng/openshift-tests-extension v0.0.0-20250401163437-6cbf1d70a450 ## explicit; go 1.23.0 github.com/openshift-eng/openshift-tests-extension/pkg/cmd github.com/openshift-eng/openshift-tests-extension/pkg/cmd/cmdimages