From 6f29571bed625b039bf950e44502f24363557f38 Mon Sep 17 00:00:00 2001 From: Calum Murray Date: Mon, 4 Dec 2023 13:21:29 -0500 Subject: [PATCH] Refactor new filters tests to use single feature set (#7424) * Refactored tests to use single feature set Signed-off-by: Calum Murray * Fix go imports Signed-off-by: Calum Murray * Call the correct test function :) Signed-off-by: Calum Murray * Small cleanup, bugfix Signed-off-by: Calum Murray * Update test/rekt/features/new_trigger_filters/feature.go Co-authored-by: Pierangelo Di Pilato * Use parallel test set Signed-off-by: Calum Murray --------- Signed-off-by: Calum Murray Co-authored-by: Pierangelo Di Pilato --- .../features/new_trigger_filters/feature.go | 70 ++++++++++++------- .../features/new_trigger_filters/filters.go | 33 ++++----- test/rekt/new_trigger_filters_test.go | 55 +-------------- test/rekt/resources/broker/broker.go | 8 +++ 4 files changed, 68 insertions(+), 98 deletions(-) diff --git a/test/rekt/features/new_trigger_filters/feature.go b/test/rekt/features/new_trigger_filters/feature.go index 035026ba85e..854dbd721f7 100644 --- a/test/rekt/features/new_trigger_filters/feature.go +++ b/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 } diff --git a/test/rekt/features/new_trigger_filters/filters.go b/test/rekt/features/new_trigger_filters/filters.go index 3bc0ceb26ae..7a06cfafbe7 100644 --- a/test/rekt/features/new_trigger_filters/filters.go +++ b/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 } diff --git a/test/rekt/new_trigger_filters_test.go b/test/rekt/new_trigger_filters_test.go index 36b5502565c..6324f0b295b 100644 --- a/test/rekt/new_trigger_filters_test.go +++ b/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)) } diff --git a/test/rekt/resources/broker/broker.go b/test/rekt/resources/broker/broker.go index 4607847a75b..b3abbda9833 100644 --- a/test/rekt/resources/broker/broker.go +++ b/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 +}