diff --git a/pkg/scheduler/framework/interface_test.go b/pkg/scheduler/framework/interface_test.go index a991ce17e79..50e8b3bbe56 100644 --- a/pkg/scheduler/framework/interface_test.go +++ b/pkg/scheduler/framework/interface_test.go @@ -23,6 +23,7 @@ import ( func TestStatus(t *testing.T) { tests := []struct { + name string status *Status expectedCode Code expectedMessage string @@ -30,6 +31,7 @@ func TestStatus(t *testing.T) { expectedAsError error }{ { + name: "success status", status: NewStatus(Success, ""), expectedCode: Success, expectedMessage: "", @@ -37,6 +39,7 @@ func TestStatus(t *testing.T) { expectedAsError: nil, }, { + name: "error status", status: NewStatus(Error, "unknown error"), expectedCode: Error, expectedMessage: "unknown error", @@ -44,6 +47,7 @@ func TestStatus(t *testing.T) { expectedAsError: errors.New("unknown error"), }, { + name: "nil status", status: nil, expectedCode: Success, expectedMessage: "", @@ -52,26 +56,28 @@ func TestStatus(t *testing.T) { }, } - for i, test := range tests { - if test.status.Code() != test.expectedCode { - t.Errorf("test #%v, expect status.Code() returns %v, but %v", i, test.expectedCode, test.status.Code()) - } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + if test.status.Code() != test.expectedCode { + t.Errorf("expect status.Code() returns %v, but %v", test.expectedCode, test.status.Code()) + } - if test.status.Message() != test.expectedMessage { - t.Errorf("test #%v, expect status.Message() returns %v, but %v", i, test.expectedMessage, test.status.Message()) - } + if test.status.Message() != test.expectedMessage { + t.Errorf("expect status.Message() returns %v, but %v", test.expectedMessage, test.status.Message()) + } - if test.status.IsSuccess() != test.expectedIsSuccess { - t.Errorf("test #%v, expect status.IsSuccess() returns %v, but %v", i, test.expectedIsSuccess, test.status.IsSuccess()) - } + if test.status.IsSuccess() != test.expectedIsSuccess { + t.Errorf("expect status.IsSuccess() returns %v, but %v", test.expectedIsSuccess, test.status.IsSuccess()) + } - if test.status.AsError() == test.expectedAsError { - continue - } + if test.status.AsError() == test.expectedAsError { + return + } - if test.status.AsError().Error() != test.expectedAsError.Error() { - t.Errorf("test #%v, expect status.AsError() returns %v, but %v", i, test.expectedAsError, test.status.AsError()) - } + if test.status.AsError().Error() != test.expectedAsError.Error() { + t.Errorf("expect status.AsError() returns %v, but %v", test.expectedAsError, test.status.AsError()) + } + }) } } @@ -93,30 +99,37 @@ func assertStatusCode(t *testing.T, code Code, value int) { func TestPluginToStatusMerge(t *testing.T) { tests := []struct { + name string statusMap PluginToStatus wantCode Code }{ { + name: "merge Error and Unschedulable statuses", statusMap: PluginToStatus{"p1": NewStatus(Error), "p2": NewStatus(Unschedulable)}, wantCode: Error, }, { + name: "merge Success and Unschedulable statuses", statusMap: PluginToStatus{"p1": NewStatus(Success), "p2": NewStatus(Unschedulable)}, wantCode: Unschedulable, }, { + name: "merge Success, UnschedulableAndUnresolvable and Unschedulable statuses", statusMap: PluginToStatus{"p1": NewStatus(Success), "p2": NewStatus(UnschedulableAndUnresolvable), "p3": NewStatus(Unschedulable)}, wantCode: UnschedulableAndUnresolvable, }, { + name: "merge nil status", wantCode: Success, }, } - for i, test := range tests { - gotStatus := test.statusMap.Merge() - if test.wantCode != gotStatus.Code() { - t.Errorf("test #%v, wantCode %v, gotCode %v", i, test.wantCode, gotStatus.Code()) - } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + gotStatus := test.statusMap.Merge() + if test.wantCode != gotStatus.Code() { + t.Errorf("wantCode %v, gotCode %v", test.wantCode, gotStatus.Code()) + } + }) } } diff --git a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go index 920595cc538..e5dad04a176 100644 --- a/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go +++ b/pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go @@ -1315,14 +1315,16 @@ func TestPodEligibleToPreemptOthers(t *testing.T) { } for _, test := range tests { - var nodes []*v1.Node - for _, n := range test.nodes { - nodes = append(nodes, st.MakeNode().Name(n).Obj()) - } - snapshot := internalcache.NewSnapshot(test.pods, nodes) - if got := PodEligibleToPreemptOthers(test.pod, snapshot.NodeInfos(), test.nominatedNodeStatus); got != test.expected { - t.Errorf("expected %t, got %t for pod: %s", test.expected, got, test.pod.Name) - } + t.Run(test.name, func(t *testing.T) { + var nodes []*v1.Node + for _, n := range test.nodes { + nodes = append(nodes, st.MakeNode().Name(n).Obj()) + } + snapshot := internalcache.NewSnapshot(test.pods, nodes) + if got := PodEligibleToPreemptOthers(test.pod, snapshot.NodeInfos(), test.nominatedNodeStatus); got != test.expected { + t.Errorf("expected %t, got %t for pod: %s", test.expected, got, test.pod.Name) + } + }) } } diff --git a/pkg/scheduler/framework/plugins/helper/normalize_score_test.go b/pkg/scheduler/framework/plugins/helper/normalize_score_test.go index f1ca1d8df2f..5deefac3513 100644 --- a/pkg/scheduler/framework/plugins/helper/normalize_score_test.go +++ b/pkg/scheduler/framework/plugins/helper/normalize_score_test.go @@ -17,6 +17,7 @@ limitations under the License. package helper import ( + "fmt" "reflect" "testing" @@ -63,19 +64,21 @@ func TestDefaultNormalizeScore(t *testing.T) { } for i, test := range tests { - scores := framework.NodeScoreList{} - for _, score := range test.scores { - scores = append(scores, framework.NodeScore{Score: score}) - } + t.Run(fmt.Sprintf("case_%d", i), func(t *testing.T) { + scores := framework.NodeScoreList{} + for _, score := range test.scores { + scores = append(scores, framework.NodeScore{Score: score}) + } - expectedScores := framework.NodeScoreList{} - for _, score := range test.expectedScores { - expectedScores = append(expectedScores, framework.NodeScore{Score: score}) - } + expectedScores := framework.NodeScoreList{} + for _, score := range test.expectedScores { + expectedScores = append(expectedScores, framework.NodeScore{Score: score}) + } - DefaultNormalizeScore(framework.MaxNodeScore, test.reverse, scores) - if !reflect.DeepEqual(scores, expectedScores) { - t.Errorf("test %d, expected %v, got %v", i, expectedScores, scores) - } + DefaultNormalizeScore(framework.MaxNodeScore, test.reverse, scores) + if !reflect.DeepEqual(scores, expectedScores) { + t.Errorf("expected %v, got %v", expectedScores, scores) + } + }) } } diff --git a/pkg/scheduler/framework/plugins/nodeports/node_ports_test.go b/pkg/scheduler/framework/plugins/nodeports/node_ports_test.go index b76fd5ae3df..5128221ec30 100644 --- a/pkg/scheduler/framework/plugins/nodeports/node_ports_test.go +++ b/pkg/scheduler/framework/plugins/nodeports/node_ports_test.go @@ -281,10 +281,12 @@ func TestGetContainerPorts(t *testing.T) { }, } - for _, test := range tests { - result := getContainerPorts(test.pod1, test.pod2) - if !reflect.DeepEqual(test.expected, result) { - t.Errorf("Got different result than expected.\nDifference detected on:\n%s", diff.ObjectGoPrintSideBySide(test.expected, result)) - } + for i, test := range tests { + t.Run(fmt.Sprintf("case_%d", i), func(t *testing.T) { + result := getContainerPorts(test.pod1, test.pod2) + if !reflect.DeepEqual(test.expected, result) { + t.Errorf("Got different result than expected.\nDifference detected on:\n%s", diff.ObjectGoPrintSideBySide(test.expected, result)) + } + }) } } diff --git a/pkg/scheduler/framework/plugins/noderesources/fit_test.go b/pkg/scheduler/framework/plugins/noderesources/fit_test.go index 00855766bda..1c6d4de3511 100644 --- a/pkg/scheduler/framework/plugins/noderesources/fit_test.go +++ b/pkg/scheduler/framework/plugins/noderesources/fit_test.go @@ -635,8 +635,10 @@ func TestValidateFitArgs(t *testing.T) { } for _, test := range argsTest { - if err := validateFitArgs(test.args); err != nil && !strings.Contains(err.Error(), test.expect) { - t.Errorf("case[%v]: error details do not include %v", test.name, err) - } + t.Run(test.name, func(t *testing.T) { + if err := validateFitArgs(test.args); err != nil && !strings.Contains(err.Error(), test.expect) { + t.Errorf("case[%v]: error details do not include %v", test.name, err) + } + }) } } diff --git a/pkg/scheduler/framework/plugins/noderesources/requested_to_capacity_ratio_test.go b/pkg/scheduler/framework/plugins/noderesources/requested_to_capacity_ratio_test.go index d95ff6fb6e7..17254b3fe84 100644 --- a/pkg/scheduler/framework/plugins/noderesources/requested_to_capacity_ratio_test.go +++ b/pkg/scheduler/framework/plugins/noderesources/requested_to_capacity_ratio_test.go @@ -18,6 +18,7 @@ package noderesources import ( "context" + "fmt" "reflect" "testing" @@ -173,11 +174,13 @@ func TestBrokenLinearFunction(t *testing.T) { }, } - for _, test := range tests { - function := buildBrokenLinearFunction(test.points) - for _, assertion := range test.assertions { - assert.InDelta(t, assertion.expected, function(assertion.p), 0.1, "points=%v, p=%f", test.points, assertion.p) - } + for i, test := range tests { + t.Run(fmt.Sprintf("case_%d", i), func(t *testing.T) { + function := buildBrokenLinearFunction(test.points) + for _, assertion := range test.assertions { + assert.InDelta(t, assertion.expected, function(assertion.p), 0.1, "points=%v, p=%f", test.points, assertion.p) + } + }) } } diff --git a/pkg/scheduler/framework/types_test.go b/pkg/scheduler/framework/types_test.go index 00567dd3f39..5518d8ffae3 100644 --- a/pkg/scheduler/framework/types_test.go +++ b/pkg/scheduler/framework/types_test.go @@ -30,14 +30,17 @@ import ( func TestNewResource(t *testing.T) { tests := []struct { + name string resourceList v1.ResourceList expected *Resource }{ { + name: "empty resource", resourceList: map[v1.ResourceName]resource.Quantity{}, expected: &Resource{}, }, { + name: "complex resource", resourceList: map[v1.ResourceName]resource.Quantity{ v1.ResourceCPU: *resource.NewScaledQuantity(4, -3), v1.ResourceMemory: *resource.NewQuantity(2000, resource.BinarySI), @@ -57,10 +60,12 @@ func TestNewResource(t *testing.T) { } for _, test := range tests { - r := NewResource(test.resourceList) - if !reflect.DeepEqual(test.expected, r) { - t.Errorf("expected: %#v, got: %#v", test.expected, r) - } + t.Run(test.name, func(t *testing.T) { + r := NewResource(test.resourceList) + if !reflect.DeepEqual(test.expected, r) { + t.Errorf("expected: %#v, got: %#v", test.expected, r) + } + }) } } @@ -102,11 +107,13 @@ func TestResourceList(t *testing.T) { }, } - for _, test := range tests { - rl := test.resource.ResourceList() - if !reflect.DeepEqual(test.expected, rl) { - t.Errorf("expected: %#v, got: %#v", test.expected, rl) - } + for i, test := range tests { + t.Run(fmt.Sprintf("case_%d", i), func(t *testing.T) { + rl := test.resource.ResourceList() + if !reflect.DeepEqual(test.expected, rl) { + t.Errorf("expected: %#v, got: %#v", test.expected, rl) + } + }) } } @@ -137,13 +144,15 @@ func TestResourceClone(t *testing.T) { }, } - for _, test := range tests { - r := test.resource.Clone() - // Modify the field to check if the result is a clone of the origin one. - test.resource.MilliCPU += 1000 - if !reflect.DeepEqual(test.expected, r) { - t.Errorf("expected: %#v, got: %#v", test.expected, r) - } + for i, test := range tests { + t.Run(fmt.Sprintf("case_%d", i), func(t *testing.T) { + r := test.resource.Clone() + // Modify the field to check if the result is a clone of the origin one. + test.resource.MilliCPU += 1000 + if !reflect.DeepEqual(test.expected, r) { + t.Errorf("expected: %#v, got: %#v", test.expected, r) + } + }) } } @@ -183,10 +192,12 @@ func TestResourceAddScalar(t *testing.T) { } for _, test := range tests { - test.resource.AddScalar(test.scalarName, test.scalarQuantity) - if !reflect.DeepEqual(test.expected, test.resource) { - t.Errorf("expected: %#v, got: %#v", test.expected, test.resource) - } + t.Run(string(test.scalarName), func(t *testing.T) { + test.resource.AddScalar(test.scalarName, test.scalarQuantity) + if !reflect.DeepEqual(test.expected, test.resource) { + t.Errorf("expected: %#v, got: %#v", test.expected, test.resource) + } + }) } } @@ -232,11 +243,13 @@ func TestSetMaxResource(t *testing.T) { }, } - for _, test := range tests { - test.resource.SetMaxResource(test.resourceList) - if !reflect.DeepEqual(test.expected, test.resource) { - t.Errorf("expected: %#v, got: %#v", test.expected, test.resource) - } + for i, test := range tests { + t.Run(fmt.Sprintf("case_%d", i), func(t *testing.T) { + test.resource.SetMaxResource(test.resourceList) + if !reflect.DeepEqual(test.expected, test.resource) { + t.Errorf("expected: %#v, got: %#v", test.expected, test.resource) + } + }) } } @@ -540,14 +553,16 @@ func TestNodeInfoClone(t *testing.T) { }, } - for _, test := range tests { - ni := test.nodeInfo.Clone() - // Modify the field to check if the result is a clone of the origin one. - test.nodeInfo.Generation += 10 - test.nodeInfo.UsedPorts.Remove("127.0.0.1", "TCP", 80) - if !reflect.DeepEqual(test.expected, ni) { - t.Errorf("expected: %#v, got: %#v", test.expected, ni) - } + for i, test := range tests { + t.Run(fmt.Sprintf("case_%d", i), func(t *testing.T) { + ni := test.nodeInfo.Clone() + // Modify the field to check if the result is a clone of the origin one. + test.nodeInfo.Generation += 10 + test.nodeInfo.UsedPorts.Remove("127.0.0.1", "TCP", 80) + if !reflect.DeepEqual(test.expected, ni) { + t.Errorf("expected: %#v, got: %#v", test.expected, ni) + } + }) } } @@ -1036,30 +1051,32 @@ func TestNodeInfoRemovePod(t *testing.T) { }, } - for _, test := range tests { - ni := fakeNodeInfo(pods...) + for i, test := range tests { + t.Run(fmt.Sprintf("case_%d", i), func(t *testing.T) { + ni := fakeNodeInfo(pods...) - gen := ni.Generation - err := ni.RemovePod(test.pod) - if err != nil { - if test.errExpected { - expectedErrorMsg := fmt.Errorf("no corresponding pod %s in pods of node %s", test.pod.Name, ni.Node().Name) - if expectedErrorMsg == err { - t.Errorf("expected error: %v, got: %v", expectedErrorMsg, err) + gen := ni.Generation + err := ni.RemovePod(test.pod) + if err != nil { + if test.errExpected { + expectedErrorMsg := fmt.Errorf("no corresponding pod %s in pods of node %s", test.pod.Name, ni.Node().Name) + if expectedErrorMsg == err { + t.Errorf("expected error: %v, got: %v", expectedErrorMsg, err) + } + } else { + t.Errorf("expected no error, got: %v", err) } } else { - t.Errorf("expected no error, got: %v", err) + if ni.Generation <= gen { + t.Errorf("Generation is not incremented. Prev: %v, current: %v", gen, ni.Generation) + } } - } else { - if ni.Generation <= gen { - t.Errorf("Generation is not incremented. Prev: %v, current: %v", gen, ni.Generation) - } - } - test.expectedNodeInfo.Generation = ni.Generation - if !reflect.DeepEqual(test.expectedNodeInfo, ni) { - t.Errorf("expected: %#v, got: %#v", test.expectedNodeInfo, ni) - } + test.expectedNodeInfo.Generation = ni.Generation + if !reflect.DeepEqual(test.expectedNodeInfo, ni) { + t.Errorf("expected: %#v, got: %#v", test.expectedNodeInfo, ni) + } + }) } } @@ -1169,17 +1186,19 @@ func TestHostPortInfo_AddRemove(t *testing.T) { } for _, test := range tests { - hp := make(HostPortInfo) - for _, param := range test.added { - hp.Add(param.ip, param.protocol, param.port) - } - for _, param := range test.removed { - hp.Remove(param.ip, param.protocol, param.port) - } - if hp.Len() != test.length { - t.Errorf("%v failed: expect length %d; got %d", test.desc, test.length, hp.Len()) - t.Error(hp) - } + t.Run(test.desc, func(t *testing.T) { + hp := make(HostPortInfo) + for _, param := range test.added { + hp.Add(param.ip, param.protocol, param.port) + } + for _, param := range test.removed { + hp.Remove(param.ip, param.protocol, param.port) + } + if hp.Len() != test.length { + t.Errorf("%v failed: expect length %d; got %d", test.desc, test.length, hp.Len()) + t.Error(hp) + } + }) } } @@ -1273,12 +1292,14 @@ func TestHostPortInfo_Check(t *testing.T) { } for _, test := range tests { - hp := make(HostPortInfo) - for _, param := range test.added { - hp.Add(param.ip, param.protocol, param.port) - } - if hp.CheckConflict(test.check.ip, test.check.protocol, test.check.port) != test.expect { - t.Errorf("%v failed, expected %t; got %t", test.desc, test.expect, !test.expect) - } + t.Run(test.desc, func(t *testing.T) { + hp := make(HostPortInfo) + for _, param := range test.added { + hp.Add(param.ip, param.protocol, param.port) + } + if hp.CheckConflict(test.check.ip, test.check.protocol, test.check.port) != test.expect { + t.Errorf("expected %t; got %t", test.expect, !test.expect) + } + }) } }