Merge pull request #1596 from benluddy/assertions-in-assertions

Replace assertions within async assertions.

Author: openshift-merge-robot

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() {
@@ -306,21 +307,33 @@ var _ = Describe("CRD Versions", func() {
 		_, err = crc.OperatorsV1alpha1().Subscriptions(testNamespace).Update(context.TODO(), sub, metav1.UpdateOptions{})
 		Expect(err).ToNot(HaveOccurred(), "could not update subscription")
 
+		subscriptionAtLatestWithDifferentInstallPlan := func(v *operatorsv1alpha1.Subscription) bool {
+			return subscriptionStateAtLatestChecker(v) && v.Status.InstallPlanRef != nil && v.Status.InstallPlanRef.Name != fetchedInstallPlan.Name
+		}
+
 		// fetch new subscription
-		s, err := fetchSubscription(crc, testNamespace, subscriptionName, subscriptionStateAtLatestChecker)
+		s, err := fetchSubscription(crc, testNamespace, subscriptionName, subscriptionAtLatestWithDifferentInstallPlan)
 		Expect(err).ToNot(HaveOccurred())
 		Expect(s).ToNot(BeNil())
 		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 +382,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 +463,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())