600 lines
20 KiB
Python
600 lines
20 KiB
Python
"""Tests for platform integration clients."""
|
|
|
|
import pytest
|
|
import respx
|
|
from httpx import Response
|
|
|
|
from arbiter.deliberation import Conflict, ConflictNature, DeliberationResult
|
|
from arbiter.integrations import (
|
|
ARBITER_MARKER,
|
|
AuthenticationError,
|
|
CommitStatus,
|
|
GitHubClient,
|
|
GitLabClient,
|
|
NotFoundError,
|
|
Platform,
|
|
PlatformError,
|
|
RateLimitError,
|
|
ReviewCommentFormatter,
|
|
has_arbiter_marker,
|
|
)
|
|
from arbiter.models import AgentName, Finding, Severity, Verdict
|
|
from arbiter.worker.tasks import detect_platform
|
|
|
|
|
|
class TestGitHubClient:
|
|
@pytest.fixture
|
|
def client(self) -> GitHubClient:
|
|
return GitHubClient(token="test-token", max_retries=1)
|
|
|
|
@respx.mock
|
|
@pytest.mark.asyncio
|
|
async def test_get_pr_diff(self, client: GitHubClient) -> None:
|
|
diff_content = "--- a/file.py\n+++ b/file.py\n@@ -1 +1 @@\n-old\n+new"
|
|
respx.get("https://api.github.com/repos/owner/repo/pulls/1").mock(
|
|
return_value=Response(200, text=diff_content)
|
|
)
|
|
|
|
diff = await client.get_pr_diff("owner/repo", 1)
|
|
|
|
assert diff == diff_content
|
|
await client.close()
|
|
|
|
@respx.mock
|
|
@pytest.mark.asyncio
|
|
async def test_post_comment(self, client: GitHubClient) -> None:
|
|
respx.post("https://api.github.com/repos/owner/repo/issues/1/comments").mock(
|
|
return_value=Response(
|
|
201,
|
|
json={"html_url": "https://github.com/owner/repo/pull/1#comment-123"},
|
|
)
|
|
)
|
|
|
|
url = await client.post_comment("owner/repo", 1, "Test comment")
|
|
|
|
assert url == "https://github.com/owner/repo/pull/1#comment-123"
|
|
await client.close()
|
|
|
|
@respx.mock
|
|
@pytest.mark.asyncio
|
|
async def test_update_commit_status(self, client: GitHubClient) -> None:
|
|
sha = "abc123def456"
|
|
respx.post(f"https://api.github.com/repos/owner/repo/statuses/{sha}").mock(
|
|
return_value=Response(201, json={"state": "pending"})
|
|
)
|
|
|
|
await client.update_commit_status(
|
|
"owner/repo",
|
|
sha,
|
|
CommitStatus.PENDING,
|
|
"Review in progress",
|
|
"arbiter",
|
|
)
|
|
|
|
request = respx.calls.last.request
|
|
assert request is not None
|
|
await client.close()
|
|
|
|
@respx.mock
|
|
@pytest.mark.asyncio
|
|
async def test_get_pr_info(self, client: GitHubClient) -> None:
|
|
respx.get("https://api.github.com/repos/owner/repo/pulls/1").mock(
|
|
return_value=Response(
|
|
200,
|
|
json={
|
|
"head": {"sha": "abc123", "ref": "feature"},
|
|
"base": {"sha": "def456", "ref": "main"},
|
|
"title": "Test PR",
|
|
"user": {"login": "testuser"},
|
|
"html_url": "https://github.com/owner/repo/pull/1",
|
|
"draft": False,
|
|
},
|
|
)
|
|
)
|
|
|
|
info = await client.get_pr_info("owner/repo", 1)
|
|
|
|
assert info.platform == Platform.GITHUB
|
|
assert info.repository == "owner/repo"
|
|
assert info.pr_number == 1
|
|
assert info.head_sha == "abc123"
|
|
assert info.base_sha == "def456"
|
|
assert info.title == "Test PR"
|
|
assert info.author == "testuser"
|
|
await client.close()
|
|
|
|
@respx.mock
|
|
@pytest.mark.asyncio
|
|
async def test_404_raises_not_found(self, client: GitHubClient) -> None:
|
|
respx.get("https://api.github.com/repos/owner/repo/pulls/999").mock(
|
|
return_value=Response(404, json={"message": "Not Found"})
|
|
)
|
|
|
|
with pytest.raises(NotFoundError):
|
|
await client.get_pr_diff("owner/repo", 999)
|
|
|
|
await client.close()
|
|
|
|
@respx.mock
|
|
@pytest.mark.asyncio
|
|
async def test_401_raises_authentication_error(self, client: GitHubClient) -> None:
|
|
respx.get("https://api.github.com/repos/owner/repo/pulls/1").mock(
|
|
return_value=Response(401, json={"message": "Bad credentials"})
|
|
)
|
|
|
|
with pytest.raises(AuthenticationError):
|
|
await client.get_pr_diff("owner/repo", 1)
|
|
|
|
await client.close()
|
|
|
|
@respx.mock
|
|
@pytest.mark.asyncio
|
|
async def test_429_raises_rate_limit_error(self, client: GitHubClient) -> None:
|
|
respx.get("https://api.github.com/repos/owner/repo/pulls/1").mock(
|
|
return_value=Response(
|
|
429,
|
|
headers={"Retry-After": "60"},
|
|
json={"message": "Rate limit exceeded"},
|
|
)
|
|
)
|
|
|
|
with pytest.raises(RateLimitError) as exc_info:
|
|
await client.get_pr_diff("owner/repo", 1)
|
|
|
|
assert exc_info.value.retry_after == 60
|
|
await client.close()
|
|
|
|
@respx.mock
|
|
@pytest.mark.asyncio
|
|
async def test_500_raises_platform_error(self, client: GitHubClient) -> None:
|
|
respx.get("https://api.github.com/repos/owner/repo/pulls/1").mock(
|
|
return_value=Response(500, json={"message": "Internal Server Error"})
|
|
)
|
|
|
|
from tenacity import RetryError
|
|
|
|
with pytest.raises(RetryError) as exc_info:
|
|
await client.get_pr_diff("owner/repo", 1)
|
|
|
|
# Verify the underlying exception is PlatformError
|
|
assert exc_info.value.last_attempt.exception() is not None
|
|
assert isinstance(exc_info.value.last_attempt.exception(), PlatformError)
|
|
|
|
await client.close()
|
|
|
|
@respx.mock
|
|
@pytest.mark.asyncio
|
|
async def test_context_manager(self) -> None:
|
|
respx.get("https://api.github.com/repos/owner/repo/pulls/1").mock(
|
|
return_value=Response(200, text="diff content")
|
|
)
|
|
|
|
async with GitHubClient(token="test") as client:
|
|
diff = await client.get_pr_diff("owner/repo", 1)
|
|
assert diff == "diff content"
|
|
|
|
@respx.mock
|
|
@pytest.mark.asyncio
|
|
async def test_get_comments(self, client: GitHubClient) -> None:
|
|
respx.get("https://api.github.com/repos/owner/repo/issues/1/comments").mock(
|
|
return_value=Response(
|
|
200,
|
|
json=[
|
|
{
|
|
"id": 123,
|
|
"body": "Test comment",
|
|
"user": {"login": "testuser"},
|
|
"html_url": "https://github.com/owner/repo/pull/1#comment-123",
|
|
"created_at": "2025-05-15T10:30:00Z",
|
|
},
|
|
{
|
|
"id": 456,
|
|
"body": f"Arbiter review {ARBITER_MARKER}",
|
|
"user": {"login": "arbiter-bot"},
|
|
"html_url": "https://github.com/owner/repo/pull/1#comment-456",
|
|
"created_at": "2025-05-15T11:00:00Z",
|
|
},
|
|
],
|
|
)
|
|
)
|
|
|
|
comments = await client.get_comments("owner/repo", 1)
|
|
|
|
assert len(comments) == 2
|
|
assert comments[0].id == "123"
|
|
assert comments[0].body == "Test comment"
|
|
assert comments[0].author == "testuser"
|
|
assert comments[1].id == "456"
|
|
assert has_arbiter_marker(comments[1].body)
|
|
await client.close()
|
|
|
|
@respx.mock
|
|
@pytest.mark.asyncio
|
|
async def test_update_comment(self, client: GitHubClient) -> None:
|
|
respx.patch("https://api.github.com/repos/owner/repo/issues/comments/123").mock(
|
|
return_value=Response(
|
|
200,
|
|
json={"html_url": "https://github.com/owner/repo/pull/1#comment-123"},
|
|
)
|
|
)
|
|
|
|
url = await client.update_comment("owner/repo", 1, "123", "Updated content")
|
|
|
|
assert url == "https://github.com/owner/repo/pull/1#comment-123"
|
|
request = respx.calls.last.request
|
|
assert request is not None
|
|
await client.close()
|
|
|
|
|
|
class TestGitLabClient:
|
|
@pytest.fixture
|
|
def client(self) -> GitLabClient:
|
|
return GitLabClient(token="test-token", max_retries=1)
|
|
|
|
@respx.mock
|
|
@pytest.mark.asyncio
|
|
async def test_get_pr_diff(self, client: GitLabClient) -> None:
|
|
respx.get("https://gitlab.com/api/v4/projects/owner%2Frepo/merge_requests/1/diffs").mock(
|
|
return_value=Response(
|
|
200,
|
|
json=[
|
|
{
|
|
"old_path": "file.py",
|
|
"new_path": "file.py",
|
|
"diff": "@@ -1 +1 @@\n-old\n+new",
|
|
}
|
|
],
|
|
)
|
|
)
|
|
|
|
diff = await client.get_pr_diff("owner/repo", 1)
|
|
|
|
assert "file.py" in diff
|
|
assert "-old" in diff
|
|
assert "+new" in diff
|
|
await client.close()
|
|
|
|
@respx.mock
|
|
@pytest.mark.asyncio
|
|
async def test_post_comment(self, client: GitLabClient) -> None:
|
|
respx.post("https://gitlab.com/api/v4/projects/owner%2Frepo/merge_requests/1/notes").mock(
|
|
return_value=Response(201, json={"id": 123})
|
|
)
|
|
|
|
url = await client.post_comment("owner/repo", 1, "Test comment")
|
|
|
|
assert "owner/repo" in url
|
|
assert "merge_requests/1" in url
|
|
assert "note_123" in url
|
|
await client.close()
|
|
|
|
@respx.mock
|
|
@pytest.mark.asyncio
|
|
async def test_update_commit_status(self, client: GitLabClient) -> None:
|
|
sha = "abc123def456"
|
|
respx.post(f"https://gitlab.com/api/v4/projects/owner%2Frepo/statuses/{sha}").mock(
|
|
return_value=Response(201, json={"status": "pending"})
|
|
)
|
|
|
|
await client.update_commit_status(
|
|
"owner/repo",
|
|
sha,
|
|
CommitStatus.PENDING,
|
|
"Review in progress",
|
|
"arbiter",
|
|
)
|
|
|
|
assert respx.calls.last is not None
|
|
await client.close()
|
|
|
|
@respx.mock
|
|
@pytest.mark.asyncio
|
|
async def test_get_pr_info(self, client: GitLabClient) -> None:
|
|
respx.get("https://gitlab.com/api/v4/projects/owner%2Frepo/merge_requests/1").mock(
|
|
return_value=Response(
|
|
200,
|
|
json={
|
|
"sha": "abc123",
|
|
"diff_refs": {"head_sha": "abc123", "base_sha": "def456"},
|
|
"source_branch": "feature",
|
|
"target_branch": "main",
|
|
"title": "Test MR",
|
|
"author": {"username": "testuser"},
|
|
"web_url": "https://gitlab.com/owner/repo/-/merge_requests/1",
|
|
"draft": False,
|
|
},
|
|
)
|
|
)
|
|
|
|
info = await client.get_pr_info("owner/repo", 1)
|
|
|
|
assert info.platform == Platform.GITLAB
|
|
assert info.repository == "owner/repo"
|
|
assert info.pr_number == 1
|
|
assert info.head_sha == "abc123"
|
|
assert info.title == "Test MR"
|
|
assert info.author == "testuser"
|
|
await client.close()
|
|
|
|
@respx.mock
|
|
@pytest.mark.asyncio
|
|
async def test_404_raises_not_found(self, client: GitLabClient) -> None:
|
|
respx.get("https://gitlab.com/api/v4/projects/owner%2Frepo/merge_requests/999/diffs").mock(
|
|
return_value=Response(404, json={"message": "404 Not found"})
|
|
)
|
|
|
|
with pytest.raises(NotFoundError):
|
|
await client.get_pr_diff("owner/repo", 999)
|
|
|
|
await client.close()
|
|
|
|
@respx.mock
|
|
@pytest.mark.asyncio
|
|
async def test_get_comments(self, client: GitLabClient) -> None:
|
|
respx.get("https://gitlab.com/api/v4/projects/owner%2Frepo/merge_requests/1/notes").mock(
|
|
return_value=Response(
|
|
200,
|
|
json=[
|
|
{
|
|
"id": 123,
|
|
"body": "Test note",
|
|
"author": {"username": "testuser"},
|
|
"created_at": "2025-05-15T10:30:00Z",
|
|
},
|
|
{
|
|
"id": 456,
|
|
"body": f"Arbiter review {ARBITER_MARKER}",
|
|
"author": {"username": "arbiter-bot"},
|
|
"created_at": "2025-05-15T11:00:00Z",
|
|
},
|
|
],
|
|
)
|
|
)
|
|
|
|
comments = await client.get_comments("owner/repo", 1)
|
|
|
|
assert len(comments) == 2
|
|
assert comments[0].id == "123"
|
|
assert comments[0].body == "Test note"
|
|
assert comments[0].author == "testuser"
|
|
assert "note_123" in comments[0].url
|
|
assert comments[1].id == "456"
|
|
assert has_arbiter_marker(comments[1].body)
|
|
await client.close()
|
|
|
|
@respx.mock
|
|
@pytest.mark.asyncio
|
|
async def test_update_comment(self, client: GitLabClient) -> None:
|
|
respx.put(
|
|
"https://gitlab.com/api/v4/projects/owner%2Frepo/merge_requests/1/notes/123"
|
|
).mock(return_value=Response(200, json={"id": 123}))
|
|
|
|
url = await client.update_comment("owner/repo", 1, "123", "Updated content")
|
|
|
|
assert "owner/repo" in url
|
|
assert "merge_requests/1" in url
|
|
assert "note_123" in url
|
|
await client.close()
|
|
|
|
|
|
class TestReviewCommentFormatter:
|
|
@pytest.fixture
|
|
def formatter(self) -> ReviewCommentFormatter:
|
|
return ReviewCommentFormatter(include_cost=True)
|
|
|
|
@pytest.fixture
|
|
def sample_finding(self) -> Finding:
|
|
return Finding(
|
|
id="finding-1",
|
|
agent=AgentName.SECURITY,
|
|
file="src/auth.py",
|
|
line_start=10,
|
|
line_end=15,
|
|
severity=Severity.HIGH,
|
|
confidence=0.9,
|
|
title="SQL Injection vulnerability",
|
|
description="User input is directly concatenated into SQL query",
|
|
reasoning="String concatenation allows SQL injection",
|
|
suggestion="Use parameterized queries",
|
|
references=["https://owasp.org/sql-injection"],
|
|
prompt_version="security-v1.0",
|
|
)
|
|
|
|
@pytest.fixture
|
|
def sample_conflict(self) -> Conflict:
|
|
return Conflict(
|
|
id="conflict-1",
|
|
finding_ids=["finding-1", "finding-2"],
|
|
nature=ConflictNature.TRADE_OFF,
|
|
description="Security vs complexity trade-off",
|
|
severity_weight=0.8,
|
|
resolution="Prefer security",
|
|
)
|
|
|
|
def test_format_approve_verdict(self, formatter: ReviewCommentFormatter) -> None:
|
|
result = DeliberationResult(
|
|
findings=[],
|
|
verdict=Verdict.APPROVE,
|
|
verdict_confidence=0.95,
|
|
verdict_reasoning="No issues found",
|
|
total_findings=0,
|
|
)
|
|
|
|
comment = formatter.format(result)
|
|
|
|
assert "Approve" in comment
|
|
assert "95%" in comment
|
|
assert "No issues found" in comment
|
|
|
|
def test_format_request_changes_verdict(
|
|
self, formatter: ReviewCommentFormatter, sample_finding: Finding
|
|
) -> None:
|
|
result = DeliberationResult(
|
|
findings=[sample_finding],
|
|
verdict=Verdict.REQUEST_CHANGES,
|
|
verdict_confidence=0.9,
|
|
verdict_reasoning="Found critical security issue",
|
|
total_findings=1,
|
|
critical_count=0,
|
|
high_count=1,
|
|
)
|
|
|
|
comment = formatter.format(result)
|
|
|
|
assert "Request Changes" in comment
|
|
assert "SQL Injection" in comment
|
|
assert "src/auth.py" in comment
|
|
assert "High" in comment
|
|
|
|
def test_format_with_conflicts(
|
|
self,
|
|
formatter: ReviewCommentFormatter,
|
|
sample_finding: Finding,
|
|
sample_conflict: Conflict,
|
|
) -> None:
|
|
result = DeliberationResult(
|
|
findings=[sample_finding],
|
|
conflicts=[sample_conflict],
|
|
verdict=Verdict.COMMENT,
|
|
verdict_confidence=0.7,
|
|
verdict_reasoning="Issues need discussion",
|
|
total_findings=1,
|
|
)
|
|
|
|
comment = formatter.format(result)
|
|
|
|
assert "Conflicts" in comment
|
|
assert "Trade Off" in comment
|
|
assert "Security vs complexity" in comment
|
|
|
|
def test_format_includes_cost(self, formatter: ReviewCommentFormatter) -> None:
|
|
result = DeliberationResult(
|
|
findings=[],
|
|
verdict=Verdict.APPROVE,
|
|
verdict_confidence=0.95,
|
|
verdict_reasoning="No issues",
|
|
tokens_used=1000,
|
|
cost_usd=0.05,
|
|
)
|
|
|
|
comment = formatter.format(result)
|
|
|
|
assert "1,000" in comment # tokens with comma
|
|
assert "$0.0500" in comment
|
|
|
|
def test_format_without_cost(self) -> None:
|
|
formatter = ReviewCommentFormatter(include_cost=False)
|
|
result = DeliberationResult(
|
|
findings=[],
|
|
verdict=Verdict.APPROVE,
|
|
verdict_confidence=0.95,
|
|
verdict_reasoning="No issues",
|
|
tokens_used=1000,
|
|
cost_usd=0.05,
|
|
)
|
|
|
|
comment = formatter.format(result)
|
|
|
|
assert "Tokens used" not in comment
|
|
assert "$0.05" not in comment
|
|
|
|
def test_format_truncates_long_comment(
|
|
self, formatter: ReviewCommentFormatter, sample_finding: Finding
|
|
) -> None:
|
|
# Create many findings
|
|
findings = []
|
|
for i in range(100):
|
|
f = sample_finding.model_copy()
|
|
f.id = f"finding-{i}"
|
|
f.description = "x" * 1000 # Long description
|
|
findings.append(f)
|
|
|
|
result = DeliberationResult(
|
|
findings=findings,
|
|
verdict=Verdict.REQUEST_CHANGES,
|
|
verdict_confidence=0.9,
|
|
verdict_reasoning="Many issues",
|
|
total_findings=100,
|
|
)
|
|
|
|
comment = formatter.format(result)
|
|
|
|
assert len(comment) <= ReviewCommentFormatter.MAX_COMMENT_LENGTH
|
|
|
|
def test_format_limits_findings(self) -> None:
|
|
formatter = ReviewCommentFormatter(max_findings=5)
|
|
findings = [
|
|
Finding(
|
|
id=f"finding-{i}",
|
|
agent=AgentName.STYLE,
|
|
file=f"file{i}.py",
|
|
line_start=1,
|
|
line_end=1,
|
|
severity=Severity.LOW,
|
|
confidence=0.5,
|
|
title=f"Issue {i}",
|
|
description="Description",
|
|
reasoning="Reasoning",
|
|
prompt_version="style-v1.0",
|
|
)
|
|
for i in range(10)
|
|
]
|
|
|
|
result = DeliberationResult(
|
|
findings=findings,
|
|
verdict=Verdict.COMMENT,
|
|
verdict_confidence=0.7,
|
|
verdict_reasoning="Multiple issues",
|
|
total_findings=10,
|
|
)
|
|
|
|
comment = formatter.format(result)
|
|
|
|
assert "more findings" in comment
|
|
|
|
def test_format_includes_marker(self, formatter: ReviewCommentFormatter) -> None:
|
|
result = DeliberationResult(
|
|
findings=[],
|
|
verdict=Verdict.APPROVE,
|
|
verdict_confidence=0.95,
|
|
verdict_reasoning="No issues found",
|
|
total_findings=0,
|
|
)
|
|
|
|
comment = formatter.format(result)
|
|
|
|
assert ARBITER_MARKER in comment
|
|
assert has_arbiter_marker(comment)
|
|
|
|
|
|
class TestArbiterMarker:
|
|
def test_has_marker_returns_true(self) -> None:
|
|
text = f"Some comment\n{ARBITER_MARKER}\nMore text"
|
|
assert has_arbiter_marker(text) is True
|
|
|
|
def test_has_marker_returns_false(self) -> None:
|
|
text = "Some comment without marker"
|
|
assert has_arbiter_marker(text) is False
|
|
|
|
def test_marker_is_html_comment(self) -> None:
|
|
assert ARBITER_MARKER.startswith("<!--")
|
|
assert ARBITER_MARKER.endswith("-->")
|
|
|
|
|
|
class TestPlatformDetection:
|
|
def test_detect_github_from_webhook(self) -> None:
|
|
platform = detect_platform("owner/repo", "github")
|
|
assert platform == Platform.GITHUB
|
|
|
|
def test_detect_gitlab_from_webhook(self) -> None:
|
|
platform = detect_platform("owner/repo", "gitlab")
|
|
assert platform == Platform.GITLAB
|
|
|
|
def test_detect_github_case_insensitive(self) -> None:
|
|
assert detect_platform("owner/repo", "GitHub") == Platform.GITHUB
|
|
assert detect_platform("owner/repo", "GITLAB") == Platform.GITLAB
|
|
|
|
def test_default_to_github(self) -> None:
|
|
platform = detect_platform("owner/repo", None)
|
|
assert platform == Platform.GITHUB
|