test(hooks): cover scripts/install-dev-hooks.sh (Closes #17) - #18
Merged
Conversation
Add tests/test_install_dev_hooks.py: a pytest that drives the real installer against a throwaway, env-isolated git repo via subprocess. The installer logic is in bash (dispatcher-block splicing, idempotent re-install, .bak preservation, --git-common-dir resolution, exec-bit repair), so the tests exercise the actual script rather than mocking the shell away. Coverage: - fresh install lands the tracked hook (exec bit + byte-identical); - aborts when the tracked source is missing / when run outside a git repo; - idempotency: re-run leaves the file byte-identical, incl. a spliced dispatcher block (marker counted == 1, no double-splice); - dispatcher-block extraction: marker + `|| exit $?` -> spliced & preserved once, the result is valid executable bash (bash -n + end-to-end run); marker without terminator -> empty output, no tail splice, original kept in .bak; - .bak: a differing original is backed up; an existing .bak is left untouched; - linked-worktree resolution via --git-common-dir (hook lands in the common hooks dir, never the per-worktree private one); - exec-bit repair on the "already up to date" branch; - repo core.hooksPath: hook still resolves to the common dir + warns; - installed hook fails closed when ./bin/3d is absent, invokes it when present, and BLOCKS the commit (exit 1, gate marker on stderr) when the gate fails. Safety: every git call runs through _git_env() — pinned HOME / XDG_CONFIG_HOME / GIT_CONFIG_GLOBAL|SYSTEM|NOSYSTEM, GIT_CEILING_DIRECTORIES, and a scrub of every foreign GIT_* (incl. GIT_CONFIG_COUNT/KEY_*/VALUE_*). The tests never read or write the developer's real ~/.gitconfig or ~/.config/git/hooks; they operate only inside pytest's tmp_path. Verified: a hostile parent core.hooksPath injected via GIT_CONFIG_* does not leak into the test repo. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EQNpHiYWBHHQyMJP34BdHH
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 10d3c0d956
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Address the codex review on PR #18: a set GIT_TEMPLATE_DIR (or a global init.templateDir) would seed every `git init` with template files — including a hooks/pre-commit. That foreign hook would make the first install create a .bak (breaking test_reinstall_is_idempotent's "no backup" assertion) and could run during the linked-worktree seed commit. Two-layer fix in _git_env / the temp-repo setup: - pop GIT_TEMPLATE_DIR from the test git env (covers the env var); - pass `--template=` (empty) to every `git init` (covers a global init.templateDir, which the env-pop does not). Verified adversarially: with a hostile GIT_TEMPLATE_DIR carrying a template pre-commit in the parent env, the idempotency / fresh-install / linked-worktree tests still pass — the seeding is fully neutralized. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EQNpHiYWBHHQyMJP34BdHH
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.
Closes #17.
Adds
tests/test_install_dev_hooks.py— automated tests forscripts/install-dev-hooks.sh, which PR #16 shipped with only sandbox verification (the review flagged the missing tests).Approach
A pytest that drives the real installer against a throwaway, env-isolated git repo via
subprocess. The installer logic lives in bash (dispatcher splicing, idempotent re-install,.bakpreservation,--git-common-dirresolution, exec-bit repair), so the tests exercise the actual script rather than mocking the shell away — mocking would test nothing real.Coverage (every path from #17 + more)
== 1, no double-splice);|| exit $?→ spliced & preserved once, result is valid executable bash (bash -n+ an end-to-end run); marker without the terminator → empty output, no tail splice, original kept in.bak;.bak: a differing original is backed up; an existing.bakis left untouched (the true original is never lost);--git-common-dir(hook lands in the commonhooks/, never the per-worktree private dir);core.hooksPathset → hook still resolves to the common dir + warns;./bin/3dis absent, invokes it when present, and blocks the commit (exit 1, gate marker on stderr) when the gate fails.Safety (git-hook-installer isolation)
Every git call runs through
_git_env(): pinnedHOME/XDG_CONFIG_HOME/GIT_CONFIG_GLOBAL|SYSTEM|NOSYSTEM,GIT_CEILING_DIRECTORIES, and a scrub of every foreignGIT_*(incl.GIT_CONFIG_COUNT/KEY_*/VALUE_*). The tests never touch the developer's real~/.gitconfigor~/.config/git/hooks— they operate only inside pytest'stmp_path. Verified adversarially: a hostile parentcore.hooksPathinjected viaGIT_CONFIG_*does not leak into the test repo.3d test(ruff + pytest + mypy) green; 15 new tests pass. Reviewed viareview diff --staged(claude-opus-4-8) →[ok].🤖 Generated with Claude Code
https://claude.ai/code/session_01EQNpHiYWBHHQyMJP34BdHH