feat(tests): load-balanced reviewer selection and Jira reviewer sync

This commit is contained in:
kitos
2026-07-06 10:56:40 +02:00
parent c41876b62f
commit 22be620665
3 changed files with 456 additions and 12 deletions
+183 -5
View File
@@ -10,6 +10,7 @@ without requiring a running database.
import sys
import os
import uuid
import pytest
from unittest.mock import MagicMock, patch
from types import ModuleType
from datetime import datetime
@@ -100,13 +101,16 @@ for _mod in [
from fastapi import HTTPException
from app.domain.exceptions import InvalidOperationError, InvalidTransitionError
from app.models.enums import TestState, TestResult
from app.models.test import Test
from app.services.test_workflow_service import (
VALID_TRANSITIONS,
can_transition,
transition_state,
start_execution,
submit_red_evidence,
approve_red_review,
submit_blue_evidence,
approve_blue_review,
validate_as_red_lead,
validate_as_blue_lead,
check_dual_validation,
@@ -158,27 +162,33 @@ def _make_db() -> MagicMock:
# ===========================================================================
@patch("app.services.test_workflow_service.select_reviewer")
@patch("app.services.test_workflow_service.log_action")
def test_full_happy_path(mock_log):
"""draft -> red_executing -> blue_evaluating -> in_review -> validated"""
def test_full_happy_path(mock_log, mock_select_reviewer):
"""draft -> red_executing -> red_review -> blue_evaluating -> blue_review -> in_review -> validated"""
test = _make_test(TestState.draft)
red_tech = _make_user("red_tech")
blue_tech = _make_user("blue_tech")
red_lead = _make_user("red_lead")
blue_lead = _make_user("blue_lead")
db = _make_db()
mock_select_reviewer.side_effect = [red_lead, blue_lead]
# Step 1: draft -> red_executing
result = start_execution(db, test, red_tech)
assert result.state == TestState.red_executing
assert result.execution_date is not None
# Step 2: red_executing -> blue_evaluating
# Step 2: red_executing -> red_review -> (Red Lead approves) -> blue_evaluating
result = submit_red_evidence(db, result, red_tech)
assert result.state == TestState.red_review
result = approve_red_review(db, result, red_lead)
assert result.state == TestState.blue_evaluating
# Step 3: blue_evaluating -> in_review
# Step 3: blue_evaluating -> blue_review -> (Blue Lead approves) -> in_review
result = submit_blue_evidence(db, result, blue_tech)
assert result.state == TestState.blue_review
result = approve_blue_review(db, result, blue_lead)
assert result.state == TestState.in_review
# Step 4: Red Lead approves
@@ -205,19 +215,24 @@ def test_full_happy_path(mock_log):
# ===========================================================================
@patch("app.services.test_workflow_service.select_reviewer")
@patch("app.services.test_workflow_service.log_action")
def test_rejection_and_reopen(mock_log):
def test_rejection_and_reopen(mock_log, mock_select_reviewer):
"""in_review -> rejected -> draft -> red_executing -> ..."""
test = _make_test(TestState.draft)
red_tech = _make_user("red_tech")
blue_tech = _make_user("blue_tech")
red_lead = _make_user("red_lead")
blue_lead = _make_user("blue_lead")
db = _make_db()
mock_select_reviewer.side_effect = [red_lead, blue_lead]
# Advance to in_review
start_execution(db, test, red_tech)
submit_red_evidence(db, test, red_tech)
approve_red_review(db, test, red_lead)
submit_blue_evidence(db, test, blue_tech)
approve_blue_review(db, test, blue_lead)
assert test.state == TestState.in_review
# Red Lead rejects -> rejected
@@ -577,6 +592,169 @@ def test_cannot_reopen_non_rejected_test(mock_log):
# Run all
# ---------------------------------------------------------------------------
# ===========================================================================
# 12b. Review-decision functions (approve/reopen/gap)
# ===========================================================================
class TestReviewDecisions:
@patch("app.services.test_workflow_service.log_action")
def test_approve_red_review_moves_to_blue_evaluating(self, mock_log):
test = _make_test(TestState.red_review)
reviewer = _make_user("red_lead")
db = _make_db()
from app.services.test_workflow_service import approve_red_review
result = approve_red_review(db, test, reviewer, notes="looks good")
assert result.state == TestState.blue_evaluating
assert result.red_review_by == reviewer.id
assert result.red_review_notes == "looks good"
assert result.blue_started_at is not None
def test_reopen_red_review_requires_notes(self):
test = _make_test(TestState.red_review)
reviewer = _make_user("red_lead")
db = _make_db()
from app.services.test_workflow_service import reopen_red_review
with pytest.raises(InvalidOperationError):
reopen_red_review(db, test, reviewer, notes="")
@patch("app.services.test_workflow_service.log_action")
def test_reopen_red_review_moves_to_red_executing_with_notes(self, mock_log):
test = _make_test(TestState.red_review, red_tech_assignee=uuid.uuid4())
reviewer = _make_user("red_lead")
db = _make_db()
from app.services.test_workflow_service import reopen_red_review
result = reopen_red_review(db, test, reviewer, notes="add more detail")
assert result.state == TestState.red_executing
assert result.red_review_notes == "add more detail"
@patch("app.services.test_workflow_service.log_action")
def test_approve_blue_review_moves_to_in_review(self, mock_log):
test = _make_test(TestState.blue_review)
reviewer = _make_user("blue_lead")
db = _make_db()
from app.services.test_workflow_service import approve_blue_review
result = approve_blue_review(db, test, reviewer)
assert result.state == TestState.in_review
def test_reopen_blue_review_requires_notes(self):
test = _make_test(TestState.blue_review)
reviewer = _make_user("blue_lead")
db = _make_db()
from app.services.test_workflow_service import reopen_blue_review
with pytest.raises(InvalidOperationError):
reopen_blue_review(db, test, reviewer, notes=None)
@patch("app.services.test_workflow_service.log_action")
def test_reopen_blue_review_moves_to_blue_evaluating(self, mock_log):
test = _make_test(TestState.blue_review, blue_tech_assignee=uuid.uuid4())
reviewer = _make_user("blue_lead")
db = _make_db()
from app.services.test_workflow_service import reopen_blue_review
result = reopen_blue_review(db, test, reviewer, notes="redo it")
assert result.state == TestState.blue_evaluating
assert result.blue_work_started_at is None
def test_flag_blue_review_gap_requires_system_gaps_text(self):
test = _make_test(TestState.blue_review)
reviewer = _make_user("blue_lead")
db = _make_db()
from app.services.test_workflow_service import flag_blue_review_gap
with pytest.raises(InvalidOperationError):
flag_blue_review_gap(db, test, reviewer, system_gaps="")
@patch("app.services.test_workflow_service.log_action")
def test_flag_blue_review_gap_moves_to_in_review(self, mock_log):
test = _make_test(TestState.blue_review)
reviewer = _make_user("blue_lead")
db = _make_db()
from app.services.test_workflow_service import flag_blue_review_gap
result = flag_blue_review_gap(db, test, reviewer, system_gaps="Missing EDR agent on host X")
assert result.state == TestState.in_review
assert result.system_gaps == "Missing EDR agent on host X"
# ===========================================================================
# 13. select_reviewer — load-balanced reviewer assignment
# ===========================================================================
class TestReviewerSelection:
"""Uses the real sqlite `db` fixture from conftest.py (not MagicMock),
since load-balancing needs real COUNT() queries."""
def _make_lead(self, db, username, role="red_lead"):
from app.models.user import User
u = User(username=username, role=role, hashed_password="x", is_active=True)
db.add(u)
db.flush()
return u
def _make_technique(self, db, mitre_id="T1059"):
from app.models.technique import Technique
t = Technique(mitre_id=mitre_id, name="Command Line", tactic="execution")
db.add(t)
db.flush()
return t
def test_picks_lead_with_fewest_active_reviews(self, db):
from app.services.test_workflow_service import select_reviewer
lead_a = self._make_lead(db, "reda_selrev")
lead_b = self._make_lead(db, "redb_selrev")
tech = self._make_technique(db, "T1059.selrev1")
busy_test = Test(
technique_id=tech.id, name="Busy",
state=TestState.red_review, red_reviewer_assignee=lead_a.id,
)
db.add(busy_test)
db.commit()
chosen = select_reviewer(db, role="red_lead")
assert chosen.id == lead_b.id
def test_excludes_the_submitter_if_they_are_a_lead(self, db):
from app.services.test_workflow_service import select_reviewer
from app.domain.exceptions import BusinessRuleViolation
lead_a = self._make_lead(db, "reda_excl")
db.commit()
with pytest.raises(BusinessRuleViolation, match="No available"):
select_reviewer(db, role="red_lead", exclude_user_id=lead_a.id)
def test_no_candidates_raises_clear_error(self, db):
from app.services.test_workflow_service import select_reviewer
from app.domain.exceptions import BusinessRuleViolation
with pytest.raises(BusinessRuleViolation, match="No available"):
select_reviewer(db, role="red_lead")
def test_ties_broken_by_username(self, db):
from app.services.test_workflow_service import select_reviewer
self._make_lead(db, "zzz_tie", role="blue_lead")
self._make_lead(db, "aaa_tie", role="blue_lead")
db.commit()
chosen = select_reviewer(db, role="blue_lead")
assert chosen.username == "aaa_tie"
if __name__ == "__main__":
print("T-125 Validation: Workflow Tests")
print("=" * 55)