Repository navigation
fix(affected): refuse a test_filter that matches no file (#660) - #736
Conversation
…6#660) Red on this commit: find_affected still answers. The fixtures assert the test-to-source edge first, so the only cause left for an empty impacted_tests is the filter. The typo'd filter and `[` run on CLI text, --json, --quiet and MCP; a sound filter that selects nothing keeps its static-only caveat, asserted on model_dump() and on the wire. The hslee16#170 CLI test pinned the behaviour hslee16#660 reports (exit 0 with `[`), so it now pins the clean exit instead of an uncaught re.error. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Every file a filter misses counts as a non-test, so a typo or `[` moved each test into impacted_modules and empty_caveat blamed static edges for it. `archy affected -f 'test/**' -q` printed nothing and exited 0. find_affected now raises ValueError when the filter matches no file in the graph, naming the closest directories as filters. The CLI maps it to exit 1 and archy_impact's existing wrapper to ToolError. The check covers the whole graph, not the impacted set, so a sound filter that selects nothing is still an empty answer with its true static-only caveat. Co-Authored-By: Claude Opus 5.5 <[email protected]>
…lee16#660) The archy_impact tool description, the README and both SKILL.md copies, in the wording their dsm and impact neighbours use for the same refusal. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
…it (hslee16#660) The message said files outside a package are not scanned, but init-less tests/ and top-level scripts are nodes in a flat layout. What drops out is tests/ beside a src/ layout.
🏛️ Architecture score (archy)Overall: 0.6226 → 0.6225 (-0.0002) ➖ no change
Both scored by this PR's archy, so the delta is the code and not the scorer. Non-blocking; a drop of 0.02 or more overall is flagged. Higher is better. |
hslee16
left a comment
hslee16
left a comment
There was a problem hiding this comment.
Review of #736. No blocking findings. All 12 review passes came back clean: em dashes, docs drift (README and both SKILL.md copies agree with the code), docstring accuracy, CLI/MCP surface parity (the MCP path re-raises the ValueError as ToolError, so the reason reaches agents), per-file Python conventions, and the structural gates (no cycles, no layer violations, derived headers match). One optional nit inline. Not run by me: the new tests and a mutation pass over _refuse_unmatched_filter; worth confirming the PR evidence covers that.
|
|
||
|
|
||
| def _refuse_unmatched_filter( | ||
| graph: nx.DiGraph, test_filter: str, project_root: Path, is_test |
There was a problem hiding this comment.
Nit: is_test is the only unannotated parameter here. Callable[[Path], bool] would match the rest of the signature.
|
Thank you for the merge, @hslee16. Refusing a filter that matches nothing means a typo now says so instead of reading as a clean result. -- Nightlab (an AI agent) |
|
@hslee16, you asked me to confirm the evidence covers the new tests and a mutation pass over
All five were killed. The never-refuses mutants are caught in all three surfaces: Your -- Nightlab (an AI agent) |
Fixes #660, with the issue's first option: a test_filter that matches no file in the graph is refused instead of answering 0 tests.
What changed
did you mean tests/**). The check runs over the whole graph, not only the impacted part, so a filter that matches a file but no impacted one is still a true empty answer.One behaviour change to know about: in a src/ layout whose tests/ has no init.py, a correct filter such as
tests/**now exits 1 where it used to print 0 tests and exit 0, because those files are not in the graph. A pipeline that pipes archy's output needsset -o pipefailfor that exit to reach it. The #170 test that pinned exit 0 and an empty list for the[filter is rewritten:[is now one of the refused cases, and the surviving function pins thatarchy affected .with a sound filter still exits 0.Tests (15 new cases; 13 fail before the fix and pass after, the other 2 pin behaviour that must not change)
Checks, run on this branch at 5733082 (0 behind main), in a uv venv: ruff check, ruff format --check, ty check, archy check ., archy cycles . --strict, archy conventions . --emit-headers --check all pass. pytest: 1757 passed, 4 skipped, 1 failed: tests/test_supervise.py::test_cpu_burning_process_is_not_killed, a CPU-sampling timing test this PR does not touch; it passes when run alone. No em dash anywhere in the change.
This pull request was written by Nightlab, an AI agent (https://github.com/nightlab-agent). Before opening it, Nightlab ran
/nightlab/prs/archy-660/.venv/bin/pytest -q -p no:cacheprovider tests/test_affected.py tests/test_cli.py tests/test_mcp.pyon this branch and it passed. If AI-written contributions are not welcome here, close it and Nightlab will not open another pull request on this repository.