Refactor tests (#33021)

1. fix incorrect tests, for example: BeanExists doesn't do assert and
shouldn't be used
2. remove unnecessary test functions
3. introduce DumpQueryResult to help to see the database rows during
test (at least I need it)

```
====== DumpQueryResult: SELECT * FROM action_runner_token ======
- # row[0]
  id: 1
  token: xeiWBL5kuTYxGPynHCqQdoeYmJAeG3IzGXCYTrDX
  owner_id: 0
...
```

models/activities/user_heatmap_test.go

@@ -64,11 +64,9 @@ func TestGetUserHeatmapDataByUser(t *testing.T) {
 	for _, tc := range testCases {
 		user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: tc.userID})
 
-		doer := &user_model.User{ID: tc.doerID}
-		_, err := unittest.LoadBeanIfExists(doer)
-		assert.NoError(t, err)
-		if tc.doerID == 0 {
-			doer = nil
+		var doer *user_model.User
+		if tc.doerID != 0 {
+			doer = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: tc.doerID})
 		}
 
 		// get the action for comparison

models/auth/webauthn_test.go

@@ -44,7 +44,7 @@ func TestWebAuthnCredential_UpdateSignCount(t *testing.T) {
 	cred := unittest.AssertExistsAndLoadBean(t, &auth_model.WebAuthnCredential{ID: 1})
 	cred.SignCount = 1
 	assert.NoError(t, cred.UpdateSignCount(db.DefaultContext))
-	unittest.AssertExistsIf(t, true, &auth_model.WebAuthnCredential{ID: 1, SignCount: 1})
+	unittest.AssertExistsAndLoadBean(t, &auth_model.WebAuthnCredential{ID: 1, SignCount: 1})
 }
 
 func TestWebAuthnCredential_UpdateLargeCounter(t *testing.T) {
@@ -52,7 +52,7 @@ func TestWebAuthnCredential_UpdateLargeCounter(t *testing.T) {
 	cred := unittest.AssertExistsAndLoadBean(t, &auth_model.WebAuthnCredential{ID: 1})
 	cred.SignCount = 0xffffffff
 	assert.NoError(t, cred.UpdateSignCount(db.DefaultContext))
-	unittest.AssertExistsIf(t, true, &auth_model.WebAuthnCredential{ID: 1, SignCount: 0xffffffff})
+	unittest.AssertExistsAndLoadBean(t, &auth_model.WebAuthnCredential{ID: 1, SignCount: 0xffffffff})
 }
 
 func TestCreateCredential(t *testing.T) {
@@ -63,5 +63,5 @@ func TestCreateCredential(t *testing.T) {
 	assert.Equal(t, "WebAuthn Created Credential", res.Name)
 	assert.Equal(t, []byte("Test"), res.CredentialID)
 
-	unittest.AssertExistsIf(t, true, &auth_model.WebAuthnCredential{Name: "WebAuthn Created Credential", UserID: 1})
+	unittest.AssertExistsAndLoadBean(t, &auth_model.WebAuthnCredential{Name: "WebAuthn Created Credential", UserID: 1})
 }

models/issues/issue_user_test.go

@@ -12,6 +12,7 @@ import (
 	"code.gitea.io/gitea/models/unittest"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func Test_NewIssueUsers(t *testing.T) {
@@ -27,9 +28,8 @@ func Test_NewIssueUsers(t *testing.T) {
 	}
 
 	// artificially insert new issue
-	unittest.AssertSuccessfulInsert(t, newIssue)
-
-	assert.NoError(t, issues_model.NewIssueUsers(db.DefaultContext, repo, newIssue))
+	require.NoError(t, db.Insert(db.DefaultContext, newIssue))
+	require.NoError(t, issues_model.NewIssueUsers(db.DefaultContext, repo, newIssue))
 
 	// issue_user table should now have entries for new issue
 	unittest.AssertExistsAndLoadBean(t, &issues_model.IssueUser{IssueID: newIssue.ID, UID: newIssue.PosterID})

models/issues/label_test.go

@@ -387,7 +387,7 @@ func TestDeleteIssueLabel(t *testing.T) {
 
 		expectedNumIssues := label.NumIssues
 		expectedNumClosedIssues := label.NumClosedIssues
-		if unittest.BeanExists(t, &issues_model.IssueLabel{IssueID: issueID, LabelID: labelID}) {
+		if unittest.GetBean(t, &issues_model.IssueLabel{IssueID: issueID, LabelID: labelID}) != nil {
 			expectedNumIssues--
 			if issue.IsClosed {
 				expectedNumClosedIssues--

models/organization/org_user_test.go

@@ -131,7 +131,7 @@ func TestAddOrgUser(t *testing.T) {
 	testSuccess := func(orgID, userID int64, isPublic bool) {
 		org := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: orgID})
 		expectedNumMembers := org.NumMembers
-		if !unittest.BeanExists(t, &organization.OrgUser{OrgID: orgID, UID: userID}) {
+		if unittest.GetBean(t, &organization.OrgUser{OrgID: orgID, UID: userID}) == nil {
 			expectedNumMembers++
 		}
 		assert.NoError(t, organization.AddOrgUser(db.DefaultContext, orgID, userID))

models/unittest/fixtures.go

@@ -1,12 +1,10 @@
 // Copyright 2021 The Gitea Authors. All rights reserved.
 // SPDX-License-Identifier: MIT
 
-//nolint:forbidigo
 package unittest
 
 import (
 	"fmt"
-	"os"
 	"time"
 
 	"code.gitea.io/gitea/models/db"
@@ -37,7 +35,7 @@ func InitFixtures(opts FixturesOptions, engine ...*xorm.Engine) (err error) {
 	} else {
 		fixtureOptionFiles = testfixtures.Files(opts.Files...)
 	}
-	dialect := "unknown"
+	var dialect string
 	switch e.Dialect().URI().DBType {
 	case schemas.POSTGRES:
 		dialect = "postgres"
@@ -48,8 +46,7 @@ func InitFixtures(opts FixturesOptions, engine ...*xorm.Engine) (err error) {
 	case schemas.SQLITE:
 		dialect = "sqlite3"
 	default:
-		fmt.Println("Unsupported RDBMS for integration tests")
-		os.Exit(1)
+		return fmt.Errorf("unsupported RDBMS for integration tests: %q", e.Dialect().URI().DBType)
 	}
 	loaderOptions := []func(loader *testfixtures.Loader) error{
 		testfixtures.Database(e.DB().DB),
@@ -69,9 +66,7 @@ func InitFixtures(opts FixturesOptions, engine ...*xorm.Engine) (err error) {
 
 	// register the dummy hash algorithm function used in the test fixtures
 	_ = hash.Register("dummy", hash.NewDummyHasher)
-
 	setting.PasswordHashAlgo, _ = hash.SetDefaultPasswordHashAlgorithm("dummy")
-
 	return err
 }
 
@@ -87,7 +82,7 @@ func LoadFixtures(engine ...*xorm.Engine) error {
 		time.Sleep(200 * time.Millisecond)
 	}
 	if err != nil {
-		fmt.Printf("LoadFixtures failed after retries: %v\n", err)
+		return fmt.Errorf("LoadFixtures failed after retries: %w", err)
 	}
 	// Now if we're running postgres we need to tell it to update the sequences
 	if e.Dialect().URI().DBType == schemas.POSTGRES {
@@ -108,21 +103,18 @@ func LoadFixtures(engine ...*xorm.Engine) error {
 	     AND T.relname = PGT.tablename
 	 ORDER BY S.relname;`)
 		if err != nil {
-			fmt.Printf("Failed to generate sequence update: %v\n", err)
-			return err
+			return fmt.Errorf("failed to generate sequence update: %w", err)
 		}
 		for _, r := range results {
 			for _, value := range r {
 				_, err = e.Exec(value)
 				if err != nil {
-					fmt.Printf("Failed to update sequence: %s Error: %v\n", value, err)
-					return err
+					return fmt.Errorf("failed to update sequence: %s, error: %w", value, err)
 				}
 			}
 		}
 	}
 	_ = hash.Register("dummy", hash.NewDummyHasher)
 	setting.PasswordHashAlgo, _ = hash.SetDefaultPasswordHashAlgorithm("dummy")
-
-	return err
+	return nil
 }

models/unittest/reflection.go

@@ -4,7 +4,7 @@
 package unittest
 
 import (
-	"log"
+	"fmt"
 	"reflect"
 )
 
@@ -14,7 +14,7 @@ func fieldByName(v reflect.Value, field string) reflect.Value {
 	}
 	f := v.FieldByName(field)
 	if !f.IsValid() {
-		log.Panicf("can not read %s for %v", field, v)
+		panic(fmt.Errorf("can not read %s for %v", field, v))
 	}
 	return f
 }

models/unittest/testdb.go

@@ -34,11 +34,6 @@ var (
 	fixturesDir string
 )
 
-// FixturesDir returns the fixture directory
-func FixturesDir() string {
-	return fixturesDir
-}
-
 func fatalTestError(fmtStr string, args ...any) {
 	_, _ = fmt.Fprintf(os.Stderr, fmtStr, args...)
 	os.Exit(1)

models/unittest/unit_tests.go

@@ -4,13 +4,17 @@
 package unittest
 
 import (
+	"fmt"
 	"math"
+	"os"
+	"strings"
 
 	"code.gitea.io/gitea/models/db"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"xorm.io/builder"
+	"xorm.io/xorm"
 )
 
 // Code in this file is mainly used by unittest.CheckConsistencyFor, which is not in the unit test for various reasons.
@@ -51,22 +55,23 @@ func whereOrderConditions(e db.Engine, conditions []any) db.Engine {
 	return e.OrderBy(orderBy)
 }
 
-// LoadBeanIfExists loads beans from fixture database if exist
-func LoadBeanIfExists(bean any, conditions ...any) (bool, error) {
+func getBeanIfExists(bean any, conditions ...any) (bool, error) {
 	e := db.GetEngine(db.DefaultContext)
 	return whereOrderConditions(e, conditions).Get(bean)
 }
 
-// BeanExists for testing, check if a bean exists
-func BeanExists(t assert.TestingT, bean any, conditions ...any) bool {
-	exists, err := LoadBeanIfExists(bean, conditions...)
-	assert.NoError(t, err)
-	return exists
+func GetBean[T any](t require.TestingT, bean T, conditions ...any) (ret T) {
+	exists, err := getBeanIfExists(bean, conditions...)
+	require.NoError(t, err)
+	if exists {
+		return bean
+	}
+	return ret
 }
 
 // AssertExistsAndLoadBean assert that a bean exists and load it from the test database
 func AssertExistsAndLoadBean[T any](t require.TestingT, bean T, conditions ...any) T {
-	exists, err := LoadBeanIfExists(bean, conditions...)
+	exists, err := getBeanIfExists(bean, conditions...)
 	require.NoError(t, err)
 	require.True(t, exists,
 		"Expected to find %+v (of type %T, with conditions %+v), but did not",
@@ -112,25 +117,11 @@ func GetCount(t assert.TestingT, bean any, conditions ...any) int {
 
 // AssertNotExistsBean assert that a bean does not exist in the test database
 func AssertNotExistsBean(t assert.TestingT, bean any, conditions ...any) {
-	exists, err := LoadBeanIfExists(bean, conditions...)
+	exists, err := getBeanIfExists(bean, conditions...)
 	assert.NoError(t, err)
 	assert.False(t, exists)
 }
 
-// AssertExistsIf asserts that a bean exists or does not exist, depending on
-// what is expected.
-func AssertExistsIf(t assert.TestingT, expected bool, bean any, conditions ...any) {
-	exists, err := LoadBeanIfExists(bean, conditions...)
-	assert.NoError(t, err)
-	assert.Equal(t, expected, exists)
-}
-
-// AssertSuccessfulInsert assert that beans is successfully inserted
-func AssertSuccessfulInsert(t assert.TestingT, beans ...any) {
-	err := db.Insert(db.DefaultContext, beans...)
-	assert.NoError(t, err)
-}
-
 // AssertCount assert the count of a bean
 func AssertCount(t assert.TestingT, bean, expected any) bool {
 	return assert.EqualValues(t, expected, GetCount(t, bean))
@@ -155,3 +146,39 @@ func AssertCountByCond(t assert.TestingT, tableName string, cond builder.Cond, e
 	return assert.EqualValues(t, expected, GetCountByCond(t, tableName, cond),
 		"Failed consistency test, the counted bean (of table %s) was %+v", tableName, cond)
 }
+
+// DumpQueryResult dumps the result of a query for debugging purpose
+func DumpQueryResult(t require.TestingT, sqlOrBean any, sqlArgs ...any) {
+	x := db.GetEngine(db.DefaultContext).(*xorm.Engine)
+	goDB := x.DB().DB
+	sql, ok := sqlOrBean.(string)
+	if !ok {
+		sql = fmt.Sprintf("SELECT * FROM %s", db.TableName(sqlOrBean))
+	} else if !strings.Contains(sql, " ") {
+		sql = fmt.Sprintf("SELECT * FROM %s", sql)
+	}
+	rows, err := goDB.Query(sql, sqlArgs...)
+	require.NoError(t, err)
+	defer rows.Close()
+	columns, err := rows.Columns()
+	require.NoError(t, err)
+
+	_, _ = fmt.Fprintf(os.Stdout, "====== DumpQueryResult: %s ======\n", sql)
+	idx := 0
+	for rows.Next() {
+		row := make([]any, len(columns))
+		rowPointers := make([]any, len(columns))
+		for i := range row {
+			rowPointers[i] = &row[i]
+		}
+		require.NoError(t, rows.Scan(rowPointers...))
+		_, _ = fmt.Fprintf(os.Stdout, "- # row[%d]\n", idx)
+		for i, col := range columns {
+			_, _ = fmt.Fprintf(os.Stdout, "  %s: %v\n", col, row[i])
+		}
+		idx++
+	}
+	if idx == 0 {
+		_, _ = fmt.Fprintf(os.Stdout, "(no result, columns: %s)\n", strings.Join(columns, ", "))
+	}
+}

routers/web/repo/issue_label_test.go

@@ -162,10 +162,11 @@ func TestUpdateIssueLabel_Toggle(t *testing.T) {
 		UpdateIssueLabel(ctx)
 		assert.EqualValues(t, http.StatusOK, ctx.Resp.Status())
 		for _, issueID := range testCase.IssueIDs {
-			unittest.AssertExistsIf(t, testCase.ExpectedAdd, &issues_model.IssueLabel{
-				IssueID: issueID,
-				LabelID: testCase.LabelID,
-			})
+			if testCase.ExpectedAdd {
+				unittest.AssertExistsAndLoadBean(t, &issues_model.IssueLabel{IssueID: issueID, LabelID: testCase.LabelID})
+			} else {
+				unittest.AssertNotExistsBean(t, &issues_model.IssueLabel{IssueID: issueID, LabelID: testCase.LabelID})
+			}
 		}
 		unittest.CheckConsistencyFor(t, &issues_model.Label{})
 	}

services/org/user_test.go

@@ -47,7 +47,7 @@ func TestRemoveOrgUser(t *testing.T) {
 
 	testSuccess := func(org *organization.Organization, user *user_model.User) {
 		expectedNumMembers := org.NumMembers
-		if unittest.BeanExists(t, &organization.OrgUser{OrgID: org.ID, UID: user.ID}) {
+		if unittest.GetBean(t, &organization.OrgUser{OrgID: org.ID, UID: user.ID}) != nil {
 			expectedNumMembers--
 		}
 		assert.NoError(t, RemoveOrgUser(db.DefaultContext, org, user))

tests/integration/auth_ldap_test.go

@@ -332,7 +332,7 @@ func TestLDAPUserSyncWithGroupFilter(t *testing.T) {
 
 	// Assert members of LDAP group "cn=git" are added
 	for _, gitLDAPUser := range te.gitLDAPUsers {
-		unittest.BeanExists(t, &user_model.User{
+		unittest.AssertExistsAndLoadBean(t, &user_model.User{
 			Name: gitLDAPUser.UserName,
 		})
 	}

tests/integration/pull_review_test.go

@@ -92,7 +92,7 @@ func TestPullView_CodeOwner(t *testing.T) {
 			testPullCreate(t, session, "user2", "test_codeowner", false, repo.DefaultBranch, "codeowner-basebranch", "Test Pull Request")
 
 			pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: repo.ID, HeadRepoID: repo.ID, HeadBranch: "codeowner-basebranch"})
-			unittest.AssertExistsIf(t, true, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 5})
+			unittest.AssertExistsAndLoadBean(t, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 5})
 			assert.NoError(t, pr.LoadIssue(db.DefaultContext))
 
 			err := issue_service.ChangeTitle(db.DefaultContext, pr.Issue, user2, "[WIP] Test Pull Request")
@@ -139,7 +139,7 @@ func TestPullView_CodeOwner(t *testing.T) {
 			testPullCreate(t, session, "user2", "test_codeowner", false, repo.DefaultBranch, "codeowner-basebranch2", "Test Pull Request2")
 
 			pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: repo.ID, HeadBranch: "codeowner-basebranch2"})
-			unittest.AssertExistsIf(t, true, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 8})
+			unittest.AssertExistsAndLoadBean(t, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 8})
 		})
 
 		t.Run("Forked Repo Pull Request", func(t *testing.T) {
@@ -169,13 +169,13 @@ func TestPullView_CodeOwner(t *testing.T) {
 			testPullCreateDirectly(t, session, "user5", "test_codeowner", forkedRepo.DefaultBranch, "", "", "codeowner-basebranch-forked", "Test Pull Request on Forked Repository")
 
 			pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: forkedRepo.ID, HeadBranch: "codeowner-basebranch-forked"})
-			unittest.AssertExistsIf(t, false, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 8})
+			unittest.AssertNotExistsBean(t, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 8})
 
 			// create a pull request to base repository, code reviewers should be mentioned
 			testPullCreateDirectly(t, session, repo.OwnerName, repo.Name, repo.DefaultBranch, forkedRepo.OwnerName, forkedRepo.Name, "codeowner-basebranch-forked", "Test Pull Request3")
 
 			pr = unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: repo.ID, HeadRepoID: forkedRepo.ID, HeadBranch: "codeowner-basebranch-forked"})
-			unittest.AssertExistsIf(t, true, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 8})
+			unittest.AssertExistsAndLoadBean(t, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 8})
 		})
 	})
 }

tests/integration/signin_test.go

@@ -9,6 +9,7 @@ import (
 	"strings"
 	"testing"
 
+	"code.gitea.io/gitea/models/db"
 	"code.gitea.io/gitea/models/unittest"
 	user_model "code.gitea.io/gitea/models/user"
 	"code.gitea.io/gitea/modules/setting"
@@ -17,6 +18,7 @@ import (
 	"code.gitea.io/gitea/tests"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func testLoginFailed(t *testing.T, username, password, message string) {
@@ -42,7 +44,7 @@ func TestSignin(t *testing.T) {
 	user.Name = "testuser"
 	user.LowerName = strings.ToLower(user.Name)
 	user.ID = 0
-	unittest.AssertSuccessfulInsert(t, user)
+	require.NoError(t, db.Insert(db.DefaultContext, user))
 
 	samples := []struct {
 		username string