Fixture git commands see nothing of the developer's environment (#423) - #442
Merged
Merged
Conversation
What: tests/helpers/git-config-isolation.mjs becomes tests/helpers/git-environment-isolation.mjs (git mv; all nine importers and the test file follow). The side-effect import now applies isolateGitEnvironment(), which keeps the four configuration sources from #412/#421 and adds the rest of the class: GIT_TEMPLATE_DIR (deleted), the repository-location overrides GIT_DIR, GIT_WORK_TREE, GIT_INDEX_FILE, GIT_OBJECT_DIRECTORY, GIT_ALTERNATE_OBJECT_DIRECTORIES, GIT_COMMON_DIR, GIT_NAMESPACE, GIT_CEILING_DIRECTORIES (deleted), the programs git would launch GIT_PAGER, GIT_ASKPASS, GIT_SSH, GIT_SSH_COMMAND (deleted), and GIT_EDITOR / GIT_SEQUENCE_EDITOR = ":" with GIT_TERMINAL_PROMPT=0. isolateGitConfig and ABSENT_GIT_CONFIG keep their names. No core/ change. Why: the helper was named for configuration and covered exactly that, but GIT_TEMPLATE_DIR is not configuration and git copies it, hooks/ included, into every `git init` — so a developer's executable hooks/pre-commit ran inside every fixture commit under "full" isolation. The issue's Option 2: name the helper for what it guarantees so the next member of the class is easier to see, and sweep the class now. Class sweep (git 2.55, `git help git` ENVIRONMENT): neutralize: GIT_TEMPLATE_DIR (proven: failing pre-commit breaks fixture commit); GIT_DIR (proven: `git -C fixture init` + `remote add` land in the exported repo, fixture never gets a .git); GIT_INDEX_FILE (proven: `git add` writes the exported index, fixture status shows ??); GIT_EDITOR (proven: `commit --amend` runs the editor and fails with it); GIT_WORK_TREE / GIT_OBJECT_DIRECTORY / GIT_ALTERNATE_* / GIT_COMMON_DIR / GIT_NAMESPACE / GIT_CEILING_DIRECTORIES (same mechanism as GIT_DIR: redirect the repository a command acts on); GIT_SEQUENCE_EDITOR / GIT_PAGER / GIT_ASKPASS / GIT_SSH(_COMMAND) / GIT_TERMINAL_PROMPT (programs and prompts; fixtures never want one). leave: GIT_AUTHOR_* / GIT_COMMITTER_* (only change identity/date, no test asserts on them, CI sets them deliberately); GIT_TRACE*, GIT_FLUSH, GIT_ADVICE, GIT_PROGRESS_DELAY (stderr diagnostics, no outcome change, wanted when debugging); GIT_DEFAULT_HASH / GIT_DEFAULT_REF_FORMAT (on-disk format the developer chose for every new repo; suite should pass under it); GIT_EXEC_PATH / PATH / HOME (which git runs is out of scope); GIT_EXTERNAL_DIFF / GIT_DIFF_OPTS (porcelain-only; fixtures use plumbing flags). No test in tests/ sets any neutralized variable deliberately (grep). Tests (tests/git-environment-isolation.test.mjs, renamed with the helper because it exercises the whole boundary, not configuration): four new cases in the #421 shape — each first proves the vector is real under configuration-only isolation, then runs a fresh child with the variable injected and the helper imported first. The allow-list test widens from ^GIT_CONFIG to ^GIT_ and requires the new members. Against the main helper: 5/12 fail (6-9 and 12); the TEMPLATE_DIR control shows `template hook ran` on the fixture commit's stderr. Mutation: removing each neutralization line alone fails its own test plus the allow-list (6+12, 7+12, 8+12, 9+12); removing the location loop fails 7+8; applying isolateGitConfig() instead of isolateGitEnvironment() at import fails 6-9. No survivors. npm run build: 0 TS errors. npm test: 1276/1276 twice (was 1272). git diff --check clean. Developer's global git config: 7 entries before and after; the helper writes only process.env. Closes #423
…names its symptom (#442 review) Review found the allow-list test's variable scan ran over the whole source, comments included, so a helper that neutralized nothing passed as long as the names survived in a comment. It now scans the same comment-stripped code the command check already used; the reviewer's probe (every assignment commented out) fails test 12 as it should. The GIT_EDITOR control also asserts the editor was the failure.
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.
What: tests/helpers/git-config-isolation.mjs becomes
tests/helpers/git-environment-isolation.mjs (git mv; all nine importers
and the test file follow). The side-effect import now applies
isolateGitEnvironment(), which keeps the four configuration sources from
#412/#421 and adds the rest of the class: GIT_TEMPLATE_DIR (deleted),
the repository-location overrides GIT_DIR, GIT_WORK_TREE, GIT_INDEX_FILE,
GIT_OBJECT_DIRECTORY, GIT_ALTERNATE_OBJECT_DIRECTORIES, GIT_COMMON_DIR,
GIT_NAMESPACE, GIT_CEILING_DIRECTORIES (deleted), the programs git would
launch GIT_PAGER, GIT_ASKPASS, GIT_SSH, GIT_SSH_COMMAND (deleted), and
GIT_EDITOR / GIT_SEQUENCE_EDITOR = ":" with GIT_TERMINAL_PROMPT=0.
isolateGitConfig and ABSENT_GIT_CONFIG keep their names. No core/ change.
Why: the helper was named for configuration and covered exactly that,
but GIT_TEMPLATE_DIR is not configuration and git copies it, hooks/
included, into every
git init— so a developer's executablehooks/pre-commit ran inside every fixture commit under "full" isolation.
The issue's Option 2: name the helper for what it guarantees so the next
member of the class is easier to see, and sweep the class now.
Class sweep (git 2.55,
git help gitENVIRONMENT):neutralize: GIT_TEMPLATE_DIR (proven: failing pre-commit breaks fixture
commit); GIT_DIR (proven:
git -C fixture init+remote addlandin the exported repo, fixture never gets a .git); GIT_INDEX_FILE
(proven:
git addwrites the exported index, fixture status shows??); GIT_EDITOR (proven:
commit --amendruns the editor and failswith it); GIT_WORK_TREE / GIT_OBJECT_DIRECTORY / GIT_ALTERNATE_* /
GIT_COMMON_DIR / GIT_NAMESPACE / GIT_CEILING_DIRECTORIES (same
mechanism as GIT_DIR: redirect the repository a command acts on);
GIT_SEQUENCE_EDITOR / GIT_PAGER / GIT_ASKPASS / GIT_SSH(COMMAND) /
GIT_TERMINAL_PROMPT (programs and prompts; fixtures never want one).
leave: GIT_AUTHOR* / GIT_COMMITTER_* (only change identity/date, no
test asserts on them, CI sets them deliberately); GIT_TRACE*,
GIT_FLUSH, GIT_ADVICE, GIT_PROGRESS_DELAY (stderr diagnostics, no
outcome change, wanted when debugging); GIT_DEFAULT_HASH /
GIT_DEFAULT_REF_FORMAT (on-disk format the developer chose for every
new repo; suite should pass under it); GIT_EXEC_PATH / PATH / HOME
(which git runs is out of scope); GIT_EXTERNAL_DIFF / GIT_DIFF_OPTS
(porcelain-only; fixtures use plumbing flags). No test in tests/
sets any neutralized variable deliberately (grep).
Tests (tests/git-environment-isolation.test.mjs, renamed with the
helper because it exercises the whole boundary, not configuration):
four new cases in the #421 shape — each first proves the vector is real
under configuration-only isolation, then runs a fresh child with the
variable injected and the helper imported first. The allow-list test
widens from ^GIT_CONFIG to ^GIT_ and requires the new members. Against
the main helper: 5/12 fail (6-9 and 12); the TEMPLATE_DIR control shows
template hook ranon the fixture commit's stderr.Mutation: removing each neutralization line alone fails its own test
plus the allow-list (6+12, 7+12, 8+12, 9+12); removing the location
loop fails 7+8; applying isolateGitConfig() instead of
isolateGitEnvironment() at import fails 6-9. No survivors.
npm run build: 0 TS errors. npm test: 1276/1276 twice (was 1272).
git diff --check clean. Developer's global git config: 7 entries before
and after; the helper writes only process.env.
Closes #423