Skip to content

Commit 9b2c665

Browse files
authored
chore: Fix flaky unit test (#4374)
1 parent da81e17 commit 9b2c665

1 file changed

Lines changed: 36 additions & 5 deletions

File tree

github/github_test.go

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2846,6 +2846,9 @@ func TestDo_rateLimit_abortSleepContextCancelled(t *testing.T) {
28462846
// We use a 1 minute reset time to ensure the sleep is not completed.
28472847
reset := time.Now().UTC().Add(time.Minute)
28482848
var requestCount atomic.Int32
2849+
// requestReceived is signaled once the handler has run, so the test can
2850+
// verify the request count independently of when the context is cancelled.
2851+
requestReceived := make(chan struct{})
28492852
mux.HandleFunc("/", func(w http.ResponseWriter, _ *http.Request) {
28502853
requestCount.Add(1)
28512854
w.Header().Set(HeaderRateLimit, "60")
@@ -2859,18 +2862,46 @@ func TestDo_rateLimit_abortSleepContextCancelled(t *testing.T) {
28592862
"message": "API rate limit exceeded for xxx.xxx.xxx.xxx. (But here's the good news: Authenticated requests get a higher rate limit. Check out the documentation for more details.)",
28602863
"documentation_url": "https://docs.github.com/en/rest/overview/resources-in-the-rest-api#abuse-rate-limits"
28612864
}`)
2865+
close(requestReceived)
28622866
})
28632867

2864-
ctx, cancel := context.WithTimeout(t.Context(), 10*time.Millisecond)
2868+
ctx, cancel := context.WithCancel(t.Context())
28652869
defer cancel()
28662870

28672871
req, _ := client.NewRequest(context.WithValue(ctx, SleepUntilPrimaryRateLimitResetWhenRateLimited, true), "GET", ".", nil)
2868-
_, err := client.Do(req, nil)
2869-
if !errors.Is(err, context.DeadlineExceeded) {
2870-
t.Error("Expected context deadline exceeded error.")
2872+
2873+
errCh := make(chan error, 1)
2874+
go func() {
2875+
_, err := client.Do(req, nil)
2876+
errCh <- err
2877+
}()
2878+
2879+
// Wait until the handler has processed the request, ensuring exactly one
2880+
// request was made before we cancel the context to abort the rate-limit
2881+
// sleep. This decouples the request-count assertion from the timing of
2882+
// the context cancellation, eliminating flakiness on slow runners.
2883+
select {
2884+
case <-requestReceived:
2885+
case <-time.After(10 * time.Second):
2886+
t.Fatal("Timed out waiting for request to be received.")
28712887
}
2888+
28722889
if got, want := int(requestCount.Load()), 1; got != want {
2873-
t.Errorf("Expected 1 requests, got %v", got)
2890+
t.Errorf("Expected 1 request, got %v", got)
2891+
}
2892+
2893+
// Now cancel the context while the client is sleeping waiting for the
2894+
// rate limit to reset. This should abort the sleep and return the
2895+
// context error.
2896+
cancel()
2897+
2898+
select {
2899+
case err := <-errCh:
2900+
if !errors.Is(err, context.Canceled) {
2901+
t.Errorf("Expected context cancelled error, got: %v", err)
2902+
}
2903+
case <-time.After(10 * time.Second):
2904+
t.Fatal("Timed out waiting for Do to return after context cancellation.")
28742905
}
28752906
}
28762907

0 commit comments

Comments
 (0)