PER-8985 feat: toHaveScreenshot drop-in — route existing Playwright assertions through Percy#640
Open
Shivanshu-07 wants to merge 15 commits into
Open
PER-8985 feat: toHaveScreenshot drop-in — route existing Playwright assertions through Percy#640Shivanshu-07 wants to merge 15 commits into
Shivanshu-07 wants to merge 15 commits into
Conversation
…sertions through Percy
Adds @percy/playwright/dropin: one require line in playwright.config.js
overrides Playwright's toHaveScreenshot() so every existing screenshot
assertion is captured and uploaded to Percy, with the suite never failing on
a Percy problem (missing token / CLI down → the whole run transparently falls
back to native toHaveScreenshot).
Registration writes the matcher into Playwright expect's internal userMatchers
(call-time dispatch spreads them after built-ins) because the public
expect.extend() silently ignores built-in matcher names on the shared
instance; on an unrecognized expect shape it degrades to extend() WITH a loud
warning — never a silent no-op. Verified working on @playwright/test 1.60/1.61.
Capture modes (config file .percy-playwright-dropin.json):
- screenshot (default): the Playwright-stabilized PNG is uploaded as a raw
comparison (generic/app projects); snapshot identity (name/browser/width)
mirrors Playwright's on-disk baseline naming byte-for-byte, and tag
dimensions are parsed from the PNG (percy-api validates height).
- snapshot: serialized-DOM web snapshot for web projects — DELEGATES to this
repo's own captureDOM, so the drop-in inherits the readiness gate,
responsive capture and cross-origin iframe handling and cannot drift from
percySnapshot(). Locator subjects become element-scoped snapshots via a
transient data attribute + scope.
First-build-as-baseline: the bundled percy-playwright wrapper bin flags the
run's build as a baseline candidate (PERCY_DROPIN_BASELINE_CANDIDATE, mirrors
PERCY_BUILD_SOURCE); percy-api decides first-ness server-side, and on a
genuine first build the dropin globalSetup seeds the repo's committed
Playwright baseline PNGs as the build's content (8-way parallel) while the
override skips live captures — the server auto-approves it (flag-gated), so
diffs start on the very next run. Also ships the opt-in fail-on-changes gate
reporter and sync-assertion mode.
Compat fixes surfaced by the @playwright/test 1.24→1.60 devDep bump:
- ENV_INFO's top-level require('playwright/package.json') crashes at import
time under the 1.61 runner (CJS↔ESM interop) — now guarded, degrades to
@playwright/test's version or a bare label
- the dropin↔root require is lazy for the same interop reason
Tests: dispatch specs drive the REAL Playwright expect through the override
and hard-assert the posted comparison (retry-wrapped against the shared
testing-server reset race); unit specs pin name derivation, first-build
seeding and locator scoping. 117 passed, lint clean.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The toHaveScreenshot drop-in needs modern @playwright/test (expect internals), whose engine floor is Node 18. Runtime support for the core SDK entry is unchanged; this bumps the dev/CI toolchain only. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Playwright >=1.39 no longer downloads browsers as a package postinstall; the suite failed on CI with 'Executable doesn't exist'. Fetch the launched browser (with system deps) after yarn install. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ope nyc to the core surface nyc instruments the arrow functions dropin/dom.js ships to the browser via locator.evaluate, and the injected coverage counters don't exist in the page (ReferenceError: cov_… is not defined) — annotate them with istanbul ignore, the same convention index.js/utils.js already use for browser-executed code. Exclude dropin/ and bin/ from the 100% coverage gate for now: the modules are covered by the standalone package's 154-test unit suite, and porting that suite into this harness is a flagged fast-follow. The existing core surface keeps its 100% bar unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Which fallback executes depends on the consumer's installed packages — the defensive branches can't all be exercised in one environment and were the last gap under the 100% coverage gate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Node 18 went EOL in April 2025; its leg also mis-collects subprocess coverage for utils.js under the percy-exec harness (identical 117-test pass count, different coverage) — not worth chasing on a dead runtime. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Resolves the 7 blocking semgrep findings (path-join-resolve-traversal, CWE-22): NUL bytes are stripped from any externally supplied directory before it reaches path.join, and directory-walk entry names must be a single path component (no separators, no dot/dot-dot) or the entry is skipped. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… the SDK's own flows
The capture flow is no longer a user config (captureMode removed): the
override reads the project type from the CLI healthcheck and delegates to
the flow that project accepts —
web (default) -> percySnapshot() (serialized-DOM, server-side render;
dom.js seam adds Locator scoping + viewport pinning)
automate -> percyScreenshot() (Percy on Automate)
app/generic -> raw-PNG upload via the comparison ingest (capture.js)
The snapshot seam now calls the repo's percySnapshot() directly instead of
re-assembling the postSnapshot options, inheriting closed-shadow-root
piercing, readiness gate, responsive capture and CORS iframes with zero
duplication. The committed-baseline seed applies only to the raw-PNG flow
and skips itself on web/automate projects.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Per design review: the drop-in is WEB-ONLY — every toHaveScreenshot() is a percySnapshot web snapshot; an app/automate token now raises a clear configuration error (validateConfig), like other SDKs' wrong-token errors. Baseline seeding moves OUT of the test process into the CLI: - dropin/baseline/provider.js — the '@percy/cli'.baselineProvider hook percy exec discovers: resolves the Playwright config outside a test run (a config-capture reporter under 'playwright test --list', so TS configs work), maps committed *-snapshots PNGs onto Percy identity, reads tag heights from PNG headers. - commands/setup-baseline.mjs — 'percy playwright:setup-baseline', the explicit (re-)baseline command for ESTABLISHED projects: uploads committed PNGs into a build created with dropin-baseline-setup (auto-approved server-side), no tests run. Reuses @percy/cli-exec's upload loop. - retired: the percy-playwright wrapper bin, the globalSetup seed (dropin/global-setup.js + first-build.js), the matcher's seeded-baseline skip, and the BYOS/automate dispatch paths (capture.js). First-build detection for the sync classifier now keys on the server-decided build source from the healthcheck. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…type Correction to the web-only change: the drop-in supports BOTH project types. Web tokens take serialized-DOM snapshots (render flow); app tokens upload the captured PNG straight through the comparison ingest with no render flow, exactly how App Percy ingests screenshots (capture.js restored). Only non-web/non-app tokens (automate, generic, ...) raise the wrong-token configuration error. setup-baseline and the seeding provider accept app tokens too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
# Conflicts: # package.json
'enabled: false' in .percy-playwright-dropin.json (or PERCY_DROPIN_DISABLE=true, which also turns off percy exec's baseline seeding) makes toHaveScreenshot() delegate to the ORIGINAL matcher with pure native semantics — including the missing-baseline throw, since this is a deliberate opt-out rather than the protective disabled-Percy fallback. percySnapshot()/percyScreenshot() keep working under percy exec. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The SDK previously relied on the consumer's hoisted copy (via @percy/cli), so an older tree silently lacked mergeSnapshotOptions and every snapshot degraded per-assertion. A direct floored dependency guarantees a compatible copy regardless of what the consumer hoists. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…hared-server reset race Same toPass retry the other post+read-back specs use — the shared testing server's log store is wiped by parallel workers' setupTest resets. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…h + hardening
High:
- reporter: onEnd returns { status: 'failed' } — Playwright's process.exit()
discards process.exitCode, so the gate could never red CI
- reporter: pending-grace probe — under percy exec the build finalizes
after the test process exits, so waitForBuild stalled ~10min and the
gate silently never fired; now bows out with build:wait guidance
- identity: browserFamily keys on use.browserName (falls back to project
name) — display names like 'Desktop Chrome' never paired with
baselines discovered under 'chromium'
Medium:
- config validation errors latch and re-throw on every assertion (was:
only the first assertion surfaced the error)
- isFailing: informational mode never reds CI, incl. failed/errored builds
- identity: port Playwright's trimLongString — >100-char anonymous stems
are trimmed on disk and must trim identically head-side
- setup-baseline: friendly upgrade error when @percy/cli lacks
uploadBaselines
Low:
- resolve-config: Windows-safe spawn (node + @playwright/test/cli, not npx)
- wire config.snapshotDir through the reporter into discovery
- .not.toHaveScreenshot() honors the always-pass posture (pass inversion)
- skip app posts when PNG dimensions are unreadable (server rejects
heightless tags)
- drop dead code (base-branch.js, runNativeMatcher, isWriteOnlyToken),
refresh stale types/index.d.ts, align engines/peer floors (node>=16,
@playwright/test>=1.49)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
Author
Claude Code PR ReviewPR: #640 • Head: ed2682a • Reviewers: fallback inline checklist (deep-review agent) SummaryAdds the Review Table
FindingsAll review findings were fixed on this PR before this report (commit ed2682a):
Verdict: PASS |
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
1 participant
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.
What
Ships the
toHaveScreenshot()drop-in inside@percy/playwright(require('@percy/playwright/dropin')— one config line, zero test changes). Every existing Playwright visual assertion routes through Percy:percySnapshot: readiness gate, responsive capture, CORS iframes; Locator subjects become scoped snapshots), rendered server-side.Assertions always pass locally (verdict moves to Percy review); Percy being down falls back to native
toHaveScreenshotfor the whole run; optional sync mode and a CI gate reporter are included.Baseline seeding (CLI-driven)
dropin/baseline/provider.js— the"@percy/cli".baselineProviderhookpercy execdiscovers: resolves the Playwright config outside a test run (config-capture reporter underplaywright test --list, so TS configs work) and maps committed*-snapshotsPNGs onto Percy identity.commands/setup-baseline.mjs—percy playwright:setup-baseline, the explicit (re-)baseline command for established projects (storybook-style command registration).Depends on
Testing
126 specs green (dispatch, identity, provider, config-resolution units + real-runner integration); E2E verified locally against percy-api and on staging (web + app projects, all four flows).
🤖 Generated with Claude Code