Impove deploy-image controller-test.go

* To(Not()) → NotTo()
* Use default eventually timeouts
* Eventually(func() error) → Eventually(func(g Gomega))
* Use Gomega assertions for the condition check, which is more robust

And then use g.Expect inside those Eventually() functions

Author: mogsie

docs/book/src/getting-started/testdata/project/internal/controller/memcached_controller_test.go

@@ -83,30 +83,24 @@ var _ = Describe("Memcached Controller", func() {
 			})
 			Expect(err).NotTo(HaveOccurred())
 			By("Checking if Deployment was successfully created in the reconciliation")
-			Eventually(func() error {
+			Eventually(func(g Gomega) {
 				found := &appsv1.Deployment{}
-				return k8sClient.Get(ctx, typeNamespacedName, found)
-			}, time.Minute, time.Second).Should(Succeed())
+				g.Expect(k8sClient.Get(ctx, typeNamespacedName, found)).To(Succeed())
+			}).Should(Succeed())
 
 			By("Checking the latest Status Condition added to the Memcached instance")
-			Eventually(func() error {
-				if memcached.Status.Conditions != nil &&
-					len(memcached.Status.Conditions) != 0 {
-					latestStatusCondition := memcached.Status.Conditions[len(memcached.Status.Conditions)-1]
-					expectedLatestStatusCondition := metav1.Condition{
-						Type:   typeAvailableMemcached,
-						Status: metav1.ConditionTrue,
-						Reason: "Reconciling",
-						Message: fmt.Sprintf(
-							"Deployment for custom resource (%s) with %d replicas created successfully",
-							memcached.Name,
-							memcached.Spec.Size),
-					}
-					if latestStatusCondition != expectedLatestStatusCondition {
-						return fmt.Errorf("The latest status condition added to the Memcached instance is not as expected")
-					}
+			Eventually(func(g Gomega) {
+				reconcilingTrue := metav1.Condition{
+					Type:   typeAvailableMemcached,
+					Status: metav1.ConditionTrue,
+					Reason: "Reconciling",
+					Message: fmt.Sprintf(
+						"Deployment for custom resource (%s) with %d replicas created successfully",
+						memcached.Name,
+						memcached.Spec.Size),
 				}
-				return nil
+				g.Expect(memcached.Status.Conditions).To(
+					ContainElement(Equal(reconcilingTrue)))
 			}, time.Minute, time.Second).Should(Succeed())
 		})
 	})

hack/docs/internal/getting-started/generate_getting_started.go

@@ -78,30 +78,24 @@ func (sp *Sample) updateControllerTest() {
 		`// TODO(user): Add more specific assertions depending on your controller's reconciliation logic.
 			// Example: If you expect a certain status condition after reconciliation, verify it here.`,
 		`By("Checking if Deployment was successfully created in the reconciliation")
-			Eventually(func() error {
+			Eventually(func(g Gomega) {
 				found := &appsv1.Deployment{}
-				return k8sClient.Get(ctx, typeNamespacedName, found)
-			}, time.Minute, time.Second).Should(Succeed())
+				g.Expect(k8sClient.Get(ctx, typeNamespacedName, found)).To(Succeed())
+			}).Should(Succeed())
 
 			By("Checking the latest Status Condition added to the Memcached instance")
-			Eventually(func() error {
-				if memcached.Status.Conditions != nil &&
-					len(memcached.Status.Conditions) != 0 {
-					latestStatusCondition := memcached.Status.Conditions[len(memcached.Status.Conditions)-1]
-					expectedLatestStatusCondition := metav1.Condition{
-						Type:   typeAvailableMemcached,
-						Status: metav1.ConditionTrue,
-						Reason: "Reconciling",
-						Message: fmt.Sprintf(
-							"Deployment for custom resource (%s) with %d replicas created successfully",
-							memcached.Name,
-							memcached.Spec.Size),
-					}
-					if latestStatusCondition != expectedLatestStatusCondition {
-						return fmt.Errorf("The latest status condition added to the Memcached instance is not as expected")
-					}
+			Eventually(func(g Gomega) {
+				reconcilingTrue := metav1.Condition{
+					Type:   typeAvailableMemcached,
+					Status: metav1.ConditionTrue,
+					Reason: "Reconciling",
+					Message: fmt.Sprintf(
+						"Deployment for custom resource (%s) with %d replicas created successfully",
+						memcached.Name,
+						memcached.Spec.Size),
 				}
-				return nil
+				g.Expect(memcached.Status.Conditions).To(
+					ContainElement(Equal(reconcilingTrue)))
 			}, time.Minute, time.Second).Should(Succeed())`,
 	)
 	hackutils.CheckError("add spec apis", err)

pkg/plugins/golang/deploy-image/v1alpha1/scaffolds/internal/templates/controllers/controller-test.go

@@ -105,14 +105,17 @@ var _ = Describe("{{ .Resource.Kind }} controller", func() {
 		}
 		{{ lower .Resource.Kind }} := &{{ .Resource.ImportAlias }}.{{ .Resource.Kind }}{}
 
+		SetDefaultEventuallyTimeout(2 * time.Minute)
+		SetDefaultEventuallyPollingInterval(time.Second)
+
 		BeforeEach(func() {
 			By("Creating the Namespace to perform the tests")
 			err := k8sClient.Create(ctx, namespace);
-			Expect(err).To(Not(HaveOccurred()))
+			Expect(err).NotTo(HaveOccurred())
 
 			By("Setting the Image ENV VAR which stores the Operand image")
 			err= os.Setenv("{{ upper .Resource.Kind }}_IMAGE", "example.com/image:test")
-			Expect(err).To(Not(HaveOccurred()))
+			Expect(err).NotTo(HaveOccurred())
 
 			By("creating the custom resource for the Kind {{ .Resource.Kind }}")
 			err = k8sClient.Get(ctx, typeNamespacedName, {{ lower .Resource.Kind }})
@@ -133,19 +136,19 @@ var _ = Describe("{{ .Resource.Kind }} controller", func() {
 				}
 				
 				err = k8sClient.Create(ctx, {{ lower .Resource.Kind }})
-				Expect(err).To(Not(HaveOccurred()))
+				Expect(err).NotTo(HaveOccurred())
 			}
 		})
 
 		AfterEach(func() {
 			By("removing the custom resource for the Kind {{ .Resource.Kind }}")
 			found := &{{ .Resource.ImportAlias }}.{{ .Resource.Kind }}{}
 			err := k8sClient.Get(ctx, typeNamespacedName, found)
-			Expect(err).To(Not(HaveOccurred()))
+			Expect(err).NotTo(HaveOccurred())
 
-			Eventually(func() error {
-				return k8sClient.Delete(context.TODO(), found)
-			}, 2*time.Minute, time.Second).Should(Succeed())
+			Eventually(func(g Gomega) {
+				g.Expect(k8sClient.Delete(context.TODO(), found)).To(Succeed())
+			}).Should(Succeed())
 
 			// TODO(user): Attention if you improve this code by adding other context test you MUST
 			// be aware of the current delete namespace limitations. 
@@ -159,10 +162,10 @@ var _ = Describe("{{ .Resource.Kind }} controller", func() {
 
 		It("should successfully reconcile a custom resource for {{ .Resource.Kind }}", func() {
 			By("Checking if the custom resource was successfully created")
-			Eventually(func() error {
+			Eventually(func(g Gomega) {
 				found := &{{ .Resource.ImportAlias }}.{{ .Resource.Kind }}{}
-				return k8sClient.Get(ctx, typeNamespacedName, found)
-			}, time.Minute, time.Second).Should(Succeed())
+				Expect(k8sClient.Get(ctx, typeNamespacedName, found)).To(Succeed())
+			}).Should(Succeed())
 
 			By("Reconciling the custom resource created")
 			{{ lower .Resource.Kind }}Reconciler := &{{ .Resource.Kind }}Reconciler{
@@ -173,34 +176,28 @@ var _ = Describe("{{ .Resource.Kind }} controller", func() {
 			_, err := {{ lower .Resource.Kind }}Reconciler.Reconcile(ctx, reconcile.Request{
 				NamespacedName: typeNamespacedName,
 			})
-			Expect(err).To(Not(HaveOccurred()))
+			Expect(err).NotTo(HaveOccurred())
 
 			By("Checking if Deployment was successfully created in the reconciliation")
-			Eventually(func() error {
+			Eventually(func(g Gomega) {
 				found := &appsv1.Deployment{}
-				return k8sClient.Get(ctx, typeNamespacedName, found)
-			}, time.Minute, time.Second).Should(Succeed())
+				Expect(k8sClient.Get(ctx, typeNamespacedName, found)).To(Succeed())
+			}).Should(Succeed())
 
 			By("Checking the latest Status Condition added to the {{ .Resource.Kind }} instance")
-			Eventually(func() error {
-				if {{ lower .Resource.Kind }}.Status.Conditions != nil &&
-					len({{ lower .Resource.Kind }}.Status.Conditions) != 0 {
-					latestStatusCondition := {{ lower .Resource.Kind }}.Status.Conditions[len({{ lower .Resource.Kind }}.Status.Conditions)-1]
-					expectedLatestStatusCondition := metav1.Condition{
-						Type:    typeAvailable{{ .Resource.Kind }},
-						Status:  metav1.ConditionTrue,
-						Reason:  "Reconciling",
-						Message: fmt.Sprintf(
-							"Deployment for custom resource (%s) with %d replicas created successfully", 
-							{{ lower .Resource.Kind }}.Name,
-							{{ lower .Resource.Kind }}.Spec.Size),
-					}
-					if latestStatusCondition != expectedLatestStatusCondition {
-						return fmt.Errorf("The latest status condition added to the {{ .Resource.Kind }} instance is not as expected")
-					}
+			Eventually(func(g Gomega) {
+				reconcilingTrue := metav1.Condition{
+					Type:    typeAvailable{{ .Resource.Kind }},
+					Status:  metav1.ConditionTrue,
+					Reason:  "Reconciling",
+					Message: fmt.Sprintf(
+						"Deployment for custom resource (%s) with %d replicas created successfully",
+						{{ lower .Resource.Kind }}.Name,
+						{{ lower .Resource.Kind }}.Spec.Size),
 				}
-				return nil
-			}, time.Minute, time.Second).Should(Succeed())
+				g.Expect({{ lower .Resource.Kind }}.Status.Conditions).To(
+					ContainElement(Equal(reconcilingTrue)))
+			}).Should(Succeed())
 		})
 	})
 })

testdata/project-v4-multigroup/internal/controller/example.com/busybox_controller_test.go

@@ -55,14 +55,17 @@ var _ = Describe("Busybox controller", func() {
 		}
 		busybox := &examplecomv1alpha1.Busybox{}
 
+		SetDefaultEventuallyTimeout(2 * time.Minute)
+		SetDefaultEventuallyPollingInterval(time.Second)
+
 		BeforeEach(func() {
 			By("Creating the Namespace to perform the tests")
 			err := k8sClient.Create(ctx, namespace)
-			Expect(err).To(Not(HaveOccurred()))
+			Expect(err).NotTo(HaveOccurred())
 
 			By("Setting the Image ENV VAR which stores the Operand image")
 			err = os.Setenv("BUSYBOX_IMAGE", "example.com/image:test")
-			Expect(err).To(Not(HaveOccurred()))
+			Expect(err).NotTo(HaveOccurred())
 
 			By("creating the custom resource for the Kind Busybox")
 			err = k8sClient.Get(ctx, typeNamespacedName, busybox)
@@ -80,19 +83,19 @@ var _ = Describe("Busybox controller", func() {
 				}
 
 				err = k8sClient.Create(ctx, busybox)
-				Expect(err).To(Not(HaveOccurred()))
+				Expect(err).NotTo(HaveOccurred())
 			}
 		})
 
 		AfterEach(func() {
 			By("removing the custom resource for the Kind Busybox")
 			found := &examplecomv1alpha1.Busybox{}
 			err := k8sClient.Get(ctx, typeNamespacedName, found)
-			Expect(err).To(Not(HaveOccurred()))
+			Expect(err).NotTo(HaveOccurred())
 
-			Eventually(func() error {
-				return k8sClient.Delete(context.TODO(), found)
-			}, 2*time.Minute, time.Second).Should(Succeed())
+			Eventually(func(g Gomega) {
+				g.Expect(k8sClient.Delete(context.TODO(), found)).To(Succeed())
+			}).Should(Succeed())
 
 			// TODO(user): Attention if you improve this code by adding other context test you MUST
 			// be aware of the current delete namespace limitations.
@@ -106,10 +109,10 @@ var _ = Describe("Busybox controller", func() {
 
 		It("should successfully reconcile a custom resource for Busybox", func() {
 			By("Checking if the custom resource was successfully created")
-			Eventually(func() error {
+			Eventually(func(g Gomega) {
 				found := &examplecomv1alpha1.Busybox{}
-				return k8sClient.Get(ctx, typeNamespacedName, found)
-			}, time.Minute, time.Second).Should(Succeed())
+				Expect(k8sClient.Get(ctx, typeNamespacedName, found)).To(Succeed())
+			}).Should(Succeed())
 
 			By("Reconciling the custom resource created")
 			busyboxReconciler := &BusyboxReconciler{
@@ -120,34 +123,28 @@ var _ = Describe("Busybox controller", func() {
 			_, err := busyboxReconciler.Reconcile(ctx, reconcile.Request{
 				NamespacedName: typeNamespacedName,
 			})
-			Expect(err).To(Not(HaveOccurred()))
+			Expect(err).NotTo(HaveOccurred())
 
 			By("Checking if Deployment was successfully created in the reconciliation")
-			Eventually(func() error {
+			Eventually(func(g Gomega) {
 				found := &appsv1.Deployment{}
-				return k8sClient.Get(ctx, typeNamespacedName, found)
-			}, time.Minute, time.Second).Should(Succeed())
+				Expect(k8sClient.Get(ctx, typeNamespacedName, found)).To(Succeed())
+			}).Should(Succeed())
 
 			By("Checking the latest Status Condition added to the Busybox instance")
-			Eventually(func() error {
-				if busybox.Status.Conditions != nil &&
-					len(busybox.Status.Conditions) != 0 {
-					latestStatusCondition := busybox.Status.Conditions[len(busybox.Status.Conditions)-1]
-					expectedLatestStatusCondition := metav1.Condition{
-						Type:   typeAvailableBusybox,
-						Status: metav1.ConditionTrue,
-						Reason: "Reconciling",
-						Message: fmt.Sprintf(
-							"Deployment for custom resource (%s) with %d replicas created successfully",
-							busybox.Name,
-							busybox.Spec.Size),
-					}
-					if latestStatusCondition != expectedLatestStatusCondition {
-						return fmt.Errorf("The latest status condition added to the Busybox instance is not as expected")
-					}
+			Eventually(func(g Gomega) {
+				reconcilingTrue := metav1.Condition{
+					Type:   typeAvailableBusybox,
+					Status: metav1.ConditionTrue,
+					Reason: "Reconciling",
+					Message: fmt.Sprintf(
+						"Deployment for custom resource (%s) with %d replicas created successfully",
+						busybox.Name,
+						busybox.Spec.Size),
 				}
-				return nil
-			}, time.Minute, time.Second).Should(Succeed())
+				g.Expect(busybox.Status.Conditions).To(
+					ContainElement(Equal(reconcilingTrue)))
+			}).Should(Succeed())
 		})
 	})
 })