Repository navigation
test(Lightbox): the busy page-settle pool fails by name when too slow - #10770
Conversation
The busy page-settle test judges six settles as one pool. On a loaded runner each settle took 12 to 17 s, mostly decoding every cast frame twice: once for the photo's marker and once for the control. Six did not fit in the test's 60 s, so the run ended in its timeout. Each frame is now decoded once for both colors. The pool also takes a deadline. Before a settle that the slowest so far would not fit, it fails by name: too slow for the pool. It drops no settle, and the proof still needs six settles and three judged runs. A harness test shows that one decode for two colors finds what one decode per color finds.
|
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.
|
|
/approve-spec 22cd59e |
cixzhang
added
the
ci:run
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Changes requested: one fix is needed before this can land.
- With no test timeout (
--timeout=0, or--debug, which sets it),testInfo.timeoutis 0, so the deadline atLightboxPaging.interaction.browser.spec.ts:845is already 12 s in the past and the page-settle proof fails after its first settle with a false "too slow for the pool" error; build the deadline only whentestInfo.timeout > 0, asharness.interaction.browser.spec.ts:1701does.
The rest checks out: the one-decode read matches the two single-color reads, the new harness test fails when every color gets the cast's step, and the proof passes at the default timeout.
[Automated review]
With no test timeout, as under --debug, testInfo.timeout is 0. The pool's deadline was then already past, and it failed after its first settle. It now sets a deadline only for a test with a timeout.
PR Analysis Report🔎 Preview ·
|
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| Lightbox · Storybook | 122.6 kB (0) · 10.6 kB CSS (0) | 3,377 (0) | ✓ · ✓ · ✓ |
Package size: no published package changed; no new dependencies.
API changes: none in this pull request
Committed API report behind this build: cli 29 rows, core 36 rows, theme-butter 1 row, theme-y2k 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
|
/approve-spec 315507f |
There was a problem hiding this comment.
Approved pending CI: CI has not run on this head yet; it runs once the pull request is marked ready for review.
The earlier finding is fixed: with no test timeout (--timeout=0 or --debug) the pool sets no deadline, and the page-settle proof passes as on main, at the default timeout too, while a short timeout now fails by name.
Non-blocking:
- Miss: the too-slow message says "s of the test's time remain", but
leftcounts down to the pool's deadline, 12 s before the test's timeout, so it understates what the test has left; "… s remain before the pool's deadline" would say what it measures (LightboxPaging.interaction.browser.spec.ts:725).
[Automated review]
Maintainer impact
On a loaded runner, Lightbox's busy page-settle test can end in its 60 s timeout, naming no cause. Nothing hangs. The test judges six settles as one pool, and each took 12 to 17 s there. The largest part decoded every cast frame twice, once per color.
Intended invariant
Lightbox's PR1, which this proof checks:
The six settles, three judged runs, every floor and the 60 s timeout stay as they are.
Change
trackMarkersdecodes each cast frame once for both the photo's marker and the control.--debug, the pool has no deadline. Such a run passes, as on main.trackMarkerswith two colors finds, frame by frame, whattrackMarkerfinds for each color.Evidence
Runs use the CI container with
CI=1. "4 workers" runs four copies of this test at once.--cpus=2with 4 workers, 8 of 8 runs end in the test timeout, in Chromium and in WebKit.--cpus=4with 4 workers, a settle spent 6.7 s decoding. One decode takes 3.5 s.Scope
Testing
At 315507f on main 16436be, core and harness
tscare clean, and ESLint is clean on both files. With--timeout=0the test passes in Chromium (25 s) and WebKit (31 s); before the second commit it failed after one settle. The Lightbox interaction specs pass 103 in Chromium (4 skips) and 98 in WebKit (9 skips).