fix(tests): revert template-database cloning, restore per-file migrations - #101
Merged
Merged
Conversation
…ions #98 replaced per-file migrations with a template database that each test file cloned via CREATE DATABASE ... TEMPLATE. That broke CI: every integration file now times out in beforeAll at createTestDb(). Test Files 57 failed | 56 passed (113) Error: Hook timed out in 10000ms. 185| beforeAll(async () => { 186| ({ db, close } = await createTestDb()); CREATE DATABASE ... TEMPLATE takes a lock on the source database, so concurrent clones serialize rather than run in parallel. On the CI runner they block past the 10s hook timeout. The change was measured only on a 14-core dev machine, where the suite went 339s -> 21s. That speedup was real but local: the problem it solved — pool connections (14 workers x pg's default 10) overrunning max_connections=100 — never occurred on CI, which has fewer cores and ran the same suite in 27s beforehand. Restore tests/global-setup.ts and tests/integration/setup.ts to their pre-#98 state. The .stryker-tmp ESLint ignore from #98 is unrelated and stays; it takes `pnpm lint` from 1024 errors to 0. The local exhaustion is better addressed with a maxWorkers cap, which costs CI nothing. Not included here — this commit only restores green.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
mainis red. #98 broke it. Every integration file times out inbeforeAll:All 56 unit files pass; all 57 integration files fail. Run: https://github.com/KenTaniguchi-R/ledgr/actions/runs/33279632754
What went wrong
#98 replaced per-file migrations with a template database cloned via
CREATE DATABASE ... TEMPLATE. That statement takes a lock on the source database, so concurrent clones serialize instead of running in parallel — on the CI runner they block past the 10s hook timeout.The deeper mistake was the measurement. The change was validated only on a 14-core dev machine, where
pnpm testwent 339s → 21s. That speedup was real, but the problem it solved was local: 14 vitest workers × pg's default pool of 10 overranmax_connections=100. CI has fewer cores, never hit that limit, and ran the same suite in 27 seconds before the change. The PR claimed a CI benefit that was never measured on CI.What this does
Restores
tests/global-setup.tsandtests/integration/setup.tsto their pre-#98 state (from402a877).Keeps the
.stryker-tmpESLint ignore from #98 — unrelated to the failure, independently verified, takespnpm lintfrom 1024 errors to 0.Verification
pnpm install --frozen-lockfilepnpm typecheckpnpm lintHonest caveat: one of three local runs showed 3 failures that did not reproduce. That is the pre-existing flakiness of the original code returning along with it — this PR restores the prior behavior, warts included. It does not claim to improve on it.
Follow-up, not included here
The local connection exhaustion is real and worth fixing, but belongs in a
maxWorkerscap invitest.config.ts— local-only, costs CI nothing. Deliberately left out so this change does one thing: restore green.🤖 Generated with Claude Code