{{ message }}
Stop the banner layout suite waiting out the update timer on every boot - #9105
Merged
Conversation
Update banner layout regression is the largest step in Chat UI Tests at 4.98 of the job's 17.6 minutes, and the second banner step adds 2.27 more. Together that is 41 percent of a job that runs on Linux, Windows and macOS. Where it goes: the suite boots a fresh page for each case it measures, and the card it is measuring is mounted by the app's 5s update-check timer, so wait_for_selector on it cannot return sooner than that. At 33 boots the timer alone is over two and a half minutes. The remaining per-boot time is the 900ms animation settle and settle_stack, both of which are render waits, both of which this leaves exactly as they are. Measured earlier as 33 gaps of about 7.5s each, which is 5 for the timer plus 0.9 plus roughly 1.6 of stability polling: the stability poll is converging, not exhausting, so it is not the thing to change. So the delay becomes overridable from a global that the hook reads AT MOUNT, and the suite sets it in the init script it already installs, to 150ms. Not zero: a card that mounts on the first frame would stop exercising the late-mount reflow this file exists to measure. Production is untouched, since the global is undefined in every real browser and the 5000ms constant stands, and it is a global rather than an env var so no build can bake the short delay in. The two sides are connected by nothing but the spelling, and a typo on either would fail in the worst way available: the timer quietly stays at 5s, the step goes back to being slow, and every assertion still passes. The new test asserts the spelling, that the suite sets it before the app runs rather than after navigation, and that the production constant is still 5000.
for more information, see https://pre-commit.ci
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.
Update banner layout regressionis the largest step inChat UI Testsat 4.98 of the job's 17.6 minutes, and the second banner step adds 2.27 more. Together that is 41% of a job that runs on Linux, Windows and macOS.Where the time goes
The suite boots a fresh page for each case it measures, and the card it is measuring is mounted by the app's 5s update-check timer, so
wait_for_selectoron it cannot return sooner than that. At 33 boots the timer alone is over two and a half minutes.The rest of each boot is the 900ms animation settle and
settle_stack, both of which are render waits, and both are left exactly as they are. Measured earlier as 33 gaps of about 7.5s: 5 for the timer, 0.9 for the settle, roughly 1.6 of stability polling. The stability poll is converging rather than exhausting, so it is not the thing to change.The change
The delay becomes overridable from a global that the hook reads at mount, and the suite sets it in the init script it already installs, to 150ms. Not zero, because a card that mounts on the first frame would stop exercising the late-mount reflow this file exists to measure.
Production is untouched: the global is undefined in every real browser and the 5000ms constant stands. It is a global rather than an env var specifically so that no build can bake the shortened delay in.
Why there is a test for a two-line change
The two sides are connected by nothing but the spelling of the global, and a typo on either would fail in the worst way available: the timer quietly stays at 5s, the step goes back to being slow, and every assertion still passes.
tests/studio/test_banner_update_delay_override.pyasserts the spelling matches, that the suite sets it before the app runs rather than after navigation, and that the production constant is still 5000.