Organize framework unit tests in subtests

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 {
+	for _, test := range tests {
-		if test.status.Code() != test.expectedCode {
+		t.Run(test.name, func(t *testing.T) {
-			t.Errorf("test #%v, expect status.Code() returns %v, but %v", i, test.expectedCode, test.status.Code())
+			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 {
+			if test.status.Message() != test.expectedMessage {
-			t.Errorf("test #%v, expect status.Message() returns %v, but %v", i, test.expectedMessage, test.status.Message())
+				t.Errorf("expect status.Message() returns %v, but %v", test.expectedMessage, test.status.Message())
-		}
+			}
 
-		if test.status.IsSuccess() != test.expectedIsSuccess {
+			if test.status.IsSuccess() != test.expectedIsSuccess {
-			t.Errorf("test #%v, expect status.IsSuccess() returns %v, but %v", i, test.expectedIsSuccess, test.status.IsSuccess())
+				t.Errorf("expect status.IsSuccess() returns %v, but %v", test.expectedIsSuccess, test.status.IsSuccess())
-		}
+			}
 
-		if test.status.AsError() == test.expectedAsError {
+			if test.status.AsError() == test.expectedAsError {
-			continue
+				return
-		}
+			}
 
-		if test.status.AsError().Error() != test.expectedAsError.Error() {
+			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())
+				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 {
+	for _, test := range tests {
-		gotStatus := test.statusMap.Merge()
+		t.Run(test.name, func(t *testing.T) {
-		if test.wantCode != gotStatus.Code() {
+			gotStatus := test.statusMap.Merge()
-			t.Errorf("test #%v, wantCode %v, gotCode %v", i, test.wantCode, gotStatus.Code())
+			if test.wantCode != gotStatus.Code() {
-		}
+				t.Errorf("wantCode %v, gotCode %v", test.wantCode, gotStatus.Code())
+			}
+		})
 	}
 }

pkg/scheduler/framework/plugins/defaultpreemption/default_preemption_test.go

@@ -1295,14 +1295,16 @@ func TestPodEligibleToPreemptOthers(t *testing.T) {
 	}
 
 	for _, test := range tests {
-		var nodes []*v1.Node
+		t.Run(test.name, func(t *testing.T) {
-		for _, n := range test.nodes {
+			var nodes []*v1.Node
-			nodes = append(nodes, st.MakeNode().Name(n).Obj())
+			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 {
+			snapshot := internalcache.NewSnapshot(test.pods, nodes)
-			t.Errorf("expected %t, got %t for pod: %s", test.expected, got, test.pod.Name)
+			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)
+			}
+		})
 	}
 }
 

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{}
+		t.Run(fmt.Sprintf("case_%d", i), func(t *testing.T) {
-		for _, score := range test.scores {
+			scores := framework.NodeScoreList{}
-			scores = append(scores, framework.NodeScore{Score: score})
+			for _, score := range test.scores {
-		}
+				scores = append(scores, framework.NodeScore{Score: score})
+			}
 
-		expectedScores := framework.NodeScoreList{}
+			expectedScores := framework.NodeScoreList{}
-		for _, score := range test.expectedScores {
+			for _, score := range test.expectedScores {
-			expectedScores = append(expectedScores, framework.NodeScore{Score: score})
+				expectedScores = append(expectedScores, framework.NodeScore{Score: score})
-		}
+			}
 
-		DefaultNormalizeScore(framework.MaxNodeScore, test.reverse, scores)
+			DefaultNormalizeScore(framework.MaxNodeScore, test.reverse, scores)
-		if !reflect.DeepEqual(scores, expectedScores) {
+			if !reflect.DeepEqual(scores, expectedScores) {
-			t.Errorf("test %d, expected %v, got %v", i, expectedScores, scores)
+				t.Errorf("expected %v, got %v", expectedScores, scores)
-		}
+			}
+		})
 	}
 }

pkg/scheduler/framework/plugins/nodeports/node_ports_test.go

@@ -281,10 +281,12 @@ func TestGetContainerPorts(t *testing.T) {
 		},
 	}
 
-	for _, test := range tests {
+	for i, test := range tests {
-		result := getContainerPorts(test.pod1, test.pod2)
+		t.Run(fmt.Sprintf("case_%d", i), func(t *testing.T) {
-		if !reflect.DeepEqual(test.expected, result) {
+			result := getContainerPorts(test.pod1, test.pod2)
-			t.Errorf("Got different result than expected.\nDifference detected on:\n%s", diff.ObjectGoPrintSideBySide(test.expected, result))
+			if !reflect.DeepEqual(test.expected, result) {
-		}
+				t.Errorf("Got different result than expected.\nDifference detected on:\n%s", diff.ObjectGoPrintSideBySide(test.expected, result))
+			}
+		})
 	}
 }

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.Run(test.name, func(t *testing.T) {
-			t.Errorf("case[%v]: error details do not include %v", test.name, err)
+			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)
+			}
+		})
 	}
 }

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 {
+	for i, test := range tests {
-		function := buildBrokenLinearFunction(test.points)
+		t.Run(fmt.Sprintf("case_%d", i), func(t *testing.T) {
-		for _, assertion := range test.assertions {
+			function := buildBrokenLinearFunction(test.points)
-			assert.InDelta(t, assertion.expected, function(assertion.p), 0.1, "points=%v, p=%f", test.points, assertion.p)
+			for _, assertion := range test.assertions {
-		}
+				assert.InDelta(t, assertion.expected, function(assertion.p), 0.1, "points=%v, p=%f", test.points, assertion.p)
+			}
+		})
 	}
 }
 

pkg/scheduler/framework/runtime/framework_test.go

@@ -1716,33 +1716,35 @@ func TestPermitPlugins(t *testing.T) {
 	}
 
 	for _, tt := range tests {
-		registry := Registry{}
+		t.Run(tt.name, func(t *testing.T) {
-		configPlugins := &config.Plugins{Permit: &config.PluginSet{}}
+			registry := Registry{}
+			configPlugins := &config.Plugins{Permit: &config.PluginSet{}}
 
-		for _, pl := range tt.plugins {
+			for _, pl := range tt.plugins {
-			tmpPl := pl
+				tmpPl := pl
-			if err := registry.Register(pl.name, func(_ runtime.Object, _ framework.Handle) (framework.Plugin, error) {
+				if err := registry.Register(pl.name, func(_ runtime.Object, _ framework.Handle) (framework.Plugin, error) {
-				return tmpPl, nil
+					return tmpPl, nil
-			}); err != nil {
+				}); err != nil {
-				t.Fatalf("Unable to register Permit plugin: %s", pl.name)
+					t.Fatalf("Unable to register Permit plugin: %s", pl.name)
+				}
+
+				configPlugins.Permit.Enabled = append(
+					configPlugins.Permit.Enabled,
+					config.Plugin{Name: pl.name},
+				)
 			}
 
-			configPlugins.Permit.Enabled = append(
+			f, err := newFrameworkWithQueueSortAndBind(registry, configPlugins, emptyArgs)
-				configPlugins.Permit.Enabled,
+			if err != nil {
-				config.Plugin{Name: pl.name},
+				t.Fatalf("fail to create framework: %s", err)
-			)
+			}
-		}
 
-		f, err := newFrameworkWithQueueSortAndBind(registry, configPlugins, emptyArgs)
+			status := f.RunPermitPlugins(context.TODO(), nil, pod, "")
-		if err != nil {
-			t.Fatalf("fail to create framework: %s", err)
-		}
 
-		status := f.RunPermitPlugins(context.TODO(), nil, pod, "")
+			if !reflect.DeepEqual(status, tt.want) {
-
+				t.Errorf("wrong status code. got %v, want %v", status, tt.want)
-		if !reflect.DeepEqual(status, tt.want) {
+			}
-			t.Errorf("wrong status code. got %v, want %v", status, tt.want)
+		})
-		}
 	}
 }
 

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)
+		t.Run(test.name, func(t *testing.T) {
-		if !reflect.DeepEqual(test.expected, r) {
+			r := NewResource(test.resourceList)
-			t.Errorf("expected: %#v, got: %#v", test.expected, r)
+			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 {
+	for i, test := range tests {
-		rl := test.resource.ResourceList()
+		t.Run(fmt.Sprintf("case_%d", i), func(t *testing.T) {
-		if !reflect.DeepEqual(test.expected, rl) {
+			rl := test.resource.ResourceList()
-			t.Errorf("expected: %#v, got: %#v", test.expected, rl)
+			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 {
+	for i, test := range tests {
-		r := test.resource.Clone()
+		t.Run(fmt.Sprintf("case_%d", i), func(t *testing.T) {
-		// Modify the field to check if the result is a clone of the origin one.
+			r := test.resource.Clone()
-		test.resource.MilliCPU += 1000
+			// Modify the field to check if the result is a clone of the origin one.
-		if !reflect.DeepEqual(test.expected, r) {
+			test.resource.MilliCPU += 1000
-			t.Errorf("expected: %#v, got: %#v", test.expected, r)
+			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)
+		t.Run(string(test.scalarName), func(t *testing.T) {
-		if !reflect.DeepEqual(test.expected, test.resource) {
+			test.resource.AddScalar(test.scalarName, test.scalarQuantity)
-			t.Errorf("expected: %#v, got: %#v", test.expected, test.resource)
+			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 {
+	for i, test := range tests {
-		test.resource.SetMaxResource(test.resourceList)
+		t.Run(fmt.Sprintf("case_%d", i), func(t *testing.T) {
-		if !reflect.DeepEqual(test.expected, test.resource) {
+			test.resource.SetMaxResource(test.resourceList)
-			t.Errorf("expected: %#v, got: %#v", test.expected, test.resource)
+			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 {
+	for i, test := range tests {
-		ni := test.nodeInfo.Clone()
+		t.Run(fmt.Sprintf("case_%d", i), func(t *testing.T) {
-		// Modify the field to check if the result is a clone of the origin one.
+			ni := test.nodeInfo.Clone()
-		test.nodeInfo.Generation += 10
+			// Modify the field to check if the result is a clone of the origin one.
-		test.nodeInfo.UsedPorts.Remove("127.0.0.1", "TCP", 80)
+			test.nodeInfo.Generation += 10
-		if !reflect.DeepEqual(test.expected, ni) {
+			test.nodeInfo.UsedPorts.Remove("127.0.0.1", "TCP", 80)
-			t.Errorf("expected: %#v, got: %#v", test.expected, ni)
+			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 {
+	for i, test := range tests {
-		ni := fakeNodeInfo(pods...)
+		t.Run(fmt.Sprintf("case_%d", i), func(t *testing.T) {
+			ni := fakeNodeInfo(pods...)
 
-		gen := ni.Generation
+			gen := ni.Generation
-		err := ni.RemovePod(test.pod)
+			err := ni.RemovePod(test.pod)
-		if err != nil {
+			if err != nil {
-			if test.errExpected {
+				if test.errExpected {
-				expectedErrorMsg := fmt.Errorf("no corresponding pod %s in pods of node %s", test.pod.Name, ni.Node().Name)
+					expectedErrorMsg := fmt.Errorf("no corresponding pod %s in pods of node %s", test.pod.Name, ni.Node().Name)
-				if expectedErrorMsg == err {
+					if expectedErrorMsg == err {
-					t.Errorf("expected error: %v, got: %v", 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
+			test.expectedNodeInfo.Generation = ni.Generation
-		if !reflect.DeepEqual(test.expectedNodeInfo, ni) {
+			if !reflect.DeepEqual(test.expectedNodeInfo, ni) {
-			t.Errorf("expected: %#v, got: %#v", 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)
+		t.Run(test.desc, func(t *testing.T) {
-		for _, param := range test.added {
+			hp := make(HostPortInfo)
-			hp.Add(param.ip, param.protocol, param.port)
+			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)
+			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())
+			if hp.Len() != test.length {
-			t.Error(hp)
+				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)
+		t.Run(test.desc, func(t *testing.T) {
-		for _, param := range test.added {
+			hp := make(HostPortInfo)
-			hp.Add(param.ip, param.protocol, param.port)
+			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)
+			if hp.CheckConflict(test.check.ip, test.check.protocol, test.check.port) != test.expect {
-		}
+				t.Errorf("expected %t; got %t", test.expect, !test.expect)
+			}
+		})
 	}
 }