From e8b6d28ab994a32b345a08241600a5ebbe0c0a4b Mon Sep 17 00:00:00 2001 From: Lunny Xiao Date: Sat, 9 Mar 2024 17:44:02 +0800 Subject: [PATCH] Fix bug hidden on CI and make ci failed if tests failure (#29254) (#29662) Backport #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 migrations 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. --------- Co-authored-by: Giteabot --- .github/workflows/pull-db-tests.yml | 18 ++++++++++--- Makefile | 18 ++++--------- models/migrations/base/db_test.go | 5 ++-- .../Test_AddIssueResourceIndexTable/issue.yml | 4 +++ .../attachment.yml | 11 ++++++++ .../Test_AddRepoIDForAttachment/issue.yml | 3 +++ .../Test_AddRepoIDForAttachment/release.yml | 3 +++ models/migrations/v1_16/v193_test.go | 26 ++++++++++++------- 8 files changed, 60 insertions(+), 28 deletions(-) create mode 100644 models/migrations/fixtures/Test_AddIssueResourceIndexTable/issue.yml create mode 100644 models/migrations/fixtures/Test_AddRepoIDForAttachment/attachment.yml create mode 100644 models/migrations/fixtures/Test_AddRepoIDForAttachment/issue.yml create mode 100644 models/migrations/fixtures/Test_AddRepoIDForAttachment/release.yml diff --git a/.github/workflows/pull-db-tests.yml b/.github/workflows/pull-db-tests.yml index a6fb85937c..ae7c762da9 100644 --- a/.github/workflows/pull-db-tests.yml +++ b/.github/workflows/pull-db-tests.yml @@ -49,7 +49,10 @@ jobs: - run: make backend env: TAGS: bindata - - run: make test-pgsql-migration test-pgsql + - name: run migration tests + run: make test-pgsql-migration + - name: run tests + run: make test-pgsql timeout-minutes: 50 env: TAGS: bindata gogit @@ -72,7 +75,10 @@ jobs: - run: make backend env: TAGS: bindata gogit sqlite sqlite_unlock_notify - - run: make test-sqlite-migration test-sqlite + - name: run migration tests + run: make test-sqlite-migration + - name: run tests + run: make test-sqlite timeout-minutes: 50 env: TAGS: bindata gogit sqlite sqlite_unlock_notify @@ -189,8 +195,10 @@ jobs: - run: make backend env: TAGS: bindata + - name: run migration tests + run: make test-mysql-migration - name: run tests - run: make test-mysql-migration integration-test-coverage + run: make integration-test-coverage env: TAGS: bindata RACE_ENABLED: true @@ -252,7 +260,9 @@ jobs: - run: make backend env: TAGS: bindata - - run: make test-mssql-migration test-mssql + - run: make test-mssql-migration + - name: run tests + run: make test-mssql timeout-minutes: 50 env: TAGS: bindata diff --git a/Makefile b/Makefile index 9bda7036c7..ece95d2ad4 100644 --- a/Makefile +++ b/Makefile @@ -111,6 +111,7 @@ LINUX_ARCHS ?= linux/amd64,linux/386,linux/arm-5,linux/arm-6,linux/arm64 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/)) 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/)) +MIGRATE_TEST_PACKAGES ?= $(shell $(GO) list code.gitea.io/gitea/models/migrations/...) FOMANTIC_WORK_DIR := web_src/fomantic @@ -748,9 +749,7 @@ migrations.sqlite.test: $(GO_SOURCES) generate-ini-sqlite .PHONY: migrations.individual.mysql.test migrations.individual.mysql.test: $(GO_SOURCES) - 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 + GITEA_ROOT="$(CURDIR)" GITEA_CONF=tests/mysql.ini $(GO) test $(GOTESTFLAGS) -tags='$(TEST_TAGS)' -p 1 $(MIGRATE_TEST_PACKAGES) .PHONY: migrations.individual.mysql8.test migrations.individual.mysql8.test: $(GO_SOURCES) @@ -764,20 +763,15 @@ migrations.individual.sqlite.test\#%: $(GO_SOURCES) generate-ini-sqlite .PHONY: migrations.individual.pgsql.test migrations.individual.pgsql.test: $(GO_SOURCES) - for pkg in $(shell $(GO) list code.gitea.io/gitea/models/migrations/...); do \ - GITEA_ROOT="$(CURDIR)" GITEA_CONF=tests/pgsql.ini $(GO) test $(GOTESTFLAGS) -tags '$(TEST_TAGS)' $$pkg; \ - done + GITEA_ROOT="$(CURDIR)" GITEA_CONF=tests/pgsql.ini $(GO) test $(GOTESTFLAGS) -tags='$(TEST_TAGS)' -p 1 $(MIGRATE_TEST_PACKAGES) .PHONY: migrations.individual.pgsql.test\#% migrations.individual.pgsql.test\#%: $(GO_SOURCES) generate-ini-pgsql GITEA_ROOT="$(CURDIR)" GITEA_CONF=tests/pgsql.ini $(GO) test $(GOTESTFLAGS) -tags '$(TEST_TAGS)' code.gitea.io/gitea/models/migrations/$* - .PHONY: migrations.individual.mssql.test migrations.individual.mssql.test: $(GO_SOURCES) generate-ini-mssql - for pkg in $(shell $(GO) list code.gitea.io/gitea/models/migrations/...); do \ - GITEA_ROOT="$(CURDIR)" GITEA_CONF=tests/mssql.ini $(GO) test $(GOTESTFLAGS) -tags '$(TEST_TAGS)' $$pkg -test.failfast; \ - done + GITEA_ROOT="$(CURDIR)" GITEA_CONF=tests/mssql.ini $(GO) test $(GOTESTFLAGS) -tags='$(TEST_TAGS)' -p 1 $(MIGRATE_TEST_PACKAGES) .PHONY: migrations.individual.mssql.test\#% migrations.individual.mssql.test\#%: $(GO_SOURCES) generate-ini-mssql @@ -785,9 +779,7 @@ migrations.individual.mssql.test\#%: $(GO_SOURCES) generate-ini-mssql .PHONY: migrations.individual.sqlite.test migrations.individual.sqlite.test: $(GO_SOURCES) generate-ini-sqlite - for pkg in $(shell $(GO) list code.gitea.io/gitea/models/migrations/...); do \ - GITEA_ROOT="$(CURDIR)" GITEA_CONF=tests/sqlite.ini $(GO) test $(GOTESTFLAGS) -tags '$(TEST_TAGS)' $$pkg; \ - done + GITEA_ROOT="$(CURDIR)" GITEA_CONF=tests/sqlite.ini $(GO) test $(GOTESTFLAGS) -tags='$(TEST_TAGS)' -p 1 $(MIGRATE_TEST_PACKAGES) .PHONY: migrations.individual.sqlite.test\#% migrations.individual.sqlite.test\#%: $(GO_SOURCES) generate-ini-sqlite diff --git a/models/migrations/base/db_test.go b/models/migrations/base/db_test.go index 4d61b758cf..80bf00b22a 100644 --- a/models/migrations/base/db_test.go +++ b/models/migrations/base/db_test.go @@ -36,12 +36,14 @@ func Test_DropTableColumns(t *testing.T) { "updated_unix", } + x.SetMapper(names.GonicMapper{}) + for i := range columns { - x.SetMapper(names.GonicMapper{}) if err := x.Sync(new(DropTest)); err != nil { t.Errorf("unable to create DropTest table: %v", err) return } + sess := x.NewSession() if err := sess.Begin(); err != nil { sess.Close() @@ -64,7 +66,6 @@ func Test_DropTableColumns(t *testing.T) { return } for j := range columns[i+1:] { - x.SetMapper(names.GonicMapper{}) if err := x.Sync(new(DropTest)); err != nil { t.Errorf("unable to create DropTest table: %v", err) return diff --git a/models/migrations/fixtures/Test_AddIssueResourceIndexTable/issue.yml b/models/migrations/fixtures/Test_AddIssueResourceIndexTable/issue.yml new file mode 100644 index 0000000000..f95d47916b --- /dev/null +++ b/models/migrations/fixtures/Test_AddIssueResourceIndexTable/issue.yml @@ -0,0 +1,4 @@ +- + id: 1 + repo_id: 1 + index: 1 diff --git a/models/migrations/fixtures/Test_AddRepoIDForAttachment/attachment.yml b/models/migrations/fixtures/Test_AddRepoIDForAttachment/attachment.yml new file mode 100644 index 0000000000..056236ba9e --- /dev/null +++ b/models/migrations/fixtures/Test_AddRepoIDForAttachment/attachment.yml @@ -0,0 +1,11 @@ +- + id: 1 + uuid: a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a11 + issue_id: 1 + release_id: 0 + +- + id: 2 + uuid: a0eebc99-9c0b-4ef8-bb6d-6bb9bd380a12 + issue_id: 0 + release_id: 1 diff --git a/models/migrations/fixtures/Test_AddRepoIDForAttachment/issue.yml b/models/migrations/fixtures/Test_AddRepoIDForAttachment/issue.yml new file mode 100644 index 0000000000..7f3255096d --- /dev/null +++ b/models/migrations/fixtures/Test_AddRepoIDForAttachment/issue.yml @@ -0,0 +1,3 @@ +- + id: 1 + repo_id: 1 diff --git a/models/migrations/fixtures/Test_AddRepoIDForAttachment/release.yml b/models/migrations/fixtures/Test_AddRepoIDForAttachment/release.yml new file mode 100644 index 0000000000..7f3255096d --- /dev/null +++ b/models/migrations/fixtures/Test_AddRepoIDForAttachment/release.yml @@ -0,0 +1,3 @@ +- + id: 1 + repo_id: 1 diff --git a/models/migrations/v1_16/v193_test.go b/models/migrations/v1_16/v193_test.go index 17669a012e..d99bbc2962 100644 --- a/models/migrations/v1_16/v193_test.go +++ b/models/migrations/v1_16/v193_test.go @@ -15,7 +15,6 @@ func Test_AddRepoIDForAttachment(t *testing.T) { type Attachment struct { ID int64 `xorm:"pk autoincr"` UUID string `xorm:"uuid UNIQUE"` - RepoID int64 `xorm:"INDEX"` // this should not be zero IssueID int64 `xorm:"INDEX"` // maybe zero when creating ReleaseID int64 `xorm:"INDEX"` // maybe zero when creating UploaderID int64 `xorm:"INDEX DEFAULT 0"` @@ -44,12 +43,21 @@ func Test_AddRepoIDForAttachment(t *testing.T) { return } - var issueAttachments []*Attachment - err := x.Where("issue_id > 0").Find(&issueAttachments) + type NewAttachment struct { + ID int64 `xorm:"pk autoincr"` + UUID string `xorm:"uuid UNIQUE"` + RepoID int64 `xorm:"INDEX"` // this should not be zero + IssueID int64 `xorm:"INDEX"` // maybe zero when creating + ReleaseID int64 `xorm:"INDEX"` // maybe zero when creating + UploaderID int64 `xorm:"INDEX DEFAULT 0"` + } + + var issueAttachments []*NewAttachment + err := x.Table("attachment").Where("issue_id > 0").Find(&issueAttachments) assert.NoError(t, err) for _, attach := range issueAttachments { - assert.Greater(t, attach.RepoID, 0) - assert.Greater(t, attach.IssueID, 0) + assert.Greater(t, attach.RepoID, int64(0)) + assert.Greater(t, attach.IssueID, int64(0)) var issue Issue has, err := x.ID(attach.IssueID).Get(&issue) assert.NoError(t, err) @@ -57,12 +65,12 @@ func Test_AddRepoIDForAttachment(t *testing.T) { assert.EqualValues(t, attach.RepoID, issue.RepoID) } - var releaseAttachments []*Attachment - err = x.Where("release_id > 0").Find(&releaseAttachments) + var releaseAttachments []*NewAttachment + err = x.Table("attachment").Where("release_id > 0").Find(&releaseAttachments) assert.NoError(t, err) for _, attach := range releaseAttachments { - assert.Greater(t, attach.RepoID, 0) - assert.Greater(t, attach.IssueID, 0) + assert.Greater(t, attach.RepoID, int64(0)) + assert.Greater(t, attach.ReleaseID, int64(0)) var release Release has, err := x.ID(attach.ReleaseID).Get(&release) assert.NoError(t, err)