revert(ORCH-017): drop shared check_tests_passed gate change — moved to ORCH-47 (own ADR); keep only approve-ping links
This commit is contained in:
@@ -179,24 +179,15 @@ _TESTS_POSITIVE_TOKENS = ("PASSED", "PASS", "READY-TO-DEPLOY", "READY_TO_DEPLOY"
|
||||
|
||||
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:` / `status:` / `result:` YAML frontmatter fields.
|
||||
|
||||
ORCH-017: the tester agent prompt (`.openclaw/agents/tester*`) documents
|
||||
`result: PASS | FAIL` as THE machine-readable field, but this gate previously
|
||||
read only `verdict:`/`status:`. A tester that followed the documented contract
|
||||
literally (e.g. ORCH-017's report: `result: PASS`, no verdict:/status:) was
|
||||
bounced back to development with a misleading "Tests FAILED". We now treat
|
||||
`result:` as a first-class machine field alongside verdict/status so the gate
|
||||
matches the contract the tester is actually told to emit. (ORCH-016 only passed
|
||||
before because its report redundantly carried both `verdict:` AND `result:`.)
|
||||
machine-readable `verdict:` (and corroborating `status:`) YAML frontmatter fields.
|
||||
|
||||
Rules:
|
||||
- No frontmatter / bad YAML / none of the three fields present -> (False, reason).
|
||||
- A negative token (BLOCKED/FAILED/...) in any field -> (False) and is
|
||||
- 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 any field
|
||||
-> (True).
|
||||
- Anything else (unrecognized / empty fields) -> (False, reason).
|
||||
- Otherwise a positive token (PASS/PASSED/READY-TO-DEPLOY/...) in verdict OR
|
||||
status -> (True).
|
||||
- Anything else (unrecognized / empty verdict) -> (False, reason).
|
||||
"""
|
||||
import yaml
|
||||
|
||||
@@ -216,24 +207,19 @@ def _parse_tests_verdict(content: str) -> tuple[bool, str]:
|
||||
|
||||
verdict = str(fm.get("verdict", "") or "").upper().strip()
|
||||
status = str(fm.get("status", "") or "").upper().strip()
|
||||
result = str(fm.get("result", "") or "").upper().strip()
|
||||
|
||||
if not verdict and not status and not result:
|
||||
return False, "No machine-readable verdict/status/result in test report frontmatter"
|
||||
if not verdict and not status:
|
||||
return False, "No machine-readable verdict/status in test report frontmatter"
|
||||
|
||||
label = verdict or status or result
|
||||
fields = f"{verdict} {status} {result}"
|
||||
fields = f"{verdict} {status}"
|
||||
for neg in _TESTS_NEGATIVE_TOKENS:
|
||||
if neg in fields:
|
||||
return False, f"Test verdict: {label} ({neg})"
|
||||
return False, f"Test verdict: {verdict or status} ({neg})"
|
||||
for pos in _TESTS_POSITIVE_TOKENS:
|
||||
if pos in fields:
|
||||
return True, f"Test verdict: {label} (PASS)"
|
||||
return True, f"Test verdict: {verdict or status} (PASS)"
|
||||
|
||||
return False, (
|
||||
"No recognized PASS verdict in frontmatter "
|
||||
f"(verdict={verdict!r}, status={status!r}, result={result!r})"
|
||||
)
|
||||
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]:
|
||||
|
||||
@@ -216,29 +216,6 @@ class TestCheckTestsPassed:
|
||||
passed, reason = check_tests_passed("enduro-trails", "ET-001")
|
||||
assert passed is True
|
||||
|
||||
def test_result_pass_only_passes(self, setup_work_item_dir):
|
||||
# ORCH-017: the tester agent prompt documents `result: PASS | FAIL` as the
|
||||
# machine-readable field. A report that follows that contract literally
|
||||
# (only `result: PASS`, no verdict:/status:) MUST pass the gate. Before this
|
||||
# fix the gate ignored `result:` and bounced such reports to development.
|
||||
self._write(
|
||||
setup_work_item_dir,
|
||||
"---\ntype: test-report\nwork_item_id: ET-001\nresult: PASS\n---\n\nbody\n",
|
||||
)
|
||||
passed, reason = check_tests_passed("enduro-trails", "ET-001")
|
||||
assert passed is True
|
||||
assert "PASS" in reason
|
||||
|
||||
def test_result_fail_only_fails(self, setup_work_item_dir):
|
||||
# The negative side of the documented `result: PASS | FAIL` contract.
|
||||
self._write(
|
||||
setup_work_item_dir,
|
||||
"---\ntype: test-report\nresult: FAIL\n---\n\n23 passed in body\n",
|
||||
)
|
||||
passed, reason = check_tests_passed("enduro-trails", "ET-001")
|
||||
assert passed is False
|
||||
assert "FAIL" in reason.upper()
|
||||
|
||||
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(
|
||||
|
||||
Reference in New Issue
Block a user