Skip to content

Commit

Permalink
allow obsoleting tests
Browse files Browse the repository at this point in the history
  • Loading branch information
stbenjam committed Oct 2, 2024
1 parent 35ec2e6 commit ddd291f
Show file tree
Hide file tree
Showing 11 changed files with 377 additions and 110 deletions.
230 changes: 230 additions & 0 deletions .openshift-tests-extension/openshift:framework:default.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,230 @@
[
{
"name": "[sig-testing] openshift-tests-extension should be run from the main directory",
"otherNames": null,
"labels": {
"framework": {}
},
"tags": null,
"resources": {
"isolation": {
"mode": "",
"conflict": null
},
"memory": "",
"duration": "",
"timeout": ""
},
"source": "openshift:framework:default",
"lifecycle": "blocking"
},
{
"name": "[sig-testing] openshift-tests-extension should run `make` command successfully",
"otherNames": null,
"labels": {
"framework": {}
},
"tags": null,
"resources": {
"isolation": {
"mode": "",
"conflict": null
},
"memory": "",
"duration": "",
"timeout": ""
},
"source": "openshift:framework:default",
"lifecycle": "blocking"
},
{
"name": "[sig-testing] openshift-tests-extension should have the example-tests binary",
"otherNames": null,
"labels": {
"framework": {}
},
"tags": null,
"resources": {
"isolation": {
"mode": "",
"conflict": null
},
"memory": "",
"duration": "",
"timeout": ""
},
"source": "openshift:framework:default",
"lifecycle": "blocking"
},
{
"name": "[sig-testing] example-tests info should have build metadata",
"otherNames": null,
"labels": {
"framework": {}
},
"tags": null,
"resources": {
"isolation": {
"mode": "",
"conflict": null
},
"memory": "",
"duration": "",
"timeout": ""
},
"source": "openshift:framework:default",
"lifecycle": "blocking"
},
{
"name": "[sig-testing] example-tests info should have the expected suites",
"otherNames": null,
"labels": {
"framework": {}
},
"tags": null,
"resources": {
"isolation": {
"mode": "",
"conflict": null
},
"memory": "",
"duration": "",
"timeout": ""
},
"source": "openshift:framework:default",
"lifecycle": "blocking"
},
{
"name": "[sig-testing] example-tests info should have the correct component information",
"otherNames": null,
"labels": {
"framework": {}
},
"tags": null,
"resources": {
"isolation": {
"mode": "",
"conflict": null
},
"memory": "",
"duration": "",
"timeout": ""
},
"source": "openshift:framework:default",
"lifecycle": "blocking"
},
{
"name": "[sig-testing] example-tests list should list all specs",
"otherNames": null,
"labels": {
"framework": {}
},
"tags": null,
"resources": {
"isolation": {
"mode": "",
"conflict": null
},
"memory": "",
"duration": "",
"timeout": ""
},
"source": "openshift:framework:default",
"lifecycle": "blocking"
},
{
"name": "[sig-testing] example-tests list should populate fields",
"otherNames": null,
"labels": {
"framework": {}
},
"tags": null,
"resources": {
"isolation": {
"mode": "",
"conflict": null
},
"memory": "",
"duration": "",
"timeout": ""
},
"source": "openshift:framework:default",
"lifecycle": "blocking"
},
{
"name": "[sig-testing] example-tests list should filter specs by suite",
"otherNames": null,
"labels": {
"framework": {}
},
"tags": null,
"resources": {
"isolation": {
"mode": "",
"conflict": null
},
"memory": "",
"duration": "",
"timeout": ""
},
"source": "openshift:framework:default",
"lifecycle": "blocking"
},
{
"name": "[sig-testing] example-tests run-suite should contain a test that passed",
"otherNames": null,
"labels": {
"framework": {}
},
"tags": null,
"resources": {
"isolation": {
"mode": "",
"conflict": null
},
"memory": "",
"duration": "",
"timeout": ""
},
"source": "openshift:framework:default",
"lifecycle": "blocking"
},
{
"name": "[sig-testing] example-tests run-suite should have the correct error message for the failed test",
"otherNames": null,
"labels": {
"framework": {}
},
"tags": null,
"resources": {
"isolation": {
"mode": "",
"conflict": null
},
"memory": "",
"duration": "",
"timeout": ""
},
"source": "openshift:framework:default",
"lifecycle": "blocking"
},
{
"name": "[sig-testing] example-tests run-suite fast suite should not have a slow test",
"otherNames": null,
"labels": {
"framework": {}
},
"tags": null,
"resources": {
"isolation": {
"mode": "",
"conflict": null
},
"memory": "",
"duration": "",
"timeout": ""
},
"source": "openshift:framework:default",
"lifecycle": "blocking"
}
]
19 changes: 0 additions & 19 deletions .openshift-tests-extension/openshift:payload:example-tests.json
Original file line number Diff line number Diff line change
Expand Up @@ -102,24 +102,5 @@
},
"source": "openshift:payload:example-tests",
"lifecycle": "blocking"
},
{
"name": "[sig-testing] openshift-tests-extension has a test with a typo",
"otherNames": {
"[sig-testing] openshift-tests-extension has a test with a tpyo": {}
},
"labels": {},
"tags": null,
"resources": {
"isolation": {
"mode": "",
"conflict": null
},
"memory": "",
"duration": "",
"timeout": ""
},
"source": "openshift:payload:example-tests",
"lifecycle": "blocking"
}
]
18 changes: 6 additions & 12 deletions cmd/example-tests/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,9 @@ import (
"os"

"github.com/spf13/cobra"
"k8s.io/apimachinery/pkg/util/sets"

"github.com/openshift-eng/openshift-tests-extension/pkg/cmd"
e "github.com/openshift-eng/openshift-tests-extension/pkg/extension"
"github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests"
g "github.com/openshift-eng/openshift-tests-extension/pkg/ginkgo"

// If using ginkgo, import your tests here
Expand Down Expand Up @@ -66,21 +64,17 @@ func main() {
// specs = specs.MustFilter([]string{`name.contains("filter")`})

// Or walked...
specs = specs.Walk(func(spec *extensiontests.ExtensionTestSpec) {
if spec.Name == "[sig-testing] openshift-tests-extension has a test with a typo" {
spec.OtherNames = sets.New[string](`[sig-testing] openshift-tests-extension has a test with a tpyo`)
}
})

// specs = specs.Walk(func(e *extensiontests.ExtensionTestSpec) {
// specs = specs.Walk(func(spec *extensiontests.ExtensionTestSpec) {
// if strings.Contains(e.Name, "scale up") {
// e.Labels.Insert("SLOW")
// }
//
// if val, ok := renameMap[e.Name]; ok {
// e.SetOtherNames(val...). // TODO
// Test renames
// if spec.Name == "[sig-testing] openshift-tests-extension has a test with a typo" {
// spec.OtherNames = sets.New[string](`[sig-testing] openshift-tests-extension has a test with a tpyo`)
// }
//})
// })

ext.AddSpecs(specs)
registry.Register(ext)

Expand Down
10 changes: 6 additions & 4 deletions pkg/cmd/cmdupdate/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ import (

const metadataDirectory = ".openshift-tests-extension"

// NewUpdateCommand adds an "update" command used to generate and verify the metadata we keep track of. This should
// be a black box to end users, i.e. we can add more criteria later they'll consume when revendoring. For now,
// we prevent a test to be renamed without updating other names, or a test to be deleted.
func NewUpdateCommand(registry *extension.Registry) *cobra.Command {
componentFlags := flags.NewComponentFlags()

Expand All @@ -33,8 +36,6 @@ func NewUpdateCommand(registry *extension.Registry) *cobra.Command {
return fmt.Errorf("failed to create directory %s: %w", metadataDirectory, err)
}

newSpecs := ext.GetSpecs()

// Read existing specs
metadataPath := filepath.Join(metadataDirectory, fmt.Sprintf("%s.json", ext.Component.Identifier()))
var oldSpecs extensiontests.ExtensionTestSpecs
Expand All @@ -48,7 +49,7 @@ func NewUpdateCommand(registry *extension.Registry) *cobra.Command {
return fmt.Errorf("failed to decode file: %s: %+w", metadataPath, err)
}

missing, err := newSpecs.FindRemovedTestsWithoutRename(oldSpecs)
missing, err := ext.FindRemovedTestsWithoutRename(oldSpecs)
if err != nil && len(missing) > 0 {
fmt.Fprintf(os.Stderr, "Missing Tests:\n")
for _, name := range missing {
Expand All @@ -62,6 +63,7 @@ func NewUpdateCommand(registry *extension.Registry) *cobra.Command {
}

// no missing tests, write the results
newSpecs := ext.GetSpecs()
data, err := json.MarshalIndent(newSpecs, "", " ")
if err != nil {
return fmt.Errorf("failed to marshal specs to JSON: %w", err)
Expand All @@ -72,7 +74,7 @@ func NewUpdateCommand(registry *extension.Registry) *cobra.Command {
return fmt.Errorf("failed to write file %s: %w", metadataPath, err)
}

fmt.Printf("successfully updated metadata")
fmt.Printf("successfully updated metadata\n")
return nil
},
}
Expand Down
41 changes: 41 additions & 0 deletions pkg/extension/extension.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,47 @@ func (e *Extension) AddSpecs(specs et.ExtensionTestSpecs) {
e.specs = append(e.specs, specs...)
}

// IgnoreObsoleteTests allows removal of a test.
func (e *Extension) IgnoreObsoleteTests(testNames ...string) {
e.obsoleteTests = append(e.obsoleteTests, testNames...)
}

// FindRemovedTestsWithoutRename compares two collections of specs, and if specs is missing a test from oldSpecs,
// including consideration of other names, we return an error. Can be used to detect test renames or removals.
func (e *Extension) FindRemovedTestsWithoutRename(oldSpecs et.ExtensionTestSpecs) ([]string, error) {
currentSpecs := e.GetSpecs()
// It's neat we can do it with CEL but can it handle it when we've got 10K tests in there?
potentiallyMissing, err := oldSpecs.Filter([]string{fmt.Sprintf(`!(name in %s)`, strSliceToCEL(currentSpecs.Names()))})
if err != nil {
return nil, err
}

actuallyMissing, err := potentiallyMissing.Filter([]string{fmt.Sprintf(`!(%s.exists(d, name == d))`,
strSliceToCEL(currentSpecs.OtherNames()))})
if err != nil {
return nil, err
}

var unpermittedMissingTests []string
for _, spec := range actuallyMissing {
missing := true
for _, allowed := range e.obsoleteTests {
if spec.Name == allowed {
missing = false
}
}
if missing {
unpermittedMissingTests = append(unpermittedMissingTests, spec.Name)
}
}

if len(unpermittedMissingTests) > 0 {
return unpermittedMissingTests, fmt.Errorf("some tests were not found")
}

return nil, nil
}

// AddGlobalSuite adds a suite whose qualifiers will apply to all tests,
// not just this one. Allowing a developer to create a composed suite of
// tests from many sources.
Expand Down
Loading

0 comments on commit ddd291f

Please sign in to comment.