From f6ccc4426ad24317a537d05d0757f73a931a1c96 Mon Sep 17 00:00:00 2001 From: Francesco Romani Date: Thu, 14 Oct 2021 16:49:58 +0200 Subject: [PATCH] cpumanager: test: use proper subtests The exisiting unit tests where performing subtests without actually using the full features of the testing package (https://pkg.go.dev/testing#hdr-Subtests_and_Sub_benchmarks) Update them with fairly minimal changes. The patch is deceptively large because we need to move the code inside a new block. Signed-off-by: Francesco Romani --- .../cm/cpumanager/cpu_assignment_test.go | 101 ++++++++++-------- 1 file changed, 56 insertions(+), 45 deletions(-) diff --git a/pkg/kubelet/cm/cpumanager/cpu_assignment_test.go b/pkg/kubelet/cm/cpumanager/cpu_assignment_test.go index 7a25e3887c1..998fab53796 100644 --- a/pkg/kubelet/cm/cpumanager/cpu_assignment_test.go +++ b/pkg/kubelet/cm/cpumanager/cpu_assignment_test.go @@ -64,11 +64,14 @@ func TestCPUAccumulatorFreeSockets(t *testing.T) { } for _, tc := range testCases { - acc := newCPUAccumulator(tc.topo, tc.availableCPUs, 0) - result := acc.freeSockets() - if !reflect.DeepEqual(result, tc.expect) { - t.Errorf("[%s] expected %v to equal %v", tc.description, result, tc.expect) - } + t.Run(tc.description, func(t *testing.T) { + acc := newCPUAccumulator(tc.topo, tc.availableCPUs, 0) + result := acc.freeSockets() + if !reflect.DeepEqual(result, tc.expect) { + t.Errorf("expected %v to equal %v", result, tc.expect) + + } + }) } } @@ -130,11 +133,13 @@ func TestCPUAccumulatorFreeCores(t *testing.T) { } for _, tc := range testCases { - acc := newCPUAccumulator(tc.topo, tc.availableCPUs, 0) - result := acc.freeCores() - if !reflect.DeepEqual(result, tc.expect) { - t.Errorf("[%s] expected %v to equal %v", tc.description, result, tc.expect) - } + t.Run(tc.description, func(t *testing.T) { + acc := newCPUAccumulator(tc.topo, tc.availableCPUs, 0) + result := acc.freeCores() + if !reflect.DeepEqual(result, tc.expect) { + t.Errorf("expected %v to equal %v", result, tc.expect) + } + }) } } @@ -184,11 +189,13 @@ func TestCPUAccumulatorFreeCPUs(t *testing.T) { } for _, tc := range testCases { - acc := newCPUAccumulator(tc.topo, tc.availableCPUs, 0) - result := acc.freeCPUs() - if !reflect.DeepEqual(result, tc.expect) { - t.Errorf("[%s] expected %v to equal %v", tc.description, result, tc.expect) - } + t.Run(tc.description, func(t *testing.T) { + acc := newCPUAccumulator(tc.topo, tc.availableCPUs, 0) + result := acc.freeCPUs() + if !reflect.DeepEqual(result, tc.expect) { + t.Errorf("expected %v to equal %v", result, tc.expect) + } + }) } } @@ -268,31 +275,33 @@ func TestCPUAccumulatorTake(t *testing.T) { } for _, tc := range testCases { - acc := newCPUAccumulator(tc.topo, tc.availableCPUs, tc.numCPUs) - totalTaken := 0 - for _, cpus := range tc.takeCPUs { - acc.take(cpus) - totalTaken += cpus.Size() - } - if tc.expectSatisfied != acc.isSatisfied() { - t.Errorf("[%s] expected acc.isSatisfied() to be %t", tc.description, tc.expectSatisfied) - } - if tc.expectFailed != acc.isFailed() { - t.Errorf("[%s] expected acc.isFailed() to be %t", tc.description, tc.expectFailed) - } - for _, cpus := range tc.takeCPUs { - availableCPUs := acc.details.CPUs() - if cpus.Intersection(availableCPUs).Size() > 0 { - t.Errorf("[%s] expected intersection of taken cpus [%s] and acc.details.CPUs() [%s] to be empty", tc.description, cpus, availableCPUs) + t.Run(tc.description, func(t *testing.T) { + acc := newCPUAccumulator(tc.topo, tc.availableCPUs, tc.numCPUs) + totalTaken := 0 + for _, cpus := range tc.takeCPUs { + acc.take(cpus) + totalTaken += cpus.Size() } - if !cpus.IsSubsetOf(acc.result) { - t.Errorf("[%s] expected [%s] to be a subset of acc.result [%s]", tc.description, cpus, acc.result) + if tc.expectSatisfied != acc.isSatisfied() { + t.Errorf("expected acc.isSatisfied() to be %t", tc.expectSatisfied) } - } - expNumCPUsNeeded := tc.numCPUs - totalTaken - if acc.numCPUsNeeded != expNumCPUsNeeded { - t.Errorf("[%s] expected acc.numCPUsNeeded to be %d (got %d)", tc.description, expNumCPUsNeeded, acc.numCPUsNeeded) - } + if tc.expectFailed != acc.isFailed() { + t.Errorf("expected acc.isFailed() to be %t", tc.expectFailed) + } + for _, cpus := range tc.takeCPUs { + availableCPUs := acc.details.CPUs() + if cpus.Intersection(availableCPUs).Size() > 0 { + t.Errorf("expected intersection of taken cpus [%s] and acc.details.CPUs() [%s] to be empty", cpus, availableCPUs) + } + if !cpus.IsSubsetOf(acc.result) { + t.Errorf("expected [%s] to be a subset of acc.result [%s]", cpus, acc.result) + } + } + expNumCPUsNeeded := tc.numCPUs - totalTaken + if acc.numCPUsNeeded != expNumCPUsNeeded { + t.Errorf("expected acc.numCPUsNeeded to be %d (got %d)", expNumCPUsNeeded, acc.numCPUsNeeded) + } + }) } } @@ -380,12 +389,14 @@ func TestTakeByTopology(t *testing.T) { } for _, tc := range testCases { - result, err := takeByTopology(tc.topo, tc.availableCPUs, tc.numCPUs) - if tc.expErr != "" && err.Error() != tc.expErr { - t.Errorf("expected error to be [%v] but it was [%v] in test \"%s\"", tc.expErr, err, tc.description) - } - if !result.Equals(tc.expResult) { - t.Errorf("expected result [%s] to equal [%s] in test \"%s\"", result, tc.expResult, tc.description) - } + t.Run(tc.description, func(t *testing.T) { + result, err := takeByTopology(tc.topo, tc.availableCPUs, tc.numCPUs) + if tc.expErr != "" && err.Error() != tc.expErr { + t.Errorf("expected error to be [%v] but it was [%v]", tc.expErr, err) + } + if !result.Equals(tc.expResult) { + t.Errorf("expected result [%s] to equal [%s]", result, tc.expResult) + } + }) } }