From ebb6fb5b0ce86851afe6886bd3f05915ceac216c Mon Sep 17 00:00:00 2001 From: drfish Date: Tue, 2 Feb 2021 21:41:20 +0800 Subject: [PATCH] Organize scheduler unit tests into subtests --- pkg/scheduler/core/generic_scheduler_test.go | 66 +++++++-------- pkg/scheduler/internal/cache/cache_test.go | 82 ++++++++++--------- pkg/scheduler/internal/cache/snapshot_test.go | 25 +++--- pkg/scheduler/util/utils_test.go | 10 ++- 4 files changed, 98 insertions(+), 85 deletions(-) diff --git a/pkg/scheduler/core/generic_scheduler_test.go b/pkg/scheduler/core/generic_scheduler_test.go index fe079d70ea5..ea172e7f45a 100644 --- a/pkg/scheduler/core/generic_scheduler_test.go +++ b/pkg/scheduler/core/generic_scheduler_test.go @@ -1146,42 +1146,44 @@ func TestFindFitPredicateCallCounts(t *testing.T) { } for _, test := range tests { - nodes := makeNodeList([]string{"1"}) + t.Run(test.name, func(t *testing.T) { + nodes := makeNodeList([]string{"1"}) - plugin := st.FakeFilterPlugin{} - registerFakeFilterFunc := st.RegisterFilterPlugin( - "FakeFilter", - func(_ runtime.Object, fh framework.Handle) (framework.Plugin, error) { - return &plugin, nil - }, - ) - registerPlugins := []st.RegisterPluginFunc{ - st.RegisterQueueSortPlugin(queuesort.Name, queuesort.New), - registerFakeFilterFunc, - st.RegisterBindPlugin(defaultbinder.Name, defaultbinder.New), - } - fwk, err := st.NewFramework( - registerPlugins, - frameworkruntime.WithPodNominator(internalqueue.NewPodNominator()), - ) - if err != nil { - t.Fatal(err) - } + plugin := st.FakeFilterPlugin{} + registerFakeFilterFunc := st.RegisterFilterPlugin( + "FakeFilter", + func(_ runtime.Object, fh framework.Handle) (framework.Plugin, error) { + return &plugin, nil + }, + ) + registerPlugins := []st.RegisterPluginFunc{ + st.RegisterQueueSortPlugin(queuesort.Name, queuesort.New), + registerFakeFilterFunc, + st.RegisterBindPlugin(defaultbinder.Name, defaultbinder.New), + } + fwk, err := st.NewFramework( + registerPlugins, + frameworkruntime.WithPodNominator(internalqueue.NewPodNominator()), + ) + if err != nil { + t.Fatal(err) + } - scheduler := makeScheduler(nodes) - if err := scheduler.cache.UpdateSnapshot(scheduler.nodeInfoSnapshot); err != nil { - t.Fatal(err) - } - fwk.PreemptHandle().AddNominatedPod(&v1.Pod{ObjectMeta: metav1.ObjectMeta{UID: "nominated"}, Spec: v1.PodSpec{Priority: &midPriority}}, "1") + scheduler := makeScheduler(nodes) + if err := scheduler.cache.UpdateSnapshot(scheduler.nodeInfoSnapshot); err != nil { + t.Fatal(err) + } + fwk.PreemptHandle().AddNominatedPod(&v1.Pod{ObjectMeta: metav1.ObjectMeta{UID: "nominated"}, Spec: v1.PodSpec{Priority: &midPriority}}, "1") - _, _, err = scheduler.findNodesThatFitPod(context.Background(), fwk, framework.NewCycleState(), test.pod) + _, _, err = scheduler.findNodesThatFitPod(context.Background(), fwk, framework.NewCycleState(), test.pod) - if err != nil { - t.Errorf("unexpected error: %v", err) - } - if test.expectedCount != plugin.NumFilterCalled { - t.Errorf("predicate was called %d times, expected is %d", plugin.NumFilterCalled, test.expectedCount) - } + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if test.expectedCount != plugin.NumFilterCalled { + t.Errorf("predicate was called %d times, expected is %d", plugin.NumFilterCalled, test.expectedCount) + } + }) } } diff --git a/pkg/scheduler/internal/cache/cache_test.go b/pkg/scheduler/internal/cache/cache_test.go index 272270622e0..c4d279221a4 100644 --- a/pkg/scheduler/internal/cache/cache_test.go +++ b/pkg/scheduler/internal/cache/cache_test.go @@ -394,32 +394,34 @@ func TestDump(t *testing.T) { podsToAdd: []*v1.Pod{testPods[0]}, }} - for _, tt := range tests { - cache := newSchedulerCache(ttl, time.Second, nil) - for _, podToAssume := range tt.podsToAssume { - if err := assumeAndFinishBinding(cache, podToAssume, now); err != nil { - t.Errorf("assumePod failed: %v", err) + for i, tt := range tests { + t.Run(fmt.Sprintf("case_%d", i), func(t *testing.T) { + cache := newSchedulerCache(ttl, time.Second, nil) + for _, podToAssume := range tt.podsToAssume { + if err := assumeAndFinishBinding(cache, podToAssume, now); err != nil { + t.Errorf("assumePod failed: %v", err) + } } - } - for _, podToAdd := range tt.podsToAdd { - if err := cache.AddPod(podToAdd); err != nil { - t.Errorf("AddPod failed: %v", err) + for _, podToAdd := range tt.podsToAdd { + if err := cache.AddPod(podToAdd); err != nil { + t.Errorf("AddPod failed: %v", err) + } } - } - snapshot := cache.Dump() - if len(snapshot.Nodes) != len(cache.nodes) { - t.Errorf("Unequal number of nodes in the cache and its snapshot. expected: %v, got: %v", len(cache.nodes), len(snapshot.Nodes)) - } - for name, ni := range snapshot.Nodes { - nItem := cache.nodes[name] - if !reflect.DeepEqual(ni, nItem.info) { - t.Errorf("expect \n%+v; got \n%+v", nItem.info, ni) + snapshot := cache.Dump() + if len(snapshot.Nodes) != len(cache.nodes) { + t.Errorf("Unequal number of nodes in the cache and its snapshot. expected: %v, got: %v", len(cache.nodes), len(snapshot.Nodes)) } - } - if !reflect.DeepEqual(snapshot.AssumedPods, cache.assumedPods) { - t.Errorf("expect \n%+v; got \n%+v", cache.assumedPods, snapshot.AssumedPods) - } + for name, ni := range snapshot.Nodes { + nItem := cache.nodes[name] + if !reflect.DeepEqual(ni, nItem.info) { + t.Errorf("expect \n%+v; got \n%+v", nItem.info, ni) + } + } + if !reflect.DeepEqual(snapshot.AssumedPods, cache.assumedPods) { + t.Errorf("expect \n%+v; got \n%+v", cache.assumedPods, snapshot.AssumedPods) + } + }) } } @@ -647,26 +649,28 @@ func TestUpdatePodAndGet(t *testing.T) { }, } - for _, tt := range tests { - cache := newSchedulerCache(ttl, time.Second, nil) + for i, tt := range tests { + t.Run(fmt.Sprintf("case_%d", i), func(t *testing.T) { + cache := newSchedulerCache(ttl, time.Second, nil) - if err := tt.handler(cache, tt.pod); err != nil { - t.Fatalf("unexpected err: %v", err) - } - - if !tt.assumePod { - if err := cache.UpdatePod(tt.pod, tt.podToUpdate); err != nil { - t.Fatalf("UpdatePod failed: %v", err) + if err := tt.handler(cache, tt.pod); err != nil { + t.Fatalf("unexpected err: %v", err) } - } - cachedPod, err := cache.GetPod(tt.pod) - if err != nil { - t.Fatalf("GetPod failed: %v", err) - } - if !reflect.DeepEqual(tt.podToUpdate, cachedPod) { - t.Fatalf("pod get=%s, want=%s", cachedPod, tt.podToUpdate) - } + if !tt.assumePod { + if err := cache.UpdatePod(tt.pod, tt.podToUpdate); err != nil { + t.Fatalf("UpdatePod failed: %v", err) + } + } + + cachedPod, err := cache.GetPod(tt.pod) + if err != nil { + t.Fatalf("GetPod failed: %v", err) + } + if !reflect.DeepEqual(tt.podToUpdate, cachedPod) { + t.Fatalf("pod get=%s, want=%s", cachedPod, tt.podToUpdate) + } + }) } } diff --git a/pkg/scheduler/internal/cache/snapshot_test.go b/pkg/scheduler/internal/cache/snapshot_test.go index e4e9712b87a..94c5d5a2ea5 100644 --- a/pkg/scheduler/internal/cache/snapshot_test.go +++ b/pkg/scheduler/internal/cache/snapshot_test.go @@ -17,6 +17,7 @@ limitations under the License. package cache import ( + "fmt" "reflect" "testing" @@ -82,11 +83,13 @@ func TestGetNodeImageStates(t *testing.T) { }, } - for _, test := range tests { - imageStates := getNodeImageStates(test.node, test.imageExistenceMap) - if !reflect.DeepEqual(test.expected, imageStates) { - t.Errorf("expected: %#v, got: %#v", test.expected, imageStates) - } + for i, test := range tests { + t.Run(fmt.Sprintf("case_%d", i), func(t *testing.T) { + imageStates := getNodeImageStates(test.node, test.imageExistenceMap) + if !reflect.DeepEqual(test.expected, imageStates) { + t.Errorf("expected: %#v, got: %#v", test.expected, imageStates) + } + }) } } @@ -168,10 +171,12 @@ func TestCreateImageExistenceMap(t *testing.T) { }, } - for _, test := range tests { - imageMap := createImageExistenceMap(test.nodes) - if !reflect.DeepEqual(test.expected, imageMap) { - t.Errorf("expected: %#v, got: %#v", test.expected, imageMap) - } + for i, test := range tests { + t.Run(fmt.Sprintf("case_%d", i), func(t *testing.T) { + imageMap := createImageExistenceMap(test.nodes) + if !reflect.DeepEqual(test.expected, imageMap) { + t.Errorf("expected: %#v, got: %#v", test.expected, imageMap) + } + }) } } diff --git a/pkg/scheduler/util/utils_test.go b/pkg/scheduler/util/utils_test.go index 0a7e6f5967a..c48e6d7336a 100644 --- a/pkg/scheduler/util/utils_test.go +++ b/pkg/scheduler/util/utils_test.go @@ -111,10 +111,12 @@ func TestMoreImportantPod(t *testing.T) { } for k, v := range tests { - got := MoreImportantPod(v.p1, v.p2) - if got != v.expected { - t.Errorf("%s failed, expected %t but got %t", k, v.expected, got) - } + t.Run(k, func(t *testing.T) { + got := MoreImportantPod(v.p1, v.p2) + if got != v.expected { + t.Errorf("expected %t but got %t", v.expected, got) + } + }) } }