From 7d45e125234687f2be9e3f8977b721f20a6f4133 Mon Sep 17 00:00:00 2001 From: wccsama Date: Wed, 29 Apr 2020 11:12:35 +0800 Subject: [PATCH 1/2] use sub test in gc_controller_test.go --- pkg/controller/podgc/gc_controller_test.go | 118 +++++++++++---------- 1 file changed, 64 insertions(+), 54 deletions(-) diff --git a/pkg/controller/podgc/gc_controller_test.go b/pkg/controller/podgc/gc_controller_test.go index 0036e220f79..9f1d21997c9 100644 --- a/pkg/controller/podgc/gc_controller_test.go +++ b/pkg/controller/podgc/gc_controller_test.go @@ -18,6 +18,7 @@ package podgc import ( "context" + "fmt" "sync" "testing" "time" @@ -72,6 +73,7 @@ func TestGCTerminated(t *testing.T) { deletedPodNames sets.String }{ { + // case 1 pods: []nameToPhase{ {name: "a", phase: v1.PodFailed}, {name: "b", phase: v1.PodSucceeded}, @@ -81,6 +83,7 @@ func TestGCTerminated(t *testing.T) { deletedPodNames: sets.NewString(), }, { + // case 2 pods: []nameToPhase{ {name: "a", phase: v1.PodFailed}, {name: "b", phase: v1.PodSucceeded}, @@ -90,6 +93,7 @@ func TestGCTerminated(t *testing.T) { deletedPodNames: sets.NewString("a", "b"), }, { + // case 3 pods: []nameToPhase{ {name: "a", phase: v1.PodRunning}, {name: "b", phase: v1.PodSucceeded}, @@ -99,6 +103,7 @@ func TestGCTerminated(t *testing.T) { deletedPodNames: sets.NewString("b"), }, { + // case 4 pods: []nameToPhase{ {name: "a", phase: v1.PodFailed}, {name: "b", phase: v1.PodSucceeded}, @@ -107,6 +112,7 @@ func TestGCTerminated(t *testing.T) { deletedPodNames: sets.NewString("a"), }, { + // case 5 pods: []nameToPhase{ {name: "a", phase: v1.PodFailed}, {name: "b", phase: v1.PodSucceeded}, @@ -117,33 +123,35 @@ func TestGCTerminated(t *testing.T) { } for i, test := range testCases { - client := fake.NewSimpleClientset(&v1.NodeList{Items: []v1.Node{*testutil.NewNode("node")}}) - gcc, podInformer, _ := NewFromClient(client, test.threshold) - deletedPodNames := make([]string, 0) - var lock sync.Mutex - gcc.deletePod = func(_, name string) error { - lock.Lock() - defer lock.Unlock() - deletedPodNames = append(deletedPodNames, name) - return nil - } + t.Run(fmt.Sprintf("case: %v", i), func(t *testing.T) { + client := fake.NewSimpleClientset(&v1.NodeList{Items: []v1.Node{*testutil.NewNode("node")}}) + gcc, podInformer, _ := NewFromClient(client, test.threshold) + deletedPodNames := make([]string, 0) + var lock sync.Mutex + gcc.deletePod = func(_, name string) error { + lock.Lock() + defer lock.Unlock() + deletedPodNames = append(deletedPodNames, name) + return nil + } - creationTime := time.Unix(0, 0) - for _, pod := range test.pods { - creationTime = creationTime.Add(1 * time.Hour) - podInformer.Informer().GetStore().Add(&v1.Pod{ - ObjectMeta: metav1.ObjectMeta{Name: pod.name, CreationTimestamp: metav1.Time{Time: creationTime}}, - Status: v1.PodStatus{Phase: pod.phase}, - Spec: v1.PodSpec{NodeName: "node"}, - }) - } + creationTime := time.Unix(0, 0) + for _, pod := range test.pods { + creationTime = creationTime.Add(1 * time.Hour) + podInformer.Informer().GetStore().Add(&v1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: pod.name, CreationTimestamp: metav1.Time{Time: creationTime}}, + Status: v1.PodStatus{Phase: pod.phase}, + Spec: v1.PodSpec{NodeName: "node"}, + }) + } - gcc.gc() + gcc.gc() - if pass := compareStringSetToList(test.deletedPodNames, deletedPodNames); !pass { - t.Errorf("[%v]pod's deleted expected and actual did not match.\n\texpected: %v\n\tactual: %v", - i, test.deletedPodNames.List(), deletedPodNames) - } + if pass := compareStringSetToList(test.deletedPodNames, deletedPodNames); !pass { + t.Errorf("[%v]pod's deleted expected and actual did not match.\n\texpected: %v\n\tactual: %v", + i, test.deletedPodNames.List(), deletedPodNames) + } + }) } } @@ -403,38 +411,40 @@ func TestGCUnscheduledTerminating(t *testing.T) { } for i, test := range testCases { - client := fake.NewSimpleClientset() - gcc, podInformer, _ := NewFromClient(client, -1) - deletedPodNames := make([]string, 0) - var lock sync.Mutex - gcc.deletePod = func(_, name string) error { - lock.Lock() - defer lock.Unlock() - deletedPodNames = append(deletedPodNames, name) - return nil - } + t.Run(test.name, func(t *testing.T) { + client := fake.NewSimpleClientset() + gcc, podInformer, _ := NewFromClient(client, -1) + deletedPodNames := make([]string, 0) + var lock sync.Mutex + gcc.deletePod = func(_, name string) error { + lock.Lock() + defer lock.Unlock() + deletedPodNames = append(deletedPodNames, name) + return nil + } - creationTime := time.Unix(0, 0) - for _, pod := range test.pods { - creationTime = creationTime.Add(1 * time.Hour) - podInformer.Informer().GetStore().Add(&v1.Pod{ - ObjectMeta: metav1.ObjectMeta{Name: pod.name, CreationTimestamp: metav1.Time{Time: creationTime}, - DeletionTimestamp: pod.deletionTimeStamp}, - Status: v1.PodStatus{Phase: pod.phase}, - Spec: v1.PodSpec{NodeName: pod.nodeName}, - }) - } + creationTime := time.Unix(0, 0) + for _, pod := range test.pods { + creationTime = creationTime.Add(1 * time.Hour) + podInformer.Informer().GetStore().Add(&v1.Pod{ + ObjectMeta: metav1.ObjectMeta{Name: pod.name, CreationTimestamp: metav1.Time{Time: creationTime}, + DeletionTimestamp: pod.deletionTimeStamp}, + Status: v1.PodStatus{Phase: pod.phase}, + Spec: v1.PodSpec{NodeName: pod.nodeName}, + }) + } - pods, err := podInformer.Lister().List(labels.Everything()) - if err != nil { - t.Errorf("Error while listing all Pods: %v", err) - return - } - gcc.gcUnscheduledTerminating(pods) + pods, err := podInformer.Lister().List(labels.Everything()) + if err != nil { + t.Errorf("Error while listing all Pods: %v", err) + return + } + gcc.gcUnscheduledTerminating(pods) - if pass := compareStringSetToList(test.deletedPodNames, deletedPodNames); !pass { - t.Errorf("[%v]pod's deleted expected and actual did not match.\n\texpected: %v\n\tactual: %v, test: %v", - i, test.deletedPodNames.List(), deletedPodNames, test.name) - } + if pass := compareStringSetToList(test.deletedPodNames, deletedPodNames); !pass { + t.Errorf("[%v]pod's deleted expected and actual did not match.\n\texpected: %v\n\tactual: %v, test: %v", + i, test.deletedPodNames.List(), deletedPodNames, test.name) + } + }) } } From c776e80109983e51da1dbbaea29766aa0c17ad0b Mon Sep 17 00:00:00 2001 From: wccsama Date: Thu, 30 Apr 2020 13:55:50 +0800 Subject: [PATCH 2/2] using case name instead --- pkg/controller/podgc/gc_controller_test.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/pkg/controller/podgc/gc_controller_test.go b/pkg/controller/podgc/gc_controller_test.go index 9f1d21997c9..b780b00b8ee 100644 --- a/pkg/controller/podgc/gc_controller_test.go +++ b/pkg/controller/podgc/gc_controller_test.go @@ -18,7 +18,6 @@ package podgc import ( "context" - "fmt" "sync" "testing" "time" @@ -68,12 +67,13 @@ func TestGCTerminated(t *testing.T) { } testCases := []struct { + name string pods []nameToPhase threshold int deletedPodNames sets.String }{ { - // case 1 + name: "threshold = 0, disables terminated pod deletion", pods: []nameToPhase{ {name: "a", phase: v1.PodFailed}, {name: "b", phase: v1.PodSucceeded}, @@ -83,7 +83,7 @@ func TestGCTerminated(t *testing.T) { deletedPodNames: sets.NewString(), }, { - // case 2 + name: "threshold = 1, delete pod a which is PodFailed and pod b which is PodSucceeded", pods: []nameToPhase{ {name: "a", phase: v1.PodFailed}, {name: "b", phase: v1.PodSucceeded}, @@ -93,7 +93,7 @@ func TestGCTerminated(t *testing.T) { deletedPodNames: sets.NewString("a", "b"), }, { - // case 3 + name: "threshold = 1, delete pod b which is PodSucceeded", pods: []nameToPhase{ {name: "a", phase: v1.PodRunning}, {name: "b", phase: v1.PodSucceeded}, @@ -103,7 +103,7 @@ func TestGCTerminated(t *testing.T) { deletedPodNames: sets.NewString("b"), }, { - // case 4 + name: "threshold = 1, delete pod a which is PodFailed", pods: []nameToPhase{ {name: "a", phase: v1.PodFailed}, {name: "b", phase: v1.PodSucceeded}, @@ -112,7 +112,7 @@ func TestGCTerminated(t *testing.T) { deletedPodNames: sets.NewString("a"), }, { - // case 5 + name: "threshold = 5, don't delete pod", pods: []nameToPhase{ {name: "a", phase: v1.PodFailed}, {name: "b", phase: v1.PodSucceeded}, @@ -123,7 +123,7 @@ func TestGCTerminated(t *testing.T) { } for i, test := range testCases { - t.Run(fmt.Sprintf("case: %v", i), func(t *testing.T) { + t.Run(test.name, func(t *testing.T) { client := fake.NewSimpleClientset(&v1.NodeList{Items: []v1.Node{*testutil.NewNode("node")}}) gcc, podInformer, _ := NewFromClient(client, test.threshold) deletedPodNames := make([]string, 0)