Repository navigation
test(Button): time a touch release from its animation's late start - #10975
Conversation
A busy page starts the release animation late, and the press controller then ends the release at that animation's end. The test timed the release from the `fading` write alone, so it failed raw under load. The test now stamps the press attribute's writes as the controller makes them, and reads the release animation's end before the removal cancels it. It holds the release to the later of the two due times, with the same 80 ms slack. A late end on a page that drew no frame in that slack fails by name, as frame starvation.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
PR Analysis Report🔎 Preview ·
|
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| Button · Storybook | 61.2 kB (0) · 10.0 kB CSS (0) | 690 (0) | ✓ · ✓ · ✗ |
Package size: no published package changed; no new dependencies.
API changes: none in this pull request
Committed API report behind this build: cli 36 rows, core 36 rows; run pnpm api:report.
Touched components checked; add ci:run-visual, ci:run-a11y or ci:run-rtl to run the full suites.
Generated by PR Enrichment workflow | View full report
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approved pending CI: CI has not run on this head yet. It is a draft from a fork, so CI starts once the pull request is marked ready for review and a maintainer adds the ci:run label.
The new due time matches how the press controller ends a release: the later of the exit clock and the end of a late-started release animation. In a Chromium check, the new bound passed a release whose animation started 150 ms late, which the old bound failed. It still failed a release that ended 100 ms late. One non-blocking hardening note is inline.
[Automated review]
cixzhang
added
the
ci:run
The release check kept every CSS animation in the button. An infinite one, such as a spinner, made the due time infinite, so both late checks passed without testing anything. The check now keeps what the press controller waits for: finite CSS animations on the button, or on a part that no nested press owner holds.
There was a problem hiding this comment.
Approved pending CI: CI has not run on this head yet. The pull request is a draft, so its CI run starts when it is marked ready for review; it already carries the ci:run label.
The new due time matches how the press controller ends a release: the later of the exit clock and the end of a late-started release animation, counting only the finite animations the controller waits for. In a Chromium check, the new bound passed a release whose animation started 150 ms late, which the old bound failed, and it still failed a release held 100 ms late, also with an infinite animation or a nested press owner inside the button. The earlier note about infinite and nested animations is resolved.
[Automated review]
There was a problem hiding this comment.
Approved pending CI: CI has not run on this head yet. The pull request is a draft, so its CI run starts when it is marked ready for review; it already carries the ci:run label.
The new due time matches how the press controller ends a release: the later of the exit clock and the end of a late-started release animation. On this head, after the merge of main, the new bound passed a release whose animation started 150 ms late, which the old bound failed, and it still failed a release held 100 ms late.
[Automated review]
Maintainer impact
On a loaded runner, the touch press test
outside a scroller, a finger’s press highlights in the frame it lands, and the lift fades it on the shared release clockfails on a rawtoBeLessThan. The load was a 2-CPU quota with 4 workers and 2 busy loops. Main failed the test 3 of 10 and 0 of 20 times in Chromium. It failed 4 of 10 and 6 of 20 times in WebKit. The release left 210–291 ms (Chromium) and 211–418 ms (WebKit) after thefadingwrite, against a 205 ms bound.Intended invariant
The press controller ends a release at the later of two times. One is its clock: the theme's exit duration from the
fadingwrite. The other is the end of the release animation. A busy page starts that animation late (endAfterReleaseinutils/pressFeedback.ts). The test holds the release to that due time with its existing slack. A run on a starved page fails by name.Change
Test only, in
ButtonPress.interaction.browser.spec.ts:fadingwrite and its removal as the press controller makes them. A mutation record arrives later on a busy page. One WebKit run measured a 115 ms release that way.RELEASE_SLACK_MS) after the due time.requireFramesthrowsFrameStarvationError.Evidence
Scope
Components: Button
Testing
--cpus=2on two pinned CPUs andCI=1. It ran 4 workers and 2 busy loops on the same CPUs, with--retries=0.--repeat-each=10: Chromium 3 failed (received 209.8, 230.1, 291.1 against < 205), WebKit 4 failed (387, 388, 397, 418).--repeat-each=20: Chromium 20 passed; WebKit 6 failed (211, 224, 231, 232, 262, 296).--repeat-each=20: Chromium 20 passed; WebKit 16 passed, 4 failed withFrameStarvationError.fadingwrite. The release ended 9–21 ms after the animation's end, and a control timer fired at most 5 ms late. WebKit drew frames 82–197 ms apart.toBeLessThan(RELEASE_SLACK_MS)(received 91.8–101).