diff --git a/pkg/scheduler/backend/cache/cache_test.go b/pkg/scheduler/backend/cache/cache_test.go index 5509213db3b..58198f6bab5 100644 --- a/pkg/scheduler/backend/cache/cache_test.go +++ b/pkg/scheduler/backend/cache/cache_test.go @@ -40,6 +40,12 @@ import ( schedutil "k8s.io/kubernetes/pkg/scheduler/util" ) +var nodeInfoCmpOpts = []cmp.Option{ + cmp.AllowUnexported(framework.NodeInfo{}), + // This field needs to be ignored because we can't call AllowUnexported for type framework.podResource (it's not visible in this package). + cmpopts.IgnoreFields(framework.PodInfo{}, "cachedResource"), +} + func deepEqualWithoutGeneration(actual *nodeInfoListItem, expected *framework.NodeInfo) error { if (actual == nil) != (expected == nil) { return errors.New("one of the actual or expected is nil and the other is not") @@ -52,7 +58,7 @@ func deepEqualWithoutGeneration(actual *nodeInfoListItem, expected *framework.No expected.Generation = 0 } if actual != nil { - if diff := cmp.Diff(expected, actual.info, cmp.AllowUnexported(framework.NodeInfo{}), cmpopts.IgnoreUnexported(framework.PodInfo{})); diff != "" { + if diff := cmp.Diff(expected, actual.info, nodeInfoCmpOpts...); diff != "" { return fmt.Errorf("Unexpected node info (-want,+got):\n%s", diff) } } @@ -466,7 +472,7 @@ func TestDump(t *testing.T) { } for name, ni := range snapshot.Nodes { nItem := cache.nodes[name] - if diff := cmp.Diff(nItem.info, ni, cmp.AllowUnexported(framework.NodeInfo{}), cmpopts.IgnoreUnexported(framework.PodInfo{})); diff != "" { + if diff := cmp.Diff(nItem.info, ni, nodeInfoCmpOpts...); diff != "" { t.Errorf("Unexpected node info (-want,+got):\n%s", diff) } } @@ -1246,7 +1252,7 @@ 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 diff := cmp.Diff(expected, got.info, cmp.AllowUnexported(framework.NodeInfo{}), cmpopts.IgnoreUnexported(framework.PodInfo{})); diff != "" { + if diff := cmp.Diff(expected, got.info, nodeInfoCmpOpts...); diff != "" { t.Errorf("Failed to add node into scheduler cache (-want,+got):\n%s", diff) } @@ -1265,8 +1271,8 @@ func TestNodeOperators(t *testing.T) { t.Errorf("failed to dump cached nodes:\n got: %v \nexpected: %v", cachedNodes.nodeInfoMap, tc.nodes) } expected.Generation = newNode.Generation - if diff := cmp.Diff(newNode, expected.Snapshot(), cmp.AllowUnexported(framework.NodeInfo{}), cmpopts.IgnoreUnexported(framework.PodInfo{})); diff != "" { - t.Errorf("Failed to clone node:\n%s", diff) + if diff := cmp.Diff(expected.Snapshot(), newNode, nodeInfoCmpOpts...); diff != "" { + t.Errorf("Failed to clone node (-want,+got):\n%s", diff) } // check imageState of NodeInfo with specific image when update snapshot if !checkImageStateSummary(cachedNodes.nodeInfoMap, "gcr.io/80:latest", "gcr.io/300:latest") { @@ -1287,7 +1293,7 @@ func TestNodeOperators(t *testing.T) { } expected.Generation = got.info.Generation - if diff := cmp.Diff(expected, got.info, cmp.AllowUnexported(framework.NodeInfo{}), cmpopts.IgnoreUnexported(framework.PodInfo{})); diff != "" { + if diff := cmp.Diff(expected, got.info, nodeInfoCmpOpts...); diff != "" { t.Errorf("Unexpected schedulertypes after updating node (-want, +got):\n%s", diff) } // check imageState of NodeInfo with specific image when update node @@ -1764,7 +1770,7 @@ func compareCacheWithNodeInfoSnapshot(t *testing.T, cache *cacheImpl, snapshot * if want.Node() == nil { want = nil } - if diff := cmp.Diff(want, snapshot.nodeInfoMap[name], cmp.AllowUnexported(framework.NodeInfo{}), cmpopts.IgnoreUnexported(framework.PodInfo{})); diff != "" { + if diff := cmp.Diff(want, snapshot.nodeInfoMap[name], nodeInfoCmpOpts...); diff != "" { return fmt.Errorf("Unexpected node info for node (-want, +got):\n%s", diff) } } diff --git a/pkg/scheduler/extender_test.go b/pkg/scheduler/extender_test.go index 2d9710a9cdd..4c2418c6659 100644 --- a/pkg/scheduler/extender_test.go +++ b/pkg/scheduler/extender_test.go @@ -18,10 +18,10 @@ package scheduler import ( "context" - "reflect" "testing" "time" + "github.com/google/go-cmp/cmp" v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/sets" @@ -40,6 +40,10 @@ import ( tf "k8s.io/kubernetes/pkg/scheduler/testing/framework" ) +var scheduleResultCmpOpts = []cmp.Option{ + cmp.AllowUnexported(ScheduleResult{}), +} + func TestSchedulerWithExtenders(t *testing.T) { tests := []struct { name string @@ -365,8 +369,8 @@ func TestSchedulerWithExtenders(t *testing.T) { return } - if !reflect.DeepEqual(result, test.expectedResult) { - t.Errorf("Expected: %+v, Saw: %+v", test.expectedResult, result) + if diff := cmp.Diff(test.expectedResult, result, scheduleResultCmpOpts...); diff != "" { + t.Errorf("Unexpected result: (-want, +got):\n%s", diff) } } }) @@ -480,8 +484,9 @@ func TestConvertToMetaVictims(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := convertToMetaVictims(tt.nodeNameToVictims); !reflect.DeepEqual(got, tt.want) { - t.Errorf("convertToMetaVictims() = %v, want %v", got, tt.want) + got := convertToMetaVictims(tt.nodeNameToVictims) + if diff := cmp.Diff(tt.want, got); diff != "" { + t.Errorf("Unexpected convertToMetaVictims(): (-want, +got):\n%s", diff) } }) } @@ -562,8 +567,8 @@ func TestConvertToVictims(t *testing.T) { t.Errorf("convertToVictims() error = %v, wantErr %v", err, tt.wantErr) return } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("convertToVictims() got = %v, want %v", got, tt.want) + if diff := cmp.Diff(tt.want, got); diff != "" { + t.Errorf("Unexpected convertToVictims(): (-want, +got):\n%s", diff) } }) } diff --git a/pkg/scheduler/framework/plugins/interpodaffinity/filtering_test.go b/pkg/scheduler/framework/plugins/interpodaffinity/filtering_test.go index 7f3c367416a..a5899a8ec48 100644 --- a/pkg/scheduler/framework/plugins/interpodaffinity/filtering_test.go +++ b/pkg/scheduler/framework/plugins/interpodaffinity/filtering_test.go @@ -18,8 +18,6 @@ package interpodaffinity import ( "context" - "fmt" - "reflect" "testing" "github.com/google/go-cmp/cmp" @@ -39,7 +37,10 @@ import ( ) var ( - defaultNamespace = "" + defaultNamespace = "" + preFilterStateCmpOpts = []cmp.Option{ + cmp.AllowUnexported(preFilterState{}, framework.PodInfo{}), + } ) func createPodWithAffinityTerms(namespace, nodeName string, labels map[string]string, affinity, antiAffinity []v1.PodAffinityTerm) *v1.Pod { @@ -550,7 +551,7 @@ func TestRequiredAffinitySingleNode(t *testing.T) { p := plugintesting.SetupPluginWithInformers(ctx, t, schedruntime.FactoryAdapter(feature.Features{}, New), &config.InterPodAffinityArgs{}, snapshot, namespaces) state := framework.NewCycleState() _, preFilterStatus := p.(framework.PreFilterPlugin).PreFilter(ctx, state, test.pod) - if diff := cmp.Diff(preFilterStatus, test.wantPreFilterStatus); diff != "" { + if diff := cmp.Diff(test.wantPreFilterStatus, preFilterStatus); diff != "" { t.Errorf("PreFilter: status does not match (-want,+got):\n%s", diff) } if !preFilterStatus.IsSuccess() { @@ -559,7 +560,7 @@ func TestRequiredAffinitySingleNode(t *testing.T) { nodeInfo := mustGetNodeInfo(t, snapshot, test.node.Name) gotStatus := p.(framework.FilterPlugin).Filter(ctx, state, test.pod, nodeInfo) - if diff := cmp.Diff(gotStatus, test.wantFilterStatus); diff != "" { + if diff := cmp.Diff(test.wantFilterStatus, gotStatus); diff != "" { t.Errorf("Filter: status does not match (-want,+got):\n%s", diff) } }) @@ -965,7 +966,7 @@ func TestRequiredAffinityMultipleNodes(t *testing.T) { }) state := framework.NewCycleState() _, preFilterStatus := p.(framework.PreFilterPlugin).PreFilter(ctx, state, test.pod) - if diff := cmp.Diff(preFilterStatus, test.wantPreFilterStatus); diff != "" { + if diff := cmp.Diff(test.wantPreFilterStatus, preFilterStatus); diff != "" { t.Errorf("PreFilter: status does not match (-want,+got):\n%s", diff) } if preFilterStatus.IsSkip() { @@ -974,7 +975,7 @@ func TestRequiredAffinityMultipleNodes(t *testing.T) { for indexNode, node := range test.nodes { nodeInfo := mustGetNodeInfo(t, snapshot, node.Name) gotStatus := p.(framework.FilterPlugin).Filter(ctx, state, test.pod, nodeInfo) - if diff := cmp.Diff(gotStatus, test.wantFilterStatuses[indexNode]); diff != "" { + if diff := cmp.Diff(test.wantFilterStatuses[indexNode], gotStatus); diff != "" { t.Errorf("index: %d: Filter: status does not match (-want,+got):\n%s", indexTest, diff) } } @@ -993,9 +994,9 @@ func TestPreFilterDisabled(t *testing.T) { p := plugintesting.SetupPluginWithInformers(ctx, t, schedruntime.FactoryAdapter(feature.Features{}, New), &config.InterPodAffinityArgs{}, cache.NewEmptySnapshot(), nil) cycleState := framework.NewCycleState() gotStatus := p.(framework.FilterPlugin).Filter(ctx, cycleState, pod, nodeInfo) - wantStatus := framework.AsStatus(fmt.Errorf(`error reading "PreFilterInterPodAffinity" from cycleState: %w`, framework.ErrNotFound)) - if !reflect.DeepEqual(gotStatus, wantStatus) { - t.Errorf("status does not match: %v, want: %v", gotStatus, wantStatus) + wantStatus := framework.AsStatus(framework.ErrNotFound) + if diff := cmp.Diff(gotStatus, wantStatus); diff != "" { + t.Errorf("Status does not match (-want,+got):\n%s", diff) } } @@ -1270,24 +1271,24 @@ func TestPreFilterStateAddRemovePod(t *testing.T) { t.Errorf("failed to get preFilterState from cycleState: %v", err) } - if !reflect.DeepEqual(newState.antiAffinityCounts, test.expectedAntiAffinity) { - t.Errorf("State is not equal, got: %v, want: %v", newState.antiAffinityCounts, test.expectedAntiAffinity) + if diff := cmp.Diff(test.expectedAntiAffinity, newState.antiAffinityCounts); diff != "" { + t.Errorf("State is not equal (-want,+got):\n%s", diff) } - if !reflect.DeepEqual(newState.affinityCounts, test.expectedAffinity) { - t.Errorf("State is not equal, got: %v, want: %v", newState.affinityCounts, test.expectedAffinity) + if diff := cmp.Diff(test.expectedAffinity, newState.affinityCounts); diff != "" { + t.Errorf("State is not equal (-want,+got):\n%s", diff) } - if !reflect.DeepEqual(allPodsState, state) { - t.Errorf("State is not equal, got: %v, want: %v", state, allPodsState) + if diff := cmp.Diff(allPodsState, state, preFilterStateCmpOpts...); diff != "" { + t.Errorf("State is not equal (-want,+got):\n%s", diff) } // Remove the added pod pod and make sure it is equal to the original state. if err := ipa.RemovePod(ctx, cycleState, test.pendingPod, mustNewPodInfo(t, test.addedPod), nodeInfo); err != nil { t.Errorf("error removing pod from meta: %v", err) } - if !reflect.DeepEqual(originalState, state) { - t.Errorf("State is not equal, got: %v, want: %v", state, originalState) + if diff := cmp.Diff(originalState, state, preFilterStateCmpOpts...); diff != "" { + t.Errorf("State is not equal (-want,+got):\n%s", diff) } }) } @@ -1313,8 +1314,8 @@ func TestPreFilterStateClone(t *testing.T) { if clone == source { t.Errorf("Clone returned the exact same object!") } - if !reflect.DeepEqual(clone, source) { - t.Errorf("Copy is not equal to source!") + if diff := cmp.Diff(source, clone, preFilterStateCmpOpts...); diff != "" { + t.Errorf("Copy is not equal to source! (-want,+got):\n%s", diff) } } @@ -1440,11 +1441,11 @@ func TestGetTPMapMatchingIncomingAffinityAntiAffinity(t *testing.T) { defer cancel() p := plugintesting.SetupPluginWithInformers(ctx, t, schedruntime.FactoryAdapter(feature.Features{}, New), &config.InterPodAffinityArgs{}, snapshot, nil) gotAffinityPodsMap, gotAntiAffinityPodsMap := p.(*InterPodAffinity).getIncomingAffinityAntiAffinityCounts(ctx, mustNewPodInfo(t, tt.pod), l) - if !reflect.DeepEqual(gotAffinityPodsMap, tt.wantAffinityPodsMap) { - t.Errorf("getTPMapMatchingIncomingAffinityAntiAffinity() gotAffinityPodsMap = %#v, want %#v", gotAffinityPodsMap, tt.wantAffinityPodsMap) + if diff := cmp.Diff(tt.wantAffinityPodsMap, gotAffinityPodsMap); diff != "" { + t.Errorf("Unexpected getTPMapMatchingIncomingAffinityAntiAffinity() (-want,+got):\n%s", diff) } - if !reflect.DeepEqual(gotAntiAffinityPodsMap, tt.wantAntiAffinityPodsMap) { - t.Errorf("getTPMapMatchingIncomingAffinityAntiAffinity() gotAntiAffinityPodsMap = %#v, want %#v", gotAntiAffinityPodsMap, tt.wantAntiAffinityPodsMap) + if diff := cmp.Diff(tt.wantAntiAffinityPodsMap, gotAntiAffinityPodsMap); diff != "" { + t.Errorf("Unexpected getTPMapMatchingIncomingAffinityAntiAffinity() (-want,+got):\n%s", diff) } }) } diff --git a/pkg/scheduler/framework/plugins/interpodaffinity/scoring_test.go b/pkg/scheduler/framework/plugins/interpodaffinity/scoring_test.go index 549c508397f..3e6f4d23634 100644 --- a/pkg/scheduler/framework/plugins/interpodaffinity/scoring_test.go +++ b/pkg/scheduler/framework/plugins/interpodaffinity/scoring_test.go @@ -18,7 +18,6 @@ package interpodaffinity import ( "context" - "reflect" "strings" "testing" @@ -978,8 +977,8 @@ func TestPreferredAffinityWithHardPodAffinitySymmetricWeight(t *testing.T) { t.Errorf("unexpected error: %v", status) } - if !reflect.DeepEqual(test.expectedList, gotList) { - t.Errorf("expected:\n\t%+v,\ngot:\n\t%+v", test.expectedList, gotList) + if diff := cmp.Diff(test.expectedList, gotList); diff != "" { + t.Errorf("unexpected NodeScoreList (-want,+got):\n%s", diff) } }) } diff --git a/pkg/scheduler/framework/plugins/nodename/node_name_test.go b/pkg/scheduler/framework/plugins/nodename/node_name_test.go index a4ffb914e95..647ee9ee865 100644 --- a/pkg/scheduler/framework/plugins/nodename/node_name_test.go +++ b/pkg/scheduler/framework/plugins/nodename/node_name_test.go @@ -17,9 +17,10 @@ limitations under the License. package nodename import ( - "reflect" "testing" + "github.com/google/go-cmp/cmp" + v1 "k8s.io/api/core/v1" "k8s.io/kubernetes/pkg/scheduler/framework" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/feature" @@ -62,8 +63,8 @@ func TestNodeName(t *testing.T) { t.Fatalf("creating plugin: %v", err) } gotStatus := p.(framework.FilterPlugin).Filter(ctx, nil, test.pod, nodeInfo) - if !reflect.DeepEqual(gotStatus, test.wantStatus) { - t.Errorf("status does not match: %v, want: %v", gotStatus, test.wantStatus) + if diff := cmp.Diff(test.wantStatus, gotStatus); diff != "" { + t.Errorf("status does not match (-want,+got):\n%s", diff) } }) } diff --git a/pkg/scheduler/framework/plugins/nodeports/node_ports_test.go b/pkg/scheduler/framework/plugins/nodeports/node_ports_test.go index 5a042f5b136..354ac4925be 100644 --- a/pkg/scheduler/framework/plugins/nodeports/node_ports_test.go +++ b/pkg/scheduler/framework/plugins/nodeports/node_ports_test.go @@ -18,7 +18,6 @@ package nodeports import ( "fmt" - "reflect" "strconv" "strings" "testing" @@ -183,9 +182,9 @@ func TestPreFilterDisabled(t *testing.T) { } cycleState := framework.NewCycleState() gotStatus := p.(framework.FilterPlugin).Filter(ctx, cycleState, pod, nodeInfo) - wantStatus := framework.AsStatus(fmt.Errorf(`reading "PreFilterNodePorts" from cycleState: %w`, framework.ErrNotFound)) - if !reflect.DeepEqual(gotStatus, wantStatus) { - t.Errorf("status does not match: %v, want: %v", gotStatus, wantStatus) + wantStatus := framework.AsStatus(framework.ErrNotFound) + if diff := cmp.Diff(wantStatus, gotStatus); diff != "" { + t.Errorf("status does not match (-want,+got):\n%s", diff) } } diff --git a/pkg/scheduler/framework/plugins/noderesources/balanced_allocation_test.go b/pkg/scheduler/framework/plugins/noderesources/balanced_allocation_test.go index 301c7222ad3..b89f1d98f1a 100644 --- a/pkg/scheduler/framework/plugins/noderesources/balanced_allocation_test.go +++ b/pkg/scheduler/framework/plugins/noderesources/balanced_allocation_test.go @@ -18,9 +18,9 @@ package noderesources import ( "context" - "reflect" "testing" + "github.com/google/go-cmp/cmp" v1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -403,8 +403,8 @@ func TestNodeResourcesBalancedAllocation(t *testing.T) { if !status.IsSuccess() { t.Errorf("Score is expected to return success, but didn't. Got status: %v", status) } - if !reflect.DeepEqual(test.expectedList[i].Score, hostResult) { - t.Errorf("got score %v for host %v, expected %v", hostResult, test.nodes[i].Name, test.expectedList[i].Score) + if diff := cmp.Diff(test.expectedList[i].Score, hostResult); diff != "" { + t.Errorf("unexpected score for host %v (-want,+got):\n%s", test.nodes[i].Name, diff) } } }) diff --git a/pkg/scheduler/framework/plugins/noderesources/fit_test.go b/pkg/scheduler/framework/plugins/noderesources/fit_test.go index d118e6f2503..b0d56a63f85 100644 --- a/pkg/scheduler/framework/plugins/noderesources/fit_test.go +++ b/pkg/scheduler/framework/plugins/noderesources/fit_test.go @@ -19,7 +19,6 @@ package noderesources import ( "context" "fmt" - "reflect" "testing" "github.com/google/go-cmp/cmp" @@ -48,6 +47,10 @@ var ( hugePageResourceA = v1.ResourceName(v1.ResourceHugePagesPrefix + "2Mi") ) +var clusterEventCmpOpts = []cmp.Option{ + cmpopts.EquateComparable(framework.ClusterEvent{}), +} + func makeResources(milliCPU, memory, pods, extendedA, storage, hugePageA int64) v1.ResourceList { return v1.ResourceList{ v1.ResourceCPU: *resource.NewMilliQuantity(milliCPU, resource.DecimalSI), @@ -584,13 +587,13 @@ func TestEnoughRequests(t *testing.T) { } gotStatus := p.(framework.FilterPlugin).Filter(ctx, cycleState, test.pod, test.nodeInfo) - if !reflect.DeepEqual(gotStatus, test.wantStatus) { - t.Errorf("status does not match: %v, want: %v", gotStatus, test.wantStatus) + if diff := cmp.Diff(test.wantStatus, gotStatus); diff != "" { + t.Errorf("status does not match (-want,+got):\n%s", diff) } gotInsufficientResources := fitsRequest(computePodResourceRequest(test.pod, ResourceRequestsOptions{EnablePodLevelResources: test.podLevelResourcesEnabled}), test.nodeInfo, p.(*Fit).ignoredResources, p.(*Fit).ignoredResourceGroups) - if !reflect.DeepEqual(gotInsufficientResources, test.wantInsufficientResources) { - t.Errorf("insufficient resources do not match: %+v, want: %v", gotInsufficientResources, test.wantInsufficientResources) + if diff := cmp.Diff(test.wantInsufficientResources, gotInsufficientResources); diff != "" { + t.Errorf("insufficient resources do not match (-want,+got):\n%s", diff) } }) } @@ -610,9 +613,9 @@ func TestPreFilterDisabled(t *testing.T) { } cycleState := framework.NewCycleState() gotStatus := p.(framework.FilterPlugin).Filter(ctx, cycleState, pod, nodeInfo) - wantStatus := framework.AsStatus(fmt.Errorf(`error reading "PreFilterNodeResourcesFit" from cycleState: %w`, framework.ErrNotFound)) - if !reflect.DeepEqual(gotStatus, wantStatus) { - t.Errorf("status does not match: %v, want: %v", gotStatus, wantStatus) + wantStatus := framework.AsStatus(framework.ErrNotFound) + if diff := cmp.Diff(wantStatus, gotStatus); diff != "" { + t.Errorf("status does not match (-want,+got):\n%s", diff) } } @@ -668,8 +671,8 @@ func TestNotEnoughRequests(t *testing.T) { } gotStatus := p.(framework.FilterPlugin).Filter(ctx, cycleState, test.pod, test.nodeInfo) - if !reflect.DeepEqual(gotStatus, test.wantStatus) { - t.Errorf("status does not match: %v, want: %v", gotStatus, test.wantStatus) + if diff := cmp.Diff(test.wantStatus, gotStatus); diff != "" { + t.Errorf("status does not match (-want,+got):\n%s", diff) } }) } @@ -729,8 +732,8 @@ func TestStorageRequests(t *testing.T) { } gotStatus := p.(framework.FilterPlugin).Filter(ctx, cycleState, test.pod, test.nodeInfo) - if !reflect.DeepEqual(gotStatus, test.wantStatus) { - t.Errorf("status does not match: %v, want: %v", gotStatus, test.wantStatus) + if diff := cmp.Diff(test.wantStatus, gotStatus); diff != "" { + t.Errorf("status does not match (-want,+got):\n%s", diff) } }) } @@ -1110,7 +1113,7 @@ func TestFitScore(t *testing.T) { gotPriorities = append(gotPriorities, framework.NodeScore{Name: n.Name, Score: score}) } - if !reflect.DeepEqual(test.expectedPriorities, gotPriorities) { + if diff := cmp.Diff(test.expectedPriorities, gotPriorities); diff != "" { t.Errorf("expected:\n\t%+v,\ngot:\n\t%+v", test.expectedPriorities, gotPriorities) } }) @@ -1276,7 +1279,7 @@ func TestEventsToRegister(t *testing.T) { for i := range actualClusterEvents { actualClusterEvents[i].QueueingHintFn = nil } - if diff := cmp.Diff(test.expectedClusterEvents, actualClusterEvents, cmpopts.EquateComparable(framework.ClusterEvent{})); diff != "" { + if diff := cmp.Diff(test.expectedClusterEvents, actualClusterEvents, clusterEventCmpOpts...); diff != "" { t.Error("Cluster Events doesn't match extected events (-expected +actual):\n", diff) } }) diff --git a/pkg/scheduler/framework/plugins/nodeunschedulable/node_unschedulable_test.go b/pkg/scheduler/framework/plugins/nodeunschedulable/node_unschedulable_test.go index ee4a6143b8c..0ded5785a3d 100644 --- a/pkg/scheduler/framework/plugins/nodeunschedulable/node_unschedulable_test.go +++ b/pkg/scheduler/framework/plugins/nodeunschedulable/node_unschedulable_test.go @@ -17,9 +17,10 @@ limitations under the License. package nodeunschedulable import ( - "reflect" "testing" + "github.com/google/go-cmp/cmp" + v1 "k8s.io/api/core/v1" "k8s.io/kubernetes/pkg/scheduler/framework" "k8s.io/kubernetes/pkg/scheduler/framework/plugins/feature" @@ -82,8 +83,8 @@ func TestNodeUnschedulable(t *testing.T) { t.Fatalf("creating plugin: %v", err) } gotStatus := p.(framework.FilterPlugin).Filter(ctx, nil, test.pod, nodeInfo) - if !reflect.DeepEqual(gotStatus, test.wantStatus) { - t.Errorf("status does not match: %v, want: %v", gotStatus, test.wantStatus) + if diff := cmp.Diff(test.wantStatus, gotStatus); diff != "" { + t.Errorf("status does not match (-want,+got):\n%s", diff) } } } diff --git a/pkg/scheduler/framework/plugins/nodevolumelimits/csi_test.go b/pkg/scheduler/framework/plugins/nodevolumelimits/csi_test.go index 65f8d140edb..801165f5bb8 100644 --- a/pkg/scheduler/framework/plugins/nodevolumelimits/csi_test.go +++ b/pkg/scheduler/framework/plugins/nodevolumelimits/csi_test.go @@ -19,7 +19,6 @@ package nodevolumelimits import ( "errors" "fmt" - "reflect" "strings" "testing" @@ -50,6 +49,18 @@ var ( scName = "csi-sc" ) +var statusCmpOpts = []cmp.Option{ + cmp.Comparer(func(s1 *framework.Status, s2 *framework.Status) bool { + if s1 == nil || s2 == nil { + return s1.IsSuccess() && s2.IsSuccess() + } + if s1.Code() == framework.Error { + return s1.AsError().Error() == s2.AsError().Error() + } + return s1.Code() == s2.Code() && s1.Plugin() == s2.Plugin() && s1.Message() == s2.Message() + }), +} + func TestCSILimits(t *testing.T) { runningPod := st.MakePod().PVC("csi-ebs.csi.aws.com-3").Obj() pendingVolumePod := st.MakePod().PVC("csi-4").Obj() @@ -637,13 +648,13 @@ func TestCSILimits(t *testing.T) { } _, ctx := ktesting.NewTestContext(t) _, gotPreFilterStatus := p.PreFilter(ctx, nil, test.newPod) - if diff := cmp.Diff(test.wantPreFilterStatus, gotPreFilterStatus); diff != "" { - t.Errorf("PreFilter status does not match (-want, +got): %s", diff) + if diff := cmp.Diff(test.wantPreFilterStatus, gotPreFilterStatus, statusCmpOpts...); diff != "" { + t.Errorf("PreFilter status does not match (-want, +got):\n%s", diff) } if gotPreFilterStatus.Code() != framework.Skip { gotStatus := p.Filter(ctx, nil, test.newPod, node) - if !reflect.DeepEqual(gotStatus, test.wantStatus) { - t.Errorf("Filter status does not match: %v, want: %v", gotStatus, test.wantStatus) + if diff := cmp.Diff(test.wantStatus, gotStatus, statusCmpOpts...); diff != "" { + t.Errorf("Filter status does not match (-want, +got):\n%s", diff) } } }) diff --git a/pkg/scheduler/framework/plugins/podtopologyspread/filtering_test.go b/pkg/scheduler/framework/plugins/podtopologyspread/filtering_test.go index e007f75abcb..54ef48b55fe 100644 --- a/pkg/scheduler/framework/plugins/podtopologyspread/filtering_test.go +++ b/pkg/scheduler/framework/plugins/podtopologyspread/filtering_test.go @@ -18,9 +18,7 @@ package podtopologyspread import ( "context" - "fmt" "math" - "reflect" "testing" "github.com/google/go-cmp/cmp" @@ -41,11 +39,14 @@ import ( "k8s.io/utils/ptr" ) -var cmpOpts = []cmp.Option{ - cmp.Comparer(func(s1 labels.Selector, s2 labels.Selector) bool { - return reflect.DeepEqual(s1, s2) - }), +var stateCmpOpts = []cmp.Option{ cmp.Comparer(func(p1, p2 criticalPaths) bool { + // Before comparing p1 and p2 we need to make sure that TopologyValues + // paired with the same MatchNum values are sorted alphabetically. + // It's not possible to substitute this sorting with calling cmpopts.SortSlices() + // instead, because the lessFn in SortSlices would not modify the actual + // criticalPaths objects, and the default comparer function would still + // operate on "unsorted" TopologyValue fields. p1.sort() p2.sort() return p1[0] == p2[0] && p1[1] == p2[1] @@ -1504,7 +1505,7 @@ func TestPreFilterState(t *testing.T) { if err != nil { t.Fatalf("failed to get PreFilterState from cyclestate: %v", err) } - if diff := cmp.Diff(tt.want, got, cmpOpts...); diff != "" { + if diff := cmp.Diff(tt.want, got, stateCmpOpts...); diff != "" { t.Errorf("PodTopologySpread#PreFilter() returned unexpected prefilter status: diff (-want,+got):\n%s", diff) } }) @@ -1999,7 +2000,7 @@ func TestPreFilterStateAddPod(t *testing.T) { if err != nil { t.Fatal(err) } - if diff := cmp.Diff(tt.want, state, cmpOpts...); diff != "" { + if diff := cmp.Diff(tt.want, state, stateCmpOpts...); diff != "" { t.Errorf("PodTopologySpread.AddPod() returned diff (-want,+got):\n%s", diff) } }) @@ -2307,7 +2308,7 @@ func TestPreFilterStateRemovePod(t *testing.T) { if err != nil { t.Fatal(err) } - if diff := cmp.Diff(tt.want, state, cmpOpts...); diff != "" { + if diff := cmp.Diff(tt.want, state, stateCmpOpts...); diff != "" { t.Errorf("PodTopologySpread.RemovePod() returned diff (-want,+got):\n%s", diff) } }) @@ -3415,9 +3416,9 @@ func TestPreFilterDisabled(t *testing.T) { p := plugintesting.SetupPlugin(ctx, t, topologySpreadFunc, &config.PodTopologySpreadArgs{DefaultingType: config.ListDefaulting}, cache.NewEmptySnapshot()) cycleState := framework.NewCycleState() gotStatus := p.(*PodTopologySpread).Filter(ctx, cycleState, pod, nodeInfo) - wantStatus := framework.AsStatus(fmt.Errorf(`reading "PreFilterPodTopologySpread" from cycleState: %w`, framework.ErrNotFound)) - if !reflect.DeepEqual(gotStatus, wantStatus) { - t.Errorf("status does not match: %v, want: %v", gotStatus, wantStatus) + wantStatus := framework.AsStatus(framework.ErrNotFound) + if diff := cmp.Diff(wantStatus, gotStatus); diff != "" { + t.Errorf("Status does not match (-want,+got):\n%s", diff) } } diff --git a/pkg/scheduler/framework/plugins/podtopologyspread/scoring_test.go b/pkg/scheduler/framework/plugins/podtopologyspread/scoring_test.go index 56a1625c613..3752e2269ea 100644 --- a/pkg/scheduler/framework/plugins/podtopologyspread/scoring_test.go +++ b/pkg/scheduler/framework/plugins/podtopologyspread/scoring_test.go @@ -612,7 +612,7 @@ func TestPreScoreStateEmptyNodes(t *testing.T) { if err != nil { t.Fatal(err) } - if diff := cmp.Diff(tt.want, got, cmpOpts...); diff != "" { + if diff := cmp.Diff(tt.want, got, stateCmpOpts...); diff != "" { t.Errorf("PodTopologySpread#PreScore() returned (-want, +got):\n%s", diff) } }) @@ -1407,7 +1407,7 @@ func TestPodTopologySpreadScore(t *testing.T) { if !status.IsSuccess() { t.Errorf("unexpected error: %v", status) } - if diff := cmp.Diff(tt.want, gotList, cmpOpts...); diff != "" { + if diff := cmp.Diff(tt.want, gotList); diff != "" { t.Errorf("unexpected scores (-want,+got):\n%s", diff) } }) diff --git a/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration_test.go b/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration_test.go index 4504531603e..73b240906c2 100644 --- a/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration_test.go +++ b/pkg/scheduler/framework/plugins/tainttoleration/taint_toleration_test.go @@ -18,7 +18,6 @@ package tainttoleration import ( "context" - "reflect" "testing" "github.com/google/go-cmp/cmp" @@ -263,8 +262,8 @@ func TestTaintTolerationScore(t *testing.T) { t.Errorf("unexpected error: %v", status) } - if !reflect.DeepEqual(test.expectedList, gotList) { - t.Errorf("expected:\n\t%+v,\ngot:\n\t%+v", test.expectedList, gotList) + if diff := cmp.Diff(test.expectedList, gotList); diff != "" { + t.Errorf("Unexpected NodeScoreList (-want,+got):\n%s", diff) } }) } @@ -349,8 +348,8 @@ func TestTaintTolerationFilter(t *testing.T) { t.Fatalf("creating plugin: %v", err) } gotStatus := p.(framework.FilterPlugin).Filter(ctx, nil, test.pod, nodeInfo) - if !reflect.DeepEqual(gotStatus, test.wantStatus) { - t.Errorf("status does not match: %v, want: %v", gotStatus, test.wantStatus) + if diff := cmp.Diff(test.wantStatus, gotStatus); diff != "" { + t.Errorf("Unexpected status (-want,+got):\n%s", diff) } }) } diff --git a/pkg/scheduler/framework/preemption/preemption_test.go b/pkg/scheduler/framework/preemption/preemption_test.go index b273cda8680..282019d9890 100644 --- a/pkg/scheduler/framework/preemption/preemption_test.go +++ b/pkg/scheduler/framework/preemption/preemption_test.go @@ -20,7 +20,6 @@ import ( "context" "errors" "fmt" - "reflect" "sort" "strings" "sync" @@ -754,8 +753,8 @@ func TestPrepareCandidate(t *testing.T) { } if asyncPreemptionEnabled { - if tt.expectedActivatedPods != nil && !reflect.DeepEqual(tt.expectedActivatedPods, fakeActivator.activatedPods) { - lastErrMsg = fmt.Sprintf("expected activated pods %v, got %v", tt.expectedActivatedPods, fakeActivator.activatedPods) + if diff := cmp.Diff(tt.expectedActivatedPods, fakeActivator.activatedPods); tt.expectedActivatedPods != nil && diff != "" { + lastErrMsg = fmt.Sprintf("Unexpected activated pods (-want,+got):\n%s", diff) return false, nil } if tt.expectedActivatedPods == nil && len(fakeActivator.activatedPods) != 0 { diff --git a/pkg/scheduler/framework/runtime/framework_test.go b/pkg/scheduler/framework/runtime/framework_test.go index 19f351ad267..07a0310b8ee 100644 --- a/pkg/scheduler/framework/runtime/framework_test.go +++ b/pkg/scheduler/framework/runtime/framework_test.go @@ -20,7 +20,6 @@ import ( "context" "errors" "fmt" - "reflect" "strings" "testing" "time" @@ -72,11 +71,14 @@ const ( var _ framework.ScorePlugin = &TestScoreWithNormalizePlugin{} var _ framework.ScorePlugin = &TestScorePlugin{} -var cmpOpts = []cmp.Option{ +var statusCmpOpts = []cmp.Option{ cmp.Comparer(func(s1 *framework.Status, s2 *framework.Status) bool { if s1 == nil || s2 == nil { return s1.IsSuccess() && s2.IsSuccess() } + if s1.Code() == framework.Error { + return s1.AsError().Error() == s2.AsError().Error() + } return s1.Code() == s2.Code() && s1.Plugin() == s2.Plugin() && s1.Message() == s2.Message() }), } @@ -924,7 +926,7 @@ func TestNewFrameworkMultiPointExpansion(t *testing.T) { } if tc.wantErr == "" { if diff := cmp.Diff(tc.wantPlugins, fw.ListPlugins()); diff != "" { - t.Fatalf("Unexpected eventToPlugin map (-want,+got):%s", diff) + t.Fatalf("Unexpected eventToPlugin map (-want,+got):\n%s", diff) } } }) @@ -982,8 +984,8 @@ func TestPreEnqueuePlugins(t *testing.T) { _ = f.Close() }() got := f.PreEnqueuePlugins() - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("PreEnqueuePlugins(): want %v, but got %v", tt.want, got) + if diff := cmp.Diff(tt.want, got); diff != "" { + t.Errorf("Unexpected PreEnqueuePlugins(): (-want,+got):\n%s", diff) } }) } @@ -1113,8 +1115,8 @@ func TestRunPreScorePlugins(t *testing.T) { t.Errorf("wrong status code. got: %v, want: %v", status, tt.wantStatusCode) } skipped := state.SkipScorePlugins - if d := cmp.Diff(skipped, tt.wantSkippedPlugins); d != "" { - t.Errorf("wrong skip score plugins. got: %v, want: %v, diff: %s", skipped, tt.wantSkippedPlugins, d) + if diff := cmp.Diff(tt.wantSkippedPlugins, skipped); diff != "" { + t.Errorf("wrong skip score plugins (-want, +got):\n%s", diff) } }) } @@ -1519,8 +1521,8 @@ func TestRunScorePlugins(t *testing.T) { if !status.IsSuccess() { t.Errorf("Expected status to be success.") } - if !reflect.DeepEqual(res, tt.want) { - t.Errorf("Score map after RunScorePlugin. got: %+v, want: %+v.", res, tt.want) + if diff := cmp.Diff(tt.want, res); diff != "" { + t.Errorf("Score map after RunScorePlugin (-want,+got):\n%s", diff) } }) } @@ -1747,15 +1749,15 @@ func TestRunPreFilterPlugins(t *testing.T) { state := framework.NewCycleState() result, status, _ := f.RunPreFilterPlugins(ctx, state, nil) - if d := cmp.Diff(result, tt.wantPreFilterResult); d != "" { - t.Errorf("wrong status. got: %v, want: %v, diff: %s", result, tt.wantPreFilterResult, d) + if diff := cmp.Diff(tt.wantPreFilterResult, result); diff != "" { + t.Errorf("wrong status (-want,+got):\n%s", diff) } if status.Code() != tt.wantStatusCode { t.Errorf("wrong status code. got: %v, want: %v", status, tt.wantStatusCode) } skipped := state.SkipFilterPlugins - if d := cmp.Diff(skipped, tt.wantSkippedPlugins); d != "" { - t.Errorf("wrong skip filter plugins. got: %v, want: %v, diff: %s", skipped, tt.wantSkippedPlugins, d) + if diff := cmp.Diff(tt.wantSkippedPlugins, skipped); diff != "" { + t.Errorf("wrong skip filter plugins (-want,+got):\n%s", diff) } }) } @@ -2136,8 +2138,8 @@ func TestFilterPlugins(t *testing.T) { state := framework.NewCycleState() state.SkipFilterPlugins = tt.skippedPlugins gotStatus := f.RunFilterPlugins(ctx, state, pod, nil) - if diff := cmp.Diff(gotStatus, tt.wantStatus, cmpOpts...); diff != "" { - t.Errorf("Unexpected status: (-got, +want):\n%s", diff) + if diff := cmp.Diff(tt.wantStatus, gotStatus, statusCmpOpts...); diff != "" { + t.Errorf("Unexpected status: (-want,+got):\n%s", diff) } }) } @@ -2262,8 +2264,9 @@ func TestPostFilterPlugins(t *testing.T) { _ = f.Close() }() _, gotStatus := f.RunPostFilterPlugins(ctx, nil, pod, nil) - if !reflect.DeepEqual(gotStatus, tt.wantStatus) { - t.Errorf("Unexpected status. got: %v, want: %v", gotStatus, tt.wantStatus) + + if diff := cmp.Diff(tt.wantStatus, gotStatus, statusCmpOpts...); diff != "" { + t.Errorf("Unexpected status (-want,+got):\n%s", diff) } }) } @@ -2430,8 +2433,8 @@ func TestFilterPluginsWithNominatedPods(t *testing.T) { }() tt.nodeInfo.SetNode(tt.node) gotStatus := f.RunFilterPluginsWithNominatedPods(ctx, framework.NewCycleState(), tt.pod, tt.nodeInfo) - if diff := cmp.Diff(gotStatus, tt.wantStatus, cmpOpts...); diff != "" { - t.Errorf("Unexpected status: (-got, +want):\n%s", diff) + if diff := cmp.Diff(tt.wantStatus, gotStatus, statusCmpOpts...); diff != "" { + t.Errorf("Unexpected status: (-want,+got):\n%s", diff) } }) } @@ -2592,8 +2595,8 @@ func TestPreBindPlugins(t *testing.T) { status := f.RunPreBindPlugins(ctx, nil, pod, "") - if !reflect.DeepEqual(status, tt.wantStatus) { - t.Errorf("wrong status code. got %v, want %v", status, tt.wantStatus) + if diff := cmp.Diff(tt.wantStatus, status, statusCmpOpts...); diff != "" { + t.Errorf("Wrong status code (-want,+got):\n%s", diff) } }) } @@ -2754,8 +2757,8 @@ func TestReservePlugins(t *testing.T) { status := f.RunReservePluginsReserve(ctx, nil, pod, "") - if !reflect.DeepEqual(status, tt.wantStatus) { - t.Errorf("wrong status code. got %v, want %v", status, tt.wantStatus) + if diff := cmp.Diff(tt.wantStatus, status, statusCmpOpts...); diff != "" { + t.Errorf("Wrong status code (-want,+got):\n%s", diff) } }) } @@ -2885,8 +2888,8 @@ func TestPermitPlugins(t *testing.T) { } status := f.RunPermitPlugins(ctx, nil, pod, "") - if !reflect.DeepEqual(status, tt.want) { - t.Errorf("wrong status code. got %v, want %v", status, tt.want) + if diff := cmp.Diff(tt.want, status, statusCmpOpts...); diff != "" { + t.Errorf("Wrong status code (-want,+got):\n%s", diff) } }) } @@ -3320,8 +3323,8 @@ func TestWaitOnPermit(t *testing.T) { go tt.action(f) got := f.WaitOnPermit(ctx, pod) - if !reflect.DeepEqual(tt.want, got) { - t.Errorf("Unexpected status: want %v, but got %v", tt.want, got) + if diff := cmp.Diff(tt.want, got, statusCmpOpts...); diff != "" { + t.Errorf("Unexpected status (-want,+got):\n%s", diff) } }) } @@ -3369,7 +3372,7 @@ func TestListPlugins(t *testing.T) { }() got := f.ListPlugins() if diff := cmp.Diff(tt.want, got); diff != "" { - t.Errorf("unexpected plugins (-want,+got):\n%s", diff) + t.Errorf("Unexpected plugins (-want,+got):\n%s", diff) } }) } diff --git a/pkg/scheduler/framework/runtime/registry_test.go b/pkg/scheduler/framework/runtime/registry_test.go index f3182d668eb..7f0d85c6231 100644 --- a/pkg/scheduler/framework/runtime/registry_test.go +++ b/pkg/scheduler/framework/runtime/registry_test.go @@ -18,9 +18,9 @@ package runtime import ( "context" - "reflect" "testing" + "github.com/google/go-cmp/cmp" "github.com/google/uuid" "k8s.io/apimachinery/pkg/runtime" @@ -66,9 +66,8 @@ func TestDecodeInto(t *testing.T) { if err := DecodeInto(test.args, &pluginFooConf); err != nil { t.Errorf("DecodeInto(): failed to decode args %+v: %v", test.args, err) } - if !reflect.DeepEqual(test.expected, pluginFooConf) { - t.Errorf("DecodeInto(): failed to decode plugin config, expected: %+v, got: %+v", - test.expected, pluginFooConf) + if diff := cmp.Diff(test.expected, pluginFooConf); diff != "" { + t.Errorf("DecodeInto(): failed to decode plugin config (-want,+got):\n%s", diff) } }) } diff --git a/pkg/scheduler/framework/types_test.go b/pkg/scheduler/framework/types_test.go index e8238eb67f4..1f8875ce78b 100644 --- a/pkg/scheduler/framework/types_test.go +++ b/pkg/scheduler/framework/types_test.go @@ -18,7 +18,6 @@ package framework import ( "fmt" - "reflect" "strings" "testing" @@ -39,6 +38,10 @@ import ( "k8s.io/kubernetes/test/utils/ktesting/initoption" ) +var nodeInfoCmpOpts = []cmp.Option{ + cmp.AllowUnexported(NodeInfo{}, PodInfo{}, podResource{}), +} + func TestNewResource(t *testing.T) { tests := []struct { name string @@ -73,8 +76,8 @@ func TestNewResource(t *testing.T) { for _, test := range tests { 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) + if diff := cmp.Diff(test.expected, r); diff != "" { + t.Errorf("Unexpected resource (-want, +got):\n%s", diff) } }) } @@ -112,8 +115,8 @@ func TestResourceClone(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) + if diff := cmp.Diff(test.expected, r); diff != "" { + t.Errorf("Unexpected resource (-want, +got):\n%s", diff) } }) } @@ -157,8 +160,8 @@ func TestResourceAddScalar(t *testing.T) { for _, test := range tests { 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) + if diff := cmp.Diff(test.expected, test.resource); diff != "" { + t.Errorf("Unexpected resource (-want, +got):\n%s", diff) } }) } @@ -209,8 +212,8 @@ func TestSetMaxResource(t *testing.T) { 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) + if diff := cmp.Diff(test.expected, test.resource); diff != "" { + t.Errorf("Unexpected resource (-want, +got):\n%s", diff) } }) } @@ -351,8 +354,8 @@ func TestNewNodeInfo(t *testing.T) { t.Errorf("Generation is not incremented. previous: %v, current: %v", gen, ni.Generation) } expected.Generation = ni.Generation - if !reflect.DeepEqual(expected, ni) { - t.Errorf("expected: %#v, got: %#v", expected, ni) + if diff := cmp.Diff(expected, ni, nodeInfoCmpOpts...); diff != "" { + t.Errorf("Unexpected NodeInfo (-want, +got):\n%s", diff) } } @@ -552,8 +555,8 @@ func TestNodeInfoClone(t *testing.T) { // 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) + if diff := cmp.Diff(test.expected, ni, nodeInfoCmpOpts...); diff != "" { + t.Errorf("Unexpected NodeInfo (-want, +got):\n%s", diff) } }) } @@ -892,8 +895,8 @@ func TestNodeInfoAddPod(t *testing.T) { } expected.Generation = ni.Generation - if diff := cmp.Diff(expected, ni, cmp.AllowUnexported(NodeInfo{}, PodInfo{}, podResource{})); diff != "" { - t.Errorf("unexpected diff (-want, +got):\n%s", diff) + if diff := cmp.Diff(expected, ni, nodeInfoCmpOpts...); diff != "" { + t.Errorf("Unexpected NodeInfo (-want, +got):\n%s", diff) } } @@ -1206,8 +1209,8 @@ func TestNodeInfoRemovePod(t *testing.T) { } test.expectedNodeInfo.Generation = ni.Generation - if !reflect.DeepEqual(test.expectedNodeInfo, ni) { - t.Errorf("expected: %#v, got: %#v", test.expectedNodeInfo, ni) + if diff := cmp.Diff(test.expectedNodeInfo, ni, nodeInfoCmpOpts...); diff != "" { + t.Errorf("Unexpected NodeInfo (-want, +got):\n%s", diff) } }) } @@ -1473,7 +1476,7 @@ func TestGetNamespacesFromPodAffinityTerm(t *testing.T) { }, }, test.term) if diff := cmp.Diff(test.want, got); diff != "" { - t.Errorf("unexpected diff (-want, +got):\n%s", diff) + t.Errorf("Unexpected namespaces (-want, +got):\n%s", diff) } }) } @@ -1867,8 +1870,8 @@ func TestPodInfoCalculateResources(t *testing.T) { }, } res := podInfo.calculateResource() - if !reflect.DeepEqual(tc.expectedResource, res) { - t.Errorf("Test: %s expected resource: %+v, got: %+v", tc.name, tc.expectedResource, res.resource) + if diff := cmp.Diff(tc.expectedResource, res, nodeInfoCmpOpts...); diff != "" { + t.Errorf("Unexpected resource (-want,+got):\n%s", diff) } }) } @@ -2065,8 +2068,8 @@ func TestCalculatePodResourcesWithResize(t *testing.T) { tt.resizeStatus) res := podInfo.calculateResource() - if !reflect.DeepEqual(tt.expectedResource, res) { - t.Errorf("Test: %s expected resource: %+v, got: %+v", tt.name, tt.expectedResource, res.resource) + if diff := cmp.Diff(tt.expectedResource, res, nodeInfoCmpOpts...); diff != "" { + t.Errorf("Unexpected podResource (-want, +got):\n%s", diff) } }) } diff --git a/pkg/scheduler/schedule_one_test.go b/pkg/scheduler/schedule_one_test.go index 6e628fa1b71..bbe0d9c9f5e 100644 --- a/pkg/scheduler/schedule_one_test.go +++ b/pkg/scheduler/schedule_one_test.go @@ -22,7 +22,6 @@ import ( "fmt" "math" "math/rand" - "reflect" "regexp" goruntime "runtime" "sort" @@ -32,7 +31,6 @@ import ( "time" "github.com/google/go-cmp/cmp" - "github.com/google/go-cmp/cmp/cmpopts" v1 "k8s.io/api/core/v1" eventsv1 "k8s.io/api/events/v1" @@ -86,6 +84,9 @@ var ( emptySnapshot = internalcache.NewEmptySnapshot() podTopologySpreadFunc = frameworkruntime.FactoryAdapter(feature.Features{}, podtopologyspread.New) errPrioritize = fmt.Errorf("priority map encounters an error") + schedulerCmpOpts = []cmp.Option{ + cmp.AllowUnexported(framework.NodeToStatus{}), + } ) type mockScheduleResult struct { @@ -387,7 +388,7 @@ func nodeToStatusDiff(want, got *framework.NodeToStatus) string { if want == nil || got == nil { return cmp.Diff(want, got) } - return cmp.Diff(*want, *got, cmp.AllowUnexported(framework.NodeToStatus{})) + return cmp.Diff(*want, *got, schedulerCmpOpts...) } func TestSchedulerMultipleProfilesScheduling(t *testing.T) { @@ -837,20 +838,24 @@ func TestSchedulerScheduleOne(t *testing.T) { } sched.ScheduleOne(ctx) <-called - if e, a := item.expectAssumedPod, gotAssumedPod; !reflect.DeepEqual(e, a) { - t.Errorf("assumed pod: wanted %v, got %v", e, a) + if diff := cmp.Diff(item.expectAssumedPod, gotAssumedPod); diff != "" { + t.Errorf("Unexpected assumed pod (-want,+got):\n%s", diff) } - if e, a := item.expectErrorPod, gotPod; !reflect.DeepEqual(e, a) { - t.Errorf("error pod: wanted %v, got %v", e, a) + if diff := cmp.Diff(item.expectErrorPod, gotPod); diff != "" { + t.Errorf("Unexpected error pod (-want,+got):\n%s", diff) } - if e, a := item.expectForgetPod, gotForgetPod; !reflect.DeepEqual(e, a) { - t.Errorf("forget pod: wanted %v, got %v", e, a) + if diff := cmp.Diff(item.expectForgetPod, gotForgetPod); diff != "" { + t.Errorf("Unexpected forget pod (-want,+got):\n%s", diff) } - if e, a := item.expectError, gotError; !reflect.DeepEqual(e, a) { - t.Errorf("error: wanted %v, got %v", e, a) + if item.expectError == nil || gotError == nil { + if !errors.Is(gotError, item.expectError) { + t.Errorf("Unexpected error. Wanted %v, got %v", item.expectError, gotError) + } + } else if item.expectError.Error() != gotError.Error() { + t.Errorf("Unexpected error. Wanted %v, got %v", item.expectError.Error(), gotError.Error()) } if diff := cmp.Diff(item.expectBind, gotBinding); diff != "" { - t.Errorf("got binding diff (-want, +got): %s", diff) + t.Errorf("Unexpected binding (-want,+got):\n%s", diff) } // We have to use wait here // because the Pod goes to the binding cycle in some test cases and the inflight pods might not be empty immediately at this point in such case. @@ -923,8 +928,8 @@ func TestSchedulerNoPhantomPodAfterExpire(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "bar", UID: types.UID("bar")}, Target: v1.ObjectReference{Kind: "Node", Name: node.Name}, } - if !reflect.DeepEqual(expectBinding, b) { - t.Errorf("binding want=%v, get=%v", expectBinding, b) + if diff := cmp.Diff(expectBinding, b); diff != "" { + t.Errorf("Unexpected binding (-want,+got):\n%s", diff) } case <-time.After(wait.ForeverTestTimeout): t.Fatalf("timeout in binding after %v", wait.ForeverTestTimeout) @@ -966,8 +971,10 @@ func TestSchedulerNoPhantomPodAfterDelete(t *testing.T) { UnschedulablePlugins: sets.New(nodeports.Name), }, } - if !reflect.DeepEqual(expectErr, err) { - t.Errorf("err want=%v, get=%v", expectErr, err) + if err == nil { + t.Errorf("expected error %v, got nil", expectErr) + } else if diff := cmp.Diff(expectErr, err, schedulerCmpOpts...); diff != "" { + t.Errorf("unexpected error (-want,+got):\n%s", diff) } case <-time.After(wait.ForeverTestTimeout): t.Fatalf("timeout in fitting after %v", wait.ForeverTestTimeout) @@ -993,8 +1000,8 @@ func TestSchedulerNoPhantomPodAfterDelete(t *testing.T) { ObjectMeta: metav1.ObjectMeta{Name: "bar", UID: types.UID("bar")}, Target: v1.ObjectReference{Kind: "Node", Name: node.Name}, } - if !reflect.DeepEqual(expectBinding, b) { - t.Errorf("binding want=%v, get=%v", expectBinding, b) + if diff := cmp.Diff(expectBinding, b); diff != "" { + t.Errorf("unexpected binding (-want,+got):\n%s", diff) } case <-time.After(wait.ForeverTestTimeout): t.Fatalf("timeout in binding after %v", wait.ForeverTestTimeout) @@ -1076,8 +1083,8 @@ func TestSchedulerFailedSchedulingReasons(t *testing.T) { if len(fmt.Sprint(expectErr)) > 150 { t.Errorf("message is too spammy ! %v ", len(fmt.Sprint(expectErr))) } - if !reflect.DeepEqual(expectErr, err) { - t.Errorf("\n err \nWANT=%+v,\nGOT=%+v", expectErr, err) + if diff := cmp.Diff(expectErr, err, schedulerCmpOpts...); diff != "" { + t.Errorf("Unexpected error (-want,+got):\n%s", diff) } case <-time.After(wait.ForeverTestTimeout): t.Fatalf("timeout after %v", wait.ForeverTestTimeout) @@ -2661,11 +2668,11 @@ func TestSchedulerSchedulePod(t *testing.T) { if gotOK != wantOK { t.Errorf("Expected err to be FitError: %v, but got %v (error: %v)", wantOK, gotOK, err) } else if gotOK { - if diff := cmp.Diff(wantFitErr, gotFitErr, cmpopts.IgnoreFields(framework.Diagnosis{}, "NodeToStatus")); diff != "" { - t.Errorf("Unexpected fitErr for map: (-want, +got): %s", diff) + if diff := cmp.Diff(wantFitErr, gotFitErr, schedulerCmpOpts...); diff != "" { + t.Errorf("Unexpected fitErr for map (-want, +got):\n%s", diff) } if diff := nodeToStatusDiff(wantFitErr.Diagnosis.NodeToStatus, gotFitErr.Diagnosis.NodeToStatus); diff != "" { - t.Errorf("Unexpected nodeToStatus within fitErr for map: (-want, +got): %s", diff) + t.Errorf("Unexpected nodeToStatus within fitErr for map: (-want, +got):\n%s", diff) } } } @@ -2719,11 +2726,11 @@ func TestFindFitAllError(t *testing.T) { }, framework.NewStatus(framework.UnschedulableAndUnresolvable)), UnschedulablePlugins: sets.New("MatchFilter"), } - if diff := cmp.Diff(diagnosis, expected, cmpopts.IgnoreFields(framework.Diagnosis{}, "NodeToStatus")); diff != "" { - t.Errorf("Unexpected diagnosis: (-want, +got): %s", diff) + if diff := cmp.Diff(expected, diagnosis, schedulerCmpOpts...); diff != "" { + t.Errorf("Unexpected diagnosis (-want, +got):\n%s", diff) } - if diff := nodeToStatusDiff(diagnosis.NodeToStatus, expected.NodeToStatus); diff != "" { - t.Errorf("Unexpected nodeToStatus within diagnosis: (-want, +got): %s", diff) + if diff := nodeToStatusDiff(expected.NodeToStatus, diagnosis.NodeToStatus); diff != "" { + t.Errorf("Unexpected nodeToStatus within diagnosis: (-want, +got):\n%s", diff) } } @@ -2761,7 +2768,7 @@ func TestFindFitSomeError(t *testing.T) { } if diff := cmp.Diff(sets.New("MatchFilter"), diagnosis.UnschedulablePlugins); diff != "" { - t.Errorf("Unexpected unschedulablePlugins: (-want, +got): %s", diagnosis.UnschedulablePlugins) + t.Errorf("Unexpected unschedulablePlugins: (-want, +got):\n%s", diff) } for _, node := range nodes { @@ -3692,8 +3699,8 @@ func setupTestSchedulerWithOnePodOnNode(ctx context.Context, t *testing.T, queue ObjectMeta: metav1.ObjectMeta{Name: pod.Name, UID: types.UID(pod.Name)}, Target: v1.ObjectReference{Kind: "Node", Name: node.Name}, } - if !reflect.DeepEqual(expectBinding, b) { - t.Errorf("binding want=%v, get=%v", expectBinding, b) + if diff := cmp.Diff(expectBinding, b); diff != "" { + t.Errorf("Unexpected binding (-want,+got):\n%s", diff) } case <-time.After(wait.ForeverTestTimeout): t.Fatalf("timeout after %v", wait.ForeverTestTimeout)