test: fix ~3% flake in resetGitWorktreeSandbox worktree-listing assertion - #6099
Merged
atomantic merged 1 commit intoSep 3, 2026
Merged
Conversation
…tion (atomantic#6059) The test asserted `.not.toContain('wt-a'/'wt-b')` against the raw `git worktree list --porcelain` output, which always includes the parent sandbox's own entry. When that entry's random mkdtemp() suffix happens to start with 'a' or 'b', the substring can coincidentally appear inside the parent's own path and the assertion false-fails (~3.2% chance per prefix). Replaced it with a parse of the porcelain output into worktree entries (mirroring the parsing resetGitWorktreeSandbox itself already does in gitTestRepo.js, which drops the first entry as always being the parent) and assert exactly 1 entry remains. This avoids the collision and also sidesteps comparing full paths directly, which git can respell on Windows (see the assertPath test in this same file, atomantic#6003). Verified by forcing the exact collision deterministically (parent sandbox created at a path ending in `wt-b-XXXXXX`): confirmed the old assertion fails in that case and the new one does not. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
atomantic
approved these changes
Sep 3, 2026
atomantic
left a comment
Owner
There was a problem hiding this comment.
✅ Approved
This is a test-only fix for a real ~3% flake in resetGitWorktreeSandbox's worktree-listing assertion, replacing a substring match that can collide with the parent sandbox's own random directory suffix with a parse of porcelain entries that mirrors logic already used in production. I could not run vitest in this sandbox (no node_modules and no network access to install), so I independently reproduced the exact git worktree sequence the test exercises using real git and confirmed the new assertion holds and the change introduces no regression.
Scope: single test file change in server/lib/gitTestRepo.test.js, replacing two substring assertions with a parsed worktree-entry count assertion
Test evidence
- 🚧
cd server && NODE_ENV=test npx vitest run lib/gitTestRepo.test.js— server/node_modules is not installed in this disposable worktree and the sandbox denies network access to the npm registry (403/deny on registry.npmjs.org), so vitest could not be installed or run. This is an environment limitation, not a patch problem. - ✅
manual git reproduction of the exact worktree add/lock/remove/prune sequence the test performs, using real git (no vitest/node_modules needed)— Created a real git repo, added worktrees on branches wt-a and wt-b, locked wt-b, then reproduced resetGitWorktreeSandbox's own removal logic (unlock, remove --force, prune) from server/lib/gitTestRepo.js:242-262. Post-removalgit worktree list --porcelaincontained exactly oneworktreeline (the parent repo itself), matching the patched assertion's expect(worktreeEntries).toHaveLength(1). Also confirmed resetGitSandbox's branch cleanup (checkout main + branch -D for all non-main branches, gitTestRepo.js:209-213) removes wt-a/wt-b afterward, so the test's later branch and HEAD assertions a
Claims verified against the code
- server/lib/gitTestRepo.test.js:191-193 — the patched assertion parses
worktreelines from the porcelain listing and asserts exactly 1 remains, which I confirmed empirically against real git worktree removal output - server/lib/gitTestRepo.js:246-262 — resetGitWorktreeSandbox's own porcelain-parsing logic treats the first entry as the parent repo and removes the rest; the patched test's approach (count worktree entries) mirrors this existing, already-relied-upon production parsing rather than introducing new logic
- server/lib/gitTestRepo.test.js:170-177 — the parent sandbox directory
destis created via mkdtemp with prefix 'portos-git-fx-reset-wt-', so a random suffix starting with 'a' or 'b' produces a path containing the literal substring 'wt-a' or 'wt-b', confirming the described flake mechanism in the old assertion was real, not hypothetical
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.
Summary
resetGitWorktreeSandboxtest asserted.not.toContain('wt-a'/'wt-b')against the rawgit worktree list --porcelainoutput, which always includes the parent sandbox's own entry. When that entry's randommkdtemp()suffix happens to start with the same letter as a child worktree's differentiator, the substring coincidentally appears inside the parent's own path and the assertion false-fails (~3.2% chance per prefix).resetGitWorktreeSandboxitself already does ingitTestRepo.js(which drops the first entry as always being the parent) — and asserts exactly 1 entry remains. This sidesteps both the collision and comparing full paths directly, which git can respell on Windows (see theassertPathtest in this same file, CI: speed up Windows git-sandbox suites (primaryCheckoutGuard, worktreeReap) #6003).Test plan
cd server && npx vitest run lib/gitTestRepo.test.js→ 10/10 passed.wt-b-XXXXXX): confirmed the old substring assertion fails in that scenario and the new entry-count assertion does not — proof the fix removes the flake rather than passing by luck.🤖 Generated with Claude Code