linter(testifylint): use Len or Empty for arrays testing (#7555)

Signed-off-by: Matthieu MOREL <[email protected]>

golangci.yaml

@@ -255,6 +255,23 @@ linters-settings:
   rowserrcheck:
     packages:
       - github.com/jmoiron/sqlx
+  testifylint:
+      # TODO: enable them all
+      disable:
+        - bool-compare
+        - compares
+        - error-is-as
+        - error-nil
+        - expected-actual
+        - go-require
+        - float-compare
+        - require-error
+        - suite-dont-use-pkg
+        - suite-extra-assert-call
+        - suite-thelper
+      enable:
+        - empty
+        - len
   testpackage:
     # regexp pattern to skip files
     skip-regexp: (export|internal)_test\.go
@@ -302,8 +319,10 @@ linters:
     - bodyclose
     - dogsled
     - durationcheck
+    - dupword
     - errcheck
     - exportloopref
+    - errchkjson
     - goconst
     - gofmt
     - goheader
@@ -312,26 +331,25 @@ linters:
     - gosec
     - gosimple
     - govet
+    - ginkgolinter
     - importas
     - ineffassign
     - misspell
     - nakedret
     - nosprintfhostport
+    - nilerr
+    - noctx
+    - nolintlint
+    - revive
     - staticcheck
     - stylecheck
-    - revive
+    - testifylint
     - typecheck
     - unconvert
     - unparam
     - unused
     - usestdlibvars
     - whitespace
-    - dupword
-    - errchkjson
-    - ginkgolinter
-    - nilerr
-    - noctx
-    - nolintlint
   fast: false
 
 

hack/build-image/Dockerfile

@@ -93,7 +93,7 @@ RUN ARCH=$(go env GOARCH) && \
     chmod +x /usr/bin/goreleaser
 
 # get golangci-lint
-RUN curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.54.2
+RUN curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.55.0
 
 # install kubectl
 RUN curl -LO https://storage.googleapis.com/kubernetes-release/release/$(curl -s https://storage.googleapis.com/kubernetes-release/release/stable.txt)/bin/linux/$(go env GOARCH)/kubectl

internal/hook/item_hook_handler_test.go

@@ -2474,7 +2474,7 @@ func TestRestoreHookTrackerAdd(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			_, _ = GroupRestoreExecHooks(tc.resourceRestoreHooks, tc.pod, velerotest.NewLogger(), tc.hookTracker)
 			tracker := tc.hookTracker.GetTracker()
-			assert.Equal(t, tc.expectedCnt, len(tracker))
+			assert.Len(t, tracker, tc.expectedCnt)
 		})
 	}
 }

pkg/backup/backup_pv_action_test.go

@@ -50,14 +50,14 @@ func TestBackupPVAction(t *testing.T) {
 	// and no additional items
 	_, additional, err := a.Execute(pvc, backup)
 	assert.NoError(t, err)
-	assert.Len(t, additional, 0)
+	assert.Empty(t, additional)
 
 	// empty spec.volumeName should result in no error
 	// and no additional items
 	pvc.Object["spec"].(map[string]interface{})["volumeName"] = ""
 	_, additional, err = a.Execute(pvc, backup)
 	assert.NoError(t, err)
-	assert.Len(t, additional, 0)
+	assert.Empty(t, additional)
 
 	// Action should clean the spec.Selector when the StorageClassName is not set.
 	input := builder.ForPersistentVolumeClaim("abc", "abc").VolumeName("pv").Selector(&metav1.LabelSelector{MatchLabels: map[string]string{"abc": "abc"}}).Phase(corev1.ClaimBound).Result()
@@ -119,21 +119,21 @@ func TestBackupPVAction(t *testing.T) {
 	pvc.Object["spec"].(map[string]interface{})["volumeName"] = "myVolume"
 	_, additional, err = a.Execute(pvc, backup)
 	require.NoError(t, err)
-	require.Len(t, additional, 0)
+	require.Empty(t, additional)
 
 	// non-empty spec.volumeName when status.phase is 'Pending'
 	// should result in no error and no additional items
 	pvc.Object["status"].(map[string]interface{})["phase"] = corev1api.ClaimPending
 	_, additional, err = a.Execute(pvc, backup)
 	require.NoError(t, err)
-	require.Len(t, additional, 0)
+	require.Empty(t, additional)
 
 	// non-empty spec.volumeName when status.phase is 'Lost'
 	// should result in no error and no additional items
 	pvc.Object["status"].(map[string]interface{})["phase"] = corev1api.ClaimLost
 	_, additional, err = a.Execute(pvc, backup)
 	require.NoError(t, err)
-	require.Len(t, additional, 0)
+	require.Empty(t, additional)
 
 	// non-empty spec.volumeName when status.phase is 'Bound'
 	// should result in no error and one additional item for the PV
@@ -148,5 +148,5 @@ func TestBackupPVAction(t *testing.T) {
 	pvc.Object["spec"].(map[string]interface{})["volumeName"] = ""
 	_, additional, err = a.Execute(pvc, backup)
 	assert.NoError(t, err)
-	assert.Len(t, additional, 0)
+	assert.Empty(t, additional)
 }

pkg/backup/pv_skip_tracker_test.go

@@ -60,5 +60,5 @@ func TestTrackUntrack(t *testing.T) {
 	tracker.Track("pv3", podVolumeApproach, "it's set to opt-out")
 	tracker.Untrack("pv3")
 	tracker.Track("pv3", csiSnapshotApproach, "not applicable for CSI ")
-	assert.Equal(t, 0, len(tracker.Summary()))
+	assert.Empty(t, tracker.Summary())
 }

pkg/client/config_test.go

@@ -91,7 +91,7 @@ func TestConfigOperations(t *testing.T) {
 
 	// Test Features
 	feature := savedConfig.Features()
-	assert.Equal(t, 1, len(feature))
+	assert.Len(t, feature, 1)
 	assert.Equal(t, expectedFeature, feature[0])
 
 	// Test Colorized

pkg/cmd/cli/backuplocation/delete_test.go

@@ -89,13 +89,13 @@ func TestDeleteFunctions(t *testing.T) {
 	bkrepoList := velerov1api.BackupRepositoryList{}
 	t.Run("findAssociatedBackups", func(t *testing.T) {
 		bkList, e := findAssociatedBackups(kbclient, "bk-loc-1", "ns1")
-		assert.Equal(t, 0, len(bkList.Items))
+		assert.Empty(t, bkList.Items)
 		assert.NoError(t, e)
 	})
 
 	t.Run("findAssociatedBackupRepos", func(t *testing.T) {
 		bkrepoList, e := findAssociatedBackupRepos(kbclient, "bk-loc-1", "ns1")
-		assert.Equal(t, 0, len(bkrepoList.Items))
+		assert.Empty(t, bkrepoList.Items)
 		assert.NoError(t, e)
 	})
 
@@ -104,15 +104,15 @@ func TestDeleteFunctions(t *testing.T) {
 		bk.Name = "bk-name-last"
 		bkList.Items = append(bkList.Items, bk)
 		errList := deleteBackups(kbclient, bkList)
-		assert.Equal(t, 1, len(errList))
+		assert.Len(t, errList, 1)
 		assert.Contains(t, errList[0].Error(), fmt.Sprintf("delete backup \"%s\" associated with deleted BSL: backups.velero.io \"%s\" not found", bk.Name, bk.Name))
 	})
 	t.Run("deleteBackupRepos", func(t *testing.T) {
 		bkrepo := velerov1api.BackupRepository{}
 		bkrepo.Name = "bk-repo-name-last"
 		bkrepoList.Items = append(bkrepoList.Items, bkrepo)
 		errList := deleteBackupRepos(kbclient, bkrepoList)
-		assert.Equal(t, 1, len(errList))
+		assert.Len(t, errList, 1)
 		assert.Contains(t, errList[0].Error(), fmt.Sprintf("delete backup repository \"%s\" associated with deleted BSL: backuprepositories.velero.io \"%s\" not found", bkrepo.Name, bkrepo.Name))
 	})
 }

pkg/controller/backup_deletion_controller_test.go

@@ -142,7 +142,7 @@ func TestBackupDeletionControllerReconcile(t *testing.T) {
 		err = td.fakeClient.Get(ctx, td.req.NamespacedName, res)
 		require.NoError(t, err)
 		assert.Equal(t, "Processed", string(res.Status.Phase))
-		assert.Equal(t, 1, len(res.Status.Errors))
+		assert.Len(t, res.Status.Errors, 1)
 		assert.Equal(t, "spec.backupName is required", res.Status.Errors[0])
 	})
 
@@ -210,7 +210,7 @@ func TestBackupDeletionControllerReconcile(t *testing.T) {
 		err = td.fakeClient.Get(ctx, td.req.NamespacedName, res)
 		require.NoError(t, err)
 		assert.Equal(t, "Processed", string(res.Status.Phase))
-		assert.Equal(t, 1, len(res.Status.Errors))
+		assert.Len(t, res.Status.Errors, 1)
 		assert.Equal(t, "backup is still in progress", res.Status.Errors[0])
 	})
 
@@ -225,7 +225,7 @@ func TestBackupDeletionControllerReconcile(t *testing.T) {
 		err = td.fakeClient.Get(ctx, td.req.NamespacedName, res)
 		require.NoError(t, err)
 		assert.Equal(t, "Processed", string(res.Status.Phase))
-		assert.Equal(t, 1, len(res.Status.Errors))
+		assert.Len(t, res.Status.Errors, 1)
 		assert.Equal(t, "backup not found", res.Status.Errors[0])
 	})
 	t.Run("unable to find backup storage location", func(t *testing.T) {
@@ -240,7 +240,7 @@ func TestBackupDeletionControllerReconcile(t *testing.T) {
 		err = td.fakeClient.Get(ctx, td.req.NamespacedName, res)
 		require.NoError(t, err)
 		assert.Equal(t, "Processed", string(res.Status.Phase))
-		assert.Equal(t, 1, len(res.Status.Errors))
+		assert.Len(t, res.Status.Errors, 1)
 		assert.Equal(t, "backup storage location default not found", res.Status.Errors[0])
 	})
 
@@ -257,7 +257,7 @@ func TestBackupDeletionControllerReconcile(t *testing.T) {
 		err = td.fakeClient.Get(ctx, td.req.NamespacedName, res)
 		require.NoError(t, err)
 		assert.Equal(t, "Processed", string(res.Status.Phase))
-		assert.Equal(t, 1, len(res.Status.Errors))
+		assert.Len(t, res.Status.Errors, 1)
 		assert.Equal(t, "cannot delete backup because backup storage location default is currently in read-only mode", res.Status.Errors[0])
 	})
 	t.Run("full delete, no errors", func(t *testing.T) {
@@ -688,7 +688,7 @@ func TestBackupDeletionControllerReconcile(t *testing.T) {
 		err = td.fakeClient.Get(ctx, td.req.NamespacedName, res)
 		require.NoError(t, err)
 		assert.Equal(t, "Processed", string(res.Status.Phase))
-		assert.Equal(t, 1, len(res.Status.Errors))
+		assert.Len(t, res.Status.Errors, 1)
 		assert.Equal(t, "backup not found", res.Status.Errors[0])
 
 	})

pkg/controller/pod_volume_restore_controller_test.go

@@ -492,7 +492,7 @@ func TestFindVolumeRestoresForPod(t *testing.T) {
 		logger: logrus.New(),
 	}
 	requests := reconciler.findVolumeRestoresForPod(context.Background(), pod)
-	assert.Len(t, requests, 0)
+	assert.Empty(t, requests)
 
 	// contain one matching PVR
 	reconciler.Client = clientBuilder.WithLists(&velerov1api.PodVolumeRestoreList{

pkg/controller/restore_controller_test.go

@@ -592,7 +592,7 @@ func TestRestoreReconcile(t *testing.T) {
 				return
 			}
 			if !test.addValidFinalizer {
-				assert.Equal(t, 1, len(restorer.Calls))
+				assert.Len(t, restorer.Calls, 1)
 			}
 
 			// validate Patch call 2 (setting phase)

pkg/controller/restore_finalizer_controller_test.go

@@ -442,7 +442,7 @@ func TestPatchDynamicPVWithVolumeInfo(t *testing.T) {
 
 		errs := ctx.patchDynamicPVWithVolumeInfo()
 		if tc.expectedErrNum > 0 {
-			assert.Equal(t, tc.expectedErrNum, len(errs.Namespaces))
+			assert.Len(t, errs.Namespaces, tc.expectedErrNum)
 		}
 
 		for pvName, expectedPVInfo := range tc.expectedPatch {

pkg/controller/schedule_controller_test.go

@@ -219,16 +219,16 @@ func TestReconcileOfSchedule(t *testing.T) {
 			// new backup shouldn't be submitted.
 			if test.backup != nil &&
 				(test.backup.Status.Phase == velerov1.BackupPhaseNew || test.backup.Status.Phase == velerov1.BackupPhaseInProgress) {
-				assert.Equal(t, 1, len(backups.Items))
+				assert.Len(t, backups.Items, 1)
 				require.Nil(t, client.Delete(ctx, test.backup))
 			}
 
 			require.Nil(t, client.List(ctx, backups))
 
 			if test.expectedBackupCreate == nil {
-				assert.Equal(t, 0, len(backups.Items))
+				assert.Empty(t, backups.Items)
 			} else {
-				assert.Equal(t, 1, len(backups.Items))
+				assert.Len(t, backups.Items, 1)
 			}
 		})
 	}

pkg/datapath/manager_test.go

@@ -42,10 +42,10 @@ func TestManager(t *testing.T) {
 	assert.Equal(t, async_job_1, ret)
 
 	m.RemoveAsyncBR("job-0")
-	assert.Equal(t, 2, len(m.tracker))
+	assert.Len(t, m.tracker, 2)
 
 	m.RemoveAsyncBR("job-1")
-	assert.Equal(t, 1, len(m.tracker))
+	assert.Len(t, m.tracker, 1)
 
 	ret = m.GetAsyncBR("job-1")
 	assert.Equal(t, nil, ret)

pkg/install/daemonset_test.go

@@ -34,8 +34,8 @@ func TestDaemonSet(t *testing.T) {
 	assert.Equal(t, corev1.PullIfNotPresent, ds.Spec.Template.Spec.Containers[0].ImagePullPolicy)
 
 	ds = DaemonSet("velero", WithSecret(true))
-	assert.Equal(t, 7, len(ds.Spec.Template.Spec.Containers[0].Env))
-	assert.Equal(t, 4, len(ds.Spec.Template.Spec.Volumes))
+	assert.Len(t, ds.Spec.Template.Spec.Containers[0].Env, 7)
+	assert.Len(t, ds.Spec.Template.Spec.Volumes, 4)
 
 	ds = DaemonSet("velero", WithFeatures([]string{"foo,bar,baz"}))
 	assert.Len(t, ds.Spec.Template.Spec.Containers[0].Args, 3)

pkg/install/deployment_test.go

@@ -43,8 +43,8 @@ func TestDeployment(t *testing.T) {
 	assert.Equal(t, corev1.PullIfNotPresent, deploy.Spec.Template.Spec.Containers[0].ImagePullPolicy)
 
 	deploy = Deployment("velero", WithSecret(true))
-	assert.Equal(t, 7, len(deploy.Spec.Template.Spec.Containers[0].Env))
-	assert.Equal(t, 3, len(deploy.Spec.Template.Spec.Volumes))
+	assert.Len(t, deploy.Spec.Template.Spec.Containers[0].Env, 7)
+	assert.Len(t, deploy.Spec.Template.Spec.Volumes, 3)
 
 	deploy = Deployment("velero", WithDefaultRepoMaintenanceFrequency(24*time.Hour))
 	assert.Len(t, deploy.Spec.Template.Spec.Containers[0].Args, 2)

pkg/metrics/metrics.go

@@ -69,7 +69,7 @@ const (
 	// data mover metrics
 	DataUploadSuccessTotal   = "data_upload_success_total"
 	DataUploadFailureTotal   = "data_upload_failure_total"
-	DataUploadCancelTotal    = "data_upload_cancel_total" //nolint:gosec // Not a hard code secret.
+	DataUploadCancelTotal    = "data_upload_cancel_total"
 	DataDownloadSuccessTotal = "data_download_success_total"
 	DataDownloadFailureTotal = "data_download_failure_total"
 	DataDownloadCancelTotal  = "data_download_cancel_total"

pkg/podvolume/backupper_test.go

@@ -642,22 +642,22 @@ func TestPVCBackupSummary(t *testing.T) {
 
 	// it won't be added if the volme is not in the pvc map.
 	pbs.addSkipped("vol-3", "whatever reason")
-	assert.Equal(t, 0, len(pbs.Skipped))
+	assert.Empty(t, pbs.Skipped)
 	pbs.addBackedup("vol-3")
-	assert.Equal(t, 0, len(pbs.Backedup))
+	assert.Empty(t, pbs.Backedup)
 
 	// only can be added as skipped when it's not in backedup set
 	pbs.addBackedup("vol-1")
-	assert.Equal(t, 1, len(pbs.Backedup))
+	assert.Len(t, pbs.Backedup, 1)
 	assert.Equal(t, "pvc-1", pbs.Backedup["vol-1"].Name)
 	pbs.addSkipped("vol-1", "whatever reason")
-	assert.Equal(t, 0, len(pbs.Skipped))
+	assert.Empty(t, pbs.Skipped)
 	pbs.addSkipped("vol-2", "vol-2 has to be skipped")
-	assert.Equal(t, 1, len(pbs.Skipped))
+	assert.Len(t, pbs.Skipped, 1)
 	assert.Equal(t, "pvc-2", pbs.Skipped["vol-2"].PVC.Name)
 
 	// adding a vol as backedup removes it from skipped set
 	pbs.addBackedup("vol-2")
-	assert.Equal(t, 0, len(pbs.Skipped))
-	assert.Equal(t, 2, len(pbs.Backedup))
+	assert.Empty(t, pbs.Skipped)
+	assert.Len(t, pbs.Backedup, 2)
 }

pkg/repository/udmrepo/kopialib/backend/utils.go

@@ -30,7 +30,7 @@ func mustHaveString(key string, flags map[string]string) (string, error) {
 	if value, exist := flags[key]; exist {
 		return value, nil
 	}
-	return "", errors.New("key " + key + " not found")
+	return "", errors.Errorf("key %s not found", key)
 }
 
 func optionalHaveString(key string, flags map[string]string) string {

pkg/util/stringslice/stringslice_test.go

@@ -36,7 +36,7 @@ func TestHas(t *testing.T) {
 func TestExcept(t *testing.T) {
 	items := []string{}
 	except := Except(items, "asdf")
-	assert.Len(t, except, 0)
+	assert.Empty(t, except)
 
 	items = []string{"a", "b", "c"}
 	assert.Equal(t, []string{"b", "c"}, Except(items, "a"))