Merge pull request 'feat: canonical validation wording, baseline proof, and closure gate (Closes #529)' (#598) from feat/issue-529-canonical-validation-wording into master
This commit was merged in pull request #598.
This commit is contained in:
@@ -0,0 +1,113 @@
|
||||
"""Controller-closure baseline-proof gate tests (#529, criterion 6)."""
|
||||
|
||||
import unittest
|
||||
|
||||
from controller_closure_baseline_proof import (
|
||||
assess_controller_closure_baseline_proof,
|
||||
)
|
||||
from premerge_baseline_proof import (
|
||||
CLEAN_PASS,
|
||||
PREMERGE_BASELINE_PROVEN_FAILURE,
|
||||
UNRESOLVED_REGRESSION_RISK,
|
||||
)
|
||||
from final_report_validator import assess_final_report_validator
|
||||
|
||||
# Complete pre-merge proof fields for a baseline claim (see #533).
|
||||
_PROOF_FIELDS = (
|
||||
"Pre-merge base commit: 1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d7e8f9a0b\n"
|
||||
"Tested commit: 1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d7e8f9a0b\n"
|
||||
"Command: venv/bin/python -m pytest tests/test_foo.py -q\n"
|
||||
"Exit status: 1\n"
|
||||
"Failure signature: AssertionError: None is not true\n"
|
||||
)
|
||||
|
||||
|
||||
class TestControllerClosureBaselineProof(unittest.TestCase):
|
||||
def test_empty_report_skipped_and_allowed(self):
|
||||
result = assess_controller_closure_baseline_proof("")
|
||||
self.assertFalse(result["block"])
|
||||
self.assertTrue(result["skipped"])
|
||||
self.assertEqual(result["label"], CLEAN_PASS)
|
||||
|
||||
def test_none_report_allowed(self):
|
||||
result = assess_controller_closure_baseline_proof(None)
|
||||
self.assertFalse(result["block"])
|
||||
self.assertTrue(result["skipped"])
|
||||
|
||||
def test_clean_pass_closure_allowed(self):
|
||||
result = assess_controller_closure_baseline_proof(
|
||||
"Closing #529. Validation: full suite passed, exit status 0. Clean pass."
|
||||
)
|
||||
self.assertFalse(result["block"])
|
||||
self.assertEqual(result["label"], CLEAN_PASS)
|
||||
|
||||
def test_expected_preexisting_failure_without_proof_blocked(self):
|
||||
report = (
|
||||
"Closing #529. Full suite: 1 failed. This is an expected "
|
||||
"pre-existing failure, safe to close."
|
||||
)
|
||||
result = assess_controller_closure_baseline_proof(report)
|
||||
self.assertTrue(result["block"])
|
||||
self.assertFalse(result["proven"])
|
||||
self.assertEqual(result["label"], UNRESOLVED_REGRESSION_RISK)
|
||||
self.assertTrue(result["reasons"])
|
||||
self.assertTrue(result["safe_next_action"])
|
||||
|
||||
def test_current_master_reproduction_only_blocked(self):
|
||||
report = (
|
||||
"Closing #529. Full suite: 1 failed. Pre-existing baseline failure — "
|
||||
"reproduced on current master after merge.\n"
|
||||
"Command: venv/bin/python -m pytest tests/test_foo.py -q\n"
|
||||
"Exit status: 1\n"
|
||||
"Failure signature: AssertionError: None is not true\n"
|
||||
)
|
||||
result = assess_controller_closure_baseline_proof(report)
|
||||
self.assertTrue(result["block"])
|
||||
|
||||
def test_proven_baseline_closure_allowed(self):
|
||||
report = (
|
||||
"Closing #529. Full suite: 1 failed, an expected pre-existing "
|
||||
"baseline failure proven on the PR pre-merge base commit.\n"
|
||||
+ _PROOF_FIELDS
|
||||
)
|
||||
result = assess_controller_closure_baseline_proof(report)
|
||||
self.assertFalse(result["block"])
|
||||
self.assertEqual(result["label"], PREMERGE_BASELINE_PROVEN_FAILURE)
|
||||
|
||||
|
||||
class TestControllerCloseTaskKind(unittest.TestCase):
|
||||
"""The composable validator must cover the controller_close task kind."""
|
||||
|
||||
def test_controller_close_blocks_unproven_baseline(self):
|
||||
report = (
|
||||
"Full suite: 1 failed. Expected pre-existing failure, closing the "
|
||||
"tracking issue."
|
||||
)
|
||||
result = assess_final_report_validator(report, "controller_close")
|
||||
self.assertTrue(result["blocked"])
|
||||
self.assertTrue(
|
||||
any(
|
||||
"premerge_baseline_proof" in f["rule_id"]
|
||||
for f in result["findings"]
|
||||
)
|
||||
)
|
||||
|
||||
def test_controller_close_alias_close_issue(self):
|
||||
report = (
|
||||
"Full suite: 1 failed. Expected pre-existing failure, closing the "
|
||||
"tracking issue."
|
||||
)
|
||||
result = assess_final_report_validator(report, "close_issue")
|
||||
self.assertTrue(result["blocked"])
|
||||
|
||||
def test_controller_close_allows_proven_baseline(self):
|
||||
report = (
|
||||
"Full suite: 1 failed, expected pre-existing baseline failure proven "
|
||||
"on the PR pre-merge base commit.\n" + _PROOF_FIELDS
|
||||
)
|
||||
result = assess_final_report_validator(report, "controller_close")
|
||||
self.assertFalse(result["blocked"])
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -2733,6 +2733,56 @@ class TestTrackerHygieneCleanup(unittest.TestCase):
|
||||
self.assertTrue(res["success"])
|
||||
self.assertEqual(res["cleanup_status"].get(1), "not present")
|
||||
|
||||
def test_close_issue_blocks_unproven_expected_preexisting_failure(self):
|
||||
# #529: a closure report calling a non-zero suite exit an "expected
|
||||
# pre-existing failure" without pre-merge proof must fail closed and
|
||||
# never PATCH the issue state.
|
||||
patched = []
|
||||
|
||||
def api_side_effect(method, url, auth, payload=None):
|
||||
if method == "PATCH":
|
||||
patched.append(url)
|
||||
return {}
|
||||
self.mock_api.side_effect = api_side_effect
|
||||
|
||||
res = gitea_close_issue(
|
||||
issue_number=1,
|
||||
closure_report=(
|
||||
"Full suite: 1 failed. Expected pre-existing failure, safe to close."
|
||||
),
|
||||
)
|
||||
self.assertFalse(res["success"])
|
||||
self.assertTrue(res.get("blocked"))
|
||||
self.assertTrue(res.get("reasons"))
|
||||
self.assertFalse(
|
||||
patched, "must not PATCH issue state when the closure gate blocks"
|
||||
)
|
||||
|
||||
def test_close_issue_allows_proven_baseline_closure(self):
|
||||
def api_side_effect(method, url, auth, payload=None):
|
||||
if method == "PATCH" and "issues/1" in url:
|
||||
return {"state": "closed"}
|
||||
if method == "GET" and "labels" in url and "issues" not in url:
|
||||
return [{"name": "bug", "id": 2}]
|
||||
if method == "GET" and "issues/1" in url:
|
||||
return {"labels": [{"name": "bug"}]}
|
||||
return {}
|
||||
self.mock_api.side_effect = api_side_effect
|
||||
|
||||
res = gitea_close_issue(
|
||||
issue_number=1,
|
||||
closure_report=(
|
||||
"Full suite: 1 failed, expected pre-existing baseline failure "
|
||||
"proven on the PR pre-merge base commit.\n"
|
||||
"Pre-merge base commit: 1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d7e8f9a0b\n"
|
||||
"Tested commit: 1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d7e8f9a0b\n"
|
||||
"Command: venv/bin/python -m pytest -q\n"
|
||||
"Exit status: 1\n"
|
||||
"Failure signature: AssertionError: None is not true\n"
|
||||
),
|
||||
)
|
||||
self.assertTrue(res["success"])
|
||||
|
||||
def test_merge_pr_with_closes_removes_label(self):
|
||||
import reviewer_pr_lease
|
||||
|
||||
|
||||
@@ -0,0 +1,119 @@
|
||||
"""Post-merge moot validation wording + label tests (#529, criteria 1/4/5)."""
|
||||
|
||||
import unittest
|
||||
|
||||
from post_merge_validation import (
|
||||
BASELINE_ACCEPTED_OUTCOME,
|
||||
BLOCKED_OUTCOME,
|
||||
CLEAN_PASS_OUTCOME,
|
||||
LABEL_BASELINE_ACCEPTED,
|
||||
LABEL_BLOCKED,
|
||||
LABEL_CLEAN_PASS,
|
||||
LABEL_POST_MERGE_MOOT,
|
||||
POST_MERGE_MOOT_OUTCOME,
|
||||
assess_post_merge_validation,
|
||||
process_state_label,
|
||||
)
|
||||
from final_report_validator import assess_final_report_validator
|
||||
from issue_workflow_labels import CANONICAL_LABELS, VALIDATION_LABELS
|
||||
|
||||
_MOOT_PROOF = (
|
||||
"Validation status: post-merge moot validation\n"
|
||||
"PR state: merged\n"
|
||||
"Merge commit sha: 1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d7e8f9a0b\n"
|
||||
"Validation finding: full suite passed, for the record.\n"
|
||||
)
|
||||
|
||||
|
||||
class TestPostMergeValidation(unittest.TestCase):
|
||||
def test_not_applicable_when_no_merged_or_moot_signal(self):
|
||||
result = assess_post_merge_validation(
|
||||
"Review decision: approve. Validation: full suite passed."
|
||||
)
|
||||
self.assertFalse(result["block"])
|
||||
self.assertTrue(result["skipped"])
|
||||
|
||||
def test_active_approval_on_merged_pr_blocked(self):
|
||||
report = (
|
||||
"PR is already merged. Review decision: approve. "
|
||||
"Validation: full suite passed."
|
||||
)
|
||||
result = assess_post_merge_validation(report)
|
||||
self.assertTrue(result["block"])
|
||||
self.assertEqual(result["outcome"], POST_MERGE_MOOT_OUTCOME)
|
||||
self.assertEqual(result["process_state_label"], LABEL_POST_MERGE_MOOT)
|
||||
|
||||
def test_active_approval_on_merged_pr_via_structured_signal_blocked(self):
|
||||
report = "Review decision: approve. Validation: full suite passed."
|
||||
result = assess_post_merge_validation(report, pr_merged_or_closed=True)
|
||||
self.assertTrue(result["block"])
|
||||
|
||||
def test_moot_wording_without_proof_blocked(self):
|
||||
report = "Recording post-merge moot validation for this PR."
|
||||
result = assess_post_merge_validation(report)
|
||||
self.assertTrue(result["block"])
|
||||
self.assertTrue(result["reasons"])
|
||||
|
||||
def test_moot_wording_with_full_proof_allowed(self):
|
||||
result = assess_post_merge_validation(_MOOT_PROOF)
|
||||
self.assertFalse(result["block"])
|
||||
self.assertEqual(result["outcome"], POST_MERGE_MOOT_OUTCOME)
|
||||
|
||||
def test_merged_signal_only_guides_without_blocking(self):
|
||||
result = assess_post_merge_validation(
|
||||
"PR was already merged before review; recording findings."
|
||||
)
|
||||
self.assertFalse(result["block"])
|
||||
self.assertTrue(result["safe_next_action"])
|
||||
|
||||
def test_process_state_label_mapping(self):
|
||||
self.assertEqual(process_state_label(CLEAN_PASS_OUTCOME), LABEL_CLEAN_PASS)
|
||||
self.assertEqual(
|
||||
process_state_label(BASELINE_ACCEPTED_OUTCOME), LABEL_BASELINE_ACCEPTED
|
||||
)
|
||||
self.assertEqual(process_state_label(BLOCKED_OUTCOME), LABEL_BLOCKED)
|
||||
self.assertEqual(
|
||||
process_state_label(POST_MERGE_MOOT_OUTCOME), LABEL_POST_MERGE_MOOT
|
||||
)
|
||||
self.assertIsNone(process_state_label("nonsense"))
|
||||
|
||||
|
||||
class TestValidationLabelsRegistered(unittest.TestCase):
|
||||
def test_validation_labels_are_canonical(self):
|
||||
for label in (
|
||||
LABEL_CLEAN_PASS,
|
||||
LABEL_BASELINE_ACCEPTED,
|
||||
LABEL_BLOCKED,
|
||||
LABEL_POST_MERGE_MOOT,
|
||||
):
|
||||
self.assertIn(label, VALIDATION_LABELS)
|
||||
self.assertIn(label, CANONICAL_LABELS)
|
||||
|
||||
|
||||
class TestPostMergeValidationWiredIntoReview(unittest.TestCase):
|
||||
def test_review_pr_blocks_active_approval_on_merged_pr(self):
|
||||
report = (
|
||||
"PR is already merged. Review decision: approve. "
|
||||
"Validation: full suite passed."
|
||||
)
|
||||
result = assess_final_report_validator(report, "review_pr")
|
||||
self.assertTrue(result["blocked"])
|
||||
self.assertTrue(
|
||||
any(
|
||||
f["rule_id"] == "reviewer.post_merge_validation"
|
||||
for f in result["findings"]
|
||||
)
|
||||
)
|
||||
|
||||
def test_review_pr_allows_proven_post_merge_moot(self):
|
||||
result = assess_final_report_validator(_MOOT_PROOF, "review_pr")
|
||||
self.assertFalse(
|
||||
any(
|
||||
f["rule_id"] == "reviewer.post_merge_validation"
|
||||
for f in result["findings"]
|
||||
)
|
||||
)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
Reference in New Issue
Block a user