test(acceptance): probe require_acceptance, run_acceptance_in_check, and stage-enforcement semantics#55
Open
0xjgv wants to merge 2 commits into
Open
test(acceptance): probe require_acceptance, run_acceptance_in_check, and stage-enforcement semantics#550xjgv wants to merge 2 commits into
0xjgv wants to merge 2 commits into
Conversation
…and stage-enforcement semantics - tests/test_acceptance_status.py: add 6 explicit is_required_failure tests covering MISSING_FEATURES_DIR, MISSING_FEATURE_FILES, MISSING_SCENARIOS (true) and OPTIONAL_MISSING, DISABLED, RUNNABLE (false) - tests/tasks/test_acceptance.py: add test for cmd_acceptance with DISABLED runner - tests/features/interlock_stages.feature: 3 new scenarios — check skips enforcement when run_acceptance_in_check=false, check gates when both flags true, ci gates when require_acceptance=true and features dir is absent - tests/step_defs/test_interlock_stages.py: step defs for the new scenarios with a shared _make_acceptance_project helper to avoid pyproject read/append/write duplication
![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 enhances the test suite for acceptance enforcement logic, adding Gherkin scenarios for the check and ci commands, as well as unit tests for the is_required_failure property and disabled runner configurations. Feedback indicates that the new tests for is_required_failure should be expanded to include the MISSING_BEHAVIOR_COVERAGE status to ensure full coverage of the property's logic.
Comment on lines
+230
to
+278
| def test_is_required_failure_true_for_missing_features_dir(tmp_path: Path) -> None: | ||
| cfg = _cfg(tmp_path, features_dir=None, require_acceptance=True) | ||
| classification = classify_acceptance_with_details(cfg) | ||
| assert classification.status is AcceptanceStatus.MISSING_FEATURES_DIR | ||
| assert classification.is_required_failure is True | ||
|
|
||
|
|
||
| def test_is_required_failure_true_for_missing_feature_files(tmp_path: Path) -> None: | ||
| features = tmp_path / "tests" / "features" | ||
| features.mkdir(parents=True) | ||
| cfg = _cfg(tmp_path, features_dir=features, require_acceptance=True) | ||
| classification = classify_acceptance_with_details(cfg) | ||
| assert classification.status is AcceptanceStatus.MISSING_FEATURE_FILES | ||
| assert classification.is_required_failure is True | ||
|
|
||
|
|
||
| def test_is_required_failure_true_for_missing_scenarios(tmp_path: Path) -> None: | ||
| features = tmp_path / "tests" / "features" | ||
| _write_feature(features / "stub.feature", "Feature: stub\n # no scenarios\n") | ||
| cfg = _cfg(tmp_path, features_dir=features, require_acceptance=True) | ||
| classification = classify_acceptance_with_details(cfg) | ||
| assert classification.status is AcceptanceStatus.MISSING_SCENARIOS | ||
| assert classification.is_required_failure is True | ||
|
|
||
|
|
||
| def test_is_required_failure_false_for_optional_missing(tmp_path: Path) -> None: | ||
| cfg = _cfg(tmp_path, features_dir=None, require_acceptance=False) | ||
| classification = classify_acceptance_with_details(cfg) | ||
| assert classification.status is AcceptanceStatus.OPTIONAL_MISSING | ||
| assert classification.is_required_failure is False | ||
|
|
||
|
|
||
| def test_is_required_failure_false_for_disabled(tmp_path: Path) -> None: | ||
| cfg = _cfg(tmp_path, acceptance_runner="off", features_dir=None) | ||
| classification = classify_acceptance_with_details(cfg) | ||
| assert classification.status is AcceptanceStatus.DISABLED | ||
| assert classification.is_required_failure is False | ||
|
|
||
|
|
||
| def test_is_required_failure_false_for_runnable(tmp_path: Path) -> None: | ||
| features = tmp_path / "tests" / "features" | ||
| _write_feature( | ||
| features / "ok.feature", | ||
| "Feature: ok\n Scenario: a thing works\n Given precondition\n", | ||
| ) | ||
| cfg = _cfg(tmp_path, features_dir=features, require_acceptance=False) | ||
| classification = classify_acceptance_with_details(cfg) | ||
| assert classification.status is AcceptanceStatus.RUNNABLE | ||
| assert classification.is_required_failure is False |
There was a problem hiding this comment.
The new test suite for is_required_failure covers most statuses but omits AcceptanceStatus.MISSING_BEHAVIOR_COVERAGE. Since this status is also defined as a required failure in interlocks/acceptance_status.py (line 53), it should be included to ensure complete coverage of the property's logic. While setting up this state is more involved than the others, it is important for verifying the enforcement semantics mentioned in the PR title.
…COVERAGE Completes the is_required_failure test matrix by covering the MISSING_BEHAVIOR_COVERAGE status, which was flagged as missing in the Gemini code review. Uses the same project-name trick as the existing classify test: naming the project "interlocks" activates INTERLOCKS_REGISTRY so any feature file without `# req:` markers triggers incomplete coverage.
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
1 participant
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
tests/test_acceptance_status.py: 6 new tests explicitly coveringAcceptanceClassification.is_required_failurefor all relevant statuses:MISSING_FEATURES_DIR,MISSING_FEATURE_FILES,MISSING_SCENARIOS→True;OPTIONAL_MISSING,DISABLED,RUNNABLE→Falsetests/tasks/test_acceptance.py: 1 new test forcmd_acceptancewhenacceptance_runner = 'off'(DISABLED path was untested)tests/features/interlock_stages.feature: 3 new Gherkin scenarios for stage-level enforcement semantics — (1)checkexits 0 whenrun_acceptance_in_check=falseeven ifrequire_acceptance=true; (2)checkexits 1 + containsinit-acceptancewhen both flags are true and features dir absent; (3)ciexits 1 + containsinit-acceptancewhenrequire_acceptance=trueand features dir absenttests/step_defs/test_interlock_stages.py: step defs for the new scenarios; extracted_make_acceptance_project()helper to avoid the read/append/write pyproject duplication across the three new fixturesNo production code changes — all 60 target tests pass, pre-commit clean.
Test plan
uv run pytest -q tests/test_acceptance_status.py tests/tasks/test_acceptance.py tests/step_defs/test_interlock_stages.py— 60 passeduv run interlocks check --changed— clean (typecheck ok, pre-commit hook passed)