From 6f5696b5373e2af7c31579a5935da143b44a48a2 Mon Sep 17 00:00:00 2001 From: Patrick Ohly Date: Wed, 27 Mar 2024 10:18:53 +0100 Subject: [PATCH] dra scheduler: simplify unit tests The guideline in https://github.com/kubernetes/community/blob/master/sig-scheduling/CONTRIBUTING.md#technical-and-style-guidelines is to not compare error strings. This makes the tests less precise. In return, unit tests don't need to be updated when error strings change. --- .../namedresourcesmodel_test.go | 34 +++++++++---------- .../structuredparameters_test.go | 22 ++++++------ 2 files changed, 26 insertions(+), 30 deletions(-) diff --git a/pkg/scheduler/framework/plugins/dynamicresources/structured/namedresources/namedresourcesmodel_test.go b/pkg/scheduler/framework/plugins/dynamicresources/structured/namedresources/namedresourcesmodel_test.go index 0195336a49a..aa920757274 100644 --- a/pkg/scheduler/framework/plugins/dynamicresources/structured/namedresources/namedresourcesmodel_test.go +++ b/pkg/scheduler/framework/plugins/dynamicresources/structured/namedresources/namedresourcesmodel_test.go @@ -189,29 +189,29 @@ func TestController(t *testing.T) { filter *resourceapi.NamedResourcesFilter requests []*resourceapi.NamedResourcesRequest - expectCreateErr string + expectCreateErr bool expectAllocation []string - expectAllocateErr string + expectAllocateErr bool }{ "empty": {}, "broken-filter": { filter: filterBrokenType, - expectCreateErr: "compile class filter CEL expression: must evaluate to bool", + expectCreateErr: true, }, "broken-request": { requests: []*resourceapi.NamedResourcesRequest{requestBrokenType}, - expectCreateErr: "compile request CEL expression: must evaluate to bool", + expectCreateErr: true, }, "no-resources": { filter: filterAny, requests: []*resourceapi.NamedResourcesRequest{requestAny}, - expectAllocateErr: "insufficient resources", + expectAllocateErr: true, }, "okay": { @@ -227,7 +227,7 @@ func TestController(t *testing.T) { filter: filterNone, requests: []*resourceapi.NamedResourcesRequest{requestAny}, - expectAllocateErr: "insufficient resources", + expectAllocateErr: true, }, "request-mismatch": { @@ -235,7 +235,7 @@ func TestController(t *testing.T) { filter: filterAny, requests: []*resourceapi.NamedResourcesRequest{requestNone}, - expectAllocateErr: "insufficient resources", + expectAllocateErr: true, }, "many": { @@ -251,7 +251,7 @@ func TestController(t *testing.T) { filter: filterAny, requests: []*resourceapi.NamedResourcesRequest{requestAny, requestAny}, - expectAllocateErr: "insufficient resources", + expectAllocateErr: true, }, "filter-evaluation-error": { @@ -259,7 +259,7 @@ func TestController(t *testing.T) { filter: filterBrokenEvaluation, requests: []*resourceapi.NamedResourcesRequest{requestAny}, - expectAllocateErr: "evaluate filter CEL expression: no such key: no-such-attribute", + expectAllocateErr: true, }, "request-evaluation-error": { @@ -267,7 +267,7 @@ func TestController(t *testing.T) { filter: filterAny, requests: []*resourceapi.NamedResourcesRequest{requestBrokenEvaluation}, - expectAllocateErr: "evaluate request CEL expression: no such key: no-such-attribute", + expectAllocateErr: true, }, "filter-attribute": { @@ -293,26 +293,24 @@ func TestController(t *testing.T) { controller, createErr := NewClaimController(tc.filter, tc.requests) if createErr != nil { - if tc.expectCreateErr == "" { + if !tc.expectCreateErr { tCtx.Fatalf("unexpected create error: %v", createErr) } - require.Equal(tCtx, tc.expectCreateErr, createErr.Error()) return } - if tc.expectCreateErr != "" { - tCtx.Fatalf("did not get expected create error: %v", tc.expectCreateErr) + if tc.expectCreateErr { + tCtx.Fatalf("did not get expected create error") } allocation, createErr := controller.Allocate(tCtx, tc.model) if createErr != nil { - if tc.expectAllocateErr == "" { + if !tc.expectAllocateErr { tCtx.Fatalf("unexpected allocate error: %v", createErr) } - require.Equal(tCtx, tc.expectAllocateErr, createErr.Error()) return } - if tc.expectAllocateErr != "" { - tCtx.Fatalf("did not get expected allocate error: %v", tc.expectAllocateErr) + if tc.expectAllocateErr { + tCtx.Fatalf("did not get expected allocate error") } expectAllocation := []*resourceapi.NamedResourcesAllocationResult{} diff --git a/pkg/scheduler/framework/plugins/dynamicresources/structuredparameters_test.go b/pkg/scheduler/framework/plugins/dynamicresources/structuredparameters_test.go index 681ce237f8e..2c82fd9f608 100644 --- a/pkg/scheduler/framework/plugins/dynamicresources/structuredparameters_test.go +++ b/pkg/scheduler/framework/plugins/dynamicresources/structuredparameters_test.go @@ -42,14 +42,14 @@ func TestModel(t *testing.T) { inFlight map[types.UID]resourceapi.ResourceClaimStatus wantResources resources - wantErr string + wantErr bool }{ "empty": {}, "slice-list-error": { slices: sliceError("slice list error"), - wantErr: "list node resource slices: slice list error", + wantErr: true, }, "unknown-model": { @@ -949,14 +949,13 @@ func TestModel(t *testing.T) { actualResources, actualErr := newResourceModel(tCtx.Logger(), slices, claims, &inFlightAllocations) if actualErr != nil { - if tc.wantErr == "" { + if !tc.wantErr { tCtx.Fatalf("unexpected error: %v", actualErr) } - require.Equal(tCtx, tc.wantErr, actualErr.Error()) return } - if tc.wantErr != "" { - tCtx.Fatalf("did not get expected error: %v", tc.wantErr) + if tc.wantErr { + tCtx.Fatalf("did not get expected error") } expectResources := tc.wantResources @@ -1095,7 +1094,7 @@ func TestController(t *testing.T) { classParameters *resourceapi.ResourceClassParameters claimParameters *resourceapi.ResourceClaimParameters - expectCreateErr string + expectCreateErr bool expectNodeResults nodeResults }{ "empty": { @@ -1127,7 +1126,7 @@ func TestController(t *testing.T) { }}, }, - expectCreateErr: "claim parameters : driverRequests[0].requests[0]: no supported structured parameters found", + expectCreateErr: true, }, "no-resources": { @@ -1212,14 +1211,13 @@ func TestController(t *testing.T) { controller, err := newClaimController(tCtx.Logger(), tc.class, tc.classParameters, tc.claimParameters) if err != nil { - if tc.expectCreateErr == "" { + if !tc.expectCreateErr { tCtx.Fatalf("unexpected error: %v", err) } - require.Equal(tCtx, tc.expectCreateErr, err.Error()) return } - if tc.expectCreateErr != "" { - tCtx.Fatalf("did not get expected error: %v", tc.expectCreateErr) + if tc.expectCreateErr { + tCtx.Fatalf("did not get expected error") } for nodeName, expect := range tc.expectNodeResults {