fix: post-then-cleanup flow, remove dead code, pagination
CI / test (pull_request) Successful in 14s
CI / review (gpt-4.1, gpt, GPT_REVIEW_TOKEN) (pull_request) Successful in 31s
CI / review (gpt-5, sonnet, SONNET_REVIEW_TOKEN) (pull_request) Successful in 1m22s

- PostReview now returns *Review (id + user login from response)
- Delete flow: post first, then delete stale reviews by same user
- No read:user scope needed (identity from POST response)
- Removed GetAuthenticatedUser (requires scope we lack)
- ListReviews: full pagination (loops until partial page)
- envOrDefaultBool: case-insensitive, whitespace-trimmed
- action.yml: document accepted boolean values
- Tests updated for new PostReview signature
This commit is contained in:
Rodin
2026-05-01 20:22:03 -07:00
parent 0d417e068e
commit 41c670b44b
4 changed files with 113 additions and 104 deletions
+54 -42
View File
@@ -123,15 +123,21 @@ func TestPostReview(t *testing.T) {
}
w.WriteHeader(http.StatusOK)
w.Write([]byte(`{}`))
w.Write([]byte(`{"id":100,"user":{"login":"review-bot"},"state":"APPROVED","stale":false}`))
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
err := client.PostReview(context.Background(), "owner", "repo", 3, "APPROVED", "LGTM")
review, err := client.PostReview(context.Background(), "owner", "repo", 3, "APPROVED", "LGTM")
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if review.ID != 100 {
t.Errorf("expected review ID 100, got %d", review.ID)
}
if review.User.Login != "review-bot" {
t.Errorf("expected user login %q, got %q", "review-bot", review.User.Login)
}
}
func TestGetPullRequest_Non200(t *testing.T) {
@@ -169,7 +175,7 @@ func TestPostReview_Non200(t *testing.T) {
defer server.Close()
client := NewClient(server.URL, "test-token")
err := client.PostReview(context.Background(), "owner", "repo", 1, "APPROVED", "test")
_, err := client.PostReview(context.Background(), "owner", "repo", 1, "APPROVED", "test")
if err == nil {
t.Fatal("expected error for 403, got nil")
}
@@ -319,49 +325,18 @@ func TestEscapePath(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)
}
if r.Method != "GET" {
t.Errorf("expected GET, got %s", r.Method)
}
w.Header().Set("Content-Type", "application/json")
w.Write([]byte(`{"login":"review-bot","id":42}`))
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
login, err := client.GetAuthenticatedUser(context.Background())
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if login != "review-bot" {
t.Errorf("expected login %q, got %q", "review-bot", login)
}
}
func TestGetAuthenticatedUser_EmptyLogin(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
w.Write([]byte(`{"login":"","id":0}`))
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
_, err := client.GetAuthenticatedUser(context.Background())
if err == nil {
t.Fatal("expected error for empty login, got nil")
}
}
func TestListReviews(t *testing.T) {
pageCount := 0
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path != "/api/v1/repos/owner/repo/pulls/5/reviews" {
t.Errorf("unexpected path: %s", r.URL.Path)
}
if r.URL.Query().Get("limit") != "50" {
t.Errorf("expected limit=50, got %s", r.URL.Query().Get("limit"))
}
pageCount++
w.Header().Set("Content-Type", "application/json")
// Return 2 results (less than page size) to signal end
w.Write([]byte(`[{"id":10,"user":{"login":"bot-a"},"state":"APPROVED","stale":false},{"id":11,"user":{"login":"bot-b"},"state":"REQUEST_CHANGES","stale":true}]`))
}))
defer server.Close()
@@ -377,8 +352,45 @@ func TestListReviews(t *testing.T) {
if reviews[0].User.Login != "bot-a" {
t.Errorf("expected bot-a, got %s", reviews[0].User.Login)
}
if reviews[1].ID != 11 {
t.Errorf("expected id 11, got %d", reviews[1].ID)
if pageCount != 1 {
t.Errorf("expected 1 page fetch (results < page size), got %d", pageCount)
}
}
func TestListReviews_Pagination(t *testing.T) {
pageCount := 0
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
pageCount++
page := r.URL.Query().Get("page")
w.Header().Set("Content-Type", "application/json")
if page == "1" {
// Return exactly 50 items to trigger next page fetch
items := "["
for i := 0; i < 50; i++ {
if i > 0 {
items += ","
}
items += fmt.Sprintf(`{"id":%d,"user":{"login":"bot"},"state":"APPROVED","stale":false}`, i+1)
}
items += "]"
w.Write([]byte(items))
} else {
// Page 2: return fewer than 50 to signal end
w.Write([]byte(`[{"id":51,"user":{"login":"bot"},"state":"APPROVED","stale":false}]`))
}
}))
defer server.Close()
client := NewClient(server.URL, "test-token")
reviews, err := client.ListReviews(context.Background(), "owner", "repo", 5)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if len(reviews) != 51 {
t.Fatalf("expected 51 reviews across 2 pages, got %d", len(reviews))
}
if pageCount != 2 {
t.Errorf("expected 2 page fetches, got %d", pageCount)
}
}