From 54513452a1134de670467e5a8d5cebd768b3c7e4 Mon Sep 17 00:00:00 2001 From: Stephen Solka Date: Sun, 19 Jul 2020 05:53:40 -0400 Subject: [PATCH] prefer NoError/Error over Nil/NotNil (#12271) --- integrations/repofiles_delete_test.go | 4 ++-- integrations/repofiles_update_test.go | 10 +++++----- modules/repofiles/blob_test.go | 2 +- modules/repofiles/content_test.go | 12 ++++++------ modules/repofiles/diff_test.go | 4 ++-- modules/repofiles/file_test.go | 2 +- modules/repofiles/tree_test.go | 2 +- modules/webhook/dingtalk_test.go | 4 ++-- modules/webhook/matrix_test.go | 14 +++++++------- modules/webhook/slack_test.go | 12 ++++++------ modules/webhook/telegram_test.go | 2 +- 11 files changed, 34 insertions(+), 34 deletions(-) diff --git a/integrations/repofiles_delete_test.go b/integrations/repofiles_delete_test.go index 754f64023f..1fa316c41e 100644 --- a/integrations/repofiles_delete_test.go +++ b/integrations/repofiles_delete_test.go @@ -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) @@ -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) diff --git a/integrations/repofiles_update_test.go b/integrations/repofiles_update_test.go index c422483bf8..9e99b36ae4 100644 --- a/integrations/repofiles_update_test.go +++ b/integrations/repofiles_update_test.go @@ -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() @@ -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() @@ -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() @@ -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 @@ -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() diff --git a/modules/repofiles/blob_test.go b/modules/repofiles/blob_test.go index ddc23aeac3..8ba80347bf 100644 --- a/modules/repofiles/blob_test.go +++ b/modules/repofiles/blob_test.go @@ -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) } diff --git a/modules/repofiles/content_test.go b/modules/repofiles/content_test.go index d024cfd549..2782161122 100644 --- a/modules/repofiles/content_test.go +++ b/modules/repofiles/content_test.go @@ -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) }) } @@ -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) }) } @@ -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) }) } diff --git a/modules/repofiles/diff_test.go b/modules/repofiles/diff_test.go index d9e144be82..d40f8a50fe 100644 --- a/modules/repofiles/diff_test.go +++ b/modules/repofiles/diff_test.go @@ -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) }) } diff --git a/modules/repofiles/file_test.go b/modules/repofiles/file_test.go index 3cb4eb472b..aafe816a16 100644 --- a/modules/repofiles/file_test.go +++ b/modules/repofiles/file_test.go @@ -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) } diff --git a/modules/repofiles/tree_test.go b/modules/repofiles/tree_test.go index e1bb812ec1..274e89e969 100644 --- a/modules/repofiles/tree_test.go +++ b/modules/repofiles/tree_test.go @@ -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", diff --git a/modules/webhook/dingtalk_test.go b/modules/webhook/dingtalk_test.go index f50cb9a587..4cb7a913e6 100644 --- a/modules/webhook/dingtalk_test.go +++ b/modules/webhook/dingtalk_test.go @@ -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) diff --git a/modules/webhook/matrix_test.go b/modules/webhook/matrix_test.go index cfe234f850..4e8b878ad4 100644 --- a/modules/webhook/matrix_test.go +++ b/modules/webhook/matrix_test.go @@ -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, "[test/repo] Issue opened: #2 crash by user1", 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, "[test/repo] Issue closed: #2 crash by user1", pl.FormattedBody) @@ -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) @@ -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) @@ -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) @@ -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) @@ -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")) diff --git a/modules/webhook/slack_test.go b/modules/webhook/slack_test.go index a418c8e2e2..15503434f8 100644 --- a/modules/webhook/slack_test.go +++ b/modules/webhook/slack_test.go @@ -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, "[] Issue opened: by ", 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, "[] Issue closed: by ", pl.Text) } @@ -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, "[] New comment on issue by ", pl.Text) @@ -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, "[] New comment on pull request by ", pl.Text) @@ -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, "[] Release created: by ", pl.Text) @@ -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, "[] Pull request opened: by ", pl.Text) diff --git a/modules/webhook/telegram_test.go b/modules/webhook/telegram_test.go index 221dc9843c..1686188b9d 100644 --- a/modules/webhook/telegram_test.go +++ b/modules/webhook/telegram_test.go @@ -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, "[test/repo] Issue closed: #2 crash by user1\n\n", pl.Message)