test(cli): give each complete-journal invariant case its own test - #628
Merged
Makisuo merged 1 commit intoAug 25, 2026
Merged
Conversation
`retains a complete journal unless every implied durable invariant is exact` seeded a full archive on disk for each of seven scenarios inside a single `it`, so one 5s budget covered the sum of all seven. It timed out on a loaded CI runner at 5002.38ms — a 2ms margin — and took an unrelated PR red with it. Splitting the table into one test per scenario gives each case its own budget and removes the structural amplification: the seven cases cost ~7ms each locally, and a failure now names the invariant in the test name instead of relying on the assertion message to say which iteration blew up. No behaviour under test changes: the same seven mutations, the same expected rejections, the same retained-journal assertion. Each case now gets its own temp archive rather than a shared parent, which is also better isolation.
🍁 Maple PR previewWarning Preview cleanup could not be confirmed. The Alchemy teardown outcome was Final commit |
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.
Stacked on #626.
Why
retains a complete journal unless every implied durable invariant is exactinapps/cli/test/archive-generation.test.tslooped a table of seven scenarios inside a singleit. Each case seeds a whole archive on disk — manifest, shard, pointer, catalog, pin, scratch, building — so one 5s budget had to cover the sum of all seven.It timed out on a loaded CI runner at 5002.38ms, a 2ms margin, and took #626 red with it. That PR touches nothing in
apps/cli; a re-run of the same commit went green, which is what identified this as structural rather than a real failure.What changed
The scenario table is hoisted to
COMPLETE_JOURNAL_CASESand emitted as oneitper case.retains a complete journal when the pin invariant is inexactinstead of one test name plus an assertion message identifying the iteration.withArchiverather than sharing one parent directory across all seven.Nothing under test changes: same seven mutations, same expected rejection patterns, same retained-journal assertion.
Note on scope
This removes the amplification — seven cases sharing one budget — which is the part that made a marginal runner stall fatal. It is not a claim that the underlying CI slowness is explained: the aggregate is ~48ms locally against 5002ms on CI, and I did not chase that gap. I deliberately did not raise the timeout, since an explicit bump would mask a genuine hang rather than fix the shape of the test.
Verification
bun run --cwd apps/cli test— 517 pass, 0 fail (was 511; +7 cases, −1 combined)bun run --cwd apps/cli typecheck,bun run lint,oxfmt --list-differentclean🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.