From dbc25f4e8a423bb76091c208271e4f8d87395543 Mon Sep 17 00:00:00 2001 From: claw Date: Wed, 13 May 2026 03:06:00 -0700 Subject: [PATCH] fix(github): add DismissReview Event comment; use t.Fatalf for routing assertions - Add comment in DismissReview explaining why the Event field is required by the GitHub API even though DISMISS is the only valid value (#18652). - Change t.Errorf to t.Fatalf for method/path routing assertions in test handlers so failures are immediately fatal instead of silently continuing handler execution (#18653). --- github/review.go | 4 +++- github/review_test.go | 16 ++++++++-------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/github/review.go b/github/review.go index bd2e05a..1a54fcf 100644 --- a/github/review.go +++ b/github/review.go @@ -194,7 +194,9 @@ func (c *Client) DismissReview(ctx context.Context, owner, repo string, number i payload := dismissReviewRequest{ Message: message, - Event: "DISMISS", + // Event is required by the GitHub API for dismissal requests, even though + // "DISMISS" is the only valid value for this endpoint. + Event: "DISMISS", } data, err := json.Marshal(payload) diff --git a/github/review_test.go b/github/review_test.go index 33f8743..da17d8e 100644 --- a/github/review_test.go +++ b/github/review_test.go @@ -17,10 +17,10 @@ import ( 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) + t.Fatalf("expected POST, got %s", r.Method) } if r.URL.Path != "/repos/owner/repo/pulls/5/reviews" { - t.Errorf("unexpected path: %s", r.URL.Path) + t.Fatalf("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")) @@ -150,10 +150,10 @@ func TestPostReview_MalformedResponse(t *testing.T) { 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) + t.Fatalf("expected GET, got %s", r.Method) } if r.URL.Path != "/repos/owner/repo/pulls/3/reviews" { - t.Errorf("unexpected path: %s", r.URL.Path) + t.Fatalf("unexpected path: %s", r.URL.Path) } json.NewEncoder(w).Encode([]map[string]interface{}{ { @@ -250,10 +250,10 @@ func TestListReviews_401(t *testing.T) { 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) + t.Fatalf("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) + t.Fatalf("unexpected path: %s", r.URL.Path) } w.WriteHeader(204) }) @@ -284,10 +284,10 @@ func TestDeleteReview_422_SubmittedReview(t *testing.T) { 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) + t.Fatalf("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) + t.Fatalf("unexpected path: %s", r.URL.Path) } body, _ := io.ReadAll(r.Body)