Merge pull request #118926 from mengjiao-liu/improve-scheduler-use-cmp.Diff

scheduler test: Use cmp.Diff instead of reflect.DeepEqual for pkg/scheduler/internal/cache

pkg/scheduler/internal/cache/cache_test.go

@@ -20,7 +20,6 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"reflect"
 	"strings"
 	"testing"
 	"time"
@@ -49,8 +48,10 @@ func deepEqualWithoutGeneration(actual *nodeInfoListItem, expected *framework.No
 	if expected != nil {
 		expected.Generation = 0
 	}
-	if actual != nil && !reflect.DeepEqual(actual.info, expected) {
+	if actual != nil {
-		return fmt.Errorf("got node info %s, want %s", actual.info, expected)
+		if diff := cmp.Diff(expected, actual.info, cmp.AllowUnexported(framework.NodeInfo{})); diff != "" {
+			return fmt.Errorf("Unexpected node info (-want,+got):\n%s", diff)
+		}
 	}
 	return nil
 }
@@ -461,12 +462,12 @@ func TestDump(t *testing.T) {
 	}
 	for name, ni := range snapshot.Nodes {
 		nItem := cache.nodes[name]
-		if !reflect.DeepEqual(ni, nItem.info) {
+		if diff := cmp.Diff(nItem.info, ni, cmp.AllowUnexported(framework.NodeInfo{})); diff != "" {
-			t.Errorf("expect \n%+v; got \n%+v", nItem.info, ni)
+			t.Errorf("Unexpected node info (-want,+got):\n%s", diff)
 		}
 	}
-	if !reflect.DeepEqual(snapshot.AssumedPods, cache.assumedPods) {
+	if diff := cmp.Diff(cache.assumedPods, snapshot.AssumedPods); diff != "" {
-		t.Errorf("expect \n%+v; got \n%+v", cache.assumedPods, snapshot.AssumedPods)
+		t.Errorf("Unexpected assumedPods (-want,+got):\n%s", diff)
 	}
 
 }
@@ -777,8 +778,8 @@ func TestUpdatePodAndGet(t *testing.T) {
 			if err != nil {
 				t.Fatalf("GetPod failed: %v", err)
 			}
-			if !reflect.DeepEqual(tc.podToUpdate, cachedPod) {
+			if diff := cmp.Diff(tc.podToUpdate, cachedPod); diff != "" {
-				t.Fatalf("pod get=%s, want=%s", cachedPod, tc.podToUpdate)
+				t.Fatalf("Unexpected pod (-want, +got):\n%s", diff)
 			}
 		})
 	}
@@ -1213,8 +1214,8 @@ func TestNodeOperators(t *testing.T) {
 
 			// Generations are globally unique. We check in our unit tests that they are incremented correctly.
 			expected.Generation = got.info.Generation
-			if !reflect.DeepEqual(got.info, expected) {
+			if diff := cmp.Diff(expected, got.info, cmp.AllowUnexported(framework.NodeInfo{})); diff != "" {
-				t.Errorf("Failed to add node into scheduler cache:\n got: %+v \nexpected: %+v", got, expected)
+				t.Errorf("Unexpected node info from cache (-want, +got):\n%s", diff)
 			}
 
 			// Step 2: dump cached nodes successfully.
@@ -1227,8 +1228,8 @@ func TestNodeOperators(t *testing.T) {
 				t.Errorf("failed to dump cached nodes:\n got: %v \nexpected: %v", cachedNodes, cache.nodes)
 			}
 			expected.Generation = newNode.Generation
-			if !reflect.DeepEqual(newNode, expected) {
+			if diff := cmp.Diff(expected, newNode, cmp.AllowUnexported(framework.NodeInfo{})); diff != "" {
-				t.Errorf("Failed to clone node:\n got: %+v, \n expected: %+v", newNode, expected)
+				t.Errorf("Unexpected clone node info (-want, +got):\n%s", diff)
 			}
 
 			// Step 3: update node attribute successfully.
@@ -1245,8 +1246,8 @@ func TestNodeOperators(t *testing.T) {
 			}
 			expected.Generation = got.info.Generation
 
-			if !reflect.DeepEqual(got.info, expected) {
+			if diff := cmp.Diff(expected, got.info, cmp.AllowUnexported(framework.NodeInfo{})); diff != "" {
-				t.Errorf("Failed to update node in schedulertypes:\n got: %+v \nexpected: %+v", got, expected)
+				t.Errorf("Unexpected schedulertypes after updating node (-want, +got):\n%s", diff)
 			}
 			// Check nodeTree after update
 			nodesList, err = cache.nodeTree.list()
@@ -1714,8 +1715,8 @@ func compareCacheWithNodeInfoSnapshot(t *testing.T, cache *cacheImpl, snapshot *
 		if want.Node() == nil {
 			want = nil
 		}
-		if !reflect.DeepEqual(snapshot.nodeInfoMap[name], want) {
+		if diff := cmp.Diff(want, snapshot.nodeInfoMap[name], cmp.AllowUnexported(framework.NodeInfo{})); diff != "" {
-			return fmt.Errorf("unexpected node info for node %q.Expected:\n%v, got:\n%v", name, ni.info, snapshot.nodeInfoMap[name])
+			return fmt.Errorf("Unexpected node info for node (-want, +got):\n%s", diff)
 		}
 	}
 
@@ -1909,8 +1910,8 @@ func TestSchedulerCache_updateNodeInfoSnapshotList(t *testing.T) {
 			for i, nodeInfo := range snapshot.nodeInfoList {
 				nodeNames[i] = nodeInfo.Node().Name
 			}
-			if !reflect.DeepEqual(nodeNames, test.expected) {
+			if diff := cmp.Diff(test.expected, nodeNames); diff != "" {
-				t.Errorf("The nodeInfoList is incorrect. Expected %v , got %v", test.expected, nodeNames)
+				t.Errorf("Unexpected nodeInfoList (-want, +got):\n%s", diff)
 			}
 		})
 	}

pkg/scheduler/internal/cache/debugger/comparer_test.go

@@ -17,9 +17,10 @@ limitations under the License.
 package debugger
 
 import (
-	"reflect"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
+
 	"k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/kubernetes/pkg/scheduler/framework"
@@ -79,12 +80,12 @@ func testCompareNodes(actual, cached, missing, redundant []string, t *testing.T)
 
 	m, r := compare.CompareNodes(nodes, nodeInfo)
 
-	if !reflect.DeepEqual(m, missing) {
+	if diff := cmp.Diff(missing, m); diff != "" {
-		t.Errorf("missing expected to be %s; got %s", missing, m)
+		t.Errorf("Unexpected missing (-want, +got):\n%s", diff)
 	}
 
-	if !reflect.DeepEqual(r, redundant) {
+	if diff := cmp.Diff(redundant, r); diff != "" {
-		t.Errorf("redundant expected to be %s; got %s", redundant, r)
+		t.Errorf("Unexpected redundant (-want, +got):\n%s", diff)
 	}
 }
 
@@ -182,11 +183,11 @@ func testComparePods(actual, cached, queued, missing, redundant []string, t *tes
 
 	m, r := compare.ComparePods(pods, queuedPods, nodeInfo)
 
-	if !reflect.DeepEqual(m, missing) {
+	if diff := cmp.Diff(missing, m); diff != "" {
-		t.Errorf("missing expected to be %s; got %s", missing, m)
+		t.Errorf("Unexpected missing (-want, +got):\n%s", diff)
 	}
 
-	if !reflect.DeepEqual(r, redundant) {
+	if diff := cmp.Diff(redundant, r); diff != "" {
-		t.Errorf("redundant expected to be %s; got %s", redundant, r)
+		t.Errorf("Unexpected redundant (-want, +got):\n%s", diff)
 	}
 }

pkg/scheduler/internal/cache/node_tree_test.go

@@ -17,9 +17,10 @@ limitations under the License.
 package cache
 
 import (
-	"reflect"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
+
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/klog/v2/ktesting"
@@ -142,8 +143,8 @@ func verifyNodeTree(t *testing.T, nt *nodeTree, expectedTree map[string][]string
 	if numNodes := nt.numNodes; numNodes != expectedNumNodes {
 		t.Errorf("unexpected nodeTree.numNodes. Expected: %v, Got: %v", expectedNumNodes, numNodes)
 	}
-	if !reflect.DeepEqual(nt.tree, expectedTree) {
+	if diff := cmp.Diff(expectedTree, nt.tree); diff != "" {
-		t.Errorf("The node tree is not the same as expected. Expected: %v, Got: %v", expectedTree, nt.tree)
+		t.Errorf("Unexpected node tree (-want, +got):\n%s", diff)
 	}
 	if len(nt.zones) != len(expectedTree) {
 		t.Errorf("Number of zones in nodeTree.zones is not expected. Expected: %v, Got: %v", len(expectedTree), len(nt.zones))
@@ -395,8 +396,8 @@ func TestNodeTree_List(t *testing.T) {
 			if err != nil {
 				t.Fatal(err)
 			}
-			if !reflect.DeepEqual(output, test.expectedOutput) {
+			if diff := cmp.Diff(test.expectedOutput, output); diff != "" {
-				t.Errorf("unexpected output. Expected: %v, Got: %v", test.expectedOutput, output)
+				t.Errorf("Unexpected output (-want, +got):\n%s", diff)
 			}
 		})
 	}
@@ -487,8 +488,8 @@ func TestNodeTreeMultiOperations(t *testing.T) {
 			if err != nil {
 				t.Fatal(err)
 			}
-			if !reflect.DeepEqual(output, test.expectedOutput) {
+			if diff := cmp.Diff(test.expectedOutput, output); diff != "" {
-				t.Errorf("unexpected output. Expected: %v, Got: %v", test.expectedOutput, output)
+				t.Errorf("Unexpected output (-want, +got):\n%s", diff)
 			}
 		})
 	}

pkg/scheduler/internal/cache/snapshot_test.go

@@ -18,7 +18,6 @@ package cache
 
 import (
 	"fmt"
-	"reflect"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -89,8 +88,8 @@ func TestGetNodeImageStates(t *testing.T) {
 	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) {
+			if diff := cmp.Diff(test.expected, imageStates); diff != "" {
-				t.Errorf("expected: %#v, got: %#v", test.expected, imageStates)
+				t.Errorf("Unexpected imageStates (-want, +got):\n%s", diff)
 			}
 		})
 	}
@@ -177,8 +176,8 @@ func TestCreateImageExistenceMap(t *testing.T) {
 	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) {
+			if diff := cmp.Diff(test.expected, imageMap); diff != "" {
-				t.Errorf("expected: %#v, got: %#v", test.expected, imageMap)
+				t.Errorf("Unexpected imageMap (-want, +got):\n%s", diff)
 			}
 		})
 	}