From bf355b111a45be109078d8cb3b6fd6ef8b6a110f Mon Sep 17 00:00:00 2001 From: ChuckBuilds Date: Mon, 24 Aug 2026 20:18:20 -0400 Subject: [PATCH] test: wait for the payload release, not just the filing The callback test waited on is_request_complete(), which goes true as soon as the worker files the result in completed_requests. The worker then runs the cleanup pass, then the callback, then releases the payload. Both of the test's assertions therefore raced the worker: `seen` is populated by the callback, and `data is None` only after the release that follows it. It passes today because a one-line callback usually finishes inside the 20ms poll interval. Confirmed by making the callback sleep 0.4s: _wait() returns with seen == {} and the payload still resident. _wait_for_release() polls for the released payload instead. Release happens strictly after the callback returns, so a released payload also means the callback has finished and one wait covers both assertions. Verified against the same 0.4s callback. _wait() stays for the other three fetch-path tests, which assert only what is already true when the result is filed -- the success flag, the error, and the cache write that happened during the fetch itself. Its docstring now says so, so the next reader picks the right one. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW --- test/test_background_payload_release.py | 30 ++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/test/test_background_payload_release.py b/test/test_background_payload_release.py index 1dbb30c3..3f6a72c1 100644 --- a/test/test_background_payload_release.py +++ b/test/test_background_payload_release.py @@ -51,11 +51,39 @@ def service(cache): def _wait(service, req_id, timeout=5): + """Wait for the result to be FILED. + + Enough for anything that is true by the time the worker stores the result: + its success flag, its error, the cache write that happened during the + fetch. + """ deadline = time.time() + timeout while not service.is_request_complete(req_id) and time.time() < deadline: time.sleep(0.02) +def _wait_for_release(service, req_id, timeout=5): + """Wait for the payload to be RELEASED, which is strictly later. + + The worker files the result, then runs the callback, then releases. So + is_request_complete() goes true while the callback still has not run -- + waiting on it alone leaves a window in which `seen` is empty and the + payload is still resident, and the assertions race the worker. It passes + in practice only because a one-line callback usually beats the 20ms poll. + + Release happens after the callback returns, so a released payload also + means the callback has finished: one wait covers both. + """ + deadline = time.time() + timeout + while time.time() < deadline: + result = service.get_result(req_id) + if result is not None and result.data is None: + return + time.sleep(0.02) + raise AssertionError( + f"payload for {req_id} was never released (callback may not have run)") + + def _resp(): r = Mock() r.json.return_value = PAYLOAD @@ -76,7 +104,7 @@ class TestFetchPath: sport="ncaa_fb", year=2026, url="https://example.com/s", cache_key="ncaa_fb_2026", callback=callback, max_retries=0, ) - _wait(service, req_id) + _wait_for_release(service, req_id) assert seen['events'] == 50, "callback must still be handed the payload"