fix: resolve conflicts for PR #363
This commit is contained in:
@@ -0,0 +1,273 @@
|
||||
"""Tests for composable final-report validator (#327)."""
|
||||
import sys
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
|
||||
sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
|
||||
|
||||
from final_report_validator import ( # noqa: E402
|
||||
FINAL_REPORT_TASK_KINDS,
|
||||
assess_final_report_validator,
|
||||
validator_finding,
|
||||
)
|
||||
|
||||
|
||||
def _review_handoff(**overrides):
|
||||
lines = [
|
||||
"## Controller Handoff",
|
||||
"",
|
||||
"- Task: review PR #203",
|
||||
"- Repo: Scaled-Tech-Consulting/Gitea-Tools",
|
||||
"- Role: reviewer",
|
||||
"- Identity: sysadmin / prgs-reviewer",
|
||||
"- Issue/PR: #182 / PR #203",
|
||||
"- Branch/SHA: feat/x @ 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
|
||||
"- Files changed: review_proofs.py",
|
||||
"- Validation: pytest tests/test_review_proofs.py -q in branches/review-203",
|
||||
"- Mutations: review only",
|
||||
"- Workspace mutations: none",
|
||||
"- File edits by reviewer: none",
|
||||
"- Worktree/index mutations: none",
|
||||
"- Git ref mutations: git fetch prgs master",
|
||||
"- MCP/Gitea mutations: review submitted",
|
||||
"- Current status: PR open",
|
||||
"- Blockers: none",
|
||||
"- Next: await author",
|
||||
"- Safety: no self-review; no self-merge",
|
||||
"- Selected PR: #203",
|
||||
"- Reviewer eligibility: eligible",
|
||||
"- Pinned reviewed head: 0fdc8f582026b72a229d59a172c0a63ac4aaeaf9",
|
||||
"- Worktree path: branches/review-203",
|
||||
"- Worktree dirty: clean",
|
||||
"- Scratch worktree used: yes (branches/review-203)",
|
||||
"- Unrelated local mutations: none",
|
||||
"- Review decision: request_changes",
|
||||
"- Merge result: not attempted",
|
||||
"- Linked issue: #182",
|
||||
"- Linked issue status: open",
|
||||
"- Cleanup status: none",
|
||||
]
|
||||
text = "\n".join(lines)
|
||||
for key, value in overrides.items():
|
||||
text = text.replace(f"- {key}:", f"- {key}: {value}")
|
||||
return text
|
||||
|
||||
|
||||
def _reconcile_handoff(**overrides):
|
||||
lines = [
|
||||
"## Controller Handoff",
|
||||
"",
|
||||
"- Task: reconcile already-landed PR #99",
|
||||
"- Repo: Scaled-Tech-Consulting/Gitea-Tools",
|
||||
"- Role: reconciler",
|
||||
"- Identity: sysadmin / prgs-author",
|
||||
"- Selected PR: #99",
|
||||
"- Eligibility class: ALREADY_LANDED_RECONCILE_REQUIRED",
|
||||
"- Linked issue: #98",
|
||||
"- Linked issue live status: not verified in this session",
|
||||
"- File edits by reconciler: none",
|
||||
"- Git ref mutations: git fetch prgs master",
|
||||
"- Reconciliation mutations: PR comment posted",
|
||||
"- Current status: reconciliation complete",
|
||||
"- Safe next action: none",
|
||||
"- No review/merge confirmation: confirmed",
|
||||
]
|
||||
text = "\n".join(lines)
|
||||
for key, value in overrides.items():
|
||||
if f"- {key}:" in text:
|
||||
text = text.replace(f"- {key}:", f"- {key}: {value}")
|
||||
return text
|
||||
|
||||
|
||||
class TestValidatorFinding(unittest.TestCase):
|
||||
def test_finding_shape(self):
|
||||
finding = validator_finding(
|
||||
"reviewer.test",
|
||||
"block",
|
||||
"Validation",
|
||||
"missing proof",
|
||||
"repair validation proof",
|
||||
)
|
||||
self.assertEqual(finding["rule_id"], "reviewer.test")
|
||||
self.assertEqual(finding["severity"], "block")
|
||||
self.assertEqual(finding["field"], "Validation")
|
||||
|
||||
|
||||
class TestReviewPrRules(unittest.TestCase):
|
||||
def test_clean_reviewer_report_passes(self):
|
||||
result = assess_final_report_validator(
|
||||
_review_handoff(),
|
||||
"review_pr",
|
||||
linked_issue_lock={"issue_number": 182, "pr_number": 203},
|
||||
)
|
||||
self.assertEqual(result["grade"], "A")
|
||||
self.assertFalse(result["blocked"])
|
||||
self.assertEqual(result["findings"], [])
|
||||
|
||||
def test_legacy_workspace_mutations_blocks(self):
|
||||
report = _review_handoff() + "\n- Workspace mutations: none"
|
||||
result = assess_final_report_validator(
|
||||
report,
|
||||
"review_pr",
|
||||
mutations_observed=True,
|
||||
)
|
||||
self.assertEqual(result["grade"], "blocked")
|
||||
rule_ids = [f["rule_id"] for f in result["findings"]]
|
||||
self.assertIn("reviewer.legacy_workspace_mutations", rule_ids)
|
||||
|
||||
def test_vague_mutations_none_blocks(self):
|
||||
report = _review_handoff().replace(
|
||||
"- Mutations: review only",
|
||||
"- Mutations: none",
|
||||
)
|
||||
result = assess_final_report_validator(
|
||||
report,
|
||||
"review_pr",
|
||||
mutations_observed=True,
|
||||
)
|
||||
self.assertTrue(result["blocked"])
|
||||
self.assertTrue(
|
||||
any(f["rule_id"] == "reviewer.vague_mutations_none" for f in result["findings"])
|
||||
)
|
||||
|
||||
def test_bare_pytest_without_cwd_downgrades(self):
|
||||
report = _review_handoff().replace(
|
||||
"- Validation: pytest tests/test_review_proofs.py -q in branches/review-203",
|
||||
"- Validation: pytest passed",
|
||||
)
|
||||
result = assess_final_report_validator(report, "review_pr")
|
||||
self.assertEqual(result["grade"], "downgraded")
|
||||
self.assertTrue(
|
||||
any(f["rule_id"] == "reviewer.validation_command_proof" for f in result["findings"])
|
||||
)
|
||||
|
||||
def test_linked_issue_mismatch_blocks(self):
|
||||
report = _review_handoff().replace("- Linked issue: #182", "- Linked issue: #999")
|
||||
result = assess_final_report_validator(
|
||||
report,
|
||||
"review_pr",
|
||||
linked_issue_lock={"issue_number": 182, "pr_number": 203},
|
||||
)
|
||||
self.assertTrue(result["blocked"])
|
||||
self.assertTrue(
|
||||
any(f["rule_id"] == "reviewer.linked_issue_mismatch" for f in result["findings"])
|
||||
)
|
||||
|
||||
def test_approval_after_full_suite_failure_blocks_without_baseline(self):
|
||||
report = (
|
||||
_review_handoff()
|
||||
.replace("- Review decision: request_changes", "- Review decision: approve")
|
||||
+ "\nSubmitted 'approve' after full suite failed with 3 failed tests."
|
||||
)
|
||||
result = assess_final_report_validator(report, "review_pr")
|
||||
self.assertTrue(result["blocked"])
|
||||
self.assertTrue(
|
||||
any(
|
||||
f["rule_id"] == "reviewer.baseline_on_full_suite_failure"
|
||||
for f in result["findings"]
|
||||
)
|
||||
)
|
||||
|
||||
def test_same_as_master_without_baseline_blocks(self):
|
||||
report = _review_handoff() + "\nFailures are same as master baseline."
|
||||
result = assess_final_report_validator(report, "review_pr")
|
||||
self.assertTrue(result["blocked"])
|
||||
self.assertTrue(
|
||||
any(f["rule_id"] == "reviewer.main_checkout_baseline" for f in result["findings"])
|
||||
)
|
||||
|
||||
def test_already_landed_eligible_wording_blocks(self):
|
||||
report = (
|
||||
_review_handoff()
|
||||
+ "\nPR is already landed and eligible for review next."
|
||||
)
|
||||
result = assess_final_report_validator(report, "review_pr")
|
||||
self.assertTrue(result["blocked"])
|
||||
self.assertTrue(
|
||||
any(
|
||||
f["rule_id"] == "reviewer.already_landed_eligible_wording"
|
||||
for f in result["findings"]
|
||||
)
|
||||
)
|
||||
|
||||
def test_git_fetch_must_not_be_readonly_only(self):
|
||||
report = (
|
||||
_review_handoff()
|
||||
.replace("- Git ref mutations: git fetch prgs master", "- Git ref mutations: none")
|
||||
+ "\n- Read-only diagnostics: git fetch prgs master"
|
||||
)
|
||||
result = assess_final_report_validator(
|
||||
report,
|
||||
"review_pr",
|
||||
action_log=[{"command": "git fetch prgs master", "performed": True}],
|
||||
)
|
||||
self.assertTrue(result["blocked"])
|
||||
self.assertTrue(
|
||||
any(f["rule_id"] == "reviewer.git_fetch_readonly" for f in result["findings"])
|
||||
)
|
||||
|
||||
|
||||
class TestReconciliationRules(unittest.TestCase):
|
||||
def test_clean_reconcile_report_passes(self):
|
||||
result = assess_final_report_validator(
|
||||
_reconcile_handoff(),
|
||||
"reconcile_already_landed",
|
||||
)
|
||||
self.assertEqual(result["grade"], "A")
|
||||
|
||||
def test_stale_author_field_blocks(self):
|
||||
report = _reconcile_handoff() + "\n- PR number opened: #12"
|
||||
result = assess_final_report_validator(report, "reconcile_already_landed")
|
||||
self.assertTrue(result["blocked"])
|
||||
self.assertTrue(
|
||||
any(
|
||||
f["rule_id"] == "reconcile.stale_author_reviewer_fields"
|
||||
for f in result["findings"]
|
||||
)
|
||||
)
|
||||
|
||||
def test_eligible_wording_for_landed_pr_blocks(self):
|
||||
report = _reconcile_handoff() + "\nPR already landed and eligible for merge."
|
||||
result = assess_final_report_validator(report, "reconcile_already_landed")
|
||||
self.assertTrue(result["blocked"])
|
||||
|
||||
def test_linked_issue_status_without_live_proof_downgrades(self):
|
||||
report = _reconcile_handoff().replace(
|
||||
"- Linked issue live status: not verified in this session",
|
||||
"- Linked issue live status: closed",
|
||||
)
|
||||
result = assess_final_report_validator(report, "reconcile_already_landed")
|
||||
self.assertEqual(result["grade"], "downgraded")
|
||||
self.assertTrue(
|
||||
any(f["rule_id"] == "reconcile.linked_issue_live_status" for f in result["findings"])
|
||||
)
|
||||
|
||||
def test_inventory_complete_requires_pagination_proof(self):
|
||||
report = _reconcile_handoff() + "\nInventory complete for open PRs."
|
||||
result = assess_final_report_validator(report, "reconcile_already_landed")
|
||||
self.assertEqual(result["grade"], "downgraded")
|
||||
self.assertTrue(
|
||||
any(f["rule_id"] == "reconcile.inventory_pagination_proof" for f in result["findings"])
|
||||
)
|
||||
|
||||
|
||||
class TestEntryPoint(unittest.TestCase):
|
||||
def test_unknown_task_kind_blocks(self):
|
||||
result = assess_final_report_validator("report", "unknown_mode")
|
||||
self.assertTrue(result["blocked"])
|
||||
self.assertEqual(result["findings"][0]["rule_id"], "shared.unknown_task_kind")
|
||||
|
||||
def test_review_proofs_reexport(self):
|
||||
from review_proofs import assess_final_report_validator as exported
|
||||
|
||||
self.assertTrue(callable(exported))
|
||||
result = exported(_review_handoff(), "review_pr")
|
||||
self.assertIn("grade", result)
|
||||
|
||||
def test_supported_task_kinds_include_review_and_reconcile(self):
|
||||
self.assertIn("review_pr", FINAL_REPORT_TASK_KINDS)
|
||||
self.assertIn("reconcile_already_landed", FINAL_REPORT_TASK_KINDS)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
@@ -118,6 +118,12 @@ def test_start_issue_template_references_work_issue_workflow():
|
||||
assert "workflows/work-issue.md" in text
|
||||
|
||||
|
||||
def test_final_report_validator_exported():
|
||||
from review_proofs import assess_final_report_validator
|
||||
|
||||
assert callable(assess_final_report_validator)
|
||||
|
||||
|
||||
def test_create_issue_final_report_verifier_exported():
|
||||
from review_proofs import assess_create_issue_final_report
|
||||
|
||||
@@ -133,4 +139,4 @@ def test_non_mergeable_skip_verifier_exported():
|
||||
def test_validation_worktree_edit_verifier_exported():
|
||||
from review_proofs import assess_validation_worktree_edit_report
|
||||
|
||||
assert callable(assess_validation_worktree_edit_report)
|
||||
assert callable(assess_validation_worktree_edit_report)
|
||||
|
||||
@@ -24,6 +24,7 @@ from review_proofs import ( # noqa: E402
|
||||
ISSUE_SELECTION_CONTINUATION_EXPLICIT,
|
||||
ISSUE_SELECTION_REPRESENTED_BY_OPEN_PR,
|
||||
assess_author_pr_report,
|
||||
assess_queue_ordering_proof,
|
||||
assess_reviewer_baseline_validation_proof,
|
||||
assess_capability_evidence,
|
||||
assess_capability_proof,
|
||||
@@ -2161,6 +2162,111 @@ class TestCreateIssueFinalReport(unittest.TestCase):
|
||||
self.assertTrue(result["downgraded"])
|
||||
|
||||
|
||||
class TestQueueOrderingProof(unittest.TestCase):
|
||||
"""Issue #321: reviewer queue selection must prove ordering."""
|
||||
|
||||
def test_next_eligible_claim_without_policy_fails(self):
|
||||
result = assess_queue_ordering_proof(
|
||||
report_text="Selected the next eligible PR: PR #286.",
|
||||
)
|
||||
self.assertFalse(result["proven"])
|
||||
self.assertTrue(result["claims_selection"])
|
||||
|
||||
def test_next_eligible_claim_with_policy_passes(self):
|
||||
result = assess_queue_ordering_proof(
|
||||
report_text=(
|
||||
"Queue ordering policy: oldest PR number first\n"
|
||||
"Selected the next eligible PR: PR #286."
|
||||
),
|
||||
)
|
||||
self.assertTrue(result["proven"])
|
||||
self.assertEqual(result["policy"], "oldest PR number first")
|
||||
|
||||
def test_newest_first_api_with_oldest_first_selection_passes(self):
|
||||
result = assess_queue_ordering_proof(
|
||||
report_text=(
|
||||
"Queue ordering policy: oldest PR number first\n"
|
||||
"API order was newest-first (PR #291 before PR #286).\n"
|
||||
"Selected the next eligible PR: PR #286."
|
||||
),
|
||||
queue_ordering={
|
||||
"policy": "oldest PR number first",
|
||||
"selected_pr_number": 286,
|
||||
"api_pr_numbers": [291, 286],
|
||||
},
|
||||
)
|
||||
self.assertTrue(result["proven"])
|
||||
|
||||
def test_earlier_actionable_pr_without_skip_reason_fails(self):
|
||||
result = assess_queue_ordering_proof(
|
||||
report_text=(
|
||||
"Queue ordering policy: oldest PR number first\n"
|
||||
"Selected the next eligible PR: PR #300."
|
||||
),
|
||||
queue_ordering={
|
||||
"policy": "oldest PR number first",
|
||||
"selected_pr_number": 300,
|
||||
"api_pr_numbers": [300, 286],
|
||||
},
|
||||
)
|
||||
self.assertFalse(result["proven"])
|
||||
self.assertTrue(result["block"])
|
||||
|
||||
def test_priority_override_requires_explanation(self):
|
||||
result = assess_queue_ordering_proof(
|
||||
report_text="Selected the next eligible PR: PR #300.",
|
||||
queue_ordering={
|
||||
"policy": "priority label",
|
||||
"selected_pr_number": 300,
|
||||
"api_pr_numbers": [286, 300],
|
||||
},
|
||||
)
|
||||
self.assertFalse(result["proven"])
|
||||
|
||||
def test_priority_override_with_explanation_passes(self):
|
||||
result = assess_queue_ordering_proof(
|
||||
report_text=(
|
||||
"Queue ordering policy: priority label\n"
|
||||
"Priority override: security label on PR #300.\n"
|
||||
"Selected the next eligible PR: PR #300."
|
||||
),
|
||||
queue_ordering={
|
||||
"policy": "priority label",
|
||||
"selected_pr_number": 300,
|
||||
"api_pr_numbers": [286, 300],
|
||||
"priority_override": "security label on PR #300",
|
||||
},
|
||||
)
|
||||
self.assertTrue(result["proven"])
|
||||
|
||||
def test_build_final_report_downgrades_missing_ordering_proof(self):
|
||||
report = build_final_report(
|
||||
checkout_proof=_good_checkout(),
|
||||
inventory=_good_inventory(),
|
||||
validation=_good_validation(),
|
||||
contamination=_good_contamination(),
|
||||
identity_eligible=True,
|
||||
merge_performed=False,
|
||||
issue_status_verified=True,
|
||||
capability_evidence=_good_capability_evidence(),
|
||||
sweep=_good_sweep(),
|
||||
live_state=_good_live_state(),
|
||||
role_boundary=_good_role_boundary(),
|
||||
review_mutation=_good_review_mutation(),
|
||||
controller_handoff=_good_handoff(),
|
||||
capability_proof=_good_capability_proof(),
|
||||
sweep_proof=_good_secret_sweep(),
|
||||
worktree_proof={
|
||||
"worktree_path": "/repo/branches/review-feat-issue-224",
|
||||
"porcelain_status": "",
|
||||
"pr_scope_files": ["docs/wiki/Repositories.md"],
|
||||
"scratch_used": True,
|
||||
},
|
||||
report_text="Selected the next eligible PR: PR #286.",
|
||||
)
|
||||
self.assertNotEqual(report["grade"], "A")
|
||||
self.assertFalse(report["queue_ordering_proven"])
|
||||
|
||||
class TestReviewerBaselineValidationProof(unittest.TestCase):
|
||||
"""Issue #325: baseline validation must use branches/ worktrees."""
|
||||
|
||||
|
||||
Reference in New Issue
Block a user