From 757745a221807ce2a2d631b8fe5925489a6b4a1e Mon Sep 17 00:00:00 2001 From: dev-agent Date: Thu, 4 Jun 2026 16:05:52 +0300 Subject: [PATCH] fix(qg): gate testing->deploy on machine-readable test verdict, not substring (ET-013) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit check_tests_passed did "if PASS in content" over the whole 13-test-report.md body, so a report explicitly marked verdict: BLOCKED / status: blocked whose prose mentioned "23 passed" / "PASS" / "All checks passed" passed the gate. On ET-013 an unfinished feature (P1 AC-19 failed) reached Done. Now mirrors check_reviewer_verdict (S-5) and check_deploy_status: read ONLY the YAML frontmatter verdict:/status: fields. Positive tokens (PASS/PASSED/ READY-TO-DEPLOY/GREEN/APPROVED) -> True; negative tokens (BLOCKED/FAILED/...) are authoritative -> False; missing/empty/no-frontmatter/bad-YAML -> False with reason; file missing -> not found. Never raises. Positive token set derived from REAL enduro-trails reports ET-001..ET-014 (inconsistent: PASS, ready-to-deploy+status:PASSED, stage:ready-to-deploy+status:pass, PASS — ready-to-deploy). Validated: all 9 prior passing WIs stay True, ET-013 -> False. --- src/qg/checks.py | 72 +++++++++++++++++++++++++++++-- tests/test_qg.py | 107 ++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 165 insertions(+), 14 deletions(-) diff --git a/src/qg/checks.py b/src/qg/checks.py index 1f07c9d..5e651ce 100644 --- a/src/qg/checks.py +++ b/src/qg/checks.py @@ -138,7 +138,16 @@ def check_review_approved(repo: str, pr_number: int) -> tuple[bool, str]: def check_tests_passed(repo: str, work_item_id: str, branch: str | None = None) -> tuple[bool, str]: """ - Check if test report exists and contains PASS indicator. + Gate the testing -> deploy transition on the tester's MACHINE-READABLE verdict + in 13-test-report.md frontmatter, NOT on a naive substring search of the body. + + ET-013 fix: the previous implementation did `if "PASS" in content`, so a report + explicitly marked `verdict: BLOCKED` / `status: blocked` but whose prose mentioned + "23 passed" / "✅ PASS" / "All checks passed" was treated as a pass, and an + unfinished feature reached Done. This mirrors check_reviewer_verdict (S-5) and + check_deploy_status (БАГ 8): read ONLY the YAML frontmatter `verdict:` / `status:` + fields, never the body. + File: docs/work-items//13-test-report.md """ repo_path = _repo_path(repo, branch) @@ -150,12 +159,67 @@ def check_tests_passed(repo: str, work_item_id: str, branch: str | None = None) try: with open(report_path, "r") as f: content = f.read() - if "PASS" in content or "All tests passed" in content: - return True, "Test report indicates PASS" - return False, "Test report exists but no PASS indicator found" except OSError as e: return False, f"Error reading test report: {e}" + return _parse_tests_verdict(content) + + +# Positive / negative verdict tokens, derived from REAL tester reports in +# enduro-trails (ET-001..ET-014). The tester is inconsistent: most write +# `verdict: PASS`, but ET-006 used `verdict: ready-to-deploy` (with `status: PASSED`), +# ET-007 `verdict: PASS — ready-to-deploy`, ET-008 `verdict: stage:ready-to-deploy` +# (with `status: pass`). ET-013 (the bug) used `verdict: BLOCKED` / `status: blocked`. +# We therefore match known positive/negative TOKENS inside the normalized +# verdict/status fields, and treat a negative token as authoritative (a BLOCKED/FAILED +# report never passes, even if another field looks positive). +_TESTS_NEGATIVE_TOKENS = ("BLOCKED", "FAILED", "FAIL", "REQUEST_CHANGES", "REJECT", "RED") +_TESTS_POSITIVE_TOKENS = ("PASSED", "PASS", "READY-TO-DEPLOY", "READY_TO_DEPLOY", "GREEN", "APPROVED") + + +def _parse_tests_verdict(content: str) -> tuple[bool, str]: + """Map a 13-test-report.md body to a quality-gate verdict by reading ONLY the + machine-readable `verdict:` (and corroborating `status:`) YAML frontmatter fields. + + Rules: + - No frontmatter / bad YAML / neither field present -> (False, reason). + - A negative token (BLOCKED/FAILED/...) in verdict OR status -> (False) and is + authoritative (ET-013 main case: verdict BLOCKED wins over any prose PASS). + - Otherwise a positive token (PASS/PASSED/READY-TO-DEPLOY/...) in verdict OR + status -> (True). + - Anything else (unrecognized / empty verdict) -> (False, reason). + """ + import yaml + + if not content.startswith("---"): + return False, "No YAML frontmatter in test report (cannot read machine verdict)" + + parts = content.split("---", 2) + if len(parts) < 3: + return False, "Malformed YAML frontmatter in test report" + + try: + fm = yaml.safe_load(parts[1]) or {} + except yaml.YAMLError as e: + return False, f"Invalid YAML frontmatter in test report: {e}" + if not isinstance(fm, dict): + return False, "Malformed YAML frontmatter in test report (not a mapping)" + + verdict = str(fm.get("verdict", "") or "").upper().strip() + status = str(fm.get("status", "") or "").upper().strip() + + if not verdict and not status: + return False, "No machine-readable verdict/status in test report frontmatter" + + fields = f"{verdict} {status}" + for neg in _TESTS_NEGATIVE_TOKENS: + if neg in fields: + return False, f"Test verdict: {verdict or status} ({neg})" + for pos in _TESTS_POSITIVE_TOKENS: + if pos in fields: + return True, f"Test verdict: {verdict or status} (PASS)" + + return False, f"No recognized PASS verdict in frontmatter (verdict={verdict!r}, status={status!r})" def check_analysis_approved(repo: str, work_item_id: str, branch: str | None = None) -> tuple[bool, str]: diff --git a/tests/test_qg.py b/tests/test_qg.py index 1c0ef96..acc02fe 100644 --- a/tests/test_qg.py +++ b/tests/test_qg.py @@ -167,23 +167,110 @@ class TestCheckReviewApproved: class TestCheckTestsPassed: - def test_report_with_pass(self, setup_work_item_dir): - repo_dir = setup_work_item_dir - wi_dir = repo_dir / "docs" / "work-items" / "ET-001" - wi_dir.mkdir(parents=True) - (wi_dir / "13-test-report.md").write_text("# Test Report\n\nResult: PASS\n") + """ET-013 fix: testing -> deploy gate reads the tester's MACHINE-READABLE verdict + in 13-test-report.md frontmatter (verdict:/status:), NOT a substring of the body. + Mirrors check_reviewer_verdict / check_deploy_status. The old `if "PASS" in content` + let a `verdict: BLOCKED` report whose prose said "23 passed"/"✅ PASS" pass the gate, + shipping an unfinished feature to Done.""" + def _write(self, repo_dir, content, wi="ET-001"): + wi_dir = repo_dir / "docs" / "work-items" / wi + wi_dir.mkdir(parents=True) + (wi_dir / "13-test-report.md").write_text(content) + + def test_verdict_pass_passes(self, setup_work_item_dir): + # Most common real form (ET-001/002/005/009/011/012/014). + self._write( + setup_work_item_dir, + "---\ntype: test-report\nverdict: PASS\nstatus: pass\n---\n\n# Test Report\n", + ) + passed, reason = check_tests_passed("enduro-trails", "ET-001") + assert passed is True + assert "PASS" in reason + + def test_verdict_pass_ready_to_deploy_passes(self, setup_work_item_dir): + # ET-007 real form: "PASS — ready-to-deploy". + self._write( + setup_work_item_dir, + "---\nverdict: PASS — ready-to-deploy\nstatus: PASS\n---\n\nbody\n", + ) passed, reason = check_tests_passed("enduro-trails", "ET-001") assert passed is True - def test_report_without_pass(self, setup_work_item_dir): - repo_dir = setup_work_item_dir - wi_dir = repo_dir / "docs" / "work-items" / "ET-001" - wi_dir.mkdir(parents=True) - (wi_dir / "13-test-report.md").write_text("# Test Report\n\nResult: FAIL\n") + def test_verdict_ready_to_deploy_with_status_passed_passes(self, setup_work_item_dir): + # ET-006 real form: verdict has no PASS word, but status: PASSED. + self._write( + setup_work_item_dir, + "---\nverdict: ready-to-deploy\nstatus: PASSED\n---\n\nbody\n", + ) + passed, reason = check_tests_passed("enduro-trails", "ET-001") + assert passed is True + def test_verdict_stage_ready_to_deploy_with_status_pass_passes(self, setup_work_item_dir): + # ET-008 real form: verdict: stage:ready-to-deploy, status: pass. + self._write( + setup_work_item_dir, + "---\nverdict: stage:ready-to-deploy\nstatus: pass\n---\n\nbody\n", + ) + passed, reason = check_tests_passed("enduro-trails", "ET-001") + assert passed is True + + def test_blocked_verdict_with_pass_in_body_fails(self, setup_work_item_dir): + # THE ET-013 BUG: verdict BLOCKED but body is full of "PASS"/"passed". + self._write( + setup_work_item_dir, + "---\ntype: test-report\nstatus: blocked\nverdict: BLOCKED\n---\n\n" + "23 passed\n✅ PASS (часть AC-18)\nAll checks passed\n", + ) passed, reason = check_tests_passed("enduro-trails", "ET-001") assert passed is False + assert "BLOCKED" in reason + + def test_failed_verdict_fails(self, setup_work_item_dir): + self._write( + setup_work_item_dir, + "---\nverdict: FAILED\nstatus: failed\n---\n\nbody\n", + ) + passed, reason = check_tests_passed("enduro-trails", "ET-001") + assert passed is False + assert "FAILED" in reason + + def test_passed_count_in_body_but_blocked_verdict_fails(self, setup_work_item_dir): + # Body says "23 passed" but frontmatter verdict BLOCKED -> substring no longer fools. + self._write( + setup_work_item_dir, + "---\nverdict: BLOCKED\n---\n\nTests: 23 passed, 0 failed.\n", + ) + passed, reason = check_tests_passed("enduro-trails", "ET-001") + assert passed is False + + def test_no_frontmatter_fails(self, setup_work_item_dir): + # Old format / prose only -> no machine verdict -> fail. + self._write( + setup_work_item_dir, + "# Test Report\n\nResult: PASS\nAll tests passed.\n", + ) + passed, reason = check_tests_passed("enduro-trails", "ET-001") + assert passed is False + + def test_no_verdict_field_fails(self, setup_work_item_dir): + # Frontmatter present but neither verdict nor status -> fail. + self._write( + setup_work_item_dir, + "---\ntype: test-report\nversion: 1\n---\n\nResult: PASS\n", + ) + passed, reason = check_tests_passed("enduro-trails", "ET-001") + assert passed is False + + def test_invalid_yaml_fails_no_exception(self, setup_work_item_dir): + # Broken YAML frontmatter -> False with reason, never raises. + self._write( + setup_work_item_dir, + "---\nverdict: [unclosed\n : : :\n---\n\nbody PASS\n", + ) + passed, reason = check_tests_passed("enduro-trails", "ET-001") + assert passed is False + assert "YAML" in reason or "frontmatter" in reason.lower() def test_no_report(self, setup_work_item_dir): passed, reason = check_tests_passed("enduro-trails", "ET-001")