testifylint: enable error-nil rule (#7670)

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

.golangci.yaml

@@ -239,7 +239,6 @@ linters-settings:
       # TODO: enable them all
       disable:
         - error-is-as
-        - error-nil
         - expected-actual
         - go-require
         - float-compare

internal/hook/hook_tracker_test.go

@@ -65,13 +65,13 @@ func TestHookTracker_Record(t *testing.T) {
 	info := tracker.tracker[key]
 	assert.True(t, info.hookFailed)
 	assert.True(t, info.hookExecuted)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 
 	err = tracker.Record("ns2", "pod2", "container1", HookSourceAnnotation, "h1", "", true, fmt.Errorf("err"))
-	assert.NotNil(t, err)
+	assert.Error(t, err)
 
 	err = tracker.Record("ns1", "pod1", "container1", HookSourceAnnotation, "h1", "", false, nil)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 	assert.True(t, info.hookFailed)
 }
 
@@ -141,13 +141,13 @@ func TestMultiHookTracker_Record(t *testing.T) {
 	info := mht.trackers["restore1"].tracker[key]
 	assert.True(t, info.hookFailed)
 	assert.True(t, info.hookExecuted)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 
 	err = mht.Record("restore1", "ns2", "pod2", "container1", HookSourceAnnotation, "h1", "", true, fmt.Errorf("err"))
-	assert.NotNil(t, err)
+	assert.Error(t, err)
 
 	err = mht.Record("restore2", "ns2", "pod2", "container1", HookSourceAnnotation, "h1", "", true, fmt.Errorf("err"))
-	assert.NotNil(t, err)
+	assert.Error(t, err)
 }
 
 func TestMultiHookTracker_Stat(t *testing.T) {

internal/hook/item_hook_handler_test.go

@@ -1199,7 +1199,7 @@ func TestGroupRestoreExecHooks(t *testing.T) {
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
 			actual, err := GroupRestoreExecHooks("restore1", tc.resourceRestoreHooks, tc.pod, velerotest.NewLogger(), hookTracker)
-			assert.Nil(t, err)
+			assert.NoError(t, err)
 			assert.Equal(t, tc.expected, actual)
 		})
 	}
@@ -1976,7 +1976,7 @@ func TestValidateContainer(t *testing.T) {
 	expectedError := fmt.Errorf("invalid InitContainer in restore hook, it doesn't have Command, Name or Image field")
 
 	// valid string should return nil as result.
-	assert.Nil(t, ValidateContainer([]byte(valid)))
+	assert.NoError(t, ValidateContainer([]byte(valid)))
 
 	// noName string should return expected error as result.
 	assert.Equal(t, expectedError, ValidateContainer([]byte(noName)))

internal/hook/wait_exec_hook_handler_test.go

@@ -732,7 +732,7 @@ func TestWaitExecHandleHooks(t *testing.T) {
 
 			for _, e := range test.expectedExecutions {
 				obj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(e.pod)
-				assert.Nil(t, err)
+				assert.NoError(t, err)
 				podCommandExecutor.On("ExecutePodCommand", mock.Anything, obj, e.pod.Namespace, e.pod.Name, e.name, e.hook).Return(e.error)
 			}
 
@@ -1269,7 +1269,7 @@ func TestRestoreHookTrackerUpdate(t *testing.T) {
 
 			for _, e := range test.expectedExecutions {
 				obj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(e.pod)
-				assert.Nil(t, err)
+				assert.NoError(t, err)
 				podCommandExecutor.On("ExecutePodCommand", mock.Anything, obj, e.pod.Namespace, e.pod.Name, e.name, e.hook).Return(e.error)
 			}
 

internal/resourcepolicies/resource_policies_test.go

@@ -232,7 +232,7 @@ func TestGetResourcePoliciesFromConfig(t *testing.T) {
 
 	// Call the function and check for errors
 	resPolicies, err := getResourcePoliciesFromConfig(cm)
-	assert.Nil(t, err)
+	assert.NoError(t, err)
 
 	// Check that the returned resourcePolicies object contains the expected data
 	assert.Equal(t, "v1", resPolicies.version)
@@ -422,12 +422,12 @@ volumePolicies:
 			if err != nil {
 				t.Fatalf("got error when get match action %v", err)
 			}
-			assert.Nil(t, err)
+			assert.NoError(t, err)
 			policies := &Policies{}
 			err = policies.BuildPolicy(resPolicies)
-			assert.Nil(t, err)
+			assert.NoError(t, err)
 			action, err := policies.GetMatchAction(tc.vol)
-			assert.Nil(t, err)
+			assert.NoError(t, err)
 
 			if tc.skip {
 				if action.Type != Skip {

internal/volumehelper/volume_policy_helper_test.go

@@ -336,7 +336,7 @@ func TestVolumeHelperImpl_ShouldPerformSnapshot(t *testing.T) {
 
 			actualShouldSnapshot, actualError := vh.ShouldPerformSnapshot(&unstructured.Unstructured{Object: obj}, tc.groupResource)
 			if tc.expectedErr {
-				require.NotNil(t, actualError, "Want error; Got nil error")
+				require.Error(t, actualError, "Want error; Got nil error")
 				return
 			}
 
@@ -698,7 +698,7 @@ func TestVolumeHelperImpl_ShouldPerformFSBackup(t *testing.T) {
 
 			actualShouldFSBackup, actualError := vh.ShouldPerformFSBackup(tc.pod.Spec.Volumes[0], *tc.pod)
 			if tc.expectedErr {
-				require.NotNil(t, actualError, "Want error; Got nil error")
+				require.Error(t, actualError, "Want error; Got nil error")
 				return
 			}
 

pkg/archive/parser_test.go

@@ -112,7 +112,7 @@ func TestParse(t *testing.T) {
 			if tc.wantErrMsg != nil {
 				assert.ErrorIs(t, err, tc.wantErrMsg, "Error should be: %v, got: %v", tc.wantErrMsg, err)
 			} else {
-				assert.Nil(t, err)
+				assert.NoError(t, err)
 				assert.Equal(t, tc.want, res)
 			}
 		})
@@ -226,7 +226,7 @@ func TestParseGroupVersions(t *testing.T) {
 			if tc.wantErrMsg != nil {
 				assert.ErrorIs(t, err, tc.wantErrMsg, "Error should be: %v, got: %v", tc.wantErrMsg, err)
 			} else {
-				assert.Nil(t, err)
+				assert.NoError(t, err)
 				assert.Equal(t, tc.want, res)
 			}
 		})

pkg/backup/actions/service_account_action_test.go

@@ -384,7 +384,7 @@ func TestServiceAccountActionExecute(t *testing.T) {
 			res, additional, err := action.Execute(test.serviceAccount, nil)
 
 			assert.Equal(t, test.serviceAccount, res)
-			assert.Nil(t, err)
+			assert.NoError(t, err)
 
 			// ensure slices are ordered for valid comparison
 			sort.Slice(test.expectedAdditionalItems, func(i, j int) bool {
@@ -591,7 +591,7 @@ func TestServiceAccountActionExecuteOnBeta1(t *testing.T) {
 			res, additional, err := action.Execute(test.serviceAccount, nil)
 
 			assert.Equal(t, test.serviceAccount, res)
-			assert.Nil(t, err)
+			assert.NoError(t, err)
 
 			// ensure slices are ordered for valid comparison
 			sort.Slice(test.expectedAdditionalItems, func(i, j int) bool {

pkg/backup/item_backupper_test.go

@@ -220,7 +220,7 @@ func TestGetPVName(t *testing.T) {
 			if tc.obj != nil {
 				data, err := runtime.DefaultUnstructuredConverter.ToUnstructured(tc.obj)
 				o = &unstructured.Unstructured{Object: data}
-				require.Nil(t, err)
+				require.NoError(t, err)
 			}
 			name, err2 := getPVName(o, tc.groupResource)
 			assert.Equal(t, tc.pvName, name)

pkg/client/client_test.go

@@ -79,7 +79,7 @@ func TestConfig(t *testing.T) {
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			client, err := Config(test.kubeconfig, test.kubecontext, "velero", test.QPS, test.burst)
-			assert.Nil(t, err)
+			assert.NoError(t, err)
 			assert.Equal(t, test.expectedHost, client.Host)
 			assert.Equal(t, test.QPS, client.QPS)
 			assert.Equal(t, test.burst, client.Burst)

pkg/client/factory_test.go

@@ -134,11 +134,11 @@ func TestFactory(t *testing.T) {
 			assert.NotNil(t, dynamicClient)
 
 			kubebuilderClient, e := f.KubebuilderClient()
-			assert.Nil(t, e)
+			assert.NoError(t, e)
 			assert.NotNil(t, kubebuilderClient)
 
 			kbClientWithWatch, e := f.KubebuilderWatchClient()
-			assert.Nil(t, e)
+			assert.NoError(t, e)
 			assert.NotNil(t, kbClientWithWatch)
 		})
 	}

pkg/cmd/cli/backup/create_test.go

@@ -129,7 +129,7 @@ func TestCreateOptions_BuildBackupFromSchedule(t *testing.T) {
 
 func TestCreateOptions_OrderedResources(t *testing.T) {
 	_, err := ParseOrderedResources("pods= ns1/p1; ns1/p2; persistentvolumeclaims=ns2/pvc1, ns2/pvc2")
-	assert.NotNil(t, err)
+	assert.Error(t, err)
 
 	orderedResources, err := ParseOrderedResources("pods= ns1/p1,ns1/p2 ; persistentvolumeclaims=ns2/pvc1,ns2/pvc2")
 	assert.NoError(t, err)

pkg/cmd/cli/backuplocation/create_test.go

@@ -148,7 +148,7 @@ func TestCreateCommand_Run(t *testing.T) {
 
 	o.Complete(args, f)
 	e := o.Validate(c, args, f)
-	assert.Nil(t, e)
+	assert.NoError(t, e)
 
 	e = o.Run(c, f)
 	assert.Contains(t, e.Error(), fmt.Sprintf("%s: no such file or directory", caCertFile))

pkg/cmd/cli/backuplocation/set_test.go

@@ -66,7 +66,7 @@ func TestNewSetCommand(t *testing.T) {
 	args := []string{backupName}
 	o.Complete(args, f)
 	e := o.Validate(c, args, f)
-	assert.Nil(t, e)
+	assert.NoError(t, e)
 
 	e = o.Run(c, f)
 	assert.Contains(t, e.Error(), fmt.Sprintf("%s: no such file or directory", cacert))

pkg/cmd/cli/select_option_test.go

@@ -28,7 +28,7 @@ import (
 func TestCompleteOfSelectOption(t *testing.T) {
 	option := &SelectOptions{}
 	args := []string{"arg1", "arg2"}
-	require.Nil(t, option.Complete(args))
+	require.NoError(t, option.Complete(args))
 	assert.Equal(t, args, option.Names)
 }
 
@@ -38,8 +38,8 @@ func TestValidateOfSelectOption(t *testing.T) {
 		Selector: flag.LabelSelector{},
 		All:      false,
 	}
-	assert.NotNil(t, option.Validate())
+	assert.Error(t, option.Validate())
 
 	option.All = true
-	assert.Nil(t, option.Validate())
+	assert.NoError(t, option.Validate())
 }

pkg/cmd/server/server_test.go

@@ -194,14 +194,14 @@ func Test_newServer(t *testing.T) {
 	_, err := newServer(factory, serverConfig{
 		uploaderType: "invalid",
 	}, logger)
-	assert.NotNil(t, err)
+	assert.Error(t, err)
 
 	// invalid clientQPS
 	_, err = newServer(factory, serverConfig{
 		uploaderType: uploader.KopiaType,
 		clientQPS:    -1,
 	}, logger)
-	assert.NotNil(t, err)
+	assert.Error(t, err)
 
 	// invalid clientBurst
 	factory.On("SetClientQPS", mock.Anything).Return()
@@ -210,7 +210,7 @@ func Test_newServer(t *testing.T) {
 		clientQPS:    1,
 		clientBurst:  -1,
 	}, logger)
-	assert.NotNil(t, err)
+	assert.Error(t, err)
 
 	// invalid clientBclientPageSizeurst
 	factory.On("SetClientQPS", mock.Anything).Return().
@@ -221,7 +221,7 @@ func Test_newServer(t *testing.T) {
 		clientBurst:    1,
 		clientPageSize: -1,
 	}, logger)
-	assert.NotNil(t, err)
+	assert.Error(t, err)
 
 	// got error when creating client
 	factory.On("SetClientQPS", mock.Anything).Return().
@@ -235,7 +235,7 @@ func Test_newServer(t *testing.T) {
 		clientBurst:    1,
 		clientPageSize: 100,
 	}, logger)
-	assert.NotNil(t, err)
+	assert.Error(t, err)
 }
 
 func Test_namespaceExists(t *testing.T) {
@@ -250,10 +250,10 @@ func Test_namespaceExists(t *testing.T) {
 	}
 
 	// namespace doesn't exist
-	assert.NotNil(t, server.namespaceExists("not-exist"))
+	assert.Error(t, server.namespaceExists("not-exist"))
 
 	// namespace exists
-	assert.Nil(t, server.namespaceExists("velero"))
+	assert.NoError(t, server.namespaceExists("velero"))
 }
 
 func Test_veleroResourcesExist(t *testing.T) {
@@ -265,7 +265,7 @@ func Test_veleroResourcesExist(t *testing.T) {
 
 	// velero resources don't exist
 	helper.On("Resources").Return(nil)
-	assert.NotNil(t, server.veleroResourcesExist())
+	assert.Error(t, server.veleroResourcesExist())
 
 	// velero resources exist
 	helper.On("Resources").Unset()
@@ -294,7 +294,7 @@ func Test_veleroResourcesExist(t *testing.T) {
 			},
 		},
 	})
-	assert.Nil(t, server.veleroResourcesExist())
+	assert.NoError(t, server.veleroResourcesExist())
 }
 
 func Test_markInProgressBackupsFailed(t *testing.T) {
@@ -329,11 +329,11 @@ func Test_markInProgressBackupsFailed(t *testing.T) {
 	markInProgressBackupsFailed(context.Background(), c, "velero", logrus.New())
 
 	backup01 := &velerov1api.Backup{}
-	require.Nil(t, c.Get(context.Background(), client.ObjectKey{Namespace: "velero", Name: "backup01"}, backup01))
+	require.NoError(t, c.Get(context.Background(), client.ObjectKey{Namespace: "velero", Name: "backup01"}, backup01))
 	assert.Equal(t, velerov1api.BackupPhaseFailed, backup01.Status.Phase)
 
 	backup02 := &velerov1api.Backup{}
-	require.Nil(t, c.Get(context.Background(), client.ObjectKey{Namespace: "velero", Name: "backup02"}, backup02))
+	require.NoError(t, c.Get(context.Background(), client.ObjectKey{Namespace: "velero", Name: "backup02"}, backup02))
 	assert.Equal(t, velerov1api.BackupPhaseCompleted, backup02.Status.Phase)
 }
 
@@ -369,11 +369,11 @@ func Test_markInProgressRestoresFailed(t *testing.T) {
 	markInProgressRestoresFailed(context.Background(), c, "velero", logrus.New())
 
 	restore01 := &velerov1api.Restore{}
-	require.Nil(t, c.Get(context.Background(), client.ObjectKey{Namespace: "velero", Name: "restore01"}, restore01))
+	require.NoError(t, c.Get(context.Background(), client.ObjectKey{Namespace: "velero", Name: "restore01"}, restore01))
 	assert.Equal(t, velerov1api.RestorePhaseFailed, restore01.Status.Phase)
 
 	restore02 := &velerov1api.Restore{}
-	require.Nil(t, c.Get(context.Background(), client.ObjectKey{Namespace: "velero", Name: "restore02"}, restore02))
+	require.NoError(t, c.Get(context.Background(), client.ObjectKey{Namespace: "velero", Name: "restore02"}, restore02))
 	assert.Equal(t, velerov1api.RestorePhaseCompleted, restore02.Status.Phase)
 }
 
@@ -403,11 +403,11 @@ func Test_setDefaultBackupLocation(t *testing.T) {
 	setDefaultBackupLocation(context.Background(), c, "velero", "default", logrus.New())
 
 	defaultLocation := &velerov1api.BackupStorageLocation{}
-	require.Nil(t, c.Get(context.Background(), client.ObjectKey{Namespace: "velero", Name: "default"}, defaultLocation))
+	require.NoError(t, c.Get(context.Background(), client.ObjectKey{Namespace: "velero", Name: "default"}, defaultLocation))
 	assert.True(t, defaultLocation.Spec.Default)
 
 	nonDefaultLocation := &velerov1api.BackupStorageLocation{}
-	require.Nil(t, c.Get(context.Background(), client.ObjectKey{Namespace: "velero", Name: "non-default"}, nonDefaultLocation))
+	require.NoError(t, c.Get(context.Background(), client.ObjectKey{Namespace: "velero", Name: "non-default"}, nonDefaultLocation))
 	assert.False(t, nonDefaultLocation.Spec.Default)
 
 	// no default location specified

pkg/cmd/util/flag/array_test.go

@@ -14,7 +14,7 @@ func TestStringOfStringArray(t *testing.T) {
 
 func TestSetOfStringArray(t *testing.T) {
 	array := NewStringArray()
-	require.Nil(t, array.Set("a,b"))
+	require.NoError(t, array.Set("a,b"))
 	assert.Equal(t, "a,b", array.String())
 }
 

pkg/cmd/util/flag/enum_test.go

@@ -14,9 +14,9 @@ func TestStringOfEnum(t *testing.T) {
 
 func TestSetOfEnum(t *testing.T) {
 	enum := NewEnum("a", "a", "b", "c")
-	assert.NotNil(t, enum.Set("d"))
+	assert.Error(t, enum.Set("d"))
 
-	require.Nil(t, enum.Set("b"))
+	require.NoError(t, enum.Set("b"))
 	assert.Equal(t, "b", enum.String())
 }
 

pkg/cmd/util/flag/label_selector_test.go

@@ -10,7 +10,7 @@ import (
 
 func TestStringOfLabelSelector(t *testing.T) {
 	ls, err := metav1.ParseToLabelSelector("k1=v1,k2=v2")
-	require.Nil(t, err)
+	require.NoError(t, err)
 	selector := &LabelSelector{
 		LabelSelector: ls,
 	}
@@ -19,7 +19,7 @@ func TestStringOfLabelSelector(t *testing.T) {
 
 func TestSetOfLabelSelector(t *testing.T) {
 	selector := &LabelSelector{}
-	require.Nil(t, selector.Set("k1=v1,k2=v2"))
+	require.NoError(t, selector.Set("k1=v1,k2=v2"))
 	str := selector.String()
 	assert.True(t, str == "k1=v1,k2=v2" || str == "k2=v2,k2=v2")
 }