fix: address review findings on RequestReviewer
CI / test (pull_request) Successful in 13s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 21s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 35s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m3s
CI / test (pull_request) Successful in 13s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 21s
CI / review (gpt-5, security, SECURITY_REVIEW.md, SECURITY_REVIEW_TOKEN) (pull_request) Successful in 35s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m3s
- Accept 204 No Content as success (idempotent operations) - Truncate error response body to 256 bytes (prevent log leakage) - Add unit tests for GetAuthenticatedUser and RequestReviewer
This commit is contained in:
+2
-2
@@ -507,8 +507,8 @@ func (c *Client) RequestReviewer(ctx context.Context, owner, repo string, number
|
|||||||
}
|
}
|
||||||
defer resp.Body.Close()
|
defer resp.Body.Close()
|
||||||
|
|
||||||
if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated {
|
if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated && resp.StatusCode != http.StatusNoContent {
|
||||||
body, _ := io.ReadAll(resp.Body)
|
body, _ := io.ReadAll(io.LimitReader(resp.Body, 256))
|
||||||
return fmt.Errorf("request reviewer failed (status %d): %s", resp.StatusCode, body)
|
return fmt.Errorf("request reviewer failed (status %d): %s", resp.StatusCode, body)
|
||||||
}
|
}
|
||||||
return nil
|
return nil
|
||||||
|
|||||||
@@ -5,8 +5,10 @@ import (
|
|||||||
"encoding/json"
|
"encoding/json"
|
||||||
"errors"
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"io"
|
||||||
"net/http"
|
"net/http"
|
||||||
"net/http/httptest"
|
"net/http/httptest"
|
||||||
|
"strings"
|
||||||
"testing"
|
"testing"
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -602,3 +604,83 @@ func TestIsNotFound(t *testing.T) {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestGetAuthenticatedUser(t *testing.T) {
|
||||||
|
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
if r.URL.Path != "/api/v1/user" {
|
||||||
|
t.Errorf("unexpected path: %s", r.URL.Path)
|
||||||
|
http.NotFound(w, r)
|
||||||
|
return
|
||||||
|
}
|
||||||
|
if r.Header.Get("Authorization") != "token test-token" {
|
||||||
|
t.Error("missing or wrong auth header")
|
||||||
|
}
|
||||||
|
w.Header().Set("Content-Type", "application/json")
|
||||||
|
fmt.Fprint(w, `{"login":"my-bot","id":42}`)
|
||||||
|
}))
|
||||||
|
defer server.Close()
|
||||||
|
|
||||||
|
client := NewClient(server.URL, "test-token")
|
||||||
|
login, err := client.GetAuthenticatedUser(context.Background())
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("GetAuthenticatedUser() error = %v", err)
|
||||||
|
}
|
||||||
|
if login != "my-bot" {
|
||||||
|
t.Errorf("login = %q, want %q", login, "my-bot")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestRequestReviewer(t *testing.T) {
|
||||||
|
var gotBody []byte
|
||||||
|
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
if r.Method != http.MethodPost {
|
||||||
|
t.Errorf("expected POST, got %s", r.Method)
|
||||||
|
}
|
||||||
|
expected := "/api/v1/repos/owner/repo/pulls/7/requested_reviewers"
|
||||||
|
if r.URL.Path != expected {
|
||||||
|
t.Errorf("path = %q, want %q", r.URL.Path, expected)
|
||||||
|
}
|
||||||
|
gotBody, _ = io.ReadAll(r.Body)
|
||||||
|
w.WriteHeader(http.StatusCreated)
|
||||||
|
}))
|
||||||
|
defer server.Close()
|
||||||
|
|
||||||
|
client := NewClient(server.URL, "test-token")
|
||||||
|
err := client.RequestReviewer(context.Background(), "owner", "repo", 7, "bot-user")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("RequestReviewer() error = %v", err)
|
||||||
|
}
|
||||||
|
if !strings.Contains(string(gotBody), `"bot-user"`) {
|
||||||
|
t.Errorf("body = %s, want to contain bot-user", gotBody)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestRequestReviewer_204(t *testing.T) {
|
||||||
|
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
w.WriteHeader(http.StatusNoContent)
|
||||||
|
}))
|
||||||
|
defer server.Close()
|
||||||
|
|
||||||
|
client := NewClient(server.URL, "test-token")
|
||||||
|
err := client.RequestReviewer(context.Background(), "owner", "repo", 1, "user")
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("RequestReviewer() should accept 204, got error = %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestRequestReviewer_Error(t *testing.T) {
|
||||||
|
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
w.WriteHeader(http.StatusForbidden)
|
||||||
|
fmt.Fprint(w, "no permission")
|
||||||
|
}))
|
||||||
|
defer server.Close()
|
||||||
|
|
||||||
|
client := NewClient(server.URL, "test-token")
|
||||||
|
err := client.RequestReviewer(context.Background(), "owner", "repo", 1, "user")
|
||||||
|
if err == nil {
|
||||||
|
t.Fatal("expected error for 403 response")
|
||||||
|
}
|
||||||
|
if !strings.Contains(err.Error(), "403") {
|
||||||
|
t.Errorf("error should mention status code: %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user