diff --git a/pkg/reconciler/customrun/customrun_test.go b/pkg/reconciler/customrun/customrun_test.go index bb1674faf99..9bf26552c57 100644 --- a/pkg/reconciler/customrun/customrun_test.go +++ b/pkg/reconciler/customrun/customrun_test.go @@ -28,7 +28,6 @@ import ( "github.com/tektoncd/pipeline/pkg/reconciler/events/cloudevent" ttesting "github.com/tektoncd/pipeline/pkg/reconciler/testing" "github.com/tektoncd/pipeline/test" - eventstest "github.com/tektoncd/pipeline/test/events" "github.com/tektoncd/pipeline/test/names" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -47,6 +46,7 @@ import ( func initializeCustomRunControllerAssets(t *testing.T, d test.Data) (test.Assets, func()) { ctx, _ := ttesting.SetupFakeContext(t) + ctx = ttesting.SetupFakeCloudClientContext(ctx, d.ExpectedCloudEventCount) ctx, cancel := context.WithCancel(ctx) test.EnsureConfigurationConfigMapsExist(&d) c, informers := test.SeedTestData(t, ctx, d) @@ -158,8 +158,9 @@ func TestReconcile_CloudEvents(t *testing.T) { customRuns := []*v1beta1.CustomRun{&customRun} d := test.Data{ - CustomRuns: customRuns, - ConfigMaps: cms, + CustomRuns: customRuns, + ConfigMaps: cms, + ExpectedCloudEventCount: len(tc.wantCloudEvents), } testAssets, cancel := getCustomRunController(t, d) defer cancel() @@ -187,19 +188,13 @@ func TestReconcile_CloudEvents(t *testing.T) { } ceClient := clients.CloudEvents.(cloudevent.FakeClient) - err = eventstest.CheckEventsUnordered(t, ceClient.Events, tc.name, tc.wantCloudEvents) - if err != nil { - t.Errorf(err.Error()) - } + ceClient.CheckCloudEventsUnordered(t, tc.name, tc.wantCloudEvents) // Try and reconcile again - expect no event if err := c.Reconciler.Reconcile(testAssets.Ctx, getCustomRunName(customRun)); err != nil { t.Fatal("Didn't expect an error, but got one.") } - err = eventstest.CheckEventsUnordered(t, ceClient.Events, tc.name, []string{}) - if err != nil { - t.Errorf(err.Error()) - } + ceClient.CheckCloudEventsUnordered(t, tc.name, []string{}) }) } } @@ -296,10 +291,7 @@ func TestReconcile_CloudEvents_Disabled(t *testing.T) { } ceClient := clients.CloudEvents.(cloudevent.FakeClient) - err = eventstest.CheckEventsUnordered(t, ceClient.Events, tc.name, []string{}) - if err != nil { - t.Errorf(err.Error()) - } + ceClient.CheckCloudEventsUnordered(t, tc.name, []string{}) }) } } diff --git a/pkg/reconciler/events/cloudevent/cloud_event_controller.go b/pkg/reconciler/events/cloudevent/cloud_event_controller.go index abe464eb362..0a3d3b27eff 100644 --- a/pkg/reconciler/events/cloudevent/cloud_event_controller.go +++ b/pkg/reconciler/events/cloudevent/cloud_event_controller.go @@ -145,7 +145,10 @@ func SendCloudEventWithRetries(ctx context.Context, object runtime.Object) error _, isCustomRun := object.(*v1beta1.CustomRun) wasIn := make(chan error) + + ceClient.addCount() go func() { + defer ceClient.decreaseCount() wasIn <- nil logger.Debugf("Sending cloudevent of type %q", event.Type()) // In case of Run event, check cache if cloudevent is already sent diff --git a/pkg/reconciler/events/cloudevent/cloud_event_controller_test.go b/pkg/reconciler/events/cloudevent/cloud_event_controller_test.go index 4c8a9eb865f..db23a4e2d2c 100644 --- a/pkg/reconciler/events/cloudevent/cloud_event_controller_test.go +++ b/pkg/reconciler/events/cloudevent/cloud_event_controller_test.go @@ -234,7 +234,7 @@ func TestSendCloudEvents(t *testing.T) { successfulBehaviour := FakeClientBehaviour{ SendSuccessfully: true, } - err := SendCloudEvents(tc.taskRun, newFakeClient(&successfulBehaviour), logger, testClock) + err := SendCloudEvents(tc.taskRun, newFakeClient(&successfulBehaviour, len(tc.wantTaskRun.Status.CloudEvents)), logger, testClock) if err != nil { t.Fatalf("Unexpected error sending cloud events: %v", err) } @@ -335,7 +335,7 @@ func TestSendCloudEventsErrors(t *testing.T) { unsuccessfulBehaviour := FakeClientBehaviour{ SendSuccessfully: false, } - err := SendCloudEvents(tc.taskRun, newFakeClient(&unsuccessfulBehaviour), logger, testClock) + err := SendCloudEvents(tc.taskRun, newFakeClient(&unsuccessfulBehaviour, len(tc.wantTaskRun.Status.CloudEvents)), logger, testClock) if err == nil { t.Fatalf("Unexpected success sending cloud events: %v", err) } @@ -614,14 +614,12 @@ func TestSendCloudEventWithRetries(t *testing.T) { }} for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - ctx := setupFakeContext(t, tc.clientBehaviour, true) + ctx := setupFakeContext(t, tc.clientBehaviour, true, len(tc.wantCEvents)) if err := SendCloudEventWithRetries(ctx, tc.object); err != nil { t.Fatalf("Unexpected error sending cloud events: %v", err) } ceClient := Get(ctx).(FakeClient) - if err := eventstest.CheckEventsUnordered(t, ceClient.Events, tc.name, tc.wantCEvents); err != nil { - t.Fatalf(err.Error()) - } + ceClient.CheckCloudEventsUnordered(t, tc.name, tc.wantCEvents) recorder := controller.GetEventRecorder(ctx).(*record.FakeRecorder) if err := eventstest.CheckEventsOrdered(t, recorder.Events, tc.name, tc.wantEvents); err != nil { t.Fatalf(err.Error()) @@ -656,7 +654,7 @@ func TestSendCloudEventWithRetriesInvalid(t *testing.T) { t.Run(tc.name, func(t *testing.T) { ctx := setupFakeContext(t, FakeClientBehaviour{ SendSuccessfully: true, - }, true) + }, true, 1) ctx, cancel := context.WithCancel(ctx) defer cancel() err := SendCloudEventWithRetries(ctx, tc.object) @@ -669,7 +667,7 @@ func TestSendCloudEventWithRetriesInvalid(t *testing.T) { func TestSendCloudEventWithRetriesNoClient(t *testing.T) { - ctx := setupFakeContext(t, FakeClientBehaviour{}, false) + ctx := setupFakeContext(t, FakeClientBehaviour{}, false, 0) err := SendCloudEventWithRetries(ctx, &v1beta1.TaskRun{Status: v1beta1.TaskRunStatus{}}) if err == nil { t.Fatalf("Expected an error sending cloud events with no client in the context, got none") @@ -679,11 +677,11 @@ func TestSendCloudEventWithRetriesNoClient(t *testing.T) { } } -func setupFakeContext(t *testing.T, behaviour FakeClientBehaviour, withClient bool) context.Context { +func setupFakeContext(t *testing.T, behaviour FakeClientBehaviour, withClient bool, expectedEventCount int) context.Context { var ctx context.Context ctx, _ = rtesting.SetupFakeContext(t) if withClient { - ctx = WithClient(ctx, &behaviour) + ctx = WithClient(ctx, &behaviour, expectedEventCount) } return ctx } diff --git a/pkg/reconciler/events/cloudevent/cloudevent.go b/pkg/reconciler/events/cloudevent/cloudevent.go index 6afe96b5dd6..dfbd896b683 100644 --- a/pkg/reconciler/events/cloudevent/cloudevent.go +++ b/pkg/reconciler/events/cloudevent/cloudevent.go @@ -88,8 +88,15 @@ func (t TektonEventType) String() string { return string(t) } -// CEClient matches the `Client` interface from github.com/cloudevents/sdk-go/v2/cloudevents -type CEClient cloudevents.Client +// CEClient wraps the `Client` interface from github.com/cloudevents/sdk-go/v2/cloudevents +// and has methods to count the cloud events being sent, those methods are for testing purposes. +type CEClient interface { + cloudevents.Client + // addCount increments the count of events to be sent + addCount() + // decreaseCount decrements the count of events to be sent, indicating the event has been sent + decreaseCount() +} // TektonCloudEventData type is used to marshal and unmarshal the payload of // a Tekton cloud event. It can include a TaskRun or a PipelineRun diff --git a/pkg/reconciler/events/cloudevent/cloudeventclient.go b/pkg/reconciler/events/cloudevent/cloudeventclient.go index be89c5f5fc9..0e398a60eb7 100644 --- a/pkg/reconciler/events/cloudevent/cloudeventclient.go +++ b/pkg/reconciler/events/cloudevent/cloudeventclient.go @@ -21,6 +21,9 @@ import ( "net/http" cloudevents "github.com/cloudevents/sdk-go/v2" + "github.com/cloudevents/sdk-go/v2/client" + "github.com/cloudevents/sdk-go/v2/event" + "github.com/cloudevents/sdk-go/v2/protocol" "k8s.io/client-go/rest" "knative.dev/pkg/injection" "knative.dev/pkg/logging" @@ -59,7 +62,38 @@ func withCloudEventClient(ctx context.Context) context.Context { logger.Panicf("Error creating the cloudevents client: %s", err) } - return context.WithValue(ctx, ceKey{}, cloudEventClient) + celient := CloudClient{ + client: cloudEventClient, + } + return context.WithValue(ctx, ceKey{}, celient) +} + +// CloudClient is a wrapper of CloudEvents client and implements addCount and decreaseCount +type CloudClient struct { + client client.Client +} + +// AddCount does nothing +func (c CloudClient) addCount() { +} + +// DecreaseCount does nothing +func (c CloudClient) decreaseCount() { +} + +// Send invokes call client.Send +func (c CloudClient) Send(ctx context.Context, event cloudevents.Event) protocol.Result { + return c.client.Send(ctx, event) +} + +// Request invokes client.Request +func (c CloudClient) Request(ctx context.Context, event event.Event) (*cloudevents.Event, protocol.Result) { + return c.client.Request(ctx, event) +} + +// StartReceiver invokes client.StartReceiver +func (c CloudClient) StartReceiver(ctx context.Context, fn interface{}) error { + return c.client.StartReceiver(ctx, fn) } // Get extracts the cloudEventClient client from the context. diff --git a/pkg/reconciler/events/cloudevent/cloudeventsfakeclient.go b/pkg/reconciler/events/cloudevent/cloudeventsfakeclient.go index b727ac537ec..07ce42f585b 100644 --- a/pkg/reconciler/events/cloudevent/cloudeventsfakeclient.go +++ b/pkg/reconciler/events/cloudevent/cloudeventsfakeclient.go @@ -19,13 +19,14 @@ package cloudevent import ( "context" "fmt" + "regexp" + "sync" + "testing" cloudevents "github.com/cloudevents/sdk-go/v2" "github.com/cloudevents/sdk-go/v2/protocol" ) -const bufferSize = 100 - // FakeClientBehaviour defines how the client will behave type FakeClientBehaviour struct { SendSuccessfully bool @@ -37,13 +38,17 @@ type FakeClient struct { behaviour *FakeClientBehaviour // Modelled after k8s.io/client-go fake recorder Events chan string + // waitGroup is used to block until all events have been sent + waitGroup *sync.WaitGroup } // newFakeClient is a FakeClient factory, it returns a client for the target -func newFakeClient(behaviour *FakeClientBehaviour) cloudevents.Client { +func newFakeClient(behaviour *FakeClientBehaviour, expectedEventCount int) CEClient { return FakeClient{ behaviour: behaviour, - Events: make(chan string, bufferSize), + // set buffersize to length of want events to make sure no extra events are sent + Events: make(chan string, expectedEventCount), + waitGroup: &sync.WaitGroup{}, } } @@ -52,8 +57,12 @@ var _ cloudevents.Client = (*FakeClient)(nil) // Send fakes the Send method from cloudevents.Client func (c FakeClient) Send(ctx context.Context, event cloudevents.Event) protocol.Result { if c.behaviour.SendSuccessfully { - c.Events <- fmt.Sprintf("%s", event.String()) - return nil + // This is to prevent extra events are sent. We don't read events from channel before we call CheckCloudEventsUnordered + if len(c.Events) < cap(c.Events) { + c.Events <- fmt.Sprintf("%s", event.String()) + return nil + } + return fmt.Errorf("Channel is full of size:%v, but extra event wants to be sent:%v", cap(c.Events), event) } return fmt.Errorf("Had to fail. Event ID: %s", event.ID()) } @@ -61,8 +70,11 @@ func (c FakeClient) Send(ctx context.Context, event cloudevents.Event) protocol. // Request fakes the Request method from cloudevents.Client func (c FakeClient) Request(ctx context.Context, event cloudevents.Event) (*cloudevents.Event, protocol.Result) { if c.behaviour.SendSuccessfully { - c.Events <- fmt.Sprintf("%v", event.String()) - return &event, nil + if len(c.Events) < cap(c.Events) { + c.Events <- fmt.Sprintf("%v", event.String()) + return &event, nil + } + return nil, fmt.Errorf("Channel is full of size:%v, but extra event wants to be sent:%v", cap(c.Events), event) } return nil, fmt.Errorf("Had to fail. Event ID: %s", event.ID()) } @@ -72,7 +84,56 @@ func (c FakeClient) StartReceiver(ctx context.Context, fn interface{}) error { return nil } -// WithClient adds to the context a fake client with the desired behaviour -func WithClient(ctx context.Context, behaviour *FakeClientBehaviour) context.Context { - return context.WithValue(ctx, ceKey{}, newFakeClient(behaviour)) +// addCount can be used to add the count when each event is going to be sent +func (c FakeClient) addCount() { + c.waitGroup.Add(1) +} + +// decreaseCount can be used to the decrease the count when each event is sent +func (c FakeClient) decreaseCount() { + c.waitGroup.Done() +} + +// WithClient adds to the context a fake client with the desired behaviour and expectedEventCount +func WithClient(ctx context.Context, behaviour *FakeClientBehaviour, expectedEventCount int) context.Context { + return context.WithValue(ctx, ceKey{}, newFakeClient(behaviour, expectedEventCount)) +} + +// CheckCloudEventsUnordered checks that all events in wantEvents, and no others, +// were received via the given chan, in any order. +// Block until all events have been sent. +func (c *FakeClient) CheckCloudEventsUnordered(t *testing.T, testName string, wantEvents []string) { + t.Helper() + c.waitGroup.Wait() + expected := append([]string{}, wantEvents...) + channelEvents := len(c.Events) + + // extra events are prevented in FakeClient's Send function. + // fewer events are detected because we collect all events from channel and compare with wantEvents + for eventCount := 0; eventCount < channelEvents; eventCount++ { + event := <-c.Events + if len(expected) == 0 { + t.Errorf("extra event received: %q", event) + } + found := false + for wantIdx, want := range expected { + matching, err := regexp.MatchString(want, event) + if err != nil { + t.Errorf("something went wrong matching an event: %s", err) + } + if matching { + found = true + // Remove event from list of those we expect to receive + expected[wantIdx] = expected[len(expected)-1] + expected = expected[:len(expected)-1] + break + } + } + if !found { + t.Errorf("unexpected event received: %q", event) + } + } + if len(expected) != 0 { + t.Errorf("%d events %#v are not received", len(expected), expected) + } } diff --git a/pkg/reconciler/events/cloudevent/cloudeventsfakeclient_test.go b/pkg/reconciler/events/cloudevent/cloudeventsfakeclient_test.go new file mode 100644 index 00000000000..52ed5f36a33 --- /dev/null +++ b/pkg/reconciler/events/cloudevent/cloudeventsfakeclient_test.go @@ -0,0 +1,74 @@ +/* +Copyright 2022 The Tekton Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cloudevent + +import ( + "testing" + + "github.com/cloudevents/sdk-go/v2/event" + rtesting "knative.dev/pkg/reconciler/testing" +) + +func TestSend_Success(t *testing.T) { + sendEvents := []event.Event{ + { + Context: event.EventContextV1{ + ID: "test-event", + }.AsV1(), + }, { + Context: event.EventContextV1{ + ID: "test-event2", + }.AsV1(), + }, + } + + wantEvents := []string{"Context Attributes,", "Context Attributes,"} + + // Setup the context and seed test event + ctx, _ := rtesting.SetupFakeContext(t) + ctx = WithClient(ctx, &FakeClientBehaviour{SendSuccessfully: true}, len(wantEvents)) + fakeClient := Get(ctx).(FakeClient) + + for _, e := range sendEvents { + err := fakeClient.Send(ctx, e) + if err != nil { + t.Fatalf("got err %v", err) + } + + } + fakeClient.CheckCloudEventsUnordered(t, "send cloud events", wantEvents) +} + +func TestSend_Error(t *testing.T) { + sendEvent := event.Event{ + Context: event.EventContextV1{ + ID: "test-event", + }.AsV1(), + } + + // Setup the context and seed test event + ctx, _ := rtesting.SetupFakeContext(t) + ctx = WithClient(ctx, &FakeClientBehaviour{SendSuccessfully: true}, 0) + fakeClient := Get(ctx).(FakeClient) + + // the channel size is 0 so no more events can be sent + err := fakeClient.Send(ctx, sendEvent) + if err == nil { + t.Fatalf("want err but got nil") + } + +} diff --git a/pkg/reconciler/events/event_test.go b/pkg/reconciler/events/event_test.go index 5462b137ff5..c54c711e514 100644 --- a/pkg/reconciler/events/event_test.go +++ b/pkg/reconciler/events/event_test.go @@ -143,7 +143,6 @@ func TestSendKubernetesEvents(t *testing.T) { fr := record.NewFakeRecorder(1) tr := &corev1.Pod{} sendKubernetesEvents(fr, ts.before, ts.after, tr) - err := eventstest.CheckEventsOrdered(t, fr.Events, ts.name, ts.wantEvents) if err != nil { t.Errorf(err.Error()) @@ -170,7 +169,6 @@ func TestEmitError(t *testing.T) { fr := record.NewFakeRecorder(1) tr := &corev1.Pod{} EmitError(fr, ts.err, tr) - err := eventstest.CheckEventsOrdered(t, fr.Events, ts.name, ts.wantEvents) if err != nil { t.Errorf(err.Error()) @@ -222,7 +220,7 @@ func TestEmit(t *testing.T) { for _, tc := range testcases { // Setup the context and seed test data ctx, _ := rtesting.SetupFakeContext(t) - ctx = cloudevent.WithClient(ctx, &cloudevent.FakeClientBehaviour{SendSuccessfully: true}) + ctx = cloudevent.WithClient(ctx, &cloudevent.FakeClientBehaviour{SendSuccessfully: true}, len(tc.wantCloudEvents)) fakeClient := cloudevent.Get(ctx).(cloudevent.FakeClient) // Setup the config and add it to the context @@ -239,9 +237,7 @@ func TestEmit(t *testing.T) { if err := eventstest.CheckEventsOrdered(t, recorder.Events, tc.name, tc.wantEvents); err != nil { t.Fatalf(err.Error()) } - if err := eventstest.CheckEventsUnordered(t, fakeClient.Events, tc.name, tc.wantCloudEvents); err != nil { - t.Fatalf(err.Error()) - } + fakeClient.CheckCloudEventsUnordered(t, tc.name, tc.wantCloudEvents) } } @@ -278,7 +274,7 @@ func TestEmitCloudEvents(t *testing.T) { for _, tc := range testcases { // Setup the context and seed test data ctx, _ := rtesting.SetupFakeContext(t) - ctx = cloudevent.WithClient(ctx, &cloudevent.FakeClientBehaviour{SendSuccessfully: true}) + ctx = cloudevent.WithClient(ctx, &cloudevent.FakeClientBehaviour{SendSuccessfully: true}, len(tc.wantCloudEvents)) fakeClient := cloudevent.Get(ctx).(cloudevent.FakeClient) // Setup the config and add it to the context @@ -295,8 +291,6 @@ func TestEmitCloudEvents(t *testing.T) { if err := eventstest.CheckEventsOrdered(t, recorder.Events, tc.name, tc.wantEvents); err != nil { t.Fatalf(err.Error()) } - if err := eventstest.CheckEventsUnordered(t, fakeClient.Events, tc.name, tc.wantCloudEvents); err != nil { - t.Fatalf(err.Error()) - } + fakeClient.CheckCloudEventsUnordered(t, tc.name, tc.wantCloudEvents) } } diff --git a/pkg/reconciler/pipelinerun/pipelinerun_test.go b/pkg/reconciler/pipelinerun/pipelinerun_test.go index 51a5d4593bf..3345595c05f 100644 --- a/pkg/reconciler/pipelinerun/pipelinerun_test.go +++ b/pkg/reconciler/pipelinerun/pipelinerun_test.go @@ -134,6 +134,7 @@ func getPipelineRunController(t *testing.T, d test.Data) (test.Assets, func()) { // controller initialization. func initializePipelineRunControllerAssets(t *testing.T, d test.Data, opts pipeline.Options) (test.Assets, func()) { ctx, _ := ttesting.SetupFakeContext(t) + ctx = ttesting.SetupFakeCloudClientContext(ctx, d.ExpectedCloudEventCount) ctx, cancel := context.WithCancel(ctx) test.EnsureConfigurationConfigMapsExist(&d) c, informers := test.SeedTestData(t, ctx, d) @@ -6579,19 +6580,21 @@ spec: } t.Logf("config maps: %s", cms) + wantEvents := []string{ + "Normal Started", + "Normal Running Tasks Completed: 0", + } + d := test.Data{ - PipelineRuns: prs, - Pipelines: ps, - Tasks: ts, - ConfigMaps: cms, + PipelineRuns: prs, + Pipelines: ps, + Tasks: ts, + ConfigMaps: cms, + ExpectedCloudEventCount: len(wantEvents), } prt := newPipelineRunTest(d, t) defer prt.Cancel() - wantEvents := []string{ - "Normal Started", - "Normal Running Tasks Completed: 0", - } reconciledRun, clients := prt.reconcileRun("foo", "test-pipelinerun", wantEvents, false) // This PipelineRun is in progress now and the status should reflect that @@ -6604,9 +6607,7 @@ spec: `(?s)dev.tekton.event.pipelinerun.running.v1.*test-pipelinerun`, } ceClient := clients.CloudEvents.(cloudevent.FakeClient) - if err := eventstest.CheckEventsUnordered(t, ceClient.Events, "reconcile-cloud-events", wantCloudEvents); err != nil { - t.Errorf(err.Error()) - } + ceClient.CheckCloudEventsUnordered(t, "reconcile-cloud-events", wantCloudEvents) } // this test validates taskSpec metadata is embedded into task run diff --git a/pkg/reconciler/run/run_test.go b/pkg/reconciler/run/run_test.go index ac55f37ccc1..2897c3e58fd 100644 --- a/pkg/reconciler/run/run_test.go +++ b/pkg/reconciler/run/run_test.go @@ -32,7 +32,6 @@ import ( "github.com/tektoncd/pipeline/pkg/reconciler/events/cloudevent" ttesting "github.com/tektoncd/pipeline/pkg/reconciler/testing" "github.com/tektoncd/pipeline/test" - eventstest "github.com/tektoncd/pipeline/test/events" "github.com/tektoncd/pipeline/test/names" corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -49,6 +48,7 @@ import ( func initializeRunControllerAssets(t *testing.T, d test.Data) (test.Assets, func()) { ctx, _ := ttesting.SetupFakeContext(t) + ctx = ttesting.SetupFakeCloudClientContext(ctx, d.ExpectedCloudEventCount) ctx, cancel := context.WithCancel(ctx) test.EnsureConfigurationConfigMapsExist(&d) c, informers := test.SeedTestData(t, ctx, d) @@ -160,8 +160,9 @@ func TestReconcile_CloudEvents(t *testing.T) { runs := []*v1alpha1.Run{&run} d := test.Data{ - Runs: runs, - ConfigMaps: cms, + Runs: runs, + ConfigMaps: cms, + ExpectedCloudEventCount: len(tc.wantCloudEvents), } testAssets, cancel := getRunController(t, d) defer cancel() @@ -189,19 +190,13 @@ func TestReconcile_CloudEvents(t *testing.T) { } ceClient := clients.CloudEvents.(cloudevent.FakeClient) - err = eventstest.CheckEventsUnordered(t, ceClient.Events, tc.name, tc.wantCloudEvents) - if err != nil { - t.Errorf(err.Error()) - } + ceClient.CheckCloudEventsUnordered(t, tc.name, tc.wantCloudEvents) // Try and reconcile again - expect no event if err := c.Reconciler.Reconcile(testAssets.Ctx, getRunName(run)); err != nil { t.Fatal("Didn't expect an error, but got one.") } - err = eventstest.CheckEventsUnordered(t, ceClient.Events, tc.name, []string{}) - if err != nil { - t.Errorf(err.Error()) - } + ceClient.CheckCloudEventsUnordered(t, tc.name, []string{}) }) } } @@ -298,10 +293,7 @@ func TestReconcile_CloudEvents_Disabled(t *testing.T) { } ceClient := clients.CloudEvents.(cloudevent.FakeClient) - err = eventstest.CheckEventsUnordered(t, ceClient.Events, tc.name, []string{}) - if err != nil { - t.Errorf(err.Error()) - } + ceClient.CheckCloudEventsUnordered(t, tc.name, []string{}) }) } } diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index 9b0257a16b7..0d4fe769ef5 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -478,6 +478,7 @@ func getTaskRunController(t *testing.T, d test.Data) (test.Assets, func()) { func initializeTaskRunControllerAssets(t *testing.T, d test.Data, opts pipeline.Options) (test.Assets, func()) { ctx, _ := ttesting.SetupFakeContext(t) + ctx = ttesting.SetupFakeCloudClientContext(ctx, d.ExpectedCloudEventCount) ctx, cancel := context.WithCancel(ctx) test.EnsureConfigurationConfigMapsExist(&d) c, informers := test.SeedTestData(t, ctx, d) @@ -658,6 +659,13 @@ spec: }, } + wantEvents := []string{ + "Normal Start", + "Normal Running", + } + + d.ExpectedCloudEventCount = len(wantEvents) + testAssets, cancel := getTaskRunController(t, d) defer cancel() c := testAssets.Controller @@ -693,10 +701,6 @@ spec: t.Errorf("Expected reason %q but was %s", v1beta1.TaskRunReasonRunning.String(), condition.Reason) } - wantEvents := []string{ - "Normal Start", - "Normal Running", - } err = eventstest.CheckEventsOrdered(t, testAssets.Recorder.Events, "reconcile-cloud-events", wantEvents) if !(err == nil) { t.Errorf(err.Error()) @@ -707,10 +711,7 @@ spec: `(?s)dev.tekton.event.taskrun.running.v1.*test-taskrun-not-started`, } ceClient := clients.CloudEvents.(cloudevent.FakeClient) - err = eventstest.CheckEventsUnordered(t, ceClient.Events, "reconcile-cloud-events", wantCloudEvents) - if !(err == nil) { - t.Errorf(err.Error()) - } + ceClient.CheckCloudEventsUnordered(t, "reconcile-cloud-events", wantCloudEvents) } func TestReconcile(t *testing.T) { @@ -2986,6 +2987,7 @@ status: }, }} { t.Run(tc.name, func(t *testing.T) { + d.ExpectedCloudEventCount = len(tc.wantCloudEvents) testAssets, cancel := getTaskRunController(t, d) defer cancel() c := testAssets.Controller diff --git a/pkg/reconciler/testing/logger.go b/pkg/reconciler/testing/logger.go index c7b8abb09c3..603df1d8c8b 100644 --- a/pkg/reconciler/testing/logger.go +++ b/pkg/reconciler/testing/logger.go @@ -24,11 +24,15 @@ import ( // SetupFakeContext sets up the Context and the fake filtered informers for the tests. func SetupFakeContext(t *testing.T) (context.Context, []controller.Informer) { ctx, _, informer := setupFakeContextWithLabelKey(t) + return WithLogger(ctx, t), informer +} + +// SetupFakeCloudClientContext sets up the fakeclient to context +func SetupFakeCloudClientContext(ctx context.Context, expectedEventCount int) context.Context { cloudEventClientBehaviour := cloudevent.FakeClientBehaviour{ SendSuccessfully: true, } - ctx = cloudevent.WithClient(ctx, &cloudEventClientBehaviour) - return WithLogger(ctx, t), informer + return cloudevent.WithClient(ctx, &cloudEventClientBehaviour, expectedEventCount) } // SetupDefaultContext sets up the Context and the default filtered informers for the tests. diff --git a/test/controller.go b/test/controller.go index d9b6b16c3bd..ff3f3f7a20e 100644 --- a/test/controller.go +++ b/test/controller.go @@ -71,20 +71,21 @@ import ( // Data represents the desired state of the system (i.e. existing resources) to seed controllers // with. type Data struct { - PipelineRuns []*v1beta1.PipelineRun - Pipelines []*v1beta1.Pipeline - TaskRuns []*v1beta1.TaskRun - Tasks []*v1beta1.Task - ClusterTasks []*v1beta1.ClusterTask - PipelineResources []*resourcev1alpha1.PipelineResource - Runs []*v1alpha1.Run - CustomRuns []*v1beta1.CustomRun - Pods []*corev1.Pod - Namespaces []*corev1.Namespace - ConfigMaps []*corev1.ConfigMap - ServiceAccounts []*corev1.ServiceAccount - LimitRange []*corev1.LimitRange - ResolutionRequests []*resolutionv1alpha1.ResolutionRequest + PipelineRuns []*v1beta1.PipelineRun + Pipelines []*v1beta1.Pipeline + TaskRuns []*v1beta1.TaskRun + Tasks []*v1beta1.Task + ClusterTasks []*v1beta1.ClusterTask + PipelineResources []*resourcev1alpha1.PipelineResource + Runs []*v1alpha1.Run + CustomRuns []*v1beta1.CustomRun + Pods []*corev1.Pod + Namespaces []*corev1.Namespace + ConfigMaps []*corev1.ConfigMap + ServiceAccounts []*corev1.ServiceAccount + LimitRange []*corev1.LimitRange + ResolutionRequests []*resolutionv1alpha1.ResolutionRequest + ExpectedCloudEventCount int } // Clients holds references to clients which are useful for reconciler tests. diff --git a/test/events/events.go b/test/events/events.go index 22e3056d10a..f4a321c9099 100644 --- a/test/events/events.go +++ b/test/events/events.go @@ -35,19 +35,6 @@ func CheckEventsOrdered(t *testing.T, eventChan chan string, testName string, wa return nil } -// CheckEventsUnordered checks that all events in wantEvents, and no others, -// were received via the given chan, in any order. -func CheckEventsUnordered(t *testing.T, eventChan chan string, testName string, wantEvents []string) error { - t.Helper() - // Sleep 50ms to make sure events have delivered - time.Sleep(50 * time.Millisecond) - err := eventsFromChannelUnordered(eventChan, wantEvents) - if err != nil { - return fmt.Errorf("error in test %s: %v", testName, err) - } - return nil -} - // eventsFromChannel takes a chan of string, a test name, and a list of events that a test // expects to receive. The events must be received in the same order they appear in the // wantEvents list. Any extra or too few received events are considered errors. @@ -87,44 +74,3 @@ func eventsFromChannel(c chan string, wantEvents []string) error { } return nil } - -// eventsFromChannelUnordered takes a chan of string and a list of events that a test -// expects to receive. The events can be received in any order. Any extra or too few -// events are both considered errors. -func eventsFromChannelUnordered(c chan string, wantEvents []string) error { - timer := time.NewTimer(10 * time.Millisecond) - expected := append([]string{}, wantEvents...) - // loop len(expected) + 1 times to catch extra erroneous events received that the test is not expecting - maxEvents := len(expected) + 1 - for eventCount := 0; eventCount < maxEvents; eventCount++ { - select { - case event := <-c: - if len(expected) == 0 { - return fmt.Errorf("extra event received: %q", event) - } - found := false - for wantIdx, want := range expected { - matching, err := regexp.MatchString(want, event) - if err != nil { - return fmt.Errorf("something went wrong matching an event: %s", err) - } - if matching { - found = true - // Remove event from list of those we expect to receive - expected[wantIdx] = expected[len(expected)-1] - expected = expected[:len(expected)-1] - break - } - } - if !found { - return fmt.Errorf("unexpected event received: %q", event) - } - case <-timer.C: - if len(expected) != 0 { - return fmt.Errorf("timed out waiting for %d more events: %#v", len(expected), expected) - } - return nil - } - } - return fmt.Errorf("too many events received") -}