From 192b4ba2eec0071ffc3792852379e4d4bc451b54 Mon Sep 17 00:00:00 2001
From: Dyson Simmons <dysonsimmons@gmail.com>
Date: Fri, 17 Jul 2020 14:14:39 +0100
Subject: [PATCH] Improve unit and integration tests

Don't set verbose in 'make test' as envTest is really noisey in the
integration tests.

Also fixes an issue with the manager not being shut down cleanly at the
end of integration tests which would result in error log output:
https://github.com/kubernetes-sigs/controller-runtime/issues/904
---
 Makefile                                        |  2 +-
 controllers/rbac/integration/suite_test.go      | 17 ++++++++++++-----
 .../workloads/integration/integration_test.go   | 12 ++++++++++--
 controllers/workloads/integration/suite_test.go | 17 ++++++++++++-----
 4 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/Makefile b/Makefile
index e0a527e7..ce8988da 100644
--- a/Makefile
+++ b/Makefile
@@ -30,7 +30,7 @@ bin/%: generate fmt vet
 
 # go get -u github.com/onsi/ginkgo/ginkgo
 test:
-	ginkgo -v -r
+	ginkgo -r
 
 fmt:
 	go fmt ./...
diff --git a/controllers/rbac/integration/suite_test.go b/controllers/rbac/integration/suite_test.go
index 4d9bdfb1..10b8c242 100644
--- a/controllers/rbac/integration/suite_test.go
+++ b/controllers/rbac/integration/suite_test.go
@@ -24,6 +24,8 @@ import (
 var (
 	mgr     ctrl.Manager
 	testEnv *envtest.Environment
+
+	finished = make(chan struct{})
 )
 
 func TestSuite(t *testing.T) {
@@ -82,18 +84,23 @@ var _ = BeforeSuite(func(done Done) {
 	}).SetupWithManager(mgr)
 	Expect(err).ToNot(HaveOccurred())
 
+	go func() {
+		<-ctrl.SetupSignalHandler()
+		close(finished)
+	}()
+
 	go func() {
 		defer GinkgoRecover()
-		err = mgr.Start(ctrl.SetupSignalHandler())
+		err = mgr.Start(finished)
 		Expect(err).ToNot(HaveOccurred(), "failed to run manager")
+		gexec.KillAndWait(4 * time.Second)
+		err := testEnv.Stop()
+		Expect(err).ToNot(HaveOccurred())
 	}()
 
 	close(done)
 }, 60)
 
 var _ = AfterSuite(func() {
-	By("tearing down the test environment")
-	gexec.KillAndWait(5 * time.Second)
-	err := testEnv.Stop()
-	Expect(err).ToNot(HaveOccurred())
+	close(finished)
 })
diff --git a/controllers/workloads/integration/integration_test.go b/controllers/workloads/integration/integration_test.go
index 0e132121..1e79939e 100644
--- a/controllers/workloads/integration/integration_test.go
+++ b/controllers/workloads/integration/integration_test.go
@@ -410,7 +410,12 @@ var _ = Describe("Console", func() {
 			}).ShouldNot(BeNil(),
 				"the console status should be defined")
 
-			Expect(updatedCsl.Status.ExpiryTime).NotTo(BeNil(), "the console expiry time should be set")
+			Eventually(func() *metav1.Time {
+				mgr.GetClient().Get(context.TODO(), identifier, updatedCsl)
+				return updatedCsl.Status.ExpiryTime
+			}).ShouldNot(BeNil(),
+				"the console expiry time should be defined")
+
 			Expect(
 				updatedCsl.Status.ExpiryTime.Time.After(time.Now())).To(BeTrue(),
 				"the console expiry time should be after now()",
@@ -462,7 +467,10 @@ var _ = Describe("Console", func() {
 			}).ShouldNot(BeNil(),
 				"the console status should be defined")
 
-			Expect(updatedCsl.Stopped()).To(BeTrue())
+			Eventually(func() bool {
+				mgr.GetClient().Get(context.TODO(), identifier, updatedCsl)
+				return updatedCsl.Stopped()
+			}).Should(BeTrue())
 			Expect(
 				updatedCsl.Status.CompletionTime.Time.Before(time.Now())).To(BeTrue(),
 				"the console completion time should be before now()",
diff --git a/controllers/workloads/integration/suite_test.go b/controllers/workloads/integration/suite_test.go
index 69a599c8..eb08f1d0 100644
--- a/controllers/workloads/integration/suite_test.go
+++ b/controllers/workloads/integration/suite_test.go
@@ -25,6 +25,8 @@ import (
 var (
 	mgr     ctrl.Manager
 	testEnv *envtest.Environment
+
+	finished = make(chan struct{})
 )
 
 func TestSuite(t *testing.T) {
@@ -69,18 +71,23 @@ var _ = BeforeSuite(func(done Done) {
 	}).SetupWithManager(context.TODO(), mgr)
 	Expect(err).ToNot(HaveOccurred())
 
+	go func() {
+		<-ctrl.SetupSignalHandler()
+		close(finished)
+	}()
+
 	go func() {
 		defer GinkgoRecover()
-		err = mgr.Start(ctrl.SetupSignalHandler())
+		err = mgr.Start(finished)
 		Expect(err).ToNot(HaveOccurred(), "failed to run manager")
+		gexec.KillAndWait(4 * time.Second)
+		err := testEnv.Stop()
+		Expect(err).ToNot(HaveOccurred())
 	}()
 
 	close(done)
 }, 60)
 
 var _ = AfterSuite(func() {
-	By("tearing down the test environment")
-	gexec.KillAndWait(5 * time.Second)
-	err := testEnv.Stop()
-	Expect(err).ToNot(HaveOccurred())
+	close(finished)
 })