From dc04044716088e3786497e200abe1fdfb3a943b6 Mon Sep 17 00:00:00 2001 From: Nanguan Lin <70063547+lng2020@users.noreply.github.com> Date: Wed, 11 Oct 2023 19:02:24 +0800 Subject: [PATCH] Replace assert.Fail with assert.FailNow (#27578) assert.Fail() will continue to execute the code while assert.FailNow() not. I thought those uses of assert.Fail() should exit immediately. PS: perhaps it's a good idea to use [require](https://pkg.go.dev/github.com/stretchr/testify/require) somewhere because the assert package's default behavior does not exit when an error occurs, which makes it difficult to find the root error reason. --- models/asymkey/ssh_key_test.go | 4 ++-- models/unittest/consistency.go | 3 +-- modules/contexttest/context_tests.go | 9 +++------ modules/git/repo_attribute_test.go | 8 ++++---- modules/git/repo_tag_test.go | 2 -- modules/indexer/code/indexer_test.go | 6 ++---- modules/process/manager_test.go | 4 ++-- services/pull/check_test.go | 2 +- tests/integration/api_packages_conan_test.go | 2 +- tests/integration/api_packages_container_test.go | 2 +- tests/integration/api_packages_nuget_test.go | 2 +- tests/integration/api_repo_test.go | 2 +- tests/integration/api_token_test.go | 3 +-- tests/integration/gpg_git_test.go | 9 --------- 14 files changed, 20 insertions(+), 38 deletions(-) diff --git a/models/asymkey/ssh_key_test.go b/models/asymkey/ssh_key_test.go index 5378ef66b..d3e886b97 100644 --- a/models/asymkey/ssh_key_test.go +++ b/models/asymkey/ssh_key_test.go @@ -51,7 +51,7 @@ func Test_SSHParsePublicKey(t *testing.T) { if err != nil { // Some servers do not support ecdsa format. if !strings.Contains(err.Error(), "line 1 too long:") { - assert.Fail(t, "%v", err) + assert.FailNow(t, "%v", err) } } assert.Equal(t, tc.keyType, keyTypeK) @@ -60,7 +60,7 @@ func Test_SSHParsePublicKey(t *testing.T) { t.Run("SSHParseKeyNative", func(t *testing.T) { keyTypeK, lengthK, err := SSHNativeParsePublicKey(tc.content) if err != nil { - assert.Fail(t, "%v", err) + assert.FailNow(t, "%v", err) } assert.Equal(t, tc.keyType, keyTypeK) assert.EqualValues(t, tc.length, lengthK) diff --git a/models/unittest/consistency.go b/models/unittest/consistency.go index faa02589a..71839001b 100644 --- a/models/unittest/consistency.go +++ b/models/unittest/consistency.go @@ -47,8 +47,7 @@ func checkForConsistency(t assert.TestingT, bean any) { assert.NoError(t, err) f := consistencyCheckMap[tb.Name] if f == nil { - assert.Fail(t, "unknown bean type: %#v", bean) - return + assert.FailNow(t, "unknown bean type: %#v", bean) } f(t, bean) } diff --git a/modules/contexttest/context_tests.go b/modules/contexttest/context_tests.go index ea91bc500..8994c1e45 100644 --- a/modules/contexttest/context_tests.go +++ b/modules/contexttest/context_tests.go @@ -83,8 +83,7 @@ func LoadRepo(t *testing.T, ctx gocontext.Context, repoID int64) { ctx.Repo = repo doer = ctx.Doer default: - assert.Fail(t, "context is not *context.Context or *context.APIContext") - return + assert.FailNow(t, "context is not *context.Context or *context.APIContext") } repo.Repository = unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: repoID}) @@ -105,8 +104,7 @@ func LoadRepoCommit(t *testing.T, ctx gocontext.Context) { case *context.APIContext: repo = ctx.Repo default: - assert.Fail(t, "context is not *context.Context or *context.APIContext") - return + assert.FailNow(t, "context is not *context.Context or *context.APIContext") } gitRepo, err := git.OpenRepository(ctx, repo.Repository.RepoPath()) @@ -130,8 +128,7 @@ func LoadUser(t *testing.T, ctx gocontext.Context, userID int64) { case *context.APIContext: ctx.Doer = doer default: - assert.Fail(t, "context is not *context.Context or *context.APIContext") - return + assert.FailNow(t, "context is not *context.Context or *context.APIContext") } } diff --git a/modules/git/repo_attribute_test.go b/modules/git/repo_attribute_test.go index 350b69dd2..ed16dccbe 100644 --- a/modules/git/repo_attribute_test.go +++ b/modules/git/repo_attribute_test.go @@ -27,7 +27,7 @@ func Test_nulSeparatedAttributeWriter_ReadAttribute(t *testing.T) { assert.Equal(t, "linguist-vendored", attr.Attribute) assert.Equal(t, "unspecified", attr.Value) case <-time.After(100 * time.Millisecond): - assert.Fail(t, "took too long to read an attribute from the list") + assert.FailNow(t, "took too long to read an attribute from the list") } // Write a second attribute again n, err = wr.Write([]byte(testStr)) @@ -41,7 +41,7 @@ func Test_nulSeparatedAttributeWriter_ReadAttribute(t *testing.T) { assert.Equal(t, "linguist-vendored", attr.Attribute) assert.Equal(t, "unspecified", attr.Value) case <-time.After(100 * time.Millisecond): - assert.Fail(t, "took too long to read an attribute from the list") + assert.FailNow(t, "took too long to read an attribute from the list") } // Write a partial attribute @@ -52,14 +52,14 @@ func Test_nulSeparatedAttributeWriter_ReadAttribute(t *testing.T) { select { case <-wr.ReadAttribute(): - assert.Fail(t, "There should not be an attribute ready to read") + assert.FailNow(t, "There should not be an attribute ready to read") case <-time.After(100 * time.Millisecond): } _, err = wr.Write([]byte("attribute\x00")) assert.NoError(t, err) select { case <-wr.ReadAttribute(): - assert.Fail(t, "There should not be an attribute ready to read") + assert.FailNow(t, "There should not be an attribute ready to read") case <-time.After(100 * time.Millisecond): } diff --git a/modules/git/repo_tag_test.go b/modules/git/repo_tag_test.go index fb6a7cdd1..4d94efd1a 100644 --- a/modules/git/repo_tag_test.go +++ b/modules/git/repo_tag_test.go @@ -71,7 +71,6 @@ func TestRepository_GetTag(t *testing.T) { if lTag == nil { assert.NotNil(t, lTag) assert.FailNow(t, "nil lTag: %s", lTagName) - return } assert.EqualValues(t, lTagName, lTag.Name) assert.EqualValues(t, lTagCommitID, lTag.ID.String()) @@ -105,7 +104,6 @@ func TestRepository_GetTag(t *testing.T) { if aTag == nil { assert.NotNil(t, aTag) assert.FailNow(t, "nil aTag: %s", aTagName) - return } assert.EqualValues(t, aTagName, aTag.Name) assert.EqualValues(t, aTagID, aTag.ID.String()) diff --git a/modules/indexer/code/indexer_test.go b/modules/indexer/code/indexer_test.go index 5b2a97d3d..5eb8e61e3 100644 --- a/modules/indexer/code/indexer_test.go +++ b/modules/indexer/code/indexer_test.go @@ -96,11 +96,10 @@ func TestBleveIndexAndSearch(t *testing.T) { idx := bleve.NewIndexer(dir) _, err := idx.Init(context.Background()) if err != nil { - assert.Fail(t, "Unable to create bleve indexer Error: %v", err) if idx != nil { idx.Close() } - return + assert.FailNow(t, "Unable to create bleve indexer Error: %v", err) } defer idx.Close() @@ -118,11 +117,10 @@ func TestESIndexAndSearch(t *testing.T) { indexer := elasticsearch.NewIndexer(u, "gitea_codes") if _, err := indexer.Init(context.Background()); err != nil { - assert.Fail(t, "Unable to init ES indexer Error: %v", err) if indexer != nil { indexer.Close() } - return + assert.FailNow(t, "Unable to init ES indexer Error: %v", err) } defer indexer.Close() diff --git a/modules/process/manager_test.go b/modules/process/manager_test.go index 2e2e35d24..36b2a912e 100644 --- a/modules/process/manager_test.go +++ b/modules/process/manager_test.go @@ -50,7 +50,7 @@ func TestManager_Cancel(t *testing.T) { select { case <-ctx.Done(): default: - assert.Fail(t, "Cancel should cancel the provided context") + assert.FailNow(t, "Cancel should cancel the provided context") } finished() @@ -62,7 +62,7 @@ func TestManager_Cancel(t *testing.T) { select { case <-ctx.Done(): default: - assert.Fail(t, "Cancel should cancel the provided context") + assert.FailNow(t, "Cancel should cancel the provided context") } finished() } diff --git a/services/pull/check_test.go b/services/pull/check_test.go index 4a99859f5..dcf5f7b93 100644 --- a/services/pull/check_test.go +++ b/services/pull/check_test.go @@ -54,7 +54,7 @@ func TestPullRequest_AddToTaskQueue(t *testing.T) { case id := <-idChan: assert.EqualValues(t, pr.ID, id) case <-time.After(time.Second): - assert.Fail(t, "Timeout: nothing was added to pullRequestQueue") + assert.FailNow(t, "Timeout: nothing was added to pullRequestQueue") } has, err = prPatchCheckerQueue.Has(strconv.FormatInt(pr.ID, 10)) diff --git a/tests/integration/api_packages_conan_test.go b/tests/integration/api_packages_conan_test.go index 1f7890f3c..ab128bf4a 100644 --- a/tests/integration/api_packages_conan_test.go +++ b/tests/integration/api_packages_conan_test.go @@ -296,7 +296,7 @@ func TestPackageConan(t *testing.T) { assert.Equal(t, int64(len(contentConaninfo)), pb.Size) } else { - assert.Fail(t, "unknown file: %s", pf.Name) + assert.FailNow(t, "unknown file: %s", pf.Name) } } }) diff --git a/tests/integration/api_packages_container_test.go b/tests/integration/api_packages_container_test.go index 01002a441..93b4ff462 100644 --- a/tests/integration/api_packages_container_test.go +++ b/tests/integration/api_packages_container_test.go @@ -349,7 +349,7 @@ func TestPackageContainer(t *testing.T) { assert.Equal(t, "application/vnd.docker.image.rootfs.diff.tar.gzip", pfd.Properties.GetByName(container_module.PropertyMediaType)) assert.Equal(t, blobDigest, pfd.Properties.GetByName(container_module.PropertyDigest)) default: - assert.Fail(t, "unknown file: %s", pfd.File.Name) + assert.FailNow(t, "unknown file: %s", pfd.File.Name) } } diff --git a/tests/integration/api_packages_nuget_test.go b/tests/integration/api_packages_nuget_test.go index 520d1dd0a..04c2fbce0 100644 --- a/tests/integration/api_packages_nuget_test.go +++ b/tests/integration/api_packages_nuget_test.go @@ -326,7 +326,7 @@ AAAjQmxvYgAAAGm7ENm9SGxMtAFVvPUsPJTF6PbtAAAAAFcVogEJAAAAAQAAAA==`) assert.Equal(t, nuget_module.PropertySymbolID, pps[0].Name) assert.Equal(t, symbolID, pps[0].Value) default: - assert.Fail(t, "unexpected file: %v", pf.Name) + assert.FailNow(t, "unexpected file: %v", pf.Name) } } diff --git a/tests/integration/api_repo_test.go b/tests/integration/api_repo_test.go index a6d32a89e..d1d669616 100644 --- a/tests/integration/api_repo_test.go +++ b/tests/integration/api_repo_test.go @@ -368,7 +368,7 @@ func TestAPIRepoMigrate(t *testing.T) { case "You can not import from disallowed hosts.": assert.EqualValues(t, "private-ip", testCase.repoName) default: - assert.Failf(t, "unexpected error '%v' on url '%s'", respJSON["message"], testCase.cloneURL) + assert.FailNow(t, "unexpected error '%v' on url '%s'", respJSON["message"], testCase.cloneURL) } } else { assert.EqualValues(t, testCase.expectedStatus, resp.Code) diff --git a/tests/integration/api_token_test.go b/tests/integration/api_token_test.go index a71392298..fe0e44d97 100644 --- a/tests/integration/api_token_test.go +++ b/tests/integration/api_token_test.go @@ -483,8 +483,7 @@ func runTestCase(t *testing.T, testCase *requiredScopeTestCase, user *user_model } else if minRequiredLevel == auth_model.Write { unauthorizedLevel = auth_model.Read } else { - assert.Failf(t, "Invalid test case", "Unknown access token scope level: %v", minRequiredLevel) - return + assert.FailNow(t, "Invalid test case: Unknown access token scope level: %v", minRequiredLevel) } } diff --git a/tests/integration/gpg_git_test.go b/tests/integration/gpg_git_test.go index 6d67bec09..00890cfb3 100644 --- a/tests/integration/gpg_git_test.go +++ b/tests/integration/gpg_git_test.go @@ -106,12 +106,10 @@ func TestGPGGit(t *testing.T) { assert.NotNil(t, response.Verification) if response.Verification == nil { assert.FailNow(t, "no verification provided with response! %v", response) - return } assert.True(t, response.Verification.Verified) if !response.Verification.Verified { t.FailNow() - return } assert.Equal(t, "gitea@fake.local", response.Verification.Signer.Email) })) @@ -120,12 +118,10 @@ func TestGPGGit(t *testing.T) { assert.NotNil(t, response.Verification) if response.Verification == nil { assert.FailNow(t, "no verification provided with response! %v", response) - return } assert.True(t, response.Verification.Verified) if !response.Verification.Verified { t.FailNow() - return } assert.Equal(t, "gitea@fake.local", response.Verification.Signer.Email) })) @@ -140,12 +136,10 @@ func TestGPGGit(t *testing.T) { assert.NotNil(t, response.Verification) if response.Verification == nil { assert.FailNow(t, "no verification provided with response! %v", response) - return } assert.True(t, response.Verification.Verified) if !response.Verification.Verified { t.FailNow() - return } assert.Equal(t, "gitea@fake.local", response.Verification.Signer.Email) })) @@ -160,17 +154,14 @@ func TestGPGGit(t *testing.T) { assert.NotNil(t, branch.Commit) if branch.Commit == nil { assert.FailNow(t, "no commit provided with branch! %v", branch) - return } assert.NotNil(t, branch.Commit.Verification) if branch.Commit.Verification == nil { assert.FailNow(t, "no verification provided with branch commit! %v", branch.Commit) - return } assert.True(t, branch.Commit.Verification.Verified) if !branch.Commit.Verification.Verified { t.FailNow() - return } assert.Equal(t, "gitea@fake.local", branch.Commit.Verification.Signer.Email) }))