fix(qg): gate testing->deploy on machine-readable test verdict, not substring (ET-013)
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.
This commit is contained in:
@@ -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/<work_item_id>/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]:
|
||||
|
||||
Reference in New Issue
Block a user