From 8a633aa6872257548c4b63156a0031e3e74244ec Mon Sep 17 00:00:00 2001 From: Tom Clegg Date: Tue, 30 Jul 2024 12:52:04 -0400 Subject: [PATCH] 21927: Fix race condition in test. Test was occasionally failing because the "wait for pending reqs, then count reqs" step ran before the background refresh had progressed far enough to be counted as a pending req. Arvados-DCO-1.1-Signed-off-by: Tom Clegg --- lib/controller/handler_test.go | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/lib/controller/handler_test.go b/lib/controller/handler_test.go index 14deb4437b..e81fda9e77 100644 --- a/lib/controller/handler_test.go +++ b/lib/controller/handler_test.go @@ -265,6 +265,22 @@ func (s *HandlerSuite) TestDiscoveryDocCache(c *check.C) { getDDConcurrently(5, http.StatusOK, check.Commentf("error with warm cache")).Wait() c.Check(countRailsReqs(), check.Equals, reqsBefore) + checkBackgroundRefresh := func(reqsExpected int) { + // There is no guarantee that a background refresh has + // progressed far enough that we can detect it + // directly (the first line of refresh() might not + // have run). So, to avoid false positives, we just + // need to poll until it happens. + for deadline := time.Now().Add(time.Second); countRailsReqs() == reqsBefore && time.Now().Before(deadline); { + c.Logf("countRailsReqs = %d", countRailsReqs()) + time.Sleep(time.Second / 100) + } + // Similarly, to ensure there are no additional + // refreshes, we just need to wait. + time.Sleep(time.Second / 2) + c.Check(countRailsReqs(), check.Equals, reqsExpected) + } + // Error with stale cache => caller gets OK with stale data // while the re-fetch is attempted in the background refreshNow() @@ -273,10 +289,10 @@ func (s *HandlerSuite) TestDiscoveryDocCache(c *check.C) { holdReqs = make(chan struct{}) getDDConcurrently(5, http.StatusOK, check.Commentf("error with stale cache")).Wait() close(holdReqs) - // Only one attempt to re-fetch (holdReqs ensured the first - // update took long enough for the last incoming request to - // arrive) - c.Check(countRailsReqs(), check.Equals, reqsBefore+1) + // After piling up 5 requests (holdReqs having ensured the + // first update took long enough for the last incoming request + // to arrive) there should be only one attempt to re-fetch. + checkBackgroundRefresh(reqsBefore + 1) refreshNow() wantError, wantBadContent = false, false @@ -284,8 +300,7 @@ func (s *HandlerSuite) TestDiscoveryDocCache(c *check.C) { holdReqs = make(chan struct{}) getDDConcurrently(5, http.StatusOK, check.Commentf("refresh cache after error condition clears")).Wait() close(holdReqs) - waitPendingUpdates() - c.Check(countRailsReqs(), check.Equals, reqsBefore+1) + checkBackgroundRefresh(reqsBefore + 1) // Make sure expireAfter is getting set waitPendingUpdates() -- 2.30.2