Replace assertions within async assertions.

Functions passed to Gomega's async assertions (Eventually,
Consistently, and friends) are called repeatedly until the first
return value satisfies the given matcher and any other return values
are their respective type's zero value. In several cases, functions
passed to async assertions are themselves directly making
assertions. If one of those fails, it can cause the entire test to
fail immediately rather than continue retrying the async condition.

Author: benluddy

test/e2e/crd_e2e_test.go

@@ -3,6 +3,8 @@ package e2e
 import (
 	"context"
 	"fmt"
+	"time"
+
 	"github.com/blang/semver"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
@@ -13,7 +15,6 @@ import (
 	apiextensionsv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
 	k8serrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-	"time"
 )
 
 var _ = Describe("CRD Versions", func() {
@@ -313,14 +314,22 @@ var _ = Describe("CRD Versions", func() {
 		Expect(s.Status.InstallPlanRef).ToNot(Equal(nil))
 
 		// Check the error on the installplan - should be related to data loss and the CRD upgrade missing a stored version
-		Eventually(func() bool {
-			ip, err := crc.OperatorsV1alpha1().InstallPlans(testNamespace).Get(context.TODO(), s.Status.InstallPlanRef.Name, metav1.GetOptions{})
-			Expect(err).ToNot(HaveOccurred(), "could not get installplan")
-
-			Expect(ip.Status.Phase).To(Equal(operatorsv1alpha1.InstallPlanPhaseFailed))
-			Expect(ip.Status.Conditions[len(ip.Status.Conditions)-1].Message).To(ContainSubstring("risk of data loss"))
-			return true
-		}).Should(BeTrue())
+		Eventually(func() (*operatorsv1alpha1.InstallPlan, error) {
+			return crc.OperatorsV1alpha1().InstallPlans(testNamespace).Get(context.TODO(), s.Status.InstallPlanRef.Name, metav1.GetOptions{})
+		}).Should(And(
+			WithTransform(
+				func(v *operatorsv1alpha1.InstallPlan) operatorsv1alpha1.InstallPlanPhase {
+					return v.Status.Phase
+				},
+				Equal(operatorsv1alpha1.InstallPlanPhaseFailed),
+			),
+			WithTransform(
+				func(v *operatorsv1alpha1.InstallPlan) string {
+					return v.Status.Conditions[len(v.Status.Conditions)-1].Message
+				},
+				ContainSubstring("risk of data loss"),
+			),
+		))
 	})
 
 	// Create a CRD on cluster with v1alpha1 (storage)
@@ -369,7 +378,9 @@ var _ = Describe("CRD Versions", func() {
 		// wrap CRD update in a poll because of the object has been modified related errors
 		Eventually(func() error {
 			oldCRD, err = c.ApiextensionsInterface().ApiextensionsV1beta1().CustomResourceDefinitions().Get(context.TODO(), oldCRD.GetName(), metav1.GetOptions{})
-			Expect(err).ToNot(HaveOccurred(), "error getting old CRD")
+			if err != nil {
+				return err
+			}
 			GinkgoT().Logf("old crd status stored versions: %#v", oldCRD.Status.StoredVersions)
 
 			// set v1alpha1 to no longer served
@@ -448,12 +459,12 @@ var _ = Describe("CRD Versions", func() {
 		Expect(catalogCSV.GetName()).To(Equal(subscription.Status.CurrentCSV))
 
 		// Check the error on the installplan - should be related to data loss and the CRD upgrade missing a stored version (v1alpha1)
-		Eventually(func() bool {
-			ip, err := crc.OperatorsV1alpha1().InstallPlans(testNamespace).Get(context.TODO(), subscription.Status.InstallPlanRef.Name, metav1.GetOptions{})
-			Expect(err).ToNot(HaveOccurred(), "could not get installplan")
-
-			return ip.Status.Phase == operatorsv1alpha1.InstallPlanPhaseFailed
-		}).Should(BeTrue())
+		Eventually(func() (*operatorsv1alpha1.InstallPlan, error) {
+			return crc.OperatorsV1alpha1().InstallPlans(testNamespace).Get(context.TODO(), subscription.Status.InstallPlanRef.Name, metav1.GetOptions{})
+		}).Should(WithTransform(
+			func(v *operatorsv1alpha1.InstallPlan) operatorsv1alpha1.InstallPlanPhase { return v.Status.Phase },
+			Equal(operatorsv1alpha1.InstallPlanPhaseFailed),
+		))
 
 		// update CRD status to remove the v1alpha1 stored version
 		newCRD, err := c.ApiextensionsInterface().ApiextensionsV1beta1().CustomResourceDefinitions().Get(context.TODO(), oldCRD.GetName(), metav1.GetOptions{})

test/e2e/gc_e2e_test.go

@@ -458,16 +458,15 @@ var _ = Describe("Garbage collection for dependent resources", func() {
 			})
 
 			It("OLM should have upgraded associated configmap in place", func() {
-				Eventually(func() bool {
+				Eventually(func() (string, error) {
 					cfg, err := kubeClient.GetConfigMap(testNamespace, configmapName)
 					if err != nil {
-						return false
+						return "", err
 					}
 					// check data in configmap to ensure it is the new data (configmap was updated in the newer bundle)
 					// new value in the configmap is "updated-very-much"
-					data := cfg.Data["special.how"]
-					return data == "updated-very-much"
-				}).Should(BeTrue())
+					return cfg.Data["special.how"], nil
+				}).Should(Equal("updated-very-much"))
 				ctx.Ctx().Logf("dependent successfully updated after csv owner was updated")
 			})
 		})

test/e2e/packagemanifest_e2e_test.go

@@ -9,7 +9,6 @@ import (
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	"k8s.io/apiextensions-apiserver/pkg/apis/apiextensions"
-	"k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/rest"
 
@@ -264,11 +263,13 @@ var _ = Describe("Package Manifest API lists available Operators from Catalog So
 			})
 			It("should successfully update the CatalogSource field", func() {
 
-				Eventually(func() string {
+				Eventually(func() (string, error) {
 					pm, err := fetchPackageManifest(pmc, testNamespace, packageName,
 						packageManifestHasStatus)
-					Expect(err).NotTo(HaveOccurred(), "error getting package manifest after updating catsrc")
-					return pm.Status.CatalogSourceDisplayName
+					if err != nil {
+						return "", err
+					}
+					return pm.Status.CatalogSourceDisplayName, nil
 				}).Should(Equal(displayName))
 			})
 		})
@@ -286,11 +287,11 @@ func fetchPackageManifest(pmc pmversioned.Interface, namespace, name string, che
 	var fetched *packagev1.PackageManifest
 	var err error
 
-	Eventually(func() (bool, error) {
+	EventuallyWithOffset(1, func() (bool, error) {
 		ctx.Ctx().Logf("Polling...")
 		fetched, err = pmc.OperatorsV1().PackageManifests(namespace).Get(context.TODO(), name, metav1.GetOptions{})
-		if err != nil && !errors.IsNotFound(err) {
-			return true, err
+		if err != nil {
+			return false, err
 		}
 		return check(fetched), nil
 	}).Should(BeTrue())