lint(ginkgolinter): expect (not)to HaveOccurred (#7565)

Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>

golangci.yaml

@@ -356,7 +356,6 @@ issues:
       linters:
         - dupword
         - errcheck
-        - ginkgolinter
         - goconst
         - gosec
         - govet
@@ -373,7 +372,6 @@ issues:
         - goconst
         - gosec
         - gosimple
-        - ginkgolinter
         - nilerr
         - noctx
         - staticcheck

internal/storage/storagelocation_test.go

@@ -166,10 +166,10 @@ func TestListBackupStorageLocations(t *testing.T) {
 			client := fake.NewClientBuilder().WithScheme(util.VeleroScheme).WithRuntimeObjects(tt.backupLocations).Build()
 			if tt.expectError {
 				_, err := ListBackupStorageLocations(context.Background(), client, "ns-1")
-				g.Expect(err).NotTo(BeNil())
+				g.Expect(err).To(HaveOccurred())
 			} else {
 				_, err := ListBackupStorageLocations(context.Background(), client, "ns-1")
-				g.Expect(err).To(BeNil())
+				g.Expect(err).ToNot(HaveOccurred())
 			}
 		})
 	}

pkg/controller/backup_storage_location_controller_test.go

@@ -100,12 +100,12 @@ var _ = Describe("Backup Storage Location Reconciler", func() {
 				NamespacedName: types.NamespacedName{Namespace: location.Namespace, Name: location.Name},
 			})
 			Expect(actualResult).To(BeEquivalentTo(ctrl.Result{}))
-			Expect(err).To(BeNil())
+			Expect(err).ToNot(HaveOccurred())
 
 			key := client.ObjectKey{Name: location.Name, Namespace: location.Namespace}
 			instance := &velerov1api.BackupStorageLocation{}
 			err = r.client.Get(ctx, key, instance)
-			Expect(err).To(BeNil())
+			Expect(err).ToNot(HaveOccurred())
 			Expect(instance.Spec.Default).To(BeIdenticalTo(tests[i].expectedIsDefault))
 			Expect(instance.Status.Phase).To(BeIdenticalTo(tests[i].expectedPhase))
 		}
@@ -165,12 +165,12 @@ var _ = Describe("Backup Storage Location Reconciler", func() {
 				NamespacedName: types.NamespacedName{Namespace: location.Namespace, Name: location.Name},
 			})
 			Expect(actualResult).To(BeEquivalentTo(ctrl.Result{}))
-			Expect(err).To(BeNil())
+			Expect(err).ToNot(HaveOccurred())
 
 			key := client.ObjectKey{Name: location.Name, Namespace: location.Namespace}
 			instance := &velerov1api.BackupStorageLocation{}
 			err = r.client.Get(ctx, key, instance)
-			Expect(err).To(BeNil())
+			Expect(err).ToNot(HaveOccurred())
 			Expect(instance.Spec.Default).To(BeIdenticalTo(tests[i].expectedIsDefault))
 		}
 	})

pkg/controller/backup_sync_controller_test.go

@@ -451,7 +451,7 @@ var _ = Describe("Backup Sync Reconciler", func() {
 			})
 
 			Expect(actualResult).To(BeEquivalentTo(ctrl.Result{}))
-			Expect(err).To(BeNil())
+			Expect(err).ToNot(HaveOccurred())
 
 			// process the cloud backups
 			for _, cloudBackupData := range test.cloudBackups {
@@ -467,7 +467,7 @@ var _ = Describe("Backup Sync Reconciler", func() {
 						cloudBackupData.backup.Status.Expiration.After(fakeClock.Now())) {
 					Expect(apierrors.IsNotFound(err)).To(BeTrue())
 				} else {
-					Expect(err).To(BeNil())
+					Expect(err).ToNot(HaveOccurred())
 
 					// did this cloud backup already exist in the cluster?
 					var existing *velerov1api.Backup
@@ -496,7 +496,7 @@ var _ = Describe("Backup Sync Reconciler", func() {
 							locationName = label.GetValidName(locationName)
 						}
 						Expect(locationName).To(BeEquivalentTo(obj.Labels[velerov1api.StorageLocationLabel]))
-						Expect(len(obj.Labels[velerov1api.StorageLocationLabel]) <= validation.DNS1035LabelMaxLength).To(BeTrue())
+						Expect(len(obj.Labels[velerov1api.StorageLocationLabel])).To(BeNumerically("<=", validation.DNS1035LabelMaxLength))
 					}
 				}
 

pkg/controller/download_request_controller_test.go

@@ -62,7 +62,7 @@ var _ = Describe("Download Request Reconciler", func() {
 			// now will be used to set the fake clock's time; capture
 			// it here so it can be referenced in the test case defs.
 			now, err := time.Parse(time.RFC1123, time.RFC1123)
-			Expect(err).To(BeNil())
+			Expect(err).ToNot(HaveOccurred())
 			now = now.Local()
 
 			rClock := testclocks.NewFakeClock(now)
@@ -86,22 +86,22 @@ var _ = Describe("Download Request Reconciler", func() {
 
 			fakeClient := fake.NewClientBuilder().WithScheme(scheme.Scheme).Build()
 			err = fakeClient.Create(context.TODO(), test.downloadRequest)
-			Expect(err).To(BeNil())
+			Expect(err).ToNot(HaveOccurred())
 
 			if test.backup != nil {
 				err := fakeClient.Create(context.TODO(), test.backup)
-				Expect(err).To(BeNil())
+				Expect(err).ToNot(HaveOccurred())
 			}
 
 			if test.backupLocation != nil {
 				err := fakeClient.Create(context.TODO(), test.backupLocation)
-				Expect(err).To(BeNil())
+				Expect(err).ToNot(HaveOccurred())
 				backupStores[test.backupLocation.Name] = &persistencemocks.BackupStore{}
 			}
 
 			if test.restore != nil {
 				err := fakeClient.Create(context.TODO(), test.restore)
-				Expect(err).To(BeNil())
+				Expect(err).ToNot(HaveOccurred())
 			}
 
 			// Setup reconciler
@@ -129,7 +129,7 @@ var _ = Describe("Download Request Reconciler", func() {
 
 			Expect(actualResult).To(BeEquivalentTo(test.expectedRequeue))
 			if test.expectedReconcileErr == "" {
-				Expect(err).To(BeNil())
+				Expect(err).ToNot(HaveOccurred())
 			} else {
 				Expect(err.Error()).To(Equal(test.expectedReconcileErr))
 			}
@@ -146,7 +146,7 @@ var _ = Describe("Download Request Reconciler", func() {
 				} else {
 					Expect(instance.Status).ToNot(Equal(test.downloadRequest.Status))
 				}
-				Expect(err).To(BeNil())
+				Expect(err).ToNot(HaveOccurred())
 			}
 
 			if test.expectGetsURL {

pkg/controller/pod_volume_backup_controller_test.go

@@ -141,7 +141,7 @@ var _ = Describe("PodVolumeBackup Reconciler", func() {
 	// `now` will be used to set the fake clock's time; capture
 	// it here so it can be referenced in the test case defs.
 	now, err := time.Parse(time.RFC1123, time.RFC1123)
-	Expect(err).To(BeNil())
+	Expect(err).ToNot(HaveOccurred())
 	now = now.Local()
 
 	DescribeTable("a pod volume backup",
@@ -150,21 +150,21 @@ var _ = Describe("PodVolumeBackup Reconciler", func() {
 
 			fakeClient := fake.NewClientBuilder().WithScheme(scheme.Scheme).Build()
 			err = fakeClient.Create(ctx, test.pvb)
-			Expect(err).To(BeNil())
+			Expect(err).ToNot(HaveOccurred())
 
 			err = fakeClient.Create(ctx, test.pod)
-			Expect(err).To(BeNil())
+			Expect(err).ToNot(HaveOccurred())
 
 			err = fakeClient.Create(ctx, test.bsl)
-			Expect(err).To(BeNil())
+			Expect(err).ToNot(HaveOccurred())
 
 			err = fakeClient.Create(ctx, test.backupRepo)
-			Expect(err).To(BeNil())
+			Expect(err).ToNot(HaveOccurred())
 
 			fakeFS := velerotest.NewFakeFileSystem()
 			pathGlob := fmt.Sprintf("/host_pods/%s/volumes/*/%s", "", "pvb-1-volume")
 			_, err = fakeFS.Create(pathGlob)
-			Expect(err).To(BeNil())
+			Expect(err).ToNot(HaveOccurred())
 
 			credentialFileStore, err := credentials.NewNamespacedFileStore(
 				fakeClient,
@@ -173,7 +173,7 @@ var _ = Describe("PodVolumeBackup Reconciler", func() {
 				fakeFS,
 			)
 
-			Expect(err).To(BeNil())
+			Expect(err).ToNot(HaveOccurred())
 
 			if test.dataMgr == nil {
 				test.dataMgr = datapath.NewManager(1)
@@ -208,7 +208,7 @@ var _ = Describe("PodVolumeBackup Reconciler", func() {
 			})
 			Expect(actualResult).To(BeEquivalentTo(test.expectedRequeue))
 			if test.expectedErrMsg == "" {
-				Expect(err).To(BeNil())
+				Expect(err).ToNot(HaveOccurred())
 			} else {
 				Expect(err.Error()).To(BeEquivalentTo(test.expectedErrMsg))
 			}
@@ -222,7 +222,7 @@ var _ = Describe("PodVolumeBackup Reconciler", func() {
 			if test.expected == nil {
 				Expect(apierrors.IsNotFound(err)).To(BeTrue())
 			} else {
-				Expect(err).To(BeNil())
+				Expect(err).ToNot(HaveOccurred())
 				Eventually(pvb.Status.Phase).Should(Equal(test.expected.Status.Phase))
 			}
 

pkg/controller/server_status_request_controller_test.go

@@ -55,7 +55,7 @@ var _ = Describe("Server Status Request Reconciler", func() {
 	// `now` will be used to set the fake clock's time; capture
 	// it here so it can be referenced in the test case defs.
 	now, err := time.Parse(time.RFC1123, time.RFC1123)
-	Expect(err).To(BeNil())
+	Expect(err).ToNot(HaveOccurred())
 	now = now.Local()
 
 	DescribeTable("a Server Status request",
@@ -79,7 +79,7 @@ var _ = Describe("Server Status Request Reconciler", func() {
 
 			Expect(actualResult).To(BeEquivalentTo(test.expectedRequeue))
 			if test.expectedErrMsg == "" {
-				Expect(err).To(BeNil())
+				Expect(err).ToNot(HaveOccurred())
 			} else {
 				Expect(err.Error()).To(BeEquivalentTo(test.expectedErrMsg))
 				return
@@ -92,7 +92,7 @@ var _ = Describe("Server Status Request Reconciler", func() {
 			if test.expected == nil {
 				Expect(apierrors.IsNotFound(err)).To(BeTrue())
 			} else {
-				Expect(err).To(BeNil())
+				Expect(err).ToNot(HaveOccurred())
 				Eventually(instance.Status.Phase == test.expected.Status.Phase, timeout).Should(BeTrue())
 			}
 		},

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

@@ -61,7 +61,7 @@ func APIExtensionsVersionsTest() {
 			Skip("CRD with apiextension versions dstVersions should have v1")
 			return ""
 		})
-		Expect(len(srcVersions) > 1 && len(dstVersions) == 1).Should(Equal(true), func() string {
+		Expect(len(srcVersions) > 1 && len(dstVersions) == 1).Should(BeTrue(), func() string {
 			Skip("Source cluster should support apiextension v1 and v1beta1, destination cluster should only support apiextension v1")
 			return ""
 		})

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

@@ -117,7 +117,7 @@ func (v *BackupVolumeInfo) CreateResources() error {
 		fmt.Printf("Creating deployment in namespaces ...%s\n", createNSName)
 		// Make sure PVC count is great than 3 to allow both empty volumes and file populated volumes exist per pod
 		pvcCount := 4
-		Expect(pvcCount > 3).To(Equal(true))
+		Expect(pvcCount).To(BeNumerically(">", 3))
 
 		var vols []*v1.Volume
 		for i := 0; i <= pvcCount-1; i++ {

test/e2e/basic/nodeport.go

@@ -70,7 +70,7 @@ func (n *NodePort) CreateResources() error {
 			Expect(createServiceWithNodeport(n.Ctx, n.Client, ns, n.serviceName, n.labels, 0)).To(Succeed(), fmt.Sprintf("Failed to create service %s", n.serviceName))
 			service, err := GetService(n.Ctx, n.Client, ns, n.serviceName)
 			Expect(err).To(Succeed())
-			Expect(len(service.Spec.Ports)).To(Equal(1))
+			Expect(service.Spec.Ports).To(HaveLen(1))
 			n.nodePort = service.Spec.Ports[0].NodePort
 			_, err = GetAllService(n.Ctx)
 			Expect(err).To(Succeed(), "fail to get service")
@@ -135,7 +135,7 @@ func (n *NodePort) Restore() error {
 		By(fmt.Sprintf("Delete service %s by deleting namespace %s", n.serviceName, ns), func() {
 			service, err := GetService(n.Ctx, n.Client, ns, n.serviceName)
 			Expect(err).To(Succeed())
-			Expect(len(service.Spec.Ports)).To(Equal(1))
+			Expect(service.Spec.Ports).To(HaveLen(1))
 			fmt.Println(service.Spec.Ports)
 			Expect(DeleteNamespace(n.Ctx, n.Client, ns, true)).To(Succeed())
 		})
@@ -159,7 +159,7 @@ func (n *NodePort) Restore() error {
 		By(fmt.Sprintf("Verify service %s was restore successfully with the origin nodeport.", ns), func() {
 			service, err := GetService(n.Ctx, n.Client, ns, n.serviceName)
 			Expect(err).To(Succeed())
-			Expect(len(service.Spec.Ports)).To(Equal(1))
+			Expect(service.Spec.Ports).To(HaveLen(1))
 			Expect(service.Spec.Ports[0].NodePort).To(Equal(n.nodePort))
 		})
 	}

test/e2e/basic/pvc-selected-node-changing.go

@@ -96,7 +96,7 @@ func (p *PVCSelectedNodeChanging) CreateResources() error {
 	By("Prepare ConfigMap data", func() {
 		nodeNameList, err := GetWorkerNodes(p.Ctx)
 		Expect(err).To(Succeed())
-		Expect(len(nodeNameList) >= 2).To(Equal(true))
+		Expect(len(nodeNameList)).To(BeNumerically(">=", 2))
 		for _, nodeName := range nodeNameList {
 			if nodeName != p.oldNodeName {
 				p.newNodeName = nodeName
@@ -142,7 +142,7 @@ func (p *PVCSelectedNodeChanging) Verify() error {
 	By(fmt.Sprintf("PVC selected node should be %s", p.newNodeName), func() {
 		pvcNameList, err := GetPvcByPVCName(p.Ctx, p.mappedNS, p.pvcName)
 		Expect(err).To(Succeed())
-		Expect(len(pvcNameList)).Should(Equal(1))
+		Expect(pvcNameList).Should(HaveLen(1))
 		pvc, err := GetPVC(p.Ctx, p.Client, p.mappedNS, pvcNameList[0])
 		Expect(err).To(Succeed())
 		Expect(pvc.Annotations[p.ann]).To(Equal(p.newNodeName))

test/e2e/bsl-mgmt/deletion.go

@@ -163,12 +163,12 @@ func BslDeletionTest(useVolumeSnapshots bool) {
 				pvc, err := GetPvcByPVCName(context.Background(), bslDeletionTestNs, podName_1)
 				Expect(err).To(Succeed())
 				fmt.Println(pvc)
-				Expect(len(pvc)).To(Equal(1))
+				Expect(pvc).To(HaveLen(1))
 				pvc1 := pvc[0]
 				pvc, err = GetPvcByPVCName(context.Background(), bslDeletionTestNs, podName_2)
 				Expect(err).To(Succeed())
 				fmt.Println(pvc)
-				Expect(len(pvc)).To(Equal(1))
+				Expect(pvc).To(HaveLen(1))
 				pvc2 := pvc[0]
 				Expect(AddLabelToPvc(context.Background(), pvc1, bslDeletionTestNs, label_1)).To(Succeed())
 				Expect(AddLabelToPvc(context.Background(), pvc2, bslDeletionTestNs, label_2)).To(Succeed())

test/e2e/privilegesmgmt/ssr.go

@@ -93,8 +93,7 @@ func SSRTest() {
 		By(fmt.Sprintf("Check ssr object in %s namespace", testNS))
 		Expect(veleroCfg.ClientToInstallVelero.Kubebuilder.List(ctx, ssrListResp, &kbclient.ListOptions{Namespace: testNS})).To(Succeed(),
 			fmt.Sprintf("Failed to list ssr object in %s namespace", testNS))
-		Expect(len(ssrListResp.Items)).To(BeNumerically("==", 1),
-			fmt.Sprintf("Count of ssr object in %s namespace is not 1 but %d", testNS, len(ssrListResp.Items)))
+		Expect(ssrListResp.Items).To(HaveLen(1), fmt.Sprintf("Count of ssr object in %s namespace is not 1 but %d", testNS, len(ssrListResp.Items)))
 		Expect(ssrListResp.Items[0].Status.Phase).To(BeEmpty(),
 			fmt.Sprintf("Status of ssr object in %s namespace should be empty", testNS))
 		Expect(ssrListResp.Items[0].Status.ServerVersion).To(BeEmpty(),

test/e2e/resource-filtering/exclude_label.go

@@ -134,7 +134,7 @@ func (e *ExcludeFromBackup) Verify() error {
 		//Check namespace
 		checkNS, err := GetNamespace(e.Ctx, e.Client, namespace)
 		Expect(err).ShouldNot(HaveOccurred(), fmt.Sprintf("Could not retrieve test namespace %s", namespace))
-		Expect(checkNS.Name == namespace).To(Equal(true), fmt.Sprintf("Retrieved namespace for %s has name %s instead", namespace, checkNS.Name))
+		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)
@@ -143,7 +143,7 @@ func (e *ExcludeFromBackup) Verify() error {
 		//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(apierrors.IsNotFound(err)).To(Equal(true))
+		Expect(apierrors.IsNotFound(err)).To(BeTrue())
 
 		//Check configmap: should be included
 		_, err = GetConfigmap(e.Client.ClientGo, namespace, e.CaseBaseName)

test/e2e/resourcemodifiers/resource_modifiers.go

@@ -125,7 +125,7 @@ func (r *ResourceModifiersCase) Verify() error {
 	for _, ns := range *r.NSIncluded {
 		By("Verify deployment has updated values", func() {
 			deploy, err := GetDeployment(r.Client.ClientGo, ns, r.CaseBaseName)
-			Expect(err).To(BeNil(), fmt.Sprintf("Failed to get deployment %s in namespace %s", r.CaseBaseName, ns))
+			Expect(err).ToNot(HaveOccurred(), fmt.Sprintf("Failed to get deployment %s in namespace %s", r.CaseBaseName, ns))
 
 			Expect(*deploy.Spec.Replicas).To(Equal(int32(2)), fmt.Sprintf("Failed to verify deployment %s's replicas in namespace %s", r.CaseBaseName, ns))
 			Expect(deploy.Spec.Template.Spec.Containers[1].Image).To(Equal("nginx:1.14.2"), fmt.Sprintf("Failed to verify deployment %s's image in namespace %s", r.CaseBaseName, ns))

test/e2e/resourcepolicies/resource_policies.go

@@ -171,7 +171,7 @@ func (r *ResourcePoliciesCase) Verify() error {
 						content = strings.Replace(content, "\n", "", -1)
 						originContent := strings.Replace(fmt.Sprintf("ns-%s pod-%s volume-%s", ns, pod.Name, vol.Name), "\n", "", -1)
 
-						Expect(content == originContent).To(BeTrue(), fmt.Sprintf("File %s does not exist in volume %s of pod %s in namespace %s",
+						Expect(content).To(Equal(originContent), fmt.Sprintf("File %s does not exist in volume %s of pod %s in namespace %s",
 							FileName, vol.Name, pod.Name, ns))
 					}
 				}

test/e2e/schedule/schedule-backup-creation.go

@@ -58,7 +58,7 @@ func (n *ScheduleBackupCreation) Init() error {
 		"--include-namespaces", n.namespace,
 		"--schedule=*/" + fmt.Sprintf("%v", n.Period) + " * * * *",
 	}
-	Expect(n.Period < 30).To(Equal(true))
+	Expect(n.Period).To(BeNumerically("<", 30))
 	return nil
 }
 
@@ -109,7 +109,7 @@ func (n *ScheduleBackupCreation) Backup() error {
 			bMap := make(map[string]string)
 			backupsInfo, err := GetScheduledBackupsCreationTime(n.Ctx, n.VeleroCfg.VeleroCLI, "default", n.ScheduleName)
 			Expect(err).To(Succeed())
-			Expect(len(backupsInfo) == i).To(Equal(true))
+			Expect(backupsInfo).To(HaveLen(i))
 			for index, bi := range backupsInfo {
 				bList := strings.Split(bi, ",")
 				fmt.Printf("Backup %d: %v\n", index, bList)

test/e2e/schedule/schedule.go

@@ -44,7 +44,7 @@ func (n *ScheduleBackup) Init() error {
 		"--schedule=*/" + fmt.Sprintf("%v", n.Period) + " * * * *",
 	}
 
-	Expect(n.Period < 30).To(Equal(true))
+	Expect(n.Period).To(BeNumerically("<", 30))
 	return nil
 }
 
@@ -92,7 +92,7 @@ func (n *ScheduleBackup) Backup() error {
 		fmt.Printf("Schedule %s created at %s\n", n.ScheduleName, creationTime)
 		now := time.Now()
 		diff := creationTime.Sub(now)
-		Expect(diff.Minutes() < 1).To(Equal(true))
+		Expect(diff.Minutes()).To(BeNumerically("<", 1))
 	})
 
 	By(fmt.Sprintf("No immediate backup is created by schedule %s\n", n.ScheduleName), func() {
@@ -104,7 +104,7 @@ func (n *ScheduleBackup) Backup() error {
 			if i != n.Period-1 {
 				backupsInfo, err := GetScheduledBackupsCreationTime(n.Ctx, n.VeleroCfg.VeleroCLI, "default", n.ScheduleName)
 				Expect(err).To(Succeed())
-				Expect(len(backupsInfo) == 0).To(Equal(true))
+				Expect(backupsInfo).To(BeEmpty())
 			}
 		}
 	})
@@ -120,7 +120,7 @@ func (n *ScheduleBackup) Backup() error {
 			bMap := make(map[string]string)
 			backupsInfo, err := GetScheduledBackupsCreationTime(n.Ctx, n.VeleroCfg.VeleroCLI, "default", n.ScheduleName)
 			Expect(err).To(Succeed())
-			Expect(len(backupsInfo) == i+2).To(Equal(true))
+			Expect(backupsInfo).To(HaveLen(i + 2))
 			for index, bi := range backupsInfo {
 				bList := strings.Split(bi, ",")
 				fmt.Printf("Backup %d: %v\n", index, bList)
@@ -168,7 +168,7 @@ func (n *ScheduleBackup) Backup() error {
 	fmt.Printf("After pause, backkups count is %d\n", backupCountPostPause)
 
 	By(fmt.Sprintf("Verify no new backups from %s ......\n", n.ScheduleName), func() {
-		Expect(backupCountPostPause == backupCount).To(Equal(true))
+		Expect(backupCountPostPause).To(Equal(backupCount))
 	})
 
 	By(fmt.Sprintf("Unpause schedule %s ......\n", n.ScheduleName), func() {
@@ -188,7 +188,7 @@ func (n *ScheduleBackup) Backup() error {
 	backupCountPostUnpause := len(backupsInfo)
 	fmt.Printf("After unpause, backkups count is %d\n", backupCountPostUnpause)
 	By(fmt.Sprintf("Verify no new backups by schedule %s ......\n", n.ScheduleName), func() {
-		Expect(backupCountPostUnpause-backupCount >= periodCount-1).To(Equal(true))
+		Expect(backupCountPostUnpause - backupCount).To(BeNumerically(">=", periodCount-1))
 	})
 	return nil
 }