F-032: test(history): hermetic git-config isolation for history tests#31
F-032: test(history): hermetic git-config isolation for history tests#31Sephyi wants to merge 1 commit intodevelopmentfrom
Conversation
…nfig
The `analyze_repo_*` and `analyze_respects_sample_size` integration
tests in `tests/history.rs` invoke `git init`, `git add`, `git commit`
and `git config` via `std::process::Command`. Without explicit env
isolation, these invocations inherit the ambient shell environment —
meaning a developer (or CI runner) with `commit.gpgsign=true`, a
global `core.hooksPath`, a non-default `init.defaultBranch`, or
missing `user.email` could see false test failures that nobody else
can reproduce.
Introduce a `hermetic_git(dir)` helper that returns a pre-configured
`Command::new("git")` with:
- `GIT_CONFIG_NOSYSTEM=1` — ignore /etc/gitconfig
- `GIT_CONFIG_GLOBAL=/dev/null` — ignore ~/.gitconfig
- `HOME=<tempdir>` — prevent fallback reads from the ambient home
- `GIT_AUTHOR_*` / `GIT_COMMITTER_*` — guarantee commits succeed
regardless of config state
Route every git invocation in the four integration tests through the
helper. The pre-supplied env-var identity makes the old
`git config user.email` / `user.name` calls redundant, so they are
removed.
Closes audit entry F-032 from #3.
There was a problem hiding this comment.
Pull request overview
Improves the hermeticity of the HistoryService git-integration tests by ensuring git commands executed during tests do not pick up ambient user/system git configuration (e.g., GPG signing, missing identity).
Changes:
- Added a
hermetic_git()helper to centralize git command setup for tests (disables system/global config and supplies author/committer identity). - Updated history git-integration tests to use
hermetic_git()forinit,add, andcommitinvocations.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let mut cmd = Command::new("git"); | ||
| cmd.current_dir(dir) | ||
| .env("GIT_CONFIG_NOSYSTEM", "1") | ||
| .env("GIT_CONFIG_GLOBAL", "/dev/null") |
There was a problem hiding this comment.
GIT_CONFIG_GLOBAL is hardcoded to /dev/null, which will break these tests on Windows (and potentially other non-Unix environments). To keep the test hermetic and cross-platform, prefer pointing GIT_CONFIG_GLOBAL at an empty file created inside the tempdir (or conditionally use the platform null device), rather than relying on /dev/null.
| let mut cmd = Command::new("git"); | |
| cmd.current_dir(dir) | |
| .env("GIT_CONFIG_NOSYSTEM", "1") | |
| .env("GIT_CONFIG_GLOBAL", "/dev/null") | |
| let global_config = dir.join(".gitconfig-empty"); | |
| std::fs::OpenOptions::new() | |
| .create(true) | |
| .write(true) | |
| .open(&global_config) | |
| .unwrap(); | |
| let mut cmd = Command::new("git"); | |
| cmd.current_dir(dir) | |
| .env("GIT_CONFIG_NOSYSTEM", "1") | |
| .env("GIT_CONFIG_GLOBAL", &global_config) |
| hermetic_git(path).args(["init"]).output().unwrap(); | ||
|
|
There was a problem hiding this comment.
These git invocations unwrap Command::output() but never assert status.success(). If git returns a non-zero exit code (e.g., missing binary, permission issue, repo corruption), the test will continue and fail later with a less actionable assertion. Consider centralizing execution in the helper to check the exit status and include stderr/stdout in the failure message.
Summary
test(history): hermetic git-config isolation for history tests.
Audit context
Closes audit entry F-032 from #3.
Verification
cargo fmt --checkcargo clippy --all-targets --all-features -- -D warningscargo test --all-targets