use subtest for table units

remove duplicate testname from error msg

remove subtest for test setup loop

do not break on test failure

  https://github.com/kubernetes/kubernetes/pull/63665#discussion_r203571355

remove duplicate test.name in output

  https://github.com/kubernetes/kubernetes/pull/63665#discussion_r203574001
  https://github.com/kubernetes/kubernetes/pull/63665#discussion_r203574012
pull/8/head
John Calabrese 2018-06-01 10:16:50 -04:00
parent 53ee0c8652
commit ad234e58be
4 changed files with 411 additions and 370 deletions

View File

@ -343,6 +343,7 @@ func TestUpdateResult(t *testing.T) {
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
node := schedulercache.NewNodeInfo()
testNode := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: test.nodeName}}
node.SetNode(testNode)
@ -373,14 +374,14 @@ func TestUpdateResult(t *testing.T) {
cachedMapItem, ok := nodeCache.cache[test.predicateKey]
if !ok {
t.Errorf("Failed: %s, can't find expected cache item: %v",
test.name, test.expectCacheItem)
t.Errorf("can't find expected cache item: %v", test.expectCacheItem)
} else {
if !reflect.DeepEqual(cachedMapItem[test.equivalenceHash], test.expectCacheItem) {
t.Errorf("Failed: %s, expected cached item: %v, but got: %v",
test.name, test.expectCacheItem, cachedMapItem[test.equivalenceHash])
t.Errorf("expected cached item: %v, but got: %v",
test.expectCacheItem, cachedMapItem[test.equivalenceHash])
}
}
})
}
}
@ -481,6 +482,7 @@ func TestLookupResult(t *testing.T) {
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
testNode := &v1.Node{ObjectMeta: metav1.ObjectMeta{Name: test.nodeName}}
// Initialize and populate equivalence class cache.
@ -536,6 +538,7 @@ func TestLookupResult(t *testing.T) {
t.Errorf("Failed: %s, expected reasons: %v, but got: %v",
test.name, test.expectedPredicateItem.reasons, reasons)
}
})
}
}
@ -658,6 +661,7 @@ func TestInvalidateCachedPredicateItemOfAllNodes(t *testing.T) {
testPredicate := "GeneralPredicates"
// tests is used to initialize all nodes
tests := []struct {
name string
podName string
nodeName string
equivalenceHashForUpdatePredicate uint64
@ -665,6 +669,7 @@ func TestInvalidateCachedPredicateItemOfAllNodes(t *testing.T) {
cache schedulercache.Cache
}{
{
name: "hash predicate 123 not fits host ports",
podName: "testPod",
nodeName: "node1",
equivalenceHashForUpdatePredicate: 123,
@ -677,6 +682,7 @@ func TestInvalidateCachedPredicateItemOfAllNodes(t *testing.T) {
cache: &upToDateCache{},
},
{
name: "hash predicate 456 not fits host ports",
podName: "testPod",
nodeName: "node2",
equivalenceHashForUpdatePredicate: 456,
@ -689,6 +695,7 @@ func TestInvalidateCachedPredicateItemOfAllNodes(t *testing.T) {
cache: &upToDateCache{},
},
{
name: "hash predicate 123 fits",
podName: "testPod",
nodeName: "node3",
equivalenceHashForUpdatePredicate: 123,
@ -723,13 +730,14 @@ func TestInvalidateCachedPredicateItemOfAllNodes(t *testing.T) {
// there should be no cached predicate any more
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
if nodeCache, exist := ecache.nodeToCache[test.nodeName]; exist {
if _, exist := nodeCache.cache[testPredicate]; exist {
t.Errorf("Failed: cached item for predicate key: %v on node: %v should be invalidated",
testPredicate, test.nodeName)
break
}
}
})
}
}
@ -737,6 +745,7 @@ func TestInvalidateAllCachedPredicateItemOfNode(t *testing.T) {
testPredicate := "GeneralPredicates"
// tests is used to initialize all nodes
tests := []struct {
name string
podName string
nodeName string
equivalenceHashForUpdatePredicate uint64
@ -744,6 +753,7 @@ func TestInvalidateAllCachedPredicateItemOfNode(t *testing.T) {
cache schedulercache.Cache
}{
{
name: "hash predicate 123 not fits host ports",
podName: "testPod",
nodeName: "node1",
equivalenceHashForUpdatePredicate: 123,
@ -754,6 +764,7 @@ func TestInvalidateAllCachedPredicateItemOfNode(t *testing.T) {
cache: &upToDateCache{},
},
{
name: "hash predicate 456 not fits host ports",
podName: "testPod",
nodeName: "node2",
equivalenceHashForUpdatePredicate: 456,
@ -764,6 +775,7 @@ func TestInvalidateAllCachedPredicateItemOfNode(t *testing.T) {
cache: &upToDateCache{},
},
{
name: "hash predicate 123 fits host ports",
podName: "testPod",
nodeName: "node3",
equivalenceHashForUpdatePredicate: 123,
@ -794,12 +806,13 @@ func TestInvalidateAllCachedPredicateItemOfNode(t *testing.T) {
}
for _, test := range tests {
// invalidate all cached predicate for node
t.Run(test.name, func(t *testing.T) {
// invalidate cached predicate for all nodes
ecache.InvalidateAllPredicatesOnNode(test.nodeName)
if _, ok := ecache.GetNodeCache(test.nodeName); ok {
t.Errorf("Failed: node: %v should not be found in internal cache", test.nodeName)
break
t.Errorf("Failed: cached item for node: %v should be invalidated", test.nodeName)
}
})
}
}

View File

@ -492,6 +492,7 @@ func TestGenericSchedulerWithExtenders(t *testing.T) {
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
extenders := []algorithm.SchedulerExtender{}
for ii := range test.extenders {
extenders = append(extenders, &test.extenders[ii])
@ -518,17 +519,18 @@ func TestGenericSchedulerWithExtenders(t *testing.T) {
machine, err := scheduler.Schedule(podIgnored, schedulertesting.FakeNodeLister(makeNodeList(test.nodes)))
if test.expectsErr {
if err == nil {
t.Errorf("Unexpected non-error for %s, machine %s", test.name, machine)
t.Errorf("Unexpected non-error, machine %s", machine)
}
} else {
if err != nil {
t.Errorf("Unexpected error: %v", err)
continue
return
}
if test.expectedHost != machine {
t.Errorf("Failed : %s, Expected: %s, Saw: %s", test.name, test.expectedHost, machine)
t.Errorf("Expected: %s, Saw: %s", test.expectedHost, machine)
}
}
})
}
}

View File

@ -123,11 +123,13 @@ func makeNodeList(nodeNames []string) []*v1.Node {
func TestSelectHost(t *testing.T) {
scheduler := genericScheduler{}
tests := []struct {
name string
list schedulerapi.HostPriorityList
possibleHosts sets.String
expectsErr bool
}{
{
name: "unique properly ordered scores",
list: []schedulerapi.HostPriority{
{Host: "machine1.1", Score: 1},
{Host: "machine2.1", Score: 2},
@ -135,8 +137,8 @@ func TestSelectHost(t *testing.T) {
possibleHosts: sets.NewString("machine2.1"),
expectsErr: false,
},
// equal scores
{
name: "equal scores",
list: []schedulerapi.HostPriority{
{Host: "machine1.1", Score: 1},
{Host: "machine1.2", Score: 2},
@ -146,8 +148,8 @@ func TestSelectHost(t *testing.T) {
possibleHosts: sets.NewString("machine1.2", "machine1.3", "machine2.1"),
expectsErr: false,
},
// out of order scores
{
name: "out of order scores",
list: []schedulerapi.HostPriority{
{Host: "machine1.1", Score: 3},
{Host: "machine1.2", Score: 3},
@ -158,8 +160,8 @@ func TestSelectHost(t *testing.T) {
possibleHosts: sets.NewString("machine1.1", "machine1.2", "machine1.3"),
expectsErr: false,
},
// empty priorityList
{
name: "empty priority list",
list: []schedulerapi.HostPriority{},
possibleHosts: sets.NewString(),
expectsErr: true,
@ -167,6 +169,7 @@ func TestSelectHost(t *testing.T) {
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
// increase the randomness
for i := 0; i < 10; i++ {
got, err := scheduler.selectHost(test.list)
@ -183,6 +186,7 @@ func TestSelectHost(t *testing.T) {
}
}
}
})
}
}
@ -398,6 +402,7 @@ func TestGenericScheduler(t *testing.T) {
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
cache := schedulercache.New(time.Duration(0), wait.NeverStop)
for _, pod := range test.pods {
cache.AddPod(pod)
@ -426,11 +431,12 @@ func TestGenericScheduler(t *testing.T) {
machine, err := scheduler.Schedule(test.pod, schedulertesting.FakeNodeLister(makeNodeList(test.nodes)))
if !reflect.DeepEqual(err, test.wErr) {
t.Errorf("Failed : %s, Unexpected error: %v, expected: %v", test.name, err, test.wErr)
t.Errorf("Unexpected error: %v, expected: %v", err, test.wErr)
}
if test.expectedHosts != nil && !test.expectedHosts.Has(machine) {
t.Errorf("Failed : %s, Expected: %s, got: %s", test.name, test.expectedHosts, machine)
t.Errorf("Expected: %s, got: %s", test.expectedHosts, machine)
}
})
}
}
@ -473,13 +479,15 @@ func TestFindFitAllError(t *testing.T) {
}
for _, node := range nodes {
t.Run(node.Name, func(t *testing.T) {
failures, found := predicateMap[node.Name]
if !found {
t.Errorf("failed to find node: %s in %v", node.Name, predicateMap)
t.Errorf("failed to find node in %v", predicateMap)
}
if len(failures) != 1 || failures[0] != algorithmpredicates.ErrFakePredicate {
t.Errorf("unexpected failures: %v", failures)
}
})
}
}
@ -503,13 +511,15 @@ func TestFindFitSomeError(t *testing.T) {
if node.Name == pod.Name {
continue
}
t.Run(node.Name, func(t *testing.T) {
failures, found := predicateMap[node.Name]
if !found {
t.Errorf("failed to find node: %s in %v", node.Name, predicateMap)
t.Errorf("failed to find node in %v", predicateMap)
}
if len(failures) != 1 || failures[0] != algorithmpredicates.ErrFakePredicate {
t.Errorf("unexpected failures: %v", failures)
}
})
}
}
@ -604,7 +614,7 @@ func TestZeroRequest(t *testing.T) {
pod *v1.Pod
pods []*v1.Pod
nodes []*v1.Node
test string
name string
}{
// The point of these next two tests is to show you get the same priority for a zero-request pod
// as for a pod with the defaults requests, both when the zero-request pod is already on the machine
@ -612,7 +622,7 @@ func TestZeroRequest(t *testing.T) {
{
pod: &v1.Pod{Spec: noResources},
nodes: []*v1.Node{makeNode("machine1", 1000, priorityutil.DefaultMemoryRequest*10), makeNode("machine2", 1000, priorityutil.DefaultMemoryRequest*10)},
test: "test priority of zero-request pod with machine with zero-request pod",
name: "test priority of zero-request pod with machine with zero-request pod",
pods: []*v1.Pod{
{Spec: large1}, {Spec: noResources1},
{Spec: large2}, {Spec: small2},
@ -621,7 +631,7 @@ func TestZeroRequest(t *testing.T) {
{
pod: &v1.Pod{Spec: small},
nodes: []*v1.Node{makeNode("machine1", 1000, priorityutil.DefaultMemoryRequest*10), makeNode("machine2", 1000, priorityutil.DefaultMemoryRequest*10)},
test: "test priority of nonzero-request pod with machine with zero-request pod",
name: "test priority of nonzero-request pod with machine with zero-request pod",
pods: []*v1.Pod{
{Spec: large1}, {Spec: noResources1},
{Spec: large2}, {Spec: small2},
@ -631,7 +641,7 @@ func TestZeroRequest(t *testing.T) {
{
pod: &v1.Pod{Spec: large},
nodes: []*v1.Node{makeNode("machine1", 1000, priorityutil.DefaultMemoryRequest*10), makeNode("machine2", 1000, priorityutil.DefaultMemoryRequest*10)},
test: "test priority of larger pod with machine with zero-request pod",
name: "test priority of larger pod with machine with zero-request pod",
pods: []*v1.Pod{
{Spec: large1}, {Spec: noResources1},
{Spec: large2}, {Spec: small2},
@ -641,6 +651,7 @@ func TestZeroRequest(t *testing.T) {
const expectedPriority int = 25
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
// This should match the configuration in defaultPriorities() in
// pkg/scheduler/algorithmprovider/defaults/defaults.go if you want
// to test what's actually in production.
@ -672,16 +683,17 @@ func TestZeroRequest(t *testing.T) {
t.Errorf("unexpected error: %v", err)
}
for _, hp := range list {
if test.test == "test priority of larger pod with machine with zero-request pod" {
if test.name == "test priority of larger pod with machine with zero-request pod" {
if hp.Score == expectedPriority {
t.Errorf("%s: expected non-%d for all priorities, got list %#v", test.test, expectedPriority, list)
t.Errorf("expected non-%d for all priorities, got list %#v", expectedPriority, list)
}
} else {
if hp.Score != expectedPriority {
t.Errorf("%s: expected %d for all priorities, got list %#v", test.test, expectedPriority, list)
t.Errorf("expected %d for all priorities, got list %#v", expectedPriority, list)
}
}
}
})
}
}
@ -697,30 +709,30 @@ func printNodeToVictims(nodeToVictims map[*v1.Node]*schedulerapi.Victims) string
return output
}
func checkPreemptionVictims(testName string, expected map[string]map[string]bool, nodeToPods map[*v1.Node]*schedulerapi.Victims) error {
func checkPreemptionVictims(expected map[string]map[string]bool, nodeToPods map[*v1.Node]*schedulerapi.Victims) error {
if len(expected) == len(nodeToPods) {
for k, victims := range nodeToPods {
if expPods, ok := expected[k.Name]; ok {
if len(victims.Pods) != len(expPods) {
return fmt.Errorf("test [%v]: unexpected number of pods. expected: %v, got: %v", testName, expected, printNodeToVictims(nodeToPods))
return fmt.Errorf("unexpected number of pods. expected: %v, got: %v", expected, printNodeToVictims(nodeToPods))
}
prevPriority := int32(math.MaxInt32)
for _, p := range victims.Pods {
// Check that pods are sorted by their priority.
if *p.Spec.Priority > prevPriority {
return fmt.Errorf("test [%v]: pod %v of node %v was not sorted by priority", testName, p.Name, k)
return fmt.Errorf("pod %v of node %v was not sorted by priority", p.Name, k)
}
prevPriority = *p.Spec.Priority
if _, ok := expPods[p.Name]; !ok {
return fmt.Errorf("test [%v]: pod %v was not expected. Expected: %v", testName, p.Name, expPods)
return fmt.Errorf("pod %v was not expected. Expected: %v", p.Name, expPods)
}
}
} else {
return fmt.Errorf("test [%v]: unexpected machines. expected: %v, got: %v", testName, expected, printNodeToVictims(nodeToPods))
return fmt.Errorf("unexpected machines. expected: %v, got: %v", expected, printNodeToVictims(nodeToPods))
}
}
} else {
return fmt.Errorf("test [%v]: unexpected number of machines. expected: %v, got: %v", testName, expected, printNodeToVictims(nodeToPods))
return fmt.Errorf("unexpected number of machines. expected: %v, got: %v", expected, printNodeToVictims(nodeToPods))
}
return nil
}
@ -906,6 +918,7 @@ func TestSelectNodesForPreemption(t *testing.T) {
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
nodes := []*v1.Node{}
for _, n := range test.nodes {
node := makeNode(n, 1000*5, priorityutil.DefaultMemoryRequest*5)
@ -920,9 +933,10 @@ func TestSelectNodesForPreemption(t *testing.T) {
if err != nil {
t.Error(err)
}
if err := checkPreemptionVictims(test.name, test.expected, nodeToPods); err != nil {
if err := checkPreemptionVictims(test.expected, nodeToPods); err != nil {
t.Error(err)
}
})
}
}
@ -1069,6 +1083,7 @@ func TestPickOneNodeForPreemption(t *testing.T) {
},
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
nodes := []*v1.Node{}
for _, n := range test.nodes {
nodes = append(nodes, makeNode(n, priorityutil.DefaultMilliCPURequest*5, priorityutil.DefaultMemoryRequest*5))
@ -1084,8 +1099,9 @@ func TestPickOneNodeForPreemption(t *testing.T) {
}
}
if !found {
t.Errorf("test [%v]: unexpected node: %v", test.name, node)
t.Errorf("unexpected node: %v", node)
}
})
}
}
@ -1159,15 +1175,17 @@ func TestNodesWherePreemptionMightHelp(t *testing.T) {
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
nodes := nodesWherePreemptionMightHelp(makeNodeList(nodeNames), test.failedPredMap)
if len(test.expected) != len(nodes) {
t.Errorf("test [%v]:number of nodes is not the same as expected. exptectd: %d, got: %d. Nodes: %v", test.name, len(test.expected), len(nodes), nodes)
t.Errorf("number of nodes is not the same as expected. exptectd: %d, got: %d. Nodes: %v", len(test.expected), len(nodes), nodes)
}
for _, node := range nodes {
if _, found := test.expected[node.Name]; !found {
t.Errorf("test [%v]: node %v is not expected.", test.name, node.Name)
t.Errorf("node %v is not expected.", node.Name)
}
}
})
}
}
@ -1290,6 +1308,7 @@ func TestPreempt(t *testing.T) {
}
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
stop := make(chan struct{})
cache := schedulercache.New(time.Duration(0), stop)
for _, pod := range test.pods {
@ -1327,13 +1346,13 @@ func TestPreempt(t *testing.T) {
// Call Preempt and check the expected results.
node, victims, _, err := scheduler.Preempt(test.pod, schedulertesting.FakeNodeLister(makeNodeList(nodeNames)), error(&FitError{Pod: test.pod, FailedPredicates: failedPredMap}))
if err != nil {
t.Errorf("test [%v]: unexpected error in preemption: %v", test.name, err)
t.Errorf("unexpected error in preemption: %v", err)
}
if (node != nil && node.Name != test.expectedNode) || (node == nil && len(test.expectedNode) != 0) {
t.Errorf("test [%v]: expected node: %v, got: %v", test.name, test.expectedNode, node)
t.Errorf("expected node: %v, got: %v", test.expectedNode, node)
}
if len(victims) != len(test.expectedPods) {
t.Errorf("test [%v]: expected %v pods, got %v.", test.name, len(test.expectedPods), len(victims))
t.Errorf("expected %v pods, got %v.", len(test.expectedPods), len(victims))
}
for _, victim := range victims {
found := false
@ -1344,7 +1363,7 @@ func TestPreempt(t *testing.T) {
}
}
if !found {
t.Errorf("test [%v]: pod %v is not expected to be a victim.", test.name, victim.Name)
t.Errorf("pod %v is not expected to be a victim.", victim.Name)
}
// Mark the victims for deletion and record the preemptor's nominated node name.
now := metav1.Now()
@ -1354,12 +1373,13 @@ func TestPreempt(t *testing.T) {
// Call preempt again and make sure it doesn't preempt any more pods.
node, victims, _, err = scheduler.Preempt(test.pod, schedulertesting.FakeNodeLister(makeNodeList(nodeNames)), error(&FitError{Pod: test.pod, FailedPredicates: failedPredMap}))
if err != nil {
t.Errorf("test [%v]: unexpected error in preemption: %v", test.name, err)
t.Errorf("unexpected error in preemption: %v", err)
}
if node != nil && len(victims) > 0 {
t.Errorf("test [%v]: didn't expect any more preemption. Node %v is selected for preemption.", test.name, node)
t.Errorf("didn't expect any more preemption. Node %v is selected for preemption.", node)
}
close(stop)
})
}
}

View File

@ -375,6 +375,7 @@ func TestUnschedulablePodsMap(t *testing.T) {
updatedPods[3] = pods[3].DeepCopy()
tests := []struct {
name string
podsToAdd []*v1.Pod
expectedMapAfterAdd map[string]*v1.Pod
podsToUpdate []*v1.Pod
@ -383,6 +384,7 @@ func TestUnschedulablePodsMap(t *testing.T) {
expectedMapAfterDelete map[string]*v1.Pod
}{
{
name: "create, update, delete subset of pods",
podsToAdd: []*v1.Pod{pods[0], pods[1], pods[2], pods[3]},
expectedMapAfterAdd: map[string]*v1.Pod{
util.GetPodFullName(pods[0]): pods[0],
@ -404,6 +406,7 @@ func TestUnschedulablePodsMap(t *testing.T) {
},
},
{
name: "create, update, delete all",
podsToAdd: []*v1.Pod{pods[0], pods[3]},
expectedMapAfterAdd: map[string]*v1.Pod{
util.GetPodFullName(pods[0]): pods[0],
@ -418,6 +421,7 @@ func TestUnschedulablePodsMap(t *testing.T) {
expectedMapAfterDelete: map[string]*v1.Pod{},
},
{
name: "delete non-existing and existing pods",
podsToAdd: []*v1.Pod{pods[1], pods[2]},
expectedMapAfterAdd: map[string]*v1.Pod{
util.GetPodFullName(pods[1]): pods[1],
@ -435,14 +439,15 @@ func TestUnschedulablePodsMap(t *testing.T) {
},
}
for i, test := range tests {
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
upm := newUnschedulablePodsMap()
for _, p := range test.podsToAdd {
upm.addOrUpdate(p)
}
if !reflect.DeepEqual(upm.pods, test.expectedMapAfterAdd) {
t.Errorf("#%d: Unexpected map after adding pods. Expected: %v, got: %v",
i, test.expectedMapAfterAdd, upm.pods)
t.Errorf("Unexpected map after adding pods. Expected: %v, got: %v",
test.expectedMapAfterAdd, upm.pods)
}
if len(test.podsToUpdate) > 0 {
@ -450,20 +455,21 @@ func TestUnschedulablePodsMap(t *testing.T) {
upm.addOrUpdate(p)
}
if !reflect.DeepEqual(upm.pods, test.expectedMapAfterUpdate) {
t.Errorf("#%d: Unexpected map after updating pods. Expected: %v, got: %v",
i, test.expectedMapAfterUpdate, upm.pods)
t.Errorf("Unexpected map after updating pods. Expected: %v, got: %v",
test.expectedMapAfterUpdate, upm.pods)
}
}
for _, p := range test.podsToDelete {
upm.delete(p)
}
if !reflect.DeepEqual(upm.pods, test.expectedMapAfterDelete) {
t.Errorf("#%d: Unexpected map after deleting pods. Expected: %v, got: %v",
i, test.expectedMapAfterDelete, upm.pods)
t.Errorf("Unexpected map after deleting pods. Expected: %v, got: %v",
test.expectedMapAfterDelete, upm.pods)
}
upm.clear()
if len(upm.pods) != 0 {
t.Errorf("Expected the map to be empty, but has %v elements.", len(upm.pods))
}
})
}
}