Skip to content

prefer NoError/Error over Nil/NotNil #12271

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Jul 19, 2020
Merged

prefer NoError/Error over Nil/NotNil #12271

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 2 additions & 2 deletions integrations/repofiles_delete_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func testDeleteRepoFile(t *testing.T, u *url.URL) {

t.Run("Delete README.md file", func(t *testing.T) {
fileResponse, err := repofiles.DeleteRepoFile(repo, doer, opts)
assert.Nil(t, err)
assert.NoError(t, err)
expectedFileResponse := getExpectedDeleteFileResponse(u)
assert.NotNil(t, fileResponse)
assert.Nil(t, fileResponse.Content)
Expand Down Expand Up @@ -122,7 +122,7 @@ func testDeleteRepoFileWithoutBranchNames(t *testing.T, u *url.URL) {

t.Run("Delete README.md without Branch Name", func(t *testing.T) {
fileResponse, err := repofiles.DeleteRepoFile(repo, doer, opts)
assert.Nil(t, err)
assert.NoError(t, err)
expectedFileResponse := getExpectedDeleteFileResponse(u)
assert.NotNil(t, fileResponse)
assert.Nil(t, fileResponse.Content)
Expand Down
10 changes: 5 additions & 5 deletions integrations/repofiles_update_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ func TestCreateOrUpdateRepoFileForCreate(t *testing.T) {
fileResponse, err := repofiles.CreateOrUpdateRepoFile(repo, doer, opts)

// asserts
assert.Nil(t, err)
assert.NoError(t, err)
gitRepo, _ := git.OpenRepository(repo.RepoPath())
defer gitRepo.Close()

Expand Down Expand Up @@ -237,7 +237,7 @@ func TestCreateOrUpdateRepoFileForUpdate(t *testing.T) {
fileResponse, err := repofiles.CreateOrUpdateRepoFile(repo, doer, opts)

// asserts
assert.Nil(t, err)
assert.NoError(t, err)
gitRepo, _ := git.OpenRepository(repo.RepoPath())
defer gitRepo.Close()

Expand Down Expand Up @@ -272,7 +272,7 @@ func TestCreateOrUpdateRepoFileForUpdateWithFileMove(t *testing.T) {
fileResponse, err := repofiles.CreateOrUpdateRepoFile(repo, doer, opts)

// asserts
assert.Nil(t, err)
assert.NoError(t, err)
gitRepo, _ := git.OpenRepository(repo.RepoPath())
defer gitRepo.Close()

Expand All @@ -287,7 +287,7 @@ func TestCreateOrUpdateRepoFileForUpdateWithFileMove(t *testing.T) {
t.Fatalf("expected git.ErrNotExist, got:%v", err)
}
toEntry, err := commit.GetTreeEntryByPath(opts.TreePath)
assert.Nil(t, err)
assert.NoError(t, err)
assert.Nil(t, fromEntry) // Should no longer exist here
assert.NotNil(t, toEntry) // Should exist here
// assert SHA has remained the same but paths use the new file name
Expand Down Expand Up @@ -322,7 +322,7 @@ func TestCreateOrUpdateRepoFileWithoutBranchNames(t *testing.T) {
fileResponse, err := repofiles.CreateOrUpdateRepoFile(repo, doer, opts)

// asserts
assert.Nil(t, err)
assert.NoError(t, err)
gitRepo, _ := git.OpenRepository(repo.RepoPath())
defer gitRepo.Close()

Expand Down
2 changes: 1 addition & 1 deletion modules/repofiles/blob_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,6 @@ func TestGetBlobBySHA(t *testing.T) {
SHA: "65f1bf27bc3bf70f64657658635e66094edbcb4d",
Size: 180,
}
assert.Nil(t, err)
assert.NoError(t, err)
assert.Equal(t, expectedGBR, gbr)
}
12 changes: 6 additions & 6 deletions modules/repofiles/content_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,13 @@ func TestGetContents(t *testing.T) {
t.Run("Get README.md contents with GetContents()", func(t *testing.T) {
fileContentResponse, err := GetContents(ctx.Repo.Repository, treePath, ref, false)
assert.EqualValues(t, expectedContentsResponse, fileContentResponse)
assert.Nil(t, err)
assert.NoError(t, err)
})

t.Run("Get REAMDE.md contents with ref as empty string (should then use the repo's default branch) with GetContents()", func(t *testing.T) {
fileContentResponse, err := GetContents(ctx.Repo.Repository, treePath, "", false)
assert.EqualValues(t, expectedContentsResponse, fileContentResponse)
assert.Nil(t, err)
assert.NoError(t, err)
})
}

Expand Down Expand Up @@ -101,13 +101,13 @@ func TestGetContentsOrListForDir(t *testing.T) {
t.Run("Get root dir contents with GetContentsOrList()", func(t *testing.T) {
fileContentResponse, err := GetContentsOrList(ctx.Repo.Repository, treePath, ref)
assert.EqualValues(t, expectedContentsListResponse, fileContentResponse)
assert.Nil(t, err)
assert.NoError(t, err)
})

t.Run("Get root dir contents with ref as empty string (should then use the repo's default branch) with GetContentsOrList()", func(t *testing.T) {
fileContentResponse, err := GetContentsOrList(ctx.Repo.Repository, treePath, "")
assert.EqualValues(t, expectedContentsListResponse, fileContentResponse)
assert.Nil(t, err)
assert.NoError(t, err)
})
}

Expand All @@ -129,13 +129,13 @@ func TestGetContentsOrListForFile(t *testing.T) {
t.Run("Get README.md contents with GetContentsOrList()", func(t *testing.T) {
fileContentResponse, err := GetContentsOrList(ctx.Repo.Repository, treePath, ref)
assert.EqualValues(t, expectedContentsResponse, fileContentResponse)
assert.Nil(t, err)
assert.NoError(t, err)
})

t.Run("Get REAMDE.md contents with ref as empty string (should then use the repo's default branch) with GetContentsOrList()", func(t *testing.T) {
fileContentResponse, err := GetContentsOrList(ctx.Repo.Repository, treePath, "")
assert.EqualValues(t, expectedContentsResponse, fileContentResponse)
assert.Nil(t, err)
assert.NoError(t, err)
})
}

Expand Down
4 changes: 2 additions & 2 deletions modules/repofiles/diff_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,13 +113,13 @@ func TestGetDiffPreview(t *testing.T) {

t.Run("with given branch", func(t *testing.T) {
diff, err := GetDiffPreview(ctx.Repo.Repository, branch, treePath, content)
assert.Nil(t, err)
assert.NoError(t, err)
assert.EqualValues(t, expectedDiff, diff)
})

t.Run("empty branch, same results", func(t *testing.T) {
diff, err := GetDiffPreview(ctx.Repo.Repository, "", treePath, content)
assert.Nil(t, err)
assert.NoError(t, err)
assert.EqualValues(t, expectedDiff, diff)
})
}
Expand Down
2 changes: 1 addition & 1 deletion modules/repofiles/file_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,6 @@ func TestGetFileResponseFromCommit(t *testing.T) {
expectedFileResponse := getExpectedFileResponse()

fileResponse, err := GetFileResponseFromCommit(repo, commit, branch, treePath)
assert.Nil(t, err)
assert.NoError(t, err)
assert.EqualValues(t, expectedFileResponse, fileResponse)
}
2 changes: 1 addition & 1 deletion modules/repofiles/tree_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func TestGetTreeBySHA(t *testing.T) {
ctx.SetParams(":sha", sha)

tree, err := GetTreeBySHA(ctx.Repo.Repository, ctx.Params(":sha"), page, perPage, true)
assert.Nil(t, err)
assert.NoError(t, err)
expectedTree := &api.GitTreeResponse{
SHA: "65f1bf27bc3bf70f64657658635e66094edbcb4d",
URL: "https://try.gitea.io/api/v1/repos/user2/repo1/git/trees/65f1bf27bc3bf70f64657658635e66094edbcb4d",
Expand Down
4 changes: 2 additions & 2 deletions modules/webhook/dingtalk_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,14 @@ func TestGetDingTalkIssuesPayload(t *testing.T) {

p.Action = api.HookIssueOpened
pl, err := getDingtalkIssuesPayload(p)
require.Nil(t, err)
require.NoError(t, err)
require.NotNil(t, pl)
assert.Equal(t, "#2 crash", pl.ActionCard.Title)
assert.Equal(t, "[test/repo] Issue opened: #2 crash by user1\r\n\r\n", pl.ActionCard.Text)

p.Action = api.HookIssueClosed
pl, err = getDingtalkIssuesPayload(p)
require.Nil(t, err)
require.NoError(t, err)
require.NotNil(t, pl)
assert.Equal(t, "#2 crash", pl.ActionCard.Title)
assert.Equal(t, "[test/repo] Issue closed: #2 crash by user1\r\n\r\n", pl.ActionCard.Text)
Expand Down
14 changes: 7 additions & 7 deletions modules/webhook/matrix_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,14 @@ func TestMatrixIssuesPayloadOpened(t *testing.T) {

p.Action = api.HookIssueOpened
pl, err := getMatrixIssuesPayload(p, sl)
require.Nil(t, err)
require.NoError(t, err)
require.NotNil(t, pl)
assert.Equal(t, "[[test/repo](http://localhost:3000/test/repo)] Issue opened: [#2 crash](http://localhost:3000/test/repo/issues/2) by [user1](https://try.gitea.io/user1)", pl.Body)
assert.Equal(t, "[<a href=\"http://localhost:3000/test/repo\">test/repo</a>] Issue opened: <a href=\"http://localhost:3000/test/repo/issues/2\">#2 crash</a> by <a href=\"https://try.gitea.io/user1\">user1</a>", pl.FormattedBody)

p.Action = api.HookIssueClosed
pl, err = getMatrixIssuesPayload(p, sl)
require.Nil(t, err)
require.NoError(t, err)
require.NotNil(t, pl)
assert.Equal(t, "[[test/repo](http://localhost:3000/test/repo)] Issue closed: [#2 crash](http://localhost:3000/test/repo/issues/2) by [user1](https://try.gitea.io/user1)", pl.Body)
assert.Equal(t, "[<a href=\"http://localhost:3000/test/repo\">test/repo</a>] Issue closed: <a href=\"http://localhost:3000/test/repo/issues/2\">#2 crash</a> by <a href=\"https://try.gitea.io/user1\">user1</a>", pl.FormattedBody)
Expand All @@ -39,7 +39,7 @@ func TestMatrixIssueCommentPayload(t *testing.T) {
sl := &MatrixMeta{}

pl, err := getMatrixIssueCommentPayload(p, sl)
require.Nil(t, err)
require.NoError(t, err)
require.NotNil(t, pl)

assert.Equal(t, "[[test/repo](http://localhost:3000/test/repo)] New comment on issue [#2 crash](http://localhost:3000/test/repo/issues/2) by [user1](https://try.gitea.io/user1)", pl.Body)
Expand All @@ -52,7 +52,7 @@ func TestMatrixPullRequestCommentPayload(t *testing.T) {
sl := &MatrixMeta{}

pl, err := getMatrixIssueCommentPayload(p, sl)
require.Nil(t, err)
require.NoError(t, err)
require.NotNil(t, pl)

assert.Equal(t, "[[test/repo](http://localhost:3000/test/repo)] New comment on pull request [#2 Fix bug](http://localhost:3000/test/repo/pulls/2) by [user1](https://try.gitea.io/user1)", pl.Body)
Expand All @@ -65,7 +65,7 @@ func TestMatrixReleasePayload(t *testing.T) {
sl := &MatrixMeta{}

pl, err := getMatrixReleasePayload(p, sl)
require.Nil(t, err)
require.NoError(t, err)
require.NotNil(t, pl)

assert.Equal(t, "[[test/repo](http://localhost:3000/test/repo)] Release created: [v1.0](http://localhost:3000/test/repo/src/v1.0) by [user1](https://try.gitea.io/user1)", pl.Body)
Expand All @@ -78,7 +78,7 @@ func TestMatrixPullRequestPayload(t *testing.T) {
sl := &MatrixMeta{}

pl, err := getMatrixPullRequestPayload(p, sl)
require.Nil(t, err)
require.NoError(t, err)
require.NotNil(t, pl)

assert.Equal(t, "[[test/repo](http://localhost:3000/test/repo)] Pull request opened: [#2 Fix bug](http://localhost:3000/test/repo/pulls/12) by [user1](https://try.gitea.io/user1)", pl.Body)
Expand Down Expand Up @@ -148,7 +148,7 @@ func TestMatrixHookRequest(t *testing.T) {
}`

req, err := getMatrixHookRequest(h)
require.Nil(t, err)
require.NoError(t, err)
require.NotNil(t, req)

assert.Equal(t, "Bearer dummy_access_token", req.Header.Get("Authorization"))
Expand Down
12 changes: 6 additions & 6 deletions modules/webhook/slack_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,13 @@ func TestSlackIssuesPayloadOpened(t *testing.T) {

p.Action = api.HookIssueOpened
pl, err := getSlackIssuesPayload(p, sl)
require.Nil(t, err)
require.NoError(t, err)
require.NotNil(t, pl)
assert.Equal(t, "[<http://localhost:3000/test/repo|test/repo>] Issue opened: <http://localhost:3000/test/repo/issues/2|#2 crash> by <https://try.gitea.io/user1|user1>", pl.Text)

p.Action = api.HookIssueClosed
pl, err = getSlackIssuesPayload(p, sl)
require.Nil(t, err)
require.NoError(t, err)
require.NotNil(t, pl)
assert.Equal(t, "[<http://localhost:3000/test/repo|test/repo>] Issue closed: <http://localhost:3000/test/repo/issues/2|#2 crash> by <https://try.gitea.io/user1|user1>", pl.Text)
}
Expand All @@ -39,7 +39,7 @@ func TestSlackIssueCommentPayload(t *testing.T) {
}

pl, err := getSlackIssueCommentPayload(p, sl)
require.Nil(t, err)
require.NoError(t, err)
require.NotNil(t, pl)

assert.Equal(t, "[<http://localhost:3000/test/repo|test/repo>] New comment on issue <http://localhost:3000/test/repo/issues/2|#2 crash> by <https://try.gitea.io/user1|user1>", pl.Text)
Expand All @@ -53,7 +53,7 @@ func TestSlackPullRequestCommentPayload(t *testing.T) {
}

pl, err := getSlackIssueCommentPayload(p, sl)
require.Nil(t, err)
require.NoError(t, err)
require.NotNil(t, pl)

assert.Equal(t, "[<http://localhost:3000/test/repo|test/repo>] New comment on pull request <http://localhost:3000/test/repo/pulls/2|#2 Fix bug> by <https://try.gitea.io/user1|user1>", pl.Text)
Expand All @@ -67,7 +67,7 @@ func TestSlackReleasePayload(t *testing.T) {
}

pl, err := getSlackReleasePayload(p, sl)
require.Nil(t, err)
require.NoError(t, err)
require.NotNil(t, pl)

assert.Equal(t, "[<http://localhost:3000/test/repo|test/repo>] Release created: <http://localhost:3000/test/repo/src/v1.0|v1.0> by <https://try.gitea.io/user1|user1>", pl.Text)
Expand All @@ -81,7 +81,7 @@ func TestSlackPullRequestPayload(t *testing.T) {
}

pl, err := getSlackPullRequestPayload(p, sl)
require.Nil(t, err)
require.NoError(t, err)
require.NotNil(t, pl)

assert.Equal(t, "[<http://localhost:3000/test/repo|test/repo>] Pull request opened: <http://localhost:3000/test/repo/pulls/12|#2 Fix bug> by <https://try.gitea.io/user1|user1>", pl.Text)
Expand Down
2 changes: 1 addition & 1 deletion modules/webhook/telegram_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ func TestGetTelegramIssuesPayload(t *testing.T) {
p.Action = api.HookIssueClosed

pl, err := getTelegramIssuesPayload(p)
require.Nil(t, err)
require.NoError(t, err)
require.NotNil(t, pl)

assert.Equal(t, "[<a href=\"http://localhost:3000/test/repo\">test/repo</a>] Issue closed: <a href=\"http://localhost:3000/test/repo/issues/2\">#2 crash</a> by <a href=\"https://try.gitea.io/user1\">user1</a>\n\n", pl.Message)
Expand Down