test(web): stop asserting ci scheduling latency in the ws broadcast test - #672
Merged
plind-junior merged 1 commit intoJul 30, 2026
Merged
Conversation
test_websocket_broadcasts_on_approve bounded the approve-to-frame round trip at 1.0s. the real number is single-digit milliseconds, so the bound was never measuring the broadcast — it was measuring whether the runner descheduled the thread, which it can do for over a second with nothing wrong in the code. it failed on py3.12 at 1.479s while 3.11 and 3.13 passed the same commit. the assertion that carries the criterion is that the frame arrives with the right view and proposal id, and that is unchanged. the timing bound stays, at 10s, for the thing it can actually detect: a broadcast that stalls on a slow client, which shows up as seconds or a hang rather than a near-miss.
github-actions
Bot
added
review-ui
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
2 participants
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.
test_websocket_broadcasts_on_approvebounds the approve-to-frame round trip at 1.0s. it just failed #666's ci on py3.12 at 1.479s while py3.11 and py3.13 passed the same commit — the classic signature of a wall-clock assertion measuring the runner rather than the code.the bound was never measuring the broadcast. the comment beside it says so: "the real number is milliseconds". what a 1.0s assertion actually tests, given three orders of magnitude of headroom, is whether a shared github runner descheduled the test thread for a second — which it can do with nothing wrong in the code, and which no change to vouch can prevent.
so the timing assertion stays, but for the failure it can genuinely detect. a broadcast that stalls on a slow client (
_Hub.broadcastblocking on a socket that stopped reading) does not land at 1.2s — it lands at seconds or hangs outright. 10s catches that and cannot be tripped by scheduling noise.what carries #194's criterion is untouched: the frame must arrive, and must carry
type=refresh,view=queueand the rightproposal_id. those are the assertions that fail if the broadcast breaks. i moved the criterion note into the docstring so the next reader does not have to infer which assertion is load-bearing.no source change, one test file,
tests/test_web_e2e.pygreen at 26 cases andruffclean.worth flagging the general shape rather than just this instance: any assertion of the form
elapsed < Xwhere the real value isX/1000is a scheduling detector wearing a performance-test costume. this is the only one i hit, but it is worth a grep if this recurs elsewhere.