Fix deletion policy tests

The deletion policy was not being set at all for tests. Why? Because
BeforeEach nodes run BEFORE the JustBeforeEach. Therefore, the
BeforeEach was setting the deletion policy in the spec, and right after
the JustAfterEach in the top container was assigning a "zero" value for
the topology type. This effectively "overrode" the value set for
deletion policy.

This was probably passing because Create/Delete were called one after
the other, and sometimes the deletion behaviour simply skipped because
the object was not found, since it was "too soon" and not returned by
the informer cache.

These tests should have been rightfuly red. The were flaking to green.

Author: Zerpet

controllers/federation_controller_test.go

@@ -233,26 +233,31 @@ var _ = Describe("federation-controller", func() {
 	When("the Federation has DeletionPolicy set to retain", func() {
 		BeforeEach(func() {
 			federationName = "federation-with-retain-policy"
-			federation.Spec.DeletionPolicy = "retain"
 			fakeRabbitMQClient.DeleteFederationUpstreamReturns(&http.Response{
 				Status:     "200 OK",
 				StatusCode: http.StatusOK,
 			}, nil)
+			fakeRabbitMQClient.PutFederationUpstreamReturns(&http.Response{StatusCode: http.StatusCreated, Status: "201 Created"}, nil)
 		})
 
 		It("deletes the k8s resource but preserves the federation in RabbitMQ server", func() {
+			federation.Spec.DeletionPolicy = "retain"
 			Expect(k8sClient.Create(ctx, &federation)).To(Succeed())
-			Expect(k8sClient.Delete(ctx, &federation)).To(Succeed())
+			Eventually(fakeRabbitMQClient.PutFederationUpstreamCallCount).
+				WithPolling(time.Second).
+				Within(time.Second*3).
+				Should(BeNumerically(">=", 1), "Expected to call RMQ API to create federation")
 
+			Expect(k8sClient.Delete(ctx, &federation)).To(Succeed())
 			Eventually(func() bool {
 				err := k8sClient.Get(ctx, types.NamespacedName{Name: federation.Name, Namespace: federation.Namespace}, &federation)
 				return apierrors.IsNotFound(err)
 			}).
 				Within(statusEventsUpdateTimeout).
 				WithPolling(time.Second).
-				Should(BeTrue())
+				Should(BeTrue(), "Federation should not be found")
 
-			Expect(fakeRabbitMQClient.DeleteFederationUpstreamCallCount()).To(Equal(0))
+			Expect(fakeRabbitMQClient.DeleteFederationUpstreamCallCount()).To(Equal(0), "Expected Federation to be deleted and no calls to RMQ API")
 		})
 	})
 })

controllers/queue_controller_test.go

@@ -229,26 +229,30 @@ var _ = Describe("queue-controller", func() {
 	When("the Queue has DeletionPolicy set to retain", func() {
 		BeforeEach(func() {
 			queueName = "queue-with-retain-policy"
-			queue.Spec.DeletionPolicy = "retain"
 			fakeRabbitMQClient.DeleteQueueReturns(&http.Response{
 				Status:     "200 OK",
 				StatusCode: http.StatusOK,
 			}, nil)
 		})
 
 		It("deletes the k8s resource but preserves the queue in RabbitMQ server", func() {
+			queue.Spec.DeletionPolicy = "retain"
 			Expect(k8sClient.Create(ctx, &queue)).To(Succeed())
-			Expect(k8sClient.Delete(ctx, &queue)).To(Succeed())
+			Eventually(fakeRabbitMQClient.DeclareQueueCallCount).
+				WithPolling(time.Second).
+				Within(time.Second*3).
+				Should(BeNumerically(">=", 1), "Expected to call RMQ API to declare queue")
 
+			Expect(k8sClient.Delete(ctx, &queue)).To(Succeed())
 			Eventually(func() bool {
 				err := k8sClient.Get(ctx, types.NamespacedName{Name: queue.Name, Namespace: queue.Namespace}, &queue)
 				return apierrors.IsNotFound(err)
 			}).
 				Within(statusEventsUpdateTimeout).
 				WithPolling(time.Second).
-				Should(BeTrue())
+				Should(BeTrue(), "Queue should not be found")
 
-			Expect(fakeRabbitMQClient.DeleteQueueCallCount()).To(Equal(0))
+			Expect(fakeRabbitMQClient.DeleteQueueCallCount()).To(Equal(0), "Expected to delete queue and no calls to RMQ API")
 		})
 	})
 })

controllers/shovel_controller_test.go

@@ -290,18 +290,24 @@ var _ = Describe("shovel-controller", func() {
 			})
 		})
 	})
+
 	When("the Shovel has DeletionPolicy set to retain", func() {
 		BeforeEach(func() {
 			shovelName = "shovel-with-retain-policy"
-			shovel.Spec.DeletionPolicy = "retain"
 			fakeRabbitMQClient.DeleteShovelReturns(&http.Response{
 				Status:     "200 OK",
 				StatusCode: http.StatusOK,
 			}, nil)
+			fakeRabbitMQClient.DeclareShovelReturns(&http.Response{StatusCode: http.StatusCreated, Status: "201 Created"}, nil)
 		})
 
 		It("deletes the k8s resource but preserves the shovel in RabbitMQ server", func() {
+			shovel.Spec.DeletionPolicy = "retain"
 			Expect(k8sClient.Create(ctx, &shovel)).To(Succeed())
+			Eventually(fakeRabbitMQClient.DeclareShovelCallCount).
+				WithPolling(time.Second).
+				Within(time.Second*3).
+				Should(BeNumerically(">=", 1), "Expected to call RMQ API to declare shovel")
 			Expect(k8sClient.Delete(ctx, &shovel)).To(Succeed())
 
 			Eventually(func() bool {
@@ -310,9 +316,9 @@ var _ = Describe("shovel-controller", func() {
 			}).
 				Within(statusEventsUpdateTimeout).
 				WithPolling(time.Second).
-				Should(BeTrue())
+				Should(BeTrue(), "Expected shovel to not be found")
 
-			Expect(fakeRabbitMQClient.DeleteShovelCallCount()).To(Equal(0))
+			Expect(fakeRabbitMQClient.DeleteShovelCallCount()).To(Equal(0), "Shovel object should have been deleted and no calls to RMQ API")
 		})
 	})
 })

controllers/vhost_controller_test.go

@@ -369,26 +369,31 @@ var _ = Describe("vhost-controller", func() {
 	When("the Vhost has DeletionPolicy set to retain", func() {
 		BeforeEach(func() {
 			vhostName = "vhost-with-retain-policy"
-			vhost.Spec.DeletionPolicy = "retain"
 			fakeRabbitMQClient.DeleteVhostReturns(&http.Response{
 				Status:     "200 OK",
 				StatusCode: http.StatusOK,
 			}, nil)
+			fakeRabbitMQClient.PutVhostReturns(&http.Response{StatusCode: http.StatusCreated, Status: "201 Created"}, nil)
 		})
 
 		It("deletes the k8s resource but preserves the vhost in RabbitMQ server", func() {
+			vhost.Spec.DeletionPolicy = "retain"
 			Expect(k8sClient.Create(ctx, &vhost)).To(Succeed())
-			Expect(k8sClient.Delete(ctx, &vhost)).To(Succeed())
+			Eventually(fakeRabbitMQClient.PutVhostCallCount).
+				WithPolling(time.Second).
+				Within(time.Second*3).
+				Should(BeNumerically(">=", 1), "Expected to call RMQ API to create vhost")
 
+			Expect(k8sClient.Delete(ctx, &vhost)).To(Succeed())
 			Eventually(func() bool {
 				err := k8sClient.Get(ctx, types.NamespacedName{Name: vhost.Name, Namespace: vhost.Namespace}, &vhost)
 				return apierrors.IsNotFound(err)
 			}).
 				Within(statusEventsUpdateTimeout).
 				WithPolling(time.Second).
-				Should(BeTrue())
+				Should(BeTrue(), "vhost should not be found")
 
-			Expect(fakeRabbitMQClient.DeleteVhostCallCount()).To(Equal(0))
+			Expect(fakeRabbitMQClient.DeleteVhostCallCount()).To(Equal(0), "Expected vhost to be deleted and no calls to RMQ API")
 		})
 	})
 })