[code-infra] Parallelize visual regression screenshots#48557
Open
Janpot wants to merge 11 commits into
Open
Conversation
Mirrors mui/mui-x#22447: routes are taken concurrently using a Playwright page pool plus Vitest `describe.concurrent`, with `maxConcurrency: 4` in the config. Bumps `test_regressions` to a `large` CircleCI box (4 vCPU / 8 GB) to actually use the new parallelism. Locally the suite drops from ~68s to ~36s (~47% faster) on the same machine; CI gains scale with the larger box.
Deploy previewhttps://deploy-preview-48557--material-ui.netlify.app/ Bundle size
Check out the code infra dashboard for more information about this PR. |
…-x-22447 # Conflicts: # test/regressions/index.test.js # test/regressions/vitest.config.ts
CPU usage on Large peaked at 95-100% only twice during the run; the rest of the time hovered around 35-70%. The regression test phase is I/O-bound (Playwright pages mostly wait on network and rendering), not CPU-bound, so 3 vCPU is sufficient for the 4 concurrent pages. RAM headroom is huge (25% of 8 GB on Large), so memory isn't a constraint either. medium+ is 1.5x the cost of medium vs 2x for Large, which is closer to break-even on the CI compute-cost rule.
The aria-busy gate only tracked font loading, which left a window where components measuring their own DOM on mount (e.g. MUI Collapse running getBoundingClientRect in a useLayoutEffect) hadn't yet reached their final height. Sequential runs masked this; under pooled-page concurrency the CollapsibleTable screenshot captured each of its 5 Collapse rows in a 1px transient state, making the table 5px taller than baseline. Wait two requestAnimationFrames after fonts settle so the browser completes a layout/paint cycle before the testcase is marked ready.
ae97045 to
33f3d8f
Compare
test_regressions was running pnpm release:build plus four post-build type/export checks (validate-declarations, test:attw, testBuiltTypes, test-module-resolution) after the actual visual regression tests. In the main pipeline workflow, test_bundle_size_monitor already runs the same pnpm release:build for its size snapshot. Move the four checks onto that existing build instead of paying for two parallel release builds. test_regressions is now a single-purpose job (run regressions, verify a11y JSON committed, upload to Argos). test_bundle_size_monitor takes over QA of the built packages alongside the size snapshot.
…busy" This reverts commit 33f3d8f.
The demo used sx={{ '& > *': { borderBottom: 'unset' } }} to drop the
main row's cell borders. That generates a rule at selector specificity
(0,1,0) — identical to MUI TableCell's own border-bottom rule — so the
cascade winner depends purely on emotion's stylesheet insertion order.
Under the parallelized visual-regression harness, pooled pages render
demos in an order where the sx rule lands before TableCell's, so the
override loses and each of the 5 rows keeps a 1px border (the table
renders 5px taller). Targeting '& > .MuiTableCell-root' raises the
selector to (0,2,0) so the override always wins, regardless of
insertion order.
Janpot
commented
May 21, 2026
| return ( | ||
| <React.Fragment> | ||
| <TableRow sx={{ '& > *': { borderBottom: 'unset' } }}> | ||
| <TableRow sx={{ '& > .MuiTableCell-root': { borderBottom: 'unset' } }}> |
Member
Author
There was a problem hiding this comment.
This was fighting with internal styles due to non-deterministic style insertion order in the test suite. increasing specificity to stabilize
test_regressions runs on medium+ (3 vCPU) and the screenshot work is I/O-bound — each pooled page spends most of its time awaiting navigation, fonts and render rather than computing. Over-subscribing to 2x the core count keeps more pages in flight while others wait. RAM is not a constraint (4 pages used ~2 GB; medium+ has 6 GB).
The screenshot load is I/O-bound, not CPU/RAM-bound, and test_regressions is no longer on the workflow's critical path (test_bundle_size_monitor is). Cranking concurrency to 8 only shaved a job that's already off the critical path, while adding more cold pooled-page renders. 4 is enough.
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
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.
Mirrors mui/mui-x#22447.
Changes
routes.forEachloop intest/regressions/index.test.jsis wrapped indescribe.concurrentand each test pulls a Playwright page out of a small lazy pool via atest.extendfixture. The fixture also clearslocalStoragebefore yielding, replacing the previousbeforeEach.Rating,Autocomplete,Switch,TextField, andTextareastill run sequentially (outsidedescribe.concurrent) but consume the same pool, so they get fresh isolated pages too. Forced-colorstry/finallycleanups are preserved.maxConcurrency: 4intest/regressions/vitest.config.tsbounds concurrent route tests, so the lazy pool naturally peaks at ~4 pages.test_regressionstolarge(4 vCPU / 8 GB) on CircleCI to actually use the new parallelism.Locally on a 4-core machine the suite drops from ~68s to ~36s (~47% faster) over 631 route tests.