Skip to content

Commit

Permalink
test: prefer NoError/Error over Nil/NotNil (#1165)
Browse files Browse the repository at this point in the history
replaces calls to require/assert Nil/NotNil to NoError/Error

[NoError](https://godoc.org/github.com/stretchr/testify/require#NoError) is for asserting no error was return
[Error](https://godoc.org/github.com/stretchr/testify/require#Error) is for asserting an error was returned

## Why is it important?

```golang
return Fail(t, fmt.Sprintf("Received unexpected error:\n%+v", err), msgAndArgs...)
```
```golang
return Fail(t, "An error is expected but got nil.", msgAndArgs...)
```
When these fail their message explicitly calls out that we were or were not expecting an error. This communicates the intent of the assertion.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
  • Loading branch information
trashhalo authored Jul 20, 2020
1 parent f6672cd commit 5745352
Show file tree
Hide file tree
Showing 36 changed files with 78 additions and 78 deletions.
10 changes: 5 additions & 5 deletions internal/pkg/addon/storage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ func TestDDBAttributeFromKey(t *testing.T) {
if tc.wantError != nil {
require.EqualError(t, err, tc.wantError.Error())
} else {
require.Nil(t, err)
require.NoError(t, err)
require.Equal(t, tc.wantName, *got.Name)
require.Equal(t, tc.wantType, *got.DataType)
}
Expand Down Expand Up @@ -179,7 +179,7 @@ func TestNewLSI(t *testing.T) {
if tc.wantError != nil {
require.EqualError(t, err, tc.wantError.Error())
} else {
require.Nil(t, err)
require.NoError(t, err)
require.Equal(t, tc.wantedLSI, got)
}
})
Expand Down Expand Up @@ -218,7 +218,7 @@ func TestBuildPartitionKey(t *testing.T) {
if tc.wantError != nil {
require.EqualError(t, err, tc.wantError.Error())
} else {
require.Nil(t, err)
require.NoError(t, err)
require.Equal(t, tc.wantPartitionKey, *props.PartitionKey)
require.Equal(t, tc.wantAttributes, props.Attributes)
}
Expand Down Expand Up @@ -274,7 +274,7 @@ func TestBuildSortKey(t *testing.T) {
if tc.wantError != nil {
require.EqualError(t, err, tc.wantError.Error())
} else {
require.Nil(t, err)
require.NoError(t, err)
require.Equal(t, tc.wantAttributes, props.Attributes)
require.Equal(t, tc.wantHasSortKey, got)
if tc.wantSortKey == "" {
Expand Down Expand Up @@ -357,7 +357,7 @@ func TestBuildLocalSecondaryIndex(t *testing.T) {
if tc.wantedError != nil {
require.EqualError(t, err, tc.wantedError.Error())
} else {
require.Nil(t, err)
require.NoError(t, err)
require.Equal(t, tc.wantedHasLSI, props.HasLSI)
require.Equal(t, tc.wantedHasLSI, got)
for idx, att := range props.Attributes {
Expand Down
6 changes: 3 additions & 3 deletions internal/pkg/aws/ec2/ec2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func TestEC2_PublicSubnetIDs(t *testing.T) {
if tc.wantedError != nil {
require.EqualError(t, tc.wantedError, err.Error())
} else {
require.Nil(t, err)
require.NoError(t, err)
require.Equal(t, tc.wantedARNs, arns)
}
})
Expand Down Expand Up @@ -145,7 +145,7 @@ func TestEC2_SubnetIDs(t *testing.T) {
if tc.wantedError != nil {
require.EqualError(t, tc.wantedError, err.Error())
} else {
require.Nil(t, err)
require.NoError(t, err)
require.Equal(t, tc.wantedARNs, arns)
}
})
Expand Down Expand Up @@ -206,7 +206,7 @@ func TestEC2_SecurityGroups(t *testing.T) {
if tc.wantedError != nil {
require.EqualError(t, tc.wantedError, err.Error())
} else {
require.Nil(t, err)
require.NoError(t, err)
require.Equal(t, tc.wantedARNs, arns)
}
})
Expand Down
4 changes: 2 additions & 2 deletions internal/pkg/cli/app_init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ func TestInitAppOpts_Ask(t *testing.T) {
if tc.wantedErr != "" {
require.EqualError(t, err, tc.wantedErr)
} else {
require.Nil(t, err)
require.NoError(t, err)
require.Equal(t, tc.wantedAppName, opts.AppName)
}
})
Expand Down Expand Up @@ -276,7 +276,7 @@ func TestInitAppOpts_Validate(t *testing.T) {
if tc.wantedError != "" {
require.EqualError(t, err, tc.wantedError)
} else {
require.Nil(t, err)
require.NoError(t, err)
}
})
}
Expand Down
6 changes: 3 additions & 3 deletions internal/pkg/cli/app_show_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func TestShowAppOpts_Validate(t *testing.T) {
if tc.wantedError != nil {
require.EqualError(t, err, tc.wantedError.Error())
} else {
require.Nil(t, err)
require.NoError(t, err)
}
})
}
Expand Down Expand Up @@ -151,7 +151,7 @@ func TestShowAppOpts_Ask(t *testing.T) {
if tc.wantedError != nil {
require.EqualError(t, err, tc.wantedError.Error())
} else {
require.Nil(t, err)
require.NoError(t, err)
require.Equal(t, tc.wantedApp, opts.AppName(), "expected app names to match")

}
Expand Down Expand Up @@ -323,7 +323,7 @@ Services
if tc.wantedError != nil {
require.EqualError(t, err, tc.wantedError.Error())
} else {
require.Nil(t, err)
require.NoError(t, err)
require.Equal(t, tc.wantedContent, b.String(), "expected output content match")
}
})
Expand Down
2 changes: 1 addition & 1 deletion internal/pkg/cli/dockerfile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func TestValidateDockerfiles(t *testing.T) {
if tc.err != nil {
require.EqualError(t, err, tc.err.Error())
} else {
require.Nil(t, err)
require.NoError(t, err)
require.Equal(t, wantedDockerfiles, got)
}
})
Expand Down
4 changes: 2 additions & 2 deletions internal/pkg/cli/env_delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ func TestDeleteEnvOpts_Ask(t *testing.T) {
if tc.wantedError == nil {
require.Equal(t, tc.wantedEnvName, opts.EnvName)
require.Equal(t, tc.wantedEnvProfile, opts.EnvProfile)
require.Nil(t, err)
require.NoError(t, err)
} else {
require.EqualError(t, err, tc.wantedError.Error())
}
Expand Down Expand Up @@ -355,7 +355,7 @@ func TestDeleteEnvOpts_Execute(t *testing.T) {
if tc.wantedError != nil {
require.EqualError(t, err, tc.wantedError.Error())
} else {
require.Nil(t, err)
require.NoError(t, err)
}
})
}
Expand Down
6 changes: 3 additions & 3 deletions internal/pkg/cli/env_init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func TestInitEnvOpts_Validate(t *testing.T) {
if tc.wantedErr != "" {
require.EqualError(t, err, tc.wantedErr)
} else {
require.Nil(t, err)
require.NoError(t, err)
}
})
}
Expand Down Expand Up @@ -592,7 +592,7 @@ func TestInitEnvOpts_Execute(t *testing.T) {
if tc.wantedErrorS != "" {
require.EqualError(t, err, tc.wantedErrorS)
} else {
require.Nil(t, err)
require.NoError(t, err)
}
})
}
Expand Down Expand Up @@ -711,7 +711,7 @@ func TestInitEnvOpts_delegateDNSFromApp(t *testing.T) {
if tc.wantedErr != "" {
require.EqualError(t, err, tc.wantedErr)
} else {
require.Nil(t, err)
require.NoError(t, err)
}
})
}
Expand Down
6 changes: 3 additions & 3 deletions internal/pkg/cli/env_show_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ func TestEnvShow_Validate(t *testing.T) {
if tc.wantedError != nil {
require.EqualError(t, err, tc.wantedError.Error())
} else {
require.Nil(t, err)
require.NoError(t, err)
}
})
}
Expand Down Expand Up @@ -199,7 +199,7 @@ func TestEnvShow_Ask(t *testing.T) {
if tc.wantedError != nil {
require.EqualError(t, err, tc.wantedError.Error())
} else {
require.Nil(t, err)
require.NoError(t, err)
require.Equal(t, tc.wantedApp, showEnvs.AppName(), "expected app name to match")
require.Equal(t, tc.wantedEnv, showEnvs.envName, "expected environment name to match")
}
Expand Down Expand Up @@ -338,7 +338,7 @@ func TestEnvShow_Execute(t *testing.T) {
if tc.wantedError != nil {
require.EqualError(t, err, tc.wantedError.Error())
} else {
require.Nil(t, err)
require.NoError(t, err)
require.Equal(t, tc.wantedContent, b.String(), "expected output content match")
}
})
Expand Down
2 changes: 1 addition & 1 deletion internal/pkg/cli/init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ func TestInitOpts_Run(t *testing.T) {
if tc.wantedError != "" {
require.EqualError(t, err, tc.wantedError)
} else {
require.Nil(t, err)
require.NoError(t, err)
}
})
}
Expand Down
6 changes: 3 additions & 3 deletions internal/pkg/cli/pipeline_init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ func TestInitPipelineOpts_Ask(t *testing.T) {
if tc.expectedError != nil {
require.EqualError(t, err, tc.expectedError.Error())
} else {
require.Nil(t, err)
require.NoError(t, err)
require.Equal(t, tc.expectedGitHubOwner, opts.GitHubOwner)
require.Equal(t, tc.expectedGitHubRepo, opts.GitHubRepo)
require.Equal(t, tc.expectedGitHubAccessToken, opts.GitHubAccessToken)
Expand Down Expand Up @@ -287,7 +287,7 @@ func TestInitPipelineOpts_Validate(t *testing.T) {
if tc.expectedError != nil {
require.Equal(t, tc.expectedError, err)
} else {
require.Nil(t, err)
require.NoError(t, err)
}
})
}
Expand Down Expand Up @@ -601,7 +601,7 @@ func TestInitPipelineOpts_Execute(t *testing.T) {
if tc.expectedError != nil {
require.EqualError(t, err, tc.expectedError.Error())
} else {
require.Nil(t, err)
require.NoError(t, err)
}
})
}
Expand Down
6 changes: 3 additions & 3 deletions internal/pkg/cli/pipeline_show_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func TestPipelineShow_Validate(t *testing.T) {
if tc.expectedErr != nil {
require.EqualError(t, err, tc.expectedErr.Error())
} else {
require.Nil(t, err)
require.NoError(t, err)
}
})
}
Expand Down Expand Up @@ -293,7 +293,7 @@ stages:
if tc.expectedErr != nil {
require.EqualError(t, err, tc.expectedErr.Error())
} else {
require.Nil(t, err)
require.NoError(t, err)
require.Equal(t, tc.expectedApp, opts.AppName(), "expected application names to match")
require.Equal(t, tc.expectedPipeline, opts.pipelineName, "expected pipeline name to match")
}
Expand Down Expand Up @@ -376,7 +376,7 @@ func TestPipelineShow_Execute(t *testing.T) {
if tc.expectedErr != nil {
require.EqualError(t, err, tc.expectedErr.Error())
} else {
require.Nil(t, err)
require.NoError(t, err)
require.Equal(t, tc.expectedContent, b.String(), "expected output content match")
}
})
Expand Down
6 changes: 3 additions & 3 deletions internal/pkg/cli/pipeline_status_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func TestPipelineStatus_Validate(t *testing.T) {
if tc.expectedErr != nil {
require.EqualError(t, err, tc.expectedErr.Error())
} else {
require.Nil(t, err)
require.NoError(t, err)
}
})
}
Expand Down Expand Up @@ -284,7 +284,7 @@ stages:
if tc.expectedErr != nil {
require.EqualError(t, err, tc.expectedErr.Error())
} else {
require.Nil(t, err)
require.NoError(t, err)
require.Equal(t, tc.expectedApp, opts.AppName(), "expected application names to match")
require.Equal(t, tc.expectedPipeline, opts.pipelineName, "expected pipeline name to match")
}
Expand Down Expand Up @@ -361,7 +361,7 @@ func TestPipelineStatus_Execute(t *testing.T) {
if tc.expectedError != nil {
require.EqualError(t, err, tc.expectedError.Error())
} else {
require.Nil(t, err)
require.NoError(t, err)
require.Equal(t, tc.expectedContent, b.String(), "expected output content to match")
}
})
Expand Down
6 changes: 3 additions & 3 deletions internal/pkg/cli/pipeline_update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ func TestUpdatePipelineOpts_convertStages(t *testing.T) {
if tc.expectedError != nil {
require.Equal(t, tc.expectedError, err)
} else {
require.Nil(t, err)
require.NoError(t, err)
require.ElementsMatch(t, tc.expectedStages, actualStages)
}
})
Expand Down Expand Up @@ -191,7 +191,7 @@ func TestUpdatePipelineOpts_getArtifactBuckets(t *testing.T) {
if tc.expectedError != nil {
require.Equal(t, tc.expectedError, err)
} else {
require.Nil(t, err)
require.NoError(t, err)
require.ElementsMatch(t, tc.expectedOut, actual)
}
})
Expand Down Expand Up @@ -586,7 +586,7 @@ stages:
if tc.expectedError != nil {
require.Equal(t, tc.expectedError.Error(), err.Error())
} else {
require.Nil(t, err)
require.NoError(t, err)
}
})
}
Expand Down
6 changes: 3 additions & 3 deletions internal/pkg/cli/storage_init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ func TestStorageInitOpts_Validate(t *testing.T) {
if tc.wantedErr != nil {
require.EqualError(t, err, tc.wantedErr.Error())
} else {
require.Nil(t, err)
require.NoError(t, err)
}
})
}
Expand Down Expand Up @@ -705,7 +705,7 @@ func TestStorageInitOpts_Ask(t *testing.T) {
if tc.wantedErr != nil {
require.EqualError(t, err, tc.wantedErr.Error())
} else {
require.Nil(t, err)
require.NoError(t, err)
}
if tc.wantedVars != nil {
tc.wantedVars.prompt = opts.prompt
Expand Down Expand Up @@ -838,7 +838,7 @@ func TestStorageInitOpts_Execute(t *testing.T) {
if tc.wantedErr != nil {
require.EqualError(t, err, tc.wantedErr.Error())
} else {
require.Nil(t, err)
require.NoError(t, err)
}
})
}
Expand Down
4 changes: 2 additions & 2 deletions internal/pkg/cli/svc_delete_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ func TestDeleteSvcOpts_Validate(t *testing.T) {
if test.want != nil {
require.EqualError(t, err, test.want.Error())
} else {
require.Nil(t, err)
require.NoError(t, err)
}
})
}
Expand Down Expand Up @@ -446,7 +446,7 @@ func TestDeleteSvcOpts_Execute(t *testing.T) {
if test.wantedError != nil {
require.EqualError(t, err, test.wantedError.Error())
} else {
require.Nil(t, err)
require.NoError(t, err)
}
})
}
Expand Down
4 changes: 2 additions & 2 deletions internal/pkg/cli/svc_deploy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ func TestSvcDeployOpts_Validate(t *testing.T) {
if tc.wantedError != nil {
require.EqualError(t, err, tc.wantedError.Error())
} else {
require.Nil(t, err)
require.NoError(t, err)
}
})
}
Expand Down Expand Up @@ -180,7 +180,7 @@ func TestSvcDeployOpts_Ask(t *testing.T) {

// THEN
if tc.wantedError == nil {
require.Nil(t, err)
require.NoError(t, err)
require.Equal(t, tc.wantedSvcName, opts.Name)
require.Equal(t, tc.wantedEnvName, opts.EnvName)
require.Equal(t, tc.wantedImageTag, opts.ImageTag)
Expand Down
Loading

0 comments on commit 5745352

Please sign in to comment.