Skip to content

Commit e2146c5

Browse files
nhortonclaude
andcommitted
fix: update stale requirement IDs, add missing tests for JOBS-REQ-004.5.3/4
- Renumber traceability comments (.5.4→.5.5, .5.6→.5.7, .5.7→.5.8) to match spec renumbering - Fix traceability comment format on new test to match project convention - Add test for JOBS-REQ-004.5.3: dynamic rules match all outputs regardless of git status - Add test for JOBS-REQ-004.5.4: get_changed_files failure skips .deepreview matching but dynamic rules still run Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 4627a2c commit e2146c5

1 file changed

Lines changed: 132 additions & 4 deletions

File tree

tests/unit/jobs/mcp/test_quality_gate.py

Lines changed: 132 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -518,7 +518,7 @@ def test_returns_feedback_when_json_schema_fails(self, tmp_path: Path) -> None:
518518
assert "JSON schema validation failed" in result
519519
assert "finished_step" in result
520520

521-
# THIS TEST VALIDATES A HARD REQUIREMENT (JOBS-REQ-004.1.3, JOBS-REQ-004.5.7, JOBS-REQ-004.6.1).
521+
# THIS TEST VALIDATES A HARD REQUIREMENT (JOBS-REQ-004.1.3, JOBS-REQ-004.5.8, JOBS-REQ-004.6.1).
522522
# YOU MUST NOT MODIFY THIS TEST UNLESS THE REQUIREMENT CHANGES
523523
def test_returns_review_instructions_when_reviews_exist(self, tmp_path: Path) -> None:
524524
"""When dynamic rules produce tasks, review instructions are returned."""
@@ -573,7 +573,7 @@ def test_returns_review_instructions_when_reviews_exist(self, tmp_path: Path) ->
573573
assert "Quality reviews are required" in result
574574
assert "step_write_output_report" in result
575575

576-
# THIS TEST VALIDATES A HARD REQUIREMENT (JOBS-REQ-004.5.6).
576+
# THIS TEST VALIDATES A HARD REQUIREMENT (JOBS-REQ-004.5.7).
577577
# YOU MUST NOT MODIFY THIS TEST UNLESS THE REQUIREMENT CHANGES
578578
def test_returns_none_when_all_reviews_already_passed(self, tmp_path: Path) -> None:
579579
"""If write_instruction_files returns empty (all .passed), result is None."""
@@ -616,7 +616,7 @@ def test_returns_none_when_all_reviews_already_passed(self, tmp_path: Path) -> N
616616

617617
assert result is None
618618

619-
# THIS TEST VALIDATES A HARD REQUIREMENT (JOBS-REQ-004.5.4).
619+
# THIS TEST VALIDATES A HARD REQUIREMENT (JOBS-REQ-004.5.5).
620620
# YOU MUST NOT MODIFY THIS TEST UNLESS THE REQUIREMENT CHANGES
621621
def test_merges_deepreview_and_dynamic_tasks(self, tmp_path: Path) -> None:
622622
"""Both .deepreview rules and dynamic rules are processed together."""
@@ -701,7 +701,8 @@ def test_merges_deepreview_and_dynamic_tasks(self, tmp_path: Path) -> None:
701701
assert all_tasks[1].rule_name == "external_rule"
702702
assert result is not None
703703

704-
# Validates JOBS-REQ-004.5.2 (deepreview rules matched only against git-changed output files)
704+
# THIS TEST VALIDATES A HARD REQUIREMENT (JOBS-REQ-004.5.2).
705+
# YOU MUST NOT MODIFY THIS TEST UNLESS THE REQUIREMENT CHANGES
705706
def test_deepreview_rules_skip_unchanged_output_files(self, tmp_path: Path) -> None:
706707
"""Deepreview rules should only match output files that are actually changed in git."""
707708
arg = StepArgument(name="refs", description="Reference files", type="file_path")
@@ -754,6 +755,133 @@ def test_deepreview_rules_skip_unchanged_output_files(self, tmp_path: Path) -> N
754755
assert mock_match.call_count == 0
755756
assert result is None
756757

758+
# THIS TEST VALIDATES A HARD REQUIREMENT (JOBS-REQ-004.5.3).
759+
# YOU MUST NOT MODIFY THIS TEST UNLESS THE REQUIREMENT CHANGES
760+
def test_dynamic_rules_match_all_outputs_regardless_of_git(self, tmp_path: Path) -> None:
761+
"""Dynamic rules run against all output files even when git says nothing changed."""
762+
review = ReviewBlock(strategy="individual", instructions="Check it")
763+
arg = StepArgument(name="report", description="Report", type="file_path")
764+
output_ref = StepOutputRef(argument_name="report", required=True, review=review)
765+
step = WorkflowStep(name="write", outputs={"report": output_ref})
766+
job, workflow = _make_job(tmp_path, [arg], step)
767+
768+
dynamic_task = ReviewTask(
769+
rule_name="step_write_output_report",
770+
files_to_review=["report.md"],
771+
instructions="Check it",
772+
agent_name=None,
773+
)
774+
instruction_path = tmp_path / ".deepwork" / "tmp" / "instr.md"
775+
instruction_path.parent.mkdir(parents=True, exist_ok=True)
776+
instruction_path.write_text("content")
777+
778+
with (
779+
patch(
780+
"deepwork.jobs.mcp.quality_gate.load_all_rules",
781+
return_value=([], []),
782+
),
783+
patch(
784+
"deepwork.jobs.mcp.quality_gate.match_files_to_rules",
785+
return_value=[dynamic_task],
786+
) as mock_match,
787+
patch(
788+
"deepwork.jobs.mcp.quality_gate.write_instruction_files",
789+
return_value=[(dynamic_task, instruction_path)],
790+
),
791+
patch(
792+
"deepwork.jobs.mcp.quality_gate.format_for_claude",
793+
return_value="formatted",
794+
),
795+
):
796+
result = run_quality_gate(
797+
step=step,
798+
job=job,
799+
workflow=workflow,
800+
outputs={"report": "report.md"},
801+
input_values={},
802+
work_summary=None,
803+
project_root=tmp_path,
804+
)
805+
806+
# Dynamic rules matched even though no deepreview rules exist and
807+
# get_changed_files was never called (no deepreview rules to trigger it)
808+
mock_match.assert_called_once()
809+
assert result is not None
810+
811+
# THIS TEST VALIDATES A HARD REQUIREMENT (JOBS-REQ-004.5.4).
812+
# YOU MUST NOT MODIFY THIS TEST UNLESS THE REQUIREMENT CHANGES
813+
def test_deepreview_skipped_when_get_changed_files_fails(self, tmp_path: Path) -> None:
814+
"""If get_changed_files() fails, .deepreview matching is skipped; dynamic rules unaffected."""
815+
review = ReviewBlock(strategy="individual", instructions="Check it")
816+
arg = StepArgument(name="report", description="Report", type="file_path")
817+
output_ref = StepOutputRef(argument_name="report", required=True, review=review)
818+
step = WorkflowStep(name="write", outputs={"report": output_ref})
819+
job, workflow = _make_job(tmp_path, [arg], step)
820+
821+
deepreview_rule = ReviewRule(
822+
name="lint",
823+
description="Lint",
824+
include_patterns=["*.md"],
825+
exclude_patterns=[],
826+
strategy="individual",
827+
instructions="Lint it",
828+
agent=None,
829+
all_changed_filenames=False,
830+
unchanged_matching_files=False,
831+
precomputed_info_bash_command=None,
832+
source_dir=tmp_path,
833+
source_file=tmp_path / ".deepreview",
834+
source_line=1,
835+
)
836+
837+
dynamic_task = ReviewTask(
838+
rule_name="step_write_output_report",
839+
files_to_review=["report.md"],
840+
instructions="Check it",
841+
agent_name=None,
842+
)
843+
instruction_path = tmp_path / ".deepwork" / "tmp" / "instr.md"
844+
instruction_path.parent.mkdir(parents=True, exist_ok=True)
845+
instruction_path.write_text("content")
846+
847+
from deepwork.review.matcher import GitDiffError
848+
849+
with (
850+
patch(
851+
"deepwork.jobs.mcp.quality_gate.load_all_rules",
852+
return_value=([deepreview_rule], []),
853+
),
854+
patch(
855+
"deepwork.jobs.mcp.quality_gate.get_changed_files",
856+
side_effect=GitDiffError("git not available"),
857+
),
858+
patch(
859+
"deepwork.jobs.mcp.quality_gate.match_files_to_rules",
860+
return_value=[dynamic_task],
861+
) as mock_match,
862+
patch(
863+
"deepwork.jobs.mcp.quality_gate.write_instruction_files",
864+
return_value=[(dynamic_task, instruction_path)],
865+
),
866+
patch(
867+
"deepwork.jobs.mcp.quality_gate.format_for_claude",
868+
return_value="formatted",
869+
),
870+
):
871+
result = run_quality_gate(
872+
step=step,
873+
job=job,
874+
workflow=workflow,
875+
outputs={"report": "report.md"},
876+
input_values={},
877+
work_summary=None,
878+
project_root=tmp_path,
879+
)
880+
881+
# match_files_to_rules called only once (for dynamic rules, not deepreview)
882+
mock_match.assert_called_once()
883+
assert result is not None
884+
757885

758886
# ---------------------------------------------------------------------------
759887
# TestValidateJsonSchemas — additional coverage

0 commit comments

Comments
 (0)