fix(verify): tests check runs the target project's own interpreter - #105
Open
kylewelsby wants to merge 1 commit into
Open
fix(verify): tests check runs the target project's own interpreter#105kylewelsby wants to merge 1 commit into
kylewelsby wants to merge 1 commit into
Conversation
_run_impacted_pytest always shelled out with sys.executable, which is roam's own interpreter when installed isolated from any one project (uv tool install, pipx — the documented install path). That interpreter has neither pytest nor the target project's dependencies, so collection fails before any test runs: "pytest exited 1 without a parsed passing result", hard_block: true, on every project whose dependencies roam's own venv doesn't happen to already have. Resolve the interpreter from a .venv/venv found by walking up from the first impacted test file instead, falling back to sys.executable unchanged when no project venv is found. Walking from the test file rather than root also picks the right interpreter for a monorepo subproject nested below the indexed root. Same bug class already fixed for the pre-push hook (this release, "ran whatever python PATH resolved to, not the project's interpreter") — this closes the same gap in `roam verify`'s own tests check.
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.
Summary
_run_impacted_pytest(thetestscheck insideroam verify) always shelled out withsys.executable. Under the documenteduv tool install/ pipx install path, that's roam's own isolated interpreter — it has neitherpytestnor the target project's dependencies installed, so collection fails before any test runs, and the gate reports it as a hard-blocking failure:pytest exited 1 without a parsed passing result.pythonPATH resolved to, not the project's interpreter") — this closes the equivalent gap inverify's owntestscheck, which wasn't touched by that fix.Fix
New
_resolve_test_interpreter()walks up from the first impacted test file (not justroot) looking for a.venv/venv, and falls back tosys.executableunchanged when none is found — so today's behaviour is preserved whenever roam is already running inside the project's own venv. Anchoring on the test file rather thanrootalso resolves correctly for a monorepo subproject nested below the indexed root (e.g. a Python service in an otherwise non-Python repo), where the project-root directory itself has no venv at all.Test plan
tests/test_verify_autofire_hardening.py: prefers the project's own root venv, prefers a venv nested under the target file over a root-level one, falls back tosys.executablewhen no project venv exists.uv run pytest tests/test_verify_autofire_hardening.py -v— 18/18 pass.uv run pytest tests/test_verify*.py -p no:randomly— 256/256 pass, run twice for determinism. (One flake surfaced under random ordering; reproduced identically on unpatchedmain, so it's pre-existingpytest-randomly+ shared index-state flakiness, unrelated to this change.)uv run ruff check/uv run ruff format --diffon both changed files — clean..venv) before writing the fix, and confirmed the patched build resolves and runs the correct interpreter there.Searched issues, PRs and discussions on this repo first — found no prior report of this.