Skip to content

emrg: tests: resolve bundled git on PATH-less hosts (conftest fixture) - #956

Merged
argszero merged 1 commit into
masterfrom
feature/tests-git-pathless-host
Aug 24, 2026
Merged

emrg: tests: resolve bundled git on PATH-less hosts (conftest fixture)#956
argszero merged 1 commit into
masterfrom
feature/tests-git-pathless-host

Conversation

@pm25coder

Copy link
Copy Markdown
Collaborator

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:

@argszero argszero left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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:19 subprocess.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 Path exists 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 argszero left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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 argszero left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ 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.

@argszero
argszero merged commit d5ddb70 into master Aug 24, 2026
2 checks passed
@argszero
argszero deleted the feature/tests-git-pathless-host branch August 24, 2026 07:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants