test(visual): add Playwright + Chromatic page visual regression suite#18009
Open
pettinarip wants to merge 32 commits intodevfrom
Open
test(visual): add Playwright + Chromatic page visual regression suite#18009pettinarip wants to merge 32 commits intodevfrom
pettinarip wants to merge 32 commits intodevfrom
Conversation
Integrated review findings: - Merged Unit 3 (scripts) into Unit 1 — not independently deliverable - Clarified two-job CI rationale: enables partial archive upload on failure - Resolved getGFIs() open question — fetch-gfis.json covers it via USE_MOCK_DATA - Documented reuseExistingServer caveat for local dev determinism - Clarified catch-all page coverage in Unit 2
Use --frozen-lockfile on both pnpm installs so CI cannot silently drift from the committed lockfile. Also drop the unneeded `if: always()` on chromatic-upload; it was only ever reachable via `needs`, so the default already gates it on the build job producing artifacts to upload.
networkidle is flaky on Next.js pages due to analytics beacons and prefetch — the explicit stable-selector + nav-skeleton-detach waits already gate readiness, so domcontentloaded is enough. Also default stableSelector to "h1" in the loop destructure and drop the 15 redundant literals from the page table.
✅ Deploy Preview for ethereumorg ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
github-actions
Bot
added
dependencies 📦
…tic-page-visual-tests
pettinarip
force-pushed
the
feat/playwright-chromatic-page-visual-tests
branch
from
67da493 to
586226c
Compare
The previous merge of origin/dev preferred dev's lockfile, which dropped @chromatic-com/playwright added by this branch. Regenerated to restore the missing entry and unblock unit-tests CI.
The webServer block ran pnpm start for every project, breaking unit tests in CI which don't need (or have) a production build. Gate it on chromatic project args so unit and e2e runs skip it.
Plans under docs/plans/ are local working artifacts and should not be committed alongside feature work.
…tic-page-visual-tests
…tic-page-visual-tests # Conflicts: # app/[locale]/_components/HomepageLazy.tsx # pnpm-lock.yaml
pettinarip
force-pushed
the
feat/playwright-chromatic-page-visual-tests
branch
from
0feba3f to
1f2458f
Compare
pettinarip
force-pushed
the
feat/playwright-chromatic-page-visual-tests
branch
from
ed0cbf5 to
306594f
Compare
This reverts commit e3a6105.
…tic-page-visual-tests # Conflicts: # app/[locale]/_components/HomepageLazy.tsx # src/components/EthPriceCard.tsx
…tic-page-visual-tests # Conflicts: # pnpm-lock.yaml
myelinated-wackerow
approved these changes
Apr 28, 2026
Collaborator
myelinated-wackerow
left a comment
myelinated-wackerow
left a comment
There was a problem hiding this comment.
LGTM on the visual-test infrastructure. A few actionables:
- PR description says "tests exercise the same webpack output the site ships" — stale after the #17906 merge. The workflow runs
pnpm build, which is now Turbopack. Either update the description, or pin the workflow topnpm build:webpackif webpack-specific output matters for snapshot fidelity. last-24-hrstranslation key is orphaned —EthPriceCardno longer renders the 24hr % change, so the key is unused in source code but still present insrc/intl/{locale}/common.jsonacross all 25 locales. Remove it from the namespace if the deprecation is intentional.- EthPriceCard quietly drops the 24hr % change indicator (arrow + percentage + colored gradient). Probably worth a one-line mention in the PR body so it's not invisible to reviewers and stakeholders watching the homepage.
- Naming nit on
maybeShuffle—safeShufflereads better; the "safe" framing makes the intent (deterministic for visual tests) clearer than the conditional-sounding "maybe." - Minor:
reuseExistingServer: trueinplaywright.visual.config.ts— fine for CI, but locally apnpm devon :3000 will be silently used in place of the production build the tests assume. Worth a one-liner in the skill doc's "Common situations" section.
Skill doc is genuinely useful — covers the non-obvious gotchas (1024 viewport, dual config, data-slot contract, HOME: /root).
Reviewed by Claude (Opus 4.7)
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
2 participants
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.
Summary
src/layouts/) at three viewports and uploads DOM archives to Chromatic.chromatic-pagesGitHub Actions workflow that builds the production app with mock data, runs Playwright, and hands off archives to Chromatic — aligned with the official Playwright + Chromatic pattern.pnpm buildwithUSE_MOCK_DATA=trueandNEXT_PUBLIC_BUILD_LOCALES=enso tests exercise the same webpack output the site ships, deterministically.data-slot="loading"contract on the sharedSkeletonandSpinnerprimitives so tests wait on one signal for all loaders to settle before snapshotting — no per-page hydration selectors.Test plan
chromatic-pagesworkflow succeeds on this PR and publishes a Chromatic buildsrc/layouts/chromatic.ymlStorybook workflow