[IAmJSD]

Why

On main's CI run 40153816430 (after #1400 moved integration tests onto one shared Postgres server per package), the Integration Tests job took 531s. Its test step was 480s of wall time holding 856s of package time on a 4-vCPU runner. internal/database/postgres/migrations was the slowest package at 190s (152 tests), and TestMigrations_DownAllRoundTrip alone took 21s. Most of the package's time went to tests walking the 127 migrations one after another. Since #1400 each test has its own database (RequireEmptyDatabase / RequireMigratedDatabase), so most of them can run at once.

What changed

Three commits:

1. fix(testhelpers): hardens the shared test server for parallel callers. These are the three non-blocking findings from the review of test: share one Postgres server per package across integration tests #1400:
   • The migrated template was built under the first caller's context, and the result, errors included, is cached for the whole binary. A canceled or timed-out first caller made every later RequireMigratedDatabase call fail. The build now runs under context.WithoutCancel with its own 10-minute backstop, the same way start() already handles the container.
   • The Docker health probe ran on every call. Under load, a probe that times out would skip a test whose database is reachable, which is a #1597-style false green. It now runs only until the shared server is up (checked under the mutex).
   • dropDatabase read s.admin without the mutex while TerminateSharedPostgres sets it to nil. It now holds the mutex and treats a stopped server as nothing left to drop.
   • Three new tests in shared_integration_test.go, one per fix. Each fails on the old code: a cached template error, a nil-pointer panic in cleanup, and a probe on a running server.
2. test(testhelpers): raises max_connections on the shared server to 500. In the worst case each running test holds about 13 connections (a pool of up to 10, the migrator's connection, and a spare session), and the admin pool adds up to 10 more. At -parallel=16 that is over 200, past PostgreSQL's default of 100. I raised the server limit instead of shrinking the per-database pool because it is the less invasive change: every test's pool stays the same size, so no test that holds several connections can end up waiting on its own pool. Only the shared server gets the flag, through an unexported setupPostgresContainer(..., extra...). SetupPostgresContainer and dedicated containers are unchanged.
3. test(migrations): runs the integration tests in parallel.
   • 62 of 69 top-level tests call t.Parallel().
   • Table subtests are parallel only where each subtest creates its own database and the parent holds none. That covers 000064, 000065 (both tests), 000086, 000088, 000096, ensure_admin_user, savings_snapshots_pk and split_savingsplans. Subtests that share the parent's database stay sequential (000078, 000095, 000102, 000128, 000132).
   • Seven tests stay sequential, each with a one-line reason:
     • TestMigrations_AutoHealDirty: t.Setenv.
     • TestPGPasswordAndKeywordDSN_ConnectWithHostilePassword: t.Setenv. It also keeps its dedicated server because of ALTER ROLE.
     • TestAdminBootstrap_LogsToStderr_NotStdout: swaps os.Stdout and the standard logger.
     • The four deadline/cancel tests in migrate_cancel_integration_test.go: they assert wall-clock bounds.
   • The parallel tests that use advisory locks or pg_stat_activity only see their own database. Advisory locks are per database (I checked: the same key is lockable from two databases at once). The pg_stat_activity queries filter by PID or current_database().
   • The doc comments on captureStdout and captureLogOutput now say their callers must not be parallel.

No assertion was changed or removed.

Verification

All runs used the -race -tags integration test binary (go test -c -race -tags integration, then run from the package directory). The machine has 16 cores and was shared with other agents' container workloads (load average 7 to 17), so absolute times are noisy.

Run	Wall time	Result	Peak connections (all / client backends)
origin/main, sequential (-test.parallel default, nothing parallel)	252s	152 pass	n/a
this branch, -test.parallel=4 (CI-like)	111s (177s in an earlier run at load 15 to 17)	152 pass	19 / 14 (21 / 16 in the earlier run)
this branch, -test.parallel=16	81s	152 pass	50 / 45
-test.shuffle=on x3 (seeds 1792896703861082493, 1792896816583106797, 1792896954462828728)	113s / 138s / 140s (golangci-lint was running at the same time)	152 pass each	48 to 54
-test.count=2	165s	304 pass	51 / 46

• Peak connections came from sampling SELECT count(*) FROM pg_stat_activity every ~0.2s via docker exec on the shared container during each run.
• go test -race -tags integration -v ./internal/database/postgres/testhelpers/: all 16 tests pass, including the false-green guard tests and the three new ones.
• go test -race -short for testhelpers and migrations: pass.
• golangci-lint v2.10.1 on both packages: 0 issues. pre-commit on the changed files: pass. trivy config (docker, v0.69.3): exit 0. git-secrets scan and scripts/test-git-secrets-allowlist.sh: clean.

Not verified locally:

• Timing on the real 4-vCPU CI runner. There, packages also run side by side (go test -p 4), so CPU contention will be higher than in my -parallel=4 run.

Expected effect

• The migrations package should drop from about 190s toward roughly half on CI. Locally it fell from 252s to 111s at -parallel=4.
• The critical path is now TestMigrations_DownAllRoundTrip plus the sequential tests (AutoHealDirty, which runs the full chain four times, the PGPASSWORD test with its own container, and a few short ones).
• How much the job's wall time drops depends on how much CPU headroom the runner has left with four packages in flight.

Note for cristim

• The max_connections=500 flag only applies to the shared server and doesn't change any pool size. If you'd rather cap pools instead (for example, MaxConnections 4 for shared test databases), that's a one-line alternative, but it could make a test that holds several connections at once wait on its own pool.
• TestMigrations_AutoHealDirty stays sequential because of t.Setenv, and it is now one of the longer items on the critical path. A follow-up could start its subtests from RequireMigratedDatabase instead of migrating an empty database four times. I left it out here to keep this PR to parallelism only.

🤖 Generated with Claude Code

[coderabbitai]

Caution

Review failed

The pull request is closed.

Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}