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

chore: refactor test assertions to be more idiomatic #5963

Open
wants to merge 1 commit into
base: mainline
Choose a base branch
from
Open
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
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
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