feat(github): implement Reviewer and Identity interfaces (#81)
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).
This commit is contained in:
@@ -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
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
}
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
@@ -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
|
||||
}
|
||||
@@ -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
|
||||
}
|
||||
Reference in New Issue
Block a user