Wait for every blanket-answer round trip before asserting the control is hidden - #160
Merged
Conversation
… is hidden test_a_wide_banner_spends_status_copy_before_action_reach released the held blanket answer and asserted the control hidden in one expect. The press decides its decisions one at a time, so that budget was covering three sequential server round trips rather than the repaint it is for; under load the third answer lands after it and the control is still reading its count. Name the transport with _until on the outbox counters, taking the number of trips owed from the control's own face.
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
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.
Problem
cion 72be4be failed with 35 nightly tests red. Rerunning all 35 node ids againstc0a5b65leaves four: the catalog digest (#156), the two page-map chord reads (#157), and this one, which nothing owns.The count is walking down and running out of budget. The blanket answer decides its decisions one at a time —
await source[verb]?.()per decision — so the press owes one round trip per decision, and the test released the held first answer straight intoexpect(answer_all).to_be_hidden(). That single 5s budget was covering three sequential server trips, not the repaint it is for.tests/CLAUDE.mdnames this exactly: "Lettingexpectabsorb the page's next read hides which mechanism supplied the wait and spends its timeout budget on transport rather than on the assertion."Instrumenting the sequence on an idle runner gives
event → state → event → state → event, 1.89s end to end. Three trips at ~0.6s each fit inside 5s on an idle machine and do not on a loaded one, which is why this is nightly-and-under-load only.test_accept_all_decides_every_pending_suggestiondrives the same control and does not have the bug: it waits for each widget's own settle first, so itsto_be_hiddenonly has to cover the last repaint, and its comment says why. This site was the one that skipped the staging.Solution
State the transport with
_untilon theTrafficcounters — the same causal helper this test already uses eight lines above to hold the answer in the wire — then letto_be_hiddenassert only the repaint it is about. The number of trips owed is read off the control's own face (✓ Accept all (3)) rather than written out, so the fixture's arithmetic stays in one place, and anowed > 1guard fails loudly if a fixture change ever leaves this test a single trip and stops it exercising the sequence at all.This is not a raised timeout.
_untilis bounded at 30s with the counters in its failure message, so a genuinely stuck answer now fails naming the missing evidence instead of pointing at a control that "never went".Testing
Playwright driving real Chromium on this runner, at
8683b9b:uv run pytest tests— 740 passed, 6 skipped. This is the gateciruns on the PR; the test fixed here is nightly-only and not exercised by it.uv run pytest tests/test_render_controls.py --run-nightly -n4— 64 passed, three times consecutively on an otherwise-idle machine.ruff checkandruff format --checkon the changed file, at thev0.16.1.pre-commit-config.yamlpin — clean.Reproducing the failure, and the mutation checks
The test passes 6/6 at
-n0onc0a5b65unmodified, so-n0alone leaves it unclassified. Stalling the driver for 300ms betweenheld[0].continue_()and the hide assertion reproduces the CI shape exactly on the unfixed test — the button walks(3) → (2) → (1)and the expect runs out:With this change in place the same stall passes. Two mutations confirm the new wait bites rather than just returning:
traffic.sends == owed→owed + 1the page never settled every one of the 3 answers the blanket press owed: the wait began on sends=1 acked=0 pending=1 asked=3 heard=3 and gave up on sends=3 acked=3 pending=0 asked=6 heard=6— and this also confirmsowedreads 3 and that the predicate's own terms are what carry itTimed trace of the released sequence on an idle runner, seconds from
continue_():The other
.lf-answer-allsites were checked for the same shape and do not have it:test_render_gate.pynavigates away,test_render_startup.pyputswait_for_function(BOTH_STAMPS)between the release and the hide assertion and has one decision rather than three, andtest_render_projection.pyasserts the control is absent.Automated fix for failed run