From 5c6d995a14c589cf8712c29571018aa9be754c78 Mon Sep 17 00:00:00 2001
From: Ishwar Kanse <ikanse@redhat.com>
Date: Mon, 28 Sep 2026 07:35:09 +0530
Subject: [PATCH] test: fix flaky retry-after ordering assertion in ratelimit
 tests

launchTestRequests fired requests from goroutines staggered only by a
1ms sleep, then stored each result by launch index. The shared rate
limiter assigns Retry-After values in server-arrival order under a
mutex, so a 1ms stagger doesn't guarantee arrival order matches launch
order under CI load. This let
TestWithSharedRateLimiter/one_rate_limiter_with_additional_retry_after_logic
flake when requests arrived out of order (observed on CircleCI,
ci/circleci: test, build 13935, on PR #934).

Since the affected subtests assert a strict, deterministic Retry-After
progression, true concurrency isn't needed: send each request
synchronously, waiting for its response before firing the next, and
accumulate results directly instead of through an intermediate slice.
---
 ratelimit/http_test.go | 61 +++++++++++++-----------------------------
 1 file changed, 19 insertions(+), 42 deletions(-)

diff --git a/ratelimit/http_test.go b/ratelimit/http_test.go
index e2d632c22..59dbce6ea 100644
--- a/ratelimit/http_test.go
+++ b/ratelimit/http_test.go
@@ -433,61 +433,38 @@ func TestWithSharedRateLimiter(t *testing.T) {
 	}
 }
 
+// launchTestRequests sends reqNum requests to baseURL+pathTest.path one at a time, waiting for
+// each response before sending the next. Some callers assert a strict, deterministic progression
+// of Retry-After values (see the "shared rate limiter" test cases), which the server assigns in
+// the order requests arrive. Sending requests sequentially instead of concurrently keeps launch
+// order and arrival order identical, so those assertions aren't flaky under load.
 func launchTestRequests(t *testing.T, baseURL string, pathTest pathTestParams, reqNum int) (int, int, []http.Header) {
-	type result struct {
-		statusCode int
-		headers    http.Header
-	}
-
-	ordered := make([]result, reqNum)
-	var wg sync.WaitGroup
-	var errOnce sync.Once
-	var requestErr error
-
-	for i := 0; i < reqNum; i++ {
-		wg.Add(1)
-		time.Sleep(pathTest.waitBetween)
-
-		go func(i int) {
-			defer wg.Done()
-
-			res, err := http.Get(baseURL + pathTest.path + "/" + testTenant)
-			if err != nil {
-				errOnce.Do(func() {
-					requestErr = err
-				})
-				return
-			}
-			defer res.Body.Close()
-
-			ordered[i] = result{
-				statusCode: res.StatusCode,
-				headers:    res.Header.Clone(),
-			}
-		}(i)
-	}
-
-	wg.Wait()
-
-	if requestErr != nil {
-		t.Fatal(requestErr)
-	}
-
 	var (
 		gotOKs             int
 		gotTooManyRequests int
 		gotHeaders         = make([]http.Header, 0, reqNum)
 	)
 
-	for _, r := range ordered {
-		switch r.statusCode {
+	for range reqNum {
+		time.Sleep(pathTest.waitBetween)
+
+		res, err := http.Get(baseURL + pathTest.path + "/" + testTenant)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		switch res.StatusCode {
 		case http.StatusOK:
 			gotOKs++
 		case http.StatusTooManyRequests:
 			gotTooManyRequests++
 		}
 
-		gotHeaders = append(gotHeaders, r.headers)
+		gotHeaders = append(gotHeaders, res.Header.Clone())
+
+		if err := res.Body.Close(); err != nil {
+			t.Fatal(err)
+		}
 	}
 
 	return gotOKs, gotTooManyRequests, gotHeaders