From 39af594f312ad46715dfb705d71ad4e607e27e5d Mon Sep 17 00:00:00 2001 From: Lukasz Szaszkiewicz Date: Thu, 14 Sep 2023 11:16:10 +0200 Subject: [PATCH] storage/testing: a simple refactor --- .../pkg/storage/testing/store_tests.go | 60 ++++++------------- .../apiserver/pkg/storage/testing/utils.go | 37 ++++++------ .../pkg/storage/testing/watcher_tests.go | 48 +++++++-------- 3 files changed, 62 insertions(+), 83 deletions(-) diff --git a/staging/src/k8s.io/apiserver/pkg/storage/testing/store_tests.go b/staging/src/k8s.io/apiserver/pkg/storage/testing/store_tests.go index 8b311f88908..da859a57ee7 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/testing/store_tests.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/testing/store_tests.go @@ -83,7 +83,7 @@ func RunTestCreateWithTTL(ctx context.Context, t *testing.T, store storage.Inter if err != nil { t.Fatalf("Watch failed: %v", err) } - testCheckEventType(t, watch.Deleted, w) + testCheckEventType(t, w, watch.Deleted) } func RunTestCreateWithKeyExist(ctx context.Context, t *testing.T, store storage.Interface) { @@ -214,15 +214,8 @@ func RunTestGet(ctx context.Context, t *testing.T, store storage.Interface) { } if tt.expectedAlternatives == nil { - ExpectNoDiff(t, fmt.Sprintf("%s: incorrect pod", tt.name), tt.expectedOut, out) + expectNoDiff(t, fmt.Sprintf("%s: incorrect pod", tt.name), tt.expectedOut, out) } else { - toInterfaceSlice := func(pods []*example.Pod) []interface{} { - result := make([]interface{}, 0, len(pods)) - for i := range pods { - result = append(result, pods[i]) - } - return result - } ExpectContains(t, fmt.Sprintf("%s: incorrect pod", tt.name), toInterfaceSlice(tt.expectedAlternatives), out) } }) @@ -268,7 +261,7 @@ func RunTestUnconditionalDelete(ctx context.Context, t *testing.T, store storage t.Errorf("expecting resource version to be updated, but get: %s", out.ResourceVersion) } out.ResourceVersion = storedObj.ResourceVersion - ExpectNoDiff(t, "incorrect pod:", tt.expectedObj, out) + expectNoDiff(t, "incorrect pod:", tt.expectedObj, out) }) } } @@ -310,7 +303,7 @@ func RunTestConditionalDelete(ctx context.Context, t *testing.T, store storage.I t.Errorf("expecting resource version to be updated, but get: %s", out.ResourceVersion) } out.ResourceVersion = storedObj.ResourceVersion - ExpectNoDiff(t, "incorrect pod:", storedObj, out) + expectNoDiff(t, "incorrect pod:", storedObj, out) obj := &example.Pod{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "test-ns", UID: "A"}} key, storedObj = testPropagateStore(ctx, t, store, obj) }) @@ -1091,16 +1084,8 @@ func RunTestList(ctx context.Context, t *testing.T, store storage.Interface, com if tt.expectedAlternatives == nil { sort.Sort(sortablePodList(tt.expectedOut)) - ExpectNoDiff(t, "incorrect list pods", tt.expectedOut, out.Items) + expectNoDiff(t, "incorrect list pods", tt.expectedOut, out.Items) } else { - toInterfaceSlice := func(podLists [][]example.Pod) []interface{} { - result := make([]interface{}, 0, len(podLists)) - for i := range podLists { - sort.Sort(sortablePodList(podLists[i])) - result = append(result, podLists[i]) - } - return result - } ExpectContains(t, "incorrect list pods", toInterfaceSlice(tt.expectedAlternatives), out.Items) } }) @@ -1174,7 +1159,7 @@ func RunTestListWithoutPaging(ctx context.Context, t *testing.T, store storage.I } for j, wantPod := range tt.expectedOut { getPod := &out.Items[j] - ExpectNoDiff(t, fmt.Sprintf("%s: incorrect pod", tt.name), wantPod, getPod) + expectNoDiff(t, fmt.Sprintf("%s: incorrect pod", tt.name), wantPod, getPod) } }) } @@ -1417,15 +1402,8 @@ func RunTestGetListNonRecursive(ctx context.Context, t *testing.T, store storage } if tt.expectedAlternatives == nil { - ExpectNoDiff(t, "incorrect list pods", tt.expectedOut, out.Items) + expectNoDiff(t, "incorrect list pods", tt.expectedOut, out.Items) } else { - toInterfaceSlice := func(podLists [][]example.Pod) []interface{} { - result := make([]interface{}, 0, len(podLists)) - for i := range podLists { - result = append(result, podLists[i]) - } - return result - } ExpectContains(t, "incorrect list pods", toInterfaceSlice(tt.expectedAlternatives), out.Items) } }) @@ -1498,7 +1476,7 @@ func RunTestListContinuation(ctx context.Context, t *testing.T, store storage.In if len(out.Continue) == 0 { t.Fatalf("No continuation token set") } - ExpectNoDiff(t, "incorrect first page", []example.Pod{*preset[0].storedObj}, out.Items) + expectNoDiff(t, "incorrect first page", []example.Pod{*preset[0].storedObj}, out.Items) if validation != nil { validation(t, 1, 1) } @@ -1520,7 +1498,7 @@ func RunTestListContinuation(ctx context.Context, t *testing.T, store storage.In } key, rv, err := storage.DecodeContinue(continueFromSecondItem, "/pods") t.Logf("continue token was %d %s %v", rv, key, err) - ExpectNoDiff(t, "incorrect second page", []example.Pod{*preset[1].storedObj, *preset[2].storedObj}, out.Items) + expectNoDiff(t, "incorrect second page", []example.Pod{*preset[1].storedObj, *preset[2].storedObj}, out.Items) if validation != nil { validation(t, 0, 2) } @@ -1538,7 +1516,7 @@ func RunTestListContinuation(ctx context.Context, t *testing.T, store storage.In if len(out.Continue) == 0 { t.Fatalf("No continuation token set") } - ExpectNoDiff(t, "incorrect second page", []example.Pod{*preset[1].storedObj}, out.Items) + expectNoDiff(t, "incorrect second page", []example.Pod{*preset[1].storedObj}, out.Items) if validation != nil { validation(t, 1, 1) } @@ -1557,7 +1535,7 @@ func RunTestListContinuation(ctx context.Context, t *testing.T, store storage.In if len(out.Continue) != 0 { t.Fatalf("Unexpected continuation token set") } - ExpectNoDiff(t, "incorrect third page", []example.Pod{*preset[2].storedObj}, out.Items) + expectNoDiff(t, "incorrect third page", []example.Pod{*preset[2].storedObj}, out.Items) if validation != nil { validation(t, 1, 1) } @@ -1668,7 +1646,7 @@ func RunTestListContinuationWithFilter(ctx context.Context, t *testing.T, store if len(out.Continue) == 0 { t.Errorf("No continuation token set") } - ExpectNoDiff(t, "incorrect first page", []example.Pod{*preset[0].storedObj, *preset[2].storedObj}, out.Items) + expectNoDiff(t, "incorrect first page", []example.Pod{*preset[0].storedObj, *preset[2].storedObj}, out.Items) if validation != nil { validation(t, 2, 3) } @@ -1695,7 +1673,7 @@ func RunTestListContinuationWithFilter(ctx context.Context, t *testing.T, store if len(out.Continue) != 0 { t.Errorf("Unexpected continuation token set") } - ExpectNoDiff(t, "incorrect second page", []example.Pod{*preset[3].storedObj}, out.Items) + expectNoDiff(t, "incorrect second page", []example.Pod{*preset[3].storedObj}, out.Items) if validation != nil { validation(t, 2, 1) } @@ -1770,7 +1748,7 @@ func RunTestListInconsistentContinuation(ctx context.Context, t *testing.T, stor if len(out.Continue) == 0 { t.Fatalf("No continuation token set") } - ExpectNoDiff(t, "incorrect first page", []example.Pod{*preset[0].storedObj}, out.Items) + expectNoDiff(t, "incorrect first page", []example.Pod{*preset[0].storedObj}, out.Items) continueFromSecondItem := out.Continue @@ -1830,7 +1808,7 @@ func RunTestListInconsistentContinuation(ctx context.Context, t *testing.T, stor t.Fatalf("No continuation token set") } validateResourceVersion := resourceVersionNotOlderThan(lastRVString) - ExpectNoDiff(t, "incorrect second page", []example.Pod{*preset[1].storedObj}, out.Items) + expectNoDiff(t, "incorrect second page", []example.Pod{*preset[1].storedObj}, out.Items) if err := validateResourceVersion(out.ResourceVersion); err != nil { t.Fatal(err) } @@ -1848,7 +1826,7 @@ func RunTestListInconsistentContinuation(ctx context.Context, t *testing.T, stor if len(out.Continue) != 0 { t.Fatalf("Unexpected continuation token set") } - ExpectNoDiff(t, "incorrect third page", []example.Pod{*preset[2].storedObj}, out.Items) + expectNoDiff(t, "incorrect third page", []example.Pod{*preset[2].storedObj}, out.Items) if out.ResourceVersion != resolvedResourceVersionFromThirdItem { t.Fatalf("Expected list resource version to be %s, got %s", resolvedResourceVersionFromThirdItem, out.ResourceVersion) } @@ -1928,7 +1906,7 @@ func RunTestConsistentList(ctx context.Context, t *testing.T, store InterfaceWit t.Fatalf("failed to list objects: %v", err) } - ExpectNoDiff(t, "incorrect lists", result1, result2) + expectNoDiff(t, "incorrect lists", result1, result2) // Now also verify the ResourceVersionMatchNotOlderThan. options.ResourceVersionMatch = metav1.ResourceVersionMatchNotOlderThan @@ -1946,7 +1924,7 @@ func RunTestConsistentList(ctx context.Context, t *testing.T, store InterfaceWit t.Fatalf("failed to list objects: %v", err) } - ExpectNoDiff(t, "incorrect lists", result3, result4) + expectNoDiff(t, "incorrect lists", result3, result4) } func RunTestGuaranteedUpdate(ctx context.Context, t *testing.T, store InterfaceWithPrefixTransformer, validation KeyValidation) { @@ -2123,7 +2101,7 @@ func RunTestGuaranteedUpdateWithTTL(ctx context.Context, t *testing.T, store sto if err != nil { t.Fatalf("Watch failed: %v", err) } - testCheckEventType(t, watch.Deleted, w) + testCheckEventType(t, w, watch.Deleted) } func RunTestGuaranteedUpdateChecksStoredData(ctx context.Context, t *testing.T, store InterfaceWithPrefixTransformer) { diff --git a/staging/src/k8s.io/apiserver/pkg/storage/testing/utils.go b/staging/src/k8s.io/apiserver/pkg/storage/testing/utils.go index 67559ecd20f..d5b6d608dfc 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/testing/utils.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/testing/utils.go @@ -100,13 +100,13 @@ func testPropagateStore(ctx context.Context, t *testing.T, store storage.Interfa return key, setOutput } -func ExpectNoDiff(t *testing.T, msg string, expected, got interface{}) { +func expectNoDiff(t *testing.T, msg string, expected, actual interface{}) { t.Helper() - if !reflect.DeepEqual(expected, got) { - if diff := cmp.Diff(expected, got); diff != "" { + if !reflect.DeepEqual(expected, actual) { + if diff := cmp.Diff(expected, actual); diff != "" { t.Errorf("%s: %s", msg, diff) } else { - t.Errorf("%s:\nexpected: %#v\ngot: %#v", msg, expected, got) + t.Errorf("%s:\nexpected: %#v\ngot: %#v", msg, expected, actual) } } } @@ -139,7 +139,7 @@ func encodeContinueOrDie(key string, resourceVersion int64) string { return token } -func testCheckEventType(t *testing.T, expectEventType watch.EventType, w watch.Interface) { +func testCheckEventType(t *testing.T, w watch.Interface, expectEventType watch.EventType) { select { case res := <-w.ResultChan(): if res.Type != expectEventType { @@ -150,27 +150,20 @@ func testCheckEventType(t *testing.T, expectEventType watch.EventType, w watch.I } } -func testCheckResult(t *testing.T, expectEventType watch.EventType, w watch.Interface, expectObj runtime.Object) { - testCheckResultFunc(t, expectEventType, w, func(object runtime.Object) error { - ExpectNoDiff(t, "incorrect object", expectObj, object) - return nil +func testCheckResult(t *testing.T, w watch.Interface, expectEvent watch.Event) { + testCheckResultFunc(t, w, func(actualEvent watch.Event) { + expectNoDiff(t, "incorrect event", expectEvent, actualEvent) }) } -func testCheckResultFunc(t *testing.T, expectEventType watch.EventType, w watch.Interface, check func(object runtime.Object) error) { +func testCheckResultFunc(t *testing.T, w watch.Interface, check func(actualEvent watch.Event)) { select { case res := <-w.ResultChan(): - if res.Type != expectEventType { - t.Errorf("event type want=%v, get=%v", expectEventType, res.Type) - return - } obj := res.Object if co, ok := obj.(runtime.CacheableObject); ok { - obj = co.GetObject() - } - if err := check(obj); err != nil { - t.Error(err) + res.Object = co.GetObject() } + check(res) case <-time.After(wait.ForeverTestTimeout): t.Errorf("time out after waiting %v on ResultChan", wait.ForeverTestTimeout) } @@ -194,6 +187,14 @@ func testCheckStop(t *testing.T, w watch.Interface) { } } +func toInterfaceSlice[T any](s []T) []interface{} { + result := make([]interface{}, len(s)) + for i, v := range s { + result[i] = v + } + return result +} + // resourceVersionNotOlderThan returns a function to validate resource versions. Resource versions // referring to points in logical time before the sentinel generate an error. All logical times as // new as the sentinel or newer generate no error. diff --git a/staging/src/k8s.io/apiserver/pkg/storage/testing/watcher_tests.go b/staging/src/k8s.io/apiserver/pkg/storage/testing/watcher_tests.go index 1f08b11c2a5..01f659decf5 100644 --- a/staging/src/k8s.io/apiserver/pkg/storage/testing/watcher_tests.go +++ b/staging/src/k8s.io/apiserver/pkg/storage/testing/watcher_tests.go @@ -184,7 +184,7 @@ func testWatch(ctx context.Context, t *testing.T, store storage.Interface, recur expectObj = prevObj expectObj.ResourceVersion = out.ResourceVersion } - testCheckResult(t, watchTest.watchType, w, expectObj) + testCheckResult(t, w, watch.Event{Type: watchTest.watchType, Object: expectObj}) } prevObj = out } @@ -206,7 +206,7 @@ func RunTestWatchFromZero(ctx context.Context, t *testing.T, store storage.Inter if err != nil { t.Fatalf("Watch failed: %v", err) } - testCheckResult(t, watch.Added, w, storedObj) + testCheckResult(t, w, watch.Event{Type: watch.Added, Object: storedObj}) // Update out := &example.Pod{} @@ -223,7 +223,7 @@ func RunTestWatchFromZero(ctx context.Context, t *testing.T, store storage.Inter // when testing with the Cacher since we may have to allow for slow // processing by allowing updates to propagate to the watch cache. // This allows for that. - testCheckResult(t, watch.Modified, w, out) + testCheckResult(t, w, watch.Event{Type: watch.Modified, Object: out}) w.Stop() // Make sure when we watch from 0 we receive an ADDED event @@ -232,7 +232,7 @@ func RunTestWatchFromZero(ctx context.Context, t *testing.T, store storage.Inter t.Fatalf("Watch failed: %v", err) } - testCheckResult(t, watch.Added, w, out) + testCheckResult(t, w, watch.Event{Type: watch.Added, Object: out}) w.Stop() // Compact previous versions @@ -259,7 +259,7 @@ func RunTestWatchFromZero(ctx context.Context, t *testing.T, store storage.Inter if err != nil { t.Fatalf("Watch failed: %v", err) } - testCheckResult(t, watch.Added, w, newOut) + testCheckResult(t, w, watch.Event{Type: watch.Added, Object: newOut}) // Make sure we can't watch from older resource versions anymoer and get a "Gone" error. tooOldWatcher, err := store.Watch(ctx, key, storage.ListOptions{ResourceVersion: out.ResourceVersion, Predicate: storage.Everything}) @@ -276,11 +276,11 @@ func RunTestWatchFromZero(ctx context.Context, t *testing.T, store storage.Inter Code: http.StatusInternalServerError, Reason: metav1.StatusReasonInternalError, } - testCheckResultFunc(t, watch.Error, tooOldWatcher, func(obj runtime.Object) error { - if !apiequality.Semantic.DeepDerivative(&expiredError, obj) && !apiequality.Semantic.DeepDerivative(&internalError, obj) { - t.Errorf("expected: %#v; got %#v", &expiredError, obj) + testCheckResultFunc(t, tooOldWatcher, func(actualEvent watch.Event) { + expectNoDiff(t, "incorrect event type", watch.Error, actualEvent.Type) + if !apiequality.Semantic.DeepDerivative(&expiredError, actualEvent.Object) && !apiequality.Semantic.DeepDerivative(&internalError, actualEvent.Object) { + t.Errorf("expected: %#v; got %#v", &expiredError, actualEvent.Object) } - return nil }) } @@ -293,7 +293,7 @@ func RunTestDeleteTriggerWatch(ctx context.Context, t *testing.T, store storage. if err := store.Delete(ctx, key, &example.Pod{}, nil, storage.ValidateAllObjectFunc, nil); err != nil { t.Fatalf("Delete failed: %v", err) } - testCheckEventType(t, watch.Deleted, w) + testCheckEventType(t, w, watch.Deleted) } func RunTestWatchFromNonZero(ctx context.Context, t *testing.T, store storage.Interface) { @@ -310,7 +310,7 @@ func RunTestWatchFromNonZero(ctx context.Context, t *testing.T, store storage.In newObj.Annotations = map[string]string{"version": "2"} return newObj, nil }), nil) - testCheckResult(t, watch.Modified, w, out) + testCheckResult(t, w, watch.Event{Type: watch.Modified, Object: out}) } func RunTestDelayedWatchDelivery(ctx context.Context, t *testing.T, store storage.Interface) { @@ -400,7 +400,7 @@ func RunTestWatchError(ctx context.Context, t *testing.T, store InterfaceWithPre if err != nil { t.Fatalf("Watch failed: %v", err) } - testCheckEventType(t, watch.Error, w) + testCheckEventType(t, w, watch.Error) } func RunTestWatchContextCancel(ctx context.Context, t *testing.T, store storage.Interface) { @@ -476,7 +476,7 @@ func RunTestWatcherTimeout(ctx context.Context, t *testing.T, store storage.Inte if err := store.Create(ctx, computePodKey(pod), pod, out, 0); err != nil { t.Fatalf("Create failed: %v", err) } - testCheckResult(t, watch.Added, readingWatcher, out) + testCheckResult(t, readingWatcher, watch.Event{Type: watch.Added, Object: out}) } if time.Since(startTime) > time.Duration(250*nonReadingWatchers)*time.Millisecond { t.Errorf("waiting for events took too long: %v", time.Since(startTime)) @@ -503,7 +503,7 @@ func RunTestWatchDeleteEventObjectHaveLatestRV(ctx context.Context, t *testing.T t.Fatalf("ResourceVersion didn't changed on deletion: %s", deletedObj.ResourceVersion) } - testCheckResult(t, watch.Deleted, w, deletedObj) + testCheckResult(t, w, watch.Event{Type: watch.Deleted, Object: deletedObj}) } func RunTestWatchInitializationSignal(ctx context.Context, t *testing.T, store storage.Interface) { @@ -546,24 +546,24 @@ func RunOptionalTestProgressNotify(ctx context.Context, t *testing.T, store stor // when we send a bookmark event, the client expects the event to contain an // object of the correct type, but with no fields set other than the resourceVersion - testCheckResultFunc(t, watch.Bookmark, w, func(object runtime.Object) error { + testCheckResultFunc(t, w, func(actualEvent watch.Event) { + expectNoDiff(t, "incorrect event type", watch.Bookmark, actualEvent.Type) // first, check that we have the correct resource version - obj, ok := object.(metav1.Object) + obj, ok := actualEvent.Object.(metav1.Object) if !ok { - return fmt.Errorf("got %T, not metav1.Object", object) + t.Fatalf("got %T, not metav1.Object", actualEvent.Object) } if err := validateResourceVersion(obj.GetResourceVersion()); err != nil { - return err + t.Fatal(err) } // then, check that we have the right type and content - pod, ok := object.(*example.Pod) + pod, ok := actualEvent.Object.(*example.Pod) if !ok { - return fmt.Errorf("got %T, not *example.Pod", object) + t.Fatalf("got %T, not *example.Pod", actualEvent.Object) } pod.ResourceVersion = "" - ExpectNoDiff(t, "bookmark event should contain an object with no fields set other than resourceVersion", &example.Pod{}, pod) - return nil + expectNoDiff(t, "bookmark event should contain an object with no fields set other than resourceVersion", &example.Pod{}, pod) }) } @@ -712,7 +712,7 @@ func RunTestClusterScopedWatch(ctx context.Context, t *testing.T, store storage. currentObjs[watchTest.obj.Name] = out } if watchTest.expectEvent { - testCheckResult(t, watchTest.watchType, w, expectObj) + testCheckResult(t, w, watch.Event{Type: watchTest.watchType, Object: expectObj}) } } w.Stop() @@ -1027,7 +1027,7 @@ func RunTestNamespaceScopedWatch(ctx context.Context, t *testing.T, store storag currentObjs[podIdentifier] = out } if watchTest.expectEvent { - testCheckResult(t, watchTest.watchType, w, expectObj) + testCheckResult(t, w, watch.Event{Type: watchTest.watchType, Object: expectObj}) } } w.Stop()