Refactor new filters tests to use single feature set

test/rekt/features/new_trigger_filters/feature.go

@@ -25,11 +25,34 @@ import (
 	eventingv1 "knative.dev/eventing/pkg/apis/eventing/v1"
 )
 
-// FiltersFeatureSet creates a feature set for testing the broker implementation of the new trigger filters experimental feature
-// (aka Cloud Events Subscriptions API filters). It requires a created and ready Broker resource with brokerName.
-//
-// The feature set tests four filter dialects: exact, prefix, suffix and cesql (aka CloudEvents SQL).
-func FiltersFeatureSet(brokerName string) *feature.FeatureSet {
+type InstallBrokerFunc func(f *feature.Feature) string
+
+type CloudEventsContext struct {
+	eventType            string
+	eventSource          string
+	eventSubject         string
+	eventID              string
+	eventDataSchema      string
+	eventDataContentType string
+	shouldDeliver        bool
+}
+
+// NewFiltersFeatureSet creates a feature set which runs tests for the new trigger filters
+// It requires a function which installs a broker implementation into the current feature for testing.
+// All new triggers tests should be registered in this feature set so that they can be easily included in other
+// broker implementations for testing.
+func NewFiltersFeatureSet(installBroker InstallBrokerFunc) *feature.FeatureSet {
+	features := SingleDialectFilterFeatures(installBroker)
+	features = append(features, AllFilterFeature(installBroker), AnyFilterFeature(installBroker), MultipleTriggersAndSinksFeature(installBroker))
+	return &feature.FeatureSet{
+		Name:     "New Trigger Filters",
+		Features: features,
+	}
+}
+
+// SingleDialectFilterFeatures creates an array of features which each test a single dialect of the new filters.
+// It requires a function which installs a broker implementation into the current feature for testing.
+func SingleDialectFilterFeatures(installBroker InstallBrokerFunc) []*feature.Feature {
 	matchedEvent := FullEvent()
 	unmatchedEvent := MinEvent()
 	unmatchedEvent.SetType("org.wrong.type")
@@ -93,27 +116,14 @@ func FiltersFeatureSet(brokerName string) *feature.FeatureSet {
 
 	for name, fs := range tests {
 		f := feature.NewFeatureNamed(name)
-		f = newEventFilterFeature(eventContexts, fs.filters, f, brokerName)
+		createNewFiltersFeature(f, eventContexts, fs.filters, installBroker)
 		features = append(features, f)
 	}
 
-	return &feature.FeatureSet{
-		Name:     "New trigger filters",
-		Features: features,
-	}
+	return features
 }
 
-type CloudEventsContext struct {
-	eventType            string
-	eventSource          string
-	eventSubject         string
-	eventID              string
-	eventDataSchema      string
-	eventDataContentType string
-	shouldDeliver        bool
-}
-
-func AnyFilterFeature(brokerName string) *feature.Feature {
+func AnyFilterFeature(installBroker InstallBrokerFunc) *feature.Feature {
 	f := feature.NewFeature()
 
 	eventContexts := []CloudEventsContext{
@@ -173,12 +183,12 @@ func AnyFilterFeature(brokerName string) *feature.Feature {
 		},
 	}
 
-	f = newEventFilterFeature(eventContexts, filters, f, brokerName)
+	createNewFiltersFeature(f, eventContexts, filters, installBroker)
 
 	return f
 }
 
-func AllFilterFeature(brokerName string) *feature.Feature {
+func AllFilterFeature(installBroker InstallBrokerFunc) *feature.Feature {
 	f := feature.NewFeature()
 
 	eventContexts := []CloudEventsContext{
@@ -229,12 +239,12 @@ func AllFilterFeature(brokerName string) *feature.Feature {
 		},
 	}
 
-	f = newEventFilterFeature(eventContexts, filters, f, brokerName)
+	createNewFiltersFeature(f, eventContexts, filters, installBroker)
 
 	return f
 }
 
-func MultipleTriggersAndSinksFeature(brokerName string) *feature.Feature {
+func MultipleTriggersAndSinksFeature(installBroker InstallBrokerFunc) *feature.Feature {
 	f := feature.NewFeature()
 
 	eventContextsFirstSink := []CloudEventsContext{
@@ -309,8 +319,14 @@ func MultipleTriggersAndSinksFeature(brokerName string) *feature.Feature {
 		},
 	}
 
-	f = newEventFilterFeature(eventContextsFirstSink, filtersFirstTrigger, f, brokerName)
-	f = newEventFilterFeature(eventContextsSecondSink, filtersSecondTrigger, f, brokerName)
+	// We need to create the broker here and mock it later so that the test uses the same broker for both filters
+	brokerName := installBroker(f)
+	fakeInstallBroker := func(_ *feature.Feature) string {
+		return brokerName
+	}
+
+	createNewFiltersFeature(f, eventContextsFirstSink, filtersFirstTrigger, fakeInstallBroker)
+	createNewFiltersFeature(f, eventContextsSecondSink, filtersSecondTrigger, fakeInstallBroker)
 
 	return f
 }

test/rekt/features/new_trigger_filters/filters.go

@@ -23,7 +23,6 @@ import (
 	"knative.dev/reconciler-test/pkg/eventshub"
 	. "knative.dev/reconciler-test/pkg/eventshub/assert"
 	"knative.dev/reconciler-test/pkg/feature"
-	"knative.dev/reconciler-test/pkg/k8s"
 	"knative.dev/reconciler-test/pkg/manifest"
 	"knative.dev/reconciler-test/pkg/resources/service"
 
@@ -33,9 +32,10 @@ import (
 	"knative.dev/eventing/test/rekt/resources/trigger"
 )
 
-func newEventFilterFeature(eventContexts []CloudEventsContext, filters []eventingv1.SubscriptionsAPIFilter, f *feature.Feature, brokerName string) *feature.Feature {
+func createNewFiltersFeature(f *feature.Feature, eventContexts []CloudEventsContext, filters []eventingv1.SubscriptionsAPIFilter, installBroker InstallBrokerFunc) {
 	subscriberName := feature.MakeRandomK8sName("subscriber")
 	triggerName := feature.MakeRandomK8sName("trigger")
+	brokerName := installBroker(f)
 
 	f.Setup("Install trigger subscriber", eventshub.Install(subscriberName, eventshub.StartReceiver))
 
@@ -46,50 +46,47 @@ func newEventFilterFeature(eventContexts []CloudEventsContext, filters []eventin
 
 	f.Setup("Install trigger", trigger.Install(triggerName, brokerName, cfg...))
 	f.Setup("Wait for trigger to become ready", trigger.IsReady(triggerName))
-	f.Setup("Broker is addressable", k8s.IsAddressable(broker.GVR(), brokerName))
 
 	asserter := f.Beta("New filters")
 
 	for _, eventCtx := range eventContexts {
-		event := newEventFromEventContext(eventCtx)
+		e := newEventFromEventContext(eventCtx)
 		eventSender := feature.MakeRandomK8sName("sender")
 
 		f.Requirement(fmt.Sprintf("Install event sender %s", eventSender), eventshub.Install(eventSender,
 			eventshub.StartSenderToResource(broker.GVR(), brokerName),
-			eventshub.InputEvent(event),
+			eventshub.InputEvent(e),
 		))
 
 		if eventCtx.shouldDeliver {
-			asserter.Must("must deliver matched event", OnStore(subscriberName).MatchEvent(HasId(event.ID())).AtLeast(1))
+			asserter.Must("must deliver matched event", OnStore(subscriberName).MatchEvent(HasId(e.ID())).AtLeast(1))
 		} else {
-			asserter.MustNot("must not deliver unmatched event", OnStore(subscriberName).MatchEvent(HasId(event.ID())).Not())
+			asserter.MustNot("must not deliver unmatched event", OnStore(subscriberName).MatchEvent(HasId(e.ID())).Not())
 		}
 	}
-
-	return f
 }
 
 func newEventFromEventContext(eventCtx CloudEventsContext) event.Event {
-	event := MinEvent()
+	e := MinEvent()
 	// Ensure that each event has a unique ID
-	event.SetID(feature.MakeRandomK8sName("event"))
+	e.SetID(feature.MakeRandomK8sName("event"))
 	if eventCtx.eventType != "" {
-		event.SetType(eventCtx.eventType)
+		e.SetType(eventCtx.eventType)
 	}
 	if eventCtx.eventSource != "" {
-		event.SetSource(eventCtx.eventSource)
+		e.SetSource(eventCtx.eventSource)
 	}
 	if eventCtx.eventSubject != "" {
-		event.SetSubject(eventCtx.eventSubject)
+		e.SetSubject(eventCtx.eventSubject)
 	}
 	if eventCtx.eventID != "" {
-		event.SetID(eventCtx.eventID)
+		e.SetID(eventCtx.eventID)
 	}
 	if eventCtx.eventDataSchema != "" {
-		event.SetDataSchema(eventCtx.eventDataSchema)
+		e.SetDataSchema(eventCtx.eventDataSchema)
 	}
 	if eventCtx.eventDataContentType != "" {
-		event.SetDataContentType(eventCtx.eventDataContentType)
+		e.SetDataContentType(eventCtx.eventDataContentType)
 	}
-	return event
+	return e
 }

test/rekt/new_trigger_filters_test.go

@@ -23,13 +23,12 @@ import (
 	"testing"
 
 	newfilters "knative.dev/eventing/test/rekt/features/new_trigger_filters"
+	"knative.dev/eventing/test/rekt/resources/broker"
 
 	"knative.dev/pkg/system"
 	"knative.dev/reconciler-test/pkg/environment"
 	"knative.dev/reconciler-test/pkg/k8s"
 	"knative.dev/reconciler-test/pkg/knative"
-
-	"knative.dev/eventing/test/rekt/resources/broker"
 )
 
 func TestMTChannelBrokerNewTriggerFilters(t *testing.T) {
@@ -42,56 +41,6 @@ func TestMTChannelBrokerNewTriggerFilters(t *testing.T) {
 		k8s.WithEventListener,
 		environment.Managed(t),
 	)
-	brokerName := "default"
-
-	env.Prerequisite(ctx, t, broker.InstallMTBroker(brokerName))
-	env.TestSet(ctx, t, newfilters.FiltersFeatureSet(brokerName))
-}
-
-func TestMTChannelBrokerAnyTriggerFilters(t *testing.T) {
-	t.Parallel()
-
-	ctx, env := global.Environment(
-		knative.WithKnativeNamespace(system.Namespace()),
-		knative.WithLoggingConfig,
-		knative.WithTracingConfig,
-		k8s.WithEventListener,
-		environment.Managed(t),
-	)
-	brokerName := "default"
-
-	env.Prerequisite(ctx, t, broker.InstallMTBroker(brokerName))
-	env.Test(ctx, t, newfilters.AnyFilterFeature(brokerName))
-}
-
-func TestMTChannelBrokerAllTriggerFilters(t *testing.T) {
-	t.Parallel()
-
-	ctx, env := global.Environment(
-		knative.WithKnativeNamespace(system.Namespace()),
-		knative.WithLoggingConfig,
-		knative.WithTracingConfig,
-		k8s.WithEventListener,
-		environment.Managed(t),
-	)
-	brokerName := "default"
-
-	env.Prerequisite(ctx, t, broker.InstallMTBroker(brokerName))
-	env.Test(ctx, t, newfilters.AllFilterFeature(brokerName))
-}
-
-func TestMultipleSinksMultipleTriggers(t *testing.T) {
-	t.Parallel()
-
-	ctx, env := global.Environment(
-		knative.WithKnativeNamespace(system.Namespace()),
-		knative.WithLoggingConfig,
-		knative.WithTracingConfig,
-		k8s.WithEventListener,
-		environment.Managed(t),
-	)
-	brokerName := "default"
 
-	env.Prerequisite(ctx, t, broker.InstallMTBroker(brokerName))
-	env.Test(ctx, t, newfilters.MultipleTriggersAndSinksFeature(brokerName))
+	env.ParallelTestSet(ctx, t, newfilters.NewFiltersFeatureSet(broker.InstallMTBrokerIntoFeature))
 }

test/rekt/resources/broker/broker.go

@@ -277,3 +277,11 @@ func InstallMTBroker(name string) *feature.Feature {
 	f.Requirement("Broker is ready", IsReady(name))
 	return f
 }
+
+func InstallMTBrokerIntoFeature(f *feature.Feature) string {
+	brokerName := feature.MakeRandomK8sName("broker")
+	f.Setup(fmt.Sprintf("Install broker %q", brokerName), Install(brokerName, WithEnvConfig()...))
+	f.Setup("Broker is ready", IsReady(brokerName))
+	f.Setup("Broker is addressable", k8s.IsAddressable(GVR(), brokerName))
+	return brokerName
+}