allow obsoleting tests

.openshift-tests-extension/openshift:payload:example-tests.json

@@ -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"
   }
 ]

cmd/example-tests/main.go

@@ -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
@@ -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)
 

pkg/cmd/cmdupdate/update.go

@@ -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()
 
@@ -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
@@ -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 {
@@ -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)
@@ -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
 		},
 	}

pkg/extension/extension.go

@@ -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.

pkg/extension/extension_test.go

@@ -0,0 +1,91 @@
+package extension
+
+import (
+	"reflect"
+	"testing"
+
+	"k8s.io/apimachinery/pkg/util/sets"
+
+	et "github.com/openshift-eng/openshift-tests-extension/pkg/extension/extensiontests"
+)
+
+func TestExtension_FindRemovedTestsWithoutRename(t *testing.T) {
+	tests := []struct {
+		name          string
+		old           et.ExtensionTestSpecs
+		new           et.ExtensionTestSpecs
+		obsoleteTests []string
+		want          []string
+		wantErr       bool
+	}{
+		{
+			name: "allows a test to be renamed",
+			old: et.ExtensionTestSpecs{
+				{
+					Name: "this test has a tpyo",
+				},
+			},
+			new: et.ExtensionTestSpecs{
+				{
+					Name:       "this test doesn't have a typo",
+					OtherNames: sets.New[string]("this test has a tpyo"),
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "fails when a test is removed",
+			old: et.ExtensionTestSpecs{
+				{
+					Name: "this test was deleted",
+				},
+			},
+			new:     et.ExtensionTestSpecs{},
+			want:    []string{"this test was deleted"},
+			wantErr: true,
+		},
+		{
+			name: "succeeds when a test is removed and it's marked obsolete",
+			old: et.ExtensionTestSpecs{
+				{
+					Name: "this test was deleted",
+				},
+			},
+			new:           et.ExtensionTestSpecs{},
+			obsoleteTests: []string{"this test was deleted"},
+			wantErr:       false,
+		},
+		{
+			name: "fails when a test is renamed without other names",
+			old: et.ExtensionTestSpecs{
+				{
+					Name: "this test has a tpyo",
+				},
+			},
+			new: et.ExtensionTestSpecs{
+				{
+					Name: "this test doesn't have a typo",
+				},
+			},
+			want:    []string{"this test has a tpyo"},
+			wantErr: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ext := NewExtension("openshift", "testing", "dummy")
+			ext.AddSpecs(tt.new)
+			ext.IgnoreObsoleteTests(tt.obsoleteTests...)
+
+			got, err := ext.FindRemovedTestsWithoutRename(tt.old)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("FindRemovedTestsWithoutRename() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("FindRemovedTestsWithoutRename() got = %v, want %v", got, tt.want)
+			}
+		})
+	}
+}

pkg/extension/extensiontests/spec.go

@@ -29,27 +29,6 @@ func (specs ExtensionTestSpecs) Run(w *ResultWriter) error {
 	return results.CheckOverallResult()
 }
 
-// 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 (specs ExtensionTestSpecs) FindRemovedTestsWithoutRename(oldSpecs ExtensionTestSpecs) ([]string, error) {
-	potentiallyMissing, err := oldSpecs.Filter([]string{fmt.Sprintf(`!(name in %s)`, strSliceToCEL(specs.Names()))})
-	if err != nil {
-		return nil, err
-	}
-
-	actuallyMissing, err := potentiallyMissing.Filter([]string{fmt.Sprintf(`!(%s.exists(d, name == d))`,
-		strSliceToCEL(specs.OtherNames()))})
-	if err != nil {
-		return nil, err
-	}
-
-	if len(actuallyMissing) > 0 {
-		return actuallyMissing.Names(), fmt.Errorf("some tests were not found")
-	}
-
-	return nil, nil
-}
-
 func (specs ExtensionTestSpecs) OtherNames() []string {
 	var names []string
 	for _, spec := range specs {

pkg/extension/extensiontests/spec_test.go

@@ -8,54 +8,6 @@ import (
 	"k8s.io/apimachinery/pkg/util/sets"
 )
 
-func TestExtensionTestSpecs_FindRemovedTestsWithoutRename(t *testing.T) {
-	tests := []struct {
-		name    string
-		old     ExtensionTestSpecs
-		new     ExtensionTestSpecs
-		want    []string
-		wantErr bool
-	}{
-		{
-			old: ExtensionTestSpecs{
-				{
-					Name: "this test has a tpyo",
-				},
-			},
-			new: ExtensionTestSpecs{
-				{
-					Name:       "this test doesn't have a typo",
-					OtherNames: sets.New[string]("this test has a tpyo"),
-				},
-			},
-			wantErr: false,
-		},
-		{
-			old: ExtensionTestSpecs{
-				{
-					Name: "this test was deleted",
-				},
-			},
-			new:     ExtensionTestSpecs{},
-			want:    []string{"this test was deleted"},
-			wantErr: true,
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			got, err := tt.new.FindRemovedTestsWithoutRename(tt.old)
-			if (err != nil) != tt.wantErr {
-				t.Errorf("FindRemovedTestsWithoutRename() error = %v, wantErr %v", err, tt.wantErr)
-				return
-			}
-			if !reflect.DeepEqual(got, tt.want) {
-				t.Errorf("FindRemovedTestsWithoutRename() got = %v, want %v", got, tt.want)
-			}
-		})
-	}
-}
-
 func TestExtensionTestSpecs_Walk(t *testing.T) {
 	specs := ExtensionTestSpecs{
 		{Name: "test1"},

pkg/extension/types.go

@@ -16,7 +16,8 @@ type Extension struct {
 	Suites []Suite `json:"suites"`
 
 	// Private data
-	specs extensiontests.ExtensionTestSpecs
+	specs         extensiontests.ExtensionTestSpecs
+	obsoleteTests []string
 }
 
 // Source contains the details of the commit and source URL.

pkg/extension/extensiontests/util.go → pkg/extension/util.go

@@ -1,4 +1,4 @@
-package extensiontests
+package extension
 
 import (
 	"fmt"

test/example/example.go

@@ -51,8 +51,4 @@ var _ = Describe("[sig-testing] openshift-tests-extension", func() {
 		time.Sleep(15 * time.Second)
 		Expect(true).To(BeTrue())
 	})
-
-	It("has a test with a typo", func() {
-		Expect(true).To(BeTrue())
-	})
 })