Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
39 changes: 20 additions & 19 deletions pkg/scheduler/internal/cache/cache_test.go
Expand Up @@ -20,7 +20,6 @@ import (
"context"
"errors"
"fmt"
"reflect"
"strings"
"testing"
"time"
Expand Down Expand Up @@ -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) {
return fmt.Errorf("got node info %s, want %s", actual.info, expected)
if actual != nil {
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
}
Expand Down Expand Up @@ -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) {
t.Errorf("expect \n%+v; got \n%+v", nItem.info, ni)
if diff := cmp.Diff(nItem.info, ni, cmp.AllowUnexported(framework.NodeInfo{})); diff != "" {
t.Errorf("Unexpected node info (-want,+got):\n%s", diff)
}
}
if !reflect.DeepEqual(snapshot.AssumedPods, cache.assumedPods) {
t.Errorf("expect \n%+v; got \n%+v", cache.assumedPods, snapshot.AssumedPods)
if diff := cmp.Diff(cache.assumedPods, snapshot.AssumedPods); diff != "" {
t.Errorf("Unexpected assumedPods (-want,+got):\n%s", diff)
}

}
Expand Down Expand Up @@ -777,8 +778,8 @@ func TestUpdatePodAndGet(t *testing.T) {
if err != nil {
t.Fatalf("GetPod failed: %v", err)
}
if !reflect.DeepEqual(tc.podToUpdate, cachedPod) {
t.Fatalf("pod get=%s, want=%s", cachedPod, tc.podToUpdate)
if diff := cmp.Diff(tc.podToUpdate, cachedPod); diff != "" {
t.Fatalf("Unexpected pod (-want, +got):\n%s", diff)
}
})
}
Expand Down Expand Up @@ -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) {
t.Errorf("Failed to add node into scheduler cache:\n got: %+v \nexpected: %+v", got, expected)
if diff := cmp.Diff(expected, got.info, cmp.AllowUnexported(framework.NodeInfo{})); diff != "" {
t.Errorf("Unexpected node info from cache (-want, +got):\n%s", diff)
}

// Step 2: dump cached nodes successfully.
Expand All @@ -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) {
t.Errorf("Failed to clone node:\n got: %+v, \n expected: %+v", newNode, expected)
if diff := cmp.Diff(expected, newNode, cmp.AllowUnexported(framework.NodeInfo{})); diff != "" {
t.Errorf("Unexpected clone node info (-want, +got):\n%s", diff)
}

// Step 3: update node attribute successfully.
Expand All @@ -1245,8 +1246,8 @@ func TestNodeOperators(t *testing.T) {
}
expected.Generation = got.info.Generation

if !reflect.DeepEqual(got.info, expected) {
t.Errorf("Failed to update node in schedulertypes:\n got: %+v \nexpected: %+v", got, expected)
if diff := cmp.Diff(expected, got.info, cmp.AllowUnexported(framework.NodeInfo{})); diff != "" {
t.Errorf("Unexpected schedulertypes after updating node (-want, +got):\n%s", diff)
}
// Check nodeTree after update
nodesList, err = cache.nodeTree.list()
Expand Down Expand Up @@ -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) {
return fmt.Errorf("unexpected node info for node %q.Expected:\n%v, got:\n%v", name, ni.info, snapshot.nodeInfoMap[name])
if diff := cmp.Diff(want, snapshot.nodeInfoMap[name], cmp.AllowUnexported(framework.NodeInfo{})); diff != "" {
return fmt.Errorf("Unexpected node info for node (-want, +got):\n%s", diff)
}
}

Expand Down Expand Up @@ -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) {
t.Errorf("The nodeInfoList is incorrect. Expected %v , got %v", test.expected, nodeNames)
if diff := cmp.Diff(test.expected, nodeNames); diff != "" {
t.Errorf("Unexpected nodeInfoList (-want, +got):\n%s", diff)
}
})
}
Expand Down
19 changes: 10 additions & 9 deletions pkg/scheduler/internal/cache/debugger/comparer_test.go
Expand Up @@ -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"
Expand Down Expand Up @@ -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) {
t.Errorf("missing expected to be %s; got %s", missing, m)
if diff := cmp.Diff(missing, m); diff != "" {
t.Errorf("Unexpected missing (-want, +got):\n%s", diff)
}

if !reflect.DeepEqual(r, redundant) {
t.Errorf("redundant expected to be %s; got %s", redundant, r)
if diff := cmp.Diff(redundant, r); diff != "" {
t.Errorf("Unexpected redundant (-want, +got):\n%s", diff)
}
}

Expand Down Expand Up @@ -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) {
t.Errorf("missing expected to be %s; got %s", missing, m)
if diff := cmp.Diff(missing, m); diff != "" {
t.Errorf("Unexpected missing (-want, +got):\n%s", diff)
}

if !reflect.DeepEqual(r, redundant) {
t.Errorf("redundant expected to be %s; got %s", redundant, r)
if diff := cmp.Diff(redundant, r); diff != "" {
t.Errorf("Unexpected redundant (-want, +got):\n%s", diff)
}
}
15 changes: 8 additions & 7 deletions pkg/scheduler/internal/cache/node_tree_test.go
Expand Up @@ -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"
Expand Down Expand Up @@ -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) {
t.Errorf("The node tree is not the same as expected. Expected: %v, Got: %v", expectedTree, nt.tree)
if diff := cmp.Diff(expectedTree, nt.tree); diff != "" {
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))
Expand Down Expand Up @@ -395,8 +396,8 @@ func TestNodeTree_List(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if !reflect.DeepEqual(output, test.expectedOutput) {
t.Errorf("unexpected output. Expected: %v, Got: %v", test.expectedOutput, output)
if diff := cmp.Diff(test.expectedOutput, output); diff != "" {
t.Errorf("Unexpected output (-want, +got):\n%s", diff)
}
})
}
Expand Down Expand Up @@ -487,8 +488,8 @@ func TestNodeTreeMultiOperations(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if !reflect.DeepEqual(output, test.expectedOutput) {
t.Errorf("unexpected output. Expected: %v, Got: %v", test.expectedOutput, output)
if diff := cmp.Diff(test.expectedOutput, output); diff != "" {
t.Errorf("Unexpected output (-want, +got):\n%s", diff)
}
})
}
Expand Down
9 changes: 4 additions & 5 deletions pkg/scheduler/internal/cache/snapshot_test.go
Expand Up @@ -18,7 +18,6 @@ package cache

import (
"fmt"
"reflect"
"testing"

"github.com/google/go-cmp/cmp"
Expand Down Expand Up @@ -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) {
t.Errorf("expected: %#v, got: %#v", test.expected, imageStates)
if diff := cmp.Diff(test.expected, imageStates); diff != "" {
t.Errorf("Unexpected imageStates (-want, +got):\n%s", diff)
}
})
}
Expand Down Expand Up @@ -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) {
t.Errorf("expected: %#v, got: %#v", test.expected, imageMap)
if diff := cmp.Diff(test.expected, imageMap); diff != "" {
t.Errorf("Unexpected imageMap (-want, +got):\n%s", diff)
}
})
}
Expand Down