test(sop-checklist): add compute_na_state and parse_directives unit tests

31 cases covering:
- parse_directives: ack/revoke/na directive extraction, edge cases
  (whitespace, tab-indent, invalid gate chars, greedy reason capture,
  mixed directives, numeric aliases)
- compute_na_state: valid/invalid declarations, self-declare rejection,
  team membership probe calls, chronological ordering, unknown gate
  handling, null-user comment guard

No network calls. All 223 tests pass.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

tests/test_sop_checklist.py

@@ -0,0 +1,399 @@
+"""Tests for `.gitea/scripts/sop-checklist.py`.
+
+Covers the N/A declarations feature (`compute_na_state`) and the
+`parse_directives` helper that powers both ack/revoke and N/A parsing.
+
+Run:
+    python3 -m pytest tests/test_sop_checklist.py -v
+
+Dependencies: stdlib + pytest. No network. No live Gitea calls.
+"""
+from __future__ import annotations
+
+import importlib.util
+import os
+import sys
+from pathlib import Path
+from unittest import mock
+
+import pytest
+
+
+# --------------------------------------------------------------------------
+# Module-import fixture
+# --------------------------------------------------------------------------
+SCRIPT_PATH = (
+    Path(__file__).resolve().parent.parent
+    / ".gitea"
+    / "scripts"
+    / "sop-checklist.py"
+)
+
+
+@pytest.fixture(scope="module")
+def sc_module():
+    """Import sop-checklist.py as a module under a known env."""
+    env = {
+        "GITEA_TOKEN": "test-token",
+        "GITEA_HOST": "git.example.test",
+        "REPO": "owner/repo",
+        "DRY_RUN": "0",
+    }
+    with mock.patch.dict(os.environ, env, clear=False):
+        spec = importlib.util.spec_from_file_location(
+            "sop_checklist", SCRIPT_PATH
+        )
+        m = importlib.util.module_from_spec(spec)
+        spec.loader.exec_module(m)
+        # Force-set module globals captured at import time.
+        m.GITEA_TOKEN = env["GITEA_TOKEN"]
+        m.GITEA_HOST = env["GITEA_HOST"]
+        m.REPO = env["REPO"]
+        yield m
+
+
+# --------------------------------------------------------------------------
+# parse_directives
+# --------------------------------------------------------------------------
+
+class TestParseDirectives:
+    def test_empty_body(self, sc_module):
+        directives, na_directives = sc_module.parse_directives("", {})
+        assert directives == []
+        assert na_directives == []
+
+    def test_ack_directive(self, sc_module):
+        directives, na_directives = sc_module.parse_directives(
+            "/sop-ack qa-review", {}
+        )
+        assert directives == [("sop-ack", "qa-review", "")]
+        assert na_directives == []
+
+    def test_ack_directive_with_note(self, sc_module):
+        directives, na_directives = sc_module.parse_directives(
+            "/sop-ack qa-review CI-only change", {}
+        )
+        assert directives == [("sop-ack", "qa-review", "CI-only change")]
+        assert na_directives == []
+
+    def test_revoke_directive(self, sc_module):
+        directives, na_directives = sc_module.parse_directives(
+            "/sop-revoke qa-review missed edge case", {}
+        )
+        assert directives == [("sop-revoke", "qa-review", "missed edge case")]
+        assert na_directives == []
+
+    def test_na_directive_no_reason(self, sc_module):
+        """Bare /sop-n/a <gate> is valid."""
+        _, na_directives = sc_module.parse_directives(
+            "/sop-n/a qa-review", {}
+        )
+        assert na_directives == [("qa-review", "")]
+
+    def test_na_directive_with_reason(self, sc_module):
+        """Full /sop-n/a <gate> <reason> is parsed correctly."""
+        _, na_directives = sc_module.parse_directives(
+            "/sop-n/a qa-review CI/non-security-touching", {}
+        )
+        assert na_directives == [("qa-review", "CI/non-security-touching")]
+
+    def test_na_directive_multiple_gates(self, sc_module):
+        body = (
+            "/sop-n/a qa-review CI-only\n"
+            "/sop-n/a security-review no-op change\n"
+        )
+        _, na_directives = sc_module.parse_directives(body, {})
+        assert na_directives == [
+            ("qa-review", "CI-only"),
+            ("security-review", "no-op change"),
+        ]
+
+    def test_na_directive_requires_space_after_gate(self, sc_module):
+        """Without a space, /sop-n/a is not matched (the regex requires \t+)."""
+        _, na_directives = sc_module.parse_directives(
+            "/sop-n/a", {}
+        )
+        assert na_directives == []
+
+    def test_na_directive_invalid_gate_characters(self, sc_module):
+        r"""Gate names must match [A-Za-z0-9_\-]+. A dot in the gate name
+        causes the entire line not to match (no backtracking to partial match)."""
+        _, na_directives = sc_module.parse_directives(
+            "/sop-n/a qa.review some reason", {}
+        )
+        # The regex cannot match qa.review as a valid gate (dot is invalid).
+        # Since the (?:[ \t]+(.*))? is optional, the whole pattern fails and
+        # no directive is extracted.
+        assert na_directives == []
+
+    def test_mixed_directives(self, sc_module):
+        """ack + na directives can appear in the same comment."""
+        body = (
+            "/sop-ack sop-checklist all boxes ticked\n"
+            "/sop-n/a qa-review no code changes\n"
+        )
+        directives, na_directives = sc_module.parse_directives(body, {})
+        assert directives == [("sop-ack", "sop-checklist", "all boxes ticked")]
+        assert na_directives == [("qa-review", "no code changes")]
+
+    def test_na_directive_leading_whitespace(self, sc_module):
+        """Indent is permitted — regex matches [ \t]*."""
+        _, na_directives = sc_module.parse_directives(
+            "  /sop-n/a qa-review indented", {}
+        )
+        assert na_directives == [("qa-review", "indented")]
+
+    def test_na_directive_tab_indent(self, sc_module):
+        _, na_directives = sc_module.parse_directives(
+            "\t/sop-n/a qa-review tab-indented", {}
+        )
+        assert na_directives == [("qa-review", "tab-indented")]
+
+    def test_na_directive_trailing_whitespace_stripped(self, sc_module):
+        """Trailing spaces after the reason are stripped."""
+        _, na_directives = sc_module.parse_directives(
+            "/sop-n/a qa-review reason   \n", {}
+        )
+        assert na_directives == [("qa-review", "reason")]
+
+    def test_na_directive_multiple_on_same_line(self, sc_module):
+        """MULTILINE anchors at line start (^), but greedy (.*) in the reason
+        group captures everything after the first space, including the second
+        /sop-n/a on the same line."""
+        _, na_directives = sc_module.parse_directives(
+            "/sop-n/a qa-review /sop-n/a security-review", {}
+        )
+        # reason = "/sop-n/a security-review" (greedy capture)
+        assert na_directives == [("qa-review", "/sop-n/a security-review")]
+
+    def test_ack_numeric_alias(self, sc_module):
+        """Numeric alias e.g. /sop-ack 1 resolves via numeric_aliases."""
+        directives, _ = sc_module.parse_directives(
+            "/sop-ack 1", {1: "qa-review"}
+        )
+        assert directives == [("sop-ack", "qa-review", "")]
+
+
+# --------------------------------------------------------------------------
+# compute_na_state
+# --------------------------------------------------------------------------
+
+class TestComputeNaState:
+    def _make_comment(
+        self,
+        body: str,
+        user_login: str,
+        created_at: str = "2024-01-01T00:00:00Z",
+    ) -> dict:
+        return {
+            "id": 0,
+            "body": body,
+            "user": {"login": user_login},
+            "created_at": created_at,
+        }
+
+    def _na_gates(self) -> dict:
+        return {
+            "qa-review": {
+                "required_teams": ["qa"],
+            },
+            "security-review": {
+                "required_teams": ["security"],
+            },
+            "sop-checklist": {
+                "required_teams": ["engineering"],
+            },
+        }
+
+    # ----- empty / no declarations -----
+
+    def test_no_comments(self, sc_module):
+        result = sc_module.compute_na_state([], "author", self._na_gates(), lambda *a: [])
+        for gate in ["qa-review", "security-review"]:
+            assert result[gate]["declared"] is False
+            assert result[gate]["valid"] is False
+            assert result[gate]["error"] is None
+
+    def test_non_na_comment_ignored(self, sc_module):
+        comments = [self._make_comment("/sop-ack qa-review", "helper")]
+        result = sc_module.compute_na_state(comments, "author", self._na_gates(), lambda *a: [])
+        assert result["qa-review"]["declared"] is False
+
+    def test_unknown_gate_ignored(self, sc_module):
+        comments = [self._make_comment("/sop-n/a unknown-gate reason", "helper")]
+        result = sc_module.compute_na_state(comments, "author", self._na_gates(), lambda *a: [])
+        assert result["qa-review"]["declared"] is False
+
+    # ----- valid declarations -----
+
+    def test_valid_na_declaration(self, sc_module):
+        """Declarer in required team → valid=True, declared=True."""
+        comments = [self._make_comment("/sop-n/a qa-review CI-only change", "qa-member")]
+        probe = lambda _slug, users: ["qa-member"]
+        result = sc_module.compute_na_state(
+            comments, "author", self._na_gates(), probe
+        )
+        assert result["qa-review"]["declared"] is True
+        assert result["qa-review"]["declared_by"] == "qa-member"
+        assert result["qa-review"]["reason"] == "CI-only change"
+        assert result["qa-review"]["valid"] is True
+        assert result["qa-review"]["error"] is None
+
+    def test_valid_na_multiple_gates(self, sc_module):
+        comments = [
+            self._make_comment("/sop-n/a qa-review CI-only", "qa-member"),
+            self._make_comment("/sop-n/a security-review no-op", "sec-member"),
+        ]
+        probe = lambda slug, users: (
+            ["qa-member"] if "qa" in slug else ["sec-member"]
+        )
+        result = sc_module.compute_na_state(
+            comments, "author", self._na_gates(), probe
+        )
+        assert result["qa-review"]["valid"] is True
+        assert result["security-review"]["valid"] is True
+
+    def test_na_without_reason(self, sc_module):
+        """N/A with no reason is still valid if team membership checks out."""
+        comments = [self._make_comment("/sop-n/a qa-review", "qa-member")]
+        probe = lambda _slug, users: ["qa-member"]
+        result = sc_module.compute_na_state(
+            comments, "author", self._na_gates(), probe
+        )
+        assert result["qa-review"]["declared"] is True
+        assert result["qa-review"]["reason"] == ""
+        assert result["qa-review"]["valid"] is True
+
+    # ----- invalid declarations -----
+
+    def test_self_declare_rejected(self, sc_module):
+        """Authors cannot self-declare N/A."""
+        comments = [self._make_comment("/sop-n/a qa-review self-declared", "author")]
+        probe = lambda _slug, users: ["author"]
+        result = sc_module.compute_na_state(
+            comments, "author", self._na_gates(), probe
+        )
+        assert result["qa-review"]["declared"] is True
+        assert result["qa-review"]["valid"] is False
+        assert result["qa-review"]["error"] == "self-declare N/A rejected"
+
+    def test_not_in_required_team(self, sc_module):
+        """Declarer not in required team → valid=False, error set."""
+        comments = [self._make_comment("/sop-n/a qa-review CI-only", "random-user")]
+        probe = lambda _slug, users: []  # nobody approved
+        result = sc_module.compute_na_state(
+            comments, "author", self._na_gates(), probe
+        )
+        assert result["qa-review"]["declared"] is True
+        assert result["qa-review"]["valid"] is False
+        assert "not in required team" in result["qa-review"]["error"]
+
+    def test_probe_returns_empty_not_member(self, sc_module):
+        """Probe returns empty list even though the team exists."""
+        comments = [self._make_comment("/sop-n/a security-review no-op", "not-sec")]
+        probe = lambda _slug, users: []
+        result = sc_module.compute_na_state(
+            comments, "author", self._na_gates(), probe
+        )
+        assert result["security-review"]["valid"] is False
+        assert "not in required team" in result["security-review"]["error"]
+
+    # ----- chronological ordering -----
+
+    def test_most_recent_na_wins_per_user(self, sc_module):
+        """For the same user+gate, the most-recent comment wins."""
+        comments = [
+            self._make_comment("/sop-n/a qa-review old reason", "qa-member",
+                              created_at="2024-01-01T00:00:00Z"),
+            self._make_comment("/sop-n/a qa-review newer reason", "qa-member",
+                              created_at="2024-01-02T00:00:00Z"),
+        ]
+        probe = lambda _slug, users: ["qa-member"]
+        result = sc_module.compute_na_state(
+            comments, "author", self._na_gates(), probe
+        )
+        assert result["qa-review"]["reason"] == "newer reason"
+
+    def test_different_users_same_gate_both_declared(self, sc_module):
+        """Two different declarers for the same gate — both recorded."""
+        comments = [
+            self._make_comment("/sop-n/a qa-review first declarer", "qa-member-1",
+                              created_at="2024-01-01T00:00:00Z"),
+            self._make_comment("/sop-n/a qa-review second declarer", "qa-member-2",
+                              created_at="2024-01-02T00:00:00Z"),
+        ]
+        probe = lambda _slug, users: ["qa-member-1", "qa-member-2"]
+        result = sc_module.compute_na_state(
+            comments, "author", self._na_gates(), probe
+        )
+        # Most-recent declaration wins (qa-member-2 is newer).
+        assert result["qa-review"]["declared_by"] == "qa-member-2"
+        assert result["qa-review"]["reason"] == "second declarer"
+        assert result["qa-review"]["valid"] is True
+
+    # ----- gate key is exact -----
+
+    def test_unknown_gate_in_na_directive_ignored(self, sc_module):
+        """A gate name not in na_gates config is silently ignored."""
+        comments = [
+            self._make_comment("/sop-n/a qa-review valid", "qa-member"),
+            self._make_comment("/sop-n/a totally-unknown-gate reason", "qa-member"),
+        ]
+        probe = lambda _slug, users: ["qa-member"]
+        result = sc_module.compute_na_state(
+            comments, "author", self._na_gates(), probe
+        )
+        assert result["qa-review"]["declared"] is True
+        # unknown-gate is not in result keys — only configured gates appear.
+        assert "totally-unknown-gate" not in result
+
+    # ----- empty comments list -----
+
+    def test_comment_with_no_user_field(self, sc_module):
+        """A comment dict with no user.login is skipped (no crash)."""
+        comments = [{"id": 1, "body": "/sop-n/a qa-review", "user": {}}]
+        probe = lambda _slug, users: ["qa-member"]
+        result = sc_module.compute_na_state(
+            comments, "author", self._na_gates(), probe
+        )
+        assert result["qa-review"]["declared"] is False
+
+    def test_comment_with_empty_body(self, sc_module):
+        """A comment with empty body is handled gracefully."""
+        comments = [{"id": 1, "body": "", "user": {"login": "qa-member"}}]
+        probe = lambda _slug, users: ["qa-member"]
+        result = sc_module.compute_na_state(
+            comments, "author", self._na_gates(), probe
+        )
+        assert result["qa-review"]["declared"] is False
+
+    # ----- probe slug format -----
+
+    def test_probe_called_with_na_prefix(self, sc_module):
+        """The team membership probe is called with slug='na:<gate>'."""
+        calls = []
+        def track_probe(slug, users):
+            calls.append((slug, users))
+            return []
+
+        comments = [self._make_comment("/sop-n/a qa-review reason", "qa-member")]
+        sc_module.compute_na_state(
+            comments, "author", self._na_gates(), track_probe
+        )
+        assert ("na:qa-review", ["qa-member"]) in calls
+
+    # ----- undeclared gate remains False -----
+
+    def test_undeclared_gate_preserved(self, sc_module):
+        """A gate with no declaration stays declared=False even when
+        other gates are declared."""
+        comments = [
+            self._make_comment("/sop-n/a qa-review CI-only", "qa-member"),
+        ]
+        probe = lambda _slug, users: ["qa-member"]
+        result = sc_module.compute_na_state(
+            comments, "author", self._na_gates(), probe
+        )
+        assert result["qa-review"]["declared"] is True
+        assert result["security-review"]["declared"] is False
+        assert result["sop-checklist"]["declared"] is False