knative
/
pkg
mirror of https://github.com/knative/pkg.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Give table test some love it sorely misses (#1573) 

Precache cmp opts, replace unnecessary diffs with equals.
Some string and formatting improvements.
This commit is contained in:
Victor Agababov 2020-07-30 16:14:00 -07:00 committed by GitHub
parent bd4b761236
commit 845edf5423
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 39 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

72
reconciler/testing/table.go
View File

@ -102,6 +102,15 @@ type TableRow struct {
CmpOpts []cmp.Option CmpOpts []cmp.Option
} }
var (
ignoreLastTransitionTime = cmp.FilterPath(func(p cmp.Path) bool {
return strings.HasSuffix(p.String(), "LastTransitionTime.Inner.Time")
}, cmp.Ignore())
ignoreQuantity = cmpopts.IgnoreUnexported(resource.Quantity{})
defaultCmpOpts = []cmp.Option{ignoreLastTransitionTime, ignoreQuantity, cmpopts.EquateEmpty()}
)
func objKey(o runtime.Object) string { func objKey(o runtime.Object) string {
on := o.(kmeta.Accessor) on := o.(kmeta.Accessor)
@ -158,6 +167,7 @@ func (r *TableRow) Test(t *testing.T, factory Factory) {
t.Errorf("Error capturing actions by verb: %q", err) t.Errorf("Error capturing actions by verb: %q", err)
} }
effectiveOpts := append(r.CmpOpts, defaultCmpOpts...)
// Previous state is used to diff resource expected state for update requests that were missed. // Previous state is used to diff resource expected state for update requests that were missed.
objPrevState := make(map[string]runtime.Object, len(r.Objects)) objPrevState := make(map[string]runtime.Object, len(r.Objects))
for _, o := range r.Objects { for _, o := range r.Objects {
@ -177,8 +187,9 @@ func (r *TableRow) Test(t *testing.T, factory Factory) {
t.Errorf("Unexpected action[%d]: %#v", i, got) t.Errorf("Unexpected action[%d]: %#v", i, got)
} }
if diff := cmp.Diff(want, obj, append(r.CmpOpts, ignoreLastTransitionTime, safeDeployDiff, cmpopts.EquateEmpty())...); diff != "" { if !cmp.Equal(want, obj, effectiveOpts...) {
t.Errorf("Unexpected create (-want, +got): %s", diff) t.Errorf("Unexpected create (-want, +got):\n%s",
cmp.Diff(want, obj, effectiveOpts...))
} }
} }
if got, want := len(actions.Creates), len(r.WantCreates); got > want { if got, want := len(actions.Creates), len(r.WantCreates); got > want {
@ -197,8 +208,8 @@ func (r *TableRow) Test(t *testing.T, factory Factory) {
t.Errorf("Object %s was never created: want: %#v", key, wo) t.Errorf("Object %s was never created: want: %#v", key, wo)
continue continue
} }
t.Errorf("Missing update for %s (-want, +prevState): %s", key, t.Errorf("Missing update for %s (-want, +prevState):\n%s", key,
cmp.Diff(wo, oldObj, append(r.CmpOpts, ignoreLastTransitionTime, safeDeployDiff, cmpopts.EquateEmpty())...)) cmp.Diff(wo, oldObj, effectiveOpts...))
continue continue
} }
@ -211,8 +222,9 @@ func (r *TableRow) Test(t *testing.T, factory Factory) {
// Update the object state. // Update the object state.
objPrevState[objKey(got)] = got objPrevState[objKey(got)] = got
if diff := cmp.Diff(want.GetObject(), got, append(r.CmpOpts, ignoreLastTransitionTime, safeDeployDiff, cmpopts.EquateEmpty())...); diff != "" { if !cmp.Equal(want.GetObject(), got, effectiveOpts...) {
t.Errorf("Unexpected update (-want, +got): %s", diff) t.Errorf("Unexpected update (-want, +got):\n%s",
cmp.Diff(want.GetObject(), got, effectiveOpts...))
} }
} }
if got, want := len(updates), len(r.WantUpdates); got > want { if got, want := len(updates), len(r.WantUpdates); got > want {
@ -232,8 +244,8 @@ func (r *TableRow) Test(t *testing.T, factory Factory) {
t.Errorf("Object %s was never created: want: %#v", key, wo) t.Errorf("Object %s was never created: want: %#v", key, wo)
continue continue
} }
t.Errorf("Missing status update for %s (-want, +prevState): %s", key, t.Errorf("Missing status update for %s (-want, +prevState):\n%s", key,
cmp.Diff(wo, oldObj, append(r.CmpOpts, ignoreLastTransitionTime, safeDeployDiff, cmpopts.EquateEmpty())...)) cmp.Diff(wo, oldObj, effectiveOpts...))
continue continue
} }
@ -242,8 +254,9 @@ func (r *TableRow) Test(t *testing.T, factory Factory) {
// Update the object state. // Update the object state.
objPrevState[objKey(got)] = got objPrevState[objKey(got)] = got
if diff := cmp.Diff(want.GetObject(), got, append(r.CmpOpts, ignoreLastTransitionTime, safeDeployDiff, cmpopts.EquateEmpty())...); diff != "" { if !cmp.Equal(want.GetObject(), got, effectiveOpts...) {
t.Errorf("Unexpected status update (-want, +got): %s\nFull: %v", diff, got) t.Errorf("Unexpected status update (-want, +got):\n%s\nFull: %v",
cmp.Diff(want.GetObject(), got, effectiveOpts...), got)
} }
} }
if got, want := len(statusUpdates), len(r.WantStatusUpdates); got > want { if got, want := len(statusUpdates), len(r.WantStatusUpdates); got > want {
@ -255,8 +268,8 @@ func (r *TableRow) Test(t *testing.T, factory Factory) {
t.Errorf("Object %s was never created: want: %#v", key, wo) t.Errorf("Object %s was never created: want: %#v", key, wo)
continue continue
} }
t.Errorf("Extra status update for %s (-extra, +prevState): %s", key, t.Errorf("Extra status update for %s (-extra, +prevState):\n%s", key,
cmp.Diff(wo, oldObj, append(r.CmpOpts, ignoreLastTransitionTime, safeDeployDiff, cmpopts.EquateEmpty())...)) cmp.Diff(wo, oldObj, effectiveOpts...))
} }
} }
@ -328,8 +341,8 @@ func (r *TableRow) Test(t *testing.T, factory Factory) {
got.GetName() != expectedNamespace) { got.GetName() != expectedNamespace) {
t.Errorf("Unexpected patch[%d]: %#v", i, got) t.Errorf("Unexpected patch[%d]: %#v", i, got)
} }
if diff := cmp.Diff(string(want.GetPatch()), string(got.GetPatch())); diff != "" { if got, want := string(got.GetPatch()), string(want.GetPatch()); got != want {
t.Errorf("Unexpected patch(-want, +got): %s", diff) t.Errorf("Unexpected patch(-want, +got):\n%s", cmp.Diff(want, got))
} }
} }
if got, want := len(actions.Patches), len(r.WantPatches); got > want { if got, want := len(actions.Patches), len(r.WantPatches); got > want {
@ -341,17 +354,17 @@ func (r *TableRow) Test(t *testing.T, factory Factory) {
gotEvents := eventList.Events() gotEvents := eventList.Events()
for i, want := range r.WantEvents { for i, want := range r.WantEvents {
if i >= len(gotEvents) { if i >= len(gotEvents) {
t.Errorf("Missing event: %s", want) t.Error("Missing event:", want)
continue continue
} }
if diff := cmp.Diff(want, gotEvents[i]); diff != "" { if !cmp.Equal(want, gotEvents[i]) {
t.Errorf("unexpected event(-want, +got): %s", diff) t.Errorf("Unexpected event(-want, +got):\n%s", cmp.Diff(want, gotEvents[i]))
} }
} }
if got, want := len(gotEvents), len(r.WantEvents); got > want { if got, want := len(gotEvents), len(r.WantEvents); got > want {
for _, extra := range gotEvents[want:] { for _, extra := range gotEvents[want:] {
t.Errorf("Extra event: %s", extra) t.Error("Extra event:", extra)
} }
} }
@ -379,25 +392,18 @@ func (tt TableTest) Test(t *testing.T, factory Factory) {
t.Helper() t.Helper()
for _, test := range tt { for _, test := range tt {
// Record the original objects in table. // Record the original objects in table.
originObjects := make([]runtime.Object, 0, len(test.Objects)) originObjects := make([]runtime.Object, len(test.Objects))
for _, obj := range test.Objects { for i, obj := range test.Objects {
originObjects = append(originObjects, obj.DeepCopyObject()) originObjects[i] = obj.DeepCopyObject()
} }
t.Run(test.Name, func(t *testing.T) { t.Run(test.Name, func(t *testing.T) {
t.Helper() t.Helper()
test.Test(t, factory) test.Test(t, factory)
// Validate cached objects do not get soiled after controller loops.
if !cmp.Equal(originObjects, test.Objects, defaultCmpOpts...) {
t.Errorf("Unexpected objects (-want, +got):\n%s",
cmp.Diff(originObjects, test.Objects, defaultCmpOpts...))
}
}) })
// Validate cached objects do not get soiled after controller loops
if diff := cmp.Diff(originObjects, test.Objects, safeDeployDiff, cmpopts.EquateEmpty()); diff != "" {
t.Errorf("Unexpected objects in test %s (-want, +got): %v", test.Name, diff)
} }
} }
}
var (
ignoreLastTransitionTime = cmp.FilterPath(func(p cmp.Path) bool {
return strings.HasSuffix(p.String(), "LastTransitionTime.Inner.Time")
}, cmp.Ignore())
safeDeployDiff = cmpopts.IgnoreUnexported(resource.Quantity{})
)