Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Mar 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 25 additions & 7 deletions golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -302,8 +319,10 @@ linters:
- bodyclose
- dogsled
- durationcheck
- dupword
- errcheck
- exportloopref
- errchkjson
- goconst
- gofmt
- goheader
Expand All @@ -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


Expand Down
2 changes: 1 addition & 1 deletion hack/build-image/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion internal/hook/item_hook_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
}
12 changes: 6 additions & 6 deletions pkg/backup/backup_pv_action_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand All @@ -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)
}
2 changes: 1 addition & 1 deletion pkg/backup/pv_skip_tracker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
2 changes: 1 addition & 1 deletion pkg/client/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions pkg/cmd/cli/backuplocation/delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})

Expand All @@ -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))
})
}
12 changes: 6 additions & 6 deletions pkg/controller/backup_deletion_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,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])
})

Expand Down Expand Up @@ -206,7 +206,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])
})

Expand All @@ -221,7 +221,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) {
Expand All @@ -236,7 +236,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])
})

Expand All @@ -253,7 +253,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) {
Expand Down Expand Up @@ -684,7 +684,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])

})
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/pod_volume_restore_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/restore_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/restore_finalizer_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions pkg/controller/schedule_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/datapath/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions pkg/install/daemonset_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions pkg/install/deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion pkg/metrics/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
14 changes: 7 additions & 7 deletions pkg/podvolume/backupper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
2 changes: 1 addition & 1 deletion pkg/repository/udmrepo/kopialib/backend/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion pkg/util/stringslice/stringslice_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down
Loading