Skip to content

Commit 4527748

Browse files
lunnylunny
authored
Fix bug hidden on CI and make ci failed if tests failure (#29254)
The tests on migration tests failed but CI reports successfully https://github.com/go-gitea/gitea/actions/runs/7364373807/job/20044685969#step:8:141 This PR will fix the bug on migration v283 and also the CI hidden behaviour. The reason is on the Makefile `GITEA_ROOT="$(CURDIR)" GITEA_CONF=tests/mysql.ini $(GO) test $(GOTESTFLAGS) -tags='$(TEST_TAGS)' $(MIGRATE_TEST_PACKAGES)` will return the error exit code. But `for pkg in $(shell $(GO) list code.gitea.io/gitea/models/migrations/...); do \ GITEA_ROOT="$(CURDIR)" GITEA_CONF=tests/mysql.ini $(GO) test $(GOTESTFLAGS) -tags '$(TEST_TAGS)' $$pkg; \ done` will not work. This also fix #29602
1 parent c1331d1 commit 4527748

File tree

20 files changed

+174
-46
lines changed

20 files changed

+174
-46
lines changed

‎.github/workflows/pull-db-tests.yml

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,10 @@ jobs:
4949
- run: make backend
5050
env:
5151
TAGS: bindata
52-
- run: make test-pgsql-migration test-pgsql
52+
- name: run migration tests
53+
run: make test-pgsql-migration
54+
- name: run tests
55+
run: make test-pgsql
5356
timeout-minutes: 50
5457
env:
5558
TAGS: bindata gogit
@@ -72,7 +75,10 @@ jobs:
7275
- run: make backend
7376
env:
7477
TAGS: bindata gogit sqlite sqlite_unlock_notify
75-
- run: make test-sqlite-migration test-sqlite
78+
- name: run migration tests
79+
run: make test-sqlite-migration
80+
- name: run tests
81+
run: make test-sqlite
7682
timeout-minutes: 50
7783
env:
7884
TAGS: bindata gogit sqlite sqlite_unlock_notify
@@ -175,8 +181,10 @@ jobs:
175181
- run: make backend
176182
env:
177183
TAGS: bindata
184+
- name: run migration tests
185+
run: make test-mysql-migration
178186
- name: run tests
179-
run: make test-mysql-migration integration-test-coverage
187+
run: make integration-test-coverage
180188
env:
181189
TAGS: bindata
182190
RACE_ENABLED: true
@@ -208,7 +216,9 @@ jobs:
208216
- run: make backend
209217
env:
210218
TAGS: bindata
211-
- run: make test-mssql-migration test-mssql
219+
- run: make test-mssql-migration
220+
- name: run tests
221+
run: make test-mssql
212222
timeout-minutes: 50
213223
env:
214224
TAGS: bindata

‎Makefile

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ LINUX_ARCHS ?= linux/amd64,linux/386,linux/arm-5,linux/arm-6,linux/arm64
115115

116116
GO_PACKAGES ?= $(filter-out code.gitea.io/gitea/tests/integration/migration-test code.gitea.io/gitea/tests code.gitea.io/gitea/tests/integration code.gitea.io/gitea/tests/e2e,$(shell $(GO) list ./... | grep -v /vendor/))
117117
GO_TEST_PACKAGES ?= $(filter-out $(shell $(GO) list code.gitea.io/gitea/models/migrations/...) code.gitea.io/gitea/tests/integration/migration-test code.gitea.io/gitea/tests code.gitea.io/gitea/tests/integration code.gitea.io/gitea/tests/e2e,$(shell $(GO) list ./... | grep -v /vendor/))
118+
MIGRATE_TEST_PACKAGES ?= $(shell $(GO) list code.gitea.io/gitea/models/migrations/...)
118119

119120
FOMANTIC_WORK_DIR := web_src/fomantic
120121

@@ -710,40 +711,31 @@ migrations.sqlite.test: $(GO_SOURCES) generate-ini-sqlite
710711

711712
.PHONY: migrations.individual.mysql.test
712713
migrations.individual.mysql.test: $(GO_SOURCES)
713-
for pkg in $(shell $(GO) list code.gitea.io/gitea/models/migrations/...); do \
714-
GITEA_ROOT="$(CURDIR)" GITEA_CONF=tests/mysql.ini $(GO) test $(GOTESTFLAGS) -tags '$(TEST_TAGS)' $$pkg; \
715-
done
714+
GITEA_ROOT="$(CURDIR)" GITEA_CONF=tests/mysql.ini $(GO) test $(GOTESTFLAGS) -tags='$(TEST_TAGS)' -p 1 $(MIGRATE_TEST_PACKAGES)
716715

717716
.PHONY: migrations.individual.sqlite.test\#%
718717
migrations.individual.sqlite.test\#%: $(GO_SOURCES) generate-ini-sqlite
719718
GITEA_ROOT="$(CURDIR)" GITEA_CONF=tests/sqlite.ini $(GO) test $(GOTESTFLAGS) -tags '$(TEST_TAGS)' code.gitea.io/gitea/models/migrations/$*
720719

721720
.PHONY: migrations.individual.pgsql.test
722721
migrations.individual.pgsql.test: $(GO_SOURCES)
723-
for pkg in $(shell $(GO) list code.gitea.io/gitea/models/migrations/...); do \
724-
GITEA_ROOT="$(CURDIR)" GITEA_CONF=tests/pgsql.ini $(GO) test $(GOTESTFLAGS) -tags '$(TEST_TAGS)' $$pkg; \
725-
done
722+
GITEA_ROOT="$(CURDIR)" GITEA_CONF=tests/pgsql.ini $(GO) test $(GOTESTFLAGS) -tags='$(TEST_TAGS)' -p 1 $(MIGRATE_TEST_PACKAGES)
726723

727724
.PHONY: migrations.individual.pgsql.test\#%
728725
migrations.individual.pgsql.test\#%: $(GO_SOURCES) generate-ini-pgsql
729726
GITEA_ROOT="$(CURDIR)" GITEA_CONF=tests/pgsql.ini $(GO) test $(GOTESTFLAGS) -tags '$(TEST_TAGS)' code.gitea.io/gitea/models/migrations/$*
730727

731-
732728
.PHONY: migrations.individual.mssql.test
733729
migrations.individual.mssql.test: $(GO_SOURCES) generate-ini-mssql
734-
for pkg in $(shell $(GO) list code.gitea.io/gitea/models/migrations/...); do \
735-
GITEA_ROOT="$(CURDIR)" GITEA_CONF=tests/mssql.ini $(GO) test $(GOTESTFLAGS) -tags '$(TEST_TAGS)' $$pkg -test.failfast; \
736-
done
730+
GITEA_ROOT="$(CURDIR)" GITEA_CONF=tests/mssql.ini $(GO) test $(GOTESTFLAGS) -tags='$(TEST_TAGS)' -p 1 $(MIGRATE_TEST_PACKAGES)
737731

738732
.PHONY: migrations.individual.mssql.test\#%
739733
migrations.individual.mssql.test\#%: $(GO_SOURCES) generate-ini-mssql
740734
GITEA_ROOT="$(CURDIR)" GITEA_CONF=tests/mssql.ini $(GO) test $(GOTESTFLAGS) -tags '$(TEST_TAGS)' code.gitea.io/gitea/models/migrations/$*
741735

742736
.PHONY: migrations.individual.sqlite.test
743737
migrations.individual.sqlite.test: $(GO_SOURCES) generate-ini-sqlite
744-
for pkg in $(shell $(GO) list code.gitea.io/gitea/models/migrations/...); do \
745-
GITEA_ROOT="$(CURDIR)" GITEA_CONF=tests/sqlite.ini $(GO) test $(GOTESTFLAGS) -tags '$(TEST_TAGS)' $$pkg; \
746-
done
738+
GITEA_ROOT="$(CURDIR)" GITEA_CONF=tests/sqlite.ini $(GO) test $(GOTESTFLAGS) -tags='$(TEST_TAGS)' -p 1 $(MIGRATE_TEST_PACKAGES)
747739

748740
.PHONY: migrations.individual.sqlite.test\#%
749741
migrations.individual.sqlite.test\#%: $(GO_SOURCES) generate-ini-sqlite

‎models/migrations/base/db_test.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,14 @@ func Test_DropTableColumns(t *testing.T) {
3636
"updated_unix",
3737
}
3838

39+
x.SetMapper(names.GonicMapper{})
40+
3941
for i := range columns {
40-
x.SetMapper(names.GonicMapper{})
4142
if err := x.Sync(new(DropTest)); err != nil {
4243
t.Errorf("unable to create DropTest table: %v", err)
4344
return
4445
}
46+
4547
sess := x.NewSession()
4648
if err := sess.Begin(); err != nil {
4749
sess.Close()
@@ -64,7 +66,6 @@ func Test_DropTableColumns(t *testing.T) {
6466
return
6567
}
6668
for j := range columns[i+1:] {
67-
x.SetMapper(names.GonicMapper{})
6869
if err := x.Sync(new(DropTest)); err != nil {
6970
t.Errorf("unable to create DropTest table: %v", err)
7071
return

‎models/migrations/fixtures/Test_AddIssueResourceIndexTable/issue.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
-
2+
id: 1
3+
repo_id: 1
4+
index: 1

‎models/migrations/fixtures/Test_AddRepoIDForAttachment/attachment.yml

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
-
2+
id: 1
3+
uuid: a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11
4+
issue_id: 1
5+
release_id: 0
6+
7+
-
8+
id: 2
9+
uuid: a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a12
10+
issue_id: 0
11+
release_id: 1

‎models/migrations/fixtures/Test_AddRepoIDForAttachment/issue.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
-
2+
id: 1
3+
repo_id: 1

‎models/migrations/fixtures/Test_AddRepoIDForAttachment/release.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
-
2+
id: 1
3+
repo_id: 1

‎models/migrations/fixtures/Test_RepositoryFormat/comment.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
-
2+
id: 1
3+
commit_sha: 19fe5caf872476db265596eaac1dc35ad1c6422d

‎models/migrations/fixtures/Test_RepositoryFormat/commit_status.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
-
2+
id: 1
3+
context_hash: 19fe5caf872476db265596eaac1dc35ad1c6422d

‎models/migrations/fixtures/Test_RepositoryFormat/pull_request.yml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
-
2+
id: 1
3+
commit_sha: 19fe5caf872476db265596eaac1dc35ad1c6422d
4+
merge_base: 19fe5caf872476db265596eaac1dc35ad1c6422d
5+
merged_commit_id: 19fe5caf872476db265596eaac1dc35ad1c6422d

‎models/migrations/fixtures/Test_RepositoryFormat/release.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
-
2+
id: 1
3+
sha1: 19fe5caf872476db265596eaac1dc35ad1c6422d

‎models/migrations/fixtures/Test_RepositoryFormat/repo_archiver.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
-
2+
id: 1
3+
commit_id: 19fe5caf872476db265596eaac1dc35ad1c6422d

‎models/migrations/fixtures/Test_RepositoryFormat/repo_indexer_status.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
-
2+
id: 1
3+
commit_sha: 19fe5caf872476db265596eaac1dc35ad1c6422d

‎models/migrations/fixtures/Test_RepositoryFormat/review_state.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
-
2+
id: 1
3+
commit_sha: 19fe5caf872476db265596eaac1dc35ad1c6422d

‎models/migrations/fixtures/Test_UpdateBadgeColName/badge.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
-
2+
id: 1
3+
description: the badge
4+
image_url: https://gitea.com/myimage.png

‎models/migrations/v1_16/v193_test.go

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ func Test_AddRepoIDForAttachment(t *testing.T) {
1515
type Attachment struct {
1616
ID int64 `xorm:"pk autoincr"`
1717
UUID string `xorm:"uuid UNIQUE"`
18-
RepoID int64 `xorm:"INDEX"` // this should not be zero
1918
IssueID int64 `xorm:"INDEX"` // maybe zero when creating
2019
ReleaseID int64 `xorm:"INDEX"` // maybe zero when creating
2120
UploaderID int64 `xorm:"INDEX DEFAULT 0"`
@@ -44,25 +43,34 @@ func Test_AddRepoIDForAttachment(t *testing.T) {
4443
return
4544
}
4645

47-
var issueAttachments []*Attachment
48-
err := x.Where("issue_id > 0").Find(&issueAttachments)
46+
type NewAttachment struct {
47+
ID int64 `xorm:"pk autoincr"`
48+
UUID string `xorm:"uuid UNIQUE"`
49+
RepoID int64 `xorm:"INDEX"` // this should not be zero
50+
IssueID int64 `xorm:"INDEX"` // maybe zero when creating
51+
ReleaseID int64 `xorm:"INDEX"` // maybe zero when creating
52+
UploaderID int64 `xorm:"INDEX DEFAULT 0"`
53+
}
54+
55+
var issueAttachments []*NewAttachment
56+
err := x.Table("attachment").Where("issue_id > 0").Find(&issueAttachments)
4957
assert.NoError(t, err)
5058
for _, attach := range issueAttachments {
51-
assert.Greater(t, attach.RepoID, 0)
52-
assert.Greater(t, attach.IssueID, 0)
59+
assert.Greater(t, attach.RepoID, int64(0))
60+
assert.Greater(t, attach.IssueID, int64(0))
5361
var issue Issue
5462
has, err := x.ID(attach.IssueID).Get(&issue)
5563
assert.NoError(t, err)
5664
assert.True(t, has)
5765
assert.EqualValues(t, attach.RepoID, issue.RepoID)
5866
}
5967

60-
var releaseAttachments []*Attachment
61-
err = x.Where("release_id > 0").Find(&releaseAttachments)
68+
var releaseAttachments []*NewAttachment
69+
err = x.Table("attachment").Where("release_id > 0").Find(&releaseAttachments)
6270
assert.NoError(t, err)
6371
for _, attach := range releaseAttachments {
64-
assert.Greater(t, attach.RepoID, 0)
65-
assert.Greater(t, attach.IssueID, 0)
72+
assert.Greater(t, attach.RepoID, int64(0))
73+
assert.Greater(t, attach.ReleaseID, int64(0))
6674
var release Release
6775
has, err := x.ID(attach.ReleaseID).Get(&release)
6876
assert.NoError(t, err)

‎models/migrations/v1_22/v283.go

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,10 @@
44
package v1_22 //nolint
55

66
import (
7+
"fmt"
8+
79
"xorm.io/xorm"
10+
"xorm.io/xorm/schemas"
811
)
912

1013
func AddCombinedIndexToIssueUser(x *xorm.Engine) error {
@@ -20,8 +23,18 @@ func AddCombinedIndexToIssueUser(x *xorm.Engine) error {
2023
return err
2124
}
2225
for _, issueUser := range duplicatedIssueUsers {
23-
if _, err := x.Exec("delete from issue_user where id in (SELECT id FROM issue_user WHERE issue_id = ? and uid = ? limit ?)", issueUser.IssueID, issueUser.UID, issueUser.Cnt-1); err != nil {
24-
return err
26+
if x.Dialect().URI().DBType == schemas.MSSQL {
27+
if _, err := x.Exec(fmt.Sprintf("delete from issue_user where id in (SELECT top %d id FROM issue_user WHERE issue_id = ? and uid = ?)", issueUser.Cnt-1), issueUser.IssueID, issueUser.UID); err != nil {
28+
return err
29+
}
30+
} else {
31+
var ids []int64
32+
if err := x.SQL("SELECT id FROM issue_user WHERE issue_id = ? and uid = ? limit ?", issueUser.IssueID, issueUser.UID, issueUser.Cnt-1).Find(&ids); err != nil {
33+
return err
34+
}
35+
if _, err := x.Table("issue_user").In("id", ids).Delete(); err != nil {
36+
return err
37+
}
2538
}
2639
}
2740

‎models/migrations/v1_22/v286.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,9 @@ func expandHashReferencesToSha256(x *xorm.Engine) error {
3636
if setting.Database.Type.IsMSSQL() {
3737
// drop indexes that need to be re-created afterwards
3838
droppedIndexes := []string{
39-
"DROP INDEX commit_status.IDX_commit_status_context_hash",
40-
"DROP INDEX review_state.UQE_review_state_pull_commit_user",
41-
"DROP INDEX repo_archiver.UQE_repo_archiver_s",
39+
"DROP INDEX IF EXISTS [IDX_commit_status_context_hash] ON [commit_status]",
40+
"DROP INDEX IF EXISTS [UQE_review_state_pull_commit_user] ON [review_state]",
41+
"DROP INDEX IF EXISTS [UQE_repo_archiver_s] ON [repo_archiver]",
4242
}
4343
for _, s := range droppedIndexes {
4444
_, err := db.Exec(s)
@@ -53,7 +53,7 @@ func expandHashReferencesToSha256(x *xorm.Engine) error {
5353
if setting.Database.Type.IsMySQL() {
5454
_, err = db.Exec(fmt.Sprintf("ALTER TABLE `%s` MODIFY COLUMN `%s` VARCHAR(64)", alts[0], alts[1]))
5555
} else if setting.Database.Type.IsMSSQL() {
56-
_, err = db.Exec(fmt.Sprintf("ALTER TABLE `%s` ALTER COLUMN `%s` VARCHAR(64)", alts[0], alts[1]))
56+
_, err = db.Exec(fmt.Sprintf("ALTER TABLE [%s] ALTER COLUMN [%s] VARCHAR(64)", alts[0], alts[1]))
5757
} else {
5858
_, err = db.Exec(fmt.Sprintf("ALTER TABLE `%s` ALTER COLUMN `%s` TYPE VARCHAR(64)", alts[0], alts[1]))
5959
}

‎models/migrations/v1_22/v286_test.go

Lines changed: 61 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,72 @@ func PrepareOldRepository(t *testing.T) (*xorm.Engine, func()) {
1717
ID int64 `xorm:"pk autoincr"`
1818
}
1919

20+
type CommitStatus struct {
21+
ID int64
22+
ContextHash string
23+
}
24+
25+
type RepoArchiver struct {
26+
ID int64
27+
RepoID int64
28+
Type int
29+
CommitID string
30+
}
31+
32+
type ReviewState struct {
33+
ID int64
34+
CommitSHA string
35+
UserID int64
36+
PullID int64
37+
}
38+
39+
type Comment struct {
40+
ID int64
41+
CommitSHA string
42+
}
43+
44+
type PullRequest struct {
45+
ID int64
46+
CommitSHA string
47+
MergeBase string
48+
MergedCommitID string
49+
}
50+
51+
type Release struct {
52+
ID int64
53+
Sha1 string
54+
}
55+
56+
type RepoIndexerStatus struct {
57+
ID int64
58+
CommitSHA string
59+
}
60+
61+
type Review struct {
62+
ID int64
63+
CommitID string
64+
}
65+
2066
// Prepare and load the testing database
21-
return base.PrepareTestEnv(t, 0, new(Repository))
67+
return base.PrepareTestEnv(t, 0,
68+
new(Repository),
69+
new(CommitStatus),
70+
new(RepoArchiver),
71+
new(ReviewState),
72+
new(Review),
73+
new(Comment),
74+
new(PullRequest),
75+
new(Release),
76+
new(RepoIndexerStatus),
77+
)
2278
}
2379

2480
func Test_RepositoryFormat(t *testing.T) {
2581
x, deferable := PrepareOldRepository(t)
2682
defer deferable()
2783

84+
assert.NoError(t, AdjustDBForSha256(x))
85+
2886
type Repository struct {
2987
ID int64 `xorm:"pk autoincr"`
3088
ObjectFormatName string `xorg:"not null default('sha1')"`
@@ -37,12 +95,10 @@ func Test_RepositoryFormat(t *testing.T) {
3795
assert.NoError(t, err)
3896
assert.EqualValues(t, 4, count)
3997

40-
assert.NoError(t, AdjustDBForSha256(x))
41-
42-
repo.ID = 20
4398
repo.ObjectFormatName = "sha256"
4499
_, err = x.Insert(repo)
45100
assert.NoError(t, err)
101+
id := repo.ID
46102

47103
count, err = x.Count(new(Repository))
48104
assert.NoError(t, err)
@@ -55,7 +111,7 @@ func Test_RepositoryFormat(t *testing.T) {
55111
assert.EqualValues(t, "sha1", repo.ObjectFormatName)
56112

57113
repo = new(Repository)
58-
ok, err = x.ID(20).Get(repo)
114+
ok, err = x.ID(id).Get(repo)
59115
assert.NoError(t, err)
60116
assert.EqualValues(t, true, ok)
61117
assert.EqualValues(t, "sha256", repo.ObjectFormatName)

0 commit comments

Comments
 (0)