testifylint: enable more rules

.golangci.yaml

@@ -238,14 +238,9 @@ linters-settings:
   testifylint:
       # TODO: enable them all
       disable:
-        - error-is-as
-        - expected-actual
         - go-require
         - float-compare
         - require-error
-        - suite-dont-use-pkg
-        - suite-extra-assert-call
-        - suite-thelper
       enable-all: true
   testpackage:
     # regexp pattern to skip files

internal/credentials/file_store_test.go

@@ -83,7 +83,7 @@ func TestNamespacedFileStore(t *testing.T) {
 				require.NoError(t, err)
 			}
 
-			require.Equal(t, path, tc.expectedPath)
+			require.Equal(t, tc.expectedPath, path)
 
 			contents, err := fs.ReadFile(path)
 			require.NoError(t, err)

internal/resourcepolicies/volume_resources_test.go

@@ -52,8 +52,8 @@ func TestParseCapacity(t *testing.T) {
 		t.Run(test.input, func(t *testing.T) {
 			actual, actualErr := parseCapacity(test.input)
 			if test.expected != emptyCapacity {
-				assert.Equal(t, test.expected.lower.Cmp(actual.lower), 0)
-				assert.Equal(t, test.expected.upper.Cmp(actual.upper), 0)
+				assert.Equal(t, 0, test.expected.lower.Cmp(actual.lower))
+				assert.Equal(t, 0, test.expected.upper.Cmp(actual.upper))
 			}
 			assert.Equal(t, test.expectedErr, actualErr)
 		})

internal/volume/volumes_information_test.go

@@ -1023,28 +1023,28 @@ func TestRestoreVolumeInfoTrackNativeSnapshot(t *testing.T) {
 	restore := builder.ForRestore("velero", "testRestore").Result()
 	tracker := NewRestoreVolInfoTracker(restore, logrus.New(), fakeCilent)
 	tracker.TrackNativeSnapshot("testPV", "snap-001", "ebs", "us-west-1", 10000)
-	assert.Equal(t, *tracker.pvNativeSnapshotMap["testPV"], NativeSnapshotInfo{
+	assert.Equal(t, NativeSnapshotInfo{
 		SnapshotHandle: "snap-001",
 		VolumeType:     "ebs",
 		VolumeAZ:       "us-west-1",
 		IOPS:           "10000",
-	})
+	}, *tracker.pvNativeSnapshotMap["testPV"])
 	tracker.TrackNativeSnapshot("testPV", "snap-002", "ebs", "us-west-2", 15000)
-	assert.Equal(t, *tracker.pvNativeSnapshotMap["testPV"], NativeSnapshotInfo{
+	assert.Equal(t, NativeSnapshotInfo{
 		SnapshotHandle: "snap-002",
 		VolumeType:     "ebs",
 		VolumeAZ:       "us-west-2",
 		IOPS:           "15000",
-	})
+	}, *tracker.pvNativeSnapshotMap["testPV"])
 	tracker.RenamePVForNativeSnapshot("testPV", "newPV")
 	_, ok := tracker.pvNativeSnapshotMap["testPV"]
 	assert.False(t, ok)
-	assert.Equal(t, *tracker.pvNativeSnapshotMap["newPV"], NativeSnapshotInfo{
+	assert.Equal(t, NativeSnapshotInfo{
 		SnapshotHandle: "snap-002",
 		VolumeType:     "ebs",
 		VolumeAZ:       "us-west-2",
 		IOPS:           "15000",
-	})
+	}, *tracker.pvNativeSnapshotMap["newPV"])
 }
 
 func TestRestoreVolumeInfoResult(t *testing.T) {

pkg/backup/actions/csi/pvc_action_test.go

@@ -203,7 +203,7 @@ func TestExecute(t *testing.T) {
 
 			resultUnstructed, _, _, _, err := pvcBIA.Execute(&unstructured.Unstructured{Object: pvcMap}, tc.backup)
 			if tc.expectedErr != nil {
-				require.Equal(t, err, tc.expectedErr)
+				require.EqualError(t, err, tc.expectedErr.Error())
 			} else {
 				require.NoError(t, err)
 			}
@@ -367,7 +367,7 @@ func TestCancel(t *testing.T) {
 
 			err = pvcBIA.Cancel(tc.operationID, tc.backup)
 			if tc.expectedErr != nil {
-				require.Equal(t, err, tc.expectedErr)
+				require.EqualError(t, err, tc.expectedErr.Error())
 			}
 
 			du := new(velerov2alpha1.DataUpload)

pkg/backup/item_collector_test.go

@@ -74,7 +74,7 @@ func TestSortOrderedResource(t *testing.T) {
 		{namespace: "ns1", name: "pod1"},
 	}
 	sortedResources := sortResourcesByOrder(log, podResources, order)
-	assert.Equal(t, sortedResources, expectedResources)
+	assert.Equal(t, expectedResources, sortedResources)
 
 	// Test cluster resources
 	pvResources := []*kubernetesResource{
@@ -87,7 +87,7 @@ func TestSortOrderedResource(t *testing.T) {
 		{name: "pv1"},
 	}
 	sortedPvResources := sortResourcesByOrder(log, pvResources, pvOrder)
-	assert.Equal(t, sortedPvResources, expectedPvResources)
+	assert.Equal(t, expectedPvResources, sortedPvResources)
 }
 
 func TestFilterNamespaces(t *testing.T) {

pkg/buildinfo/buildinfo_test.go

@@ -46,7 +46,7 @@ func TestFormattedGitSHA(t *testing.T) {
 		t.Run(test.name, func(t *testing.T) {
 			GitSHA = test.sha
 			GitTreeState = test.state
-			assert.Equal(t, FormattedGitSHA(), test.expected)
+			assert.Equal(t, test.expected, FormattedGitSHA())
 		})
 	}
 }

pkg/client/client_test.go

@@ -45,7 +45,7 @@ func TestBuildUserAgent(t *testing.T) {
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			resp := buildUserAgent(test.command, test.version, test.gitSha, test.os, test.arch)
-			assert.Equal(t, resp, test.expected)
+			assert.Equal(t, test.expected, resp)
 		})
 	}
 }

pkg/cmd/cli/backup/create_test.go

@@ -138,7 +138,7 @@ func TestCreateOptions_OrderedResources(t *testing.T) {
 		"pods":                   "ns1/p1,ns1/p2",
 		"persistentvolumeclaims": "ns2/pvc1,ns2/pvc2",
 	}
-	assert.Equal(t, orderedResources, expectedResources)
+	assert.Equal(t, expectedResources, orderedResources)
 
 	orderedResources, err = ParseOrderedResources("pods= ns1/p1,ns1/p2 ; persistentvolumes=pv1,pv2")
 	assert.NoError(t, err)
@@ -147,7 +147,7 @@ func TestCreateOptions_OrderedResources(t *testing.T) {
 		"pods":              "ns1/p1,ns1/p2",
 		"persistentvolumes": "pv1,pv2",
 	}
-	assert.Equal(t, orderedResources, expectedMixedResources)
+	assert.Equal(t, expectedMixedResources, orderedResources)
 }
 
 func TestCreateCommand(t *testing.T) {

pkg/cmd/cli/backup/logs_test.go

@@ -79,7 +79,7 @@ func TestNewLogsCommand(t *testing.T) {
 
 		err = l.Run(c, f)
 		require.Error(t, err)
-		require.Contains(t, err.Error(), fmt.Sprintf("logs for backup \"%s\" are not available until it's finished processing", backupName))
+		require.ErrorContains(t, err, fmt.Sprintf("logs for backup \"%s\" are not available until it's finished processing", backupName))
 	})
 
 	t.Run("Backup not exist test", func(t *testing.T) {

pkg/cmd/cli/nodeagent/server_test.go

@@ -20,7 +20,6 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
-	"strings"
 	"testing"
 
 	"github.com/pkg/errors"
@@ -166,7 +165,7 @@ func Test_getDataPathConfigs(t *testing.T) {
 			if test.expectLog == "" {
 				assert.Equal(t, "", logBuffer)
 			} else {
-				assert.True(t, strings.Contains(logBuffer, test.expectLog))
+				assert.Contains(t, logBuffer, test.expectLog)
 			}
 		})
 	}
@@ -384,7 +383,7 @@ func Test_getDataPathConcurrentNum(t *testing.T) {
 			if test.expectLog == "" {
 				assert.Equal(t, "", logBuffer)
 			} else {
-				assert.True(t, strings.Contains(logBuffer, test.expectLog))
+				assert.Contains(t, logBuffer, test.expectLog)
 			}
 		})
 	}

pkg/controller/backup_controller_test.go

@@ -139,7 +139,7 @@ func TestProcessBackupNonProcessedItems(t *testing.T) {
 				require.NoError(t, c.kbClient.Create(context.Background(), test.backup))
 			}
 			actualResult, err := c.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Namespace: test.backup.Namespace, Name: test.backup.Name}})
-			assert.Equal(t, actualResult, ctrl.Result{})
+			assert.Equal(t, ctrl.Result{}, actualResult)
 			assert.NoError(t, err)
 
 			// Any backup that would actually proceed to validation will cause a segfault because this
@@ -229,7 +229,7 @@ func TestProcessBackupValidationFailures(t *testing.T) {
 			require.NoError(t, c.kbClient.Create(context.Background(), test.backup))
 
 			actualResult, err := c.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Namespace: test.backup.Namespace, Name: test.backup.Name}})
-			assert.Equal(t, actualResult, ctrl.Result{})
+			assert.Equal(t, ctrl.Result{}, actualResult)
 			assert.NoError(t, err)
 			res := &velerov1api.Backup{}
 			err = c.kbClient.Get(context.Background(), kbclient.ObjectKey{Namespace: test.backup.Namespace, Name: test.backup.Name}, res)
@@ -1377,7 +1377,7 @@ func TestProcessBackupCompletions(t *testing.T) {
 			}
 
 			actualResult, err := c.Reconcile(ctx, ctrl.Request{NamespacedName: types.NamespacedName{Namespace: test.backup.Namespace, Name: test.backup.Name}})
-			assert.Equal(t, actualResult, ctrl.Result{})
+			assert.Equal(t, ctrl.Result{}, actualResult)
 			assert.NoError(t, err)
 
 			// Disable CSI feature to not impact other test cases.

pkg/controller/backup_repository_controller_test.go

@@ -66,10 +66,10 @@ func TestPatchBackupRepository(t *testing.T) {
 	assert.NoError(t, err)
 	err = reconciler.patchBackupRepository(context.Background(), rr, repoReady())
 	assert.NoError(t, err)
-	assert.Equal(t, rr.Status.Phase, velerov1api.BackupRepositoryPhaseReady)
+	assert.Equal(t, velerov1api.BackupRepositoryPhaseReady, rr.Status.Phase)
 	err = reconciler.patchBackupRepository(context.Background(), rr, repoNotReady("not ready"))
 	assert.NoError(t, err)
-	assert.NotEqual(t, rr.Status.Phase, velerov1api.BackupRepositoryPhaseReady)
+	assert.NotEqual(t, velerov1api.BackupRepositoryPhaseReady, rr.Status.Phase)
 }
 
 func TestCheckNotReadyRepo(t *testing.T) {
@@ -94,7 +94,7 @@ func TestCheckNotReadyRepo(t *testing.T) {
 	assert.NoError(t, err)
 	_, err = reconciler.checkNotReadyRepo(context.TODO(), rr, reconciler.logger)
 	assert.NoError(t, err)
-	assert.Equal(t, rr.Status.Phase, velerov1api.BackupRepositoryPhaseReady)
+	assert.Equal(t, velerov1api.BackupRepositoryPhaseReady, rr.Status.Phase)
 	assert.Equal(t, "s3:test.amazonaws.com/bucket/restic/volume-ns-1", rr.Spec.ResticIdentifier)
 }
 
@@ -135,7 +135,7 @@ func TestInitializeRepo(t *testing.T) {
 	assert.NoError(t, err)
 	err = reconciler.initializeRepo(context.TODO(), rr, reconciler.logger)
 	assert.NoError(t, err)
-	assert.Equal(t, rr.Status.Phase, velerov1api.BackupRepositoryPhaseReady)
+	assert.Equal(t, velerov1api.BackupRepositoryPhaseReady, rr.Status.Phase)
 }
 
 func TestBackupRepoReconcile(t *testing.T) {

pkg/controller/data_download_controller_test.go

@@ -446,7 +446,7 @@ func TestDataDownloadReconcile(t *testing.T) {
 			})
 
 			if test.expectedStatusMsg != "" {
-				assert.Contains(t, err.Error(), test.expectedStatusMsg)
+				require.ErrorContains(t, err, test.expectedStatusMsg)
 			} else {
 				require.NoError(t, err)
 			}

pkg/controller/data_upload_controller_test.go

@@ -561,11 +561,11 @@ func TestReconcile(t *testing.T) {
 				},
 			})
 
-			assert.Equal(t, actualResult, test.expectedRequeue)
+			assert.Equal(t, test.expectedRequeue, actualResult)
 			if test.expectedErrMsg == "" {
 				require.NoError(t, err)
 			} else {
-				assert.Contains(t, err.Error(), test.expectedErrMsg)
+				require.ErrorContains(t, err, test.expectedErrMsg)
 			}
 
 			du := velerov2alpha1api.DataUpload{}

pkg/discovery/helper_test.go

@@ -23,6 +23,7 @@ import (
 
 	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/version"
@@ -311,7 +312,7 @@ func TestHelper_ResourceFor(t *testing.T) {
 			if tc.err == "" {
 				assert.NoError(t, err)
 			} else {
-				assert.Contains(t, err.Error(), tc.err)
+				require.ErrorContains(t, err, tc.err)
 			}
 			assert.Equal(t, *tc.expectedGVR, gvr)
 			assert.Equal(t, *tc.expectedAPIResource, apiResource)
@@ -416,7 +417,7 @@ func TestHelper_KindFor(t *testing.T) {
 			if tc.err == "" {
 				assert.NoError(t, err)
 			} else {
-				assert.Contains(t, err.Error(), tc.err)
+				require.ErrorContains(t, err, tc.err)
 			}
 			assert.Equal(t, *tc.expectedGVR, gvr)
 			assert.Equal(t, *tc.expectedAPIResource, apiResource)

pkg/exposer/csi_snapshot_test.go

@@ -427,7 +427,7 @@ func TestExpose(t *testing.T) {
 
 				assert.Equal(t, expectedVS.Annotations, vsObject.Annotations)
 				assert.Equal(t, *expectedVS.Spec.VolumeSnapshotClassName, *vsObject.Spec.VolumeSnapshotClassName)
-				assert.Equal(t, *expectedVS.Spec.Source.VolumeSnapshotContentName, expectedVSC.Name)
+				assert.Equal(t, expectedVSC.Name, *expectedVS.Spec.Source.VolumeSnapshotContentName)
 
 				assert.Equal(t, expectedVSC.Annotations, vscObj.Annotations)
 				assert.Equal(t, expectedVSC.Spec.DeletionPolicy, vscObj.Spec.DeletionPolicy)

pkg/install/resources_test.go

@@ -45,12 +45,12 @@ func TestResources(t *testing.T) {
 	// For k8s version v1.25 and later, need to add the following labels to make
 	// velero installation namespace has privileged version to work with
 	// PSA(Pod Security Admission) and PSS(Pod Security Standards).
-	assert.Equal(t, ns.Labels["pod-security.kubernetes.io/enforce"], "privileged")
-	assert.Equal(t, ns.Labels["pod-security.kubernetes.io/enforce-version"], "latest")
-	assert.Equal(t, ns.Labels["pod-security.kubernetes.io/audit"], "privileged")
-	assert.Equal(t, ns.Labels["pod-security.kubernetes.io/audit-version"], "latest")
-	assert.Equal(t, ns.Labels["pod-security.kubernetes.io/warn"], "privileged")
-	assert.Equal(t, ns.Labels["pod-security.kubernetes.io/warn-version"], "latest")
+	assert.Equal(t, "privileged", ns.Labels["pod-security.kubernetes.io/enforce"])
+	assert.Equal(t, "latest", ns.Labels["pod-security.kubernetes.io/enforce-version"])
+	assert.Equal(t, "privileged", ns.Labels["pod-security.kubernetes.io/audit"])
+	assert.Equal(t, "latest", ns.Labels["pod-security.kubernetes.io/audit-version"])
+	assert.Equal(t, "privileged", ns.Labels["pod-security.kubernetes.io/warn"])
+	assert.Equal(t, "latest", ns.Labels["pod-security.kubernetes.io/warn-version"])
 
 	crb := ClusterRoleBinding(DefaultVeleroNamespace)
 	// The CRB is a cluster-scoped resource

pkg/plugin/clientmgmt/manager_test.go

@@ -813,7 +813,7 @@ func TestSanitizeName(t *testing.T) {
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
 			sanitizedName := sanitizeName(tc.pluginName)
-			assert.Equal(t, sanitizedName, tc.expectedName)
+			assert.Equal(t, tc.expectedName, sanitizedName)
 		})
 	}
 }

pkg/plugin/clientmgmt/process/client_builder_test.go

@@ -37,7 +37,7 @@ func TestNewClientBuilder(t *testing.T) {
 	logger := test.NewLogger()
 	logLevel := logrus.InfoLevel
 	cb := newClientBuilder("velero", logger, logLevel)
-	assert.Equal(t, cb.commandName, "velero")
+	assert.Equal(t, "velero", cb.commandName)
 	assert.Equal(t, []string{"--log-level", "info"}, cb.commandArgs)
 	assert.Equal(t, newLogrusAdapter(logger, logLevel), cb.pluginLogger)
 

pkg/restore/actions/change_image_name_action_test.go

@@ -173,7 +173,7 @@ func TestChangeImageRepositoryActionExecute(t *testing.T) {
 				pod := new(corev1.Pod)
 				err = runtime.DefaultUnstructuredConverter.FromUnstructured(res.UpdatedItem.UnstructuredContent(), pod)
 				require.NoError(t, err)
-				assert.Equal(t, pod.Spec.Containers[0].Image, tc.want)
+				assert.Equal(t, tc.want, pod.Spec.Containers[0].Image)
 			}
 		})
 	}

pkg/restore/actions/change_pvc_node_selector_test.go

@@ -20,7 +20,6 @@ import (
 	"bytes"
 	"context"
 	"fmt"
-	"strings"
 	"testing"
 
 	"github.com/sirupsen/logrus"
@@ -168,7 +167,7 @@ func TestChangePVCNodeSelectorActionExecute(t *testing.T) {
 
 			// Make sure mapped selected-node exists.
 			logOutput := buf.String()
-			assert.False(t, strings.Contains(logOutput, "Selected-node's mapped node doesn't exist"))
+			assert.NotContains(t, logOutput, "Selected-node's mapped node doesn't exist")
 
 			// validate for both error and non-error cases
 			switch {

pkg/restore/actions/csi/pvc_action_test.go

@@ -250,7 +250,7 @@ func TestResetPVCSpec(t *testing.T) {
 			assert.Emptyf(t, tc.pvc.Spec.VolumeName, "expected change to Spec.VolumeName missing, Want: \"\"; Got: %s", tc.pvc.Spec.VolumeName)
 			assert.Equalf(t, *tc.pvc.Spec.VolumeMode, *before.Spec.VolumeMode, "expected change to Spec.VolumeName missing, Want: \"\"; Got: %s", tc.pvc.Spec.VolumeName)
 			assert.NotNil(t, tc.pvc.Spec.DataSource, "expected change to Spec.DataSource missing")
-			assert.Equalf(t, tc.pvc.Spec.DataSource.Kind, "VolumeSnapshot", "expected change to Spec.DataSource.Kind missing, Want: VolumeSnapshot, Got: %s", tc.pvc.Spec.DataSource.Kind)
+			assert.Equalf(t, "VolumeSnapshot", tc.pvc.Spec.DataSource.Kind, "expected change to Spec.DataSource.Kind missing, Want: VolumeSnapshot, Got: %s", tc.pvc.Spec.DataSource.Kind)
 			assert.Equalf(t, tc.pvc.Spec.DataSource.Name, tc.vsName, "expected change to Spec.DataSource.Name missing, Want: %s, Got: %s", tc.vsName, tc.pvc.Spec.DataSource.Name)
 		})
 	}

pkg/restore/restore_test.go

@@ -3678,7 +3678,7 @@ func assertNonEmptyResults(t *testing.T, typeMsg string, res ...Result) {
 		total += len(r.Namespaces)
 		total += len(r.Velero)
 	}
-	assert.Greater(t, total, 0, "Expected at least one "+typeMsg)
+	assert.Positive(t, total, "Expected at least one "+typeMsg)
 }
 
 type harness struct {

pkg/uploader/kopia/shim_test.go

@@ -28,6 +28,7 @@ import (
 	"github.com/kopia/kopia/repo/object"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
 
 	"github.com/vmware-tanzu/velero/pkg/repository/udmrepo"
 	"github.com/vmware-tanzu/velero/pkg/repository/udmrepo/mocks"
@@ -109,7 +110,7 @@ func TestOpenObject(t *testing.T) {
 			ctx := context.Background()
 			reader, err := NewShimRepo(tc.backupRepo).OpenObject(ctx, object.ID{})
 			if tc.isOpenObjectError {
-				assert.Contains(t, err.Error(), "failed to open object")
+				require.ErrorContains(t, err, "failed to open object")
 			} else if tc.isReaderNil {
 				assert.Nil(t, reader)
 			} else {
@@ -151,7 +152,7 @@ func TestFindManifests(t *testing.T) {
 			ctx := context.Background()
 			_, err := NewShimRepo(tc.backupRepo).FindManifests(ctx, map[string]string{})
 			if tc.isGetManifestError {
-				assert.Contains(t, err.Error(), "failed")
+				require.ErrorContains(t, err, "failed")
 			} else {
 				assert.NoError(t, err)
 			}