diff --git a/pkg/scheduler/factory/cache_comparer_test.go b/pkg/scheduler/factory/cache_comparer_test.go index 1741330995b..2a4ed53df4e 100644 --- a/pkg/scheduler/factory/cache_comparer_test.go +++ b/pkg/scheduler/factory/cache_comparer_test.go @@ -27,27 +27,29 @@ import ( ) func TestCompareNodes(t *testing.T) { - compare := compareStrategy{} - tests := []struct { + name string actual []string cached []string missing []string redundant []string }{ { + name: "redundant cached value", actual: []string{"foo", "bar"}, cached: []string{"bar", "foo", "foobar"}, missing: []string{}, redundant: []string{"foobar"}, }, { + name: "missing cached value", actual: []string{"foo", "bar", "foobar"}, cached: []string{"bar", "foo"}, missing: []string{"foobar"}, redundant: []string{}, }, { + name: "proper cache set", actual: []string{"foo", "bar", "foobar"}, cached: []string{"bar", "foobar", "foo"}, missing: []string{}, @@ -56,34 +58,40 @@ func TestCompareNodes(t *testing.T) { } for _, test := range tests { - nodes := []*v1.Node{} - for _, nodeName := range test.actual { - node := &v1.Node{} - node.Name = nodeName - nodes = append(nodes, node) - } + t.Run(test.name, func(t *testing.T) { + testCompareNodes(test.actual, test.cached, test.missing, test.redundant, t) + }) + } +} - nodeInfo := make(map[string]*schedulercache.NodeInfo) - for _, nodeName := range test.cached { - nodeInfo[nodeName] = &schedulercache.NodeInfo{} - } +func testCompareNodes(actual, cached, missing, redundant []string, t *testing.T) { + compare := compareStrategy{} + nodes := []*v1.Node{} + for _, nodeName := range actual { + node := &v1.Node{} + node.Name = nodeName + nodes = append(nodes, node) + } - m, r := compare.CompareNodes(nodes, nodeInfo) + nodeInfo := make(map[string]*schedulercache.NodeInfo) + for _, nodeName := range cached { + nodeInfo[nodeName] = &schedulercache.NodeInfo{} + } - if !reflect.DeepEqual(m, test.missing) { - t.Errorf("missing expected to be %s; got %s", test.missing, m) - } + m, r := compare.CompareNodes(nodes, nodeInfo) - if !reflect.DeepEqual(r, test.redundant) { - t.Errorf("redundant expected to be %s; got %s", test.redundant, r) - } + if !reflect.DeepEqual(m, missing) { + t.Errorf("missing expected to be %s; got %s", missing, m) + } + + if !reflect.DeepEqual(r, redundant) { + t.Errorf("redundant expected to be %s; got %s", redundant, r) } } func TestComparePods(t *testing.T) { - compare := compareStrategy{} - tests := []struct { + name string actual []string cached []string queued []string @@ -91,6 +99,7 @@ func TestComparePods(t *testing.T) { redundant []string }{ { + name: "redundant cached value", actual: []string{"foo", "bar"}, cached: []string{"bar", "foo", "foobar"}, queued: []string{}, @@ -98,6 +107,7 @@ func TestComparePods(t *testing.T) { redundant: []string{"foobar"}, }, { + name: "redundant and queued values", actual: []string{"foo", "bar"}, cached: []string{"foo", "foobar"}, queued: []string{"bar"}, @@ -105,6 +115,7 @@ func TestComparePods(t *testing.T) { redundant: []string{"foobar"}, }, { + name: "missing cached value", actual: []string{"foo", "bar", "foobar"}, cached: []string{"bar", "foo"}, queued: []string{}, @@ -112,6 +123,7 @@ func TestComparePods(t *testing.T) { redundant: []string{}, }, { + name: "missing and queued values", actual: []string{"foo", "bar", "foobar"}, cached: []string{"foo"}, queued: []string{"bar"}, @@ -119,6 +131,7 @@ func TestComparePods(t *testing.T) { redundant: []string{}, }, { + name: "correct cache set", actual: []string{"foo", "bar", "foobar"}, cached: []string{"bar", "foobar", "foo"}, queued: []string{}, @@ -126,6 +139,7 @@ func TestComparePods(t *testing.T) { redundant: []string{}, }, { + name: "queued cache value", actual: []string{"foo", "bar", "foobar"}, cached: []string{"foobar", "foo"}, queued: []string{"bar"}, @@ -135,64 +149,73 @@ func TestComparePods(t *testing.T) { } for _, test := range tests { - pods := []*v1.Pod{} - for _, uid := range test.actual { - pod := &v1.Pod{} - pod.UID = types.UID(uid) - pods = append(pods, pod) - } + t.Run(test.name, func(t *testing.T) { + testComparePods(test.actual, test.cached, test.queued, test.missing, test.redundant, t) + }) + } +} - queuedPods := []*v1.Pod{} - for _, uid := range test.queued { - pod := &v1.Pod{} - pod.UID = types.UID(uid) - queuedPods = append(queuedPods, pod) - } +func testComparePods(actual, cached, queued, missing, redundant []string, t *testing.T) { + compare := compareStrategy{} + pods := []*v1.Pod{} + for _, uid := range actual { + pod := &v1.Pod{} + pod.UID = types.UID(uid) + pods = append(pods, pod) + } - nodeInfo := make(map[string]*schedulercache.NodeInfo) - for _, uid := range test.cached { - pod := &v1.Pod{} - pod.UID = types.UID(uid) - pod.Namespace = "ns" - pod.Name = uid + queuedPods := []*v1.Pod{} + for _, uid := range queued { + pod := &v1.Pod{} + pod.UID = types.UID(uid) + queuedPods = append(queuedPods, pod) + } - nodeInfo[uid] = schedulercache.NewNodeInfo(pod) - } + nodeInfo := make(map[string]*schedulercache.NodeInfo) + for _, uid := range cached { + pod := &v1.Pod{} + pod.UID = types.UID(uid) + pod.Namespace = "ns" + pod.Name = uid - m, r := compare.ComparePods(pods, queuedPods, nodeInfo) + nodeInfo[uid] = schedulercache.NewNodeInfo(pod) + } - if !reflect.DeepEqual(m, test.missing) { - t.Errorf("missing expected to be %s; got %s", test.missing, m) - } + m, r := compare.ComparePods(pods, queuedPods, nodeInfo) - if !reflect.DeepEqual(r, test.redundant) { - t.Errorf("redundant expected to be %s; got %s", test.redundant, r) - } + if !reflect.DeepEqual(m, missing) { + t.Errorf("missing expected to be %s; got %s", missing, m) + } + + if !reflect.DeepEqual(r, redundant) { + t.Errorf("redundant expected to be %s; got %s", redundant, r) } } func TestComparePdbs(t *testing.T) { - compare := compareStrategy{} - tests := []struct { + name string actual []string cached []string missing []string redundant []string }{ { + name: "redundant cache value", actual: []string{"foo", "bar"}, cached: []string{"bar", "foo", "foobar"}, missing: []string{}, redundant: []string{"foobar"}, }, { + name: "missing cache value", actual: []string{"foo", "bar", "foobar"}, cached: []string{"bar", "foo"}, missing: []string{"foobar"}, redundant: []string{}, }, { + name: "correct cache", actual: []string{"foo", "bar", "foobar"}, cached: []string{"bar", "foobar", "foo"}, missing: []string{}, @@ -201,28 +224,35 @@ func TestComparePdbs(t *testing.T) { } for _, test := range tests { - pdbs := []*policy.PodDisruptionBudget{} - for _, uid := range test.actual { - pdb := &policy.PodDisruptionBudget{} - pdb.UID = types.UID(uid) - pdbs = append(pdbs, pdb) - } - - cache := make(map[string]*policy.PodDisruptionBudget) - for _, uid := range test.cached { - pdb := &policy.PodDisruptionBudget{} - pdb.UID = types.UID(uid) - cache[uid] = pdb - } - - m, r := compare.ComparePdbs(pdbs, cache) - - if !reflect.DeepEqual(m, test.missing) { - t.Errorf("missing expected to be %s; got %s", test.missing, m) - } - - if !reflect.DeepEqual(r, test.redundant) { - t.Errorf("redundant expected to be %s; got %s", test.redundant, r) - } + t.Run(test.name, func(t *testing.T) { + testComparePdbs(test.actual, test.cached, test.missing, test.redundant, t) + }) + } +} + +func testComparePdbs(actual, cached, missing, redundant []string, t *testing.T) { + compare := compareStrategy{} + pdbs := []*policy.PodDisruptionBudget{} + for _, uid := range actual { + pdb := &policy.PodDisruptionBudget{} + pdb.UID = types.UID(uid) + pdbs = append(pdbs, pdb) + } + + cache := make(map[string]*policy.PodDisruptionBudget) + for _, uid := range cached { + pdb := &policy.PodDisruptionBudget{} + pdb.UID = types.UID(uid) + cache[uid] = pdb + } + + m, r := compare.ComparePdbs(pdbs, cache) + + if !reflect.DeepEqual(m, missing) { + t.Errorf("missing expected to be %s; got %s", missing, m) + } + + if !reflect.DeepEqual(r, redundant) { + t.Errorf("redundant expected to be %s; got %s", redundant, r) } } diff --git a/pkg/scheduler/factory/factory_test.go b/pkg/scheduler/factory/factory_test.go index 36ecf26ecb9..e8382c99296 100644 --- a/pkg/scheduler/factory/factory_test.go +++ b/pkg/scheduler/factory/factory_test.go @@ -331,53 +331,65 @@ func TestNodeEnumerator(t *testing.T) { t.Fatalf("expected %v, got %v", e, a) } for i := range testList.Items { - gotObj := me.Get(i) - if e, a := testList.Items[i].Name, gotObj.(*v1.Node).Name; e != a { - t.Errorf("Expected %v, got %v", e, a) - } - if e, a := &testList.Items[i], gotObj; !reflect.DeepEqual(e, a) { - t.Errorf("Expected %#v, got %v#", e, a) - } + t.Run(fmt.Sprintf("node enumerator/%v", i), func(t *testing.T) { + gotObj := me.Get(i) + if e, a := testList.Items[i].Name, gotObj.(*v1.Node).Name; e != a { + t.Errorf("Expected %v, got %v", e, a) + } + if e, a := &testList.Items[i], gotObj; !reflect.DeepEqual(e, a) { + t.Errorf("Expected %#v, got %v#", e, a) + } + }) } } func TestBind(t *testing.T) { table := []struct { + name string binding *v1.Binding }{ - {binding: &v1.Binding{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: metav1.NamespaceDefault, - Name: "foo", + { + name: "binding can bind and validate request", + binding: &v1.Binding{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: metav1.NamespaceDefault, + Name: "foo", + }, + Target: v1.ObjectReference{ + Name: "foohost.kubernetes.mydomain.com", + }, }, - Target: v1.ObjectReference{ - Name: "foohost.kubernetes.mydomain.com", - }, - }}, + }, } - for _, item := range table { - handler := utiltesting.FakeHandler{ - StatusCode: 200, - ResponseBody: "", - T: t, - } - server := httptest.NewServer(&handler) - defer server.Close() - client := clientset.NewForConfigOrDie(&restclient.Config{Host: server.URL, ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Group: "", Version: "v1"}}}) - b := binder{client} - - if err := b.Bind(item.binding); err != nil { - t.Errorf("Unexpected error: %v", err) - continue - } - expectedBody := runtime.EncodeOrDie(schedulertesting.Test.Codec(), item.binding) - handler.ValidateRequest(t, - schedulertesting.Test.SubResourcePath(string(v1.ResourcePods), metav1.NamespaceDefault, "foo", "binding"), - "POST", &expectedBody) + for _, test := range table { + t.Run(test.name, func(t *testing.T) { + testBind(test.binding, t) + }) } } +func testBind(binding *v1.Binding, t *testing.T) { + handler := utiltesting.FakeHandler{ + StatusCode: 200, + ResponseBody: "", + T: t, + } + server := httptest.NewServer(&handler) + defer server.Close() + client := clientset.NewForConfigOrDie(&restclient.Config{Host: server.URL, ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Group: "", Version: "v1"}}}) + b := binder{client} + + if err := b.Bind(binding); err != nil { + t.Errorf("Unexpected error: %v", err) + return + } + expectedBody := runtime.EncodeOrDie(schedulertesting.Test.Codec(), binding) + handler.ValidateRequest(t, + schedulertesting.Test.SubResourcePath(string(v1.ResourcePods), metav1.NamespaceDefault, "foo", "binding"), + "POST", &expectedBody) +} + func TestInvalidHardPodAffinitySymmetricWeight(t *testing.T) { handler := utiltesting.FakeHandler{ StatusCode: 500, @@ -406,38 +418,44 @@ func TestInvalidFactoryArgs(t *testing.T) { client := clientset.NewForConfigOrDie(&restclient.Config{Host: server.URL, ContentConfig: restclient.ContentConfig{GroupVersion: &schema.GroupVersion{Group: "", Version: "v1"}}}) testCases := []struct { + name string hardPodAffinitySymmetricWeight int32 expectErr string }{ { + name: "symmetric weight below range", hardPodAffinitySymmetricWeight: -1, expectErr: "invalid hardPodAffinitySymmetricWeight: -1, must be in the range 0-100", }, { + name: "symmetric weight above range", hardPodAffinitySymmetricWeight: 101, expectErr: "invalid hardPodAffinitySymmetricWeight: 101, must be in the range 0-100", }, } for _, test := range testCases { - factory := newConfigFactory(client, test.hardPodAffinitySymmetricWeight) - _, err := factory.Create() - if err == nil { - t.Errorf("expected err: %s, got nothing", test.expectErr) - } + t.Run(test.name, func(t *testing.T) { + factory := newConfigFactory(client, test.hardPodAffinitySymmetricWeight) + _, err := factory.Create() + if err == nil { + t.Errorf("expected err: %s, got nothing", test.expectErr) + } + }) } } func TestSkipPodUpdate(t *testing.T) { - for _, test := range []struct { + table := []struct { pod *v1.Pod isAssumedPodFunc func(*v1.Pod) bool getPodFunc func(*v1.Pod) *v1.Pod expected bool + name string }{ - // Non-assumed pod should not be skipped. { + name: "Non-assumed pod", pod: &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: "pod-0", @@ -453,9 +471,8 @@ func TestSkipPodUpdate(t *testing.T) { }, expected: false, }, - // Pod update (with changes on ResourceVersion, Spec.NodeName and/or - // Annotations) for an already assumed pod should be skipped. { + name: "with changes on ResourceVersion, Spec.NodeName and/or Annotations", pod: &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: "pod-0", @@ -483,9 +500,8 @@ func TestSkipPodUpdate(t *testing.T) { }, expected: true, }, - // Pod update (with changes on Labels) for an already assumed pod - // should not be skipped. { + name: "with changes on Labels", pod: &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: "pod-0", @@ -505,17 +521,20 @@ func TestSkipPodUpdate(t *testing.T) { }, expected: false, }, - } { - c := &configFactory{ - schedulerCache: &schedulertesting.FakeCache{ - IsAssumedPodFunc: test.isAssumedPodFunc, - GetPodFunc: test.getPodFunc, - }, - } - got := c.skipPodUpdate(test.pod) - if got != test.expected { - t.Errorf("skipPodUpdate() = %t, expected = %t", got, test.expected) - } + } + for _, test := range table { + t.Run(test.name, func(t *testing.T) { + c := &configFactory{ + schedulerCache: &schedulertesting.FakeCache{ + IsAssumedPodFunc: test.isAssumedPodFunc, + GetPodFunc: test.getPodFunc, + }, + } + got := c.skipPodUpdate(test.pod) + if got != test.expected { + t.Errorf("skipPodUpdate() = %t, expected = %t", got, test.expected) + } + }) } } @@ -593,24 +612,22 @@ func (f *fakeExtender) IsInterested(pod *v1.Pod) bool { } func TestGetBinderFunc(t *testing.T) { - for _, test := range []struct { - podName string - extenders []algorithm.SchedulerExtender - + table := []struct { + podName string + extenders []algorithm.SchedulerExtender expectedBinderType string + name string }{ - // Expect to return the default binder because the extender is not a - // binder, even though it's interested in the pod. { + name: "the extender is not a binder", podName: "pod0", extenders: []algorithm.SchedulerExtender{ &fakeExtender{isBinder: false, interestedPodName: "pod0"}, }, expectedBinderType: "*factory.binder", }, - // Expect to return the fake binder because one of the extenders is a - // binder and it's interested in the pod. { + name: "one of the extenders is a binder and interested in pod", podName: "pod0", extenders: []algorithm.SchedulerExtender{ &fakeExtender{isBinder: false, interestedPodName: "pod0"}, @@ -618,9 +635,8 @@ func TestGetBinderFunc(t *testing.T) { }, expectedBinderType: "*factory.fakeExtender", }, - // Expect to return the default binder because one of the extenders is - // a binder but the binder is not interested in the pod. { + name: "one of the extenders is a binder, but not interested in pod", podName: "pod1", extenders: []algorithm.SchedulerExtender{ &fakeExtender{isBinder: false, interestedPodName: "pod1"}, @@ -628,20 +644,28 @@ func TestGetBinderFunc(t *testing.T) { }, expectedBinderType: "*factory.binder", }, - } { - pod := &v1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Name: test.podName, - }, - } + } - f := &configFactory{} - binderFunc := f.getBinderFunc(test.extenders) - binder := binderFunc(pod) - - binderType := fmt.Sprintf("%s", reflect.TypeOf(binder)) - if binderType != test.expectedBinderType { - t.Errorf("Expected binder %q but got %q", test.expectedBinderType, binderType) - } + for _, test := range table { + t.Run(test.name, func(t *testing.T) { + testGetBinderFunc(test.expectedBinderType, test.podName, test.extenders, t) + }) + } +} + +func testGetBinderFunc(expectedBinderType, podName string, extenders []algorithm.SchedulerExtender, t *testing.T) { + pod := &v1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: podName, + }, + } + + f := &configFactory{} + binderFunc := f.getBinderFunc(extenders) + binder := binderFunc(pod) + + binderType := fmt.Sprintf("%s", reflect.TypeOf(binder)) + if binderType != expectedBinderType { + t.Errorf("Expected binder %q but got %q", expectedBinderType, binderType) } } diff --git a/pkg/scheduler/factory/plugins_test.go b/pkg/scheduler/factory/plugins_test.go index ee813de97ac..9b82056ef82 100644 --- a/pkg/scheduler/factory/plugins_test.go +++ b/pkg/scheduler/factory/plugins_test.go @@ -36,14 +36,18 @@ func TestAlgorithmNameValidation(t *testing.T) { "Some,Alg:orithm", } for _, name := range algorithmNamesShouldValidate { - if !validName.MatchString(name) { - t.Errorf("%v should be a valid algorithm name but is not valid.", name) - } + t.Run(name, func(t *testing.T) { + if !validName.MatchString(name) { + t.Errorf("should be a valid algorithm name but is not valid.") + } + }) } for _, name := range algorithmNamesShouldNotValidate { - if validName.MatchString(name) { - t.Errorf("%v should be an invalid algorithm name but is valid.", name) - } + t.Run(name, func(t *testing.T) { + if validName.MatchString(name) { + t.Errorf("should be an invalid algorithm name but is valid.") + } + }) } } @@ -70,16 +74,18 @@ func TestValidatePriorityConfigOverFlow(t *testing.T) { }, } for _, test := range tests { - err := validateSelectedConfigs(test.configs) - if test.expected { - if err == nil { - t.Errorf("Expected Overflow for %s", test.description) + t.Run(test.description, func(t *testing.T) { + err := validateSelectedConfigs(test.configs) + if test.expected { + if err == nil { + t.Errorf("Expected Overflow") + } + } else { + if err != nil { + t.Errorf("Did not expect an overflow") + } } - } else { - if err != nil { - t.Errorf("Did not expect an overflow for %s", test.description) - } - } + }) } }