test(check): probe --changed scope boundaries with named gate assertions#48
test(check): probe --changed scope boundaries with named gate assertions#480xjgv wants to merge 2 commits into
Conversation
Add two targeted e2e scenarios that prove the --changed contract in full: - each graph-wide gate (test, deps, attribution) is skipped with its explicit named message (not just the generic "skipped under --changed" fragment) - file-level gates (fix, typecheck) DO run on the scoped file set Extract _stage_combined() helper in the step defs to deduplicate the stdout+stderr combination shared by both output assertion steps.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces new BDD scenarios to verify the interlocks check --changed command and refactors test utilities by extracting a helper function for process output. Feedback includes recommendations to consolidate the new scenarios for better performance, expand assertions for file-level gates, and ensure the new helper function is used consistently throughout the test suite.
| Scenario: `interlocks check --changed` skips each graph-wide gate with a named reason | ||
| Given a minimal tmp project initialized as a git repo | ||
| And the tmp project has a changed Python file | ||
| When I run "interlocks check --changed=HEAD" in the tmp project | ||
| Then the stage exits 0 | ||
| And the stage output contains "test: skipped under --changed" | ||
| And the stage output contains "deps: skipped under --changed" | ||
| And the stage output contains "attribution: skipped under --changed" | ||
|
|
||
| # req: stage-check | ||
| Scenario: `interlocks check --changed` runs file-level gates on changed Python files | ||
| Given a minimal tmp project initialized as a git repo | ||
| And the tmp project has a changed Python file | ||
| When I run "interlocks check --changed=HEAD" in the tmp project | ||
| Then the stage exits 0 | ||
| And the stage output contains "[fix]" | ||
| And the stage output contains "[typecheck]" |
There was a problem hiding this comment.
These two new scenarios (and the existing one at line 64) perform identical setup and execution steps (interlocks check --changed=HEAD on a project with a changed file). Since these are E2E tests involving git initialization and subprocess execution, they are relatively expensive. Combining them into a single scenario with multiple assertions would improve test suite performance while maintaining the same level of coverage.
| And the stage output contains "[fix]" | ||
| And the stage output contains "[typecheck]" |
There was a problem hiding this comment.
To fully verify the contract for file-level gates under the --changed flag, consider adding assertions for [format] and [crap]. These tasks are also scoped to changed files in the check stage and should appear in the output alongside [fix] and [typecheck].
And the stage output contains "[fix]"
And the stage output contains "[format]"
And the stage output contains "[typecheck]"
And the stage output contains "[crap]"
| def _stage_combined(result: subprocess.CompletedProcess[str]) -> str: | ||
| return result.stdout + result.stderr |
There was a problem hiding this comment.
The PR description states that _stage_combined() was extracted to deduplicate logic shared by both output assertion steps. However, it is currently only used in _stage_output_contains. If the intention was to standardize output handling, consider also using it in _stage_exits (lines 134-138), although the current separate view in that function is admittedly better for debugging.
…rtions Merges two separate --changed scenarios that had identical setup into one, reducing redundant E2E subprocess + git-init overhead. Adds [format] and [crap] assertions to fully cover all four file-level gates under --changed scope.
1 participant
Summary
tests/features/interlock_stages.featurethat prove the--changedcontract explicitly:test: skipped under --changed,deps: skipped under --changed, andattribution: skipped under --changedeach appear individually (not just the generic fragment)[fix]and[typecheck]appear in output when a changed Python file is present_stage_combined()helper intests/step_defs/test_interlock_stages.pyto deduplicate thestdout + stderrcombination shared by both output assertion stepsTest plan
uv run pytest -q tests/step_defs/test_interlock_stages.py -k "changed"→ 8 passed (6 pre-existing + 2 new)uv run interlocks check --changed→ lint, format, typecheck all green