Skip to content

Commit 0ed160f

Browse files
wxiaoguangwxiaoguang
authored
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 ... ```
1 parent e95b946 commit 0ed160f

File tree

14 files changed

+83
-68
lines changed

14 files changed

+83
-68
lines changed

‎models/activities/user_heatmap_test.go

+3-5
Original file line numberDiff line numberDiff line change
@@ -64,11 +64,9 @@ func TestGetUserHeatmapDataByUser(t *testing.T) {
6464
for _, tc := range testCases {
6565
user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: tc.userID})
6666

67-
doer := &user_model.User{ID: tc.doerID}
68-
_, err := unittest.LoadBeanIfExists(doer)
69-
assert.NoError(t, err)
70-
if tc.doerID == 0 {
71-
doer = nil
67+
var doer *user_model.User
68+
if tc.doerID != 0 {
69+
doer = unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: tc.doerID})
7270
}
7371

7472
// get the action for comparison

‎models/auth/webauthn_test.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -44,15 +44,15 @@ func TestWebAuthnCredential_UpdateSignCount(t *testing.T) {
4444
cred := unittest.AssertExistsAndLoadBean(t, &auth_model.WebAuthnCredential{ID: 1})
4545
cred.SignCount = 1
4646
assert.NoError(t, cred.UpdateSignCount(db.DefaultContext))
47-
unittest.AssertExistsIf(t, true, &auth_model.WebAuthnCredential{ID: 1, SignCount: 1})
47+
unittest.AssertExistsAndLoadBean(t, &auth_model.WebAuthnCredential{ID: 1, SignCount: 1})
4848
}
4949

5050
func TestWebAuthnCredential_UpdateLargeCounter(t *testing.T) {
5151
assert.NoError(t, unittest.PrepareTestDatabase())
5252
cred := unittest.AssertExistsAndLoadBean(t, &auth_model.WebAuthnCredential{ID: 1})
5353
cred.SignCount = 0xffffffff
5454
assert.NoError(t, cred.UpdateSignCount(db.DefaultContext))
55-
unittest.AssertExistsIf(t, true, &auth_model.WebAuthnCredential{ID: 1, SignCount: 0xffffffff})
55+
unittest.AssertExistsAndLoadBean(t, &auth_model.WebAuthnCredential{ID: 1, SignCount: 0xffffffff})
5656
}
5757

5858
func TestCreateCredential(t *testing.T) {
@@ -63,5 +63,5 @@ func TestCreateCredential(t *testing.T) {
6363
assert.Equal(t, "WebAuthn Created Credential", res.Name)
6464
assert.Equal(t, []byte("Test"), res.CredentialID)
6565

66-
unittest.AssertExistsIf(t, true, &auth_model.WebAuthnCredential{Name: "WebAuthn Created Credential", UserID: 1})
66+
unittest.AssertExistsAndLoadBean(t, &auth_model.WebAuthnCredential{Name: "WebAuthn Created Credential", UserID: 1})
6767
}

‎models/issues/issue_user_test.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"code.gitea.io/gitea/models/unittest"
1313

1414
"github.com/stretchr/testify/assert"
15+
"github.com/stretchr/testify/require"
1516
)
1617

1718
func Test_NewIssueUsers(t *testing.T) {
@@ -27,9 +28,8 @@ func Test_NewIssueUsers(t *testing.T) {
2728
}
2829

2930
// artificially insert new issue
30-
unittest.AssertSuccessfulInsert(t, newIssue)
31-
32-
assert.NoError(t, issues_model.NewIssueUsers(db.DefaultContext, repo, newIssue))
31+
require.NoError(t, db.Insert(db.DefaultContext, newIssue))
32+
require.NoError(t, issues_model.NewIssueUsers(db.DefaultContext, repo, newIssue))
3333

3434
// issue_user table should now have entries for new issue
3535
unittest.AssertExistsAndLoadBean(t, &issues_model.IssueUser{IssueID: newIssue.ID, UID: newIssue.PosterID})

‎models/issues/label_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -387,7 +387,7 @@ func TestDeleteIssueLabel(t *testing.T) {
387387

388388
expectedNumIssues := label.NumIssues
389389
expectedNumClosedIssues := label.NumClosedIssues
390-
if unittest.BeanExists(t, &issues_model.IssueLabel{IssueID: issueID, LabelID: labelID}) {
390+
if unittest.GetBean(t, &issues_model.IssueLabel{IssueID: issueID, LabelID: labelID}) != nil {
391391
expectedNumIssues--
392392
if issue.IsClosed {
393393
expectedNumClosedIssues--

‎models/organization/org_user_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ func TestAddOrgUser(t *testing.T) {
131131
testSuccess := func(orgID, userID int64, isPublic bool) {
132132
org := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: orgID})
133133
expectedNumMembers := org.NumMembers
134-
if !unittest.BeanExists(t, &organization.OrgUser{OrgID: orgID, UID: userID}) {
134+
if unittest.GetBean(t, &organization.OrgUser{OrgID: orgID, UID: userID}) == nil {
135135
expectedNumMembers++
136136
}
137137
assert.NoError(t, organization.AddOrgUser(db.DefaultContext, orgID, userID))

‎models/unittest/fixtures.go

+6-14
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,10 @@
11
// Copyright 2021 The Gitea Authors. All rights reserved.
22
// SPDX-License-Identifier: MIT
33

4-
//nolint:forbidigo
54
package unittest
65

76
import (
87
"fmt"
9-
"os"
108
"time"
119

1210
"code.gitea.io/gitea/models/db"
@@ -37,7 +35,7 @@ func InitFixtures(opts FixturesOptions, engine ...*xorm.Engine) (err error) {
3735
} else {
3836
fixtureOptionFiles = testfixtures.Files(opts.Files...)
3937
}
40-
dialect := "unknown"
38+
var dialect string
4139
switch e.Dialect().URI().DBType {
4240
case schemas.POSTGRES:
4341
dialect = "postgres"
@@ -48,8 +46,7 @@ func InitFixtures(opts FixturesOptions, engine ...*xorm.Engine) (err error) {
4846
case schemas.SQLITE:
4947
dialect = "sqlite3"
5048
default:
51-
fmt.Println("Unsupported RDBMS for integration tests")
52-
os.Exit(1)
49+
return fmt.Errorf("unsupported RDBMS for integration tests: %q", e.Dialect().URI().DBType)
5350
}
5451
loaderOptions := []func(loader *testfixtures.Loader) error{
5552
testfixtures.Database(e.DB().DB),
@@ -69,9 +66,7 @@ func InitFixtures(opts FixturesOptions, engine ...*xorm.Engine) (err error) {
6966

7067
// register the dummy hash algorithm function used in the test fixtures
7168
_ = hash.Register("dummy", hash.NewDummyHasher)
72-
7369
setting.PasswordHashAlgo, _ = hash.SetDefaultPasswordHashAlgorithm("dummy")
74-
7570
return err
7671
}
7772

@@ -87,7 +82,7 @@ func LoadFixtures(engine ...*xorm.Engine) error {
8782
time.Sleep(200 * time.Millisecond)
8883
}
8984
if err != nil {
90-
fmt.Printf("LoadFixtures failed after retries: %v\n", err)
85+
return fmt.Errorf("LoadFixtures failed after retries: %w", err)
9186
}
9287
// Now if we're running postgres we need to tell it to update the sequences
9388
if e.Dialect().URI().DBType == schemas.POSTGRES {
@@ -108,21 +103,18 @@ func LoadFixtures(engine ...*xorm.Engine) error {
108103
AND T.relname = PGT.tablename
109104
ORDER BY S.relname;`)
110105
if err != nil {
111-
fmt.Printf("Failed to generate sequence update: %v\n", err)
112-
return err
106+
return fmt.Errorf("failed to generate sequence update: %w", err)
113107
}
114108
for _, r := range results {
115109
for _, value := range r {
116110
_, err = e.Exec(value)
117111
if err != nil {
118-
fmt.Printf("Failed to update sequence: %s Error: %v\n", value, err)
119-
return err
112+
return fmt.Errorf("failed to update sequence: %s, error: %w", value, err)
120113
}
121114
}
122115
}
123116
}
124117
_ = hash.Register("dummy", hash.NewDummyHasher)
125118
setting.PasswordHashAlgo, _ = hash.SetDefaultPasswordHashAlgorithm("dummy")
126-
127-
return err
119+
return nil
128120
}

‎models/unittest/reflection.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
package unittest
55

66
import (
7-
"log"
7+
"fmt"
88
"reflect"
99
)
1010

@@ -14,7 +14,7 @@ func fieldByName(v reflect.Value, field string) reflect.Value {
1414
}
1515
f := v.FieldByName(field)
1616
if !f.IsValid() {
17-
log.Panicf("can not read %s for %v", field, v)
17+
panic(fmt.Errorf("can not read %s for %v", field, v))
1818
}
1919
return f
2020
}

‎models/unittest/testdb.go

-5
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,6 @@ var (
3434
fixturesDir string
3535
)
3636

37-
// FixturesDir returns the fixture directory
38-
func FixturesDir() string {
39-
return fixturesDir
40-
}
41-
4237
func fatalTestError(fmtStr string, args ...any) {
4338
_, _ = fmt.Fprintf(os.Stderr, fmtStr, args...)
4439
os.Exit(1)

‎models/unittest/unit_tests.go

+50-23
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,17 @@
44
package unittest
55

66
import (
7+
"fmt"
78
"math"
9+
"os"
10+
"strings"
811

912
"code.gitea.io/gitea/models/db"
1013

1114
"github.com/stretchr/testify/assert"
1215
"github.com/stretchr/testify/require"
1316
"xorm.io/builder"
17+
"xorm.io/xorm"
1418
)
1519

1620
// 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 {
5155
return e.OrderBy(orderBy)
5256
}
5357

54-
// LoadBeanIfExists loads beans from fixture database if exist
55-
func LoadBeanIfExists(bean any, conditions ...any) (bool, error) {
58+
func getBeanIfExists(bean any, conditions ...any) (bool, error) {
5659
e := db.GetEngine(db.DefaultContext)
5760
return whereOrderConditions(e, conditions).Get(bean)
5861
}
5962

60-
// BeanExists for testing, check if a bean exists
61-
func BeanExists(t assert.TestingT, bean any, conditions ...any) bool {
62-
exists, err := LoadBeanIfExists(bean, conditions...)
63-
assert.NoError(t, err)
64-
return exists
63+
func GetBean[T any](t require.TestingT, bean T, conditions ...any) (ret T) {
64+
exists, err := getBeanIfExists(bean, conditions...)
65+
require.NoError(t, err)
66+
if exists {
67+
return bean
68+
}
69+
return ret
6570
}
6671

6772
// AssertExistsAndLoadBean assert that a bean exists and load it from the test database
6873
func AssertExistsAndLoadBean[T any](t require.TestingT, bean T, conditions ...any) T {
69-
exists, err := LoadBeanIfExists(bean, conditions...)
74+
exists, err := getBeanIfExists(bean, conditions...)
7075
require.NoError(t, err)
7176
require.True(t, exists,
7277
"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 {
112117

113118
// AssertNotExistsBean assert that a bean does not exist in the test database
114119
func AssertNotExistsBean(t assert.TestingT, bean any, conditions ...any) {
115-
exists, err := LoadBeanIfExists(bean, conditions...)
120+
exists, err := getBeanIfExists(bean, conditions...)
116121
assert.NoError(t, err)
117122
assert.False(t, exists)
118123
}
119124

120-
// AssertExistsIf asserts that a bean exists or does not exist, depending on
121-
// what is expected.
122-
func AssertExistsIf(t assert.TestingT, expected bool, bean any, conditions ...any) {
123-
exists, err := LoadBeanIfExists(bean, conditions...)
124-
assert.NoError(t, err)
125-
assert.Equal(t, expected, exists)
126-
}
127-
128-
// AssertSuccessfulInsert assert that beans is successfully inserted
129-
func AssertSuccessfulInsert(t assert.TestingT, beans ...any) {
130-
err := db.Insert(db.DefaultContext, beans...)
131-
assert.NoError(t, err)
132-
}
133-
134125
// AssertCount assert the count of a bean
135126
func AssertCount(t assert.TestingT, bean, expected any) bool {
136127
return assert.EqualValues(t, expected, GetCount(t, bean))
@@ -155,3 +146,39 @@ func AssertCountByCond(t assert.TestingT, tableName string, cond builder.Cond, e
155146
return assert.EqualValues(t, expected, GetCountByCond(t, tableName, cond),
156147
"Failed consistency test, the counted bean (of table %s) was %+v", tableName, cond)
157148
}
149+
150+
// DumpQueryResult dumps the result of a query for debugging purpose
151+
func DumpQueryResult(t require.TestingT, sqlOrBean any, sqlArgs ...any) {
152+
x := db.GetEngine(db.DefaultContext).(*xorm.Engine)
153+
goDB := x.DB().DB
154+
sql, ok := sqlOrBean.(string)
155+
if !ok {
156+
sql = fmt.Sprintf("SELECT * FROM %s", db.TableName(sqlOrBean))
157+
} else if !strings.Contains(sql, " ") {
158+
sql = fmt.Sprintf("SELECT * FROM %s", sql)
159+
}
160+
rows, err := goDB.Query(sql, sqlArgs...)
161+
require.NoError(t, err)
162+
defer rows.Close()
163+
columns, err := rows.Columns()
164+
require.NoError(t, err)
165+
166+
_, _ = fmt.Fprintf(os.Stdout, "====== DumpQueryResult: %s ======\n", sql)
167+
idx := 0
168+
for rows.Next() {
169+
row := make([]any, len(columns))
170+
rowPointers := make([]any, len(columns))
171+
for i := range row {
172+
rowPointers[i] = &row[i]
173+
}
174+
require.NoError(t, rows.Scan(rowPointers...))
175+
_, _ = fmt.Fprintf(os.Stdout, "- # row[%d]\n", idx)
176+
for i, col := range columns {
177+
_, _ = fmt.Fprintf(os.Stdout, " %s: %v\n", col, row[i])
178+
}
179+
idx++
180+
}
181+
if idx == 0 {
182+
_, _ = fmt.Fprintf(os.Stdout, "(no result, columns: %s)\n", strings.Join(columns, ", "))
183+
}
184+
}

‎routers/web/repo/issue_label_test.go

+5-4
Original file line numberDiff line numberDiff line change
@@ -162,10 +162,11 @@ func TestUpdateIssueLabel_Toggle(t *testing.T) {
162162
UpdateIssueLabel(ctx)
163163
assert.EqualValues(t, http.StatusOK, ctx.Resp.Status())
164164
for _, issueID := range testCase.IssueIDs {
165-
unittest.AssertExistsIf(t, testCase.ExpectedAdd, &issues_model.IssueLabel{
166-
IssueID: issueID,
167-
LabelID: testCase.LabelID,
168-
})
165+
if testCase.ExpectedAdd {
166+
unittest.AssertExistsAndLoadBean(t, &issues_model.IssueLabel{IssueID: issueID, LabelID: testCase.LabelID})
167+
} else {
168+
unittest.AssertNotExistsBean(t, &issues_model.IssueLabel{IssueID: issueID, LabelID: testCase.LabelID})
169+
}
169170
}
170171
unittest.CheckConsistencyFor(t, &issues_model.Label{})
171172
}

‎services/org/user_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ func TestRemoveOrgUser(t *testing.T) {
4747

4848
testSuccess := func(org *organization.Organization, user *user_model.User) {
4949
expectedNumMembers := org.NumMembers
50-
if unittest.BeanExists(t, &organization.OrgUser{OrgID: org.ID, UID: user.ID}) {
50+
if unittest.GetBean(t, &organization.OrgUser{OrgID: org.ID, UID: user.ID}) != nil {
5151
expectedNumMembers--
5252
}
5353
assert.NoError(t, RemoveOrgUser(db.DefaultContext, org, user))

‎tests/integration/auth_ldap_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,7 @@ func TestLDAPUserSyncWithGroupFilter(t *testing.T) {
332332

333333
// Assert members of LDAP group "cn=git" are added
334334
for _, gitLDAPUser := range te.gitLDAPUsers {
335-
unittest.BeanExists(t, &user_model.User{
335+
unittest.AssertExistsAndLoadBean(t, &user_model.User{
336336
Name: gitLDAPUser.UserName,
337337
})
338338
}

‎tests/integration/pull_review_test.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ func TestPullView_CodeOwner(t *testing.T) {
9292
testPullCreate(t, session, "user2", "test_codeowner", false, repo.DefaultBranch, "codeowner-basebranch", "Test Pull Request")
9393

9494
pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: repo.ID, HeadRepoID: repo.ID, HeadBranch: "codeowner-basebranch"})
95-
unittest.AssertExistsIf(t, true, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 5})
95+
unittest.AssertExistsAndLoadBean(t, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 5})
9696
assert.NoError(t, pr.LoadIssue(db.DefaultContext))
9797

9898
err := issue_service.ChangeTitle(db.DefaultContext, pr.Issue, user2, "[WIP] Test Pull Request")
@@ -139,7 +139,7 @@ func TestPullView_CodeOwner(t *testing.T) {
139139
testPullCreate(t, session, "user2", "test_codeowner", false, repo.DefaultBranch, "codeowner-basebranch2", "Test Pull Request2")
140140

141141
pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: repo.ID, HeadBranch: "codeowner-basebranch2"})
142-
unittest.AssertExistsIf(t, true, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 8})
142+
unittest.AssertExistsAndLoadBean(t, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 8})
143143
})
144144

145145
t.Run("Forked Repo Pull Request", func(t *testing.T) {
@@ -169,13 +169,13 @@ func TestPullView_CodeOwner(t *testing.T) {
169169
testPullCreateDirectly(t, session, "user5", "test_codeowner", forkedRepo.DefaultBranch, "", "", "codeowner-basebranch-forked", "Test Pull Request on Forked Repository")
170170

171171
pr := unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: forkedRepo.ID, HeadBranch: "codeowner-basebranch-forked"})
172-
unittest.AssertExistsIf(t, false, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 8})
172+
unittest.AssertNotExistsBean(t, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 8})
173173

174174
// create a pull request to base repository, code reviewers should be mentioned
175175
testPullCreateDirectly(t, session, repo.OwnerName, repo.Name, repo.DefaultBranch, forkedRepo.OwnerName, forkedRepo.Name, "codeowner-basebranch-forked", "Test Pull Request3")
176176

177177
pr = unittest.AssertExistsAndLoadBean(t, &issues_model.PullRequest{BaseRepoID: repo.ID, HeadRepoID: forkedRepo.ID, HeadBranch: "codeowner-basebranch-forked"})
178-
unittest.AssertExistsIf(t, true, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 8})
178+
unittest.AssertExistsAndLoadBean(t, &issues_model.Review{IssueID: pr.IssueID, Type: issues_model.ReviewTypeRequest, ReviewerID: 8})
179179
})
180180
})
181181
}

‎tests/integration/signin_test.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99
"strings"
1010
"testing"
1111

12+
"code.gitea.io/gitea/models/db"
1213
"code.gitea.io/gitea/models/unittest"
1314
user_model "code.gitea.io/gitea/models/user"
1415
"code.gitea.io/gitea/modules/setting"
@@ -17,6 +18,7 @@ import (
1718
"code.gitea.io/gitea/tests"
1819

1920
"github.com/stretchr/testify/assert"
21+
"github.com/stretchr/testify/require"
2022
)
2123

2224
func testLoginFailed(t *testing.T, username, password, message string) {
@@ -42,7 +44,7 @@ func TestSignin(t *testing.T) {
4244
user.Name = "testuser"
4345
user.LowerName = strings.ToLower(user.Name)
4446
user.ID = 0
45-
unittest.AssertSuccessfulInsert(t, user)
47+
require.NoError(t, db.Insert(db.DefaultContext, user))
4648

4749
samples := []struct {
4850
username string

0 commit comments

Comments
 (0)