From 2a283463b4e4bf2ccbfb541261b4d3ce9356b357 Mon Sep 17 00:00:00 2001 From: Jordan Keister Date: Thu, 3 Oct 2024 11:31:55 -0500 Subject: [PATCH] Add testify linter and address fixes While reviewing https://github.com/operator-framework/operator-controller/pull/1330 we had a discussion about the ability to generally prevent the use of `requires` within `Eventually` calls to prevent future flakes, and that led us to evaluate the `testifylint`-er to give us some cautionary guidelines. It did _not_ resolve the requires-within-eventually issue, but we may try to tackle that with some custom AST in the future. Signed-off-by: Jordan Keister --- .golangci.yaml | 1 + internal/action/helm_test.go | 5 +- internal/catalogmetadata/cache/cache_test.go | 18 +- .../catalogmetadata/client/client_test.go | 11 +- .../filter/bundle_predicates_test.go | 3 +- .../catalogmetadata/filter/successors_test.go | 6 +- .../clusterextension_controller_test.go | 2 +- internal/resolve/catalog_test.go | 8 +- .../rukpak/source/containers_image_test.go | 20 +-- test/e2e/cluster_extension_install_test.go | 154 ++++++++---------- 10 files changed, 106 insertions(+), 122 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 6ccb7c35d..1ecb40994 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -30,6 +30,7 @@ linters: - nonamedreturns - prealloc - stylecheck + - testifylint - tparallel - unconvert - unparam diff --git a/internal/action/helm_test.go b/internal/action/helm_test.go index 8979f1831..d07dc53bc 100644 --- a/internal/action/helm_test.go +++ b/internal/action/helm_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" "helm.sh/helm/v3/pkg/chart" "helm.sh/helm/v3/pkg/release" "sigs.k8s.io/controller-runtime/pkg/client" @@ -130,12 +131,12 @@ func TestActionClientFor(t *testing.T) { // Test the successful case actionClient, err := acg.ActionClientFor(ctx, obj) - assert.NoError(t, err) + require.NoError(t, err) assert.NotNil(t, actionClient) assert.IsType(t, &ActionClient{}, actionClient) // Test the error case actionClient, err = acg.ActionClientFor(ctx, obj) - assert.Error(t, err) + require.Error(t, err) assert.Nil(t, actionClient) } diff --git a/internal/catalogmetadata/cache/cache_test.go b/internal/catalogmetadata/cache/cache_test.go index f898e23e3..e40009ccf 100644 --- a/internal/catalogmetadata/cache/cache_test.go +++ b/internal/catalogmetadata/cache/cache_test.go @@ -79,18 +79,18 @@ func TestFilesystemCachePutAndGet(t *testing.T) { t.Log("Get empty v1 cache") actualFSGet, err := c.Get(catalogName, resolvedRef1) - assert.NoError(t, err) + require.NoError(t, err) assert.Nil(t, actualFSGet) t.Log("Put v1 content into cache") actualFSPut, err := c.Put(catalogName, resolvedRef1, defaultContent(), nil) - assert.NoError(t, err) + require.NoError(t, err) require.NotNil(t, actualFSPut) - assert.NoError(t, equalFilesystems(defaultFS(), actualFSPut)) + require.NoError(t, equalFilesystems(defaultFS(), actualFSPut)) t.Log("Get v1 content from cache") actualFSGet, err = c.Get(catalogName, resolvedRef1) - assert.NoError(t, err) + require.NoError(t, err) require.NotNil(t, actualFSGet) assert.NoError(t, equalFilesystems(defaultFS(), actualFSPut)) assert.NoError(t, equalFilesystems(actualFSPut, actualFSGet)) @@ -98,7 +98,7 @@ func TestFilesystemCachePutAndGet(t *testing.T) { t.Log("Put v1 error into cache") actualFSPut, err = c.Put(catalogName, resolvedRef1, nil, errors.New("fake put error")) // Errors do not override previously successfully populated cache - assert.NoError(t, err) + require.NoError(t, err) require.NotNil(t, actualFSPut) assert.NoError(t, equalFilesystems(defaultFS(), actualFSPut)) assert.NoError(t, equalFilesystems(actualFSPut, actualFSGet)) @@ -115,13 +115,13 @@ func TestFilesystemCachePutAndGet(t *testing.T) { t.Log("Put v2 content into cache") actualFSPut, err = c.Put(catalogName, resolvedRef2, defaultContent(), nil) - assert.NoError(t, err) + require.NoError(t, err) require.NotNil(t, actualFSPut) - assert.NoError(t, equalFilesystems(defaultFS(), actualFSPut)) + require.NoError(t, equalFilesystems(defaultFS(), actualFSPut)) t.Log("Get v2 content from cache") actualFSGet, err = c.Get(catalogName, resolvedRef2) - assert.NoError(t, err) + require.NoError(t, err) require.NotNil(t, actualFSGet) assert.NoError(t, equalFilesystems(defaultFS(), actualFSPut)) assert.NoError(t, equalFilesystems(actualFSPut, actualFSGet)) @@ -130,7 +130,7 @@ func TestFilesystemCachePutAndGet(t *testing.T) { // Cache should be empty and no error because // Put with a new version overrides the old version actualFSGet, err = c.Get(catalogName, resolvedRef1) - assert.NoError(t, err) + require.NoError(t, err) assert.Nil(t, actualFSGet) } diff --git a/internal/catalogmetadata/client/client_test.go b/internal/catalogmetadata/client/client_test.go index a1cec5d4a..d4787a7f9 100644 --- a/internal/catalogmetadata/client/client_test.go +++ b/internal/catalogmetadata/client/client_test.go @@ -11,6 +11,7 @@ import ( "testing/fstest" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" catalogd "github.com/operator-framework/catalogd/api/core/v1alpha1" @@ -77,7 +78,7 @@ func TestClientGetPackage(t *testing.T) { pkgName: "pkg-missing", cache: &fakeCache{getFS: testFS}, assert: func(t *testing.T, fbc *declcfg.DeclarativeConfig, err error) { - assert.NoError(t, err) + require.NoError(t, err) assert.Equal(t, &declcfg.DeclarativeConfig{}, fbc) }, }, @@ -89,7 +90,7 @@ func TestClientGetPackage(t *testing.T) { "invalid-pkg-present/olm.package/invalid-pkg-present.json": &fstest.MapFile{Data: []byte(`{"schema": "olm.package","name": 12345}`)}, }}, assert: func(t *testing.T, fbc *declcfg.DeclarativeConfig, err error) { - assert.ErrorContains(t, err, `error loading package "invalid-pkg-present"`) + require.ErrorContains(t, err, `error loading package "invalid-pkg-present"`) assert.Nil(t, fbc) }, }, @@ -99,7 +100,7 @@ func TestClientGetPackage(t *testing.T) { pkgName: "pkg-present", cache: &fakeCache{getFS: testFS}, assert: func(t *testing.T, fbc *declcfg.DeclarativeConfig, err error) { - assert.NoError(t, err) + require.NoError(t, err) assert.Equal(t, &declcfg.DeclarativeConfig{Packages: []declcfg.Package{{Schema: declcfg.SchemaPackage, Name: "pkg-present"}}}, fbc) }, }, @@ -111,7 +112,7 @@ func TestClientGetPackage(t *testing.T) { return testFS, nil }}, assert: func(t *testing.T, fbc *declcfg.DeclarativeConfig, err error) { - assert.NoError(t, err) + require.NoError(t, err) assert.Equal(t, &declcfg.DeclarativeConfig{Packages: []declcfg.Package{{Schema: declcfg.SchemaPackage, Name: "pkg-present"}}}, fbc) }, }, @@ -170,7 +171,7 @@ func TestClientPopulateCache(t *testing.T) { }, nil }, assert: func(t *testing.T, fs fs.FS, err error) { - assert.NoError(t, err) + require.NoError(t, err) assert.Equal(t, testFS, fs) }, putFuncConstructor: func(t *testing.T) func(source string, errToCache error) (fs.FS, error) { diff --git a/internal/catalogmetadata/filter/bundle_predicates_test.go b/internal/catalogmetadata/filter/bundle_predicates_test.go index dd6abe6d7..beb2950f1 100644 --- a/internal/catalogmetadata/filter/bundle_predicates_test.go +++ b/internal/catalogmetadata/filter/bundle_predicates_test.go @@ -6,6 +6,7 @@ import ( mmsemver "github.com/Masterminds/semver/v3" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/operator-framework/operator-registry/alpha/declcfg" "github.com/operator-framework/operator-registry/alpha/property" @@ -40,7 +41,7 @@ func TestInMastermindsSemverRange(t *testing.T) { } vRange, err := mmsemver.NewConstraint(">=1.0.0") - assert.NoError(t, err) + require.NoError(t, err) f := filter.InMastermindsSemverRange(vRange) diff --git a/internal/catalogmetadata/filter/successors_test.go b/internal/catalogmetadata/filter/successors_test.go index 1d7cc72c3..1b27f9778 100644 --- a/internal/catalogmetadata/filter/successors_test.go +++ b/internal/catalogmetadata/filter/successors_test.go @@ -171,7 +171,7 @@ func TestSuccessorsPredicateWithForceSemverUpgradeConstraintsEnabled(t *testing. } { t.Run(tt.name, func(t *testing.T) { successors, err := SuccessorsOf(tt.installedBundle, channelSet[testPackageName]) - assert.NoError(t, err) + require.NoError(t, err) allBundles := make([]declcfg.Bundle, 0, len(bundleSet)) for _, bundle := range bundleSet { @@ -328,7 +328,7 @@ func TestSuccessorsPredicateWithForceSemverUpgradeConstraintsDisabled(t *testing } { t.Run(tt.name, func(t *testing.T) { successors, err := SuccessorsOf(tt.installedBundle, channelSet[testPackageName]) - assert.NoError(t, err) + require.NoError(t, err) allBundles := make([]declcfg.Bundle, 0, len(bundleSet)) for _, bundle := range bundleSet { @@ -380,7 +380,7 @@ func TestLegacySuccessor(t *testing.T) { emptyBundle := declcfg.Bundle{} f, err := legacySuccessor(installedBundle, fakeChannel) - assert.NoError(t, err) + require.NoError(t, err) assert.True(t, f(b2)) assert.False(t, f(b3)) diff --git a/internal/controllers/clusterextension_controller_test.go b/internal/controllers/clusterextension_controller_test.go index 79f3be476..23f81363c 100644 --- a/internal/controllers/clusterextension_controller_test.go +++ b/internal/controllers/clusterextension_controller_test.go @@ -167,7 +167,7 @@ func TestClusterExtensionResolutionSuccessfulUnpackFails(t *testing.T) { isTerminal := errors.Is(err, reconcile.TerminalError(nil)) assert.Equal(t, tc.expectTerminal, isTerminal, "expected terminal error: %v, got: %v", tc.expectTerminal, isTerminal) - assert.ErrorContains(t, err, tc.unpackErr.Error()) + require.ErrorContains(t, err, tc.unpackErr.Error()) t.Log("By fetching updated cluster extension after reconcile") require.NoError(t, cl.Get(ctx, extKey, clusterExtension)) diff --git a/internal/resolve/catalog_test.go b/internal/resolve/catalog_test.go index 17d0007bc..bce3b5778 100644 --- a/internal/resolve/catalog_test.go +++ b/internal/resolve/catalog_test.go @@ -397,7 +397,7 @@ func TestPackageVariationsBetweenCatalogs(t *testing.T) { gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil) require.Error(t, err) // We will not make a decision on which catalog to use - assert.ErrorContains(t, err, "in multiple catalogs with the same priority [b c]") + require.ErrorContains(t, err, "in multiple catalogs with the same priority [b c]") assert.Nil(t, gotBundle) assert.Nil(t, gotVersion) assert.Nil(t, gotDeprecation) @@ -408,7 +408,7 @@ func TestPackageVariationsBetweenCatalogs(t *testing.T) { gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil) require.Error(t, err) // We will not make a decision on which catalog to use - assert.ErrorContains(t, err, "in multiple catalogs with the same priority [d f]") + require.ErrorContains(t, err, "in multiple catalogs with the same priority [d f]") assert.Nil(t, gotBundle) assert.Nil(t, gotVersion) assert.Nil(t, gotDeprecation) @@ -635,7 +635,7 @@ func TestCatalogWalker(t *testing.T) { seenCatalogs = append(seenCatalogs, cat.Name) return nil } - assert.NoError(t, w(context.Background(), "", walkFunc)) + require.NoError(t, w(context.Background(), "", walkFunc)) assert.Equal(t, []string{"a", "b"}, seenCatalogs) }) } @@ -936,7 +936,7 @@ func TestMultiplePriority(t *testing.T) { ce := buildFooClusterExtension(pkgName, []string{}, ">=1.0.0 <=1.0.1", ocv1alpha1.UpgradeConstraintPolicyCatalogProvided) gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil) require.Error(t, err) - assert.ErrorContains(t, err, "in multiple catalogs with the same priority [a b c]") + require.ErrorContains(t, err, "in multiple catalogs with the same priority [a b c]") assert.Nil(t, gotBundle) assert.Nil(t, gotVersion) assert.Nil(t, gotDeprecation) diff --git a/internal/rukpak/source/containers_image_test.go b/internal/rukpak/source/containers_image_test.go index 104a85721..4d793a000 100644 --- a/internal/rukpak/source/containers_image_test.go +++ b/internal/rukpak/source/containers_image_test.go @@ -53,12 +53,12 @@ func TestUnpackValidInsecure(t *testing.T) { result, err := unpacker.Unpack(context.Background(), bundleSource) require.NoError(t, err) require.NotNil(t, result) - assert.Equal(t, result.State, source.StateUnpacked) + assert.Equal(t, source.StateUnpacked, result.State) require.NoDirExists(t, oldBundlePath) unpackedFile, err := fs.ReadFile(result.Bundle, testFileName) - assert.NoError(t, err) + require.NoError(t, err) // Ensure the unpacked file matches the source content assert.Equal(t, []byte(testFileContents), unpackedFile) assert.NoError(t, unpacker.Cleanup(context.Background(), bundleSource)) @@ -87,9 +87,9 @@ func TestUnpackValidUsesCache(t *testing.T) { // Attempt to pull and unpack the image result, err := unpacker.Unpack(context.Background(), bundleSource) - assert.NoError(t, err) + require.NoError(t, err) require.NotNil(t, result) - assert.Equal(t, result.State, source.StateUnpacked) + assert.Equal(t, source.StateUnpacked, result.State) // Make sure the original contents of the cache are still present. If the cached contents // were not used, we would expect the original contents to be removed. @@ -144,7 +144,7 @@ func TestUnpackNameOnlyImageReference(t *testing.T) { // Attempt to pull and unpack the image _, err := unpacker.Unpack(context.Background(), bundleSource) - assert.ErrorContains(t, err, "tag or digest is needed") + require.ErrorContains(t, err, "tag or digest is needed") assert.ErrorIs(t, err, reconcile.TerminalError(nil)) } @@ -226,8 +226,8 @@ func TestUnpackInvalidNilImage(t *testing.T) { // Attempt to unpack result, err := unpacker.Unpack(context.Background(), bundleSource) assert.Nil(t, result) - assert.ErrorContains(t, err, "nil image source") - assert.ErrorIs(t, err, reconcile.TerminalError(nil)) + require.ErrorContains(t, err, "nil image source") + require.ErrorIs(t, err, reconcile.TerminalError(nil)) assert.NoDirExists(t, filepath.Join(unpacker.BaseCachePath, bundleSource.Name)) } @@ -245,8 +245,8 @@ func TestUnpackInvalidImageRef(t *testing.T) { // Attempt to unpack result, err := unpacker.Unpack(context.Background(), bundleSource) assert.Nil(t, result) - assert.ErrorContains(t, err, "error parsing image reference") - assert.ErrorIs(t, err, reconcile.TerminalError(nil)) + require.ErrorContains(t, err, "error parsing image reference") + require.ErrorIs(t, err, reconcile.TerminalError(nil)) assert.NoDirExists(t, filepath.Join(unpacker.BaseCachePath, bundleSource.Name)) } @@ -322,7 +322,7 @@ func TestCleanup(t *testing.T) { // Clean up the bundle err := unpacker.Cleanup(context.Background(), bundleSource) - assert.NoError(t, err) + require.NoError(t, err) assert.NoDirExists(t, bundleDir) } diff --git a/test/e2e/cluster_extension_install_test.go b/test/e2e/cluster_extension_install_test.go index f3f9d230e..18ade9677 100644 --- a/test/e2e/cluster_extension_install_test.go +++ b/test/e2e/cluster_extension_install_test.go @@ -274,24 +274,22 @@ func TestClusterExtensionInstallRegistry(t *testing.T) { require.EventuallyWithT(t, func(ct *assert.CollectT) { assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeProgressing) - if !assert.NotNil(ct, cond) { - return + if assert.NotNil(ct, cond) { + assert.Equal(ct, metav1.ConditionFalse, cond.Status) + assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason) } - assert.Equal(ct, metav1.ConditionFalse, cond.Status) - assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason) }, pollDuration, pollInterval) t.Log("By eventually installing the package successfully") require.EventuallyWithT(t, func(ct *assert.CollectT) { assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeInstalled) - if !assert.NotNil(ct, cond) { - return + if assert.NotNil(ct, cond) { + assert.Equal(ct, metav1.ConditionTrue, cond.Status) + assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason) + assert.Contains(ct, cond.Message, "Installed bundle") + assert.NotEmpty(ct, clusterExtension.Status.Install.Bundle) } - assert.Equal(ct, metav1.ConditionTrue, cond.Status) - assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason) - assert.Contains(ct, cond.Message, "Installed bundle") - assert.NotEmpty(ct, clusterExtension.Status.Install.Bundle) }, pollDuration, pollInterval) }) } @@ -331,12 +329,11 @@ func TestClusterExtensionInstallRegistryMultipleBundles(t *testing.T) { require.EventuallyWithT(t, func(ct *assert.CollectT) { assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeProgressing) - if !assert.NotNil(ct, cond) { - return + if assert.NotNil(ct, cond) { + assert.Equal(ct, metav1.ConditionTrue, cond.Status) + assert.Equal(ct, ocv1alpha1.ReasonRetrying, cond.Reason) + assert.Contains(ct, cond.Message, "in multiple catalogs with the same priority [operatorhubio test-catalog]") } - assert.Equal(ct, metav1.ConditionTrue, cond.Status) - assert.Equal(ct, ocv1alpha1.ReasonRetrying, cond.Reason) - assert.Contains(ct, cond.Message, "in multiple catalogs with the same priority [operatorhubio test-catalog]") }, pollDuration, pollInterval) } @@ -378,11 +375,10 @@ func TestClusterExtensionBlockInstallNonSuccessorVersion(t *testing.T) { ) cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeProgressing) - if !assert.NotNil(ct, cond) { - return + if assert.NotNil(ct, cond) { + assert.Equal(ct, metav1.ConditionFalse, cond.Status) + assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason) } - assert.Equal(ct, metav1.ConditionFalse, cond.Status) - assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason) }, pollDuration, pollInterval) t.Log("It does not allow to upgrade the ClusterExtension to a non-successor version") @@ -399,11 +395,10 @@ func TestClusterExtensionBlockInstallNonSuccessorVersion(t *testing.T) { require.EventuallyWithT(t, func(ct *assert.CollectT) { assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeProgressing) - if !assert.NotNil(ct, cond) { - return + if assert.NotNil(ct, cond) { + assert.Equal(ct, ocv1alpha1.ReasonRetrying, cond.Reason) + assert.Equal(ct, "error upgrading from currently installed version \"1.0.0\": no bundles found for package \"prometheus\" matching version \"1.2.0\"", cond.Message) } - assert.Equal(ct, ocv1alpha1.ReasonRetrying, cond.Reason) - assert.Equal(ct, "error upgrading from currently installed version \"1.0.0\": no bundles found for package \"prometheus\" matching version \"1.2.0\"", cond.Message) }, pollDuration, pollInterval) } @@ -436,11 +431,10 @@ func TestClusterExtensionForceInstallNonSuccessorVersion(t *testing.T) { require.EventuallyWithT(t, func(ct *assert.CollectT) { assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeProgressing) - if !assert.NotNil(ct, cond) { - return + if assert.NotNil(ct, cond) { + assert.Equal(ct, metav1.ConditionFalse, cond.Status) + assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason) } - assert.Equal(ct, metav1.ConditionFalse, cond.Status) - assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason) }, pollDuration, pollInterval) t.Log("It allows to upgrade the ClusterExtension to a non-successor version") @@ -453,11 +447,10 @@ func TestClusterExtensionForceInstallNonSuccessorVersion(t *testing.T) { require.EventuallyWithT(t, func(ct *assert.CollectT) { assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeProgressing) - if !assert.NotNil(ct, cond) { - return + if assert.NotNil(ct, cond) { + assert.Equal(ct, metav1.ConditionFalse, cond.Status) + assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason) } - assert.Equal(ct, metav1.ConditionFalse, cond.Status) - assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason) }, pollDuration, pollInterval) } @@ -489,11 +482,10 @@ func TestClusterExtensionInstallSuccessorVersion(t *testing.T) { require.EventuallyWithT(t, func(ct *assert.CollectT) { assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeProgressing) - if !assert.NotNil(ct, cond) { - return + if assert.NotNil(ct, cond) { + assert.Equal(ct, metav1.ConditionFalse, cond.Status) + assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason) } - assert.Equal(ct, metav1.ConditionFalse, cond.Status) - assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason) }, pollDuration, pollInterval) t.Log("It does allow to upgrade the ClusterExtension to any of the successor versions within non-zero major version") @@ -505,11 +497,10 @@ func TestClusterExtensionInstallSuccessorVersion(t *testing.T) { require.EventuallyWithT(t, func(ct *assert.CollectT) { assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeProgressing) - if !assert.NotNil(ct, cond) { - return + if assert.NotNil(ct, cond) { + assert.Equal(ct, metav1.ConditionFalse, cond.Status) + assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason) } - assert.Equal(ct, metav1.ConditionFalse, cond.Status) - assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason) }, pollDuration, pollInterval) } @@ -551,11 +542,10 @@ func TestClusterExtensionInstallReResolvesWhenCatalogIsPatched(t *testing.T) { require.EventuallyWithT(t, func(ct *assert.CollectT) { assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeProgressing) - if !assert.NotNil(ct, cond) { - return + if assert.NotNil(ct, cond) { + assert.Equal(ct, metav1.ConditionFalse, cond.Status) + assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason) } - assert.Equal(ct, metav1.ConditionFalse, cond.Status) - assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason) }, pollDuration, pollInterval) // patch imageRef tag on test-catalog image with v2 image @@ -566,22 +556,20 @@ func TestClusterExtensionInstallReResolvesWhenCatalogIsPatched(t *testing.T) { require.EventuallyWithT(t, func(ct *assert.CollectT) { assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: extensionCatalog.Name}, extensionCatalog)) cond := apimeta.FindStatusCondition(extensionCatalog.Status.Conditions, catalogd.TypeServing) - if !assert.NotNil(ct, cond) { - return + if assert.NotNil(ct, cond) { + assert.Equal(ct, metav1.ConditionTrue, cond.Status) + assert.Equal(ct, catalogd.ReasonAvailable, cond.Reason) } - assert.Equal(ct, metav1.ConditionTrue, cond.Status) - assert.Equal(ct, catalogd.ReasonAvailable, cond.Reason) }, pollDuration, pollInterval) t.Log("By eventually reporting a successful resolution and bundle path") require.EventuallyWithT(t, func(ct *assert.CollectT) { assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeProgressing) - if !assert.NotNil(ct, cond) { - return + if assert.NotNil(ct, cond) { + assert.Equal(ct, metav1.ConditionFalse, cond.Status) + assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason) } - assert.Equal(ct, metav1.ConditionFalse, cond.Status) - assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason) }, pollDuration, pollInterval) } @@ -635,11 +623,10 @@ func TestClusterExtensionInstallReResolvesWhenNewCatalog(t *testing.T) { require.EventuallyWithT(t, func(ct *assert.CollectT) { assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeProgressing) - if !assert.NotNil(ct, cond) { - return + if assert.NotNil(ct, cond) { + assert.Equal(ct, metav1.ConditionFalse, cond.Status) + assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason) } - assert.Equal(ct, metav1.ConditionFalse, cond.Status) - assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason) }, pollDuration, pollInterval) // update tag on test-catalog image with v2 image @@ -650,22 +637,20 @@ func TestClusterExtensionInstallReResolvesWhenNewCatalog(t *testing.T) { require.EventuallyWithT(t, func(ct *assert.CollectT) { assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: extensionCatalog.Name}, extensionCatalog)) cond := apimeta.FindStatusCondition(extensionCatalog.Status.Conditions, catalogd.TypeServing) - if !assert.NotNil(ct, cond) { - return + if assert.NotNil(ct, cond) { + assert.Equal(ct, metav1.ConditionTrue, cond.Status) + assert.Equal(ct, catalogd.ReasonAvailable, cond.Reason) } - assert.Equal(ct, metav1.ConditionTrue, cond.Status) - assert.Equal(ct, catalogd.ReasonAvailable, cond.Reason) }, pollDuration, pollInterval) t.Log("By eventually reporting a successful resolution and bundle path") require.EventuallyWithT(t, func(ct *assert.CollectT) { assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeProgressing) - if !assert.NotNil(ct, cond) { - return + if assert.NotNil(ct, cond) { + assert.Equal(ct, metav1.ConditionFalse, cond.Status) + assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason) } - assert.Equal(ct, metav1.ConditionFalse, cond.Status) - assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason) }, pollDuration, pollInterval) } @@ -701,12 +686,11 @@ func TestClusterExtensionInstallReResolvesWhenManagedContentChanged(t *testing.T require.EventuallyWithT(t, func(ct *assert.CollectT) { assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeInstalled) - if !assert.NotNil(ct, cond) { - return + if assert.NotNil(ct, cond) { + assert.Equal(ct, metav1.ConditionTrue, cond.Status) + assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason) + assert.Contains(ct, cond.Message, "Installed bundle") } - assert.Equal(ct, metav1.ConditionTrue, cond.Status) - assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason) - assert.Contains(ct, cond.Message, "Installed bundle") }, pollDuration, pollInterval) t.Log("By deleting a managed resource") @@ -772,23 +756,21 @@ func TestClusterExtensionRecoversFromInitialInstallFailedWhenFailureFixed(t *tes require.EventuallyWithT(t, func(ct *assert.CollectT) { assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeProgressing) - if !assert.NotNil(ct, cond) { - return + if assert.NotNil(ct, cond) { + assert.Equal(ct, metav1.ConditionTrue, cond.Status) + assert.Equal(ct, ocv1alpha1.ReasonRetrying, cond.Reason) } - assert.Equal(ct, metav1.ConditionTrue, cond.Status) - assert.Equal(ct, ocv1alpha1.ReasonRetrying, cond.Reason) }, pollDuration, pollInterval) t.Log("By eventually failing to install the package successfully due to insufficient ServiceAccount permissions") require.EventuallyWithT(t, func(ct *assert.CollectT) { assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeInstalled) - if !assert.NotNil(ct, cond) { - return + if assert.NotNil(ct, cond) { + assert.Equal(ct, metav1.ConditionFalse, cond.Status) + assert.Equal(ct, ocv1alpha1.ReasonFailed, cond.Reason) + assert.Contains(ct, cond.Message, "forbidden") } - assert.Equal(ct, metav1.ConditionFalse, cond.Status) - assert.Equal(ct, ocv1alpha1.ReasonFailed, cond.Reason) - assert.Contains(ct, cond.Message, "forbidden") }, pollDuration, pollInterval) t.Log("By fixing the ServiceAccount permissions") @@ -802,24 +784,22 @@ func TestClusterExtensionRecoversFromInitialInstallFailedWhenFailureFixed(t *tes require.EventuallyWithT(t, func(ct *assert.CollectT) { assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeInstalled) - if !assert.NotNil(ct, cond) { - return + if assert.NotNil(ct, cond) { + assert.Equal(ct, metav1.ConditionTrue, cond.Status) + assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason) + assert.Contains(ct, cond.Message, "Installed bundle") + assert.NotEmpty(ct, clusterExtension.Status.Install) } - assert.Equal(ct, metav1.ConditionTrue, cond.Status) - assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason) - assert.Contains(ct, cond.Message, "Installed bundle") - assert.NotEmpty(ct, clusterExtension.Status.Install) }, pollDuration, pollInterval) t.Log("By eventually reporting Progressing == False with Reason Success") require.EventuallyWithT(t, func(ct *assert.CollectT) { assert.NoError(ct, c.Get(context.Background(), types.NamespacedName{Name: clusterExtension.Name}, clusterExtension)) cond := apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeProgressing) - if !assert.NotNil(ct, cond) { - return + if assert.NotNil(ct, cond) { + assert.Equal(ct, metav1.ConditionFalse, cond.Status) + assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason) } - assert.Equal(ct, metav1.ConditionFalse, cond.Status) - assert.Equal(ct, ocv1alpha1.ReasonSuccess, cond.Reason) }, pollDuration, pollInterval) }