From 7b17027bc33141f0ba68e6634a7377f360b25f6b Mon Sep 17 00:00:00 2001 From: Oleksandr Redko Date: Wed, 30 Oct 2024 22:52:36 +0200 Subject: [PATCH] chore: refactor test assertions to be more idiomatic --- .github/workflows/ci.yml | 2 +- .golangci.yml | 13 +++++++++++++ internal/pkg/addon/output_test.go | 2 +- internal/pkg/aws/apprunner/apprunner_test.go | 4 ++-- internal/pkg/aws/s3/s3_test.go | 2 +- internal/pkg/aws/sessions/sessions_test.go | 2 +- internal/pkg/cli/pipeline_override_test.go | 1 - internal/pkg/cli/validate_test.go | 8 ++++---- .../deploy/cloudformation/cloudformation_test.go | 2 +- .../pkg/docker/dockerengine/dockerengine_test.go | 4 ++-- internal/pkg/ecs/run_task_request_test.go | 4 ++-- internal/pkg/manifest/svc_test.go | 2 +- internal/pkg/manifest/validate_test.go | 6 +++--- internal/pkg/term/progress/cloudformation_test.go | 4 ++-- 14 files changed, 34 insertions(+), 22 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 42036324cba..630913c2a64 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -106,7 +106,7 @@ jobs: - name: Run golangci-lint uses: golangci/golangci-lint-action@v3 with: - version: v1.54 + version: v1.60 args: --timeout 5m0s license: diff --git a/.golangci.yml b/.golangci.yml index 824e032ab15..f148fc12a1e 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -8,10 +8,19 @@ issues: - errcheck - path: internal/pkg/exec/(.+)\.go text: 'is unused' + - text: "S1009: should omit nil check" # TODO: remove this line after fixing all the issues + linters: + - gosimple + - text: "printf: non-constant format string in call to" # TODO: remove this line after fixing all the issues + linters: + - govet + + linters: enable: - revive + - testifylint linters-settings: revive: @@ -19,3 +28,7 @@ linters-settings: - name: exported arguments: - disableStutteringCheck + testifylint: + disable-all: true + enable: + - error-nil diff --git a/internal/pkg/addon/output_test.go b/internal/pkg/addon/output_test.go index 1ab6cc474f0..9ce98afdc03 100644 --- a/internal/pkg/addon/output_test.go +++ b/internal/pkg/addon/output_test.go @@ -136,7 +136,7 @@ Outputs: // THEN if tc.wantedErr != nil { - require.NotNil(t, err, "expected a non-nil error to be returned") + require.Error(t, err, "expected a non-nil error to be returned") require.True(t, strings.HasPrefix(err.Error(), tc.wantedErr.Error()), "expected the error %v to be wrapped by our prefix %v", err, tc.wantedErr) } else { require.NoError(t, err) diff --git a/internal/pkg/aws/apprunner/apprunner_test.go b/internal/pkg/aws/apprunner/apprunner_test.go index afa94170ef7..b4d82cc58c3 100644 --- a/internal/pkg/aws/apprunner/apprunner_test.go +++ b/internal/pkg/aws/apprunner/apprunner_test.go @@ -293,7 +293,7 @@ func Test_LogGroupName(t *testing.T) { if tc.wantErr != nil { require.EqualError(t, err, tc.wantErr.Error()) } else { - require.Equal(t, nil, err) + require.NoError(t, err) require.Equal(t, tc.wantLogGroupName, logGroupName) } }) @@ -327,7 +327,7 @@ func Test_SystemLogGroupName(t *testing.T) { if tc.wantErr != nil { require.EqualError(t, err, tc.wantErr.Error()) } else { - require.Equal(t, nil, err) + require.NoError(t, err) require.Equal(t, tc.wantLogGroupName, logGroupName) } }) diff --git a/internal/pkg/aws/s3/s3_test.go b/internal/pkg/aws/s3/s3_test.go index bf1485f1f9f..fb76fc92df7 100644 --- a/internal/pkg/aws/s3/s3_test.go +++ b/internal/pkg/aws/s3/s3_test.go @@ -72,7 +72,7 @@ func TestS3_Upload(t *testing.T) { if gotErr != nil { require.EqualError(t, gotErr, tc.wantError.Error()) } else { - require.Equal(t, gotErr, nil) + require.NoError(t, gotErr) require.Equal(t, gotURL, tc.wantedURL) } }) diff --git a/internal/pkg/aws/sessions/sessions_test.go b/internal/pkg/aws/sessions/sessions_test.go index 007660f5a21..4e0576c7f8a 100644 --- a/internal/pkg/aws/sessions/sessions_test.go +++ b/internal/pkg/aws/sessions/sessions_test.go @@ -169,7 +169,7 @@ func TestProvider_FromProfile(t *testing.T) { sess, err := ImmutableProvider().FromProfile("walk-like-an-egyptian") // THEN - require.NotNil(t, err) + require.Error(t, err) require.EqualError(t, errors.New("missing region configuration"), err.Error()) require.Nil(t, sess) }) diff --git a/internal/pkg/cli/pipeline_override_test.go b/internal/pkg/cli/pipeline_override_test.go index e747bf2d455..6303c7ebd9d 100644 --- a/internal/pkg/cli/pipeline_override_test.go +++ b/internal/pkg/cli/pipeline_override_test.go @@ -383,7 +383,6 @@ func TestOverridePipeline_Execute(t *testing.T) { } require.Error(t, err) - require.NotNil(t, err) require.Contains(t, err.Error(), tc.wanted.Error()) } else { if err != nil { diff --git a/internal/pkg/cli/validate_test.go b/internal/pkg/cli/validate_test.go index 22ae6b44e4a..c2e778d2d4c 100644 --- a/internal/pkg/cli/validate_test.go +++ b/internal/pkg/cli/validate_test.go @@ -364,7 +364,7 @@ func TestValidatePath(t *testing.T) { // THEN if tc.want == nil { - require.Nil(t, got) + require.NoError(t, got) } else { require.EqualError(t, tc.want, got.Error()) } @@ -518,7 +518,7 @@ func TestValidateLSIs(t *testing.T) { if tc.wantError != nil { require.EqualError(t, got, tc.wantError.Error()) } else { - require.Nil(t, got) + require.NoError(t, got) } }) } @@ -544,7 +544,7 @@ func TestValidateCIDR(t *testing.T) { if tc.wantError != nil { require.EqualError(t, got, tc.wantError.Error()) } else { - require.Nil(t, got) + require.NoError(t, got) } }) } @@ -708,7 +708,7 @@ func TestValidateCron(t *testing.T) { if tc.shouldPass { require.NoError(t, got) } else { - require.NotNil(t, got) + require.Error(t, got) } }) } diff --git a/internal/pkg/deploy/cloudformation/cloudformation_test.go b/internal/pkg/deploy/cloudformation/cloudformation_test.go index 81f012f66c3..1abd8070644 100644 --- a/internal/pkg/deploy/cloudformation/cloudformation_test.go +++ b/internal/pkg/deploy/cloudformation/cloudformation_test.go @@ -626,7 +626,7 @@ func testDeployTask_ReturnNilOnEmptyChangeSetWhileUpdatingStack(t *testing.T, wh err := when(client) // THEN - require.Nil(t, err, "should not fail if the changeset is empty") + require.NoError(t, err, "should not fail if the changeset is empty") } func testDeployTask_OnUpdateChangeSetFailure(t *testing.T, when func(cf CloudFormation) error) { diff --git a/internal/pkg/docker/dockerengine/dockerengine_test.go b/internal/pkg/docker/dockerengine/dockerengine_test.go index 10d6c342379..0a5e152f2b6 100644 --- a/internal/pkg/docker/dockerengine/dockerengine_test.go +++ b/internal/pkg/docker/dockerengine/dockerengine_test.go @@ -235,7 +235,7 @@ func TestDockerCommand_Build(t *testing.T) { if tc.wantedError != nil { require.EqualError(t, tc.wantedError, got.Error()) } else { - require.Nil(t, got) + require.NoError(t, got) } }) } @@ -815,7 +815,7 @@ func TestDockerCommand_Run(t *testing.T) { return } - require.Nil(t, err) + require.NoError(t, err) split := strings.Split(out.String(), "\n") require.ElementsMatch(t, tc.wantedOutput, split[:len(split)-1]) }) diff --git a/internal/pkg/ecs/run_task_request_test.go b/internal/pkg/ecs/run_task_request_test.go index 07316d5cc0d..273a256dc1a 100644 --- a/internal/pkg/ecs/run_task_request_test.go +++ b/internal/pkg/ecs/run_task_request_test.go @@ -443,7 +443,7 @@ func TestRunTaskRequest_CLIString(t *testing.T) { for name, tc := range testCases { t.Run(name, func(t *testing.T) { got, err := tc.in.CLIString() - require.Nil(t, err) + require.NoError(t, err) require.Equal(t, tc.wanted, got) }) } @@ -475,7 +475,7 @@ func TestRunTaskRequest_fmtStringMapToString(t *testing.T) { for name, tc := range testCases { t.Run(name, func(t *testing.T) { got, err := fmtStringMapToString(tc.in) - require.Nil(t, err) + require.NoError(t, err) require.Equal(t, tc.wanted, got) }) } diff --git a/internal/pkg/manifest/svc_test.go b/internal/pkg/manifest/svc_test.go index db896d33e6b..3c84716df8c 100644 --- a/internal/pkg/manifest/svc_test.go +++ b/internal/pkg/manifest/svc_test.go @@ -888,7 +888,7 @@ func TestQueueScaling_AcceptableBacklogPerTask(t *testing.T) { t.Run(name, func(t *testing.T) { actual, err := tc.in.AcceptableBacklogPerTask() if tc.wantedErr != nil { - require.NotNil(t, err) + require.Error(t, err) } else { require.Equal(t, tc.wantedBacklog, actual) } diff --git a/internal/pkg/manifest/validate_test.go b/internal/pkg/manifest/validate_test.go index 9983dbd893d..b0cda58b6b2 100644 --- a/internal/pkg/manifest/validate_test.go +++ b/internal/pkg/manifest/validate_test.go @@ -3166,7 +3166,7 @@ func TestObservability_validate(t *testing.T) { gotErr := tc.config.validate() if tc.wantedErrorPrefix != "" { - require.NotNil(t, gotErr) + require.Error(t, gotErr) require.Contains(t, gotErr.Error(), tc.wantedErrorPrefix) } else { require.NoError(t, gotErr) @@ -3895,7 +3895,7 @@ func TestDeploymentConfig_validate(t *testing.T) { gotErr := tc.deployConfig.validate() if tc.wanted != "" { - require.NotNil(t, gotErr) + require.Error(t, gotErr) require.Contains(t, gotErr.Error(), tc.wanted) } else { require.NoError(t, gotErr) @@ -3926,7 +3926,7 @@ func TestFromEnvironment_validate(t *testing.T) { gotErr := tc.in.validate() if tc.wantedError != nil { - require.NotNil(t, gotErr) + require.Error(t, gotErr) require.EqualError(t, gotErr, tc.wantedError.Error()) } else { require.NoError(t, gotErr) diff --git a/internal/pkg/term/progress/cloudformation_test.go b/internal/pkg/term/progress/cloudformation_test.go index 43e73b9b09c..72735148965 100644 --- a/internal/pkg/term/progress/cloudformation_test.go +++ b/internal/pkg/term/progress/cloudformation_test.go @@ -594,7 +594,7 @@ func TestEcsServiceResourceComponent_Render(t *testing.T) { nl, err := c.Render(buf) // THEN - require.Nil(t, err) + require.NoError(t, err) require.Equal(t, 1, nl) require.Equal(t, "resource\n", buf.String()) }) @@ -614,7 +614,7 @@ func TestEcsServiceResourceComponent_Render(t *testing.T) { nl, err := c.Render(buf) // THEN - require.Nil(t, err) + require.NoError(t, err) require.Equal(t, 2, nl) require.Equal(t, "resource\n"+ "deployment\t\t\n", buf.String())