From be3f696a70758d54e7cc3f4cf035ec6250cd8669 Mon Sep 17 00:00:00 2001 From: claw Date: Wed, 13 May 2026 00:25:50 -0700 Subject: [PATCH] feat(github): implement Reviewer and Identity interfaces (#81) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implement the remaining vcs.Client interface methods for github.Client: Reviewer: - PostReview: POST /repos/{owner}/{repo}/pulls/{number}/reviews - ListReviews: GET /repos/{owner}/{repo}/pulls/{number}/reviews with state translation (CHANGES_REQUESTED → REQUEST_CHANGES, etc.) - DeleteReview: DELETE /repos/{owner}/{repo}/pulls/{number}/reviews/{id} Returns ErrCannotDeleteSubmittedReview on 422 - DismissReview: PUT /repos/{owner}/{repo}/pulls/{number}/reviews/{id}/dismissals Identity: - GetAuthenticatedUser: GET /user Infrastructure: - Add doRequestWithBody helper for POST/PUT/DELETE with JSON bodies - Update conformance_test.go: var _ vcs.Client = (*github.Client)(nil) All unit tests pass including error cases (401, 404, 422, malformed). --- github/client.go | 111 +++++++++++ github/conformance_test.go | 10 +- github/identity.go | 29 +++ github/identity_test.go | 46 +++++ github/review.go | 191 ++++++++++++++++++ github/review_test.go | 394 +++++++++++++++++++++++++++++++++++++ 6 files changed, 775 insertions(+), 6 deletions(-) create mode 100644 github/identity.go create mode 100644 github/identity_test.go create mode 100644 github/review.go create mode 100644 github/review_test.go diff --git a/github/client.go b/github/client.go index dc10c7a..9fd7eb8 100644 --- a/github/client.go +++ b/github/client.go @@ -4,6 +4,7 @@ package github import ( + "bytes" "context" "errors" "fmt" @@ -342,3 +343,113 @@ func (c *Client) handleResponse(resp *http.Response, maxRespBytes int, maxErrByt func (c *Client) doGet(ctx context.Context, reqURL string) ([]byte, error) { return c.doRequest(ctx, http.MethodGet, reqURL, "") } + +// doRequestWithBody is like doRequest but sends a request body. +// It accepts the raw body bytes and sets Content-Type to application/json. +// Retry semantics match doRequest (retries on 429 with Retry-After support). +func (c *Client) doRequestWithBody(ctx context.Context, method, reqURL string, body []byte) ([]byte, error) { + const maxRetryAfter = 120 * time.Second + + defaultBackoff := []time.Duration{1 * time.Second, 2 * time.Second} + var backoff []time.Duration + if c.retryBackoff != nil && len(c.retryBackoff) == maxRetryAttempts-1 { + backoff = make([]time.Duration, len(c.retryBackoff)) + copy(backoff, c.retryBackoff) + } else { + backoff = make([]time.Duration, len(defaultBackoff)) + copy(backoff, defaultBackoff) + } + + const maxErrorBodyBytes = 4 * 1024 + + if c.token != "" && !c.allowInsecureHTTP { + parsed, err := url.Parse(reqURL) + if err != nil { + return nil, fmt.Errorf("parse request URL: %w", err) + } + if !strings.EqualFold(parsed.Scheme, "https") { + return nil, fmt.Errorf("refusing to send credentials over non-HTTPS URL %q (use AllowInsecureHTTP option for trusted networks)", reqURL) + } + } + + var lastErr error + for attempt := 0; attempt < maxRetryAttempts; attempt++ { + if attempt > 0 { + var delay time.Duration + if attempt-1 < len(backoff) { + delay = backoff[attempt-1] + } + if delay > 0 { + timer := time.NewTimer(delay) + select { + case <-timer.C: + timer.Stop() + case <-ctx.Done(): + timer.Stop() + return nil, ctx.Err() + } + } + } + + var bodyReader io.Reader + if body != nil { + bodyReader = bytes.NewReader(body) + } + req, err := http.NewRequestWithContext(ctx, method, reqURL, bodyReader) + if err != nil { + return nil, fmt.Errorf("create request: %w", err) + } + if c.token != "" { + req.Header.Set("Authorization", "Bearer "+c.token) + } + req.Header.Set("User-Agent", userAgent) + req.Header.Set("Accept", "application/vnd.github+json") + if body != nil { + req.Header.Set("Content-Type", "application/json") + } + + resp, err := c.httpClient.Do(req) + if err != nil { + return nil, fmt.Errorf("do request: %w", err) + } + + respStatus := resp.StatusCode + retryAfterHeader := resp.Header.Get("Retry-After") + + respBody, done, handleErr := c.handleResponse(resp, maxResponseBytes, maxErrorBodyBytes) + if done { + return respBody, handleErr + } + lastErr = handleErr + + if respStatus == http.StatusTooManyRequests && attempt < maxRetryAttempts-1 { + if ra := retryAfterHeader; ra != "" { + if seconds, err := strconv.Atoi(ra); err == nil && seconds > 0 { + delay := time.Duration(seconds) * time.Second + if delay > maxRetryAfter { + delay = maxRetryAfter + } + if attempt < len(backoff) { + backoff[attempt] = delay + } + } else if retryAt, err := http.ParseTime(ra); err == nil { + delay := time.Until(retryAt) + if delay < 0 { + delay = 0 + } + if delay > maxRetryAfter { + delay = maxRetryAfter + } + if attempt < len(backoff) { + backoff[attempt] = delay + } + } + } + continue + } + + return nil, lastErr + } + + return nil, lastErr +} diff --git a/github/conformance_test.go b/github/conformance_test.go index ca13188..3e06e2d 100644 --- a/github/conformance_test.go +++ b/github/conformance_test.go @@ -5,9 +5,7 @@ import ( "gitea.weiker.me/rodin/review-bot/vcs" ) -// Compile-time interface conformance assertions. -// These verify github.Client satisfies vcs.PRReader and vcs.FileReader. -var ( - _ vcs.PRReader = (*github.Client)(nil) - _ vcs.FileReader = (*github.Client)(nil) -) +// Compile-time interface conformance assertion. +// This verifies github.Client satisfies the full vcs.Client interface +// (PRReader, FileReader, Reviewer, Identity). +var _ vcs.Client = (*github.Client)(nil) diff --git a/github/identity.go b/github/identity.go new file mode 100644 index 0000000..46c26a8 --- /dev/null +++ b/github/identity.go @@ -0,0 +1,29 @@ +package github + +import ( + "context" + "encoding/json" + "fmt" +) + +// userResponse is the GitHub API response for the authenticated user. +type userResponse struct { + Login string `json:"login"` +} + +// GetAuthenticatedUser returns the login of the currently authenticated user. +func (c *Client) GetAuthenticatedUser(ctx context.Context) (string, error) { + reqURL := fmt.Sprintf("%s/user", c.baseURL) + + body, err := c.doGet(ctx, reqURL) + if err != nil { + return "", fmt.Errorf("get authenticated user: %w", err) + } + + var resp userResponse + if err := json.Unmarshal(body, &resp); err != nil { + return "", fmt.Errorf("parse user response: %w", err) + } + + return resp.Login, nil +} diff --git a/github/identity_test.go b/github/identity_test.go new file mode 100644 index 0000000..9187ce9 --- /dev/null +++ b/github/identity_test.go @@ -0,0 +1,46 @@ +package github + +import ( + "context" + "encoding/json" + "net/http" + "testing" +) + +func TestGetAuthenticatedUser_HappyPath(t *testing.T) { + c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { + if r.Method != "GET" { + t.Errorf("expected GET, got %s", r.Method) + } + if r.URL.Path != "/user" { + t.Errorf("unexpected path: %s", r.URL.Path) + } + if r.Header.Get("Authorization") != "Bearer test-token" { + t.Errorf("unexpected auth header: %s", r.Header.Get("Authorization")) + } + json.NewEncoder(w).Encode(map[string]string{"login": "review-bot"}) + }) + + login, err := c.GetAuthenticatedUser(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if login != "review-bot" { + t.Errorf("expected login 'review-bot', got %q", login) + } +} + +func TestGetAuthenticatedUser_401(t *testing.T) { + c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(401) + w.Write([]byte(`{"message":"Bad credentials"}`)) + }) + + _, err := c.GetAuthenticatedUser(context.Background()) + if err == nil { + t.Fatal("expected error for 401") + } + if !IsUnauthorized(err) { + t.Errorf("expected IsUnauthorized=true, got error: %v", err) + } +} diff --git a/github/review.go b/github/review.go new file mode 100644 index 0000000..e9e20cd --- /dev/null +++ b/github/review.go @@ -0,0 +1,191 @@ +package github + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "net/url" + + "gitea.weiker.me/rodin/review-bot/vcs" +) + +// ErrCannotDeleteSubmittedReview is returned when DeleteReview is called on +// a review that has already been submitted (APPROVED, REQUEST_CHANGES, COMMENT). +// GitHub only allows deletion of PENDING reviews. Callers that need to replace +// a submitted review should use DismissReview instead. +var ErrCannotDeleteSubmittedReview = errors.New("cannot delete submitted review: use DismissReview instead") + +// postReviewRequest is the GitHub API request body for creating a review. +type postReviewRequest struct { + CommitID string `json:"commit_id,omitempty"` + Body string `json:"body"` + Event string `json:"event"` + Comments []reviewCommentEntry `json:"comments,omitempty"` +} + +// reviewCommentEntry is a single inline comment in a review creation request. +type reviewCommentEntry struct { + Path string `json:"path"` + Position int `json:"position"` + Body string `json:"body"` +} + +// reviewResponse is the GitHub API response for a review. +type reviewResponse struct { + ID int64 `json:"id"` + Body string `json:"body"` + State string `json:"state"` + CommitID string `json:"commit_id"` + User struct { + Login string `json:"login"` + } `json:"user"` +} + +// dismissReviewRequest is the GitHub API request body for dismissing a review. +type dismissReviewRequest struct { + Message string `json:"message"` + Event string `json:"event"` +} + +// translateGitHubReviewState translates a GitHub API review state to the +// canonical vcs.Review.State value. +func translateGitHubReviewState(state string) string { + switch state { + case "APPROVED": + return "APPROVED" + case "CHANGES_REQUESTED": + return "REQUEST_CHANGES" + case "COMMENTED": + return "COMMENT" + case "DISMISSED": + return "DISMISSED" + default: + return state + } +} + +// PostReview submits a review on a pull request. +// The ReviewRequest.Event values (APPROVE, REQUEST_CHANGES, COMMENT) are sent +// directly — they match GitHub's canonical event strings. +// ReviewComment.Position maps directly to the GitHub API position field. +func (c *Client) PostReview(ctx context.Context, owner, repo string, number int, req vcs.ReviewRequest) (*vcs.Review, error) { + reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/reviews", + c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number) + + payload := postReviewRequest{ + Body: req.Body, + Event: string(req.Event), + } + + // Populate CommitID from the first comment if available. + // All comments in a single review share the same commit_id. + for _, comment := range req.Comments { + if comment.CommitID != "" { + payload.CommitID = comment.CommitID + break + } + } + + for _, comment := range req.Comments { + payload.Comments = append(payload.Comments, reviewCommentEntry{ + Path: comment.Path, + Position: comment.Position, + Body: comment.Body, + }) + } + + data, err := json.Marshal(payload) + if err != nil { + return nil, fmt.Errorf("marshal review request: %w", err) + } + + body, err := c.doRequestWithBody(ctx, "POST", reqURL, data) + if err != nil { + return nil, fmt.Errorf("post review: %w", err) + } + + var resp reviewResponse + if err := json.Unmarshal(body, &resp); err != nil { + return nil, fmt.Errorf("parse review response: %w", err) + } + + return &vcs.Review{ + ID: resp.ID, + Body: resp.Body, + User: vcs.UserInfo{Login: resp.User.Login}, + State: translateGitHubReviewState(resp.State), + CommitID: resp.CommitID, + }, nil +} + +// ListReviews retrieves all reviews for a pull request. +// GitHub review states are translated to canonical vcs values. +func (c *Client) ListReviews(ctx context.Context, owner, repo string, number int) ([]vcs.Review, error) { + reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/reviews", + c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number) + + body, err := c.doGet(ctx, reqURL) + if err != nil { + return nil, fmt.Errorf("list reviews: %w", err) + } + + var responses []reviewResponse + if err := json.Unmarshal(body, &responses); err != nil { + return nil, fmt.Errorf("parse reviews response: %w", err) + } + + reviews := make([]vcs.Review, len(responses)) + for i, r := range responses { + reviews[i] = vcs.Review{ + ID: r.ID, + Body: r.Body, + User: vcs.UserInfo{Login: r.User.Login}, + State: translateGitHubReviewState(r.State), + CommitID: r.CommitID, + } + } + return reviews, nil +} + +// DeleteReview deletes a pull request review. +// Only PENDING reviews can be deleted; attempting to delete a submitted review +// (APPROVED, CHANGES_REQUESTED, COMMENTED) returns ErrCannotDeleteSubmittedReview. +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) + + _, err := c.doRequestWithBody(ctx, "DELETE", reqURL, nil) + if err != nil { + var apiErr *APIError + if errors.As(err, &apiErr) && apiErr.StatusCode == 422 { + return ErrCannotDeleteSubmittedReview + } + return fmt.Errorf("delete review: %w", err) + } + return nil +} + +// DismissReview dismisses a submitted review on a pull request. +// This is the correct way to "remove" a submitted review (APPROVED, REQUEST_CHANGES). +// GitHub does not allow deleting submitted reviews — they must be dismissed. +func (c *Client) DismissReview(ctx context.Context, owner, repo string, number int, reviewID int64, message string) error { + reqURL := fmt.Sprintf("%s/repos/%s/%s/pulls/%d/reviews/%d/dismissals", + c.baseURL, url.PathEscape(owner), url.PathEscape(repo), number, reviewID) + + payload := dismissReviewRequest{ + Message: message, + Event: "DISMISS", + } + + data, err := json.Marshal(payload) + if err != nil { + return fmt.Errorf("marshal dismiss request: %w", err) + } + + _, err = c.doRequestWithBody(ctx, "PUT", reqURL, data) + if err != nil { + return fmt.Errorf("dismiss review: %w", err) + } + return nil +} diff --git a/github/review_test.go b/github/review_test.go new file mode 100644 index 0000000..50e6700 --- /dev/null +++ b/github/review_test.go @@ -0,0 +1,394 @@ +package github + +import ( + "context" + "encoding/json" + "errors" + "io" + "net/http" + "net/http/httptest" + "testing" + "time" + + "gitea.weiker.me/rodin/review-bot/vcs" +) + +func newTestClient(t *testing.T, handler http.HandlerFunc) *Client { + t.Helper() + srv := httptest.NewServer(handler) + t.Cleanup(srv.Close) + c := NewClient("test-token", srv.URL, AllowInsecureHTTP()) + c.SetHTTPClient(srv.Client()) + if err := c.SetRetryBackoff([]time.Duration{1 * time.Millisecond, 1 * time.Millisecond}); err != nil { + t.Fatalf("SetRetryBackoff: %v", err) + } + return c +} + +// --- PostReview tests --- + +func TestPostReview_HappyPath(t *testing.T) { + c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { + if r.Method != "POST" { + t.Errorf("expected POST, got %s", r.Method) + } + if r.URL.Path != "/repos/owner/repo/pulls/5/reviews" { + t.Errorf("unexpected path: %s", r.URL.Path) + } + if r.Header.Get("Content-Type") != "application/json" { + t.Errorf("expected Content-Type application/json, got %q", r.Header.Get("Content-Type")) + } + + // Verify request body + body, _ := io.ReadAll(r.Body) + var req postReviewRequest + if err := json.Unmarshal(body, &req); err != nil { + t.Fatalf("unmarshal request: %v", err) + } + if req.Event != "APPROVE" { + t.Errorf("expected event APPROVE, got %q", req.Event) + } + if req.Body != "LGTM" { + t.Errorf("expected body 'LGTM', got %q", req.Body) + } + if req.CommitID != "abc123" { + t.Errorf("expected commit_id 'abc123', got %q", req.CommitID) + } + if len(req.Comments) != 1 { + t.Fatalf("expected 1 comment, got %d", len(req.Comments)) + } + if req.Comments[0].Path != "main.go" { + t.Errorf("expected comment path 'main.go', got %q", req.Comments[0].Path) + } + if req.Comments[0].Position != 4 { + t.Errorf("expected comment position 4, got %d", req.Comments[0].Position) + } + + json.NewEncoder(w).Encode(map[string]interface{}{ + "id": 100, + "body": "LGTM", + "state": "APPROVED", + "commit_id": "abc123", + "user": map[string]string{"login": "reviewer"}, + }) + }) + + review, err := c.PostReview(context.Background(), "owner", "repo", 5, vcs.ReviewRequest{ + Body: "LGTM", + Event: vcs.ReviewEventApprove, + Comments: []vcs.ReviewComment{ + {Path: "main.go", Position: 4, CommitID: "abc123", Body: "nit: rename"}, + }, + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if review.ID != 100 { + t.Errorf("expected ID 100, got %d", review.ID) + } + if review.Body != "LGTM" { + t.Errorf("expected body 'LGTM', got %q", review.Body) + } + if review.State != "APPROVED" { + t.Errorf("expected state 'APPROVED', got %q", review.State) + } + if review.User.Login != "reviewer" { + t.Errorf("expected user 'reviewer', got %q", review.User.Login) + } + if review.CommitID != "abc123" { + t.Errorf("expected commit_id 'abc123', got %q", review.CommitID) + } +} + +func TestPostReview_401(t *testing.T) { + c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(401) + w.Write([]byte(`{"message":"Bad credentials"}`)) + }) + + _, err := c.PostReview(context.Background(), "owner", "repo", 5, vcs.ReviewRequest{ + Body: "LGTM", + Event: vcs.ReviewEventApprove, + }) + if err == nil { + t.Fatal("expected error for 401") + } + if !IsUnauthorized(err) { + t.Errorf("expected IsUnauthorized=true, got error: %v", err) + } +} + +func TestPostReview_422(t *testing.T) { + c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(422) + w.Write([]byte(`{"message":"Unprocessable Entity"}`)) + }) + + _, err := c.PostReview(context.Background(), "owner", "repo", 5, vcs.ReviewRequest{ + Body: "LGTM", + Event: vcs.ReviewEventApprove, + }) + if err == nil { + t.Fatal("expected error for 422") + } + // 422 should surface as a wrapped APIError + var apiErr *APIError + if !errors.As(err, &apiErr) { + t.Fatalf("expected *APIError, got %T: %v", err, err) + } + if apiErr.StatusCode != 422 { + t.Errorf("expected status 422, got %d", apiErr.StatusCode) + } +} + +func TestPostReview_MalformedResponse(t *testing.T) { + c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte(`not json`)) + }) + + _, err := c.PostReview(context.Background(), "owner", "repo", 5, vcs.ReviewRequest{ + Body: "LGTM", + Event: vcs.ReviewEventApprove, + }) + if err == nil { + t.Fatal("expected error for malformed response") + } + if !containsStr(err.Error(), "parse review response") { + t.Errorf("expected parse error, got: %v", err) + } +} + +// --- ListReviews tests --- + +func TestListReviews_HappyPath(t *testing.T) { + c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { + if r.Method != "GET" { + t.Errorf("expected GET, got %s", r.Method) + } + if r.URL.Path != "/repos/owner/repo/pulls/3/reviews" { + t.Errorf("unexpected path: %s", r.URL.Path) + } + json.NewEncoder(w).Encode([]map[string]interface{}{ + { + "id": 1, + "body": "Approved", + "state": "APPROVED", + "commit_id": "sha1", + "user": map[string]string{"login": "user1"}, + }, + { + "id": 2, + "body": "Needs work", + "state": "CHANGES_REQUESTED", + "commit_id": "sha2", + "user": map[string]string{"login": "user2"}, + }, + { + "id": 3, + "body": "Comment only", + "state": "COMMENTED", + "commit_id": "sha3", + "user": map[string]string{"login": "user3"}, + }, + { + "id": 4, + "body": "Old review", + "state": "DISMISSED", + "commit_id": "sha4", + "user": map[string]string{"login": "user4"}, + }, + }) + }) + + reviews, err := c.ListReviews(context.Background(), "owner", "repo", 3) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(reviews) != 4 { + t.Fatalf("expected 4 reviews, got %d", len(reviews)) + } + + // Check state translation + expected := []struct { + id int64 + state string + }{ + {1, "APPROVED"}, + {2, "REQUEST_CHANGES"}, + {3, "COMMENT"}, + {4, "DISMISSED"}, + } + for i, e := range expected { + if reviews[i].ID != e.id { + t.Errorf("review[%d]: expected ID %d, got %d", i, e.id, reviews[i].ID) + } + if reviews[i].State != e.state { + t.Errorf("review[%d]: expected state %q, got %q", i, e.state, reviews[i].State) + } + } +} + +func TestListReviews_404(t *testing.T) { + c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(404) + w.Write([]byte(`{"message":"Not Found"}`)) + }) + + _, err := c.ListReviews(context.Background(), "owner", "repo", 999) + if err == nil { + t.Fatal("expected error for 404") + } + if !IsNotFound(err) { + t.Errorf("expected IsNotFound=true, got error: %v", err) + } +} + +func TestListReviews_401(t *testing.T) { + c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(401) + w.Write([]byte(`{"message":"Bad credentials"}`)) + }) + + _, err := c.ListReviews(context.Background(), "owner", "repo", 3) + if err == nil { + t.Fatal("expected error for 401") + } + if !IsUnauthorized(err) { + t.Errorf("expected IsUnauthorized=true, got error: %v", err) + } +} + +// --- DeleteReview tests --- + +func TestDeleteReview_HappyPath(t *testing.T) { + c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { + if r.Method != "DELETE" { + t.Errorf("expected DELETE, got %s", r.Method) + } + if r.URL.Path != "/repos/owner/repo/pulls/5/reviews/42" { + t.Errorf("unexpected path: %s", r.URL.Path) + } + w.WriteHeader(204) + }) + + err := c.DeleteReview(context.Background(), "owner", "repo", 5, 42) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestDeleteReview_422_SubmittedReview(t *testing.T) { + c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(422) + w.Write([]byte(`{"message":"Can not delete a non pending review"}`)) + }) + + err := c.DeleteReview(context.Background(), "owner", "repo", 5, 42) + if err == nil { + t.Fatal("expected error for 422") + } + if !errors.Is(err, ErrCannotDeleteSubmittedReview) { + t.Errorf("expected ErrCannotDeleteSubmittedReview, got: %v", err) + } +} + +// --- DismissReview tests --- + +func TestDismissReview_HappyPath(t *testing.T) { + c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { + if r.Method != "PUT" { + t.Errorf("expected PUT, got %s", r.Method) + } + if r.URL.Path != "/repos/owner/repo/pulls/5/reviews/10/dismissals" { + t.Errorf("unexpected path: %s", r.URL.Path) + } + + body, _ := io.ReadAll(r.Body) + var req dismissReviewRequest + if err := json.Unmarshal(body, &req); err != nil { + t.Fatalf("unmarshal request: %v", err) + } + if req.Message != "Superseded by new review" { + t.Errorf("expected message 'Superseded by new review', got %q", req.Message) + } + if req.Event != "DISMISS" { + t.Errorf("expected event 'DISMISS', got %q", req.Event) + } + + json.NewEncoder(w).Encode(map[string]interface{}{ + "id": 10, + "state": "DISMISSED", + }) + }) + + err := c.DismissReview(context.Background(), "owner", "repo", 5, 10, "Superseded by new review") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestDismissReview_404(t *testing.T) { + c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(404) + w.Write([]byte(`{"message":"Not Found"}`)) + }) + + err := c.DismissReview(context.Background(), "owner", "repo", 5, 999, "dismiss") + if err == nil { + t.Fatal("expected error for 404") + } + if !IsNotFound(err) { + t.Errorf("expected IsNotFound=true, got error: %v", err) + } +} + +func TestDismissReview_401(t *testing.T) { + c := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(401) + w.Write([]byte(`{"message":"Bad credentials"}`)) + }) + + err := c.DismissReview(context.Background(), "owner", "repo", 5, 10, "dismiss") + if err == nil { + t.Fatal("expected error for 401") + } + if !IsUnauthorized(err) { + t.Errorf("expected IsUnauthorized=true, got error: %v", err) + } +} + +// --- State translation tests --- + +func TestTranslateGitHubReviewState(t *testing.T) { + tests := []struct { + input string + expected string + }{ + {"APPROVED", "APPROVED"}, + {"CHANGES_REQUESTED", "REQUEST_CHANGES"}, + {"COMMENTED", "COMMENT"}, + {"DISMISSED", "DISMISSED"}, + {"UNKNOWN_STATE", "UNKNOWN_STATE"}, + {"", ""}, + } + for _, tt := range tests { + got := translateGitHubReviewState(tt.input) + if got != tt.expected { + t.Errorf("translateGitHubReviewState(%q) = %q, want %q", tt.input, got, tt.expected) + } + } +} + +// containsStr is a test helper for checking error messages. +func containsStr(s, substr string) bool { + return len(s) >= len(substr) && (s == substr || len(s) > 0 && containsSubstring(s, substr)) +} + +func containsSubstring(s, sub string) bool { + for i := 0; i <= len(s)-len(sub); i++ { + if s[i:i+len(sub)] == sub { + return true + } + } + return false +}