From 873607343dc0e5c07b5b350dee26e9bced1ca250 Mon Sep 17 00:00:00 2001 From: Vince Prignano Date: Tue, 22 Jun 2021 08:43:11 -0700 Subject: [PATCH] :seedling: Remove async ginkgo tests Ginkgo has deprecated async tests with channels, and given that we're not really using those properly or consistently, remove them from controller runtime and simplify our tests. Signed-off-by: Vince Prignano --- pkg/builder/builder_suite_test.go | 4 +- pkg/builder/controller_test.go | 10 +- pkg/cache/cache_suite_test.go | 4 +- pkg/cache/cache_test.go | 12 +- pkg/certwatcher/certwatcher_suite_test.go | 7 +- pkg/client/apiutil/apiutil_suite_test.go | 4 +- pkg/client/apiutil/dynamicrestmapper_test.go | 1 - pkg/client/client_suite_test.go | 4 +- pkg/client/client_test.go | 322 +++++------------- pkg/client/config/config_suite_test.go | 4 +- pkg/client/fake/client_suite_test.go | 3 +- pkg/client/fake/client_test.go | 6 +- pkg/client/watch_test.go | 19 +- pkg/cluster/cluster_suite_test.go | 4 +- pkg/cluster/cluster_test.go | 24 +- pkg/controller/controller_integration_test.go | 4 +- pkg/controller/controller_suite_test.go | 4 +- pkg/controller/controller_test.go | 16 +- .../controllerutil_suite_test.go | 8 +- pkg/envtest/envtest_suite_test.go | 8 +- pkg/envtest/envtest_test.go | 64 +--- pkg/envtest/webhook_test.go | 3 +- pkg/handler/eventhandler_test.go | 53 +-- .../controller/controller_suite_test.go | 4 +- pkg/internal/controller/controller_test.go | 57 +--- .../recorder/recorder_integration_test.go | 4 +- pkg/internal/recorder/recorder_suite_test.go | 4 +- pkg/manager/manager_suite_test.go | 4 +- pkg/manager/manager_test.go | 114 ++----- pkg/predicate/predicate_test.go | 12 +- pkg/source/internal/internal_test.go | 48 +-- pkg/source/source_integration_test.go | 27 +- pkg/source/source_suite_test.go | 8 +- pkg/source/source_test.go | 56 +-- pkg/webhook/admission/admission_suite_test.go | 4 +- .../authentication_suite_test.go | 4 +- .../conversion/conversion_suite_test.go | 4 +- pkg/webhook/webhook_integration_test.go | 12 +- pkg/webhook/webhook_suite_test.go | 3 +- 39 files changed, 262 insertions(+), 691 deletions(-) diff --git a/pkg/builder/builder_suite_test.go b/pkg/builder/builder_suite_test.go index 0dc260d4bf..26c2366889 100644 --- a/pkg/builder/builder_suite_test.go +++ b/pkg/builder/builder_suite_test.go @@ -45,7 +45,7 @@ func TestBuilder(t *testing.T) { var testenv *envtest.Environment var cfg *rest.Config -var _ = BeforeSuite(func(done Done) { +var _ = BeforeSuite(func() { logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) testenv = &envtest.Environment{} @@ -63,8 +63,6 @@ var _ = BeforeSuite(func(done Done) { webhook.DefaultPort, _, err = addr.Suggest("") Expect(err).NotTo(HaveOccurred()) - - close(done) }, 60) var _ = AfterSuite(func() { diff --git a/pkg/builder/controller_test.go b/pkg/builder/controller_test.go index 683d87925b..793513b203 100644 --- a/pkg/builder/controller_test.go +++ b/pkg/builder/controller_test.go @@ -297,7 +297,7 @@ var _ = Describe("application", func() { }) Describe("Start with ControllerManagedBy", func() { - It("should Reconcile Owns objects", func(done Done) { + It("should Reconcile Owns objects", func() { m, err := manager.New(cfg, manager.Options{}) Expect(err).NotTo(HaveOccurred()) @@ -308,10 +308,9 @@ var _ = Describe("application", func() { ctx, cancel := context.WithCancel(context.Background()) defer cancel() doReconcileTest(ctx, "3", bldr, m, false) - close(done) }, 10) - It("should Reconcile Watches objects", func(done Done) { + It("should Reconcile Watches objects", func() { m, err := manager.New(cfg, manager.Options{}) Expect(err).NotTo(HaveOccurred()) @@ -324,12 +323,11 @@ var _ = Describe("application", func() { ctx, cancel := context.WithCancel(context.Background()) defer cancel() doReconcileTest(ctx, "4", bldr, m, true) - close(done) }, 10) }) Describe("Set custom predicates", func() { - It("should execute registered predicates only for assigned kind", func(done Done) { + It("should execute registered predicates only for assigned kind", func() { m, err := manager.New(cfg, manager.Options{}) Expect(err).NotTo(HaveOccurred()) @@ -385,8 +383,6 @@ var _ = Describe("application", func() { Expect(deployPrctExecuted).To(BeTrue(), "Deploy predicated should be called at least once") Expect(replicaSetPrctExecuted).To(BeTrue(), "ReplicaSet predicated should be called at least once") Expect(allPrctExecuted).To(BeNumerically(">=", 2), "Global Predicated should be called at least twice") - - close(done) }) }) diff --git a/pkg/cache/cache_suite_test.go b/pkg/cache/cache_suite_test.go index 900e87e56e..2517777d39 100644 --- a/pkg/cache/cache_suite_test.go +++ b/pkg/cache/cache_suite_test.go @@ -39,7 +39,7 @@ var testenv *envtest.Environment var cfg *rest.Config var clientset *kubernetes.Clientset -var _ = BeforeSuite(func(done Done) { +var _ = BeforeSuite(func() { logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) testenv = &envtest.Environment{} @@ -50,8 +50,6 @@ var _ = BeforeSuite(func(done Done) { clientset, err = kubernetes.NewForConfig(cfg) Expect(err).NotTo(HaveOccurred()) - - close(done) }, 60) var _ = AfterSuite(func() { diff --git a/pkg/cache/cache_test.go b/pkg/cache/cache_test.go index 35cfd149de..9bf49ec40b 100644 --- a/pkg/cache/cache_test.go +++ b/pkg/cache/cache_test.go @@ -886,7 +886,7 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca }) Describe("as an Informer", func() { Context("with structured objects", func() { - It("should be able to get informer for the object", func(done Done) { + It("should be able to get informer for the object", func() { By("getting a shared index informer for a pod") pod := &kcorev1.Pod{ ObjectMeta: kmetav1.ObjectMeta{ @@ -922,9 +922,8 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca By("verifying the object is received on the channel") Eventually(out).Should(Receive(Equal(pod))) - close(done) }) - It("should be able to get an informer by group/version/kind", func(done Done) { + It("should be able to get an informer by group/version/kind", func() { By("getting an shared index informer for gvk = core/v1/pod") gvk := schema.GroupVersionKind{Group: "", Version: "v1", Kind: "Pod"} sii, err := informerCache.GetInformerForKind(context.TODO(), gvk) @@ -961,7 +960,6 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca By("verifying the object is received on the channel") Eventually(out).Should(Receive(Equal(pod))) - close(done) }) It("should be able to index an object field then retrieve objects by that field", func() { By("creating the cache") @@ -1031,7 +1029,7 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca }) }) Context("with unstructured objects", func() { - It("should be able to get informer for the object", func(done Done) { + It("should be able to get informer for the object", func() { By("getting a shared index informer for a pod") pod := &unstructured.Unstructured{ @@ -1073,7 +1071,6 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca By("verifying the object is received on the channel") Eventually(out).Should(Receive(Equal(pod))) - close(done) }, 3) It("should be able to index an object field then retrieve objects by that field", func() { @@ -1146,7 +1143,7 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca }) }) Context("with metadata-only objects", func() { - It("should be able to get informer for the object", func(done Done) { + It("should be able to get informer for the object", func() { By("getting a shared index informer for a pod") pod := &kcorev1.Pod{ @@ -1194,7 +1191,6 @@ func CacheTest(createCacheFunc func(config *rest.Config, opts cache.Options) (ca By("verifying the object's metadata is received on the channel") Eventually(out).Should(Receive(Equal(podMeta))) - close(done) }, 3) It("should be able to index an object field then retrieve objects by that field", func() { diff --git a/pkg/certwatcher/certwatcher_suite_test.go b/pkg/certwatcher/certwatcher_suite_test.go index dfbd40a524..e1e9861ea5 100644 --- a/pkg/certwatcher/certwatcher_suite_test.go +++ b/pkg/certwatcher/certwatcher_suite_test.go @@ -38,15 +38,12 @@ func TestSource(t *testing.T) { RunSpecsWithDefaultAndCustomReporters(t, suiteName, []Reporter{printer.NewlineReporter{}, printer.NewProwReporter(suiteName)}) } -var _ = BeforeSuite(func(done Done) { +var _ = BeforeSuite(func() { logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) - - close(done) }, 60) -var _ = AfterSuite(func(done Done) { +var _ = AfterSuite(func() { for _, file := range []string{certPath, keyPath} { _ = os.Remove(file) } - close(done) }, 60) diff --git a/pkg/client/apiutil/apiutil_suite_test.go b/pkg/client/apiutil/apiutil_suite_test.go index 4a4d7905a5..757bc42fec 100644 --- a/pkg/client/apiutil/apiutil_suite_test.go +++ b/pkg/client/apiutil/apiutil_suite_test.go @@ -36,11 +36,9 @@ func TestSource(t *testing.T) { var cfg *rest.Config -var _ = BeforeSuite(func(done Done) { +var _ = BeforeSuite(func() { logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) // for things that technically need a rest.Config for defaulting, but don't actually use them cfg = &rest.Config{} - - close(done) }, 60) diff --git a/pkg/client/apiutil/dynamicrestmapper_test.go b/pkg/client/apiutil/dynamicrestmapper_test.go index 1e294b793d..8f22800cab 100644 --- a/pkg/client/apiutil/dynamicrestmapper_test.go +++ b/pkg/client/apiutil/dynamicrestmapper_test.go @@ -133,7 +133,6 @@ var _ = Describe("Dynamic REST Mapper", func() { go func() { defer GinkgoRecover() Expect(callWithOther()).To(Succeed()) - close(done) }() Expect(callWithOther()).To(Succeed()) diff --git a/pkg/client/client_suite_test.go b/pkg/client/client_suite_test.go index e0e02575b2..980150c91b 100644 --- a/pkg/client/client_suite_test.go +++ b/pkg/client/client_suite_test.go @@ -40,7 +40,7 @@ var testenv *envtest.Environment var cfg *rest.Config var clientset *kubernetes.Clientset -var _ = BeforeSuite(func(done Done) { +var _ = BeforeSuite(func() { logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) testenv = &envtest.Environment{} @@ -51,8 +51,6 @@ var _ = BeforeSuite(func(done Done) { clientset, err = kubernetes.NewForConfig(cfg) Expect(err).NotTo(HaveOccurred()) - - close(done) }, 60) var _ = AfterSuite(func() { diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index 617fae6f5a..9b1c3d7818 100644 --- a/pkg/client/client_test.go +++ b/pkg/client/client_test.go @@ -136,7 +136,7 @@ var _ = Describe("Client", func() { var ns = "default" ctx := context.TODO() - BeforeEach(func(done Done) { + BeforeEach(func() { atomic.AddUint64(&count, 1) dep = &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{Name: fmt.Sprintf("deployment-name-%v", count), Namespace: ns, Labels: map[string]string{"app": fmt.Sprintf("bar-%v", count)}}, @@ -166,12 +166,10 @@ var _ = Describe("Client", func() { Spec: corev1.NodeSpec{}, } scheme = kscheme.Scheme - - close(done) }, serverSideTimeoutSeconds) var delOptions *metav1.DeleteOptions - AfterEach(func(done Done) { + AfterEach(func() { // Cleanup var zero int64 = 0 policy := metav1.DeletePropagationForeground @@ -185,44 +183,35 @@ var _ = Describe("Client", func() { err = clientset.CoreV1().Nodes().Delete(ctx, node.Name, *delOptions) Expect(err).NotTo(HaveOccurred()) } - close(done) }, serverSideTimeoutSeconds) // TODO(seans): Cast "cl" as "client" struct from "Client" interface. Then validate the // instance values for the "client" struct. Describe("New", func() { - It("should return a new Client", func(done Done) { + It("should return a new Client", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) - - close(done) }) - It("should fail if the config is nil", func(done Done) { + It("should fail if the config is nil", func() { cl, err := client.New(nil, client.Options{}) Expect(err).To(HaveOccurred()) Expect(cl).To(BeNil()) - - close(done) }) // TODO(seans): cast as client struct and inspect Scheme - It("should use the provided Scheme if provided", func(done Done) { + It("should use the provided Scheme if provided", func() { cl, err := client.New(cfg, client.Options{Scheme: scheme}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) - - close(done) }) // TODO(seans): cast as client struct and inspect Scheme - It("should default the Scheme if not provided", func(done Done) { + It("should default the Scheme if not provided", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) - - close(done) }) PIt("should use the provided Mapper if provided", func() { @@ -230,18 +219,16 @@ var _ = Describe("Client", func() { }) // TODO(seans): cast as client struct and inspect Mapper - It("should create a Mapper if not provided", func(done Done) { + It("should create a Mapper if not provided", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) - - close(done) }) }) Describe("Create", func() { Context("with structured objects", func() { - It("should create a new object from a go struct", func(done Done) { + It("should create a new object from a go struct", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -256,11 +243,9 @@ var _ = Describe("Client", func() { By("writing the result back to the go struct") Expect(dep).To(Equal(actual)) - - close(done) }) - It("should create a new object non-namespace object from a go struct", func(done Done) { + It("should create a new object non-namespace object from a go struct", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -275,11 +260,9 @@ var _ = Describe("Client", func() { By("writing the result back to the go struct") Expect(node).To(Equal(actual)) - - close(done) }) - It("should fail if the object already exists", func(done Done) { + It("should fail if the object already exists", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -298,11 +281,9 @@ var _ = Describe("Client", func() { err = cl.Create(context.TODO(), old) Expect(err).To(HaveOccurred()) Expect(apierrors.IsAlreadyExists(err)).To(BeTrue()) - - close(done) }) - It("should fail if the object does not pass server-side validation", func(done Done) { + It("should fail if the object does not pass server-side validation", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -312,8 +293,6 @@ var _ = Describe("Client", func() { Expect(err).To(HaveOccurred()) // TODO(seans): Add test to validate the returned error. Problems currently with // different returned error locally versus travis. - - close(done) }, serverSideTimeoutSeconds) It("should fail if the object cannot be mapped to a GVK", func() { @@ -335,7 +314,7 @@ var _ = Describe("Client", func() { }) Context("with the DryRun option", func() { - It("should not create a new object", func(done Done) { + It("should not create a new object", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -348,14 +327,12 @@ var _ = Describe("Client", func() { Expect(err).To(HaveOccurred()) Expect(apierrors.IsNotFound(err)).To(BeTrue()) Expect(actual).To(Equal(&appsv1.Deployment{})) - - close(done) }) }) }) Context("with unstructured objects", func() { - It("should create a new object from a go struct", func(done Done) { + It("should create a new object from a go struct", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -376,10 +353,9 @@ var _ = Describe("Client", func() { actual, err := clientset.AppsV1().Deployments(ns).Get(ctx, dep.Name, metav1.GetOptions{}) Expect(err).NotTo(HaveOccurred()) Expect(actual).NotTo(BeNil()) - close(done) }) - It("should create a new non-namespace object ", func(done Done) { + It("should create a new non-namespace object ", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -406,11 +382,9 @@ var _ = Describe("Client", func() { By("writing the result back to the go struct") Expect(u).To(Equal(au)) - - close(done) }) - It("should fail if the object already exists", func(done Done) { + It("should fail if the object already exists", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -437,11 +411,9 @@ var _ = Describe("Client", func() { err = cl.Create(context.TODO(), u) Expect(err).To(HaveOccurred()) Expect(apierrors.IsAlreadyExists(err)).To(BeTrue()) - - close(done) }) - It("should fail if the object does not pass server-side validation", func(done Done) { + It("should fail if the object does not pass server-side validation", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -458,8 +430,6 @@ var _ = Describe("Client", func() { Expect(err).To(HaveOccurred()) // TODO(seans): Add test to validate the returned error. Problems currently with // different returned error locally versus travis. - - close(done) }, serverSideTimeoutSeconds) }) @@ -475,7 +445,7 @@ var _ = Describe("Client", func() { }) Context("with the DryRun option", func() { - It("should not create a new object from a go struct", func(done Done) { + It("should not create a new object from a go struct", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -497,15 +467,13 @@ var _ = Describe("Client", func() { Expect(err).To(HaveOccurred()) Expect(apierrors.IsNotFound(err)).To(BeTrue()) Expect(actual).To(Equal(&appsv1.Deployment{})) - - close(done) }) }) }) Describe("Update", func() { Context("with structured objects", func() { - It("should update an existing object from a go struct", func(done Done) { + It("should update an existing object from a go struct", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -524,11 +492,9 @@ var _ = Describe("Client", func() { Expect(err).NotTo(HaveOccurred()) Expect(actual).NotTo(BeNil()) Expect(actual.Annotations["foo"]).To(Equal("bar")) - - close(done) }) - It("should update and preserve type information", func(done Done) { + It("should update and preserve type information", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -544,11 +510,9 @@ var _ = Describe("Client", func() { By("validating updated Deployment has type information") Expect(dep.GroupVersionKind()).To(Equal(depGvk)) - - close(done) }) - It("should update an existing object non-namespace object from a go struct", func(done Done) { + It("should update an existing object non-namespace object from a go struct", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -566,11 +530,9 @@ var _ = Describe("Client", func() { Expect(err).NotTo(HaveOccurred()) Expect(actual).NotTo(BeNil()) Expect(actual.Annotations["foo"]).To(Equal("bar")) - - close(done) }) - It("should fail if the object does not exist", func(done Done) { + It("should fail if the object does not exist", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -578,8 +540,6 @@ var _ = Describe("Client", func() { By("updating non-existent object") err = cl.Update(context.TODO(), dep) Expect(err).To(HaveOccurred()) - - close(done) }) PIt("should fail if the object does not pass server-side validation", func() { @@ -590,7 +550,7 @@ var _ = Describe("Client", func() { }) - It("should fail if the object cannot be mapped to a GVK", func(done Done) { + It("should fail if the object cannot be mapped to a GVK", func() { By("creating client with empty Scheme") emptyScheme := runtime.NewScheme() cl, err := client.New(cfg, client.Options{Scheme: emptyScheme}) @@ -606,8 +566,6 @@ var _ = Describe("Client", func() { err = cl.Update(context.TODO(), dep) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("no kind is registered for the type")) - - close(done) }) PIt("should fail if the GVK cannot be mapped to a Resource", func() { @@ -615,7 +573,7 @@ var _ = Describe("Client", func() { }) }) Context("with unstructured objects", func() { - It("should update an existing object from a go struct", func(done Done) { + It("should update an existing object from a go struct", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -641,11 +599,9 @@ var _ = Describe("Client", func() { Expect(err).NotTo(HaveOccurred()) Expect(actual).NotTo(BeNil()) Expect(actual.Annotations["foo"]).To(Equal("bar")) - - close(done) }) - It("should update and preserve type information", func(done Done) { + It("should update and preserve type information", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -664,11 +620,9 @@ var _ = Describe("Client", func() { By("validating updated Deployment has type information") Expect(u.GroupVersionKind()).To(Equal(depGvk)) - - close(done) }) - It("should update an existing object non-namespace object from a go struct", func(done Done) { + It("should update an existing object non-namespace object from a go struct", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -693,10 +647,8 @@ var _ = Describe("Client", func() { Expect(err).NotTo(HaveOccurred()) Expect(actual).NotTo(BeNil()) Expect(actual.Annotations["foo"]).To(Equal("bar")) - - close(done) }) - It("should fail if the object does not exist", func(done Done) { + It("should fail if the object does not exist", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -707,8 +659,6 @@ var _ = Describe("Client", func() { u.SetGroupVersionKind(depGvk) err = cl.Update(context.TODO(), dep) Expect(err).To(HaveOccurred()) - - close(done) }) }) Context("with metadata objects", func() { @@ -725,7 +675,7 @@ var _ = Describe("Client", func() { Describe("Patch", func() { Context("Metadata Client", func() { - It("should merge patch with options", func(done Done) { + It("should merge patch with options", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -751,14 +701,13 @@ var _ = Describe("Client", func() { By("validating patch options were applied") Expect(testOption.applied).To(Equal(true)) - close(done) }) }) }) Describe("StatusClient", func() { Context("with structured objects", func() { - It("should update status of an existing object", func(done Done) { + It("should update status of an existing object", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -777,11 +726,9 @@ var _ = Describe("Client", func() { Expect(err).NotTo(HaveOccurred()) Expect(actual).NotTo(BeNil()) Expect(actual.Status.Replicas).To(BeEquivalentTo(1)) - - close(done) }) - It("should update status and preserve type information", func(done Done) { + It("should update status and preserve type information", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -798,11 +745,9 @@ var _ = Describe("Client", func() { By("validating updated Deployment has type information") Expect(dep.GroupVersionKind()).To(Equal(depGvk)) - - close(done) }) - It("should patch status and preserve type information", func(done Done) { + It("should patch status and preserve type information", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -820,11 +765,9 @@ var _ = Describe("Client", func() { By("validating updated Deployment has type information") Expect(dep.GroupVersionKind()).To(Equal(depGvk)) - - close(done) }) - It("should not update spec of an existing object", func(done Done) { + It("should not update spec of an existing object", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -846,11 +789,9 @@ var _ = Describe("Client", func() { Expect(actual).NotTo(BeNil()) Expect(actual.Status.Replicas).To(BeEquivalentTo(1)) Expect(*actual.Spec.Replicas).To(BeEquivalentTo(replicaCount)) - - close(done) }) - It("should update an existing object non-namespace object", func(done Done) { + It("should update an existing object non-namespace object", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -868,11 +809,9 @@ var _ = Describe("Client", func() { Expect(err).NotTo(HaveOccurred()) Expect(actual).NotTo(BeNil()) Expect(actual.Status.Phase).To(Equal(corev1.NodeRunning)) - - close(done) }) - It("should fail if the object does not exist", func(done Done) { + It("should fail if the object does not exist", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -880,11 +819,9 @@ var _ = Describe("Client", func() { By("updating status of a non-existent object") err = cl.Status().Update(context.TODO(), dep) Expect(err).To(HaveOccurred()) - - close(done) }) - It("should fail if the object cannot be mapped to a GVK", func(done Done) { + It("should fail if the object cannot be mapped to a GVK", func() { By("creating client with empty Scheme") emptyScheme := runtime.NewScheme() cl, err := client.New(cfg, client.Options{Scheme: emptyScheme}) @@ -900,8 +837,6 @@ var _ = Describe("Client", func() { err = cl.Status().Update(context.TODO(), dep) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("no kind is registered for the type")) - - close(done) }) PIt("should fail if the GVK cannot be mapped to a Resource", func() { @@ -914,7 +849,7 @@ var _ = Describe("Client", func() { }) Context("with unstructured objects", func() { - It("should update status of an existing object", func(done Done) { + It("should update status of an existing object", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -935,11 +870,9 @@ var _ = Describe("Client", func() { Expect(err).NotTo(HaveOccurred()) Expect(actual).NotTo(BeNil()) Expect(actual.Status.Replicas).To(BeEquivalentTo(1)) - - close(done) }) - It("should update status and preserve type information", func(done Done) { + It("should update status and preserve type information", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -957,11 +890,9 @@ var _ = Describe("Client", func() { By("validating updated Deployment has type information") Expect(u.GroupVersionKind()).To(Equal(depGvk)) - - close(done) }) - It("should patch status and preserve type information", func(done Done) { + It("should patch status and preserve type information", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -986,11 +917,9 @@ var _ = Describe("Client", func() { Expect(err).NotTo(HaveOccurred()) Expect(actual).NotTo(BeNil()) Expect(actual.Status.Replicas).To(BeEquivalentTo(1)) - - close(done) }) - It("should not update spec of an existing object", func(done Done) { + It("should not update spec of an existing object", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -1014,11 +943,9 @@ var _ = Describe("Client", func() { Expect(actual).NotTo(BeNil()) Expect(actual.Status.Replicas).To(BeEquivalentTo(1)) Expect(*actual.Spec.Replicas).To(BeEquivalentTo(replicaCount)) - - close(done) }) - It("should update an existing object non-namespace object", func(done Done) { + It("should update an existing object non-namespace object", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -1038,11 +965,9 @@ var _ = Describe("Client", func() { Expect(err).NotTo(HaveOccurred()) Expect(actual).NotTo(BeNil()) Expect(actual.Status.Phase).To(Equal(corev1.NodeRunning)) - - close(done) }) - It("should fail if the object does not exist", func(done Done) { + It("should fail if the object does not exist", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -1052,8 +977,6 @@ var _ = Describe("Client", func() { Expect(scheme.Convert(dep, u, nil)).To(Succeed()) err = cl.Status().Update(context.TODO(), u) Expect(err).To(HaveOccurred()) - - close(done) }) PIt("should fail if the GVK cannot be mapped to a Resource", func() { @@ -1075,7 +998,7 @@ var _ = Describe("Client", func() { Expect(cl.Status().Update(context.TODO(), obj)).NotTo(Succeed()) }) - It("should patch status and preserve type information", func(done Done) { + It("should patch status and preserve type information", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -1099,15 +1022,13 @@ var _ = Describe("Client", func() { Expect(err).NotTo(HaveOccurred()) Expect(actual).NotTo(BeNil()) Expect(actual.Annotations).To(HaveKeyWithValue("some-new-annotation", "some-new-value")) - - close(done) }) }) }) Describe("Delete", func() { Context("with structured objects", func() { - It("should delete an existing object from a go struct", func(done Done) { + It("should delete an existing object from a go struct", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -1124,11 +1045,9 @@ var _ = Describe("Client", func() { By("validating the Deployment no longer exists") _, err = clientset.AppsV1().Deployments(ns).Get(ctx, depName, metav1.GetOptions{}) Expect(err).To(HaveOccurred()) - - close(done) }) - It("should delete an existing object non-namespace object from a go struct", func(done Done) { + It("should delete an existing object non-namespace object from a go struct", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -1145,11 +1064,9 @@ var _ = Describe("Client", func() { By("validating the Node no longer exists") _, err = clientset.CoreV1().Nodes().Get(ctx, nodeName, metav1.GetOptions{}) Expect(err).To(HaveOccurred()) - - close(done) }) - It("should fail if the object does not exist", func(done Done) { + It("should fail if the object does not exist", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -1157,15 +1074,13 @@ var _ = Describe("Client", func() { By("Deleting node before it is ever created") err = cl.Delete(context.TODO(), node) Expect(err).To(HaveOccurred()) - - close(done) }) PIt("should fail if the object doesn't have meta", func() { }) - It("should fail if the object cannot be mapped to a GVK", func(done Done) { + It("should fail if the object cannot be mapped to a GVK", func() { By("creating client with empty Scheme") emptyScheme := runtime.NewScheme() cl, err := client.New(cfg, client.Options{Scheme: emptyScheme}) @@ -1180,15 +1095,13 @@ var _ = Describe("Client", func() { err = cl.Delete(context.TODO(), dep) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("no kind is registered for the type")) - - close(done) }) PIt("should fail if the GVK cannot be mapped to a Resource", func() { }) - It("should delete a collection of objects", func(done Done) { + It("should delete a collection of objects", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -1215,12 +1128,10 @@ var _ = Describe("Client", func() { Expect(err).To(HaveOccurred()) _, err = clientset.AppsV1().Deployments(ns).Get(ctx, dep2Name, metav1.GetOptions{}) Expect(err).To(HaveOccurred()) - - close(done) }) }) Context("with unstructured objects", func() { - It("should delete an existing object from a go struct", func(done Done) { + It("should delete an existing object from a go struct", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -1244,11 +1155,9 @@ var _ = Describe("Client", func() { By("validating the Deployment no longer exists") _, err = clientset.AppsV1().Deployments(ns).Get(ctx, depName, metav1.GetOptions{}) Expect(err).To(HaveOccurred()) - - close(done) }) - It("should delete an existing object non-namespace object from a go struct", func(done Done) { + It("should delete an existing object non-namespace object from a go struct", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -1272,11 +1181,9 @@ var _ = Describe("Client", func() { By("validating the Node no longer exists") _, err = clientset.CoreV1().Nodes().Get(ctx, nodeName, metav1.GetOptions{}) Expect(err).To(HaveOccurred()) - - close(done) }) - It("should fail if the object does not exist", func(done Done) { + It("should fail if the object does not exist", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -1291,11 +1198,9 @@ var _ = Describe("Client", func() { }) err = cl.Delete(context.TODO(), node) Expect(err).To(HaveOccurred()) - - close(done) }) - It("should delete a collection of object", func(done Done) { + It("should delete a collection of object", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -1329,12 +1234,10 @@ var _ = Describe("Client", func() { Expect(err).To(HaveOccurred()) _, err = clientset.AppsV1().Deployments(ns).Get(ctx, dep2Name, metav1.GetOptions{}) Expect(err).To(HaveOccurred()) - - close(done) }) }) Context("with metadata objects", func() { - It("should delete an existing object from a go struct", func(done Done) { + It("should delete an existing object from a go struct", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -1351,11 +1254,9 @@ var _ = Describe("Client", func() { By("validating the Deployment no longer exists") _, err = clientset.AppsV1().Deployments(ns).Get(ctx, dep.Name, metav1.GetOptions{}) Expect(err).To(HaveOccurred()) - - close(done) }) - It("should delete an existing object non-namespace object from a go struct", func(done Done) { + It("should delete an existing object non-namespace object from a go struct", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -1372,11 +1273,9 @@ var _ = Describe("Client", func() { By("validating the Node no longer exists") _, err = clientset.CoreV1().Nodes().Get(ctx, node.Name, metav1.GetOptions{}) Expect(err).To(HaveOccurred()) - - close(done) }) - It("should fail if the object does not exist", func(done Done) { + It("should fail if the object does not exist", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -1385,11 +1284,9 @@ var _ = Describe("Client", func() { metaObj := metaOnlyFromObj(node, scheme) err = cl.Delete(context.TODO(), metaObj) Expect(err).To(HaveOccurred()) - - close(done) }) - It("should delete a collection of object", func(done Done) { + It("should delete a collection of object", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -1417,15 +1314,13 @@ var _ = Describe("Client", func() { Expect(err).To(HaveOccurred()) _, err = clientset.AppsV1().Deployments(ns).Get(ctx, dep2Name, metav1.GetOptions{}) Expect(err).To(HaveOccurred()) - - close(done) }) }) }) Describe("Get", func() { Context("with structured objects", func() { - It("should fetch an existing object for a go struct", func(done Done) { + It("should fetch an existing object for a go struct", func() { By("first creating the Deployment") dep, err := clientset.AppsV1().Deployments(ns).Create(ctx, dep, metav1.CreateOptions{}) Expect(err).NotTo(HaveOccurred()) @@ -1443,11 +1338,9 @@ var _ = Describe("Client", func() { By("validating the fetched deployment equals the created one") Expect(dep).To(Equal(&actual)) - - close(done) }) - It("should fetch an existing non-namespace object for a go struct", func(done Done) { + It("should fetch an existing non-namespace object for a go struct", func() { By("first creating the object") node, err := clientset.CoreV1().Nodes().Create(ctx, node, metav1.CreateOptions{}) Expect(err).NotTo(HaveOccurred()) @@ -1464,11 +1357,9 @@ var _ = Describe("Client", func() { Expect(actual).NotTo(BeNil()) Expect(node).To(Equal(&actual)) - - close(done) }) - It("should fail if the object does not exist", func(done Done) { + It("should fail if the object does not exist", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -1478,8 +1369,6 @@ var _ = Describe("Client", func() { var actual appsv1.Deployment err = cl.Get(context.TODO(), key, &actual) Expect(err).To(HaveOccurred()) - - close(done) }) PIt("should fail if the object doesn't have meta", func() { @@ -1511,7 +1400,7 @@ var _ = Describe("Client", func() { }) Context("with unstructured objects", func() { - It("should fetch an existing object", func(done Done) { + It("should fetch an existing object", func() { By("first creating the Deployment") dep, err := clientset.AppsV1().Deployments(ns).Create(ctx, dep, metav1.CreateOptions{}) Expect(err).NotTo(HaveOccurred()) @@ -1538,11 +1427,9 @@ var _ = Describe("Client", func() { By("validating the fetched Deployment equals the created one") Expect(u).To(Equal(&actual)) - - close(done) }) - It("should fetch an existing non-namespace object", func(done Done) { + It("should fetch an existing non-namespace object", func() { By("first creating the Node") node, err := clientset.CoreV1().Nodes().Create(ctx, node, metav1.CreateOptions{}) Expect(err).NotTo(HaveOccurred()) @@ -1569,11 +1456,9 @@ var _ = Describe("Client", func() { By("validating the fetched Node equals the created one") Expect(u).To(Equal(&actual)) - - close(done) }) - It("should fail if the object does not exist", func(done Done) { + It("should fail if the object does not exist", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -1583,12 +1468,10 @@ var _ = Describe("Client", func() { u := &unstructured.Unstructured{} err = cl.Get(context.TODO(), key, u) Expect(err).To(HaveOccurred()) - - close(done) }) }) Context("with metadata objects", func() { - It("should fetch an existing object for a go struct", func(done Done) { + It("should fetch an existing object for a go struct", func() { By("first creating the Deployment") dep, err := clientset.AppsV1().Deployments(ns).Create(ctx, dep, metav1.CreateOptions{}) Expect(err).NotTo(HaveOccurred()) @@ -1615,11 +1498,9 @@ var _ = Describe("Client", func() { By("validating the fetched deployment equals the created one") Expect(metaOnlyFromObj(dep, scheme)).To(Equal(&actual)) - - close(done) }) - It("should fetch an existing non-namespace object for a go struct", func(done Done) { + It("should fetch an existing non-namespace object for a go struct", func() { By("first creating the object") node, err := clientset.CoreV1().Nodes().Create(ctx, node, metav1.CreateOptions{}) Expect(err).NotTo(HaveOccurred()) @@ -1640,11 +1521,9 @@ var _ = Describe("Client", func() { Expect(actual).NotTo(BeNil()) Expect(metaOnlyFromObj(node, scheme)).To(Equal(&actual)) - - close(done) }) - It("should fail if the object does not exist", func(done Done) { + It("should fail if the object does not exist", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) @@ -1659,8 +1538,6 @@ var _ = Describe("Client", func() { }) err = cl.Get(context.TODO(), key, &actual) Expect(err).To(HaveOccurred()) - - close(done) }) PIt("should fail if the object doesn't have meta", func() { @@ -1675,7 +1552,7 @@ var _ = Describe("Client", func() { Describe("List", func() { Context("with structured objects", func() { - It("should fetch collection of objects", func(done Done) { + It("should fetch collection of objects", func() { By("creating an initial object") dep, err := clientset.AppsV1().Deployments(ns).Create(ctx, dep, metav1.CreateOptions{}) Expect(err).NotTo(HaveOccurred()) @@ -1696,11 +1573,9 @@ var _ = Describe("Client", func() { } } Expect(hasDep).To(BeTrue()) - - close(done) }, serverSideTimeoutSeconds) - It("should fetch unstructured collection of objects", func(done Done) { + It("should fetch unstructured collection of objects", func() { By("create an initial object") _, err := clientset.AppsV1().Deployments(ns).Create(ctx, dep, metav1.CreateOptions{}) Expect(err).NotTo(HaveOccurred()) @@ -1732,10 +1607,9 @@ var _ = Describe("Client", func() { } } Expect(hasDep).To(BeTrue()) - close(done) }, serverSideTimeoutSeconds) - It("should fetch unstructured collection of objects, even if scheme is empty", func(done Done) { + It("should fetch unstructured collection of objects, even if scheme is empty", func() { By("create an initial object") _, err := clientset.AppsV1().Deployments(ns).Create(ctx, dep, metav1.CreateOptions{}) Expect(err).NotTo(HaveOccurred()) @@ -1762,10 +1636,9 @@ var _ = Describe("Client", func() { } } Expect(hasDep).To(BeTrue()) - close(done) }, serverSideTimeoutSeconds) - It("should return an empty list if there are no matching objects", func(done Done) { + It("should return an empty list if there are no matching objects", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) @@ -1775,12 +1648,10 @@ var _ = Describe("Client", func() { By("validating no Deployments are returned") Expect(deps.Items).To(BeEmpty()) - - close(done) }, serverSideTimeoutSeconds) // TODO(seans): get label selector test working - It("should filter results by label selector", func(done Done) { + It("should filter results by label selector", func() { By("creating a Deployment with the app=frontend label") depFrontend := &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ @@ -1838,11 +1709,9 @@ var _ = Describe("Client", func() { deleteDeployment(ctx, depFrontend, ns) deleteDeployment(ctx, depBackend, ns) - - close(done) }, serverSideTimeoutSeconds) - It("should filter results by namespace selector", func(done Done) { + It("should filter results by namespace selector", func() { By("creating a Deployment in test-namespace-1") tns1 := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test-namespace-1"}} _, err := clientset.CoreV1().Namespaces().Create(ctx, tns1, metav1.CreateOptions{}) @@ -1899,11 +1768,9 @@ var _ = Describe("Client", func() { deleteDeployment(ctx, depBackend, "test-namespace-2") deleteNamespace(ctx, tns1) deleteNamespace(ctx, tns2) - - close(done) }, serverSideTimeoutSeconds) - It("should filter results by field selector", func(done Done) { + It("should filter results by field selector", func() { By("creating a Deployment with name deployment-frontend") depFrontend := &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{Name: "deployment-frontend", Namespace: ns}, @@ -1953,11 +1820,9 @@ var _ = Describe("Client", func() { deleteDeployment(ctx, depFrontend, ns) deleteDeployment(ctx, depBackend, ns) - - close(done) }, serverSideTimeoutSeconds) - It("should filter results by namespace selector and label selector", func(done Done) { + It("should filter results by namespace selector and label selector", func() { By("creating a Deployment in test-namespace-3 with the app=frontend label") tns3 := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test-namespace-3"}} _, err := clientset.CoreV1().Namespaces().Create(ctx, tns3, metav1.CreateOptions{}) @@ -2048,8 +1913,6 @@ var _ = Describe("Client", func() { deleteDeployment(ctx, depFrontend4, "test-namespace-4") deleteNamespace(ctx, tns3) deleteNamespace(ctx, tns4) - - close(done) }, serverSideTimeoutSeconds) It("should filter results using limit and continue options", func() { @@ -2149,7 +2012,7 @@ var _ = Describe("Client", func() { }) Context("with unstructured objects", func() { - It("should fetch collection of objects", func(done Done) { + It("should fetch collection of objects", func() { By("create an initial object") _, err := clientset.AppsV1().Deployments(ns).Create(ctx, dep, metav1.CreateOptions{}) Expect(err).NotTo(HaveOccurred()) @@ -2176,10 +2039,9 @@ var _ = Describe("Client", func() { } } Expect(hasDep).To(BeTrue()) - close(done) }, serverSideTimeoutSeconds) - It("should return an empty list if there are no matching objects", func(done Done) { + It("should return an empty list if there are no matching objects", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) @@ -2194,11 +2056,9 @@ var _ = Describe("Client", func() { By("validating no Deployments are returned") Expect(deps.Items).To(BeEmpty()) - - close(done) }, serverSideTimeoutSeconds) - It("should filter results by namespace selector", func(done Done) { + It("should filter results by namespace selector", func() { By("creating a Deployment in test-namespace-5") tns1 := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test-namespace-5"}} _, err := clientset.CoreV1().Namespaces().Create(ctx, tns1, metav1.CreateOptions{}) @@ -2260,11 +2120,9 @@ var _ = Describe("Client", func() { deleteDeployment(ctx, depBackend, "test-namespace-6") deleteNamespace(ctx, tns1) deleteNamespace(ctx, tns2) - - close(done) }, serverSideTimeoutSeconds) - It("should filter results by field selector", func(done Done) { + It("should filter results by field selector", func() { By("creating a Deployment with name deployment-frontend") depFrontend := &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{Name: "deployment-frontend", Namespace: ns}, @@ -2319,11 +2177,9 @@ var _ = Describe("Client", func() { deleteDeployment(ctx, depFrontend, ns) deleteDeployment(ctx, depBackend, ns) - - close(done) }, serverSideTimeoutSeconds) - It("should filter results by namespace selector and label selector", func(done Done) { + It("should filter results by namespace selector and label selector", func() { By("creating a Deployment in test-namespace-7 with the app=frontend label") tns3 := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test-namespace-7"}} _, err := clientset.CoreV1().Namespaces().Create(ctx, tns3, metav1.CreateOptions{}) @@ -2417,8 +2273,6 @@ var _ = Describe("Client", func() { deleteDeployment(ctx, depFrontend4, "test-namespace-8") deleteNamespace(ctx, tns3) deleteNamespace(ctx, tns4) - - close(done) }, serverSideTimeoutSeconds) PIt("should fail if the object doesn't have meta", func() { @@ -2430,7 +2284,7 @@ var _ = Describe("Client", func() { }) }) Context("with metadata objects", func() { - It("should fetch collection of objects", func(done Done) { + It("should fetch collection of objects", func() { By("creating an initial object") dep, err := clientset.AppsV1().Deployments(ns).Create(ctx, dep, metav1.CreateOptions{}) Expect(err).NotTo(HaveOccurred()) @@ -2467,11 +2321,9 @@ var _ = Describe("Client", func() { } } Expect(hasDep).To(BeTrue()) - - close(done) }, serverSideTimeoutSeconds) - It("should return an empty list if there are no matching objects", func(done Done) { + It("should return an empty list if there are no matching objects", func() { cl, err := client.New(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) @@ -2486,12 +2338,10 @@ var _ = Describe("Client", func() { By("validating no Deployments are returned") Expect(metaList.Items).To(BeEmpty()) - - close(done) }, serverSideTimeoutSeconds) // TODO(seans): get label selector test working - It("should filter results by label selector", func(done Done) { + It("should filter results by label selector", func() { By("creating a Deployment with the app=frontend label") depFrontend := &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{ @@ -2554,11 +2404,9 @@ var _ = Describe("Client", func() { deleteDeployment(ctx, depFrontend, ns) deleteDeployment(ctx, depBackend, ns) - - close(done) }, serverSideTimeoutSeconds) - It("should filter results by namespace selector", func(done Done) { + It("should filter results by namespace selector", func() { By("creating a Deployment in test-namespace-1") tns1 := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test-namespace-1"}} _, err := clientset.CoreV1().Namespaces().Create(ctx, tns1, metav1.CreateOptions{}) @@ -2620,11 +2468,9 @@ var _ = Describe("Client", func() { deleteDeployment(ctx, depBackend, "test-namespace-2") deleteNamespace(ctx, tns1) deleteNamespace(ctx, tns2) - - close(done) }, serverSideTimeoutSeconds) - It("should filter results by field selector", func(done Done) { + It("should filter results by field selector", func() { By("creating a Deployment with name deployment-frontend") depFrontend := &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{Name: "deployment-frontend", Namespace: ns}, @@ -2679,11 +2525,9 @@ var _ = Describe("Client", func() { deleteDeployment(ctx, depFrontend, ns) deleteDeployment(ctx, depBackend, ns) - - close(done) }, serverSideTimeoutSeconds) - It("should filter results by namespace selector and label selector", func(done Done) { + It("should filter results by namespace selector and label selector", func() { By("creating a Deployment in test-namespace-3 with the app=frontend label") tns3 := &corev1.Namespace{ObjectMeta: metav1.ObjectMeta{Name: "test-namespace-3"}} _, err := clientset.CoreV1().Namespaces().Create(ctx, tns3, metav1.CreateOptions{}) @@ -2779,8 +2623,6 @@ var _ = Describe("Client", func() { deleteDeployment(ctx, depFrontend4, "test-namespace-4") deleteNamespace(ctx, tns3) deleteNamespace(ctx, tns4) - - close(done) }, serverSideTimeoutSeconds) It("should filter results using limit and continue options", func() { diff --git a/pkg/client/config/config_suite_test.go b/pkg/client/config/config_suite_test.go index b69c2e1e30..4d07c03c4b 100644 --- a/pkg/client/config/config_suite_test.go +++ b/pkg/client/config/config_suite_test.go @@ -33,8 +33,6 @@ func TestConfig(t *testing.T) { RunSpecsWithDefaultAndCustomReporters(t, suiteName, []Reporter{printer.NewlineReporter{}, printer.NewProwReporter(suiteName)}) } -var _ = BeforeSuite(func(done Done) { +var _ = BeforeSuite(func() { logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) - - close(done) }, 60) diff --git a/pkg/client/fake/client_suite_test.go b/pkg/client/fake/client_suite_test.go index b697144d8b..ac5540106e 100644 --- a/pkg/client/fake/client_suite_test.go +++ b/pkg/client/fake/client_suite_test.go @@ -33,7 +33,6 @@ func TestSource(t *testing.T) { RunSpecsWithDefaultAndCustomReporters(t, suiteName, []Reporter{printer.NewlineReporter{}, printer.NewProwReporter(suiteName)}) } -var _ = BeforeSuite(func(done Done) { +var _ = BeforeSuite(func() { logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) - close(done) }, 60) diff --git a/pkg/client/fake/client_test.go b/pkg/client/fake/client_test.go index 6cf1e9b2ad..267b9f1c0c 100644 --- a/pkg/client/fake/client_test.go +++ b/pkg/client/fake/client_test.go @@ -814,17 +814,16 @@ var _ = Describe("Fake client", func() { } Context("with default scheme.Scheme", func() { - BeforeEach(func(done Done) { + BeforeEach(func() { cl = NewClientBuilder(). WithObjects(dep, dep2, cm). Build() - close(done) }) AssertClientBehavior() }) Context("with given scheme", func() { - BeforeEach(func(done Done) { + BeforeEach(func() { scheme := runtime.NewScheme() Expect(corev1.AddToScheme(scheme)).To(Succeed()) Expect(appsv1.AddToScheme(scheme)).To(Succeed()) @@ -834,7 +833,6 @@ var _ = Describe("Fake client", func() { WithObjects(cm). WithLists(&appsv1.DeploymentList{Items: []appsv1.Deployment{*dep, *dep2}}). Build() - close(done) }) AssertClientBehavior() }) diff --git a/pkg/client/watch_test.go b/pkg/client/watch_test.go index 7d770cb09c..6181596b5e 100644 --- a/pkg/client/watch_test.go +++ b/pkg/client/watch_test.go @@ -40,7 +40,7 @@ var _ = Describe("ClientWithWatch", func() { var ns = "kube-public" ctx := context.TODO() - BeforeEach(func(done Done) { + BeforeEach(func() { atomic.AddUint64(&count, 1) dep = &appsv1.Deployment{ ObjectMeta: metav1.ObjectMeta{Name: fmt.Sprintf("watch-deployment-name-%v", count), Namespace: ns, Labels: map[string]string{"app": fmt.Sprintf("bar-%v", count)}}, @@ -59,21 +59,17 @@ var _ = Describe("ClientWithWatch", func() { var err error dep, err = clientset.AppsV1().Deployments(ns).Create(ctx, dep, metav1.CreateOptions{}) Expect(err).NotTo(HaveOccurred()) - close(done) }, serverSideTimeoutSeconds) - AfterEach(func(done Done) { + AfterEach(func() { deleteDeployment(ctx, dep, ns) - close(done) }, serverSideTimeoutSeconds) Describe("NewWithWatch", func() { - It("should return a new Client", func(done Done) { + It("should return a new Client", func() { cl, err := client.NewWithWatch(cfg, client.Options{}) Expect(err).NotTo(HaveOccurred()) Expect(cl).NotTo(BeNil()) - - close(done) }) watchSuite := func(through client.ObjectList, expectedType client.Object) { @@ -105,12 +101,11 @@ var _ = Describe("ClientWithWatch", func() { } - It("should receive a create event when watching the typed object", func(done Done) { + It("should receive a create event when watching the typed object", func() { watchSuite(&appsv1.DeploymentList{}, &appsv1.Deployment{}) - close(done) }, 15) - It("should receive a create event when watching the unstructured object", func(done Done) { + It("should receive a create event when watching the unstructured object", func() { u := &unstructured.UnstructuredList{} u.SetGroupVersionKind(schema.GroupVersionKind{ Group: "apps", @@ -118,13 +113,11 @@ var _ = Describe("ClientWithWatch", func() { Version: "v1", }) watchSuite(u, &unstructured.Unstructured{}) - close(done) }, 15) - It("should receive a create event when watching the metadata object", func(done Done) { + It("should receive a create event when watching the metadata object", func() { m := &metav1.PartialObjectMetadataList{TypeMeta: metav1.TypeMeta{Kind: "Deployment", APIVersion: "apps/v1"}} watchSuite(m, &metav1.PartialObjectMetadata{}) - close(done) }, 15) }) diff --git a/pkg/cluster/cluster_suite_test.go b/pkg/cluster/cluster_suite_test.go index fe8873473a..92fe2ca082 100644 --- a/pkg/cluster/cluster_suite_test.go +++ b/pkg/cluster/cluster_suite_test.go @@ -43,7 +43,7 @@ var clientset *kubernetes.Clientset // clientTransport is used to force-close keep-alives in tests that check for leaks var clientTransport *http.Transport -var _ = BeforeSuite(func(done Done) { +var _ = BeforeSuite(func() { logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) testenv = &envtest.Environment{} @@ -63,8 +63,6 @@ var _ = BeforeSuite(func(done Done) { clientset, err = kubernetes.NewForConfig(cfg) Expect(err).NotTo(HaveOccurred()) - - close(done) }, 60) var _ = AfterSuite(func() { diff --git a/pkg/cluster/cluster_test.go b/pkg/cluster/cluster_test.go index d0b03785c3..f9f7a0bdf3 100644 --- a/pkg/cluster/cluster_test.go +++ b/pkg/cluster/cluster_test.go @@ -55,7 +55,7 @@ var _ = Describe("cluster.Cluster", func() { }) - It("should return an error it can't create a client.Client", func(done Done) { + It("should return an error it can't create a client.Client", func() { c, err := New(cfg, func(o *Options) { o.NewClient = func(cache cache.Cache, config *rest.Config, options client.Options, uncachedObjects ...client.Object) (client.Client, error) { return nil, errors.New("expected error") @@ -64,11 +64,9 @@ var _ = Describe("cluster.Cluster", func() { Expect(c).To(BeNil()) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("expected error")) - - close(done) }) - It("should return an error it can't create a cache.Cache", func(done Done) { + It("should return an error it can't create a cache.Cache", func() { c, err := New(cfg, func(o *Options) { o.NewCache = func(config *rest.Config, opts cache.Options) (cache.Cache, error) { return nil, fmt.Errorf("expected error") @@ -77,11 +75,9 @@ var _ = Describe("cluster.Cluster", func() { Expect(c).To(BeNil()) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("expected error")) - - close(done) }) - It("should create a client defined in by the new client function", func(done Done) { + It("should create a client defined in by the new client function", func() { c, err := New(cfg, func(o *Options) { o.NewClient = func(cache cache.Cache, config *rest.Config, options client.Options, uncachedObjects ...client.Object) (client.Client, error) { return nil, nil @@ -90,11 +86,9 @@ var _ = Describe("cluster.Cluster", func() { Expect(c).ToNot(BeNil()) Expect(err).ToNot(HaveOccurred()) Expect(c.GetClient()).To(BeNil()) - - close(done) }) - It("should return an error it can't create a recorder.Provider", func(done Done) { + It("should return an error it can't create a recorder.Provider", func() { c, err := New(cfg, func(o *Options) { o.newRecorderProvider = func(_ *rest.Config, _ *runtime.Scheme, _ logr.Logger, _ intrec.EventBroadcasterProducer) (*intrec.Provider, error) { return nil, fmt.Errorf("expected error") @@ -103,26 +97,22 @@ var _ = Describe("cluster.Cluster", func() { Expect(c).To(BeNil()) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("expected error")) - - close(done) }) }) Describe("Start", func() { - It("should stop when context is cancelled", func(done Done) { + It("should stop when context is cancelled", func() { c, err := New(cfg) Expect(err).NotTo(HaveOccurred()) ctx, cancel := context.WithCancel(context.Background()) cancel() Expect(c.Start(ctx)).NotTo(HaveOccurred()) - - close(done) }) }) Describe("SetFields", func() { - It("should inject field values", func(done Done) { + It("should inject field values", func() { c, err := New(cfg, func(o *Options) { o.NewCache = func(_ *rest.Config, _ cache.Options) (cache.Cache, error) { return &informertest.FakeInformers{}, nil @@ -190,8 +180,6 @@ var _ = Describe("cluster.Cluster", func() { }, }) Expect(err).To(Equal(expected)) - - close(done) }) }) diff --git a/pkg/controller/controller_integration_test.go b/pkg/controller/controller_integration_test.go index 762b3d9fbb..9f347b0032 100644 --- a/pkg/controller/controller_integration_test.go +++ b/pkg/controller/controller_integration_test.go @@ -48,7 +48,7 @@ var _ = Describe("controller", func() { Describe("controller", func() { // TODO(directxman12): write a whole suite of controller-client interaction tests - It("should reconcile", func(done Done) { + It("should reconcile", func() { By("Creating the Manager") cm, err := manager.New(cfg, manager.Options{}) Expect(err).NotTo(HaveOccurred()) @@ -169,8 +169,6 @@ var _ = Describe("controller", func() { err = cm.GetClient(). List(context.Background(), &controllertest.UnconventionalListTypeList{}) Expect(err).NotTo(HaveOccurred()) - - close(done) }, 5) }) }) diff --git a/pkg/controller/controller_suite_test.go b/pkg/controller/controller_suite_test.go index b4d5111bb8..f4c800ac40 100644 --- a/pkg/controller/controller_suite_test.go +++ b/pkg/controller/controller_suite_test.go @@ -49,7 +49,7 @@ var clientset *kubernetes.Clientset // clientTransport is used to force-close keep-alives in tests that check for leaks var clientTransport *http.Transport -var _ = BeforeSuite(func(done Done) { +var _ = BeforeSuite(func() { logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) err := (&crscheme.Builder{ @@ -82,8 +82,6 @@ var _ = BeforeSuite(func(done Done) { // Prevent the metrics listener being created metrics.DefaultBindAddress = "0" - - close(done) }, 60) var _ = AfterSuite(func() { diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index 4b0a2ee914..d3e8419a16 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -42,28 +42,24 @@ var _ = Describe("controller.Controller", func() { }) Describe("New", func() { - It("should return an error if Name is not Specified", func(done Done) { + It("should return an error if Name is not Specified", func() { m, err := manager.New(cfg, manager.Options{}) Expect(err).NotTo(HaveOccurred()) c, err := controller.New("", m, controller.Options{Reconciler: rec}) Expect(c).To(BeNil()) Expect(err.Error()).To(ContainSubstring("must specify Name for Controller")) - - close(done) }) - It("should return an error if Reconciler is not Specified", func(done Done) { + It("should return an error if Reconciler is not Specified", func() { m, err := manager.New(cfg, manager.Options{}) Expect(err).NotTo(HaveOccurred()) c, err := controller.New("foo", m, controller.Options{}) Expect(c).To(BeNil()) Expect(err.Error()).To(ContainSubstring("must specify Reconciler")) - - close(done) }) - It("NewController should return an error if injecting Reconciler fails", func(done Done) { + It("NewController should return an error if injecting Reconciler fails", func() { m, err := manager.New(cfg, manager.Options{}) Expect(err).NotTo(HaveOccurred()) @@ -71,11 +67,9 @@ var _ = Describe("controller.Controller", func() { Expect(c).To(BeNil()) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("expected error")) - - close(done) }) - It("should not return an error if two controllers are registered with different names", func(done Done) { + It("should not return an error if two controllers are registered with different names", func() { m, err := manager.New(cfg, manager.Options{}) Expect(err).NotTo(HaveOccurred()) @@ -86,8 +80,6 @@ var _ = Describe("controller.Controller", func() { c2, err := controller.New("c2", m, controller.Options{Reconciler: rec}) Expect(err).NotTo(HaveOccurred()) Expect(c2).ToNot(BeNil()) - - close(done) }) It("should not leak goroutines when stopped", func() { diff --git a/pkg/controller/controllerutil/controllerutil_suite_test.go b/pkg/controller/controllerutil/controllerutil_suite_test.go index 0cfd6d1e02..da4c5cf4ac 100644 --- a/pkg/controller/controllerutil/controllerutil_suite_test.go +++ b/pkg/controller/controllerutil/controllerutil_suite_test.go @@ -34,16 +34,16 @@ func TestControllerutil(t *testing.T) { RunSpecsWithDefaultAndCustomReporters(t, suiteName, []Reporter{printer.NewlineReporter{}, printer.NewProwReporter(suiteName)}) } -var t *envtest.Environment +var testenv *envtest.Environment var cfg *rest.Config var c client.Client var _ = BeforeSuite(func() { var err error - t = &envtest.Environment{} + testenv = &envtest.Environment{} - cfg, err = t.Start() + cfg, err = testenv.Start() Expect(err).NotTo(HaveOccurred()) c, err = client.New(cfg, client.Options{}) @@ -51,5 +51,5 @@ var _ = BeforeSuite(func() { }) var _ = AfterSuite(func() { - Expect(t.Stop()).To(Succeed()) + Expect(testenv.Stop()).To(Succeed()) }) diff --git a/pkg/envtest/envtest_suite_test.go b/pkg/envtest/envtest_suite_test.go index d778c88077..7b555ef126 100644 --- a/pkg/envtest/envtest_suite_test.go +++ b/pkg/envtest/envtest_suite_test.go @@ -38,15 +38,13 @@ func TestSource(t *testing.T) { var env *Environment -var _ = BeforeSuite(func(done Done) { +var _ = BeforeSuite(func() { logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) env = &Environment{} // we're initializing webhook here and not in webhook.go to also test the envtest install code via WebhookOptions initializeWebhookInEnvironment() _, err := env.Start() Expect(err).NotTo(HaveOccurred()) - - close(done) }, StartTimeout) func initializeWebhookInEnvironment() { @@ -136,8 +134,6 @@ func initializeWebhookInEnvironment() { } } -var _ = AfterSuite(func(done Done) { +var _ = AfterSuite(func() { Expect(env.Stop()).NotTo(HaveOccurred()) - - close(done) }, StopTimeout) diff --git a/pkg/envtest/envtest_test.go b/pkg/envtest/envtest_test.go index fb5fb87a7f..220f64ca86 100644 --- a/pkg/envtest/envtest_test.go +++ b/pkg/envtest/envtest_test.go @@ -45,7 +45,7 @@ var _ = Describe("Test", func() { var teardownTimeoutSeconds float64 = 10 // Initialize the client - BeforeEach(func(done Done) { + BeforeEach(func() { crds = []client.Object{} s = runtime.NewScheme() err = v1beta1.AddToScheme(s) @@ -55,12 +55,10 @@ var _ = Describe("Test", func() { c, err = client.New(env.Config, client.Options{Scheme: s}) Expect(err).NotTo(HaveOccurred()) - - close(done) }) // Cleanup CRDs - AfterEach(func(done Done) { + AfterEach(func() { for _, crd := range runtimeCRDListToUnstructured(crds) { // Delete only if CRD exists. crdObjectKey := client.ObjectKey{ @@ -79,7 +77,6 @@ var _ = Describe("Test", func() { return apierrors.IsNotFound(err) }, 5*time.Second).Should(BeTrue()) } - close(done) }, teardownTimeoutSeconds) Describe("InstallCRDs", func() { @@ -121,7 +118,7 @@ var _ = Describe("Test", func() { ) Expect(err).NotTo(HaveOccurred()) }) - It("should install the CRDs into the cluster using directory", func(done Done) { + It("should install the CRDs into the cluster using directory", func() { crds, err = InstallCRDs(env.Config, CRDInstallOptions{ Paths: []string{validDirectory}, }) @@ -221,11 +218,9 @@ var _ = Describe("Test", func() { CRDInstallOptions{MaxTime: 50 * time.Millisecond, PollInterval: 15 * time.Millisecond}, ) Expect(err).NotTo(HaveOccurred()) - - close(done) }, 5) - It("should install the CRDs into the cluster using file", func(done Done) { + It("should install the CRDs into the cluster using file", func() { crds, err = InstallCRDs(env.Config, CRDInstallOptions{ Paths: []string{filepath.Join(".", "testdata", "crds", "examplecrd3.yaml")}, }) @@ -249,11 +244,9 @@ var _ = Describe("Test", func() { CRDInstallOptions{MaxTime: 50 * time.Millisecond, PollInterval: 15 * time.Millisecond}, ) Expect(err).NotTo(HaveOccurred()) - - close(done) }, 10) - It("should be able to install CRDs using multiple files", func(done Done) { + It("should be able to install CRDs using multiple files", func() { crds, err = InstallCRDs(env.Config, CRDInstallOptions{ Paths: []string{ filepath.Join(".", "testdata", "examplecrd.yaml"), @@ -262,11 +255,9 @@ var _ = Describe("Test", func() { }) Expect(err).NotTo(HaveOccurred()) Expect(crds).To(HaveLen(2)) - - close(done) }, 10) - It("should filter out already existent CRD", func(done Done) { + It("should filter out already existent CRD", func() { crds, err = InstallCRDs(env.Config, CRDInstallOptions{ Paths: []string{ filepath.Join(".", "testdata"), @@ -304,36 +295,28 @@ var _ = Describe("Test", func() { CRDInstallOptions{MaxTime: 50 * time.Millisecond, PollInterval: 15 * time.Millisecond}, ) Expect(err).NotTo(HaveOccurred()) - - close(done) }, 10) - It("should not return an not error if the directory doesn't exist", func(done Done) { + It("should not return an not error if the directory doesn't exist", func() { crds, err = InstallCRDs(env.Config, CRDInstallOptions{Paths: []string{invalidDirectory}}) Expect(err).NotTo(HaveOccurred()) - - close(done) }, 5) - It("should return an error if the directory doesn't exist", func(done Done) { + It("should return an error if the directory doesn't exist", func() { crds, err = InstallCRDs(env.Config, CRDInstallOptions{ Paths: []string{invalidDirectory}, ErrorIfPathMissing: true, }) Expect(err).To(HaveOccurred()) - - close(done) }, 5) - It("should return an error if the file doesn't exist", func(done Done) { + It("should return an error if the file doesn't exist", func() { crds, err = InstallCRDs(env.Config, CRDInstallOptions{Paths: []string{ filepath.Join(".", "testdata", "fake.yaml")}, ErrorIfPathMissing: true, }) Expect(err).To(HaveOccurred()) - - close(done) }, 5) - It("should return an error if the resource group version isn't found", func(done Done) { + It("should return an error if the resource group version isn't found", func() { // Wait for a CRD where the Group and Version don't exist err := WaitForCRDs(env.Config, []client.Object{ @@ -348,11 +331,9 @@ var _ = Describe("Test", func() { CRDInstallOptions{MaxTime: 50 * time.Millisecond, PollInterval: 15 * time.Millisecond}, ) Expect(err).To(HaveOccurred()) - - close(done) }, 5) - It("should return an error if the resource isn't found in the group version", func(done Done) { + It("should return an error if the resource isn't found in the group version", func() { crds, err = InstallCRDs(env.Config, CRDInstallOptions{ Paths: []string{"."}, }) @@ -379,11 +360,9 @@ var _ = Describe("Test", func() { CRDInstallOptions{MaxTime: 50 * time.Millisecond, PollInterval: 15 * time.Millisecond}, ) Expect(err).To(HaveOccurred()) - - close(done) }, 5) - It("should reinstall the CRDs if already present in the cluster", func(done Done) { + It("should reinstall the CRDs if already present in the cluster", func() { crds, err = InstallCRDs(env.Config, CRDInstallOptions{ Paths: []string{filepath.Join(".", "testdata")}, @@ -586,12 +565,10 @@ var _ = Describe("Test", func() { CRDInstallOptions{MaxTime: 50 * time.Millisecond, PollInterval: 15 * time.Millisecond}, ) Expect(err).NotTo(HaveOccurred()) - - close(done) }, 5) }) - It("should update CRDs if already present in the cluster", func(done Done) { + It("should update CRDs if already present in the cluster", func() { // Install only the CRDv1 multi-version example crds, err = InstallCRDs(env.Config, CRDInstallOptions{ @@ -679,12 +656,10 @@ var _ = Describe("Test", func() { CRDInstallOptions{MaxTime: 50 * time.Millisecond, PollInterval: 15 * time.Millisecond}, ) Expect(err).NotTo(HaveOccurred()) - - close(done) }, 5) Describe("UninstallCRDs", func() { - It("should uninstall the CRDs from the cluster", func(done Done) { + It("should uninstall the CRDs from the cluster", func() { crds, err = InstallCRDs(env.Config, CRDInstallOptions{ Paths: []string{validDirectory}, @@ -825,31 +800,27 @@ var _ = Describe("Test", func() { } return true }, 20).Should(BeTrue()) - - close(done) }, 30) }) Describe("Start", func() { - It("should raise an error on invalid dir when flag is enabled", func(done Done) { + It("should raise an error on invalid dir when flag is enabled", func() { env := &Environment{ErrorIfCRDPathMissing: true, CRDDirectoryPaths: []string{invalidDirectory}} _, err := env.Start() Expect(err).To(HaveOccurred()) Expect(env.Stop()).To(Succeed()) - close(done) }, 30) - It("should not raise an error on invalid dir when flag is disabled", func(done Done) { + It("should not raise an error on invalid dir when flag is disabled", func() { env := &Environment{ErrorIfCRDPathMissing: false, CRDDirectoryPaths: []string{invalidDirectory}} _, err := env.Start() Expect(err).NotTo(HaveOccurred()) Expect(env.Stop()).To(Succeed()) - close(done) }, 30) }) Describe("Stop", func() { - It("should cleanup webhook /tmp folder with no error when using existing cluster", func(done Done) { + It("should cleanup webhook /tmp folder with no error when using existing cluster", func() { env := &Environment{} _, err := env.Start() Expect(err).NotTo(HaveOccurred()) @@ -857,7 +828,6 @@ var _ = Describe("Test", func() { // check if the /tmp/envtest-serving-certs-* dir doesnt exists any more Expect(env.WebhookInstallOptions.LocalServingCertDir).ShouldNot(BeADirectory()) - close(done) }, 30) }) }) diff --git a/pkg/envtest/webhook_test.go b/pkg/envtest/webhook_test.go index d6c9ab0f14..1efc60d9ff 100644 --- a/pkg/envtest/webhook_test.go +++ b/pkg/envtest/webhook_test.go @@ -37,7 +37,7 @@ import ( var _ = Describe("Test", func() { Describe("Webhook", func() { - It("should reject create request for webhook that rejects all requests", func(done Done) { + It("should reject create request for webhook that rejects all requests", func() { m, err := manager.New(env.Config, manager.Options{ Port: env.WebhookInstallOptions.LocalServingPort, Host: env.WebhookInstallOptions.LocalServingHost, @@ -88,7 +88,6 @@ var _ = Describe("Test", func() { }, 1*time.Second).Should(BeTrue()) cancel() - close(done) }) It("should load webhooks from directory", func() { diff --git a/pkg/handler/eventhandler_test.go b/pkg/handler/eventhandler_test.go index 7ba3b8aa84..61db62e66a 100644 --- a/pkg/handler/eventhandler_test.go +++ b/pkg/handler/eventhandler_test.go @@ -54,7 +54,7 @@ var _ = Describe("Eventhandler", func() { }) Describe("EnqueueRequestForObject", func() { - It("should enqueue a Request with the Name / Namespace of the object in the CreateEvent.", func(done Done) { + It("should enqueue a Request with the Name / Namespace of the object in the CreateEvent.", func() { evt := event.CreateEvent{ Object: pod, } @@ -66,11 +66,9 @@ var _ = Describe("Eventhandler", func() { req, ok := i.(reconcile.Request) Expect(ok).To(BeTrue()) Expect(req.NamespacedName).To(Equal(types.NamespacedName{Namespace: "biz", Name: "baz"})) - - close(done) }) - It("should enqueue a Request with the Name / Namespace of the object in the DeleteEvent.", func(done Done) { + It("should enqueue a Request with the Name / Namespace of the object in the DeleteEvent.", func() { evt := event.DeleteEvent{ Object: pod, } @@ -82,12 +80,10 @@ var _ = Describe("Eventhandler", func() { req, ok := i.(reconcile.Request) Expect(ok).To(BeTrue()) Expect(req.NamespacedName).To(Equal(types.NamespacedName{Namespace: "biz", Name: "baz"})) - - close(done) }) It("should enqueue a Request with the Name / Namespace of one object in the UpdateEvent.", - func(done Done) { + func() { newPod := pod.DeepCopy() newPod.Name = "baz2" newPod.Namespace = "biz2" @@ -104,11 +100,9 @@ var _ = Describe("Eventhandler", func() { req, ok := i.(reconcile.Request) Expect(ok).To(BeTrue()) Expect(req.NamespacedName).To(Equal(types.NamespacedName{Namespace: "biz2", Name: "baz2"})) - - close(done) }) - It("should enqueue a Request with the Name / Namespace of the object in the GenericEvent.", func(done Done) { + It("should enqueue a Request with the Name / Namespace of the object in the GenericEvent.", func() { evt := event.GenericEvent{ Object: pod, } @@ -119,21 +113,18 @@ var _ = Describe("Eventhandler", func() { req, ok := i.(reconcile.Request) Expect(ok).To(BeTrue()) Expect(req.NamespacedName).To(Equal(types.NamespacedName{Namespace: "biz", Name: "baz"})) - - close(done) }) Context("for a runtime.Object without Object", func() { - It("should do nothing if the Object is missing for a CreateEvent.", func(done Done) { + It("should do nothing if the Object is missing for a CreateEvent.", func() { evt := event.CreateEvent{ Object: nil, } instance.Create(evt, q) Expect(q.Len()).To(Equal(0)) - close(done) }) - It("should do nothing if the Object is missing for a UpdateEvent.", func(done Done) { + It("should do nothing if the Object is missing for a UpdateEvent.", func() { newPod := pod.DeepCopy() newPod.Name = "baz2" newPod.Namespace = "biz2" @@ -159,26 +150,22 @@ var _ = Describe("Eventhandler", func() { req, ok = i.(reconcile.Request) Expect(ok).To(BeTrue()) Expect(req.NamespacedName).To(Equal(types.NamespacedName{Namespace: "biz", Name: "baz"})) - - close(done) }) - It("should do nothing if the Object is missing for a DeleteEvent.", func(done Done) { + It("should do nothing if the Object is missing for a DeleteEvent.", func() { evt := event.DeleteEvent{ Object: nil, } instance.Delete(evt, q) Expect(q.Len()).To(Equal(0)) - close(done) }) - It("should do nothing if the Object is missing for a GenericEvent.", func(done Done) { + It("should do nothing if the Object is missing for a GenericEvent.", func() { evt := event.GenericEvent{ Object: nil, } instance.Generic(evt, q) Expect(q.Len()).To(Equal(0)) - close(done) }) }) }) @@ -823,7 +810,7 @@ var _ = Describe("Eventhandler", func() { }, } - It("should call CreateFunc for a CreateEvent if provided.", func(done Done) { + It("should call CreateFunc for a CreateEvent if provided.", func() { instance := failingFuncs evt := event.CreateEvent{ Object: pod, @@ -834,20 +821,18 @@ var _ = Describe("Eventhandler", func() { Expect(evt2).To(Equal(evt)) } instance.Create(evt, q) - close(done) }) - It("should NOT call CreateFunc for a CreateEvent if NOT provided.", func(done Done) { + It("should NOT call CreateFunc for a CreateEvent if NOT provided.", func() { instance := failingFuncs instance.CreateFunc = nil evt := event.CreateEvent{ Object: pod, } instance.Create(evt, q) - close(done) }) - It("should call UpdateFunc for an UpdateEvent if provided.", func(done Done) { + It("should call UpdateFunc for an UpdateEvent if provided.", func() { newPod := pod.DeepCopy() newPod.Name = pod.Name + "2" newPod.Namespace = pod.Namespace + "2" @@ -864,10 +849,9 @@ var _ = Describe("Eventhandler", func() { } instance.Update(evt, q) - close(done) }) - It("should NOT call UpdateFunc for an UpdateEvent if NOT provided.", func(done Done) { + It("should NOT call UpdateFunc for an UpdateEvent if NOT provided.", func() { newPod := pod.DeepCopy() newPod.Name = pod.Name + "2" newPod.Namespace = pod.Namespace + "2" @@ -876,10 +860,9 @@ var _ = Describe("Eventhandler", func() { ObjectNew: newPod, } instance.Update(evt, q) - close(done) }) - It("should call DeleteFunc for a DeleteEvent if provided.", func(done Done) { + It("should call DeleteFunc for a DeleteEvent if provided.", func() { instance := failingFuncs evt := event.DeleteEvent{ Object: pod, @@ -890,20 +873,18 @@ var _ = Describe("Eventhandler", func() { Expect(evt2).To(Equal(evt)) } instance.Delete(evt, q) - close(done) }) - It("should NOT call DeleteFunc for a DeleteEvent if NOT provided.", func(done Done) { + It("should NOT call DeleteFunc for a DeleteEvent if NOT provided.", func() { instance := failingFuncs instance.DeleteFunc = nil evt := event.DeleteEvent{ Object: pod, } instance.Delete(evt, q) - close(done) }) - It("should call GenericFunc for a GenericEvent if provided.", func(done Done) { + It("should call GenericFunc for a GenericEvent if provided.", func() { instance := failingFuncs evt := event.GenericEvent{ Object: pod, @@ -914,17 +895,15 @@ var _ = Describe("Eventhandler", func() { Expect(evt2).To(Equal(evt)) } instance.Generic(evt, q) - close(done) }) - It("should NOT call GenericFunc for a GenericEvent if NOT provided.", func(done Done) { + It("should NOT call GenericFunc for a GenericEvent if NOT provided.", func() { instance := failingFuncs instance.GenericFunc = nil evt := event.GenericEvent{ Object: pod, } instance.Generic(evt, q) - close(done) }) }) }) diff --git a/pkg/internal/controller/controller_suite_test.go b/pkg/internal/controller/controller_suite_test.go index 31567e66a5..6091dd746c 100644 --- a/pkg/internal/controller/controller_suite_test.go +++ b/pkg/internal/controller/controller_suite_test.go @@ -39,7 +39,7 @@ var testenv *envtest.Environment var cfg *rest.Config var clientset *kubernetes.Clientset -var _ = BeforeSuite(func(done Done) { +var _ = BeforeSuite(func() { logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) testenv = &envtest.Environment{} @@ -50,8 +50,6 @@ var _ = BeforeSuite(func(done Done) { clientset, err = kubernetes.NewForConfig(cfg) Expect(err).NotTo(HaveOccurred()) - - close(done) }, 60) var _ = AfterSuite(func() { diff --git a/pkg/internal/controller/controller_test.go b/pkg/internal/controller/controller_test.go index 5bed7696ac..7545be73a7 100644 --- a/pkg/internal/controller/controller_test.go +++ b/pkg/internal/controller/controller_test.go @@ -91,7 +91,7 @@ var _ = Describe("controller", func() { }) Describe("Start", func() { - It("should return an error if there is an error waiting for the informers", func(done Done) { + It("should return an error if there is an error waiting for the informers", func() { f := false ctrl.startWatches = []watchDescription{{ src: source.NewKindWithCache(&corev1.Pod{}, &informertest.FakeInformers{Synced: &f}), @@ -102,11 +102,9 @@ var _ = Describe("controller", func() { err := ctrl.Start(ctx) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("failed to wait for foo caches to sync")) - - close(done) }) - It("should error when cache sync timeout occurs", func(done Done) { + It("should error when cache sync timeout occurs", func() { ctrl.CacheSyncTimeout = 10 * time.Nanosecond c, err := cache.New(cfg, cache.Options{}) @@ -121,11 +119,9 @@ var _ = Describe("controller", func() { err = ctrl.Start(context.TODO()) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("failed to wait for testcontroller caches to sync: timed out waiting for cache to be synced")) - - close(done) }) - It("should not error when cache sync timeout is of sufficiently high", func(done Done) { + It("should not error when cache sync timeout is of sufficiently high", func() { ctrl.CacheSyncTimeout = 1 * time.Second ctx, cancel := context.WithCancel(context.Background()) @@ -152,10 +148,9 @@ var _ = Describe("controller", func() { }() <-sourceSynced - close(done) }, 10.0) - It("should process events from source.Channel", func(done Done) { + It("should process events from source.Channel", func() { // channel to be closed when event is processed processed := make(chan struct{}) // source channel to be injected @@ -194,10 +189,9 @@ var _ = Describe("controller", func() { Expect(ctrl.Start(ctx)).To(Succeed()) }() <-processed - close(done) }) - It("should error when channel is passed as a source but stop channel is not injected", func(done Done) { + It("should error when channel is passed as a source but stop channel is not injected", func() { ch := make(chan event.GenericEvent) ctx, cancel := context.WithCancel(context.TODO()) defer cancel() @@ -211,10 +205,9 @@ var _ = Describe("controller", func() { Expect(e).NotTo(BeNil()) Expect(e.Error()).To(ContainSubstring("must call InjectStop on Channel before calling Start")) - close(done) }) - It("should error when channel source is not specified", func(done Done) { + It("should error when channel source is not specified", func() { ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -228,8 +221,6 @@ var _ = Describe("controller", func() { e := ctrl.Start(ctx) Expect(e).NotTo(BeNil()) Expect(e.Error()).To(ContainSubstring("must specify Channel.Source")) - - close(done) }) It("should call Start on sources with the appropriate EventHandler, Queue, and Predicates", func() { @@ -403,7 +394,7 @@ var _ = Describe("controller", func() { }) Describe("Processing queue items from a Controller", func() { - It("should call Reconciler if an item is enqueued", func(done Done) { + It("should call Reconciler if an item is enqueued", func() { ctx, cancel := context.WithCancel(context.Background()) defer cancel() go func() { @@ -419,11 +410,9 @@ var _ = Describe("controller", func() { By("Removing the item from the queue") Eventually(queue.Len).Should(Equal(0)) Eventually(func() int { return queue.NumRequeues(request) }).Should(Equal(0)) - - close(done) }) - It("should continue to process additional queue items after the first", func(done Done) { + It("should continue to process additional queue items after the first", func() { ctrl.Do = reconcile.Func(func(context.Context, reconcile.Request) (reconcile.Result, error) { defer GinkgoRecover() Fail("Reconciler should not have been called") @@ -443,15 +432,13 @@ var _ = Describe("controller", func() { By("expecting both of them to be skipped") Eventually(queue.Len).Should(Equal(0)) Eventually(func() int { return queue.NumRequeues(request) }).Should(Equal(0)) - - close(done) }) PIt("should forget an item if it is not a Request and continue processing items", func() { // TODO(community): write this test }) - It("should requeue a Request if there is an error and continue processing items", func(done Done) { + It("should requeue a Request if there is an error and continue processing items", func() { ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -473,8 +460,6 @@ var _ = Describe("controller", func() { By("Removing the item from the queue") Eventually(queue.Len).Should(Equal(0)) Eventually(func() int { return queue.NumRequeues(request) }).Should(Equal(0)) - - close(done) }, 1.0) // TODO(directxman12): we should ensure that backoff occurrs with error requeue @@ -624,7 +609,7 @@ var _ = Describe("controller", func() { reconcileTotal.Reset() }) - It("should get updated on successful reconciliation", func(done Done) { + It("should get updated on successful reconciliation", func() { Expect(func() error { Expect(ctrlmetrics.ReconcileTotal.WithLabelValues(ctrl.Name, "success").Write(&reconcileTotal)).To(Succeed()) if reconcileTotal.GetCounter().GetValue() != 0.0 { @@ -651,11 +636,9 @@ var _ = Describe("controller", func() { } return nil }, 2.0).Should(Succeed()) - - close(done) }, 2.0) - It("should get updated on reconcile errors", func(done Done) { + It("should get updated on reconcile errors", func() { Expect(func() error { Expect(ctrlmetrics.ReconcileTotal.WithLabelValues(ctrl.Name, "error").Write(&reconcileTotal)).To(Succeed()) if reconcileTotal.GetCounter().GetValue() != 0.0 { @@ -682,11 +665,9 @@ var _ = Describe("controller", func() { } return nil }, 2.0).Should(Succeed()) - - close(done) }, 2.0) - It("should get updated when reconcile returns with retry enabled", func(done Done) { + It("should get updated when reconcile returns with retry enabled", func() { Expect(func() error { Expect(ctrlmetrics.ReconcileTotal.WithLabelValues(ctrl.Name, "retry").Write(&reconcileTotal)).To(Succeed()) if reconcileTotal.GetCounter().GetValue() != 0.0 { @@ -714,11 +695,9 @@ var _ = Describe("controller", func() { } return nil }, 2.0).Should(Succeed()) - - close(done) }, 2.0) - It("should get updated when reconcile returns with retryAfter enabled", func(done Done) { + It("should get updated when reconcile returns with retryAfter enabled", func() { Expect(func() error { Expect(ctrlmetrics.ReconcileTotal.WithLabelValues(ctrl.Name, "retry_after").Write(&reconcileTotal)).To(Succeed()) if reconcileTotal.GetCounter().GetValue() != 0.0 { @@ -745,13 +724,11 @@ var _ = Describe("controller", func() { } return nil }, 2.0).Should(Succeed()) - - close(done) }, 2.0) }) Context("should update prometheus metrics", func() { - It("should requeue a Request if there is an error and continue processing items", func(done Done) { + It("should requeue a Request if there is an error and continue processing items", func() { var reconcileErrs dto.Metric ctrlmetrics.ReconcileErrors.Reset() Expect(func() error { @@ -788,11 +765,9 @@ var _ = Describe("controller", func() { By("Removing the item from the queue") Eventually(queue.Len).Should(Equal(0)) Eventually(func() int { return queue.NumRequeues(request) }).Should(Equal(0)) - - close(done) }, 2.0) - It("should add a reconcile time to the reconcile time histogram", func(done Done) { + It("should add a reconcile time to the reconcile time histogram", func() { var reconcileTime dto.Metric ctrlmetrics.ReconcileTime.Reset() @@ -831,8 +806,6 @@ var _ = Describe("controller", func() { } return nil }, 2.0).Should(Succeed()) - - close(done) }, 4.0) }) }) diff --git a/pkg/internal/recorder/recorder_integration_test.go b/pkg/internal/recorder/recorder_integration_test.go index a67d0e1ed5..5bafaabf5a 100644 --- a/pkg/internal/recorder/recorder_integration_test.go +++ b/pkg/internal/recorder/recorder_integration_test.go @@ -37,7 +37,7 @@ import ( var _ = Describe("recorder", func() { Describe("recorder", func() { - It("should publish events", func(done Done) { + It("should publish events", func() { By("Creating the Manager") cm, err := manager.New(cfg, manager.Options{}) Expect(err).NotTo(HaveOccurred()) @@ -108,8 +108,6 @@ var _ = Describe("recorder", func() { Expect(evt.Type).To(Equal(corev1.EventTypeNormal)) Expect(evt.Reason).To(Equal("test-reason")) Expect(evt.Message).To(Equal("test-msg")) - - close(done) }) }) }) diff --git a/pkg/internal/recorder/recorder_suite_test.go b/pkg/internal/recorder/recorder_suite_test.go index ed4a5c4140..ee8f98fae0 100644 --- a/pkg/internal/recorder/recorder_suite_test.go +++ b/pkg/internal/recorder/recorder_suite_test.go @@ -39,7 +39,7 @@ var testenv *envtest.Environment var cfg *rest.Config var clientset *kubernetes.Clientset -var _ = BeforeSuite(func(done Done) { +var _ = BeforeSuite(func() { logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) testenv = &envtest.Environment{} @@ -50,8 +50,6 @@ var _ = BeforeSuite(func(done Done) { clientset, err = kubernetes.NewForConfig(cfg) Expect(err).NotTo(HaveOccurred()) - - close(done) }, 60) var _ = AfterSuite(func() { diff --git a/pkg/manager/manager_suite_test.go b/pkg/manager/manager_suite_test.go index 27341129c8..9a544fa19a 100644 --- a/pkg/manager/manager_suite_test.go +++ b/pkg/manager/manager_suite_test.go @@ -45,7 +45,7 @@ var clientset *kubernetes.Clientset // clientTransport is used to force-close keep-alives in tests that check for leaks var clientTransport *http.Transport -var _ = BeforeSuite(func(done Done) { +var _ = BeforeSuite(func() { logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) testenv = &envtest.Environment{} @@ -72,8 +72,6 @@ var _ = BeforeSuite(func(done Done) { // Prevent the metrics listener being created metrics.DefaultBindAddress = "0" - - close(done) }, 60) var _ = AfterSuite(func() { diff --git a/pkg/manager/manager_test.go b/pkg/manager/manager_test.go index 50b99efa94..0eb12fa117 100644 --- a/pkg/manager/manager_test.go +++ b/pkg/manager/manager_test.go @@ -75,7 +75,7 @@ var _ = Describe("manger.Manager", func() { }) - It("should return an error it can't create a client.Client", func(done Done) { + It("should return an error it can't create a client.Client", func() { m, err := New(cfg, Options{ NewClient: func(cache cache.Cache, config *rest.Config, options client.Options, uncachedObjects ...client.Object) (client.Client, error) { return nil, errors.New("expected error") @@ -84,11 +84,9 @@ var _ = Describe("manger.Manager", func() { Expect(m).To(BeNil()) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("expected error")) - - close(done) }) - It("should return an error it can't create a cache.Cache", func(done Done) { + It("should return an error it can't create a cache.Cache", func() { m, err := New(cfg, Options{ NewCache: func(config *rest.Config, opts cache.Options) (cache.Cache, error) { return nil, fmt.Errorf("expected error") @@ -97,11 +95,9 @@ var _ = Describe("manger.Manager", func() { Expect(m).To(BeNil()) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("expected error")) - - close(done) }) - It("should create a client defined in by the new client function", func(done Done) { + It("should create a client defined in by the new client function", func() { m, err := New(cfg, Options{ NewClient: func(cache cache.Cache, config *rest.Config, options client.Options, uncachedObjects ...client.Object) (client.Client, error) { return nil, nil @@ -110,11 +106,9 @@ var _ = Describe("manger.Manager", func() { Expect(m).ToNot(BeNil()) Expect(err).ToNot(HaveOccurred()) Expect(m.GetClient()).To(BeNil()) - - close(done) }) - It("should return an error it can't create a recorder.Provider", func(done Done) { + It("should return an error it can't create a recorder.Provider", func() { m, err := New(cfg, Options{ newRecorderProvider: func(_ *rest.Config, _ *runtime.Scheme, _ logr.Logger, _ intrec.EventBroadcasterProducer) (*intrec.Provider, error) { return nil, fmt.Errorf("expected error") @@ -123,11 +117,9 @@ var _ = Describe("manger.Manager", func() { Expect(m).To(BeNil()) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("expected error")) - - close(done) }) - It("should be able to load Options from cfg.ControllerManagerConfiguration type", func(done Done) { + It("should be able to load Options from cfg.ControllerManagerConfiguration type", func() { duration := metav1.Duration{Duration: 48 * time.Hour} port := int(6090) leaderElect := false @@ -180,11 +172,9 @@ var _ = Describe("manger.Manager", func() { Expect(m.Port).To(Equal(port)) Expect(m.Host).To(Equal("localhost")) Expect(m.CertDir).To(Equal("/certs")) - - close(done) }) - It("should be able to keep Options when cfg.ControllerManagerConfiguration set", func(done Done) { + It("should be able to keep Options when cfg.ControllerManagerConfiguration set", func() { optDuration := time.Duration(2) duration := metav1.Duration{Duration: 48 * time.Hour} port := int(6090) @@ -255,11 +245,9 @@ var _ = Describe("manger.Manager", func() { Expect(m.Port).To(Equal(8080)) Expect(m.Host).To(Equal("example.com")) Expect(m.CertDir).To(Equal("/pki")) - - close(done) }) - It("should lazily initialize a webhook server if needed", func(done Done) { + It("should lazily initialize a webhook server if needed", func() { By("creating a manager with options") m, err := New(cfg, Options{Port: 9440, Host: "foo.com"}) Expect(err).NotTo(HaveOccurred()) @@ -270,11 +258,9 @@ var _ = Describe("manger.Manager", func() { Expect(svr).NotTo(BeNil()) Expect(svr.Port).To(Equal(9440)) Expect(svr.Host).To(Equal("foo.com")) - - close(done) }) - It("should not initialize a webhook server if Options.WebhookServer is set", func(done Done) { + It("should not initialize a webhook server if Options.WebhookServer is set", func() { By("creating a manager with options") m, err := New(cfg, Options{Port: 9441, WebhookServer: &webhook.Server{Port: 9440}}) Expect(err).NotTo(HaveOccurred()) @@ -284,8 +270,6 @@ var _ = Describe("manger.Manager", func() { svr := m.GetWebhookServer() Expect(svr).NotTo(BeNil()) Expect(svr.Port).To(Equal(9440)) - - close(done) }) Context("with leader election enabled", func() { @@ -599,7 +583,7 @@ var _ = Describe("manger.Manager", func() { Describe("Start", func() { var startSuite = func(options Options, callbacks ...func(Manager)) { - It("should Start each Component", func(done Done) { + It("should Start each Component", func() { m, err := New(cfg, options) Expect(err).NotTo(HaveOccurred()) for _, cb := range callbacks { @@ -629,7 +613,6 @@ var _ = Describe("manger.Manager", func() { }() wgRunnableStarted.Wait() - close(done) }) It("should not manipulate the provided config", func() { @@ -651,7 +634,7 @@ var _ = Describe("manger.Manager", func() { Expect(m.GetConfig()).To(Equal(originalCfg)) }) - It("should stop when context is cancelled", func(done Done) { + It("should stop when context is cancelled", func() { m, err := New(cfg, options) Expect(err).NotTo(HaveOccurred()) for _, cb := range callbacks { @@ -660,11 +643,9 @@ var _ = Describe("manger.Manager", func() { ctx, cancel := context.WithCancel(context.Background()) cancel() Expect(m.Start(ctx)).NotTo(HaveOccurred()) - - close(done) }) - It("should return an error if it can't start the cache", func(done Done) { + It("should return an error if it can't start the cache", func() { m, err := New(cfg, options) Expect(err).NotTo(HaveOccurred()) for _, cb := range callbacks { @@ -677,11 +658,9 @@ var _ = Describe("manger.Manager", func() { ctx, cancel := context.WithCancel(context.Background()) defer cancel() Expect(m.Start(ctx)).To(MatchError(ContainSubstring("expected error"))) - - close(done) }) - It("should start the cache before starting anything else", func(done Done) { + It("should start the cache before starting anything else", func() { fakeCache := &startSignalingInformer{Cache: &informertest.FakeInformers{}} options.NewCache = func(_ *rest.Config, _ cache.Options) (cache.Cache, error) { return fakeCache, nil @@ -710,10 +689,9 @@ var _ = Describe("manger.Manager", func() { }() <-runnableWasStarted - close(done) }) - It("should start additional clusters before anything else", func(done Done) { + It("should start additional clusters before anything else", func() { fakeCache := &startSignalingInformer{Cache: &informertest.FakeInformers{}} options.NewCache = func(_ *rest.Config, _ cache.Options) (cache.Cache, error) { return fakeCache, nil @@ -754,10 +732,9 @@ var _ = Describe("manger.Manager", func() { }() <-runnableWasStarted - close(done) }) - It("should return an error if any Components fail to Start", func(done Done) { + It("should return an error if any Components fail to Start", func() { m, err := New(cfg, options) Expect(err).NotTo(HaveOccurred()) for _, cb := range callbacks { @@ -786,11 +763,9 @@ var _ = Describe("manger.Manager", func() { err = m.Start(ctx) Expect(err).ToNot(BeNil()) Expect(err.Error()).To(Equal("expected error")) - - close(done) }) - It("should wait for runnables to stop", func(done Done) { + It("should wait for runnables to stop", func() { m, err := New(cfg, options) Expect(err).NotTo(HaveOccurred()) for _, cb := range callbacks { @@ -840,10 +815,9 @@ var _ = Describe("manger.Manager", func() { cancel() wgManagerRunning.Wait() - close(done) }) - It("should return an error if any Components fail to Start and wait for runnables to stop", func(done Done) { + It("should return an error if any Components fail to Start and wait for runnables to stop", func() { m, err := New(cfg, options) Expect(err).NotTo(HaveOccurred()) for _, cb := range callbacks { @@ -875,11 +849,9 @@ var _ = Describe("manger.Manager", func() { defer cancel() Expect(m.Start(ctx)).To(HaveOccurred()) Expect(runnableDoneCount).To(Equal(2)) - - close(done) }) - It("should refuse to add runnable if stop procedure is already engaged", func(done Done) { + It("should refuse to add runnable if stop procedure is already engaged", func() { m, err := New(cfg, options) Expect(err).NotTo(HaveOccurred()) for _, cb := range callbacks { @@ -907,11 +879,9 @@ var _ = Describe("manger.Manager", func() { defer GinkgoRecover() return nil }))).NotTo(Succeed()) - - close(done) }) - It("should return both runnables and stop errors when both error", func(done Done) { + It("should return both runnables and stop errors when both error", func() { m, err := New(cfg, options) Expect(err).NotTo(HaveOccurred()) for _, cb := range callbacks { @@ -942,11 +912,9 @@ var _ = Describe("manger.Manager", func() { Expect(err.Error()).To(Equal(eMsg)) Expect(errors.Is(err, context.DeadlineExceeded)).To(BeTrue()) Expect(errors.Is(err, runnableError{})).To(BeTrue()) - - close(done) }) - It("should return only stop errors if runnables dont error", func(done Done) { + It("should return only stop errors if runnables dont error", func() { m, err := New(cfg, options) Expect(err).NotTo(HaveOccurred()) for _, cb := range callbacks { @@ -982,11 +950,9 @@ var _ = Describe("manger.Manager", func() { Expect(err.Error()).To(Equal("failed waiting for all runnables to end within grace period of 1ns: context deadline exceeded")) Expect(errors.Is(err, context.DeadlineExceeded)).To(BeTrue()) Expect(errors.Is(err, runnableError{})).ToNot(BeTrue()) - - close(done) }) - It("should return only runnables error if stop doesn't error", func(done Done) { + It("should return only runnables error if stop doesn't error", func() { m, err := New(cfg, options) Expect(err).NotTo(HaveOccurred()) for _, cb := range callbacks { @@ -1002,11 +968,9 @@ var _ = Describe("manger.Manager", func() { Expect(err.Error()).To(Equal("not feeling like that")) Expect(errors.Is(err, context.DeadlineExceeded)).ToNot(BeTrue()) Expect(errors.Is(err, runnableError{})).To(BeTrue()) - - close(done) }) - It("should not wait for runnables if gracefulShutdownTimeout is 0", func(done Done) { + It("should not wait for runnables if gracefulShutdownTimeout is 0", func() { m, err := New(cfg, options) Expect(err).NotTo(HaveOccurred()) for _, cb := range callbacks { @@ -1033,7 +997,6 @@ var _ = Describe("manger.Manager", func() { <-managerStopDone <-runnableStopped - close(done) }) } @@ -1079,7 +1042,7 @@ var _ = Describe("manger.Manager", func() { } }) - It("should stop serving metrics when stop is called", func(done Done) { + It("should stop serving metrics when stop is called", func() { opts.MetricsBindAddress = ":0" m, err := New(cfg, opts) Expect(err).NotTo(HaveOccurred()) @@ -1088,7 +1051,6 @@ var _ = Describe("manger.Manager", func() { go func() { defer GinkgoRecover() Expect(m.Start(ctx)).NotTo(HaveOccurred()) - close(done) }() // Check the metrics started @@ -1106,7 +1068,7 @@ var _ = Describe("manger.Manager", func() { }).ShouldNot(Succeed()) }) - It("should serve metrics endpoint", func(done Done) { + It("should serve metrics endpoint", func() { opts.MetricsBindAddress = ":0" m, err := New(cfg, opts) Expect(err).NotTo(HaveOccurred()) @@ -1116,7 +1078,6 @@ var _ = Describe("manger.Manager", func() { go func() { defer GinkgoRecover() Expect(m.Start(ctx)).NotTo(HaveOccurred()) - close(done) }() metricsEndpoint := fmt.Sprintf("http://%s/metrics", listener.Addr().String()) @@ -1125,7 +1086,7 @@ var _ = Describe("manger.Manager", func() { Expect(resp.StatusCode).To(Equal(200)) }) - It("should not serve anything other than metrics endpoint by default", func(done Done) { + It("should not serve anything other than metrics endpoint by default", func() { opts.MetricsBindAddress = ":0" m, err := New(cfg, opts) Expect(err).NotTo(HaveOccurred()) @@ -1135,7 +1096,6 @@ var _ = Describe("manger.Manager", func() { go func() { defer GinkgoRecover() Expect(m.Start(ctx)).NotTo(HaveOccurred()) - close(done) }() endpoint := fmt.Sprintf("http://%s/should-not-exist", listener.Addr().String()) @@ -1144,7 +1104,7 @@ var _ = Describe("manger.Manager", func() { Expect(resp.StatusCode).To(Equal(404)) }) - It("should serve metrics in its registry", func(done Done) { + It("should serve metrics in its registry", func() { one := prometheus.NewCounter(prometheus.CounterOpts{ Name: "test_one", Help: "test metric for testing", @@ -1162,7 +1122,6 @@ var _ = Describe("manger.Manager", func() { go func() { defer GinkgoRecover() Expect(m.Start(ctx)).NotTo(HaveOccurred()) - close(done) }() metricsEndpoint := fmt.Sprintf("http://%s/metrics", listener.Addr().String()) @@ -1183,7 +1142,7 @@ var _ = Describe("manger.Manager", func() { Expect(ok).To(BeTrue()) }) - It("should serve extra endpoints", func(done Done) { + It("should serve extra endpoints", func() { opts.MetricsBindAddress = ":0" m, err := New(cfg, opts) Expect(err).NotTo(HaveOccurred()) @@ -1204,7 +1163,6 @@ var _ = Describe("manger.Manager", func() { go func() { defer GinkgoRecover() Expect(m.Start(ctx)).NotTo(HaveOccurred()) - close(done) }() endpoint := fmt.Sprintf("http://%s/debug", listener.Addr().String()) @@ -1240,7 +1198,7 @@ var _ = Describe("manger.Manager", func() { } }) - It("should stop serving health probes when stop is called", func(done Done) { + It("should stop serving health probes when stop is called", func() { opts.HealthProbeBindAddress = ":0" m, err := New(cfg, opts) Expect(err).NotTo(HaveOccurred()) @@ -1249,7 +1207,6 @@ var _ = Describe("manger.Manager", func() { go func() { defer GinkgoRecover() Expect(m.Start(ctx)).NotTo(HaveOccurred()) - close(done) }() // Check the health probes started @@ -1267,7 +1224,7 @@ var _ = Describe("manger.Manager", func() { }).ShouldNot(Succeed()) }) - It("should serve readiness endpoint", func(done Done) { + It("should serve readiness endpoint", func() { opts.HealthProbeBindAddress = ":0" m, err := New(cfg, opts) Expect(err).NotTo(HaveOccurred()) @@ -1282,7 +1239,6 @@ var _ = Describe("manger.Manager", func() { go func() { defer GinkgoRecover() Expect(m.Start(ctx)).NotTo(HaveOccurred()) - close(done) }() readinessEndpoint := fmt.Sprint("http://", listener.Addr().String(), defaultReadinessEndpoint) @@ -1318,7 +1274,7 @@ var _ = Describe("manger.Manager", func() { Expect(resp.StatusCode).To(Equal(http.StatusOK)) }) - It("should serve liveness endpoint", func(done Done) { + It("should serve liveness endpoint", func() { opts.HealthProbeBindAddress = ":0" m, err := New(cfg, opts) Expect(err).NotTo(HaveOccurred()) @@ -1333,7 +1289,6 @@ var _ = Describe("manger.Manager", func() { go func() { defer GinkgoRecover() Expect(m.Start(ctx)).NotTo(HaveOccurred()) - close(done) }() livenessEndpoint := fmt.Sprint("http://", listener.Addr().String(), defaultLivenessEndpoint) @@ -1372,7 +1327,7 @@ var _ = Describe("manger.Manager", func() { Describe("Add", func() { It("should immediately start the Component if the Manager has already Started another Component", - func(done Done) { + func() { m, err := New(cfg, Options{}) Expect(err).NotTo(HaveOccurred()) mgr, ok := m.(*controllerManager) @@ -1409,11 +1364,9 @@ var _ = Describe("manger.Manager", func() { }))).To(Succeed()) <-c1 <-c2 - - close(done) }) - It("should immediately start the Component if the Manager has already Started", func(done Done) { + It("should immediately start the Component if the Manager has already Started", func() { m, err := New(cfg, Options{}) Expect(err).NotTo(HaveOccurred()) mgr, ok := m.(*controllerManager) @@ -1440,8 +1393,6 @@ var _ = Describe("manger.Manager", func() { return nil }))).To(Succeed()) <-c1 - - close(done) }) It("should fail if SetFields fails", func() { @@ -1451,7 +1402,7 @@ var _ = Describe("manger.Manager", func() { }) }) Describe("SetFields", func() { - It("should inject field values", func(done Done) { + It("should inject field values", func() { m, err := New(cfg, Options{ NewCache: func(_ *rest.Config, _ cache.Options) (cache.Cache, error) { return &informertest.FakeInformers{}, nil @@ -1543,7 +1494,6 @@ var _ = Describe("manger.Manager", func() { }, }) Expect(err).To(Equal(expected)) - close(done) }) }) diff --git a/pkg/predicate/predicate_test.go b/pkg/predicate/predicate_test.go index f8fb15bc0c..005d170a66 100644 --- a/pkg/predicate/predicate_test.go +++ b/pkg/predicate/predicate_test.go @@ -59,7 +59,7 @@ var _ = Describe("Predicate", func() { }, } - It("should call Create", func(done Done) { + It("should call Create", func() { instance := failingFuncs instance.CreateFunc = func(evt event.CreateEvent) bool { defer GinkgoRecover() @@ -80,10 +80,9 @@ var _ = Describe("Predicate", func() { instance.CreateFunc = nil Expect(instance.Create(evt)).To(BeTrue()) - close(done) }) - It("should call Update", func(done Done) { + It("should call Update", func() { newPod := pod.DeepCopy() newPod.Name = "baz2" newPod.Namespace = "biz2" @@ -111,10 +110,9 @@ var _ = Describe("Predicate", func() { instance.UpdateFunc = nil Expect(instance.Update(evt)).To(BeTrue()) - close(done) }) - It("should call Delete", func(done Done) { + It("should call Delete", func() { instance := failingFuncs instance.DeleteFunc = func(evt event.DeleteEvent) bool { defer GinkgoRecover() @@ -135,10 +133,9 @@ var _ = Describe("Predicate", func() { instance.DeleteFunc = nil Expect(instance.Delete(evt)).To(BeTrue()) - close(done) }) - It("should call Generic", func(done Done) { + It("should call Generic", func() { instance := failingFuncs instance.GenericFunc = func(evt event.GenericEvent) bool { defer GinkgoRecover() @@ -159,7 +156,6 @@ var _ = Describe("Predicate", func() { instance.GenericFunc = nil Expect(instance.Generic(evt)).To(BeTrue()) - close(done) }) }) diff --git a/pkg/source/internal/internal_test.go b/pkg/source/internal/internal_test.go index 7b9ab7a223..9b96c6d46d 100644 --- a/pkg/source/internal/internal_test.go +++ b/pkg/source/internal/internal_test.go @@ -91,17 +91,16 @@ var _ = Describe("Internal", func() { newPod.Labels = map[string]string{"foo": "bar"} }) - It("should create a CreateEvent", func(done Done) { + It("should create a CreateEvent", func() { funcs.CreateFunc = func(evt event.CreateEvent, q workqueue.RateLimitingInterface) { defer GinkgoRecover() Expect(q).To(Equal(instance.Queue)) Expect(evt.Object).To(Equal(pod)) } instance.OnAdd(pod) - close(done) }) - It("should used Predicates to filter CreateEvents", func(done Done) { + It("should used Predicates to filter CreateEvents", func() { instance = internal.EventHandler{ Queue: controllertest.Queue{}, EventHandler: setfuncs, @@ -144,21 +143,17 @@ var _ = Describe("Internal", func() { } instance.OnAdd(pod) Expect(set).To(BeTrue()) - - close(done) }) - It("should not call Create EventHandler if the object is not a runtime.Object", func(done Done) { + It("should not call Create EventHandler if the object is not a runtime.Object", func() { instance.OnAdd(&metav1.ObjectMeta{}) - close(done) }) - It("should not call Create EventHandler if the object does not have metadata", func(done Done) { + It("should not call Create EventHandler if the object does not have metadata", func() { instance.OnAdd(FooRuntimeObject{}) - close(done) }) - It("should create an UpdateEvent", func(done Done) { + It("should create an UpdateEvent", func() { funcs.UpdateFunc = func(evt event.UpdateEvent, q workqueue.RateLimitingInterface) { defer GinkgoRecover() Expect(q).To(Equal(instance.Queue)) @@ -167,10 +162,9 @@ var _ = Describe("Internal", func() { Expect(evt.ObjectNew).To(Equal(newPod)) } instance.OnUpdate(pod, newPod) - close(done) }) - It("should used Predicates to filter UpdateEvents", func(done Done) { + It("should used Predicates to filter UpdateEvents", func() { instance = internal.EventHandler{ Queue: controllertest.Queue{}, EventHandler: setfuncs, @@ -213,23 +207,19 @@ var _ = Describe("Internal", func() { } instance.OnUpdate(pod, newPod) Expect(set).To(BeTrue()) - - close(done) }) - It("should not call Update EventHandler if the object is not a runtime.Object", func(done Done) { + It("should not call Update EventHandler if the object is not a runtime.Object", func() { instance.OnUpdate(&metav1.ObjectMeta{}, &corev1.Pod{}) instance.OnUpdate(&corev1.Pod{}, &metav1.ObjectMeta{}) - close(done) }) - It("should not call Update EventHandler if the object does not have metadata", func(done Done) { + It("should not call Update EventHandler if the object does not have metadata", func() { instance.OnUpdate(FooRuntimeObject{}, &corev1.Pod{}) instance.OnUpdate(&corev1.Pod{}, FooRuntimeObject{}) - close(done) }) - It("should create a DeleteEvent", func(done Done) { + It("should create a DeleteEvent", func() { funcs.DeleteFunc = func(evt event.DeleteEvent, q workqueue.RateLimitingInterface) { defer GinkgoRecover() Expect(q).To(Equal(instance.Queue)) @@ -237,10 +227,9 @@ var _ = Describe("Internal", func() { Expect(evt.Object).To(Equal(pod)) } instance.OnDelete(pod) - close(done) }) - It("should used Predicates to filter DeleteEvents", func(done Done) { + It("should used Predicates to filter DeleteEvents", func() { instance = internal.EventHandler{ Queue: controllertest.Queue{}, EventHandler: setfuncs, @@ -283,21 +272,17 @@ var _ = Describe("Internal", func() { } instance.OnDelete(pod) Expect(set).To(BeTrue()) - - close(done) }) - It("should not call Delete EventHandler if the object is not a runtime.Object", func(done Done) { + It("should not call Delete EventHandler if the object is not a runtime.Object", func() { instance.OnDelete(&metav1.ObjectMeta{}) - close(done) }) - It("should not call Delete EventHandler if the object does not have metadata", func(done Done) { + It("should not call Delete EventHandler if the object does not have metadata", func() { instance.OnDelete(FooRuntimeObject{}) - close(done) }) - It("should create a DeleteEvent from a tombstone", func(done Done) { + It("should create a DeleteEvent from a tombstone", func() { tombstone := cache.DeletedFinalStateUnknown{ Obj: pod, @@ -309,19 +294,16 @@ var _ = Describe("Internal", func() { } instance.OnDelete(tombstone) - close(done) }) - It("should ignore tombstone objects without meta", func(done Done) { + It("should ignore tombstone objects without meta", func() { tombstone := cache.DeletedFinalStateUnknown{Obj: Foo{}} instance.OnDelete(tombstone) - close(done) }) - It("should ignore objects without meta", func(done Done) { + It("should ignore objects without meta", func() { instance.OnAdd(Foo{}) instance.OnUpdate(Foo{}, Foo{}) instance.OnDelete(Foo{}) - close(done) }) }) }) diff --git a/pkg/source/source_integration_test.go b/pkg/source/source_integration_test.go index 087cdbcb4c..f05a154d14 100644 --- a/pkg/source/source_integration_test.go +++ b/pkg/source/source_integration_test.go @@ -44,7 +44,7 @@ var _ = Describe("Source", func() { var ns string count := 0 - BeforeEach(func(done Done) { + BeforeEach(func() { // Create the namespace for the test ns = fmt.Sprintf("controller-source-kindsource-%v", count) count++ @@ -56,8 +56,6 @@ var _ = Describe("Source", func() { q = workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "test") c1 = make(chan interface{}) c2 = make(chan interface{}) - - close(done) }) JustBeforeEach(func() { @@ -68,20 +66,18 @@ var _ = Describe("Source", func() { Expect(inject.CacheInto(icache, instance2)).To(BeTrue()) }) - AfterEach(func(done Done) { + AfterEach(func() { err := clientset.CoreV1().Namespaces().Delete(ctx, ns, metav1.DeleteOptions{}) Expect(err).NotTo(HaveOccurred()) close(c1) close(c2) - - close(done) }) Describe("Kind", func() { Context("for a Deployment resource", func() { obj = &appsv1.Deployment{} - It("should provide Deployment Events", func(done Done) { + It("should provide Deployment Events", func() { var created, updated, deleted *appsv1.Deployment var err error @@ -192,8 +188,6 @@ var _ = Describe("Source", func() { Expect(ok).To(BeTrue(), fmt.Sprintf("expect %T to be %T", evt, event.DeleteEvent{})) deleteEvt.Object.SetResourceVersion("") Expect(deleteEvt.Object).To(Equal(deleted)) - - close(done) }, 5) }) @@ -212,7 +206,7 @@ var _ = Describe("Source", func() { var informerFactory kubeinformers.SharedInformerFactory var stopTest chan struct{} - BeforeEach(func(done Done) { + BeforeEach(func() { stopTest = make(chan struct{}) informerFactory = kubeinformers.NewSharedInformerFactory(clientset, time.Second*30) depInformer = informerFactory.Apps().V1().ReplicaSets().Informer() @@ -239,16 +233,14 @@ var _ = Describe("Source", func() { }, }, } - close(done) }) - AfterEach(func(done Done) { + AfterEach(func() { close(stopTest) - close(done) }) Context("for a ReplicaSet resource", func() { - It("should provide a ReplicaSet CreateEvent", func(done Done) { + It("should provide a ReplicaSet CreateEvent", func() { c := make(chan struct{}) q := workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "test") @@ -282,10 +274,9 @@ var _ = Describe("Source", func() { _, err = clientset.AppsV1().ReplicaSets("default").Create(ctx, rs, metav1.CreateOptions{}) Expect(err).NotTo(HaveOccurred()) <-c - close(done) }, 30) - It("should provide a ReplicaSet UpdateEvent", func(done Done) { + It("should provide a ReplicaSet UpdateEvent", func() { var err error rs, err = clientset.AppsV1().ReplicaSets("default").Get(ctx, rs.Name, metav1.GetOptions{}) Expect(err).NotTo(HaveOccurred()) @@ -325,10 +316,9 @@ var _ = Describe("Source", func() { _, err = clientset.AppsV1().ReplicaSets("default").Update(ctx, rs2, metav1.UpdateOptions{}) Expect(err).NotTo(HaveOccurred()) <-c - close(done) }) - It("should provide a ReplicaSet DeletedEvent", func(done Done) { + It("should provide a ReplicaSet DeletedEvent", func() { c := make(chan struct{}) q := workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "test") @@ -354,7 +344,6 @@ var _ = Describe("Source", func() { err = clientset.AppsV1().ReplicaSets("default").Delete(ctx, rs.Name, metav1.DeleteOptions{}) Expect(err).NotTo(HaveOccurred()) <-c - close(done) }) }) }) diff --git a/pkg/source/source_suite_test.go b/pkg/source/source_suite_test.go index 7be654bf0a..9fd9671cd0 100644 --- a/pkg/source/source_suite_test.go +++ b/pkg/source/source_suite_test.go @@ -44,7 +44,7 @@ var icache cache.Cache var ctx context.Context var cancel context.CancelFunc -var _ = BeforeSuite(func(done Done) { +var _ = BeforeSuite(func() { ctx, cancel = context.WithCancel(context.Background()) logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) @@ -64,13 +64,9 @@ var _ = BeforeSuite(func(done Done) { defer GinkgoRecover() Expect(icache.Start(ctx)).NotTo(HaveOccurred()) }() - - close(done) }, 60) -var _ = AfterSuite(func(done Done) { +var _ = AfterSuite(func() { cancel() Expect(testenv.Stop()).To(Succeed()) - - close(done) }, 5) diff --git a/pkg/source/source_test.go b/pkg/source/source_test.go index 9b0a1c9744..966e1bb95f 100644 --- a/pkg/source/source_test.go +++ b/pkg/source/source_test.go @@ -40,7 +40,7 @@ var _ = Describe("Source", func() { var p *corev1.Pod var ic *informertest.FakeInformers - BeforeEach(func(done Done) { + BeforeEach(func() { ic = &informertest.FakeInformers{} c = make(chan struct{}) p = &corev1.Pod{ @@ -50,11 +50,10 @@ var _ = Describe("Source", func() { }, }, } - close(done) }) Context("for a Pod resource", func() { - It("should provide a Pod CreateEvent", func(done Done) { + It("should provide a Pod CreateEvent", func() { c := make(chan struct{}) p := &corev1.Pod{ Spec: corev1.PodSpec{ @@ -97,10 +96,9 @@ var _ = Describe("Source", func() { i.Add(p) <-c - close(done) }) - It("should provide a Pod UpdateEvent", func(done Done) { + It("should provide a Pod UpdateEvent", func() { p2 := p.DeepCopy() p2.SetLabels(map[string]string{"biz": "baz"}) @@ -141,10 +139,9 @@ var _ = Describe("Source", func() { i.Update(p, p2) <-c - close(done) }) - It("should provide a Pod DeletedEvent", func(done Done) { + It("should provide a Pod DeletedEvent", func() { c := make(chan struct{}) p := &corev1.Pod{ Spec: corev1.PodSpec{ @@ -187,30 +184,25 @@ var _ = Describe("Source", func() { i.Delete(p) <-c - close(done) }) }) - It("should return an error from Start if informers were not injected", func(done Done) { + It("should return an error from Start if informers were not injected", func() { instance := source.Kind{Type: &corev1.Pod{}} err := instance.Start(ctx, nil, nil) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("must call CacheInto on Kind before calling Start")) - - close(done) }) - It("should return an error from Start if a type was not provided", func(done Done) { + It("should return an error from Start if a type was not provided", func() { instance := source.Kind{} Expect(instance.InjectCache(&informertest.FakeInformers{})).To(Succeed()) err := instance.Start(ctx, nil, nil) Expect(err).To(HaveOccurred()) Expect(err.Error()).To(ContainSubstring("must specify Kind.Type")) - - close(done) }) - It("should return an error if syncing fails", func(done Done) { + It("should return an error if syncing fails", func() { instance := source.Kind{Type: &corev1.Pod{}} f := false Expect(instance.InjectCache(&informertest.FakeInformers{Synced: &f})).To(Succeed()) @@ -219,12 +211,10 @@ var _ = Describe("Source", func() { Expect(err).To(HaveOccurred()) Expect(err.Error()).To(Equal("cache did not sync")) - close(done) - }) Context("for a Kind not in the cache", func() { - It("should return an error when WaitForSync is called", func(done Done) { + It("should return an error when WaitForSync is called", func() { ic.Error = fmt.Errorf("test error") q := workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "test") @@ -235,8 +225,6 @@ var _ = Describe("Source", func() { err := instance.Start(ctx, handler.Funcs{}, q) Expect(err).NotTo(HaveOccurred()) Expect(instance.WaitForSync(context.Background())).To(HaveOccurred()) - - close(done) }) }) }) @@ -249,7 +237,7 @@ var _ = Describe("Source", func() { Expect(injected).To(BeFalse()) }) - It("should return an error if syncing fails", func(done Done) { + It("should return an error if syncing fails", func() { f := false instance := source.NewKindWithCache(&corev1.Pod{}, &informertest.FakeInformers{Synced: &f}) Expect(instance.Start(context.Background(), nil, nil)).NotTo(HaveOccurred()) @@ -257,13 +245,11 @@ var _ = Describe("Source", func() { Expect(err).To(HaveOccurred()) Expect(err.Error()).To(Equal("cache did not sync")) - close(done) - }) }) Describe("Func", func() { - It("should be called from Start", func(done Done) { + It("should be called from Start", func() { run := false instance := source.Func(func( context.Context, @@ -283,8 +269,6 @@ var _ = Describe("Source", func() { return expected }) Expect(instance.Start(ctx, nil, nil)).To(Equal(expected)) - - close(done) }) }) @@ -304,7 +288,7 @@ var _ = Describe("Source", func() { }) Context("for a source", func() { - It("should provide a GenericEvent", func(done Done) { + It("should provide a GenericEvent", func() { ch := make(chan event.GenericEvent) c := make(chan struct{}) p := &corev1.Pod{ @@ -353,9 +337,8 @@ var _ = Describe("Source", func() { ch <- invalidEvt ch <- evt <-c - close(done) }) - It("should get pending events processed once channel unblocked", func(done Done) { + It("should get pending events processed once channel unblocked", func() { ch := make(chan event.GenericEvent) unblock := make(chan struct{}) processed := make(chan struct{}) @@ -412,10 +395,8 @@ var _ = Describe("Source", func() { // Validate all of the events have been processed. Expect(eventCount).To(Equal(3)) - - close(done) }) - It("should be able to cope with events in the channel before the source is started", func(done Done) { + It("should be able to cope with events in the channel before the source is started", func() { ch := make(chan event.GenericEvent, 1) processed := make(chan struct{}) evt := event.GenericEvent{} @@ -449,8 +430,6 @@ var _ = Describe("Source", func() { Expect(err).NotTo(HaveOccurred()) <-processed - - close(done) }) It("should stop when the source channel is closed", func() { q := workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "test") @@ -496,24 +475,22 @@ var _ = Describe("Source", func() { Eventually(processed).Should(Receive()) Consistently(processed).ShouldNot(Receive()) }) - It("should get error if no source specified", func(done Done) { + It("should get error if no source specified", func() { q := workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "test") instance := &source.Channel{ /*no source specified*/ } Expect(inject.StopChannelInto(ctx.Done(), instance)).To(BeTrue()) err := instance.Start(ctx, handler.Funcs{}, q) Expect(err).To(Equal(fmt.Errorf("must specify Channel.Source"))) - close(done) }) - It("should get error if no stop channel injected", func(done Done) { + It("should get error if no stop channel injected", func() { q := workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "test") instance := &source.Channel{Source: ch} err := instance.Start(ctx, handler.Funcs{}, q) Expect(err).To(Equal(fmt.Errorf("must call InjectStop on Channel before calling Start"))) - close(done) }) }) Context("for multi sources (handlers)", func() { - It("should provide GenericEvents for all handlers", func(done Done) { + It("should provide GenericEvents for all handlers", func() { ch := make(chan event.GenericEvent) p := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "bar"}, @@ -581,7 +558,6 @@ var _ = Describe("Source", func() { // Validate the two handlers received same event Expect(resEvent1).To(Equal(resEvent2)) - close(done) }) }) }) diff --git a/pkg/webhook/admission/admission_suite_test.go b/pkg/webhook/admission/admission_suite_test.go index 339c7d83c3..3648aa45b7 100644 --- a/pkg/webhook/admission/admission_suite_test.go +++ b/pkg/webhook/admission/admission_suite_test.go @@ -33,8 +33,6 @@ func TestAdmissionWebhook(t *testing.T) { RunSpecsWithDefaultAndCustomReporters(t, suiteName, []Reporter{printer.NewlineReporter{}, printer.NewProwReporter(suiteName)}) } -var _ = BeforeSuite(func(done Done) { +var _ = BeforeSuite(func() { logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) - - close(done) }, 60) diff --git a/pkg/webhook/authentication/authentication_suite_test.go b/pkg/webhook/authentication/authentication_suite_test.go index 0988f81285..b993d1ef80 100644 --- a/pkg/webhook/authentication/authentication_suite_test.go +++ b/pkg/webhook/authentication/authentication_suite_test.go @@ -33,8 +33,6 @@ func TestAuthenticationWebhook(t *testing.T) { RunSpecsWithDefaultAndCustomReporters(t, suiteName, []Reporter{printer.NewlineReporter{}, printer.NewProwReporter(suiteName)}) } -var _ = BeforeSuite(func(done Done) { +var _ = BeforeSuite(func() { logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) - - close(done) }, 60) diff --git a/pkg/webhook/conversion/conversion_suite_test.go b/pkg/webhook/conversion/conversion_suite_test.go index 76bbf505ff..bb3798747c 100644 --- a/pkg/webhook/conversion/conversion_suite_test.go +++ b/pkg/webhook/conversion/conversion_suite_test.go @@ -32,8 +32,6 @@ func TestConversionWebhook(t *testing.T) { RunSpecsWithDefaultAndCustomReporters(t, suiteName, []Reporter{printer.NewlineReporter{}, printer.NewProwReporter(suiteName)}) } -var _ = BeforeSuite(func(done Done) { +var _ = BeforeSuite(func() { logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) - - close(done) }) diff --git a/pkg/webhook/webhook_integration_test.go b/pkg/webhook/webhook_integration_test.go index 9cf8c7e0a3..386816b510 100644 --- a/pkg/webhook/webhook_integration_test.go +++ b/pkg/webhook/webhook_integration_test.go @@ -80,7 +80,7 @@ var _ = Describe("Webhook", func() { } }) Context("when running a webhook server with a manager", func() { - It("should reject create request for webhook that rejects all requests", func(done Done) { + It("should reject create request for webhook that rejects all requests", func() { m, err := manager.New(cfg, manager.Options{ Port: testenv.WebhookInstallOptions.LocalServingPort, Host: testenv.WebhookInstallOptions.LocalServingHost, @@ -102,9 +102,8 @@ var _ = Describe("Webhook", func() { }, 1*time.Second).Should(BeTrue()) cancel() - close(done) }) - It("should reject create request for multi-webhook that rejects all requests", func(done Done) { + It("should reject create request for multi-webhook that rejects all requests", func() { m, err := manager.New(cfg, manager.Options{ Port: testenv.WebhookInstallOptions.LocalServingPort, Host: testenv.WebhookInstallOptions.LocalServingHost, @@ -126,11 +125,10 @@ var _ = Describe("Webhook", func() { }, 1*time.Second).Should(BeTrue()) cancel() - close(done) }) }) Context("when running a webhook server without a manager", func() { - It("should reject create request for webhook that rejects all requests", func(done Done) { + It("should reject create request for webhook that rejects all requests", func() { server := webhook.Server{ Port: testenv.WebhookInstallOptions.LocalServingPort, Host: testenv.WebhookInstallOptions.LocalServingHost, @@ -150,11 +148,10 @@ var _ = Describe("Webhook", func() { }, 1*time.Second).Should(BeTrue()) cancel() - close(done) }) }) Context("when running a standalone webhook", func() { - It("should reject create request for webhook that rejects all requests", func(done Done) { + It("should reject create request for webhook that rejects all requests", func() { ctx, cancel := context.WithCancel(context.Background()) By("generating the TLS config") @@ -206,7 +203,6 @@ var _ = Describe("Webhook", func() { }, 1*time.Second).Should(BeTrue()) cancel() - close(done) }) }) }) diff --git a/pkg/webhook/webhook_suite_test.go b/pkg/webhook/webhook_suite_test.go index fb2b02f195..799749d4c7 100644 --- a/pkg/webhook/webhook_suite_test.go +++ b/pkg/webhook/webhook_suite_test.go @@ -42,7 +42,7 @@ func TestSource(t *testing.T) { var testenv *envtest.Environment var cfg *rest.Config -var _ = BeforeSuite(func(done Done) { +var _ = BeforeSuite(func() { logf.SetLogger(zap.New(zap.WriteTo(GinkgoWriter), zap.UseDevMode(true))) testenv = &envtest.Environment{} @@ -51,7 +51,6 @@ var _ = BeforeSuite(func(done Done) { var err error cfg, err = testenv.Start() Expect(err).NotTo(HaveOccurred()) - close(done) }, 60) var _ = AfterSuite(func() {