Repository navigation
Keep the first run marker tests off the host's free disk space - #160
Merged
Merged
Conversation
MatiasFernandez
marked this pull request as ready for review
October 20, 2026 17:35
ericwinger
self-requested a review
October 20, 2026 18:16
ericwinger
approved these changes
Oct 20, 2026
Stub the free-space and NFS checks that the real runSetup reaches through prepareFiles, as setup.test.ts already does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The marker tests resolve the pending OS step only after their runSetup assertion. When that assertion failed, the first-run work kept the setup lock (heldHere is module-level), and the next tests failed waiting on it with errors that named neither the cause nor the test that caused it. Releasing it in a finally keeps a failure in the test that has it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MatiasFernandez
force-pushed
the
mfernandez/fix-space-flakiness
branch
from
October 20, 2026 18:45
1e01885 to
3e7e1d9
Compare
MatiasFernandez
enabled auto-merge
October 20, 2026 18:47
Closed
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
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.
Three unit tests in
activation.test.tsfail on any machine with less than about 12 GB free: both cases of "the first run marker" (cancelled/failed) and "never asks about shared memory on first run for an external database". That is what failed CI on #148, hours aftermainpassed on the same code.The marker tests run the real
runSetup, which reachesprepareFilesand the realassertRoomForSetup. On a low-disk host that throwsDiskSpaceError,runSetupreturnsfailed, and thecancelledcase fails its assertion before it callsreleaseOs. That leaves the first-run setup lock held (heldHereinlock.tsis module-level), so the next two tests wait on it and time out withexpected +0 to be 1.Two changes:
prepareFilesmakes (assertRoomForSetupandassertDatabaseIsLocal) in the existing../databasemock, next toassertDatabaseMatchesEngineandcreateDatabase.setup.test.tsalready stubs both the same way. The NFS check reads the host's file system the same way the free-space check does: withTMPDIRon NFS, the marker tests failed for that reason too (reproduced in review).finally, so any future failure of thatrunSetupassertion stays in the test that has it. Without this the stub removes one trigger, but the next unrelated failure there would again show up as timeouts in tests that never touch it.The stubs leave
assertRoomForSetupwith no unit coverage, which was already true onmain: these tests ran the real check only by accident and asserted nothing about it. Tests for it, a sturdier release of the OS step (one that also holds on a timeout), and one shared inert../databasemock are tracked in #161.Verified on a Mac with 7.1 GB free. Before the change,
npx vitest run src/__tests__/activation.test.tsgave 3 failed and 32 passed, and the fullnpm testgave 3 failed and 1058 passed. After it, the file passes 35/35 andnpm testpasses 1061/1061. To check thefinally, I temporarily removed the free-space stub: only the two marker cases failed, each with the realexpected 'failed' to be 'completed', and the external-database test passed. Lint, format check and both typechecks are clean.🤖 Generated with Claude Code