Skip to content

Commit

Permalink
Refactor CloudEvent related tests to use common set of helpers
Browse files Browse the repository at this point in the history
Prior to this change, the event-related testing helpers were copied
and pasted in several files. The TaskRun Reconcile tests were updated
to not expect cloud events in a particular order, reducing flakiness,
but the PipelineRun reconcile tests and the Event tests did not receive
the same updates.

This commit moves event-related test helpers into a common package
and updates all CloudEvent tests to not expect events to occur in a given order.
This should address some of the causes of flakes in #2992 but not all.

Co-authored-by: Scott Seaward [email protected]
  • Loading branch information
lbernick committed Dec 3, 2021
1 parent 620443c commit d003103
Show file tree
Hide file tree
Showing 5 changed files with 192 additions and 265 deletions.
51 changes: 10 additions & 41 deletions pkg/reconciler/events/cloudevent/cloud_event_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,7 @@ package cloudevent

import (
"context"
"fmt"
"strings"
"testing"
"time"

"github.com/google/go-cmp/cmp"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
Expand Down Expand Up @@ -553,8 +550,8 @@ func TestSendCloudEventWithRetries(t *testing.T) {
name string
clientBehaviour FakeClientBehaviour
object objectWithCondition
wantCEvent string
wantEvent string
wantCEvents []string
wantEvents []string
}{{
name: "test-send-cloud-event-taskrun",
clientBehaviour: FakeClientBehaviour{
Expand All @@ -566,8 +563,8 @@ func TestSendCloudEventWithRetries(t *testing.T) {
},
Status: v1beta1.TaskRunStatus{Status: objectStatus},
},
wantCEvent: "Context Attributes,",
wantEvent: "",
wantCEvents: []string{"Context Attributes,"},
wantEvents: []string{},
}, {
name: "test-send-cloud-event-pipelinerun",
clientBehaviour: FakeClientBehaviour{
Expand All @@ -579,8 +576,8 @@ func TestSendCloudEventWithRetries(t *testing.T) {
},
Status: v1beta1.PipelineRunStatus{Status: objectStatus},
},
wantCEvent: "Context Attributes,",
wantEvent: "",
wantCEvents: []string{"Context Attributes,"},
wantEvents: []string{},
}, {
name: "test-send-cloud-event-failed",
clientBehaviour: FakeClientBehaviour{
Expand All @@ -589,8 +586,8 @@ func TestSendCloudEventWithRetries(t *testing.T) {
object: &v1beta1.PipelineRun{
Status: v1beta1.PipelineRunStatus{Status: objectStatus},
},
wantCEvent: "",
wantEvent: "Warning Cloud Event Failure",
wantCEvents: []string{},
wantEvents: []string{"Warning Cloud Event Failure"},
}}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
Expand All @@ -601,11 +598,11 @@ func TestSendCloudEventWithRetries(t *testing.T) {
t.Fatalf("Unexpected error sending cloud events: %v", err)
}
ceClient := Get(ctx).(FakeClient)
if err := checkCloudEvents(t, &ceClient, tc.name, tc.wantCEvent); err != nil {
if err := CheckCloudEvents(t, &ceClient, tc.name, tc.wantCEvents); err != nil {
t.Fatalf(err.Error())
}
recorder := controller.GetEventRecorder(ctx).(*record.FakeRecorder)
if err := checkEvents(t, recorder, tc.name, tc.wantEvent); err != nil {
if err := CheckEvents(t, recorder, tc.name, tc.wantEvents); err != nil {
t.Fatalf(err.Error())
}
})
Expand Down Expand Up @@ -669,31 +666,3 @@ func setupFakeContext(t *testing.T, behaviour FakeClientBehaviour, withClient bo
}
return ctx
}

func eventFromChannel(c chan string, testName string, wantEvent string) error {
timer := time.NewTimer(10 * time.Millisecond)
select {
case event := <-c:
if wantEvent == "" {
return fmt.Errorf("received event \"%s\" for %s but none expected", event, testName)
}
if !(strings.HasPrefix(event, wantEvent)) {
return fmt.Errorf("expected event \"%s\" but got \"%s\" instead for %s", wantEvent, event, testName)
}
case <-timer.C:
if wantEvent != "" {
return fmt.Errorf("received no events for %s but %s expected", testName, wantEvent)
}
}
return nil
}

func checkEvents(t *testing.T, fr *record.FakeRecorder, testName string, wantEvent string) error {
t.Helper()
return eventFromChannel(fr.Events, testName, wantEvent)
}

func checkCloudEvents(t *testing.T, fce *FakeClient, testName string, wantEvent string) error {
t.Helper()
return eventFromChannel(fce.Events, testName, wantEvent)
}
128 changes: 128 additions & 0 deletions pkg/reconciler/events/cloudevent/testing.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
/*
Copyright 2021 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 (
"fmt"
"regexp"
"testing"
"time"

"k8s.io/client-go/tools/record"
)

// CheckEvents checks that the events received by the FakeRecorder are the same as wantEvents,
// in the same order.
func CheckEvents(t *testing.T, fr *record.FakeRecorder, testName string, wantEvents []string) error {
t.Helper()
err := eventsFromChannel(fr.Events, wantEvents)
if err != nil {
return fmt.Errorf("error in test %s: %v", testName, err)
}
return nil
}

// CheckCloudEvents checks that all events in wantEvents, and no others, were received by the FakeClient
// in any order.
func CheckCloudEvents(t *testing.T, fce *FakeClient, testName string, wantEvents []string) error {
t.Helper()
err := eventsFromChannelUnordered(fce.Events, 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.
func eventsFromChannel(c chan string, wantEvents []string) error {
// We get events from a channel, so the timeout is here to avoid waiting
// on the channel forever if fewer than expected events are received.
// We only hit the timeout in case of failure of the test, so the actual value
// of the timeout is not so relevant, it's only used when tests are going to fail.
// on the channel forever if fewer than expected events are received
timer := time.NewTimer(10 * time.Millisecond)
foundEvents := []string{}
for ii := 0; ii < len(wantEvents)+1; ii++ {
// We loop over all the events that we expect. Once they are all received
// we exit the loop. If we never receive enough events, the timeout takes us
// out of the loop.
select {
case event := <-c:
foundEvents = append(foundEvents, event)
if ii > len(wantEvents)-1 {
return fmt.Errorf("received event \"%s\" but not more expected", event)
}
wantEvent := wantEvents[ii]
matching, err := regexp.MatchString(wantEvent, event)
if err == nil {
if !matching {
return fmt.Errorf("expected event \"%s\" but got \"%s\" instead", wantEvent, event)
}
} else {
return fmt.Errorf("something went wrong matching the event: %s", err)
}
case <-timer.C:
if len(foundEvents) != len(wantEvents) {
return fmt.Errorf("received %d events but %d expected. Found events: %#v", len(foundEvents), len(wantEvents), foundEvents)
}
return nil
}
}
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")
}
Loading

0 comments on commit d003103

Please sign in to comment.