Merge pull request #10453 from opbot-xd/fix-ginkgo-v2-cleanup-10440

test: resolve remaining Ginkgo V2 and Gomega anti-patterns

changelogs/unreleased/10453-opbot-xd

@@ -0,0 +1 @@
+test: resolve remaining Ginkgo V2 and Gomega anti-patterns (#10440)

pkg/controller/backup_sync_controller_test.go

@@ -439,7 +439,7 @@ var _ = Describe("Backup Sync Reconciler", func() {
 			}
 
 			if test.location != nil {
-				Expect(r.client.Create(ctx, test.location)).ShouldNot(HaveOccurred())
+				Expect(r.client.Create(ctx, test.location)).ToNot(HaveOccurred())
 				backupStores[test.location.Name] = &persistencemocks.BackupStore{}
 
 				backupStore, ok := backupStores[test.location.Name]
@@ -457,12 +457,12 @@ var _ = Describe("Backup Sync Reconciler", func() {
 
 			for _, existingBackup := range test.existingBackups {
 				err := client.Create(context.TODO(), existingBackup, &ctrlClient.CreateOptions{})
-				Expect(err).ShouldNot(HaveOccurred())
+				Expect(err).ToNot(HaveOccurred())
 			}
 
 			for _, existingPodVolumeBackup := range test.existingPodVolumeBackups {
 				err := client.Create(context.TODO(), existingPodVolumeBackup, &ctrlClient.CreateOptions{})
-				Expect(err).ShouldNot(HaveOccurred())
+				Expect(err).ToNot(HaveOccurred())
 			}
 
 			actualResult, err := r.Reconcile(ctx, ctrl.Request{
@@ -534,7 +534,7 @@ var _ = Describe("Backup Sync Reconciler", func() {
 							cloudBackupData.backup.Status.Expiration.After(fakeClock.Now())) {
 						Expect(apierrors.IsNotFound(err)).To(BeTrue())
 					} else {
-						Expect(err).ShouldNot(HaveOccurred())
+						Expect(err).ToNot(HaveOccurred())
 
 						// did this cloud pod volume backup already exist in the cluster?
 						var existingPodVolumeBackup *velerov1api.PodVolumeBackup
@@ -673,7 +673,7 @@ var _ = Describe("Backup Sync Reconciler", func() {
 		}
 
 		queueScheme := runtime.NewScheme()
-		Expect(velerov1api.AddToScheme(queueScheme)).ShouldNot(HaveOccurred())
+		Expect(velerov1api.AddToScheme(queueScheme)).ToNot(HaveOccurred())
 
 		for _, test := range tests {
 			var (
@@ -693,7 +693,7 @@ var _ = Describe("Backup Sync Reconciler", func() {
 				logger:                  velerotest.NewLogger(),
 			}
 
-			Expect(client.Create(ctx, location)).ShouldNot(HaveOccurred(), test.name)
+			Expect(client.Create(ctx, location)).ToNot(HaveOccurred(), test.name)
 			backupStore := &persistencemocks.BackupStore{}
 			backupStores[location.Name] = backupStore
 			backupStore.On("ListBackups").Return([]string{test.cloudBackup.Name}, nil)
@@ -704,7 +704,7 @@ var _ = Describe("Backup Sync Reconciler", func() {
 			_, err := syncReconciler.Reconcile(ctx, ctrl.Request{
 				NamespacedName: types.NamespacedName{Namespace: location.Namespace, Name: location.Name},
 			})
-			Expect(err).ShouldNot(HaveOccurred(), test.name)
+			Expect(err).ToNot(HaveOccurred(), test.name)
 
 			backupKey := types.NamespacedName{Namespace: "ns-1", Name: test.cloudBackup.Name}
 			synced := &velerov1api.Backup{}
@@ -714,7 +714,7 @@ var _ = Describe("Backup Sync Reconciler", func() {
 				Expect(apierrors.IsNotFound(err)).To(BeTrue(), test.name)
 				continue
 			}
-			Expect(err).ShouldNot(HaveOccurred(), test.name)
+			Expect(err).ToNot(HaveOccurred(), test.name)
 
 			// Reconcile the synced backup with the queue controller twice: the first
 			// reconcile would move a New/empty-phase backup to Queued, the second one
@@ -723,11 +723,11 @@ var _ = Describe("Backup Sync Reconciler", func() {
 			queueReconciler := NewBackupQueueReconciler(client, queueScheme, velerotest.NewLogger(), 1, NewBackupTracker())
 			for range 2 {
 				_, err = queueReconciler.Reconcile(ctx, ctrl.Request{NamespacedName: backupKey})
-				Expect(err).ShouldNot(HaveOccurred(), test.name)
+				Expect(err).ToNot(HaveOccurred(), test.name)
 			}
 
 			after := &velerov1api.Backup{}
-			Expect(client.Get(ctx, backupKey, after)).ShouldNot(HaveOccurred(), test.name)
+			Expect(client.Get(ctx, backupKey, after)).ToNot(HaveOccurred(), test.name)
 			Expect(after.Status.Phase).To(BeEquivalentTo(test.expectPhase), test.name)
 			// Hooks are dropped on sync, so the stored metadata cannot carry a payload
 			// that a later code path could execute.
@@ -880,7 +880,7 @@ var _ = Describe("Backup Sync Reconciler", func() {
 			for _, backup := range test.k8sBackups {
 				// add test backup to client
 				err := client.Create(context.TODO(), backup, &ctrlClient.CreateOptions{})
-				Expect(err).ShouldNot(HaveOccurred())
+				Expect(err).ToNot(HaveOccurred())
 			}
 
 			bslName := "default"
@@ -890,7 +890,7 @@ var _ = Describe("Backup Sync Reconciler", func() {
 			r.deleteOrphanedBackups(ctx, bslName, test.cloudBackups, velerotest.NewLogger())
 
 			numBackups, err := numBackups(client)
-			Expect(err).ShouldNot(HaveOccurred())
+			Expect(err).ToNot(HaveOccurred())
 
 			fmt.Println("")
 
@@ -913,7 +913,7 @@ var _ = Describe("Backup Sync Reconciler", func() {
 
 		testObjList := backupSyncSourceOrderFunc(locationList)
 		testObjArray, err := meta.ExtractList(testObjList)
-		Expect(err).ShouldNot(HaveOccurred())
+		Expect(err).ToNot(HaveOccurred())
 
 		expectLocation := testObjArray[0].(*velerov1api.BackupStorageLocation)
 		Expect(expectLocation.Spec.Default).To(BeEquivalentTo(true))
@@ -1085,7 +1085,7 @@ var _ = Describe("Backup Sync Reconciler", func() {
 				//create all required schedules as needed.
 				for _, creatable := range test.toCreate {
 					err := b.client.Create(context.Background(), creatable)
-					Expect(err).ShouldNot(HaveOccurred())
+					Expect(err).ToNot(HaveOccurred())
 				}
 
 				references := b.filterBackupOwnerReferences(context.Background(), test.backup, logger)

pkg/controller/server_status_request_controller_test.go

@@ -92,7 +92,7 @@ var _ = Describe("Server Status Request Reconciler", func() {
 				Expect(apierrors.IsNotFound(err)).To(BeTrue())
 			} else {
 				Expect(err).ToNot(HaveOccurred())
-				Eventually(instance.Status.Phase == test.expected.Status.Phase, timeout).Should(BeTrue())
+				Expect(instance.Status.Phase).To(Equal(test.expected.Status.Phase))
 			}
 		},
 		Entry("with phase=empty will be processed and phased successfully patched", request{

test/e2e/basic/api-group/enable_api_group_extentions.go

@@ -19,6 +19,7 @@ package basic
 import (
 	"context"
 	"fmt"
+	"slices"
 	"time"
 
 	. "github.com/onsi/ginkgo/v2"
@@ -45,26 +46,22 @@ func APIExtensionsVersionsTest() {
 		veleroCfg = VeleroCfg
 		Expect(KubectlConfigUseContext(context.Background(), veleroCfg.DefaultClusterContext)).To(Succeed())
 		srcVersions, err := GetAPIVersions(veleroCfg.DefaultClient, resourceName)
-		Expect(err).ShouldNot(HaveOccurred())
+		Expect(err).ToNot(HaveOccurred())
 		dstVersions, err := GetAPIVersions(veleroCfg.StandbyClient, resourceName)
-		Expect(err).ShouldNot(HaveOccurred())
+		Expect(err).ToNot(HaveOccurred())
 
-		Expect(srcVersions).Should(ContainElement("v1"), func() string {
+		if !slices.Contains(srcVersions, "v1") {
 			Skip("CRD with apiextension versions srcVersions should have v1")
-			return ""
-		})
-		Expect(srcVersions).Should(ContainElement("v1beta1"), func() string {
-			Skip("CRD with apiextension versions srcVersions should have v1")
-			return ""
-		})
-		Expect(dstVersions).Should(ContainElement("v1"), func() string {
+		}
+		if !slices.Contains(srcVersions, "v1beta1") {
+			Skip("CRD with apiextension versions srcVersions should have v1beta1")
+		}
+		if !slices.Contains(dstVersions, "v1") {
 			Skip("CRD with apiextension versions dstVersions should have v1")
-			return ""
-		})
-		Expect(len(srcVersions) > 1 && len(dstVersions) == 1).Should(BeTrue(), func() string {
+		}
+		if !(len(srcVersions) > 1 && len(dstVersions) == 1) {
 			Skip("Source cluster should support apiextension v1 and v1beta1, destination cluster should only support apiextension v1")
-			return ""
-		})
+		}
 	})
 	AfterEach(func() {
 		By(fmt.Sprintf("Switch to default kubeconfig context %s", veleroCfg.DefaultClusterContext), func() {

test/e2e/basic/backup-volume-info/csi_data_mover.go

@@ -55,7 +55,7 @@ func (c *CSIDataMoverVolumeInfo) Verify() error {
 		BackupObjectsPrefix+"/"+c.BackupName,
 	)
 
-	Expect(err).ShouldNot(HaveOccurred(), "Fail to get VolumeInfo metadata in the Backup Repository.")
+	Expect(err).ToNot(HaveOccurred(), "Fail to get VolumeInfo metadata in the Backup Repository.")
 
 	fmt.Printf("The VolumeInfo metadata content: %+v\n", *volumeInfo[0])
 	Expect(volumeInfo).ToNot(BeEmpty())

test/e2e/basic/backup-volume-info/csi_snapshot.go

@@ -54,7 +54,7 @@ func (c *CSISnapshotVolumeInfo) Verify() error {
 		BackupObjectsPrefix+"/"+c.BackupName,
 	)
 
-	Expect(err).ShouldNot(HaveOccurred(), "Fail to get VolumeInfo metadata in the Backup Repository.")
+	Expect(err).ToNot(HaveOccurred(), "Fail to get VolumeInfo metadata in the Backup Repository.")
 
 	fmt.Printf("The VolumeInfo metadata content: %+v\n", *volumeInfo[0])
 	Expect(volumeInfo).ToNot(BeEmpty())

test/e2e/basic/backup-volume-info/filesystem_upload.go

@@ -54,7 +54,7 @@ func (f *FilesystemUploadVolumeInfo) Verify() error {
 		BackupObjectsPrefix+"/"+f.BackupName,
 	)
 
-	Expect(err).ShouldNot(HaveOccurred(), "Fail to get VolumeInfo metadata in the Backup Repository.")
+	Expect(err).ToNot(HaveOccurred(), "Fail to get VolumeInfo metadata in the Backup Repository.")
 
 	fmt.Printf("The VolumeInfo metadata content: %+v\n", *volumeInfo[0])
 	Expect(volumeInfo).ToNot(BeEmpty())

test/e2e/basic/backup-volume-info/native_snapshot.go

@@ -55,7 +55,7 @@ func (n *NativeSnapshotVolumeInfo) Verify() error {
 		BackupObjectsPrefix+"/"+n.BackupName,
 	)
 
-	Expect(err).ShouldNot(HaveOccurred(), "Fail to get VolumeInfo metadata in the Backup Repository.")
+	Expect(err).ToNot(HaveOccurred(), "Fail to get VolumeInfo metadata in the Backup Repository.")
 
 	fmt.Printf("The VolumeInfo metadata content: %+v\n", *volumeInfo[0])
 	Expect(volumeInfo).ToNot(BeEmpty())

test/e2e/basic/backup-volume-info/skipped_volumes.go

@@ -54,7 +54,7 @@ func (s *SkippedVolumeInfo) Verify() error {
 		BackupObjectsPrefix+"/"+s.BackupName,
 	)
 
-	Expect(err).ShouldNot(HaveOccurred(), "Fail to get VolumeInfo metadata in the Backup Repository.")
+	Expect(err).ToNot(HaveOccurred(), "Fail to get VolumeInfo metadata in the Backup Repository.")
 
 	fmt.Printf("The VolumeInfo metadata content: %+v\n", *volumeInfo[0])
 	Expect(volumeInfo).ToNot(BeEmpty())

test/e2e/bsl-mgmt/deletion.go

@@ -308,7 +308,7 @@ func BslDeletionTest(useVolumeSnapshots bool) {
 			By(fmt.Sprintf("Get all backups from 2 BSLs %s before deleting one of them", backupLocation1), func() {
 				backupsBeforeDel, err := GetAllBackups(context.Background(), veleroCfg.VeleroCLI)
 				Expect(err).To(Succeed())
-				Expect(cmp.Diff(backupsInBsl1AndBsl2, backupsBeforeDel, cmpopts.SortSlices(less))).Should(BeEmpty())
+				Expect(cmp.Diff(backupsInBsl1AndBsl2, backupsBeforeDel, cmpopts.SortSlices(less))).To(BeEmpty())
 
 				By(fmt.Sprintf("Backup1 %s should exist in cloud object store before bsl deletion", backupName1), func() {
 					Expect(ObjectsShouldBeInBucket(veleroCfg.ObjectStoreProvider, veleroCfg.CloudCredentialsFile,
@@ -325,7 +325,7 @@ func BslDeletionTest(useVolumeSnapshots bool) {
 					backupsAfterDel, err := GetAllBackups(context.Background(), veleroCfg.VeleroCLI)
 					Expect(err).To(Succeed())
 					// Default BSL is deleted, so backups in additional BSL should be left only
-					Expect(cmp.Diff(backupsInBSL2, backupsAfterDel, cmpopts.SortSlices(less))).Should(BeEmpty())
+					Expect(cmp.Diff(backupsInBSL2, backupsAfterDel, cmpopts.SortSlices(less))).To(BeEmpty())
 				})
 			})
 

test/e2e/resource-filtering/exclude_label.go

@@ -127,7 +127,7 @@ func (e *ExcludeFromBackup) CreateResources() error {
 	}
 	By(fmt.Sprintf("Checking secret %s should exists in namespaces ...%s\n", secretName, namespace), func() {
 		_, err = GetSecret(e.Client.ClientGo, namespace, e.CaseBaseName)
-		Expect(err).ShouldNot(HaveOccurred(), fmt.Sprintf("failed to list deployment in namespace: %q", namespace))
+		Expect(err).ToNot(HaveOccurred(), fmt.Sprintf("failed to list deployment in namespace: %q", namespace))
 	})
 	//Create Configmap: to be included
 	configmaptName := e.CaseBaseName
@@ -148,21 +148,21 @@ func (e *ExcludeFromBackup) Verify() error {
 	By(fmt.Sprintf("Checking resources in namespaces ...%s\n", namespace), func() {
 		//Check namespace
 		checkNS, err := GetNamespace(e.Ctx, e.Client, namespace)
-		Expect(err).ShouldNot(HaveOccurred(), fmt.Sprintf("Could not retrieve test namespace %s", namespace))
+		Expect(err).ToNot(HaveOccurred(), fmt.Sprintf("Could not retrieve test namespace %s", namespace))
 		Expect(checkNS.Name).To(Equal(namespace), fmt.Sprintf("Retrieved namespace for %s has name %s instead", namespace, checkNS.Name))
 
 		//Check deployment: should be included
 		_, err = GetDeployment(e.Client.ClientGo, namespace, e.CaseBaseName)
-		Expect(err).ShouldNot(HaveOccurred(), fmt.Sprintf("failed to list deployment in namespace: %q", namespace))
+		Expect(err).ToNot(HaveOccurred(), fmt.Sprintf("failed to list deployment in namespace: %q", namespace))
 
 		//Check secrets: secrets should not be included
 		_, err = GetSecret(e.Client.ClientGo, namespace, e.CaseBaseName)
-		Expect(err).Should(HaveOccurred(), fmt.Sprintf("failed to list deployment in namespace: %q", namespace))
+		Expect(err).To(HaveOccurred(), fmt.Sprintf("failed to list deployment in namespace: %q", namespace))
 		Expect(apierrors.IsNotFound(err)).To(BeTrue())
 
 		//Check configmap: should be included
 		_, err = GetConfigMap(e.Client.ClientGo, namespace, e.CaseBaseName)
-		Expect(err).ShouldNot(HaveOccurred(), fmt.Sprintf("failed to list configmap in namespace: %q", namespace))
+		Expect(err).ToNot(HaveOccurred(), fmt.Sprintf("failed to list configmap in namespace: %q", namespace))
 	})
 	return nil
 }

test/e2e/schedule/in_progress.go

@@ -170,7 +170,7 @@ func (s *InProgressCase) Backup() error {
 				}
 
 				// There should be at most one in-progress backup per schedule.
-				Expect(inProgressBackupCount).Should(BeNumerically("<=", 1))
+				Expect(inProgressBackupCount).To(BeNumerically("<=", 1))
 
 				// Already ensured at most one in-progress backup when schedule triggered 2 backups.
 				// Succeed.

test/e2e/schedule/periodical.go

@@ -184,7 +184,7 @@ func (n *PeriodicalCase) Verify() error {
 	By("Namespaces were restored", func() {
 		for _, ns := range *n.NSIncluded {
 			_, err := k8sutil.GetConfigMap(n.Client.ClientGo, ns, n.CaseBaseName)
-			Expect(err).ShouldNot(HaveOccurred(), fmt.Sprintf("failed to list CM in namespace: %s\n", ns))
+			Expect(err).ToNot(HaveOccurred(), fmt.Sprintf("failed to list CM in namespace: %s\n", ns))
 		}
 	})
 	return nil