Repository navigation
test(motion): a retarget behind a block fails by name when starved - #10784
Conversation
|
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 ·
|
![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. The retarget-behind-a-block case keeps its windows and bounds, and a starved run now fails as FrameStarvationError; both variants pass on this head in Chromium.
Next: mark the pull request ready for review, and a maintainer adds the ci:run label so CI runs on this fork branch.
- Optional: the fit message rounds with
toFixed(1), so a 4.02 px miss prints as "4.0 px (at most 4 px)"; two decimals keep it readable as a failure (inline note). - Optional: sort
onClockwithinPresentationOrderso a late-stamped frame cannot inflate the reported gap (inline note).
[Automated review]
| // 100 ms after the block. Too few frames there cannot tell the call's | ||
| // clock from the screen's, so such a run fails by name, with its gaps. | ||
| // The gaps are read on the controls' clock, past the window's edges too. | ||
| const onClock = dated.map(({x, at}) => ({t: at, x})); |
There was a problem hiding this comment.
Non-blocking: onClock keeps the cast's stamp order, but its t is the controls' clock. Under load, a frame stamped late sits after frames it came before, and requireFrames reads gaps between array neighbours, so the reported longest gap can grow: frames at 5, 30, 47 and 160 ms after the block report 113 ms in time order and 130 ms when the frame at 30 is stamped late. The frame count and the pass or fail do not change.
const onClock = inPresentationOrder(dated.map(({x, at}) => ({t: at, x})));inPresentationOrder is already exported by the harness.
The retarget-behind-a-block case counted its frames with bare expects whose message was the raw JSON of its reads. On a loaded runner the compositor can present no frame for over 100 ms after the block, and the case then failed with only that JSON. It now asks requireFrames for the frames in the block and after it, read on the controls' clock, so a starved run fails as FrameStarvationError with its gaps. The fit checks say in words what they measured.
9cebc60 to
61afefe
Compare
There was a problem hiding this comment.
Approved pending CI: CI has not run on this head yet. The retarget-behind-a-block case keeps its windows and bounds, a starved run now fails as FrameStarvationError, and both variants pass on this head in Chromium.
Next: mark the pull request ready for review, and a maintainer adds the ci:run label so CI runs on this fork branch.
- Optional: the fit message rounds with
toFixed(1), so a 4.02 px miss prints as "4.0 px (at most 4 px)"; two decimals keep it readable as a failure (inline note). - Optional: sort
onClockwithinPresentationOrderso a late-stamped frame cannot inflate the reported gap (see the open inline thread on that line).
[Automated review]
| expect(read.worst, JSON.stringify(read)).toBeLessThanOrEqual(4); | ||
| expect( | ||
| read.worst, | ||
| `the frames miss the call, started at a frame after the block from the value and the velocity on screen there, by ${read.worst.toFixed(1)} px (at most 4 px): ${JSON.stringify(read)}`, |
There was a problem hiding this comment.
Non-blocking: toFixed(1) prints the 4.02 px miss from the description as "by 4.0 px (at most 4 px)", which reads as a pass while the check fails; only the JSON after it shows 4.02. Two decimals keep the words honest:
`the frames miss the call, started at a frame after the block from the value and the velocity on screen there, by ${read.worst.toFixed(2)} px (at most 4 px): ${JSON.stringify(read)}`,
cixzhang
added
the
ci:run
|
/approve-spec 61afefe |
Maintainer impact
Under load, the retarget-behind-a-block case fails with only the raw JSON of its reads. On main 738919b, 11 of 90 runs at 4 workers on two cores failed that way. In 10, the compositor presented no frame for 67 ms or more after the block, leaving too few frames to judge. The eleventh missed the fit by 0.02 px over its bound.
Intended invariant
The case keeps its claim and every check with its bound:
Change
61afefe changes one spec file:
requireFramesover the block and the 130 ms after it. They count the frames the controls' clock dates. Gaps are read past the window's edges, so a hole at the window's end counts. A starved run fails asFrameStarvationError, with its frame count and longest gap.Evidence
FrameStarvationError: "the page drew 1 frame … where the proof needs 3. No frame came for 133.3 ms".FrameStarvationError.FrameStarvationError.Scope
Testing
CI=trueESLint and Prettier on the spec, andreview:prepass.--cpus=4,CI=1,LP_NUM_THREADS=4) passes it 150 of 150 (×10), with 70 skipped by engine gates.