Files
review-bot/review/formatter_test.go
T
Rodin d33a45329c
PR Ready Gate / clear-labels (pull_request) Successful in 1s
CI / test (pull_request) Successful in 12s
CI / review (/anthropic/v1, anthropic--claude-4.6-sonnet, sonnet, anthropic, SONNET_REVIEW_TOKEN) (pull_request) Successful in 37s
CI / review (/openai/v1, gpt-5, gpt, openai, GPT_REVIEW_TOKEN) (pull_request) Successful in 1m20s
CI / review (/openai/v1, gpt-5, security, openai, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 1m21s
feat(persona): add role-based review personas (#51)
Implement role-based review personas that provide specialized review focus:
- Security: vulnerabilities, auth, secrets, injection attacks
- Architect: design patterns, code organization, API contracts
- Docs: documentation quality, API clarity, error messages

Changes:
- Add persona loading from JSON files and embedded built-ins
- Add --persona and --persona-file CLI flags (mutually exclusive)
- Add BuildPersonaSystemPrompt for persona-specific prompts
- Add FormatMarkdownWithDisplay for persona display names
- Update action.yml with persona and persona-file inputs
- Add comprehensive tests for all new functionality
- Document personas in README with examples

The persona system replaces the generic 'You are an expert code reviewer'
prompt with domain-specific identity, focus areas, ignore list, and
severity calibration. This reduces redundancy between multiple reviewers
and catches domain-specific issues that generic reviewers miss.

Closes #51
2026-05-10 08:42:26 -07:00

217 lines
6.1 KiB
Go

package review
import (
"strings"
"testing"
)
func TestFormatMarkdown_EmptyFindings(t *testing.T) {
result := &ReviewResult{
Verdict: "APPROVE",
Summary: "All good, no issues.",
Findings: []Finding{},
Recommendation: "Merge this PR.",
}
got := FormatMarkdown(result, "Sonnet")
if !strings.Contains(got, "## Summary") {
t.Error("expected Summary header")
}
if !strings.Contains(got, "All good, no issues.") {
t.Error("expected summary text")
}
if strings.Contains(got, "## Findings") {
t.Error("should not contain Findings header when empty")
}
if !strings.Contains(got, "**APPROVE**") {
t.Error("expected verdict in recommendation")
}
if !strings.Contains(got, "Review by Sonnet") {
t.Error("expected reviewer name")
}
}
func TestFormatMarkdown_MultipleFindings(t *testing.T) {
result := &ReviewResult{
Verdict: "REQUEST_CHANGES",
Summary: "Several issues found.",
Findings: []Finding{
{Severity: "MAJOR", File: "main.go", Line: 42, Finding: "Nil pointer dereference"},
{Severity: "MINOR", File: "util.go", Line: 7, Finding: "Unused variable"},
{Severity: "NIT", File: "doc.go", Line: 1, Finding: "Typo in comment"},
},
Recommendation: "Fix the nil pointer issue before merging.",
}
got := FormatMarkdown(result, "GPT")
if !strings.Contains(got, "## Findings") {
t.Error("expected Findings header")
}
if !strings.Contains(got, "| 1 | [MAJOR] | `main.go` | 42 | Nil pointer dereference |") {
t.Error("expected first finding row")
}
if !strings.Contains(got, "| 2 | [MINOR] | `util.go` | 7 | Unused variable |") {
t.Error("expected second finding row")
}
if !strings.Contains(got, "| 3 | [NIT] | `doc.go` | 1 | Typo in comment |") {
t.Error("expected third finding row")
}
if !strings.Contains(got, "**REQUEST_CHANGES**") {
t.Error("expected verdict in recommendation")
}
}
func TestFormatMarkdown_NoReviewerName(t *testing.T) {
result := &ReviewResult{
Verdict: "APPROVE",
Summary: "Fine.",
Findings: []Finding{},
Recommendation: "Go ahead.",
}
got := FormatMarkdown(result, "")
if strings.Contains(got, "Review by") {
t.Error("should not contain reviewer line when name is empty")
}
}
func TestFormatMarkdown_SpecialChars(t *testing.T) {
result := &ReviewResult{
Verdict: "REQUEST_CHANGES",
Summary: "Issues with `fmt.Sprintf` usage.",
Findings: []Finding{
{Severity: "MAJOR", File: "render.go", Line: 15, Finding: "Use `%v` instead of `%s` for interface{}"},
},
Recommendation: "Fix the format verb.",
}
got := FormatMarkdown(result, "Test")
// Should contain the backtick content without breaking the table
if !strings.Contains(got, "`render.go`") {
t.Error("expected file in backticks")
}
if !strings.Contains(got, "Use `%v` instead of `%s` for interface{}") {
t.Error("expected finding text with backticks preserved")
}
}
func TestGiteaEvent(t *testing.T) {
tests := []struct {
verdict string
expected string
}{
{"APPROVE", "APPROVED"},
{"REQUEST_CHANGES", "REQUEST_CHANGES"},
{"UNKNOWN", "COMMENT"},
{"", "COMMENT"},
}
for _, tc := range tests {
got := GiteaEvent(tc.verdict)
if got != tc.expected {
t.Errorf("GiteaEvent(%q) = %q, want %q", tc.verdict, got, tc.expected)
}
}
}
func TestFormatMarkdown_Sentinel(t *testing.T) {
result := &ReviewResult{
Verdict: "APPROVE",
Summary: "All good.",
Recommendation: "Merge it.",
}
output := FormatMarkdown(result, "security")
if !strings.Contains(output, "<!-- review-bot:security -->") {
t.Error("expected sentinel comment in output")
}
// Empty reviewer name should NOT have sentinel
output2 := FormatMarkdown(result, "")
if strings.Contains(output2, "<!-- review-bot") {
t.Error("should not contain sentinel when reviewer name is empty")
}
}
func TestFormatMarkdown_RoleTitle(t *testing.T) {
result := &ReviewResult{
Verdict: "APPROVE",
Summary: "All good.",
Recommendation: "Merge it.",
}
// With reviewer name: should have title header
output := FormatMarkdown(result, "security")
if !strings.Contains(output, "# Security Review\n") {
t.Error("expected '# Security Review' header when reviewer name is set")
}
output2 := FormatMarkdown(result, "gpt")
if !strings.Contains(output2, "# Gpt Review\n") {
t.Error("expected '# Gpt Review' header")
}
// Without reviewer name: no title header
output3 := FormatMarkdown(result, "")
if strings.Contains(output3, "# ") && strings.Contains(output3, " Review\n") {
t.Error("should not contain role title header when reviewer name is empty")
}
}
func TestFormatMarkdownWithDisplay(t *testing.T) {
result := &ReviewResult{
Verdict: "APPROVE",
Summary: "Test summary",
Findings: nil,
Recommendation: "Test recommendation",
}
t.Run("with display name", func(t *testing.T) {
body := FormatMarkdownWithDisplay(result, "Security Specialist", "security")
// Header should use display name
if !strings.Contains(body, "# Security Specialist Review") {
t.Error("header should use display name")
}
// Sentinel should use sentinel name
if !strings.Contains(body, "<!-- review-bot:security -->") {
t.Error("sentinel should use sentinel name")
}
// Footer "Review by" should use display name
if !strings.Contains(body, "*Review by Security Specialist*") {
t.Error("footer should use display name")
}
})
t.Run("without display name", func(t *testing.T) {
body := FormatMarkdownWithDisplay(result, "", "reviewer")
// Should fall back to sentinel name for header
if !strings.Contains(body, "# Reviewer Review") {
t.Error("header should fall back to sentinel name")
}
if !strings.Contains(body, "<!-- review-bot:reviewer -->") {
t.Error("sentinel should use sentinel name")
}
})
t.Run("empty both names", func(t *testing.T) {
body := FormatMarkdownWithDisplay(result, "", "")
// Should not have header
if strings.Contains(body, "# ") && strings.Contains(body, " Review") {
t.Error("should not have header when both names empty")
}
// Should not have sentinel
if strings.Contains(body, "<!-- review-bot:") {
t.Error("should not have sentinel when sentinel name empty")
}
})
}