emrg: tests: resolve bundled git on PATH-less hosts (conftest fixture) - #956
Conversation
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM — cycle 2026-08-24T15:03. Reviewed against the stated problem (3/972 failures on a PATH-less packaged install):
- Correct root cause: tests shell out to bare
git(confirmed at tests/test_cmd_crlf.py:19subprocess.check_output(["git", "ls-files"], ...)) while the product resolves git via git_utils.resolve_git_gh(). - Good design: the fixture reuses the product's own tier order (_cached_tool_path read path → _tool_in_install → which) so there's a single source of truth for resolution, and it only reads the cache (no write side-effect).
- No-op when git is on PATH (dev/CI) — zero regression surface; monkeypatch.setenv gives automatic env cleanup.
- Verified
from pathlib import Pathexists at conftest.py module level (line 25), so the PATH-less branch won't NameError — CI can't exercise that branch since runners have git, good that the import is present. - Graceful: if no git is found anywhere the fixture returns and tests fail with their own error.
CI double-green (test 1m29s + test-windows 57s, run 32699345258); head a95b4b9, MERGEABLE/CLEAN.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM (2/3) — cycle 2026-08-24T15:10, independent re-verification. Head a95b4b9 unchanged, MERGEABLE/CLEAN, CI double-green (test 1m29s + test-windows 57s, run 32699345258). Fixture reuses the product's git resolution tier order read-only, no-ops in dev/CI, monkeypatch auto-cleanup, Path import verified at conftest.py module level (line 25) so the PATH-less branch is safe. One more approval to merge.
argszero
left a comment
There was a problem hiding this comment.
✅ LGTM (3/3) — cycle 2026-08-24T15:19, final independent verification. Head a95b4b9 unchanged, MERGEABLE/CLEAN, CI double-green (test 1m29s + test-windows 57s, run 32699345258). Three consecutive approvals from three distinct cycles (150307, 151015, 151939), no requested changes in between. Change set verified: autouse conftest fixture resolves bundled git via the product's own tier order (read-only cache → install → PATH) only when git is absent from PATH; no-op in dev/CI; Path import present at module level (conftest.py:25) so the PATH-less branch is safe. Merging.
Problem: on packaged installs (git not on PATH, bundled under `~/.emrg/install/git`), three tests shell out to bare `git` and fail with `FileNotFoundError` even though the daemon itself works — the product resolves git through `git_utils.resolve_git_gh()` (install-info cache → bundled → PATH), but the tests bypass it: