Skip to content

Commit

Permalink
chore: refactor test assertions to be more idiomatic
Browse files Browse the repository at this point in the history
  • Loading branch information
alexandear committed Oct 30, 2024
1 parent b1c4c42 commit 7b17027
Show file tree
Hide file tree
Showing 14 changed files with 34 additions and 22 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
13 changes: 13 additions & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,27 @@ 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:
rules:
- name: exported
arguments:
- disableStutteringCheck
testifylint:
disable-all: true
enable:
- error-nil
2 changes: 1 addition & 1 deletion internal/pkg/addon/output_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions internal/pkg/aws/apprunner/apprunner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
Expand Down Expand Up @@ -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)
}
})
Expand Down
2 changes: 1 addition & 1 deletion internal/pkg/aws/s3/s3_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
Expand Down
2 changes: 1 addition & 1 deletion internal/pkg/aws/sessions/sessions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
Expand Down
1 change: 0 additions & 1 deletion internal/pkg/cli/pipeline_override_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
8 changes: 4 additions & 4 deletions internal/pkg/cli/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand Down Expand Up @@ -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)
}
})
}
Expand All @@ -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)
}
})
}
Expand Down Expand Up @@ -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)
}
})
}
Expand Down
2 changes: 1 addition & 1 deletion internal/pkg/deploy/cloudformation/cloudformation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions internal/pkg/docker/dockerengine/dockerengine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
Expand Down Expand Up @@ -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])
})
Expand Down
4 changes: 2 additions & 2 deletions internal/pkg/ecs/run_task_request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
Expand Down Expand Up @@ -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)
})
}
Expand Down
2 changes: 1 addition & 1 deletion internal/pkg/manifest/svc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
6 changes: 3 additions & 3 deletions internal/pkg/manifest/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions internal/pkg/term/progress/cloudformation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
})
Expand All @@ -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())
Expand Down

0 comments on commit 7b17027

Please sign in to comment.