fix(#44): harden remaining fixture blocks against silent setup failure - #63
Merged
Conversation
Adds a fixture_repo() helper that runs git init + an initial commit in a throwaway dir and, on failure, reports via bad() naming the dir - instead of the bare ( ... ) 2>/dev/null a setup failure could silently vanish into, surfacing downstream as a bogus assertion failure against hooks/_lib.sh rather than "the fixture didn't build". Replaces the 14 remaining unguarded git-init-and-commit fixture blocks (the project-root fixture plus FRESH_A..N). The 3 worktree-specific chains already hardened in #43 are left as-is; they run additional commands (git worktree add) beyond what this helper covers. Closes #44 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011s2om7vED2R8ZohhkN11qe
Extends the hardening to FRESH_G/G2/G3, which stage and commit a pre-written .gitignore rather than an empty commit and so didn't fit fixture_repo() verbatim - adds fixture_repo_gitignore() for that shape. These three back the gitignore-nudge tests (12g/12g2/12g3), the exact class of assertion #42 documented as misreporting a silent fixture failure as a hooks/_lib.sh bug. Also hardens fixture_repo() itself against its own worst failure mode: under POSIX sh, `cd ""` is a no-op rather than an error, so an empty or missing $1 would silently git-init the caller's own cwd instead of reporting via bad(). Not reachable from any current call site (every caller passes a non-empty, freshly-mkdir'd path), but a helper whose entire purpose is turning silent failure loud should not have a silent failure mode of its own. Both gaps found by /review-pr on PR #63; verified live (empty-arg fired bad() with no cwd side effect; a stray probe commit created during verification was cleanly reset before this commit). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011s2om7vED2R8ZohhkN11qe
CI's apt-installed shellcheck flags the &&/|| guard added in fafc06e (SC2015: not if-then-else, the else branch can run when the first check is true) even though local shellcheck 0.11.0 (homebrew) doesn't. Rather than chase version skew, switch both guards to an explicit if/then - unambiguous either way and closer to this file's existing style (see present()/dir_present()/absent() above). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011s2om7vED2R8ZohhkN11qe
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.
Summary
fixture_repo()helper (tests/run.sh) that runsgit init+ an initial commit in a throwaway dir and, on failure, reports viabad()naming the dir - instead of the bare( ... ) 2>/dev/nulla setup failure could silently vanish into.git init && git commitfixture blocks (the project-root fixture plusFRESH_A..FRESH_N) with calls to the new helper.wt-linked,wt-old-linked,wt-ign-linked) are left as-is - they chain an additionalgit worktree addbeyond what this helper covers, and already have their own|| bad ...guard.Follow-up from #42/#43 review (two independent PR review agents flagged this same structural pattern on the remaining fixture blocks).
Test plan
sh tests/run.sh- all 167 assertions passshellcheck -s sh tests/run.sh- cleanfixture_repoagainst a nonexistent directory in isolation and confirmed it reports viabad()(FAIL=1) instead of silently succeeding/local-ci --strict- all checks passCloses #44
🤖 Generated with Claude Code
https://claude.ai/code/session_011s2om7vED2R8ZohhkN11qe