From 3c536c42d5ea4c75b01c320e635bfb8b95f2b5cd Mon Sep 17 00:00:00 2001 From: Rodin Date: Fri, 1 May 2026 10:03:44 -0700 Subject: [PATCH] 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 --- .gitea/workflows/ci.yml | 53 +++++++++++ CONVENTIONS.md | 32 +++++++ README.md | 101 +++++++++++++------- gitea/client_test.go | 195 +++++++++++++++++++++++++++++++++++++++ integration_test.go | 103 +++++++++++++++++++++ llm/client_test.go | 110 ++++++++++++++++++++++ review/formatter_test.go | 118 +++++++++++++++++++++++ review/parser_test.go | 114 +++++++++++++++++++++++ review/prompt_test.go | 77 ++++++++++++++++ 9 files changed, 869 insertions(+), 34 deletions(-) create mode 100644 .gitea/workflows/ci.yml create mode 100644 CONVENTIONS.md create mode 100644 gitea/client_test.go create mode 100644 integration_test.go create mode 100644 llm/client_test.go create mode 100644 review/formatter_test.go create mode 100644 review/parser_test.go create mode 100644 review/prompt_test.go diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml new file mode 100644 index 0000000..e0a5214 --- /dev/null +++ b/.gitea/workflows/ci.yml @@ -0,0 +1,53 @@ +name: CI +on: + push: + branches: [main] + pull_request: + types: [opened, synchronize] + +jobs: + test: + runs-on: ubuntu-24.04 + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version: "1.26" + - run: go test ./... + - run: go vet ./... + - run: go build -o review-bot ./cmd/review-bot + + review: + runs-on: ubuntu-24.04 + if: github.event_name == 'pull_request' + needs: test + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-go@v5 + with: + go-version: "1.26" + - run: go build -o review-bot ./cmd/review-bot + - name: Run Sonnet Review + env: + GITEA_URL: ${{ github.server_url }} + GITEA_REPO: ${{ github.repository }} + PR_NUMBER: ${{ github.event.pull_request.number }} + REVIEWER_TOKEN: ${{ secrets.SONNET_REVIEW_TOKEN }} + LLM_BASE_URL: ${{ secrets.LLM_BASE_URL }} + LLM_API_KEY: ${{ secrets.LLM_API_KEY }} + LLM_MODEL: "anthropic--claude-4.6-sonnet" + CONVENTIONS_FILE: "CONVENTIONS.md" + REVIEWER_NAME: "Sonnet" + run: ./review-bot + - name: Run GPT Review + env: + GITEA_URL: ${{ github.server_url }} + GITEA_REPO: ${{ github.repository }} + PR_NUMBER: ${{ github.event.pull_request.number }} + REVIEWER_TOKEN: ${{ secrets.GPT_REVIEW_TOKEN }} + LLM_BASE_URL: ${{ secrets.LLM_BASE_URL }} + LLM_API_KEY: ${{ secrets.LLM_API_KEY }} + LLM_MODEL: "sap-ai-opus-latest-openai/gpt-5" + CONVENTIONS_FILE: "CONVENTIONS.md" + REVIEWER_NAME: "GPT" + run: ./review-bot diff --git a/CONVENTIONS.md b/CONVENTIONS.md new file mode 100644 index 0000000..ce4406a --- /dev/null +++ b/CONVENTIONS.md @@ -0,0 +1,32 @@ +# Conventions + +## Language & Dependencies + +- Go standard library only — no external dependencies. +- Target the latest stable Go release. + +## Error Handling + +- Return errors; never panic. +- Wrap errors with context using `fmt.Errorf("context: %w", err)`. +- Check all error returns. + +## Testing + +- Test every exported function. +- Use `net/http/httptest` for HTTP mocking. +- Table-driven tests where multiple inputs share the same assertion logic. +- Integration tests use build tags (`//go:build integration`). + +## Style + +- Keep functions short and focused. +- Prefer early returns over deep nesting. +- Meaningful variable names — no single-letter names outside loop indices. +- Comments explain *why*, not *what*. + +## Process + +- `go test ./...` must pass before commit. +- `go vet ./...` must pass before commit. +- Keep commits atomic and well-described. diff --git a/README.md b/README.md index c16aeaf..ff2356a 100644 --- a/README.md +++ b/README.md @@ -1,58 +1,91 @@ # review-bot -AI-powered code review bot for Gitea pull requests. +Automated code review bot for Gitea. Fetches a pull request diff, sends it to an LLM for analysis, and posts a structured review back to the PR. -## Overview +## Features -`review-bot` fetches a PR's diff, title, description, and CI status from Gitea, sends it to an LLM via an OpenAI-compatible API, and posts a structured code review back to Gitea. +- Fetches PR metadata, diff, and CI status from Gitea API +- Sends context-rich prompts to any OpenAI-compatible LLM +- Parses structured JSON review responses +- Posts formatted reviews (APPROVE / REQUEST_CHANGES) back to Gitea +- Supports custom coding conventions via repo files +- Zero external dependencies — Go stdlib only ## Usage ```bash review-bot \ - --gitea-url https://gitea.weiker.me \ + --gitea-url https://gitea.example.com \ --repo owner/name \ - --pr 123 \ - --reviewer-name "sonnet-review-bot" \ - --reviewer-token "$(cat /path/to/token)" \ - --llm-base-url "https://proxy.example.com/v1" \ - --llm-api-key "key" \ - --llm-model "anthropic--claude-4.6-sonnet" \ - --conventions-file "CLAUDE.md" + --pr 42 \ + --reviewer-token "$GITEA_TOKEN" \ + --llm-base-url https://api.openai.com/v1 \ + --llm-api-key "$OPENAI_API_KEY" \ + --llm-model gpt-4 \ + --reviewer-name "Sonnet" \ + --conventions-file CONVENTIONS.md \ + --dry-run ``` -All flags can also be set via environment variables: +## Environment Variables -| Flag | Env Var | -|------|---------| -| `--gitea-url` | `GITEA_URL` | -| `--repo` | `GITEA_REPO` | -| `--pr` | `PR_NUMBER` | -| `--reviewer-name` | `REVIEWER_NAME` | -| `--reviewer-token` | `REVIEWER_TOKEN` | -| `--llm-base-url` | `LLM_BASE_URL` | -| `--llm-api-key` | `LLM_API_KEY` | -| `--llm-model` | `LLM_MODEL` | -| `--conventions-file` | `CONVENTIONS_FILE` | +All flags can be set via environment variables: -Use `--dry-run` to print the review to stdout without posting. +| Flag | Env Var | Required | Description | +|------|---------|----------|-------------| +| `--gitea-url` | `GITEA_URL` | Yes | Gitea instance base URL | +| `--repo` | `GITEA_REPO` | Yes | Repository in `owner/name` format | +| `--pr` | `PR_NUMBER` | Yes | Pull request number | +| `--reviewer-token` | `REVIEWER_TOKEN` | Yes | Gitea API token for posting reviews | +| `--llm-base-url` | `LLM_BASE_URL` | Yes | OpenAI-compatible API base URL | +| `--llm-api-key` | `LLM_API_KEY` | Yes | LLM API key | +| `--llm-model` | `LLM_MODEL` | Yes | Model identifier | +| `--reviewer-name` | `REVIEWER_NAME` | No | Display name in review footer | +| `--conventions-file` | `CONVENTIONS_FILE` | No | Path to conventions file in repo | +| `--dry-run` | — | No | Print review to stdout instead of posting | -## Build +## Adding to a Gitea Repository + +1. Build the binary or use the CI workflow approach (build in CI). + +2. Add secrets to your Gitea repo (Settings → Actions → Secrets): + - `SONNET_REVIEW_TOKEN` — Gitea token for the Sonnet reviewer account + - `GPT_REVIEW_TOKEN` — Gitea token for the GPT reviewer account + - `LLM_BASE_URL` — Your LLM API endpoint + - `LLM_API_KEY` — Your LLM API key + +3. Copy `.gitea/workflows/ci.yml` to your repo (or adapt it). + +4. On every PR, the bot will: + - Run tests and vet + - Build review-bot + - Post reviews from each configured LLM reviewer + +## Development ```bash +# Run tests +go test ./... + +# Run vet +go vet ./... + +# Build go build -o review-bot ./cmd/review-bot + +# Integration tests (requires env vars) +go test -tags=integration ./... ``` ## Architecture -- `cmd/review-bot/main.go` — CLI entry point -- `gitea/client.go` — Gitea API interactions (fetch PR, diff, CI status, post review) -- `llm/client.go` — OpenAI-compatible chat completion client -- `review/prompt.go` — System/user prompt construction -- `review/parser.go` — Parse LLM JSON response -- `review/formatter.go` — Format markdown review body +``` +cmd/review-bot/ CLI entrypoint +gitea/ Gitea API client +llm/ OpenAI-compatible LLM client +review/ Prompt building, response parsing, formatting +``` -## Constraints +## License -- Pure Go stdlib, no external dependencies -- No CGO +MIT diff --git a/gitea/client_test.go b/gitea/client_test.go new file mode 100644 index 0000000..f84b2ee --- /dev/null +++ b/gitea/client_test.go @@ -0,0 +1,195 @@ +package gitea + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" +) + +func TestGetPullRequest(t *testing.T) { + pr := PullRequest{ + Title: "Add feature X", + Body: "This adds feature X.", + } + pr.Head.Sha = "abc123" + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/v1/repos/owner/repo/pulls/1" { + t.Errorf("unexpected path: %s", r.URL.Path) + } + if r.Header.Get("Authorization") != "token test-token" { + t.Errorf("unexpected auth header: %s", r.Header.Get("Authorization")) + } + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(pr) + })) + defer server.Close() + + client := NewClient(server.URL, "test-token") + got, err := client.GetPullRequest("owner", "repo", 1) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got.Title != "Add feature X" { + t.Errorf("expected title %q, got %q", "Add feature X", got.Title) + } + if got.Body != "This adds feature X." { + t.Errorf("expected body %q, got %q", "This adds feature X.", got.Body) + } + if got.Head.Sha != "abc123" { + t.Errorf("expected sha %q, got %q", "abc123", got.Head.Sha) + } +} + +func TestGetPullRequestDiff(t *testing.T) { + expectedDiff := "diff --git a/file.go b/file.go\n--- a/file.go\n+++ b/file.go\n@@ -1 +1 @@\n-old\n+new\n" + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/v1/repos/owner/repo/pulls/5.diff" { + t.Errorf("unexpected path: %s", r.URL.Path) + } + w.Write([]byte(expectedDiff)) + })) + defer server.Close() + + client := NewClient(server.URL, "test-token") + got, err := client.GetPullRequestDiff("owner", "repo", 5) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != expectedDiff { + t.Errorf("expected diff %q, got %q", expectedDiff, got) + } +} + +func TestGetCommitStatuses(t *testing.T) { + statuses := []CommitStatus{ + {Status: "success", Context: "ci/test", Description: "All tests passed"}, + {Status: "failure", Context: "ci/lint", Description: "Lint failed"}, + } + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/v1/repos/owner/repo/commits/abc123/statuses" { + t.Errorf("unexpected path: %s", r.URL.Path) + } + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(statuses) + })) + defer server.Close() + + client := NewClient(server.URL, "test-token") + got, err := client.GetCommitStatuses("owner", "repo", "abc123") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(got) != 2 { + t.Fatalf("expected 2 statuses, got %d", len(got)) + } + if got[0].Status != "success" { + t.Errorf("expected first status %q, got %q", "success", got[0].Status) + } + if got[1].Status != "failure" { + t.Errorf("expected second status %q, got %q", "failure", got[1].Status) + } +} + +func TestPostReview(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != "POST" { + t.Errorf("expected POST, got %s", r.Method) + } + if r.URL.Path != "/api/v1/repos/owner/repo/pulls/3/reviews" { + t.Errorf("unexpected path: %s", r.URL.Path) + } + if r.Header.Get("Content-Type") != "application/json" { + t.Errorf("unexpected content type: %s", r.Header.Get("Content-Type")) + } + + var payload struct { + Body string `json:"body"` + Event string `json:"event"` + } + if err := json.NewDecoder(r.Body).Decode(&payload); err != nil { + t.Fatalf("failed to decode payload: %v", err) + } + if payload.Body != "LGTM" { + t.Errorf("expected body %q, got %q", "LGTM", payload.Body) + } + if payload.Event != "APPROVED" { + t.Errorf("expected event %q, got %q", "APPROVED", payload.Event) + } + + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{}`)) + })) + defer server.Close() + + client := NewClient(server.URL, "test-token") + err := client.PostReview("owner", "repo", 3, "APPROVED", "LGTM") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestGetPullRequest_Non200(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + w.Write([]byte(`{"message":"not found"}`)) + })) + defer server.Close() + + client := NewClient(server.URL, "test-token") + _, err := client.GetPullRequest("owner", "repo", 999) + if err == nil { + t.Fatal("expected error for 404, got nil") + } +} + +func TestGetPullRequest_BadJSON(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte(`not json`)) + })) + defer server.Close() + + client := NewClient(server.URL, "test-token") + _, err := client.GetPullRequest("owner", "repo", 1) + if err == nil { + t.Fatal("expected error for bad JSON, got nil") + } +} + +func TestPostReview_Non200(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusForbidden) + w.Write([]byte(`{"message":"forbidden"}`)) + })) + defer server.Close() + + client := NewClient(server.URL, "test-token") + err := client.PostReview("owner", "repo", 1, "APPROVED", "test") + if err == nil { + t.Fatal("expected error for 403, got nil") + } +} + +func TestGetFileContent(t *testing.T) { + expected := "# Conventions\n- Be nice\n" + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/v1/repos/owner/repo/raw/CONVENTIONS.md" { + t.Errorf("unexpected path: %s", r.URL.Path) + } + w.Write([]byte(expected)) + })) + defer server.Close() + + client := NewClient(server.URL, "test-token") + got, err := client.GetFileContent("owner", "repo", "CONVENTIONS.md") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != expected { + t.Errorf("expected %q, got %q", expected, got) + } +} diff --git a/integration_test.go b/integration_test.go new file mode 100644 index 0000000..4c3fc28 --- /dev/null +++ b/integration_test.go @@ -0,0 +1,103 @@ +//go:build integration + +package main + +import ( + "os" + "strconv" + "testing" + + "gitea.weiker.me/rodin/review-bot/gitea" + "gitea.weiker.me/rodin/review-bot/llm" + "gitea.weiker.me/rodin/review-bot/review" +) + +// Integration test requires a running Gitea instance and LLM endpoint. +// Set environment variables: +// INTEGRATION_GITEA_URL - Gitea base URL +// INTEGRATION_GITEA_TOKEN - Gitea API token with repo access +// INTEGRATION_GITEA_REPO - owner/repo with an open PR +// INTEGRATION_PR_NUMBER - PR number to test against +// INTEGRATION_LLM_BASE_URL - LLM API base URL +// INTEGRATION_LLM_API_KEY - LLM API key +// INTEGRATION_LLM_MODEL - Model name + +func TestIntegration_FullReviewFlow(t *testing.T) { + giteaURL := os.Getenv("INTEGRATION_GITEA_URL") + giteaToken := os.Getenv("INTEGRATION_GITEA_TOKEN") + giteaRepo := os.Getenv("INTEGRATION_GITEA_REPO") + prNumStr := os.Getenv("INTEGRATION_PR_NUMBER") + llmBaseURL := os.Getenv("INTEGRATION_LLM_BASE_URL") + llmAPIKey := os.Getenv("INTEGRATION_LLM_API_KEY") + llmModel := os.Getenv("INTEGRATION_LLM_MODEL") + + if giteaURL == "" || giteaToken == "" || giteaRepo == "" || prNumStr == "" || + llmBaseURL == "" || llmAPIKey == "" || llmModel == "" { + t.Skip("Integration test env vars not set, skipping") + } + + prNumber, err := strconv.Atoi(prNumStr) + if err != nil { + t.Fatalf("Invalid PR number %q: %v", prNumStr, err) + } + + // Parse owner/repo + owner, repoName := "", "" + for i, c := range giteaRepo { + if c == / { + owner = giteaRepo[:i] + repoName = giteaRepo[i+1:] + break + } + } + if owner == "" || repoName == "" { + t.Fatalf("Invalid repo format %q", giteaRepo) + } + + // Step 1: Fetch PR + giteaClient := gitea.NewClient(giteaURL, giteaToken) + pr, err := giteaClient.GetPullRequest(owner, repoName, prNumber) + if err != nil { + t.Fatalf("GetPullRequest: %v", err) + } + t.Logf("PR: %s (sha: %s)", pr.Title, pr.Head.Sha) + + // Step 2: Fetch diff + diff, err := giteaClient.GetPullRequestDiff(owner, repoName, prNumber) + if err != nil { + t.Fatalf("GetPullRequestDiff: %v", err) + } + if diff == "" { + t.Fatal("diff is empty") + } + t.Logf("Diff size: %d bytes", len(diff)) + + // Step 3: Build prompts + systemPrompt := review.BuildSystemPrompt("") + userPrompt := review.BuildUserPrompt(pr.Title, pr.Body, diff, true, "") + + // Step 4: Call LLM + llmClient := llm.NewClient(llmBaseURL, llmAPIKey, llmModel) + response, err := llmClient.Complete([]llm.Message{ + {Role: "system", Content: systemPrompt}, + {Role: "user", Content: userPrompt}, + }) + if err != nil { + t.Fatalf("LLM Complete: %v", err) + } + t.Logf("LLM response: %d bytes", len(response)) + + // Step 5: Parse response + result, err := review.ParseResponse(response) + if err != nil { + t.Fatalf("ParseResponse: %v", err) + } + t.Logf("Verdict: %s, Findings: %d", result.Verdict, len(result.Findings)) + + // Step 6: Format (dry-run validation) + body := review.FormatMarkdown(result, "integration-test") + if body == "" { + t.Fatal("formatted review body is empty") + } + t.Logf("Review body:\n%s", body) +} diff --git a/llm/client_test.go b/llm/client_test.go new file mode 100644 index 0000000..278668d --- /dev/null +++ b/llm/client_test.go @@ -0,0 +1,110 @@ +package llm + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" +) + +func TestComplete_Success(t *testing.T) { + resp := ChatResponse{ + Choices: []struct { + Message struct { + Content string `json:"content"` + } `json:"message"` + }{ + {Message: struct { + Content string `json:"content"` + }{Content: "Hello, world!"}}, + }, + } + + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/chat/completions" { + t.Errorf("unexpected path: %s", r.URL.Path) + } + if r.Method != "POST" { + t.Errorf("expected POST, got %s", r.Method) + } + if r.Header.Get("Authorization") != "Bearer test-key" { + t.Errorf("unexpected auth: %s", r.Header.Get("Authorization")) + } + if r.Header.Get("Content-Type") != "application/json" { + t.Errorf("unexpected content type: %s", r.Header.Get("Content-Type")) + } + + var req ChatRequest + if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + t.Fatalf("decode request: %v", err) + } + if req.Model != "gpt-4" { + t.Errorf("expected model %q, got %q", "gpt-4", req.Model) + } + if len(req.Messages) != 1 { + t.Errorf("expected 1 message, got %d", len(req.Messages)) + } + + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(resp) + })) + defer server.Close() + + client := NewClient(server.URL, "test-key", "gpt-4") + got, err := client.Complete([]Message{{Role: "user", Content: "Hi"}}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != "Hello, world!" { + t.Errorf("expected %q, got %q", "Hello, world!", got) + } +} + +func TestComplete_APIError(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusTooManyRequests) + w.Write([]byte(`{"error":"rate limited"}`)) + })) + defer server.Close() + + client := NewClient(server.URL, "test-key", "gpt-4") + _, err := client.Complete([]Message{{Role: "user", Content: "Hi"}}) + if err == nil { + t.Fatal("expected error for 429, got nil") + } +} + +func TestComplete_NoChoices(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`{"choices":[]}`)) + })) + defer server.Close() + + client := NewClient(server.URL, "test-key", "gpt-4") + _, err := client.Complete([]Message{{Role: "user", Content: "Hi"}}) + if err == nil { + t.Fatal("expected error for no choices, got nil") + } +} + +func TestComplete_BadJSON(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte(`not json at all`)) + })) + defer server.Close() + + client := NewClient(server.URL, "test-key", "gpt-4") + _, err := client.Complete([]Message{{Role: "user", Content: "Hi"}}) + if err == nil { + t.Fatal("expected error for bad JSON, got nil") + } +} + +func TestComplete_ServerDown(t *testing.T) { + client := NewClient("http://127.0.0.1:1", "test-key", "gpt-4") + _, err := client.Complete([]Message{{Role: "user", Content: "Hi"}}) + if err == nil { + t.Fatal("expected error for connection refused, got nil") + } +} diff --git a/review/formatter_test.go b/review/formatter_test.go new file mode 100644 index 0000000..f15dc2c --- /dev/null +++ b/review/formatter_test.go @@ -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) + } + } +} diff --git a/review/parser_test.go b/review/parser_test.go new file mode 100644 index 0000000..138a866 --- /dev/null +++ b/review/parser_test.go @@ -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) + } +} diff --git a/review/prompt_test.go b/review/prompt_test.go new file mode 100644 index 0000000..c224619 --- /dev/null +++ b/review/prompt_test.go @@ -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") + } +}