From 39f3326674fcb61781508329cb97c654f947d3a9 Mon Sep 17 00:00:00 2001 From: Rodin Date: Thu, 14 May 2026 20:43:09 +0000 Subject: [PATCH] feat(github): add PR review API methods Implement the higher-level GitHub API methods that were TODO since issue #120. The github package now provides: - GetPullRequest: PR metadata (title, body, head SHA/ref, draft) - GetPullRequestDiff: unified diff via Accept: application/vnd.github.diff - GetPullRequestFiles: changed files list (paginated, 100/page) - GetCommitStatuses: CI statuses (GitHub uses 'state' field, normalized) - GetFileContent: file content with base64 decode (strips embedded newlines) - GetFileContentRef: file at a specific ref - ListContents: directory listing or single-file normalization - GetAllFilesInPath: recursive file fetching - PostReview: submit review with event/body/commit/inline comments - ListReviews: list PR reviews (paginated) - DeleteReview: delete review (GitHub only allows PENDING deletion) - GetAuthenticatedUser: returns login of the authed user - RequestReviewer: add a user as requested reviewer API types added: PullRequest, CommitStatus, ChangedFile, ReviewComment, Review, ContentEntry. Notable edge cases handled: - GitHub embeds newlines in base64 content; stripped before decode - GetFileContent returns error for non-file paths (type=dir) - ListContents normalizes single-file response to a slice - DeleteReview documents GitHub's PENDING-only constraint Removes TODO comment from baseURL field (now consumed by all methods). Closes part of #130. Co-authored-by: Rodin --- github/client.go | 454 +++++++++++++++++++++++++++++++++++++++++- github/client_test.go | 438 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 888 insertions(+), 4 deletions(-) diff --git a/github/client.go b/github/client.go index 227df06..cb566e7 100644 --- a/github/client.go +++ b/github/client.go @@ -4,7 +4,10 @@ package github import ( + "bytes" "context" + "encoding/base64" + "encoding/json" "errors" "fmt" "io" @@ -92,10 +95,6 @@ func asAPIError(err error) (*APIError, bool) { // SetHTTPClient and SetRetryBackoff are intended for test setup only and must // be called before any goroutines issue requests; they have no synchronization. type Client struct { - // TODO: baseURL is populated by NewClient but not yet consumed by doRequest/doGet. - // Higher-level exported methods (GetPullRequest, etc.) will use it to - // construct request URLs; remove this field if those methods end up - // accepting full URLs instead. baseURL string token string httpClient *http.Client @@ -376,3 +375,450 @@ func (c *Client) doRequest(ctx context.Context, method, reqURL string, accept st func (c *Client) doGet(ctx context.Context, url string) ([]byte, error) { return c.doRequest(ctx, http.MethodGet, url, "") } + +// --- API types --- + +// PullRequest holds relevant PR metadata. +type PullRequest struct { + Title string `json:"title"` + Body string `json:"body"` + Head struct { + Sha string `json:"sha"` + Ref string `json:"ref"` + } `json:"head"` + Draft bool `json:"draft"` +} + +// CommitStatus represents a single CI status entry. +// GitHub returns "state" not "status"; this type uses Status for consistency +// with the gitea package (both are normalized before use). +type CommitStatus struct { + Status string `json:"state"` // GitHub field is "state" + Context string `json:"context"` + Description string `json:"description"` + TargetURL string `json:"target_url"` +} + +// ChangedFile represents a file modified in a PR. +type ChangedFile struct { + Filename string `json:"filename"` + Status string `json:"status"` +} + +// ReviewComment represents an inline comment to attach to a review. +// GitHub uses "position" (diff hunk position), whereas Gitea uses "new_position" (line number). +// When posting inline comments on GitHub, position is required; line numbers +// from the diff cannot be used directly. +type ReviewComment struct { + ID int64 `json:"id,omitempty"` + Path string `json:"path"` + Position int64 `json:"position,omitempty"` // GitHub diff hunk position + Line int64 `json:"line,omitempty"` // GitHub absolute line number (alternative to position) + Side string `json:"side,omitempty"` // "RIGHT" or "LEFT" + Body string `json:"body"` +} + +// Review represents a pull request review from the GitHub API. +type Review struct { + ID int64 `json:"id"` + Body string `json:"body"` + User struct { + Login string `json:"login"` + } `json:"user"` + State string `json:"state"` +} + +// contentResponse is the GitHub contents API response for a single file. +type contentResponse struct { + Name string `json:"name"` + Path string `json:"path"` + Type string `json:"type"` // "file" or "dir" or "symlink" or "submodule" + Content string `json:"content"` // Base64-encoded file content (with embedded newlines) + Encoding string `json:"encoding"` // "base64" or "" +} + +// ContentEntry represents a file or directory entry from the contents API. +type ContentEntry struct { + Name string `json:"name"` + Path string `json:"path"` + Type string `json:"type"` // "file" or "dir" +} + +// --- PR methods --- + +// GetPullRequest fetches PR metadata. +func (c *Client) GetPullRequest(ctx context.Context, owner, repo string, number int) (*PullRequest, error) { + reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d", + c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number) + body, err := c.doGet(ctx, reqURL) + if err != nil { + return nil, fmt.Errorf("fetch PR: %w", err) + } + var pr PullRequest + if err := json.Unmarshal(body, &pr); err != nil { + return nil, fmt.Errorf("parse PR JSON: %w", err) + } + return &pr, nil +} + +// GetPullRequestDiff fetches the unified diff for a PR. +func (c *Client) GetPullRequestDiff(ctx context.Context, owner, repo string, number int) (string, error) { + reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d", + c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number) + body, err := c.doRequest(ctx, http.MethodGet, reqURL, "application/vnd.github.diff") + if err != nil { + return "", fmt.Errorf("fetch diff: %w", err) + } + return string(body), nil +} + +// GetPullRequestFiles fetches the list of files changed in a PR. +// GitHub paginates this endpoint (100 per page max). +func (c *Client) GetPullRequestFiles(ctx context.Context, owner, repo string, number int) ([]ChangedFile, error) { + const perPage = 100 + var all []ChangedFile + for page := 1; ; page++ { + reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/files?per_page=%d&page=%d", + c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number, perPage, page) + body, err := c.doGet(ctx, reqURL) + if err != nil { + return nil, fmt.Errorf("fetch PR files (page %d): %w", page, err) + } + var batch []ChangedFile + if err := json.Unmarshal(body, &batch); err != nil { + return nil, fmt.Errorf("parse PR files JSON (page %d): %w", page, err) + } + all = append(all, batch...) + if len(batch) < perPage { + break + } + } + return all, nil +} + +// GetCommitStatuses fetches CI statuses for a commit SHA. +// GitHub has two status systems: legacy "commit statuses" and newer "check runs". +// This method returns commit statuses only; check runs are a separate API. +// Note: GitHub returns "state" in the JSON; CommitStatus.Status is tagged accordingly. +func (c *Client) GetCommitStatuses(ctx context.Context, owner, repo, sha string) ([]CommitStatus, error) { + const perPage = 100 + var all []CommitStatus + for page := 1; ; page++ { + reqURL := fmt.Sprintf("%s/repos/%s/%s/commits/%s/statuses?per_page=%d&page=%d", + c.baseURL, url.PathEscape(owner), url.PathEscape(repo), url.PathEscape(sha), perPage, page) + body, err := c.doGet(ctx, reqURL) + if err != nil { + return nil, fmt.Errorf("fetch commit statuses (page %d): %w", page, err) + } + var batch []CommitStatus + if err := json.Unmarshal(body, &batch); err != nil { + return nil, fmt.Errorf("parse statuses JSON (page %d): %w", page, err) + } + all = append(all, batch...) + if len(batch) < perPage { + break + } + } + return all, nil +} + +// --- File content methods --- + +// GetFileContent fetches a file from the default branch of a repo. +// GitHub returns base64-encoded content; this method decodes it. +func (c *Client) GetFileContent(ctx context.Context, owner, repo, filepath string) (string, error) { + return c.getFileContentAtRef(ctx, owner, repo, filepath, "") +} + +// GetFileContentRef fetches a file from a specific ref (branch/tag/sha). +func (c *Client) GetFileContentRef(ctx context.Context, owner, repo, filepath, ref string) (string, error) { + return c.getFileContentAtRef(ctx, owner, repo, filepath, ref) +} + +// getFileContentAtRef fetches a file at the given ref (empty = default branch). +// GitHub's contents API returns base64-encoded file content. +func (c *Client) getFileContentAtRef(ctx context.Context, owner, repo, filepath, ref string) (string, error) { + reqURL := fmt.Sprintf("%s/repos/%s/%s/contents/%s", + c.baseURL, url.PathEscape(owner), url.PathEscape(repo), escapePath(filepath)) + if ref != "" { + reqURL += "?ref=" + url.QueryEscape(ref) + } + body, err := c.doGet(ctx, reqURL) + if err != nil { + return "", fmt.Errorf("fetch file %s: %w", filepath, err) + } + var resp contentResponse + if err := json.Unmarshal(body, &resp); err != nil { + return "", fmt.Errorf("parse file content JSON for %s: %w", filepath, err) + } + if resp.Type != "file" { + return "", fmt.Errorf("path %s is a %s, not a file", filepath, resp.Type) + } + if resp.Encoding == "base64" { + // GitHub embeds newlines in the base64 content for readability. + // Strip them before decoding. + cleaned := strings.ReplaceAll(resp.Content, "\n", "") + decoded, err := base64.StdEncoding.DecodeString(cleaned) + if err != nil { + return "", fmt.Errorf("decode base64 content for %s: %w", filepath, err) + } + return string(decoded), nil + } + // Non-base64 encoding (shouldn't happen normally, but handle gracefully). + return resp.Content, nil +} + +// ListContents lists files and directories at a given path. +// Pass an empty path to list the repository root. +// GitHub returns a single object (not array) when path is a file — this +// method normalizes both cases to a slice, matching Gitea's behavior. +func (c *Client) ListContents(ctx context.Context, owner, repo, path string) ([]ContentEntry, error) { + var reqURL string + if path == "" || path == "." { + reqURL = fmt.Sprintf("%s/repos/%s/%s/contents", + c.baseURL, url.PathEscape(owner), url.PathEscape(repo)) + } else { + reqURL = fmt.Sprintf("%s/repos/%s/%s/contents/%s", + c.baseURL, url.PathEscape(owner), url.PathEscape(repo), escapePath(path)) + } + body, err := c.doGet(ctx, reqURL) + if err != nil { + return nil, fmt.Errorf("list contents %s: %w", path, err) + } + + var entries []ContentEntry + if err := json.Unmarshal(body, &entries); err != nil { + // GitHub returns a single object when path is a file (not an array). + var single contentResponse + if err2 := json.Unmarshal(body, &single); err2 != nil { + return nil, fmt.Errorf("parse contents JSON: %w", err) + } + if single.Name == "" && single.Path == "" { + return nil, fmt.Errorf("parse contents JSON: empty response for path %q", path) + } + entries = []ContentEntry{{ + Name: single.Name, + Path: single.Path, + Type: single.Type, + }} + } + return entries, nil +} + +// GetAllFilesInPath recursively fetches all file contents under a path. +// If the path is a file, returns just that file's content. +// If the path is a directory, recursively fetches all files within it. +func (c *Client) GetAllFilesInPath(ctx context.Context, owner, repo, path string) (map[string]string, error) { + results := make(map[string]string) + + entries, err := c.ListContents(ctx, owner, repo, path) + if err != nil { + if !IsNotFound(err) { + return nil, fmt.Errorf("list contents %q: %w", path, err) + } + // 404 means path may be a file — try fetching directly. + content, fileErr := c.GetFileContent(ctx, owner, repo, path) + if fileErr != nil { + return nil, fmt.Errorf("path %q is neither a file nor directory: %w", path, fileErr) + } + results[path] = content + return results, nil + } + + for _, entry := range entries { + switch entry.Type { + case "file": + content, err := c.GetFileContent(ctx, owner, repo, entry.Path) + if err != nil { + slog.Warn("could not fetch file from patterns repo", "file", entry.Path, "error", err) + continue + } + results[entry.Path] = content + case "dir": + subResults, err := c.GetAllFilesInPath(ctx, owner, repo, entry.Path) + if err != nil { + slog.Warn("could not recurse into directory", "dir", entry.Path, "error", err) + continue + } + for k, v := range subResults { + results[k] = v + } + } + } + return results, nil +} + +// --- Review methods --- + +// PostReview submits a review to a PR. +// event should be one of "APPROVE", "REQUEST_CHANGES", or "COMMENT". +// commitID anchors the review to a specific commit SHA. If empty, defaults to current HEAD. +// comments are optional inline comments; GitHub uses diff hunk position (not line numbers). +// Note: unlike Gitea, GitHub does not support deleting submitted reviews. +// Use COMMENT event to supersede old reviews. +func (c *Client) PostReview(ctx context.Context, owner, repo string, number int, event, body, commitID string, comments []ReviewComment) (*Review, error) { + reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/reviews", + c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number) + + payload := struct { + Body string `json:"body"` + Event string `json:"event"` + CommitID string `json:"commit_id,omitempty"` + Comments []ReviewComment `json:"comments,omitempty"` + }{ + Body: body, + Event: event, + CommitID: commitID, + Comments: comments, + } + + data, err := json.Marshal(payload) + if err != nil { + return nil, fmt.Errorf("marshal review payload: %w", err) + } + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, reqURL, bytes.NewReader(data)) + if err != nil { + return nil, fmt.Errorf("create review request: %w", err) + } + req.Header.Set("Authorization", "Bearer "+c.token) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Accept", "application/vnd.github+json") + + resp, err := c.httpClient.Do(req) + if err != nil { + return nil, fmt.Errorf("post review: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + respBody, _ := io.ReadAll(io.LimitReader(resp.Body, maxErrorBodyBytes)) + return nil, &APIError{StatusCode: resp.StatusCode, Body: string(respBody)} + } + + respBody, err := io.ReadAll(io.LimitReader(resp.Body, maxResponseBodyBytes)) + if err != nil { + return nil, fmt.Errorf("read review response: %w", err) + } + var review Review + if err := json.Unmarshal(respBody, &review); err != nil { + return nil, fmt.Errorf("parse review response: %w", err) + } + return &review, nil +} + +// ListReviews returns all reviews on a pull request. +// GitHub paginates via Link header; this method uses per_page=100. +func (c *Client) ListReviews(ctx context.Context, owner, repo string, number int) ([]Review, error) { + const perPage = 100 + var all []Review + for page := 1; ; page++ { + reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/reviews?per_page=%d&page=%d", + c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number, perPage, page) + body, err := c.doGet(ctx, reqURL) + if err != nil { + return nil, fmt.Errorf("list reviews (page %d): %w", page, err) + } + var batch []Review + if err := json.Unmarshal(body, &batch); err != nil { + return nil, fmt.Errorf("parse reviews (page %d): %w", page, err) + } + all = append(all, batch...) + if len(batch) < perPage { + break + } + } + return all, nil +} + +// DeleteReview attempts to delete a pull request review. +// GitHub only allows deleting PENDING (draft) reviews. Submitted reviews cannot +// be deleted via the API; this method returns a descriptive error in that case. +// review-bot callers should handle this error gracefully (e.g., by not attempting +// supersede and instead posting a new review alongside the old one). +func (c *Client) DeleteReview(ctx context.Context, owner, repo string, number int, reviewID int64) error { + reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/reviews/%d", + c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number, reviewID) + + req, err := http.NewRequestWithContext(ctx, http.MethodDelete, reqURL, nil) + if err != nil { + return fmt.Errorf("create delete request: %w", err) + } + req.Header.Set("Authorization", "Bearer "+c.token) + req.Header.Set("Accept", "application/vnd.github+json") + + resp, err := c.httpClient.Do(req) + if err != nil { + return fmt.Errorf("delete review: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + respBody, _ := io.ReadAll(io.LimitReader(resp.Body, maxErrorBodyBytes)) + return &APIError{StatusCode: resp.StatusCode, Body: string(respBody)} + } + return nil +} + +// GetAuthenticatedUser returns the login of the authenticated user. +func (c *Client) GetAuthenticatedUser(ctx context.Context) (string, error) { + reqURL := c.baseURL + "/user" + body, err := c.doGet(ctx, reqURL) + if err != nil { + return "", fmt.Errorf("get authenticated user: %w", err) + } + var result struct { + Login string `json:"login"` + } + if err := json.Unmarshal(body, &result); err != nil { + return "", fmt.Errorf("parse user response: %w", err) + } + return result.Login, nil +} + +// RequestReviewer adds a user as a requested reviewer on a pull request. +// This is idempotent — requesting an already-requested reviewer is a no-op. +func (c *Client) RequestReviewer(ctx context.Context, owner, repo string, number int, reviewer string) error { + reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/requested_reviewers", + c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number) + + payload := struct { + Reviewers []string `json:"reviewers"` + }{Reviewers: []string{reviewer}} + data, err := json.Marshal(payload) + if err != nil { + return fmt.Errorf("marshal reviewer request: %w", err) + } + + req, err := http.NewRequestWithContext(ctx, http.MethodPost, reqURL, bytes.NewReader(data)) + if err != nil { + return fmt.Errorf("create reviewer request: %w", err) + } + req.Header.Set("Authorization", "Bearer "+c.token) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Accept", "application/vnd.github+json") + + resp, err := c.httpClient.Do(req) + if err != nil { + return fmt.Errorf("request reviewer: %w", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated && resp.StatusCode != http.StatusNoContent { + respBody, _ := io.ReadAll(io.LimitReader(resp.Body, 256)) + return &APIError{StatusCode: resp.StatusCode, Body: string(respBody)} + } + return nil +} + +// --- helpers --- + +// escapePath escapes each segment of a relative file path for use in URLs. +// Slashes are preserved as path separators; other special characters are escaped. +func escapePath(p string) string { + parts := strings.Split(p, "/") + for i, part := range parts { + parts[i] = url.PathEscape(part) + } + return strings.Join(parts, "/") +} diff --git a/github/client_test.go b/github/client_test.go index bb2f9f9..4b944a1 100644 --- a/github/client_test.go +++ b/github/client_test.go @@ -2,7 +2,9 @@ package github import ( "context" + "encoding/json" "errors" + "fmt" "net/http" "net/http/httptest" "net/url" @@ -656,3 +658,439 @@ func TestRedactURL_UserinfoWithQuery(t *testing.T) { t.Errorf("redactURL = %q, want %q", got, want) } } + +// --- Tests for API methods --- + +func TestGetPullRequest_Success(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/repos/owner/repo/pulls/42" { + t.Errorf("unexpected path: %s", r.URL.Path) + } + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"title":"Test PR","body":"description","head":{"sha":"abc123","ref":"feature"},"draft":false}`)) + })) + defer srv.Close() + + c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest()) + pr, err := c.GetPullRequest(context.Background(), "owner", "repo", 42) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if pr.Title != "Test PR" { + t.Errorf("Title = %q, want %q", pr.Title, "Test PR") + } + if pr.Head.Sha != "abc123" { + t.Errorf("Head.Sha = %q, want %q", pr.Head.Sha, "abc123") + } + if pr.Head.Ref != "feature" { + t.Errorf("Head.Ref = %q, want %q", pr.Head.Ref, "feature") + } +} + +func TestGetPullRequest_NotFound(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusNotFound) + w.Write([]byte(`{"message":"Not Found"}`)) + })) + defer srv.Close() + + c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest()) + _, err := c.GetPullRequest(context.Background(), "owner", "repo", 99) + if err == nil { + t.Fatal("expected error, got nil") + } + if !IsNotFound(err) { + t.Errorf("expected IsNotFound=true, got false for error: %v", err) + } +} + +func TestGetPullRequestDiff_Success(t *testing.T) { + const wantDiff = "diff --git a/foo.go b/foo.go\n--- a/foo.go\n+++ b/foo.go\n" + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("Accept") != "application/vnd.github.diff" { + t.Errorf("Accept = %q, want application/vnd.github.diff", r.Header.Get("Accept")) + } + w.WriteHeader(http.StatusOK) + w.Write([]byte(wantDiff)) + })) + defer srv.Close() + + c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest()) + diff, err := c.GetPullRequestDiff(context.Background(), "owner", "repo", 1) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if diff != wantDiff { + t.Errorf("diff = %q, want %q", diff, wantDiff) + } +} + +func TestGetPullRequestFiles_Success(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write([]byte(`[{"filename":"foo.go","status":"modified"},{"filename":"bar.go","status":"added"}]`)) + })) + defer srv.Close() + + c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest()) + files, err := c.GetPullRequestFiles(context.Background(), "owner", "repo", 1) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(files) != 2 { + t.Fatalf("len(files) = %d, want 2", len(files)) + } + if files[0].Filename != "foo.go" || files[0].Status != "modified" { + t.Errorf("files[0] = %+v, want {foo.go modified}", files[0]) + } +} + +func TestGetPullRequestFiles_Paginated(t *testing.T) { + page := 0 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + page++ + if page == 1 { + // Return 100 items (page full → expect another request) + items := make([]map[string]string, 100) + for i := range items { + items[i] = map[string]string{"filename": fmt.Sprintf("file%d.go", i), "status": "modified"} + } + data, _ := json.Marshal(items) + w.WriteHeader(http.StatusOK) + w.Write(data) + return + } + // Page 2: return fewer than perPage → stop + w.WriteHeader(http.StatusOK) + w.Write([]byte(`[{"filename":"last.go","status":"added"}]`)) + })) + defer srv.Close() + + c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest()) + files, err := c.GetPullRequestFiles(context.Background(), "owner", "repo", 1) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(files) != 101 { + t.Errorf("len(files) = %d, want 101", len(files)) + } + if page != 2 { + t.Errorf("page = %d, want 2", page) + } +} + +func TestGetCommitStatuses_Success(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + // GitHub uses "state" field + w.Write([]byte(`[{"state":"success","context":"ci/test","description":"Tests pass","target_url":"https://ci.example.com"}]`)) + })) + defer srv.Close() + + c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest()) + statuses, err := c.GetCommitStatuses(context.Background(), "owner", "repo", "deadbeef") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(statuses) != 1 { + t.Fatalf("len(statuses) = %d, want 1", len(statuses)) + } + if statuses[0].Status != "success" { + t.Errorf("Status = %q, want %q", statuses[0].Status, "success") + } + if statuses[0].Context != "ci/test" { + t.Errorf("Context = %q, want %q", statuses[0].Context, "ci/test") + } +} + +func TestGetFileContent_Base64(t *testing.T) { + // "hello world\n" base64-encoded with embedded newlines (as GitHub does it) + encoded := "aGVsbG8gd29ybGQK" + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !strings.HasSuffix(r.URL.Path, "/contents/README.md") { + t.Errorf("unexpected path: %s", r.URL.Path) + } + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"name":"README.md","path":"README.md","type":"file","content":"` + encoded + `","encoding":"base64"}`)) + })) + defer srv.Close() + + c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest()) + content, err := c.GetFileContent(context.Background(), "owner", "repo", "README.md") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if content != "hello world\n" { + t.Errorf("content = %q, want %q", content, "hello world\n") + } +} + +func TestGetFileContent_Base64WithNewlines(t *testing.T) { + // GitHub embeds newlines in base64 content for readability (every 60 chars) + // Test that we strip them correctly before decoding + // "hello world\n" = aGVsbG8gd29ybGQK — split it with embedded \n + encoded := "aGVs\nbG8g\nd29y\nbGQK" + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + // JSON-encode the embedded newlines as \n + body := `{"name":"README.md","path":"README.md","type":"file","content":"aGVs\nbG8g\nd29y\nbGQK","encoding":"base64"}` + _ = encoded // suppress unused warning + w.Write([]byte(body)) + })) + defer srv.Close() + + c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest()) + content, err := c.GetFileContent(context.Background(), "owner", "repo", "README.md") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if content != "hello world\n" { + t.Errorf("content = %q, want %q", content, "hello world\n") + } +} + +func TestGetFileContent_IsDirectory(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"name":"docs","path":"docs","type":"dir","content":"","encoding":""}`)) + })) + defer srv.Close() + + c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest()) + _, err := c.GetFileContent(context.Background(), "owner", "repo", "docs") + if err == nil { + t.Fatal("expected error for directory, got nil") + } +} + +func TestGetFileContentRef_Success(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Query().Get("ref") != "main" { + t.Errorf("ref = %q, want %q", r.URL.Query().Get("ref"), "main") + } + encoded := "dGVzdA==" // "test" + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"name":"foo.go","path":"foo.go","type":"file","content":"` + encoded + `","encoding":"base64"}`)) + })) + defer srv.Close() + + c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest()) + content, err := c.GetFileContentRef(context.Background(), "owner", "repo", "foo.go", "main") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if content != "test" { + t.Errorf("content = %q, want %q", content, "test") + } +} + +func TestListContents_Directory(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write([]byte(`[{"name":"foo.go","path":"foo.go","type":"file"},{"name":"bar","path":"bar","type":"dir"}]`)) + })) + defer srv.Close() + + c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest()) + entries, err := c.ListContents(context.Background(), "owner", "repo", "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(entries) != 2 { + t.Fatalf("len(entries) = %d, want 2", len(entries)) + } + if entries[0].Name != "foo.go" || entries[0].Type != "file" { + t.Errorf("entries[0] = %+v, unexpected", entries[0]) + } +} + +func TestListContents_SingleFile(t *testing.T) { + // GitHub returns a single object when the path is a file + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"name":"README.md","path":"README.md","type":"file","content":"","encoding":""}`)) + })) + defer srv.Close() + + c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest()) + entries, err := c.ListContents(context.Background(), "owner", "repo", "README.md") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(entries) != 1 { + t.Fatalf("len(entries) = %d, want 1", len(entries)) + } + if entries[0].Name != "README.md" { + t.Errorf("entries[0].Name = %q, want README.md", entries[0].Name) + } +} + +func TestPostReview_Success(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + t.Errorf("method = %s, want POST", r.Method) + } + if r.URL.Path != "/repos/owner/repo/pulls/1/reviews" { + t.Errorf("path = %s, unexpected", r.URL.Path) + } + var payload map[string]interface{} + if err := json.NewDecoder(r.Body).Decode(&payload); err != nil { + t.Errorf("decode body: %v", err) + } + if payload["event"] != "APPROVE" { + t.Errorf("event = %v, want APPROVE", payload["event"]) + } + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"id":99,"body":"looks good","user":{"login":"bot"},"state":"APPROVED"}`)) + })) + defer srv.Close() + + c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest()) + review, err := c.PostReview(context.Background(), "owner", "repo", 1, "APPROVE", "looks good", "abc", nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if review.ID != 99 { + t.Errorf("review.ID = %d, want 99", review.ID) + } + if review.User.Login != "bot" { + t.Errorf("review.User.Login = %q, want bot", review.User.Login) + } +} + +func TestPostReview_Unauthorized(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + w.Write([]byte(`{"message":"Bad credentials"}`)) + })) + defer srv.Close() + + c := NewClient("bad-tok", srv.URL, AllowInsecureHTTPForTest()) + _, err := c.PostReview(context.Background(), "owner", "repo", 1, "APPROVE", "body", "", nil) + if err == nil { + t.Fatal("expected error, got nil") + } + if !IsUnauthorized(err) { + t.Errorf("expected IsUnauthorized=true, got false for error: %v", err) + } +} + +func TestListReviews_Success(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + w.Write([]byte(`[{"id":1,"body":"review 1","user":{"login":"alice"},"state":"APPROVED"},{"id":2,"body":"review 2","user":{"login":"bob"},"state":"CHANGES_REQUESTED"}]`)) + })) + defer srv.Close() + + c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest()) + reviews, err := c.ListReviews(context.Background(), "owner", "repo", 1) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(reviews) != 2 { + t.Fatalf("len(reviews) = %d, want 2", len(reviews)) + } + if reviews[0].ID != 1 || reviews[0].User.Login != "alice" { + t.Errorf("reviews[0] = %+v, unexpected", reviews[0]) + } +} + +func TestDeleteReview_Success(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodDelete { + t.Errorf("method = %s, want DELETE", r.Method) + } + if r.URL.Path != "/repos/owner/repo/pulls/1/reviews/42" { + t.Errorf("path = %s, unexpected", r.URL.Path) + } + w.WriteHeader(http.StatusNoContent) + })) + defer srv.Close() + + c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest()) + err := c.DeleteReview(context.Background(), "owner", "repo", 1, 42) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestDeleteReview_SubmittedReview(t *testing.T) { + // GitHub returns 422 for trying to delete a non-pending review + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusUnprocessableEntity) + w.Write([]byte(`{"message":"Can only delete a pending review"}`)) + })) + defer srv.Close() + + c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest()) + err := c.DeleteReview(context.Background(), "owner", "repo", 1, 99) + if err == nil { + t.Fatal("expected error, got nil") + } +} + +func TestGetAuthenticatedUser_Success(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/user" { + t.Errorf("path = %s, want /user", r.URL.Path) + } + w.WriteHeader(http.StatusOK) + w.Write([]byte(`{"login":"review-bot","id":12345}`)) + })) + defer srv.Close() + + c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest()) + login, err := c.GetAuthenticatedUser(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if login != "review-bot" { + t.Errorf("login = %q, want review-bot", login) + } +} + +func TestRequestReviewer_Success(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + t.Errorf("method = %s, want POST", r.Method) + } + if r.URL.Path != "/repos/owner/repo/pulls/1/requested_reviewers" { + t.Errorf("path = %s, unexpected", r.URL.Path) + } + var payload map[string]interface{} + if err := json.NewDecoder(r.Body).Decode(&payload); err != nil { + t.Errorf("decode body: %v", err) + } + reviewers, ok := payload["reviewers"].([]interface{}) + if !ok || len(reviewers) != 1 || reviewers[0] != "reviewer1" { + t.Errorf("reviewers = %v, unexpected", payload["reviewers"]) + } + w.WriteHeader(http.StatusCreated) + w.Write([]byte(`{}`)) + })) + defer srv.Close() + + c := NewClient("tok", srv.URL, AllowInsecureHTTPForTest()) + err := c.RequestReviewer(context.Background(), "owner", "repo", 1, "reviewer1") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestEscapePath_SpecialChars(t *testing.T) { + tests := []struct { + input string + want string + }{ + {"README.md", "README.md"}, + {"docs/guide.md", "docs/guide.md"}, + {"path with spaces/file.md", "path%20with%20spaces/file.md"}, + {"path/with [brackets]/file.md", "path/with%20%5Bbrackets%5D/file.md"}, + } + for _, tt := range tests { + got := escapePath(tt.input) + if got != tt.want { + t.Errorf("escapePath(%q) = %q, want %q", tt.input, got, tt.want) + } + } +}