Repository navigation
test(ProgressBar, CircularProgress): motion traces measure what they claim on a loaded runner - #9413
Conversation
…claim on a loaded runner Three checks read a property of the motion from which screencast frames a loaded runner happened to capture. - ProgressBar's storyboard judged "no jump bigger than a quarter of the travel between frames" between consecutive captured frames: one frame the capture misses doubles the step between the two around it (a missed frame read 122 then 199 px against a 55.5 px bound). The storyboard now runs on the change's own clock: the page holds the fill's transition as it starts (`armHold`), and each frame is the transition seeked one 60 Hz interval on (`seekAnimations`) and photographed, through its end. The checks, the JSON trace and the contact sheet read those frames. - CircularProgress's block test required at least ten distinct reaches of the arc inside the main-thread block. The arc's visible motion inside the block lasts about ten to twelve 60 Hz frames, so the count needed nearly every one captured; under load it read as few as 4. The test now judges the arc against the harness's busy-window control, a box the compositor moves in the same screencast (`startBusyControl`, `busyWindowProof`): the arc's reach must change on every frame the control moved and stall no longer than the control, until it settles within 3 degrees of its end. On the change's own clock each 1-degree step of the reading takes 2 to 8 ms up to 324 degrees, then 12, 12 and 18 ms for 325, 326 and 327 before it rests at 328, so from 325 two frames can read the same degree while the arc still moves. The control runs only through the block: its first turn comes after the judged part, and the page is still again when the screencast stops. The arc's reach is read at the ring in each viewport frame, the same pixels a crop to the ring holds, so each frame is decoded once. One piece, never backwards and landing at 90% stay. - ProgressBar's block test checked that the block started before the fill had made 80% of its trip from the first frame the screencast captured after the block began; a capture gap there read the fill at 287 px against a 273.8 px bound, though the block had started 18 to 42 ms into the change. The block's own task now reads where the fill's leading edge is as it starts, and the check judges that. Seams that thin the capture turn the old checks red: one missed frame fails the storyboard's jump check 5 of 5, dropping alternate frames fails the distinct count 5 of 5, and dropping the block's first 100 ms of frames fails the block-start check 5 of 5 (287 and 302 px). Mutants show the new checks still catch what they claim: - a fill moving in four held steps fails the jump check 5 of 5; - the arc paused through the block fails the busy proof 10 of 10 (it moved on none of the frames the control moved); - a fill paused through the block fails its block test 5 of 5; - a block that really starts 110 ms late fails the block-start check 5 of 5.
…ndow control The test required at least five distinct leading-edge positions among the screencast frames inside the main-thread block, a count a loaded capture thins. It now judges the fill as CircularProgress's block test judges its arc: against the harness's busy-window control in the same screencast, the fill's edge must change on every frame the control moved and stall no longer than the control, until it is within half a pixel of its end. On the change's own clock each half-pixel step of the reading takes 10 ms or less until the last, which takes 20 ms, longer than a frame. What the test cannot arrange is that the main thread reaches the block before the fill arrives: the page can block only after the frame that hands the transition to the compositor, and a loaded machine can keep the main thread from the block for most of the move. The check that the block began before 80% of the trip becomes that precondition: the fill still had a step of the cast to go, read on the page as the block began.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
meta-cla
Bot
added
the
CLA Signed
PR Analysis Report🔎 Preview ·
|
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| ProgressBar · Storybook | 7.9 kB (0) · 6.9 kB CSS (0) | 413 (0) | ✗ · ✓ · ✓ |
| lab/CircularProgress · Storybook | 8.5 kB (0) · 1.1 kB CSS (0) | 597 (0) | — · ✓ · — |
Visual changes: 2 snapshots in 1 component
A change is a question for the reviewer, not a failure.
| Component · story | Before | After | Diff |
|---|---|---|---|
| ProgressBar · Default (neutral, dark) |  |
 |
 |
| ProgressBar · Default (neutral, light) |  |
 |
 |
Package size: no published package changed; no new dependencies.
API changes: none in this pull request
Committed API report behind this build: core 1 row; 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.
Requesting changes: the reworked motion checks look sound and keep the ProgressBar FR4 evidence, but the new ProgressBar storyboard loop re-implements the interaction harness's animationStoryboard instead of calling it. Please pass the armed hold's held to animationStoryboard and read storyboard.frames for the edge checks, trace and contact sheet; if the half-pixel edge reading needs device-scale frames, add a capture-scale option to Storyboard rather than keeping a local copy. The changed Chromium specs have not run in CI yet because the pull request is a draft.
[Automated review]
| const held = await hold.held(); | ||
| const end = Math.max(...held.map(a => a.offsetMs + a.curveEndMs)); | ||
| const times: number[] = []; | ||
| for (let t = 0; t < end; t += FRAME_MS) { | ||
| times.push(t); | ||
| } | ||
| times.push(end); | ||
| const frames: Frame[] = []; | ||
| for (const t of times) { | ||
| const shown = await seekAnimations(page, t); | ||
| frames.push({t: shown, data: (await page.screenshot()).toString('base64')}); | ||
| } | ||
| await releaseAnimations(page, {finish: true}); |
There was a problem hiding this comment.
This grid-seek-photograph loop is what animationStoryboard in internal/interaction-harness/src/storyboard.ts already does, and it takes an armed hold's held for exactly this case. Calling it keeps one path: the shared loop also retries Chromium's empty surface copies and skips held animations that never end (here end would be Infinity and this loop would not terminate). If the edge reader needs device-scale frames, a capture-scale option on Storyboard covers it.
cixzhang
added
the
ci:run
… animationStoryboard The storyboard held the fill's transition with armHold and filmed it with its own seek-and-photograph loop. It passes the armed hold's set to the harness's animationStoryboard instead, which seeks the same 16.7 ms grid to the curve's end, and reads the storyboard's frames for the edge checks, the JSON trace and the contact sheet. The checks read whole CSS pixels from those frames, the precision they assert.
There was a problem hiding this comment.
Recommending approval: the ProgressBar storyboard now films the held transition with the interaction harness's animationStoryboard, which resolves the earlier request, and both block tests judge the motion against the compositor control instead of a capture count. The storyboard's whole-pixel frames are enough for its checks, and no product behavior changes.
[Automated review]
Follows #9378, which reads these specs' screencast frames by timestamp.
Maintainer impact
Checks in the progress motion specs fail on a loaded runner without anything wrong in the product. Each reads a property of the motion from which screencast frames the runner happened to capture. Pinned to two cores with 4 workers:
Intended invariant
Each check judges what it claims, whatever the screencast's capture rate:
Change
armHold), and the harness'sanimationStoryboardfilms that held set: the transition seeked one 60 Hz interval at a time and photographed, through its end.Why the count failed: the arc's visible motion inside the block lasts only ten to twelve 60 Hz frames. The theme's easing makes most of the trip in the first ~60 ms, and the block starts 20 to 50 ms into the change. The count needed nearly every one of those frames captured.
Now: a box the compositor moves along the top edge (
startBusyControl) is in the same screencast.busyWindowProofjudges the arc against it. The arc's reach must change on every frame the control moved and never stall longer than the control, until it settles. A capture gap thins both alike.The control runs only through the block. Its first turn comes after the judged part, and the page is still again when the screencast stops.
Each frame is decoded once. The arc's reach is read at the ring in the viewport frame, the same pixels a crop to the ring holds: 8 runs of 48 frames each read alike both ways, so no crop pass is needed.
It settles 3 degrees short of its end (
settledWithin: 3), from the reading's own mechanics. Held and seeked every 2 ms, the change reads:One piece, never backwards and landing at 90% stay.
Evidence
steps(4, jump-none), endpoints right) fails the storyboard's jump check 5 of 5 (74 px);Scope
Testing
packages/core/src/ProgressBar/__tests__/ProgressBarMotion.a11y.chromium.spec.tsandpackages/lab/src/CircularProgress/__tests__/CircularProgressMotion.a11y.chromium.spec.tsin Chromium. The seams and mutants were local copies of the specs.startBusyControl,busyWindowProof) are its own self-tests' judge of motion through a block.Local gates at 074bb65:
pnpm lint(0 errors);pnpm check:repoandpnpm check:knowledge;