From 0aa9ef9dec8a0fb165159c7c31de755f9f38c26d Mon Sep 17 00:00:00 2001 From: Adam Harwayne Date: Thu, 16 May 2019 10:58:51 -0700 Subject: [PATCH] defaultTriggerFilter is no longer a package level variable, which lead to flakiness. --- .../v1alpha1/trigger_defaults_test.go | 23 +++-- .../v1alpha1/trigger_lifecycle_test.go | 83 ++++++++++--------- 2 files changed, 57 insertions(+), 49 deletions(-) diff --git a/pkg/apis/eventing/v1alpha1/trigger_defaults_test.go b/pkg/apis/eventing/v1alpha1/trigger_defaults_test.go index 4fe1b82ef07..c450a2c31bb 100644 --- a/pkg/apis/eventing/v1alpha1/trigger_defaults_test.go +++ b/pkg/apis/eventing/v1alpha1/trigger_defaults_test.go @@ -24,13 +24,9 @@ import ( ) var ( - defaultBroker = "default" - otherBroker = "other_broker" - defaultTriggerFilter = &TriggerFilter{ - SourceAndType: &TriggerFilterSourceAndType{ - Type: TriggerAnyFilter, - Source: TriggerAnyFilter}, - } + defaultBroker = "default" + otherBroker = "other_broker" + otherTriggerFilter = &TriggerFilter{ SourceAndType: &TriggerFilterSourceAndType{ Type: "other_type", @@ -39,7 +35,7 @@ var ( defaultTrigger = Trigger{ Spec: TriggerSpec{ Broker: defaultBroker, - Filter: defaultTriggerFilter, + Filter: defaultTriggerFilter(), }, } ) @@ -55,7 +51,7 @@ func TestTriggerDefaults(t *testing.T) { }, "nil filter": { initial: Trigger{Spec: TriggerSpec{Broker: otherBroker}}, - expected: Trigger{Spec: TriggerSpec{Broker: otherBroker, Filter: defaultTriggerFilter}}, + expected: Trigger{Spec: TriggerSpec{Broker: otherBroker, Filter: defaultTriggerFilter()}}, }, "nil broker and nil filter": { initial: Trigger{}, @@ -71,3 +67,12 @@ func TestTriggerDefaults(t *testing.T) { }) } } + +func defaultTriggerFilter() *TriggerFilter { + // Can't just be a package level var because it gets mutated. + return &TriggerFilter{ + SourceAndType: &TriggerFilterSourceAndType{ + Type: TriggerAnyFilter, + Source: TriggerAnyFilter}, + } +} diff --git a/pkg/apis/eventing/v1alpha1/trigger_lifecycle_test.go b/pkg/apis/eventing/v1alpha1/trigger_lifecycle_test.go index 9d31a5e1aaa..5c7c31e8f4c 100644 --- a/pkg/apis/eventing/v1alpha1/trigger_lifecycle_test.go +++ b/pkg/apis/eventing/v1alpha1/trigger_lifecycle_test.go @@ -272,47 +272,50 @@ func TestTriggerAnnotateUserInfo(t *testing.T) { this *Trigger prev *Trigger wantedAnns map[string]string - }{{ - "create new trigger", - u1, - &Trigger{}, - nil, - map[string]string{ - eventing.CreatorAnnotation: u1, - eventing.UpdaterAnnotation: u1, - }, - }, { - "update trigger which has no annotations without diff", - u1, - &Trigger{Spec: TriggerSpec{Broker: defaultBroker, Filter: defaultTriggerFilter}}, - &Trigger{Spec: TriggerSpec{Broker: defaultBroker, Filter: defaultTriggerFilter}}, - map[string]string{}, - }, { - "update trigger which has annotations without diff", - u2, - withUserAnns(u1, u1, &Trigger{Spec: TriggerSpec{Broker: defaultBroker, Filter: defaultTriggerFilter}}), - withUserAnns(u1, u1, &Trigger{Spec: TriggerSpec{Broker: defaultBroker, Filter: defaultTriggerFilter}}), - map[string]string{ - eventing.CreatorAnnotation: u1, - eventing.UpdaterAnnotation: u1, - }, - }, { - "update trigger which has no annotations with diff", - u2, - &Trigger{Spec: TriggerSpec{Broker: defaultBroker}}, - &Trigger{Spec: TriggerSpec{Broker: otherBroker}}, - map[string]string{ - eventing.UpdaterAnnotation: u2, - }}, { - "update trigger which has annotations with diff", - u3, - withUserAnns(u1, u2, &Trigger{Spec: TriggerSpec{Broker: otherBroker}}), - withUserAnns(u1, u2, &Trigger{Spec: TriggerSpec{Broker: defaultBroker}}), - map[string]string{ - eventing.CreatorAnnotation: u1, - eventing.UpdaterAnnotation: u3, + }{ + { + name: "create new trigger", + user: u1, + this: &Trigger{}, + prev: nil, + wantedAnns: map[string]string{ + eventing.CreatorAnnotation: u1, + eventing.UpdaterAnnotation: u1, + }, + }, { + name: "update trigger which has no annotations without diff", + user: u1, + this: &Trigger{Spec: TriggerSpec{Broker: defaultBroker, Filter: defaultTriggerFilter()}}, + prev: &Trigger{Spec: TriggerSpec{Broker: defaultBroker, Filter: defaultTriggerFilter()}}, + wantedAnns: map[string]string{}, + }, { + name: "update trigger which has annotations without diff", + user: u2, + this: withUserAnns(u1, u1, &Trigger{Spec: TriggerSpec{Broker: defaultBroker, Filter: defaultTriggerFilter()}}), + prev: withUserAnns(u1, u1, &Trigger{Spec: TriggerSpec{Broker: defaultBroker, Filter: defaultTriggerFilter()}}), + wantedAnns: map[string]string{ + eventing.CreatorAnnotation: u1, + eventing.UpdaterAnnotation: u1, + }, + }, { + name: "update trigger which has no annotations with diff", + user: u2, + this: &Trigger{Spec: TriggerSpec{Broker: defaultBroker}}, + prev: &Trigger{Spec: TriggerSpec{Broker: otherBroker}}, + wantedAnns: map[string]string{ + eventing.UpdaterAnnotation: u2, + }, + }, { + name: "update trigger which has annotations with diff", + user: u3, + this: withUserAnns(u1, u2, &Trigger{Spec: TriggerSpec{Broker: otherBroker}}), + prev: withUserAnns(u1, u2, &Trigger{Spec: TriggerSpec{Broker: defaultBroker}}), + wantedAnns: map[string]string{ + eventing.CreatorAnnotation: u1, + eventing.UpdaterAnnotation: u3, + }, }, - }} + } for _, test := range tests { test := test