From 1256224b037190a5a96ea6ece727800c834ad9f4 Mon Sep 17 00:00:00 2001 From: Matthieu MOREL Date: Wed, 17 Jul 2024 09:38:02 +0200 Subject: [PATCH] testifylint: enable more rules Signed-off-by: Matthieu MOREL --- .golangci.yaml | 5 ----- internal/credentials/file_store_test.go | 2 +- internal/resourcepolicies/volume_resources_test.go | 4 ++-- internal/volume/volumes_information_test.go | 12 ++++++------ pkg/backup/actions/csi/pvc_action_test.go | 4 ++-- pkg/backup/item_collector_test.go | 4 ++-- pkg/buildinfo/buildinfo_test.go | 2 +- pkg/client/client_test.go | 2 +- pkg/cmd/cli/backup/create_test.go | 4 ++-- pkg/cmd/cli/nodeagent/server_test.go | 5 ++--- pkg/controller/backup_controller_test.go | 6 +++--- pkg/controller/backup_repository_controller_test.go | 8 ++++---- pkg/controller/data_upload_controller_test.go | 2 +- pkg/exposer/csi_snapshot_test.go | 2 +- pkg/install/resources_test.go | 12 ++++++------ pkg/plugin/clientmgmt/manager_test.go | 2 +- pkg/plugin/clientmgmt/process/client_builder_test.go | 2 +- pkg/restore/actions/change_image_name_action_test.go | 2 +- pkg/restore/actions/change_pvc_node_selector_test.go | 3 +-- pkg/restore/actions/csi/pvc_action_test.go | 2 +- pkg/restore/restore_test.go | 2 +- pkg/util/logging/dual_mode_logger_test.go | 5 ++--- test/pkg/client/client_test.go | 2 +- 23 files changed, 43 insertions(+), 51 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 880e30737c..4c414b8439 100644 --- a/.golangci.yaml +++ b/.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 diff --git a/internal/credentials/file_store_test.go b/internal/credentials/file_store_test.go index 9244007b66..fcb5c5957b 100644 --- a/internal/credentials/file_store_test.go +++ b/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) diff --git a/internal/resourcepolicies/volume_resources_test.go b/internal/resourcepolicies/volume_resources_test.go index 4ff7a6c9c1..4d5d7a743a 100644 --- a/internal/resourcepolicies/volume_resources_test.go +++ b/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) }) diff --git a/internal/volume/volumes_information_test.go b/internal/volume/volumes_information_test.go index 4aefad7e6b..b3eb107146 100644 --- a/internal/volume/volumes_information_test.go +++ b/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) { diff --git a/pkg/backup/actions/csi/pvc_action_test.go b/pkg/backup/actions/csi/pvc_action_test.go index 790dde6e76..59da6145f4 100644 --- a/pkg/backup/actions/csi/pvc_action_test.go +++ b/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) diff --git a/pkg/backup/item_collector_test.go b/pkg/backup/item_collector_test.go index 562e6a2c33..f24b42486d 100644 --- a/pkg/backup/item_collector_test.go +++ b/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) { diff --git a/pkg/buildinfo/buildinfo_test.go b/pkg/buildinfo/buildinfo_test.go index be12bdcab6..a2d4547191 100644 --- a/pkg/buildinfo/buildinfo_test.go +++ b/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()) }) } } diff --git a/pkg/client/client_test.go b/pkg/client/client_test.go index e54e72b61c..b336bdd433 100644 --- a/pkg/client/client_test.go +++ b/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) }) } } diff --git a/pkg/cmd/cli/backup/create_test.go b/pkg/cmd/cli/backup/create_test.go index 83e5098795..a099c3adbf 100644 --- a/pkg/cmd/cli/backup/create_test.go +++ b/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) { diff --git a/pkg/cmd/cli/nodeagent/server_test.go b/pkg/cmd/cli/nodeagent/server_test.go index 187cc6dc0a..fd2c6de94f 100644 --- a/pkg/cmd/cli/nodeagent/server_test.go +++ b/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) } }) } diff --git a/pkg/controller/backup_controller_test.go b/pkg/controller/backup_controller_test.go index ca439611d4..c175c70415 100644 --- a/pkg/controller/backup_controller_test.go +++ b/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. diff --git a/pkg/controller/backup_repository_controller_test.go b/pkg/controller/backup_repository_controller_test.go index 4c8f5fedc8..519f3cb026 100644 --- a/pkg/controller/backup_repository_controller_test.go +++ b/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) { diff --git a/pkg/controller/data_upload_controller_test.go b/pkg/controller/data_upload_controller_test.go index b8e4c88a2e..8d9d795e0e 100644 --- a/pkg/controller/data_upload_controller_test.go +++ b/pkg/controller/data_upload_controller_test.go @@ -561,7 +561,7 @@ 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 { diff --git a/pkg/exposer/csi_snapshot_test.go b/pkg/exposer/csi_snapshot_test.go index 643fd70fce..5591237d73 100644 --- a/pkg/exposer/csi_snapshot_test.go +++ b/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) diff --git a/pkg/install/resources_test.go b/pkg/install/resources_test.go index 2722c26df5..58bbac58ca 100644 --- a/pkg/install/resources_test.go +++ b/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 diff --git a/pkg/plugin/clientmgmt/manager_test.go b/pkg/plugin/clientmgmt/manager_test.go index 1e8c149268..816fef6c02 100644 --- a/pkg/plugin/clientmgmt/manager_test.go +++ b/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) }) } } diff --git a/pkg/plugin/clientmgmt/process/client_builder_test.go b/pkg/plugin/clientmgmt/process/client_builder_test.go index a8bf3ad46e..cff06d93fb 100644 --- a/pkg/plugin/clientmgmt/process/client_builder_test.go +++ b/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) diff --git a/pkg/restore/actions/change_image_name_action_test.go b/pkg/restore/actions/change_image_name_action_test.go index 78898613a5..134fc154ce 100644 --- a/pkg/restore/actions/change_image_name_action_test.go +++ b/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) } }) } diff --git a/pkg/restore/actions/change_pvc_node_selector_test.go b/pkg/restore/actions/change_pvc_node_selector_test.go index 83d9267a83..eee2bca092 100644 --- a/pkg/restore/actions/change_pvc_node_selector_test.go +++ b/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 { diff --git a/pkg/restore/actions/csi/pvc_action_test.go b/pkg/restore/actions/csi/pvc_action_test.go index 095bd8dbe4..53e1908f2f 100644 --- a/pkg/restore/actions/csi/pvc_action_test.go +++ b/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) }) } diff --git a/pkg/restore/restore_test.go b/pkg/restore/restore_test.go index 8bf24d8457..4aa0d6d6a8 100644 --- a/pkg/restore/restore_test.go +++ b/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 { diff --git a/pkg/util/logging/dual_mode_logger_test.go b/pkg/util/logging/dual_mode_logger_test.go index 26bd252e16..c22d19dcaa 100644 --- a/pkg/util/logging/dual_mode_logger_test.go +++ b/pkg/util/logging/dual_mode_logger_test.go @@ -20,7 +20,6 @@ import ( "compress/gzip" "io" "os" - "strings" "testing" "github.com/sirupsen/logrus" @@ -49,8 +48,8 @@ func TestDualModeLogger(t *testing.T) { logStr, err := readLogString(logFile) require.NoError(t, err) - assert.True(t, strings.Contains(logStr, logMsgExpect)) - assert.False(t, strings.Contains(logStr, logMsgUnexpect)) + assert.Contains(t, logStr, logMsgExpect) + assert.NotContains(t, logStr, logMsgUnexpect) logger.Dispose(velerotest.NewLogger()) diff --git a/test/pkg/client/client_test.go b/test/pkg/client/client_test.go index df8df51e43..49c0fa0305 100644 --- a/test/pkg/client/client_test.go +++ b/test/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) }) } }