Add unit tests, integration test, CI workflow, and conventions

- gitea/client_test.go: mock HTTP tests for all API methods + error cases - llm/client_test.go: mock tests for completion, errors, timeouts - review/parser_test.go: JSON parsing, markdown fences, validation - review/formatter_test.go: markdown output, empty/multiple findings - review/prompt_test.go: system/user prompt construction - integration_test.go: full end-to-end flow (build tag: integration) - .gitea/workflows/ci.yml: test + vet + build on push, dual LLM review on PRs - CONVENTIONS.md: coding standards for self-review dogfooding - README.md: usage docs, env vars, architecture
2026-05-01 10:03:44 -07:00
parent 700f186023
commit 3c536c42d5
9 changed files with 869 additions and 34 deletions
@@ -0,0 +1,118 @@
+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)
+		}
+	}
+}
@@ -0,0 +1,114 @@
+package review
+
+import (
+	"testing"
+)
+
+func TestParseResponse_ValidJSON(t *testing.T) {
+	input := `{
+		"verdict": "APPROVE",
+		"summary": "Looks good",
+		"findings": [
+			{"severity": "NIT", "file": "main.go", "line": 10, "finding": "Consider renaming"}
+		],
+		"recommendation": "Ship it"
+	}`
+
+	result, err := ParseResponse(input)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if result.Verdict != "APPROVE" {
+		t.Errorf("expected verdict APPROVE, got %q", result.Verdict)
+	}
+	if result.Summary != "Looks good" {
+		t.Errorf("expected summary %q, got %q", "Looks good", result.Summary)
+	}
+	if len(result.Findings) != 1 {
+		t.Fatalf("expected 1 finding, got %d", len(result.Findings))
+	}
+	if result.Findings[0].Severity != "NIT" {
+		t.Errorf("expected severity NIT, got %q", result.Findings[0].Severity)
+	}
+	if result.Findings[0].File != "main.go" {
+		t.Errorf("expected file main.go, got %q", result.Findings[0].File)
+	}
+	if result.Findings[0].Line != 10 {
+		t.Errorf("expected line 10, got %d", result.Findings[0].Line)
+	}
+	if result.Recommendation != "Ship it" {
+		t.Errorf("expected recommendation %q, got %q", "Ship it", result.Recommendation)
+	}
+}
+
+func TestParseResponse_MarkdownFences(t *testing.T) {
+	input := "```json\n{\"verdict\": \"REQUEST_CHANGES\", \"summary\": \"Issues found\", \"findings\": [{\"severity\": \"MAJOR\", \"file\": \"a.go\", \"line\": 5, \"finding\": \"Bug\"}], \"recommendation\": \"Fix it\"}\n```"
+
+	result, err := ParseResponse(input)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if result.Verdict != "REQUEST_CHANGES" {
+		t.Errorf("expected verdict REQUEST_CHANGES, got %q", result.Verdict)
+	}
+	if len(result.Findings) != 1 {
+		t.Fatalf("expected 1 finding, got %d", len(result.Findings))
+	}
+	if result.Findings[0].Severity != "MAJOR" {
+		t.Errorf("expected severity MAJOR, got %q", result.Findings[0].Severity)
+	}
+}
+
+func TestParseResponse_InvalidJSON(t *testing.T) {
+	_, err := ParseResponse("this is not json")
+	if err == nil {
+		t.Fatal("expected error for invalid JSON, got nil")
+	}
+}
+
+func TestParseResponse_InvalidVerdict(t *testing.T) {
+	input := `{"verdict": "MAYBE", "summary": "Hmm", "findings": [], "recommendation": "Dunno"}`
+	_, err := ParseResponse(input)
+	if err == nil {
+		t.Fatal("expected error for invalid verdict, got nil")
+	}
+}
+
+func TestParseResponse_InvalidSeverity(t *testing.T) {
+	input := `{"verdict": "APPROVE", "summary": "Ok", "findings": [{"severity": "CRITICAL", "file": "x.go", "line": 1, "finding": "bad"}], "recommendation": "Fix"}`
+	_, err := ParseResponse(input)
+	if err == nil {
+		t.Fatal("expected error for invalid severity, got nil")
+	}
+}
+
+func TestParseResponse_EmptyFindings(t *testing.T) {
+	input := `{"verdict": "APPROVE", "summary": "All good", "findings": [], "recommendation": "Merge"}`
+	result, err := ParseResponse(input)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if len(result.Findings) != 0 {
+		t.Errorf("expected 0 findings, got %d", len(result.Findings))
+	}
+}
+
+func TestParseResponse_MissingFields(t *testing.T) {
+	// verdict is empty string — should fail validation
+	input := `{"summary": "Ok", "findings": [], "recommendation": "Merge"}`
+	_, err := ParseResponse(input)
+	if err == nil {
+		t.Fatal("expected error for missing verdict, got nil")
+	}
+}
+
+func TestParseResponse_MarkdownFencesNoLang(t *testing.T) {
+	input := "```\n{\"verdict\": \"APPROVE\", \"summary\": \"Fine\", \"findings\": [], \"recommendation\": \"Good\"}\n```"
+	result, err := ParseResponse(input)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if result.Verdict != "APPROVE" {
+		t.Errorf("expected APPROVE, got %q", result.Verdict)
+	}
+}
@@ -0,0 +1,77 @@
+package review
+
+import (
+	"strings"
+	"testing"
+)
+
+func TestBuildSystemPrompt_NoConventions(t *testing.T) {
+	prompt := BuildSystemPrompt("")
+
+	if !strings.Contains(prompt, "expert code reviewer") {
+		t.Error("expected system prompt to mention code reviewer role")
+	}
+	if strings.Contains(prompt, "coding conventions") {
+		t.Error("should not mention conventions when empty")
+	}
+}
+
+func TestBuildSystemPrompt_WithConventions(t *testing.T) {
+	conventions := "- Use stdlib only\n- No panics\n"
+	prompt := BuildSystemPrompt(conventions)
+
+	if !strings.Contains(prompt, "coding conventions") {
+		t.Error("expected conventions section")
+	}
+	if !strings.Contains(prompt, "Use stdlib only") {
+		t.Error("expected conventions content")
+	}
+}
+
+func TestBuildUserPrompt_Basic(t *testing.T) {
+	prompt := BuildUserPrompt("Fix bug", "Fixes the crash", "diff content here", true, "all checks passed")
+
+	if !strings.Contains(prompt, "Fix bug") {
+		t.Error("expected PR title")
+	}
+	if !strings.Contains(prompt, "Fixes the crash") {
+		t.Error("expected PR description")
+	}
+	if !strings.Contains(prompt, "diff content here") {
+		t.Error("expected diff content")
+	}
+	if !strings.Contains(prompt, "PASSED") {
+		t.Error("expected CI status PASSED")
+	}
+}
+
+func TestBuildUserPrompt_CIFailed(t *testing.T) {
+	prompt := BuildUserPrompt("Add tests", "", "some diff", false, "lint: failed")
+
+	if !strings.Contains(prompt, "FAILED") {
+		t.Error("expected CI status FAILED")
+	}
+	if !strings.Contains(prompt, "lint: failed") {
+		t.Error("expected CI details")
+	}
+}
+
+func TestBuildUserPrompt_NoDescription(t *testing.T) {
+	prompt := BuildUserPrompt("Quick fix", "", "diff", true, "")
+
+	if strings.Contains(prompt, "### Description") {
+		t.Error("should not contain Description header when body is empty")
+	}
+}
+
+func TestBuildUserPrompt_DiffIncluded(t *testing.T) {
+	diff := "+func Hello() string {\n+\treturn \"hello\"\n+}"
+	prompt := BuildUserPrompt("Greeting", "Add greeting func", diff, true, "")
+
+	if !strings.Contains(prompt, "```diff") {
+		t.Error("expected diff fence")
+	}
+	if !strings.Contains(prompt, diff) {
+		t.Error("expected diff content in prompt")
+	}
+}