[test] Add a visual regression fixture for the DocSearch modal - #49002
Open
LukasTy wants to merge 8 commits into
Open
[test] Add a visual regression fixture for the DocSearch modal#49002LukasTy wants to merge 8 commits into
LukasTy wants to merge 8 commits into
Conversation
Argos covers `@mui/material` fixtures and docs demos. It never mounts the docs app shell, so `AppSearch` and the DocSearch overrides it carries had no coverage. Two layout regressions reached review during the v4 -> v5 bump that way: the hit list picked up a 1em indent the section headings did not, and the hit title started wrapping below 768px. Render the real `AppSearch`, seed the stored recent searches so the hit list comes up without an Algolia round-trip, and screenshot the open modal. The route loop also captures the closed search button. Two things the regression bundle does not give us that the docs do: - The layer order. `BrandingCssVarsProvider` declares `docsearch` ahead of `mui`; without it the DocSearch stylesheet outranks every override. A layer's position is fixed where it is first named and emotion keeps prepending its tags above everything in `<head>`, so the test declares it once the modal has rendered. - `CssBaseline`. `TestViewer` sets `box-sizing: content-box` on purpose, and with the layer order corrected that beats the `border-box` DocSearch sets on the modal subtree, which overflows it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Deploy previewBundle size
Check out the code infra dashboard for more information about this PR. |
LukasTy
added
test
type: enhancement
The v3 -> v4 migration broke dark mode, and `AppSearch` carries a second `GlobalStyles` block just for it, so screenshot both schemes. Move the setup into `AppSearchFixture` and leave one route per scheme behind it. Helpers cannot live under `fixtures/`, where the eager glob turns every file into a route. Use the docs theme through `getTheme` rather than `BrandingProvider`. It carries `theme.vars`, so the dark block keys off `[data-mui-color-scheme="dark"]` as it does in production instead of the `.mode-dark` fallback. The light screenshot is byte-identical either way. The mode storage key is per scheme: the route loop reuses one page, and a mode persisted by one route would otherwise carry into the other. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…into claude/docsearch-argos-fixture
The start screen shows stored recent searches, which carry no `_snippetResult`, so the baseline had no highlighted matches, no breadcrumbs and no tree connectors -- the markup most likely to shift between DocSearch majors. Answer the search request from `index.test.js` with a canned `/1/indexes/*/queries` response and screenshot the results screen. Stubbing at the network layer keeps `AppSearch` untouched and still exercises the real search client, `transformItems` and hit rendering. It also keeps the baseline off the live index, which is recrawled regularly. Six baselines now: light and dark, each closed, open and with results. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds visual regression coverage for the real DocSearch-based AppSearch UI across light and dark themes.
Changes:
- Adds fixtures for closed, start, and results states.
- Stubs Algolia responses for deterministic screenshots.
- Configures theme, storage, CSS layers, and search index.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
test/regressions/vite.config.mts |
Defines the fixture search index. |
test/regressions/index.test.js |
Captures modal and results screenshots. |
test/regressions/fixtures/AppSearch/SearchModalDark.tsx |
Adds the dark fixture route. |
test/regressions/fixtures/AppSearch/SearchModal.tsx |
Adds the light fixture route. |
test/regressions/AppSearchFixture.tsx |
Provides production-like AppSearch setup. |
test/regressions/algoliaSearchStub.js |
Supplies deterministic search results. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Two items from review. The dedicated modal tests inherit the suite's 1000px viewport, so they could not reach the rule that motivated half of this fixture: below 768px DocSearch lets the hit title and path wrap instead of truncating. Cropping to the modal does not help, since media queries read the viewport. Add a results capture at 767px, which keeps the modal at the width our `max-width` override gives it on desktop so the typography is the only thing that differs. The stub's content hit is now long enough to actually wrap; a string that fits on one line rendered the same either way. The open capture waited on `.DocSearch-Modal` and the lazy stylesheet, and neither says anything about the two areas it screenshots. The stylesheet request starts when `AppSearch` mounts, well before the modal opens, so that wait can already be satisfied. Wait for a stored hit and a start-screen item instead. That surfaced a real ordering bug: the seed was written by the fixture on mount, but `pooled` clears storage on acquisition and navigating to a route the page already rendered does not remount it, so the modal could open with no recent searches. The previous baselines were luck. Seed from the test instead, with the data and the index name shared through `docsearchFixtureData`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Pooled pages only get their viewport and storage reset between tests, so the `@layer docsearch, mui;` style stayed in `<head>` for every later test on that page. It is inert for fixtures that put nothing in `docsearch`, but leaving it behind makes their layer order depend on which test ran first. Move it inside the existing `try` and drop it alongside the search stub. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Why
Argos renders
@mui/materialfixtures and docs demos. It never mounts the docs app shell, soAppSearchand the DocSearch overrides it carries have no coverage at all --grep -rl "AppSearch\|DocSearch" test/regressions/returns nothing. The only DocSearch test istest/e2e-website/material-docs.spec.ts, which types a query and clicks a result; it asserts nothing about layout.Two layout regressions reached review in #48983 that way:
padding-inline-start: 1emto the hit lists, indenting the result cards but not the section headings above them.Both are CSS-only, on class names we already override, and nothing in CI flagged either.
What
test/regressions/AppSearchFixture.tsxrenders the realAppSearch, with one route per colour scheme behind it.index.test.jsopens the modal and screenshots it; the route loop also captures the closed search button. Four baselines in total: light and dark, closed and open.Dark mode is covered because
AppSearchcarries a secondGlobalStylesblock just for it, and that is what the v3 -> v4 migration broke. The fixture uses the docs theme throughgetThemerather thanBrandingProvider, so it carriestheme.varsand the dark block keys off[data-mui-color-scheme="dark"]exactly as in production rather than the.mode-darkfallback.Both screens are covered. The start screen comes from
localStorageseeded with two stored recent searches, in the exact shape DocSearch persists. The results screen comes from a canned/1/indexes/*/queriesresponse thatindex.test.jsfulfills throughpage.route, so the real search client,transformItemsand hit rendering all run, but the baseline never touches the live index. Stubbing at the network layer meansAppSearchneeds no test-only prop.That matters because stored recent searches carry no
_snippetResult. Without the results screen there would be no<mark>anywhere in the baseline, and highlighted matches, breadcrumbs and the tree connector between a section and its children are the markup most likely to shift between DocSearch majors.Eight baselines: light and dark, each closed, open, with results, and with results at 767px. The narrow capture is what reaches the rule that motivated half of this fixture -- below 768px DocSearch lets the hit title and path wrap instead of truncating, and media queries read the viewport, so cropping to the modal never gets there. 767px keeps the modal at the width our
max-widthoverride gives it on desktop, leaving the typography as the only thing that differs.Two things the regression bundle does not give us that the docs do:
BrandingCssVarsProviderdeclaresdocsearchahead ofmui. Without it the DocSearch stylesheet outranks every override and the modal renders unstyled. A layer's position is fixed where it is first named, and emotion keeps prepending its tags above everything in<head>as components mount, so the only stable point is after the modal has rendered. That is why the declaration lives in the test rather than the fixture.CssBaseline.TestViewersetsbox-sizing: content-boxon purpose, to catch components that lean on the docs' reset. With the layer order corrected that now beats theborder-boxDocSearch sets on its own subtree, and the modal overflows. The docs renderCssBaseline, so the fixture does too.Notes
The shared setup lives at
test/regressions/AppSearchFixture.tsxrather than underfixtures/, where the eager glob turns every file into a route.MarketingWrapper.tsxsets the same precedent.Based on
master, so the baseline is DocSearch v4. #48983 should then show the v4 -> v5 diff in Argos once it picks this up.test:regressions:devcurrently fails to boot on an unrelatedsinondep-optimization error, so this was verified throughtest:regressions:buildplustest:regressions:server, which is what CI runs.One known wobble: the light desktop results screenshot is not byte-identical between runs. The difference is 50 pixels on a single row, one unit apart in the green channel -- the underline under a highlighted match. It sits half inside the
overflow: hiddenclip on.DocSearch-Hit-title, so it composites at fractional pixel coverage and Chromium rounds it either way. Two consecutive captures of the same DOM differ, so it is rasterization rather than anything the test can wait for. It is invisible and far below any sane diff threshold, but worth knowing about, and it settles once #48983 lands, since the fix there gives the underline real room instead of half a pixel. Byte hashes are reproducible within a session but shift between sessions for every screenshot in the suite, the pre-existing ones included, which is why Argos compares perceptually rather than by hash.