Skip to content

Commit

Permalink
dra scheduler: simplify unit tests
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
pohly committed Mar 27, 2024
1 parent 458e227 commit 6f5696b
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 30 deletions.
Expand Up @@ -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": {
Expand All @@ -227,15 +227,15 @@ func TestController(t *testing.T) {
filter: filterNone,
requests: []*resourceapi.NamedResourcesRequest{requestAny},

expectAllocateErr: "insufficient resources",
expectAllocateErr: true,
},

"request-mismatch": {
model: oneInstance,
filter: filterAny,
requests: []*resourceapi.NamedResourcesRequest{requestNone},

expectAllocateErr: "insufficient resources",
expectAllocateErr: true,
},

"many": {
Expand All @@ -251,23 +251,23 @@ func TestController(t *testing.T) {
filter: filterAny,
requests: []*resourceapi.NamedResourcesRequest{requestAny, requestAny},

expectAllocateErr: "insufficient resources",
expectAllocateErr: true,
},

"filter-evaluation-error": {
model: oneInstance,
filter: filterBrokenEvaluation,
requests: []*resourceapi.NamedResourcesRequest{requestAny},

expectAllocateErr: "evaluate filter CEL expression: no such key: no-such-attribute",
expectAllocateErr: true,
},

"request-evaluation-error": {
model: oneInstance,
filter: filterAny,
requests: []*resourceapi.NamedResourcesRequest{requestBrokenEvaluation},

expectAllocateErr: "evaluate request CEL expression: no such key: no-such-attribute",
expectAllocateErr: true,
},

"filter-attribute": {
Expand All @@ -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{}
Expand Down
Expand Up @@ -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": {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1095,7 +1094,7 @@ func TestController(t *testing.T) {
classParameters *resourceapi.ResourceClassParameters
claimParameters *resourceapi.ResourceClaimParameters

expectCreateErr string
expectCreateErr bool
expectNodeResults nodeResults
}{
"empty": {
Expand Down Expand Up @@ -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": {
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 6f5696b

Please sign in to comment.