Replace assert.Fail with assert.FailNow (#27578)

lng2020 · web-flow · commit dc0404471608 · 2023-10-11T11:02:24.000Z
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.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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)
 			}))

Original file line number	Diff line number	Diff line change
`@@ -51,7 +51,7 @@ func Test_SSHParsePublicKey(t *testing.T) {`
`51`	`51`	`if err != nil {`
`52`	`52`	`// Some servers do not support ecdsa format.`
`53`	`53`	`if !strings.Contains(err.Error(), "line 1 too long:") {`
`54`		`- assert.Fail(t, "%v", err)`
	`54`	`+ assert.FailNow(t, "%v", err)`
`55`	`55`	`}`
`56`	`56`	`}`
`57`	`57`	`assert.Equal(t, tc.keyType, keyTypeK)`
`@@ -60,7 +60,7 @@ func Test_SSHParsePublicKey(t *testing.T) {`
`60`	`60`	`t.Run("SSHParseKeyNative", func(t *testing.T) {`
`61`	`61`	`keyTypeK, lengthK, err := SSHNativeParsePublicKey(tc.content)`
`62`	`62`	`if err != nil {`
`63`		`- assert.Fail(t, "%v", err)`
	`63`	`+ assert.FailNow(t, "%v", err)`
`64`	`64`	`}`
`65`	`65`	`assert.Equal(t, tc.keyType, keyTypeK)`
`66`	`66`	`assert.EqualValues(t, tc.length, lengthK)`
Original file line number	Diff line number	Diff line change
`@@ -47,8 +47,7 @@ func checkForConsistency(t assert.TestingT, bean any) {`
`47`	`47`	`assert.NoError(t, err)`
`48`	`48`	`f := consistencyCheckMap[tb.Name]`
`49`	`49`	`if f == nil {`
`50`		`- assert.Fail(t, "unknown bean type: %#v", bean)`
`51`		`- return`
	`50`	`+ assert.FailNow(t, "unknown bean type: %#v", bean)`
`52`	`51`	`}`
`53`	`52`	`f(t, bean)`
`54`	`53`	`}`
Original file line number	Diff line number	Diff line change
`@@ -96,11 +96,10 @@ func TestBleveIndexAndSearch(t *testing.T) {`
`96`	`96`	`idx := bleve.NewIndexer(dir)`
`97`	`97`	`_, err := idx.Init(context.Background())`
`98`	`98`	`if err != nil {`
`99`		`- assert.Fail(t, "Unable to create bleve indexer Error: %v", err)`
`100`	`99`	`if idx != nil {`
`101`	`100`	`idx.Close()`
`102`	`101`	`}`
`103`		`- return`
	`102`	`+ assert.FailNow(t, "Unable to create bleve indexer Error: %v", err)`
`104`	`103`	`}`
`105`	`104`	`defer idx.Close()`
`106`	`105`
`@@ -118,11 +117,10 @@ func TestESIndexAndSearch(t *testing.T) {`
`118`	`117`
`119`	`118`	`indexer := elasticsearch.NewIndexer(u, "gitea_codes")`
`120`	`119`	`if _, err := indexer.Init(context.Background()); err != nil {`
`121`		`- assert.Fail(t, "Unable to init ES indexer Error: %v", err)`
`122`	`120`	`if indexer != nil {`
`123`	`121`	`indexer.Close()`
`124`	`122`	`}`
`125`		`- return`
	`123`	`+ assert.FailNow(t, "Unable to init ES indexer Error: %v", err)`
`126`	`124`	`}`
`127`	`125`
`128`	`126`	`defer indexer.Close()`
Original file line number	Diff line number	Diff line change
`@@ -50,7 +50,7 @@ func TestManager_Cancel(t *testing.T) {`
`50`	`50`	`select {`
`51`	`51`	`case <-ctx.Done():`
`52`	`52`	`default:`
`53`		`- assert.Fail(t, "Cancel should cancel the provided context")`
	`53`	`+ assert.FailNow(t, "Cancel should cancel the provided context")`
`54`	`54`	`}`
`55`	`55`	`finished()`
`56`	`56`
`@@ -62,7 +62,7 @@ func TestManager_Cancel(t *testing.T) {`
`62`	`62`	`select {`
`63`	`63`	`case <-ctx.Done():`
`64`	`64`	`default:`
`65`		`- assert.Fail(t, "Cancel should cancel the provided context")`
	`65`	`+ assert.FailNow(t, "Cancel should cancel the provided context")`
`66`	`66`	`}`
`67`	`67`	`finished()`
`68`	`68`	`}`
Original file line number	Diff line number	Diff line change
`@@ -54,7 +54,7 @@ func TestPullRequest_AddToTaskQueue(t *testing.T) {`
`54`	`54`	`case id := <-idChan:`
`55`	`55`	`assert.EqualValues(t, pr.ID, id)`
`56`	`56`	`case <-time.After(time.Second):`
`57`		`- assert.Fail(t, "Timeout: nothing was added to pullRequestQueue")`
	`57`	`+ assert.FailNow(t, "Timeout: nothing was added to pullRequestQueue")`
`58`	`58`	`}`
`59`	`59`
`60`	`60`	`has, err = prPatchCheckerQueue.Has(strconv.FormatInt(pr.ID, 10))`
Original file line number	Diff line number	Diff line change
`@@ -296,7 +296,7 @@ func TestPackageConan(t *testing.T) {`
`296`	`296`
`297`	`297`	`assert.Equal(t, int64(len(contentConaninfo)), pb.Size)`
`298`	`298`	`} else {`
`299`		`- assert.Fail(t, "unknown file: %s", pf.Name)`
	`299`	`+ assert.FailNow(t, "unknown file: %s", pf.Name)`
`300`	`300`	`}`
`301`	`301`	`}`
`302`	`302`	`})`
Original file line number	Diff line number	Diff line change
`@@ -349,7 +349,7 @@ func TestPackageContainer(t *testing.T) {`
`349`	`349`	`assert.Equal(t, "application/vnd.docker.image.rootfs.diff.tar.gzip", pfd.Properties.GetByName(container_module.PropertyMediaType))`
`350`	`350`	`assert.Equal(t, blobDigest, pfd.Properties.GetByName(container_module.PropertyDigest))`
`351`	`351`	`default:`
`352`		`- assert.Fail(t, "unknown file: %s", pfd.File.Name)`
	`352`	`+ assert.FailNow(t, "unknown file: %s", pfd.File.Name)`
`353`	`353`	`}`
`354`	`354`	`}`
`355`	`355`