test: make the event-loop-blocking guard causal instead of wall-clock - #203
Draft
lstein wants to merge 1 commit into
Draft
test: make the event-loop-blocking guard causal instead of wall-clock#203lstein wants to merge 1 commit into
lstein wants to merge 1 commit into
Conversation
test_slow_gallery_read_leaves_the_event_loop_free asserted the probe request finished in under BLOCKING_SECONDS/2 (0.5s) of wall time. A descheduled CI runner fails that with a perfectly free event loop - a loaded macOS runner froze the process ~1s and produced a 1.09s probe, the exact timing signature of a blocked loop. Machine noise and the regression being guarded against were indistinguishable. Rework the simulation so the assertion is causal: the patched service blocks on an Event only the TEST releases, and it is released strictly AFTER the probe completes. On a healthy 'def' route the handler cannot have finished when the probe returns; if the route regresses to 'async def', the loop is stuck inside the handler's wait, the probe can only be answered after the 30s backstop expires, and the finished-flag assertion fires with the same actionable message. No wall-clock threshold remains on the passing path. Verified in both directions: 6/6 pass in 0.24s (down from ~7s - the real 1s sleeps are gone too), and temporarily regressing /api/v1/gallery/items/ to 'async def' fails deterministically in ~15s with the intended assertion. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014xFbHnmFTLsHE9e1PAdvme
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The flake
test_slow_gallery_read_leaves_the_event_loop_freefailed transiently on #195's CI (py3.12 macOS): the probe request took 1.09s against a 0.5s threshold. That's the exact timing signature of a blocked event loop — but the loop was fine; a loaded runner descheduled the whole process for ~1s. Any wall-clock threshold makes machine noise and the guarded regression indistinguishable.The fix
Make the assertion causal. The patched service now blocks on an
Eventthat only the test releases — strictly after the probe completes:defroute: handler waits in the threadpool, probe answers, handler provably hasn't finished (it can't have — nothing released it), then the test releases it. No timing anywhere on the passing path.async defregression: the loop is stuck inside the handler's wait; the probe can only be answered after a 30s backstop expires, by which point the handler has finished → the finished-flag assertion fires with the same actionable "declare the routedef" message. The backstop is paid only when the guard actually catches a regression.The test also waits (off-loop, via
asyncio.to_thread) for the handler to have started before probing, so the probe is provably concurrent rather than hoping tensleep(0)s sufficed.Verified in both directions
/api/v1/gallery/items/toasync deffails deterministically in ~15s with the intended assertion, then the route was restored.(#195 itself is separately unblocked: its conflict with main — both sides had re-recorded the webv2 performance baselines — is resolved by a merge commit that regenerates both baselines on the merged tree; all its checks were already green after the rerun.)
🤖 Generated with Claude Code
https://claude.ai/code/session_014xFbHnmFTLsHE9e1PAdvme