test(sop-checklist): add compute_na_state coverage
Some checks failed
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 41s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 35s
gate-check-v3 / gate-check (pull_request) Successful in 43s
CI / Detect changes (pull_request) Successful in 1m51s
qa-review / approved (pull_request) Successful in 41s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 1m24s
Runtime PR-Built Compatibility / detect-changes (pull_request) Successful in 1m24s
sop-checklist / all-items-acked (pull_request) Successful in 28s
sop-tier-check / tier-check (pull_request) Successful in 27s
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 1m59s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 12s
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 14s
CI / Python Lint & Test (pull_request) Successful in 13s
Runtime PR-Built Compatibility / PR-built wheel + import smoke (pull_request) Successful in 10s
CI / Canvas (Next.js) (pull_request) Successful in 9m59s
CI / Canvas Deploy Reminder (pull_request) Has been skipped
CI / Platform (Go) (pull_request) Has been cancelled
CI / all-required (pull_request) Has been cancelled
E2E API Smoke Test / E2E API Smoke Test (pull_request) Has been cancelled
E2E Chat / E2E Chat (pull_request) Has been cancelled
Ops Scripts Tests / Ops scripts (unittest) (pull_request) Has been cancelled
security-review / approved (pull_request) Has been cancelled
E2E API Smoke Test / detect-changes (pull_request) Has been cancelled
E2E Chat / detect-changes (pull_request) Has been cancelled
Some checks failed
Block internal-flavored paths / Block forbidden paths (pull_request) Successful in 41s
Secret scan / Scan diff for credential-shaped strings (pull_request) Successful in 35s
gate-check-v3 / gate-check (pull_request) Successful in 43s
CI / Detect changes (pull_request) Successful in 1m51s
qa-review / approved (pull_request) Successful in 41s
Handlers Postgres Integration / detect-changes (pull_request) Successful in 1m24s
Runtime PR-Built Compatibility / detect-changes (pull_request) Successful in 1m24s
sop-checklist / all-items-acked (pull_request) Successful in 28s
sop-tier-check / tier-check (pull_request) Successful in 27s
lint-required-no-paths / lint-required-no-paths (pull_request) Successful in 1m59s
CI / Shellcheck (E2E scripts) (pull_request) Successful in 12s
Handlers Postgres Integration / Handlers Postgres Integration (pull_request) Successful in 14s
CI / Python Lint & Test (pull_request) Successful in 13s
Runtime PR-Built Compatibility / PR-built wheel + import smoke (pull_request) Successful in 10s
CI / Canvas (Next.js) (pull_request) Successful in 9m59s
CI / Canvas Deploy Reminder (pull_request) Has been skipped
CI / Platform (Go) (pull_request) Has been cancelled
CI / all-required (pull_request) Has been cancelled
E2E API Smoke Test / E2E API Smoke Test (pull_request) Has been cancelled
E2E Chat / E2E Chat (pull_request) Has been cancelled
Ops Scripts Tests / Ops scripts (unittest) (pull_request) Has been cancelled
security-review / approved (pull_request) Has been cancelled
E2E API Smoke Test / detect-changes (pull_request) Has been cancelled
E2E Chat / detect-changes (pull_request) Has been cancelled
Addresses core-qa-agent CHANGES_REQUESTED on PR #1294. compute_na_state (~81 lines) had zero unit tests. Added 11 cases: - undeclared gates: declared=False, valid=False - self-declare: rejected with "self-declare N/A rejected" - non-team-member: valid=False + "not in required team" - team-member: valid=True, declared_by/reason populated - any-required-team: OR semantics across qa/security/engineers - most-recent wins per (user, gate) - different users/gates independent - empty/None body handled - unknown gate in directive silently skipped Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
parent
f63e19e9d0
commit
bcca4e4e08
@ -551,3 +551,136 @@ class TestEndToEndAckFlow(unittest.TestCase):
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main(verbosity=2)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# compute_na_state
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
# Minimal n/a_gates fixture mirroring sop-checklist-config.yaml §n/a_gates.
|
||||
_NA_GATES = {
|
||||
"qa-review": {"required_teams": ["qa", "security", "engineers"]},
|
||||
"security-review": {"required_teams": ["security", "managers", "ceo"]},
|
||||
}
|
||||
|
||||
|
||||
class TestComputeNaState(unittest.TestCase):
|
||||
@staticmethod
|
||||
def _approve_all(slug, users):
|
||||
return list(users)
|
||||
|
||||
@staticmethod
|
||||
def _approve_none(slug, users):
|
||||
return []
|
||||
|
||||
def _approve_only(self, allowed_users):
|
||||
return lambda slug, users: [u for u in users if u in allowed_users]
|
||||
|
||||
# -- undeclared gates --------------------------------------------------------
|
||||
|
||||
def test_undeclared_gate_returns_declared_false(self):
|
||||
"""Gates with no N/A comment are undeclared, not invalid."""
|
||||
state = sop.compute_na_state([], "alice-author", _NA_GATES, self._approve_all)
|
||||
self.assertFalse(state["qa-review"]["declared"])
|
||||
self.assertFalse(state["qa-review"]["valid"])
|
||||
self.assertEqual(state["qa-review"]["declared_by"], "")
|
||||
|
||||
def test_all_gates_present_even_when_empty_comments(self):
|
||||
state = sop.compute_na_state([], "alice-author", _NA_GATES, self._approve_all)
|
||||
self.assertIn("qa-review", state)
|
||||
self.assertIn("security-review", state)
|
||||
|
||||
# -- self-declare rejection --------------------------------------------------
|
||||
|
||||
def test_self_declare_rejected(self):
|
||||
"""Authors cannot N/A their own PR."""
|
||||
comments = [_comment("alice-author", "/sop-n/a qa-review needs-qa")]
|
||||
state = sop.compute_na_state(comments, "alice-author", _NA_GATES, self._approve_all)
|
||||
self.assertTrue(state["qa-review"]["declared"])
|
||||
self.assertFalse(state["qa-review"]["valid"])
|
||||
self.assertEqual(state["qa-review"]["error"], "self-declare N/A rejected")
|
||||
|
||||
def test_self_declare_does_not_leak_to_other_gates(self):
|
||||
"""A self-declare on qa-review does not affect security-review."""
|
||||
comments = [_comment("alice-author", "/sop-n/a qa-review I did QA myself")]
|
||||
state = sop.compute_na_state(comments, "alice-author", _NA_GATES, self._approve_all)
|
||||
self.assertFalse(state["security-review"]["declared"])
|
||||
|
||||
# -- team-membership enforcement ----------------------------------------------
|
||||
|
||||
def test_non_team_member_declares_invalid(self):
|
||||
"""Non-team-member declaration is valid=False with error."""
|
||||
comments = [_comment("random-user", "/sop-n/a qa-review pure-infra")]
|
||||
state = sop.compute_na_state(comments, "alice-author", _NA_GATES, self._approve_none)
|
||||
self.assertTrue(state["qa-review"]["declared"])
|
||||
self.assertFalse(state["qa-review"]["valid"])
|
||||
self.assertIn("not in required team", state["qa-review"]["error"])
|
||||
|
||||
def test_team_member_declares_valid(self):
|
||||
"""Team-member declaration is valid=True."""
|
||||
comments = [_comment("core-qa-agent", "/sop-n/a qa-review CI/non-touching")]
|
||||
state = sop.compute_na_state(
|
||||
comments, "alice-author", _NA_GATES, self._approve_only(["core-qa-agent"])
|
||||
)
|
||||
self.assertTrue(state["qa-review"]["declared"])
|
||||
self.assertTrue(state["qa-review"]["valid"])
|
||||
self.assertEqual(state["qa-review"]["declared_by"], "core-qa-agent")
|
||||
self.assertEqual(state["qa-review"]["reason"], "CI/non-touching")
|
||||
|
||||
def test_any_required_team_member_satisfies(self):
|
||||
"""qa-review requires [qa, security, engineers]; engineer passes."""
|
||||
comments = [_comment("infra-engineer", "/sop-n/a qa-review infra-only change")]
|
||||
state = sop.compute_na_state(
|
||||
comments, "alice-author", _NA_GATES, self._approve_only(["infra-engineer"])
|
||||
)
|
||||
self.assertTrue(state["qa-review"]["valid"])
|
||||
|
||||
# -- most-recent wins -------------------------------------------------------
|
||||
|
||||
def test_most_recent_na_wins_per_user_gate(self):
|
||||
"""When the same user posts multiple /sop-n/a for the same gate,
|
||||
the most recent (last in list) wins."""
|
||||
comments = [
|
||||
_comment("core-qa-agent", "/sop-n/a qa-review first-reason"),
|
||||
_comment("core-qa-agent", "/sop-n/a qa-review last-reason"),
|
||||
]
|
||||
state = sop.compute_na_state(
|
||||
comments, "alice-author", _NA_GATES, self._approve_only(["core-qa-agent"])
|
||||
)
|
||||
self.assertEqual(state["qa-review"]["reason"], "last-reason")
|
||||
|
||||
def test_different_users_different_gates_independent(self):
|
||||
"""N/A on qa-review does not affect security-review."""
|
||||
comments = [_comment("core-qa-agent", "/sop-n/a qa-review no-qa-surface")]
|
||||
state = sop.compute_na_state(
|
||||
comments, "alice-author", _NA_GATES, self._approve_only(["core-qa-agent"])
|
||||
)
|
||||
self.assertTrue(state["qa-review"]["valid"])
|
||||
self.assertFalse(state["security-review"]["declared"])
|
||||
|
||||
# -- idempotence ------------------------------------------------------------
|
||||
|
||||
def test_empty_body_and_none_body_handled(self):
|
||||
"""Comments with empty or None body do not break parsing."""
|
||||
comments = [
|
||||
{"user": {"login": "core-qa-agent"}, "body": ""},
|
||||
{"user": {"login": "core-qa-agent"}, "body": None},
|
||||
_comment("core-qa-agent", "/sop-n/a qa-review ok-reason"),
|
||||
]
|
||||
state = sop.compute_na_state(
|
||||
comments, "alice-author", _NA_GATES, self._approve_only(["core-qa-agent"])
|
||||
)
|
||||
self.assertTrue(state["qa-review"]["valid"])
|
||||
self.assertEqual(state["qa-review"]["reason"], "ok-reason")
|
||||
|
||||
# -- unknown gates filtered -------------------------------------------------
|
||||
|
||||
def test_unknown_gate_in_comment_ignored(self):
|
||||
"""A /sop-n/a for a gate not in na_gates is silently skipped."""
|
||||
comments = [_comment("core-qa-agent", "/sop-n/a unknown-gate no-such-gate")]
|
||||
state = sop.compute_na_state(
|
||||
comments, "alice-author", _NA_GATES, self._approve_only(["core-qa-agent"])
|
||||
)
|
||||
self.assertFalse(state["qa-review"]["declared"])
|
||||
self.assertFalse(state["security-review"]["declared"])
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user