Skip to content

Commit

Permalink
testifylint: enable go-require, float-compare and require-error rules
Browse files Browse the repository at this point in the history
Signed-off-by: Matthieu MOREL <[email protected]>
  • Loading branch information
mmorel-35 committed Jul 18, 2024
1 parent 3e9f6cc commit a14de0f
Show file tree
Hide file tree
Showing 102 changed files with 602 additions and 585 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/pr-linter-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,5 @@ jobs:
- name: Linter check
uses: golangci/golangci-lint-action@v6
with:
version: v1.57.2
version: v1.59.1
args: --verbose
5 changes: 0 additions & 5 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -236,11 +236,6 @@ linters-settings:
packages:
- github.com/jmoiron/sqlx
testifylint:
# TODO: enable them all
disable:
- go-require
- float-compare
- require-error
enable-all: true
testpackage:
# regexp pattern to skip files
Expand Down
2 changes: 1 addition & 1 deletion hack/build-image/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,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.57.2
RUN curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.59.1

# 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
13 changes: 7 additions & 6 deletions internal/hook/hook_tracker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestNewHookTracker(t *testing.T) {
Expand Down Expand Up @@ -65,13 +66,13 @@ func TestHookTracker_Record(t *testing.T) {
info := tracker.tracker[key]
assert.True(t, info.hookFailed)
assert.True(t, info.hookExecuted)
assert.NoError(t, err)
require.NoError(t, err)

err = tracker.Record("ns2", "pod2", "container1", HookSourceAnnotation, "h1", "", true, fmt.Errorf("err"))
assert.Error(t, err)
require.Error(t, err)

err = tracker.Record("ns1", "pod1", "container1", HookSourceAnnotation, "h1", "", false, nil)
assert.NoError(t, err)
require.NoError(t, err)
assert.True(t, info.hookFailed)
}

Expand Down Expand Up @@ -141,13 +142,13 @@ func TestMultiHookTracker_Record(t *testing.T) {
info := mht.trackers["restore1"].tracker[key]
assert.True(t, info.hookFailed)
assert.True(t, info.hookExecuted)
assert.NoError(t, err)
require.NoError(t, err)

err = mht.Record("restore1", "ns2", "pod2", "container1", HookSourceAnnotation, "h1", "", true, fmt.Errorf("err"))
assert.Error(t, err)
require.Error(t, err)

err = mht.Record("restore2", "ns2", "pod2", "container1", HookSourceAnnotation, "h1", "", true, fmt.Errorf("err"))
assert.Error(t, err)
require.Error(t, err)
}

func TestMultiHookTracker_Stat(t *testing.T) {
Expand Down
12 changes: 6 additions & 6 deletions internal/hook/item_hook_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ func TestHandleHooksSkips(t *testing.T) {

groupResource := schema.ParseGroupResource(test.groupResource)
err := h.HandleHooks(velerotest.NewLogger(), groupResource, test.item, test.hooks, PhasePre, hookTracker)
assert.NoError(t, err)
require.NoError(t, err)
})
}
}
Expand Down Expand Up @@ -490,7 +490,7 @@ func TestHandleHooks(t *testing.T) {
err := h.HandleHooks(velerotest.NewLogger(), groupResource, test.item, test.hooks, test.phase, hookTracker)

if test.expectedError != nil {
assert.EqualError(t, err, test.expectedError.Error())
require.EqualError(t, err, test.expectedError.Error())
return
}

Expand Down Expand Up @@ -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.NoError(t, err)
require.NoError(t, err)
assert.Equal(t, tc.expected, actual)
})
}
Expand Down Expand Up @@ -1955,13 +1955,13 @@ func TestHandleRestoreHooks(t *testing.T) {
t.Run(tc.name, func(t *testing.T) {
handler := InitContainerRestoreHookHandler{}
podMap, err := runtime.DefaultUnstructuredConverter.ToUnstructured(&tc.podInput)
assert.NoError(t, err)
require.NoError(t, err)
actual, err := handler.HandleRestoreHooks(velerotest.NewLogger(), kuberesource.Pods, &unstructured.Unstructured{Object: podMap}, tc.restoreHooks, tc.namespaceMapping)
assert.Equal(t, tc.expectedError, err)
if actual != nil {
actualPod := new(corev1api.Pod)
err = runtime.DefaultUnstructuredConverter.FromUnstructured(actual.UnstructuredContent(), actualPod)
assert.NoError(t, err)
require.NoError(t, err)
assert.Equal(t, tc.expectedPod, actualPod)
}
})
Expand All @@ -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.NoError(t, ValidateContainer([]byte(valid)))
require.NoError(t, ValidateContainer([]byte(valid)))

// noName string should return expected error as result.
assert.Equal(t, expectedError, ValidateContainer([]byte(noName)))
Expand Down
6 changes: 3 additions & 3 deletions internal/hook/wait_exec_hook_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -732,7 +732,7 @@ func TestWaitExecHandleHooks(t *testing.T) {

for _, e := range test.expectedExecutions {
obj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(e.pod)
assert.NoError(t, err)
require.NoError(t, err)
podCommandExecutor.On("ExecutePodCommand", mock.Anything, obj, e.pod.Namespace, e.pod.Name, e.name, e.hook).Return(e.error)
}

Expand All @@ -749,7 +749,7 @@ func TestWaitExecHandleHooks(t *testing.T) {
// for i, ee := range test.expectedErrors {
require.Len(t, errs, len(test.expectedErrors))
for i, ee := range test.expectedErrors {
assert.EqualError(t, errs[i], ee.Error())
require.EqualError(t, errs[i], ee.Error())
}
})
}
Expand Down Expand Up @@ -1269,7 +1269,7 @@ func TestRestoreHookTrackerUpdate(t *testing.T) {

for _, e := range test.expectedExecutions {
obj, err := runtime.DefaultUnstructuredConverter.ToUnstructured(e.pod)
assert.NoError(t, err)
require.NoError(t, err)
podCommandExecutor.On("ExecutePodCommand", mock.Anything, obj, e.pod.Namespace, e.pod.Name, e.name, e.hook).Return(e.error)
}

Expand Down
6 changes: 3 additions & 3 deletions internal/resourcemodifiers/json_merge_patch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"testing"

"github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
clientgoscheme "k8s.io/client-go/kubernetes/scheme"
Expand All @@ -28,14 +28,14 @@ func TestJsonMergePatchFailure(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
scheme := runtime.NewScheme()
err := clientgoscheme.AddToScheme(scheme)
assert.NoError(t, err)
require.NoError(t, err)
pt := &JSONMergePatcher{
patches: []JSONMergePatch{{PatchData: tt.data}},
}

u := &unstructured.Unstructured{}
_, err = pt.Patch(u, logrus.New())
assert.Error(t, err)
require.Error(t, err)
})
}
}
27 changes: 14 additions & 13 deletions internal/resourcemodifiers/resource_modifiers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (

"github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
v1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
Expand Down Expand Up @@ -1303,27 +1304,27 @@ func TestResourceModifiers_ApplyResourceModifierRules_StrategicMergePatch(t *tes
utilruntime.Must(clientgoscheme.AddToScheme(scheme))
unstructuredSerializer := yaml.NewDecodingSerializer(unstructured.UnstructuredJSONScheme)
o1, _, err := unstructuredSerializer.Decode([]byte(podYAMLWithNFSVolume), nil, nil)
assert.NoError(t, err)
require.NoError(t, err)
podWithNFSVolume := o1.(*unstructured.Unstructured)

o2, _, err := unstructuredSerializer.Decode([]byte(podYAMLWithPVCVolume), nil, nil)
assert.NoError(t, err)
require.NoError(t, err)
podWithPVCVolume := o2.(*unstructured.Unstructured)

o3, _, err := unstructuredSerializer.Decode([]byte(svcYAMLWithPort8000), nil, nil)
assert.NoError(t, err)
require.NoError(t, err)
svcWithPort8000 := o3.(*unstructured.Unstructured)

o4, _, err := unstructuredSerializer.Decode([]byte(svcYAMLWithPort9000), nil, nil)
assert.NoError(t, err)
require.NoError(t, err)
svcWithPort9000 := o4.(*unstructured.Unstructured)

o5, _, err := unstructuredSerializer.Decode([]byte(podYAMLWithNginxImage), nil, nil)
assert.NoError(t, err)
require.NoError(t, err)
podWithNginxImage := o5.(*unstructured.Unstructured)

o6, _, err := unstructuredSerializer.Decode([]byte(podYAMLWithNginx1Image), nil, nil)
assert.NoError(t, err)
require.NoError(t, err)
podWithNginx1Image := o6.(*unstructured.Unstructured)

tests := []struct {
Expand Down Expand Up @@ -1467,15 +1468,15 @@ func TestResourceModifiers_ApplyResourceModifierRules_StrategicMergePatch(t *tes
func TestResourceModifiers_ApplyResourceModifierRules_JSONMergePatch(t *testing.T) {
unstructuredSerializer := yaml.NewDecodingSerializer(unstructured.UnstructuredJSONScheme)
o1, _, err := unstructuredSerializer.Decode([]byte(cmYAMLWithLabelAToB), nil, nil)
assert.NoError(t, err)
require.NoError(t, err)
cmWithLabelAToB := o1.(*unstructured.Unstructured)

o2, _, err := unstructuredSerializer.Decode([]byte(cmYAMLWithLabelAToC), nil, nil)
assert.NoError(t, err)
require.NoError(t, err)
cmWithLabelAToC := o2.(*unstructured.Unstructured)

o3, _, err := unstructuredSerializer.Decode([]byte(cmYAMLWithoutLabelA), nil, nil)
assert.NoError(t, err)
require.NoError(t, err)
cmWithoutLabelA := o3.(*unstructured.Unstructured)

tests := []struct {
Expand Down Expand Up @@ -1618,11 +1619,11 @@ func TestResourceModifiers_ApplyResourceModifierRules_JSONMergePatch(t *testing.
func TestResourceModifiers_wildcard_in_GroupResource(t *testing.T) {
unstructuredSerializer := yaml.NewDecodingSerializer(unstructured.UnstructuredJSONScheme)
o1, _, err := unstructuredSerializer.Decode([]byte(cmYAMLWithLabelAToB), nil, nil)
assert.NoError(t, err)
require.NoError(t, err)
cmWithLabelAToB := o1.(*unstructured.Unstructured)

o2, _, err := unstructuredSerializer.Decode([]byte(cmYAMLWithLabelAToC), nil, nil)
assert.NoError(t, err)
require.NoError(t, err)
cmWithLabelAToC := o2.(*unstructured.Unstructured)

tests := []struct {
Expand Down Expand Up @@ -1694,11 +1695,11 @@ func TestResourceModifiers_wildcard_in_GroupResource(t *testing.T) {
func TestResourceModifiers_conditional_patches(t *testing.T) {
unstructuredSerializer := yaml.NewDecodingSerializer(unstructured.UnstructuredJSONScheme)
o1, _, err := unstructuredSerializer.Decode([]byte(cmYAMLWithLabelAToB), nil, nil)
assert.NoError(t, err)
require.NoError(t, err)
cmWithLabelAToB := o1.(*unstructured.Unstructured)

o2, _, err := unstructuredSerializer.Decode([]byte(cmYAMLWithLabelAToC), nil, nil)
assert.NoError(t, err)
require.NoError(t, err)
cmWithLabelAToC := o2.(*unstructured.Unstructured)

tests := []struct {
Expand Down
6 changes: 3 additions & 3 deletions internal/resourcemodifiers/strategic_merge_patch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import (
"testing"

"github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
Expand Down Expand Up @@ -37,7 +37,7 @@ func TestStrategicMergePatchFailure(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
scheme := runtime.NewScheme()
err := clientgoscheme.AddToScheme(scheme)
assert.NoError(t, err)
require.NoError(t, err)
pt := &StrategicMergePatcher{
patches: []StrategicMergePatch{{PatchData: tt.data}},
scheme: scheme,
Expand All @@ -46,7 +46,7 @@ func TestStrategicMergePatchFailure(t *testing.T) {
u := &unstructured.Unstructured{}
u.SetGroupVersionKind(schema.GroupVersionKind{Version: "v1", Kind: tt.kind})
_, err = pt.Patch(u, logrus.New())
assert.Error(t, err)
require.Error(t, err)
})
}
}
9 changes: 5 additions & 4 deletions internal/resourcepolicies/resource_policies_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -232,7 +233,7 @@ func TestGetResourcePoliciesFromConfig(t *testing.T) {

// Call the function and check for errors
resPolicies, err := getResourcePoliciesFromConfig(cm)
assert.NoError(t, err)
require.NoError(t, err)

// Check that the returned resourcePolicies object contains the expected data
assert.Equal(t, "v1", resPolicies.version)
Expand Down Expand Up @@ -422,12 +423,12 @@ volumePolicies:
if err != nil {
t.Fatalf("got error when get match action %v", err)
}
assert.NoError(t, err)
require.NoError(t, err)
policies := &Policies{}
err = policies.BuildPolicy(resPolicies)
assert.NoError(t, err)
require.NoError(t, err)
action, err := policies.GetMatchAction(tc.vol)
assert.NoError(t, err)
require.NoError(t, err)

if tc.skip {
if action.Type != Skip {
Expand Down
2 changes: 1 addition & 1 deletion internal/restartabletest/restartable_delegate.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func RunRestartableDelegateTests(
expectedErr, expectedErrOk := expected[i].(error)
// If both are errors, use EqualError
if actualErrOk && expectedErrOk {
assert.EqualError(t, actualErr, expectedErr.Error())
require.EqualError(t, actualErr, expectedErr.Error())
continue
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/archive/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.NoError(t, err)
require.NoError(t, err)
assert.Equal(t, tc.want, res)
}
})
Expand Down Expand Up @@ -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.NoError(t, err)
require.NoError(t, err)
assert.Equal(t, tc.want, res)
}
})
Expand Down
6 changes: 3 additions & 3 deletions pkg/backup/actions/backup_pv_action_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,14 @@ func TestBackupPVAction(t *testing.T) {
// no spec.volumeName should result in no error
// and no additional items
_, additional, err := a.Execute(pvc, backup)
assert.NoError(t, err)
require.NoError(t, err)
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)
require.NoError(t, err)
assert.Empty(t, additional)

// Action should clean the spec.Selector when the StorageClassName is not set.
Expand Down Expand Up @@ -147,6 +147,6 @@ func TestBackupPVAction(t *testing.T) {
// 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)
require.NoError(t, err)
assert.Empty(t, additional)
}
Loading

0 comments on commit a14de0f

Please sign in to comment.