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.

Sign up for GitHub

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

Jump to bottom

More code-review changes from k/utlils cpuset review #115359

Merged
merged 3 commits into from Feb 28, 2023
Merged

More code-review changes from k/utlils cpuset review #115359

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
22d3f67
cpuset: Fix Parse() error message for n-k s.t. k<n
iancoolidge Jan 27, 2023
b536851
cpuset: Add a few more test cases
iancoolidge Jan 27, 2023
d4a1bf8
cpuset: Convert Fatalf to Errrof in tests
iancoolidge Feb 21, 2023
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
2 changes: 1 addition & 1 deletion pkg/kubelet/cm/cpuset/cpuset.go
View file
Open in desktop
Expand Up @@ -230,7 +230,7 @@ func Parse(s string) (CPUSet, error) {
return New(), err
}
if start > end {
return New(), fmt.Errorf("invalid range %q (%d >= %d)", r, start, end)
return New(), fmt.Errorf("invalid range %q (%d > %d)", r, start, end)
}
// start == end is acceptable (1-1 -> 1)

Expand Down
62 changes: 44 additions & 18 deletions pkg/kubelet/cm/cpuset/cpuset_test.go
View file
Open in desktop
Expand Up @@ -18,6 +18,7 @@ package cpuset

import (
"reflect"
"sort"
"testing"
)

Expand All @@ -34,7 +35,7 @@ func TestCPUSetSize(t *testing.T) {
for _, c := range testCases {
actual := c.cpuset.Size()
if actual != c.expected {
t.Fatalf("expected: %d, actual: %d, cpuset: [%v]", c.expected, actual, c.cpuset)
t.Errorf("expected: %d, actual: %d, cpuset: [%v]", c.expected, actual, c.cpuset)
}
}
}
Expand All @@ -52,7 +53,7 @@ func TestCPUSetIsEmpty(t *testing.T) {
for _, c := range testCases {
actual := c.cpuset.IsEmpty()
if actual != c.expected {
t.Fatalf("expected: %t, IsEmpty() returned: %t, cpuset: [%v]", c.expected, actual, c.cpuset)
t.Errorf("expected: %t, IsEmpty() returned: %t, cpuset: [%v]", c.expected, actual, c.cpuset)
}
}
}
Expand All @@ -71,12 +72,12 @@ func TestCPUSetContains(t *testing.T) {
for _, c := range testCases {
for _, elem := range c.mustContain {
if !c.cpuset.Contains(elem) {
t.Fatalf("expected cpuset to contain element %d: [%v]", elem, c.cpuset)
t.Errorf("expected cpuset to contain element %d: [%v]", elem, c.cpuset)
}
}
for _, elem := range c.mustNotContain {
if c.cpuset.Contains(elem) {
t.Fatalf("expected cpuset not to contain element %d: [%v]", elem, c.cpuset)
t.Errorf("expected cpuset not to contain element %d: [%v]", elem, c.cpuset)
}
}
}
Expand All @@ -90,6 +91,7 @@ func TestCPUSetEqual(t *testing.T) {
{New(), New()},
{New(5), New(5)},
{New(1, 2, 3, 4, 5), New(1, 2, 3, 4, 5)},
{New(5, 4, 3, 2, 1), New(1, 2, 3, 4, 5)},
}

shouldNotEqual := []struct {
Expand All @@ -106,12 +108,12 @@ func TestCPUSetEqual(t *testing.T) {

for _, c := range shouldEqual {
if !c.s1.Equals(c.s2) {
t.Fatalf("expected cpusets to be equal: s1: [%v], s2: [%v]", c.s1, c.s2)
t.Errorf("expected cpusets to be equal: s1: [%v], s2: [%v]", c.s1, c.s2)
}
}
for _, c := range shouldNotEqual {
if c.s1.Equals(c.s2) {
t.Fatalf("expected cpusets to not be equal: s1: [%v], s2: [%v]", c.s1, c.s2)
t.Errorf("expected cpusets to not be equal: s1: [%v], s2: [%v]", c.s1, c.s2)
}
}
}
Expand Down Expand Up @@ -139,16 +141,22 @@ func TestCPUSetIsSubsetOf(t *testing.T) {
shouldNotBeSubset := []struct {
s1 CPUSet
s2 CPUSet
}{}
}{
// A set with more elements is not a subset.
{New(5), New()},

// Disjoint set is not a subset.
{New(6), New(5)},
}

for _, c := range shouldBeSubset {
if !c.s1.IsSubsetOf(c.s2) {
t.Fatalf("expected s1 to be a subset of s2: s1: [%v], s2: [%v]", c.s1, c.s2)
t.Errorf("expected s1 to be a subset of s2: s1: [%v], s2: [%v]", c.s1, c.s2)
}
}
for _, c := range shouldNotBeSubset {
if c.s1.IsSubsetOf(c.s2) {
t.Fatalf("expected s1 to not be a subset of s2: s1: [%v], s2: [%v]", c.s1, c.s2)
t.Errorf("expected s1 to not be a subset of s2: s1: [%v], s2: [%v]", c.s1, c.s2)
}
}
}
Expand Down Expand Up @@ -185,7 +193,7 @@ func TestCPUSetUnion(t *testing.T) {
for _, c := range testCases {
result := c.s1.Union(c.others...)
if !result.Equals(c.expected) {
t.Fatalf("expected the union of s1 and s2 to be [%v] (got [%v]), others: [%v]", c.expected, result, c.others)
t.Errorf("expected the union of s1 and s2 to be [%v] (got [%v]), others: [%v]", c.expected, result, c.others)
}
}
}
Expand Down Expand Up @@ -216,7 +224,7 @@ func TestCPUSetIntersection(t *testing.T) {
for _, c := range testCases {
result := c.s1.Intersection(c.s2)
if !result.Equals(c.expected) {
t.Fatalf("expected the intersection of s1 and s2 to be [%v] (got [%v]), s1: [%v], s2: [%v]", c.expected, result, c.s1, c.s2)
t.Errorf("expected the intersection of s1 and s2 to be [%v] (got [%v]), s1: [%v], s2: [%v]", c.expected, result, c.s1, c.s2)
}
}
}
Expand Down Expand Up @@ -247,25 +255,34 @@ func TestCPUSetDifference(t *testing.T) {
for _, c := range testCases {
result := c.s1.Difference(c.s2)
if !result.Equals(c.expected) {
t.Fatalf("expected the difference of s1 and s2 to be [%v] (got [%v]), s1: [%v], s2: [%v]", c.expected, result, c.s1, c.s2)
t.Errorf("expected the difference of s1 and s2 to be [%v] (got [%v]), s1: [%v], s2: [%v]", c.expected, result, c.s1, c.s2)
}
}
}

func TestCPUSetList(t *testing.T) {
testCases := []struct {
set CPUSet
expected []int
expected []int // must be sorted
}{
{New(), []int{}},
{New(5), []int{5}},
{New(1, 2, 3, 4, 5), []int{1, 2, 3, 4, 5}},
{New(5, 4, 3, 2, 1), []int{1, 2, 3, 4, 5}},
}

for _, c := range testCases {
result := c.set.List()
if !reflect.DeepEqual(result, c.expected) {
t.Fatalf("expected set as slice to be [%v] (got [%v]), s: [%v]", c.expected, result, c.set)
t.Errorf("unexpected List() contents. got [%v] want [%v] (set: [%v])", result, c.expected, c.set)
}

// We cannot rely on internal storage order details for a unit test.
// The best we can do is to sort the output of 'UnsortedList'.
result = c.set.UnsortedList()
sort.Ints(result)
if !reflect.DeepEqual(result, c.expected) {
t.Errorf("unexpected UnsortedList() contents. got [%v] want [%v] (set: [%v])", result, c.expected, c.set)
}
}
}
Expand All @@ -284,7 +301,7 @@ func TestCPUSetString(t *testing.T) {
for _, c := range testCases {
result := c.set.String()
if result != c.expected {
t.Fatalf("expected set as string to be %s (got \"%s\"), s: [%v]", c.expected, result, c.set)
t.Errorf("expected set as string to be %s (got \"%s\"), s: [%v]", c.expected, result, c.set)
}
}
}
Expand All @@ -307,10 +324,10 @@ func TestParse(t *testing.T) {
for _, c := range positiveTestCases {
result, err := Parse(c.cpusetString)
if err != nil {
t.Fatalf("expected error not to have occurred: %v", err)
t.Errorf("expected error not to have occurred: %v", err)
}
if !result.Equals(c.expected) {
t.Fatalf("expected string \"%s\" to parse as [%v] (got [%v])", c.cpusetString, c.expected, result)
t.Errorf("expected string \"%s\" to parse as [%v] (got [%v])", c.cpusetString, c.expected, result)
}
}

Expand All @@ -326,7 +343,16 @@ func TestParse(t *testing.T) {
for _, c := range negativeTestCases {
result, err := Parse(c)
if err == nil {
t.Fatalf("expected parse failure of \"%s\", but it succeeded as \"%s\"", c, result.String())
t.Errorf("expected parse failure of \"%s\", but it succeeded as \"%s\"", c, result.String())
}
}
}

func TestClone(t *testing.T) {
original := New(1, 2, 3, 4, 5)
clone := original.Clone()

if !original.Equals(clone) {
t.Errorf("expected clone [%v] to equal original [%v]", clone, original)
}
}