Refactor integration tests infrastructure by silverwind · Pull Request #37462 · go-gitea/gitea

silverwind · 2026-04-28T00:41:56Z

Related to #37434 but does not solve it.

This PR was written with the help of Claude Opus 4.7

Replace `go test -c -o foo.test; ./foo.test` with direct `go test` invocation so Go's testcache works for integration and migration tests (the pre-built-binary pattern bypasses the cache entirely). Collapse 28 near-identical per-DB targets (test-X, test-X#%, test-X-migration, bench-X, migrations.X.test, migrations.individual.X.test and its #% variant) into a `define` macro called once per DB. As a drive-by, the macro generates `migrations.individual.mysql.test#%` which the original Makefile was missing for mysql only. Also drop the now-redundant `\$(GO_SOURCES)` prereq from the migrations.individual targets — `go test` does its own staleness check, the prereq just stat'd thousands of files for nothing. Closes go-gitea#37434 Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>

Several test files use repo-relative paths (tests/integration/avatar.png, tests/integration/migration-test/, etc.) that worked when the pre-built test binary ran from repo root but break under direct `go test` (cwd = package dir). Anchor cwd in the test setup to keep both invocation styles working. Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>

Predates the per-DB test binary split; no longer produced. The broad `*.test` entry on line 40 covers any stray Go test binary. Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>

Copilot

Pull request overview

Enables Go test result caching for integration/migration tests by switching Makefile targets from precompiled go test -c binaries to direct go test execution, and updates test environment setup so repo-relative fixture paths remain stable.

Changes:

Replace integration/benchmark/migration Makefile targets to run go test directly (cacheable) and factor per-DB targets into a define macro.
Update SetupGiteaTestEnv to chdir to the repo root so repo-relative test files work when go test runs with cwd = package directory.
Clean up generated artifacts handling (make clean removes old migrations*.test; remove redundant .gitignore entry).

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.

File	Description
`modules/setting/testenv.go`	Anchors test process working directory to the source root for consistent repo-relative fixture access under `go test`.
`Makefile`	Reworks DB integration/benchmark/migration targets to use `go test` directly and consolidates duplicated targets via a macro.
`.gitignore`	Removes an obsolete ignore entry for a no-longer-generated test binary name.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Putting the chdir in SetupGiteaTestEnv broke unit tests that legitimately rely on cwd = package dir (services/attachment opens ./attachment_test.go, etc). Move the chdir to tests.InitTest (covers integration tests) and initMigrationTest (covers migration tests) instead — both are entry points only used by tests that historically ran from repo root. Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>

Replaces os.Chdir+require.NoError with t.Chdir, which auto-restores the cwd at test cleanup and satisfies the usetesting lint. Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>

silverwind · 2026-04-28T01:45:14Z

This should improve local integration test re-runs considerably. If we want CI to benefit fully from this caching we need to enable build-cache-rotate for the db jobs and find a larger storage to cache on, the default 10GB GHA cache is too small for full caching, needs at least ~50GB.

`go test` runs each package with cwd set to its directory. Reference the avatar/README/key fixtures and migration dumps relative to the package dir instead of the repo root, dropping the chdir hacks in InitTest and initMigrationTest. Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>

The integration test cwd is now the package dir (`tests/integration/`), not the repo root, so `tools/test-echo.go` no longer resolves. Use `../../tools/test-echo.go` so the markup-renderer subprocess can find it. Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>

wxiaoguang · 2026-04-28T04:45:07Z

The "paths" should be joint with setting.GetGiteaTestSourceRoot, then it can always be correct for various cases.

wxiaoguang · 2026-04-28T04:49:09Z

Integration benchmark tests can be removed.

They are useless after Add integration test for repository migration (#1983)

Reviewer feedback: derive paths from setting.GetGiteaTestSourceRoot() instead of relying on cwd, so the fixtures resolve under both `go test` (cwd = package dir) and the prebuilt binary style. Switch the avatar/README/key fixtures and migration SQL dumps to `filepath.Join(setting.GetGiteaTestSourceRoot(), …)`. For the markup test render command in sqlite.ini.tmpl, add a `{{REPO_ROOT}}` token substituted to `\$(CURDIR)` by `generate-ini-sqlite`. Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>

Reviewer feedback: the per-DB `bench-*` Make targets and the two `BenchmarkAPICreateFile*` integration cases are superseded by the package-level benchmarks added in go-gitea#3161 — drop them. Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>

silverwind · 2026-04-28T05:16:37Z

REPO_ROOT addition may need review.

Per review feedback: align with the existing GITEA_TEST_ROOT env var that setting.GetGiteaTestSourceRoot() already honors. Falls back to $(CURDIR) when unset. Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>

silverwind · 2026-04-28T05:28:41Z

In case you question this fallback:

make runs without GITEA_TEST_ROOT set in the common case (just make test-sqlite from the repo root), so without the fallback the substitution would yield an empty string and RENDER_COMMAND = go run /tools/test-echo.go. The pre-existing WORK_PATH line already uses $(CURDIR) for the same reason

wxiaoguang · 2026-04-28T05:38:20Z

In case you question this fallback:

No, actually I don't have such question, I know that "GITEA_TEST_ROOT" is never set. It is just a legacy env var from very old code base. Since it is used, keep using the same var name is more consistent, easy to grep, and it's easier to reuse it correctly in the future.

wxiaoguang · 2026-04-28T19:56:51Z

I don't like this new test-e2e CI step output. Previously one could see time taken for build (~2min} and the actual e2e test (~45s), now it's all one big step at ~3min.

You can still separate the steps.

However, it should share the same build output with "bindata".

E2e should use the "real distribution binary" to test, it shouldn't use a specially built binary.

And for all CI build, I would remove -v from go build.

It's up to you

Run `make backend` as its own step before `make test-e2e`. The backend dep on test-e2e becomes a no-op, so the build (~2 min) and the e2e run (~45 s) appear in separate CI step rows instead of one combined ~3 min step. Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>

silverwind · 2026-04-28T20:44:36Z

Added separate step, decided against removing -v for now, it's not a problem as long as these builds are hidden in separate steps.

- Test_DropTableColumns logged "is skipped" but never called t.Skip(), so the test reported PASS without running. Replace with t.Skip() and move defer deferable() above the guard so it still runs after Goexit. - The TODO on test-integration cited a 10-min sqlite deadlock that was actually a missing -timeout in the workflow's GOTEST_FLAGS (already fixed in 845f7de). Rewrite the comment to state the real reasons the binary path is kept: testlogger forwards to t.Log so `go test -v` floods output, and testcache can't help these tests anyway because they mutate the work directory. Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>

a1012112796

some desciption update suggestions about act_runner local runing.

test run mysql looks ok in my local test run. but sqlite test found an amazing data crace.

log-sqlite.txt
log-mysql.txt

wxiaoguang · 2026-04-29T04:07:54Z

some desciption update suggestions about act_runner local runing.

Thank you very much. This PR only moved some parts of the README. IMHO the "Actions" related readme is not really related to this one.

And there are many suggested changes, so it's better to propose a new and complete improvement PR after this PR, what do you think?

test run mysql looks ok in my local test run. but sqlite test found an amazing data crace.

~~It seems to some abuse, can be (if the fix is easy and quick ) or no need to be in this PR's scope (otherwise)~~

Highly likely unrelated.

wxiaoguang · 2026-04-29T05:57:17Z

test run mysql looks ok in my local test run. but sqlite test found an amazing data crace.

@silverwind to help you understand more about why the logs are useful, you can try to debug this problem.

The logs give you an overview of the context, when the case is executed.

Maybe you can spend some time on studying the data race problems too.

wxiaoguang

Nothing worse, no blocker problems. Data race is highly likely unrelated.

wxiaoguang · 2026-04-29T13:26:46Z

Made a new change to try to reduce CI time.

Before: any change to Makefile will trigger "docker-dryrun", it takes about 1h 40m
After: about 50minutes (half). The root reason is that non-amd64 host needs to use VM (actually not really VM, but binary translation) to run the container build, it is extremely slow.
- amd64 3-4 minutes.
- arm64 42 minutes
- riscv64: 54 minutes

Feel free to reset/revert if this change is not wanted.

wxiaoguang · 2026-04-29T15:09:57Z

Should be good enough now (container build dryrun time: 1h40m -> ~ 50m)

TODOs for following up PRs:

use gotestsum to optimize test log output
- gotestsum -f testname should have pretty good output without noisy logs
refactor test logger (if needed)
use go test instead of pre-compiled testing binary
merge lint steps into one (too many "lint" items in the "PR checks" list)
fine tune integration testing README
compare sqlite CI time between -race vs normal build, also compare it with the modernc library again

wxiaoguang · 2026-04-30T02:05:28Z

merge lint steps into one (too many "lint" items in the "PR checks" list)

-> Refactor CI workflows #37487

silverwind · 2026-04-30T18:11:06Z

TODOs for following up PRs

👍 to gotestsum

And #37434 will be the true big win when we achieve cacheable integration tests, it should be possible.

* origin/main: Refactor CI workflows (go-gitea#37487) Allow multiple projects per issue and pull requests (go-gitea#36784) [skip ci] Updated translations via Crowdin Refactor compare diff/pull page (1) (go-gitea#37481) Fix review submission from single-commit PR view (go-gitea#37475) Refactor integration tests infrastructure (go-gitea#37462) Fix allow maintainer edit permission check (go-gitea#37479) Serve OpenAPI 3.0 spec at /openapi.v1.json (go-gitea#37038) Batch-load related data in actions run, job, and task API endpoints (go-gitea#37032) Add DEFAULT_TITLE_SOURCE setting for pull request title default behavior (go-gitea#37465) Fix compare dropdown for branches without common history (go-gitea#37470) FIX: URL sanitization to handle schemeless credentials (go-gitea#37440) Refactor pull request view (4) (go-gitea#37451) # Conflicts: # modules/indexer/issues/elasticsearch/elasticsearch.go

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 28, 2026

silverwind added type/testing skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. labels Apr 28, 2026

silverwind changed the title ~~Make integration tests cacheable, dedupe Makefile DB targets~~ Cleanup makefile test targets Apr 28, 2026

silverwind changed the title ~~Cleanup makefile test targets~~ Cleanup Makefile test targets Apr 28, 2026

silverwind changed the title ~~Cleanup Makefile test targets~~ Cleanup Makefile test targets and enable partial integration test caching Apr 28, 2026

silverwind changed the title ~~Cleanup Makefile test targets and enable partial integration test caching~~ Enable partial integration test caching, clean up Makefile Apr 28, 2026

Drop stale /integrations.test from .gitignore

4adf51d

Predates the per-DB test binary split; no longer produced. The broad `*.test` entry on line 40 covers any stray Go test binary. Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>

silverwind changed the title ~~Enable partial integration test caching, clean up Makefile~~ Enable integration test caching, clean up Makefile Apr 28, 2026

silverwind requested a review from Copilot April 28, 2026 00:54

Copilot started reviewing on behalf of silverwind April 28, 2026 00:57 View session

Copilot AI reviewed Apr 28, 2026

View reviewed changes

silverwind and others added 2 commits April 28, 2026 03:07

Use t.Chdir in migration test init

2ef5389

Replaces os.Chdir+require.NoError with t.Chdir, which auto-restores the cwd at test cleanup and satisfies the usetesting lint. Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>

silverwind and others added 2 commits April 28, 2026 04:40

silverwind mentioned this pull request Apr 28, 2026

Don't unblock run-level-concurrency-blocked runs in the resolver #37461

Open

silverwind and others added 2 commits April 28, 2026 07:13

This comment was marked as resolved.

Sign in to view

Use GITEA_TEST_ROOT for ini render-command substitution

9d32c0a

Per review feedback: align with the existing GITEA_TEST_ROOT env var that setting.GetGiteaTestSourceRoot() already honors. Falls back to $(CURDIR) when unset. Co-Authored-By: Claude (Opus 4.7) <noreply@anthropic.com>

wxiaoguang reviewed Apr 28, 2026

View reviewed changes

Comment thread Makefile Outdated

silverwind added the type/refactoring Existing code has been cleaned up. There should be no new functionality. label Apr 28, 2026

silverwind commented Apr 28, 2026

View reviewed changes

Comment thread Makefile

Merge branch 'main' into cacheable-integration-tests

73e1a7b

a1012112796 reviewed Apr 29, 2026

View reviewed changes

Comment thread tests/integration/README.md

Comment thread tests/integration/README.md

Comment thread tests/integration/README.md

Comment thread tests/integration/README.md

Comment thread tests/integration/README.md

wxiaoguang approved these changes Apr 29, 2026

View reviewed changes

GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 29, 2026

Merge branch 'main' into cacheable-integration-tests

c8e0df7

wxiaoguang force-pushed the cacheable-integration-tests branch from 66b68a3 to addb9c0 Compare April 29, 2026 13:24

docke dryrun in parallel

2b22e24

wxiaoguang force-pushed the cacheable-integration-tests branch from addb9c0 to 2b22e24 Compare April 29, 2026 14:01

lunny approved these changes Apr 29, 2026

View reviewed changes

GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 29, 2026

lunny added this to the 1.27.0 milestone Apr 29, 2026

Merge branch 'main' into cacheable-integration-tests

54243d7

wxiaoguang enabled auto-merge (squash) April 29, 2026 16:17

wxiaoguang merged commit d57d063 into go-gitea:main Apr 29, 2026
28 checks passed

wxiaoguang deleted the cacheable-integration-tests branch April 29, 2026 16:37

Uh oh!

Conversation

silverwind commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

silverwind commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wxiaoguang commented Apr 28, 2026

Uh oh!

wxiaoguang commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

silverwind commented Apr 28, 2026

Uh oh!

This comment was marked as resolved.

silverwind commented Apr 28, 2026

Uh oh!

wxiaoguang commented Apr 28, 2026

Uh oh!

Uh oh!

wxiaoguang commented Apr 28, 2026

Uh oh!

silverwind commented Apr 28, 2026

Uh oh!

Uh oh!

a1012112796 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wxiaoguang commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wxiaoguang commented Apr 29, 2026

Uh oh!

wxiaoguang left a comment

Choose a reason for hiding this comment

Uh oh!

wxiaoguang commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wxiaoguang commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

wxiaoguang commented Apr 30, 2026

Uh oh!

silverwind commented Apr 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

silverwind commented Apr 28, 2026 •

edited

Loading

silverwind commented Apr 28, 2026 •

edited

Loading

wxiaoguang commented Apr 28, 2026 •

edited

Loading

wxiaoguang commented Apr 29, 2026 •

edited

Loading

wxiaoguang commented Apr 29, 2026 •

edited

Loading

wxiaoguang commented Apr 29, 2026 •

edited

Loading