PER-8985 feat: Percy drop-in example for Playwright toHaveScreenshot() - #16
Conversation
One-line config registration routes plain toHaveScreenshot() assertions through Percy — the spec imports nothing from Percy. Pins @playwright/test 1.61.1 (the drop-in's matcher injection does not engage on 1.53) and the beta floors for @percy/cli / @percy/playwright. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
aryanku-dev
left a comment
There was a problem hiding this comment.
Nice example — the spec really does read as plain Playwright with zero Percy imports, which is exactly the point of the drop-in. A few things I'd want resolved before this merges, the first one being a blocker.
1. Beta floors at the repo root break npm install for everyone (blocker).
@percy/playwright@1.1.2-beta.0 declares peerDependencies: { "@percy/cli": ">=1.32.3" }, and the lockfile resolves @percy/cli@1.32.6-beta.2. Under semver, a prerelease does not satisfy a range with no prerelease component:
$ npx semver 1.32.6-beta.2 -r ">=1.32.3" # -> no match, exit 1
$ npx semver 1.32.6 -r ">=1.32.3" # -> 1.32.6, exit 0
That's precisely why the README has to tell people --legacy-peer-deps. But the repo's .npmrc pins legacy-peer-deps=false, and README Step 1 of the tutorial still says plain npm install — so a fresh clone should now ERESOLVE at the very first step of the getting-started flow.
Green CI isn't evidence against this: the basic job runs npm ci, which installs the already-resolved tree from the lockfile and never re-runs peer resolution. Could you confirm with rm -rf node_modules && npm install on a clean clone?
2. Related — should this be its own sub-example rather than a root-level bump?
The repo already has the precedent in advanced/: self-contained package.json + package-lock.json + playwright.config.js. Moving this to dropin/ would let the drop-in pin whatever betas it needs while the root example — the one the README calls "Minimum viable integration. Start here." — stays on GA @percy/cli/@percy/playwright and keeps a clean npm install. It also localizes the eventual beta→GA cleanup to one directory, and makes issue #1 disappear entirely. Was there a reason to bump the root instead?
3. Nothing in CI exercises this.
.github/workflows/test.yml's basic job runs npm test, and package.json has no test script — so that step is a no-op, and test-dropin is never invoked anywhere. The whole point of an example repo is that the examples are known-good; right now the drop-in path only has your manual local verification behind it. Worth adding a job (or a dropin job mirroring the advanced one) so a future SDK release can't silently break it.
4. The headline feature isn't actually demonstrated.
The README leads with first-run baseline seeding, but no *-snapshots directory is committed, so a reader who clones and runs npm run test-dropin hits the empty-baseline path, not the seeding path. Committing the generated baselines would make the flagship behaviour work out of the box — and would let CI assert it. Relatedly, please confirm what a zero-baseline first run does here: vanilla toHaveScreenshot() writes the actual and fails the test on first run, so the README's "Your suite passes as usual" needs to be true of the drop-in's interception path too.
Also, .gitignore only covers /node_modules — playwright test writes test-results/ and playwright-report/, so following the walkthrough leaves an untracked mess (and the suggested git add tests/dropin could sweep artifacts in).
Inline notes on the smaller stuff below.
…e; add Percy on Automate drop-in example - package.json: exact pins (repo is save-exact) and an npm overrides entry so a plain npm install resolves against the SDK's non-prerelease peer range — removes the --legacy-peer-deps requirement entirely - playwright.dropin.config.js: webServer starts the TodoMVC server once per run (specs no longer bind ports); viewport comment rewritten to state the actual seeding identity contract - README: drop-in section moved before the Automate tutorial, indexed in the Examples table, npx fix, Node 18+ (Playwright 1.61 engines), Versions note - New: tests/dropin-automate + playwright.dropin.automate.config.js — the drop-in on a Percy on Automate project via a BrowserStack session fixture Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
All review feedback addressed in 13f2faf, plus the Percy on Automate example added: Review fixes
New: drop-in on Percy on Automate
Verified against production on this head: web drop-in build #988 (2 snapshots via the webServer variant), automate drop-in build #63 (remote capture on a live BrowserStack session, 🤖 Generated with Claude Code |
Plain npm install verified on a fresh clone — the overrides entry keeps the prerelease peer range satisfied with no flags needed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Drops the beta-only npm overrides entry — the stable versions satisfy the SDK's peer range natively, so a plain npm install just works. NOTE: merge only after the v1.32.6 / v1.1.2 GitHub Releases are published to npm (package-lock regeneration + install verification pending that).
… walkthrough package-lock now resolves @percy/cli@1.32.6 + @percy/playwright@1.1.2 from npm latest (plain npm i, no overrides). README seeding walkthrough reframed for standard single-config projects — baseline discovery reads Playwright's default config, which this multi-example repo deliberately doesn't use.
|
Stable releases are live — this PR is now verified against them end to end, exactly as a customer would run it (1453a33):
One scoping note added to the README: baseline discovery reads Playwright's default config, and this repo deliberately keeps the drop-in config under a custom filename to coexist with the other examples — so the auto-seed/ 🤖 Generated with Claude Code |
Percy's baseline discovery (first-run seeding and playwright:setup-baseline) reads Playwright's DEFAULT config — configs loaded via --config are invisible to it, which made the seeding walkthrough impossible in this repo. The drop-in config is now the default config (testDir ./tests, drop-in example filtered by path), with its static server moved to port 8001 so the percySnapshot example keeps its own server on 8000. Verified: setup-baseline now discovers and maps 2/2 (previously project_missing_browser_or_viewport); test-dropin and test-web both green with the default config present. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
What
Adds a working example of the Percy drop-in for Playwright's built-in
toHaveScreenshot()assertions (shipping in@percy/playwright@1.1.2-beta.0+@percy/cli@1.32.6-beta.1):playwright.dropin.config.js— the one required line (require('@percy/playwright/dropin')) plus an explicitbrowserName/viewportso first-run baseline seeding can map committed screenshotstests/dropin/todomvc_tohavescreenshot.spec.js— plain Playwright visual assertions against the repo's TodoMVC app, zero Percy imports (page subject + locator subject)npm run test-dropinscript and a README section covering the run command, first-run baseline seeding (incl. the full-access-token recommendation),playwright:setup-baseline, and the beta install caveat (--legacy-peer-deps)Version notes
@playwright/testbumped 1.53.2 → 1.61.1 for the whole repo. Verified working end to end on 1.61.1 (build finalized, both snapshots posted).Percy: could not inject the toHaveScreenshot override into this Playwright version…) — flagged separately as an SDK compat follow-up; the README states 1.60–1.61 until that's resolved.@percy/cli/@percy/playwrightfloors set to the released betas (caret ranges roll forward to the GA versions).Testing
PERCY_TOKEN=<web token> npm run test-dropin→capture=snapshot (web)mode line,Snapshot taken: todomvc-initial+todomvc-new-todo, build finalized (verified against production).🤖 Generated with Claude Code