Repository navigation
fix(percy): restore Percy capture on WebdriverIO (screenshot + snapshot) - #210
Conversation
Percy capture has been silently disabled for any install resolving
@percy/selenium-webdriver 2.2.6 or newer (published 2026-04-01). Tests still
passed, so the only symptoms were a log line and missing visual coverage.
2.2.6 added a top-level require('selenium-webdriver') for a Selenium-only
iframe helper, but that package is shipped as a devDependency only and is never
present in a WebdriverIO project, so the module throws MODULE_NOT_FOUND on load.
tryRequire swallowed the throw and left the Percy entry points as no-op stubs
logging "Unsupported driver for percy" — a message about the driver for what was
actually a failed module load. Our floating "^2.2.2" range is what admitted the
broken version, on the v9 and v8 lines alike.
- Cap @percy/selenium-webdriver below 2.2.6.
- Log the underlying load failure instead of discarding it, so a future
packaging break surfaces as itself.
- Route all three Percy entry points through a shared helper that logs errors
rather than propagating them into the user's test. Percy raises its misuse
guards before its own try/catch, so capping alone would have started throwing
from snapshot/screenshot calls. PERCY_RAISE_ERROR is honoured for users who
want failures to fail the build.
- Document Percy in the README for the first time: the percy and
percyCaptureMode options, that this service runs Percy on Automate, and how to
drive a Percy web project alongside it.
App Automate is unaffected — it uses @percy/appium-app, a separate package.
The v8 line declares the same floating range and needs the equivalent change on
the v8 branch; this covers v9 only.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Percy released 2.2.8, which guards the require that broke module load, so the cap added in the previous commit is no longer needed and the range returns to ^2.2.2. The lockfile is refreshed onto 2.2.8. That leaves the second, older defect. The service routes its `snapshot` (Percy web) export to @percy/selenium-webdriver's percySnapshot, which drives the browser through Selenium client APIs — executeScript(script) with a single argument, By, switchTo(). A WebdriverIO browser provides none of those, so the call fails on the first driver interaction, the SDK swallows the failure, and zero snapshots are posted. That path has never worked on WebdriverIO at any version. @percy/webdriverio is the WebdriverIO-native port and is what `snapshot` now binds to. percyScreenshot (Percy on Automate) deliberately stays on @percy/selenium-webdriver: it is driver-agnostic — it reads session metadata and posts, with capture happening server-side — and carries an explicit wdio branch in its DriverMetadata. @percy/webdriverio is pinned below 3.3.3; that release omits a file its own entry point requires and fails to load. Verified on a live BrowserStack WebdriverIO v9 session against a web-typed Percy runtime: through the service's own export, snapshot posts one snapshot carrying a 594-byte serialized DOM, where the Selenium SDK on the identical browser posts none and logs "Wrong parameters applied for executeScript". Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
harshit-browserstack
changed the title
|
🔴 SDK PR Review gate is red. Pending:
It turns green once the SDK PR Review Agent has run on the current head commit (any verdict — the gate only requires that the review ran). A native reviewer approval is separately required by branch protection before merge. |
1 similar comment
|
🔴 SDK PR Review gate is red. Pending:
It turns green once the SDK PR Review Agent has run on the current head commit (any verdict — the gate only requires that the review ran). A native reviewer approval is separately required by branch protection before merge. |
Process integrityThe review gate could NOT certify this run — the verdict is held at
Change map (generated deterministically from the diff)graph LR
subgraph nwdio_service["wdio-service"]
npackages_browserstack_service_src_Percy_PercySDK_ts["PercySDK.ts<br/>~59 lines"]
n_changeset_pr_210_md["pr-210.md<br/>~21 lines"]
npackages_browserstack_service_README_md["README.md<br/>~21 lines"]
npackages_browserstack_service_package_json["package.json<br/>~11 lines"]
end
↻ This verdict comment is the review anchor — it's updated in place on each run (the gate posts its status separately). — SDK PR Review Agent |
|
🟢 SDK PR Review gate is green — the SDK PR Review Agent has run on the current head commit (verdict: This gate confirms a review ran on the latest commit. The verdict itself is advisory — read the findings and use your judgement; it does not block merge. A native GitHub reviewer approval is still separately required by branch protection before this PR can merge. |
1 similar comment
|
🟢 SDK PR Review gate is green — the SDK PR Review Agent has run on the current head commit (verdict: This gate confirms a review ran on the latest commit. The verdict itself is advisory — read the findings and use your judgement; it does not block merge. A native GitHub reviewer approval is still separately required by branch protection before this PR can merge. |
|
🟢 SDK PR Review gate is green — the SDK PR Review Agent has run on the current head commit (verdict: This gate confirms a review ran on the latest commit. The verdict itself is advisory — read the findings and use your judgement; it does not block merge. A native GitHub reviewer approval is still separately required by branch protection before this PR can merge. |
|
RUN_TESTS |
|
RUN_SCA |
The disabled-path stub stayed synchronous while its enabled branch became async, so TypeScript inferred the export from the initial assignment and published `snapshot` as `=> void` while `screenshot` and `screenshotApp` were `Promise<unknown>`. Consumers awaiting it awaited a void. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
0939a85
|
🔴 SDK PR Review gate is red. Pending:
It turns green once the SDK PR Review Agent has run on the current head commit (any verdict — the gate only requires that the review ran). A native reviewer approval is separately required by branch protection before merge. |
|
RUN_TESTS |
|
🟢 SDK PR Review gate is green — the SDK PR Review Agent has run on the current head commit (verdict: This gate confirms a review ran on the latest commit. The verdict itself is advisory — read the findings and use your judgement; it does not block merge. A native GitHub reviewer approval is still separately required by branch protection before this PR can merge. |
|
🟢 SDK PR Review gate is green — the SDK PR Review Agent has run on the current head commit (verdict: This gate confirms a review ran on the latest commit. The verdict itself is advisory — read the findings and use your judgement; it does not block merge. A native GitHub reviewer approval is still separately required by branch protection before this PR can merge. |
1 similar comment
|
🟢 SDK PR Review gate is green — the SDK PR Review Agent has run on the current head commit (verdict: This gate confirms a review ran on the latest commit. The verdict itself is advisory — read the findings and use your judgement; it does not block merge. A native GitHub reviewer approval is still separately required by branch protection before this PR can merge. |
AakashHotchandani
left a comment
AakashHotchandani
left a comment
There was a problem hiding this comment.
SDK PR Review — findings at 0939a858
Verdict: 🔴 Fix 2 blocking issues
0 critical · 2 warning(s) · 3 suggestion(s) · 0 needing human review
5 finding(s) inline. The signed verdict comment carries the merge-gate state.
— SDK PR Review Agent
| "@grpc/grpc-js": "~1.13.5", | ||
| "@percy/appium-app": "^2.0.9", | ||
| "@percy/selenium-webdriver": "^2.2.2", | ||
| "@percy/webdriverio": ">=3.3.0 <3.3.3", |
There was a problem hiding this comment.
⚠️ Warning — [CONFIG] Defect A fix lives only in the lockfile; the declared range still admits broken 2.2.6/2.2.7
Problem
The release note says "Fixed Percy capture on WebdriverIO", but the fix for Defect A is only the package-lock.json move 2.2.6 → 2.2.8. Customers resolve the declared range, and packages/browserstack-service/package.json:60 (the line two above this anchor) still reads "@percy/selenium-webdriver": "^2.2.2", which admits the unloadable 2.2.6 and 2.2.7.
The PR body confirms the consequence: "a lockfile pinned below 2.2.8 does not self-heal on the next npm install — npm keeps a locked resolution while it satisfies the declared range". Those customers upgrade to this version and keep MODULE_NOT_FOUND → stubbed screenshot → no Percy coverage. Acknowledging the gap in the body does not close it.
Suggested Fix
Raise the floor so the broken window is excluded for every install:
"@percy/selenium-webdriver": "^2.2.8",2.2.8 is the version the lockfile already resolves and whose index.js:14-21 guards require('selenium-webdriver'). Apply the same floor on the v8 line.
Impact / likelihood: medium — A customer whose lockfile already resolved 2.2.6 or 2.2.7 upgrades to this release and still gets 'Unsupported driver for percy' and zero Percy screenshots - the exact Defect A symptom the release note says is fixed. · medium — Any existing project that installed or refreshed its lockfile while 2.2.6/2.2.7 were latest (from 2026-04-01) and upgrades normally keeps that resolution; fresh installs are fine.
Confidence: 🟢 — package.json:60 keeps "@percy/selenium-webdriver": "^2.2.2", which admits 2.2.6/2.2.7 by semver; the only change to the resolved version is in package-lock.json, which a published library does not ship; the PR body states the locked resolution does not self-heal.
— SDK PR Review Agent
| testCase: name || '' | ||
| } | ||
| return percySnapshot(browser, snapshotName, options) | ||
| return await runPercy(`snapshot "${snapshotName}"`, () => webSnapshot(browser, snapshotName, options)) |
There was a problem hiding this comment.
⚠️ Warning — [CORRECTNESS] snapshot's @percy/webdriverio fix never takes effect on a run the service manages
Problem
The changeset says "Fixed Percy web snapshots, which previously captured nothing on WebdriverIO", but the rebound snapshot only runs when process.env.PERCY_SNAPSHOT === 'true' (PercySDK.ts:66), and no product configuration reaches @percy/webdriverio with a web project:
percyon:launcher.ts:240-243setsBEST_PLATFORM_CID,service.ts:139-140setsPERCY_SNAPSHOT, butPercy.ts:132always requeststype=automate(orapp) andPercy.ts:88overridesPERCY_TOKENwith that project's token.@percy/webdriverio@3.3.2index.js:40-41throwsYou are using Percy on Automate session with WebdriverIO...fortype === 'automate', so every call ends inrunPercy'sPercy snapshot ... failedlog.percyunset (the README's own web-project recipe):BEST_PLATFORM_CIDis never set, soWDIO_WORKER_ID === BEST_PLATFORM_CIDis false,PERCY_SNAPSHOTstays unset, andsnapshotreturnsundefinedwithout even a debug log.
The only way to reach the new binding is a hand-set PERCY_SNAPSHOT=true plus an externally started web-type Percy server, which is what the author's local-stub verification needed. The PR body itself notes the existing cases 'all run percy: true, which provisions an automate project where snapshot is misuse by design'. The release note therefore claims a fix customers cannot obtain through this service.
Suggested Fix
Either make the export reachable for web projects, or narrow the claim:
- Open the gate for the web path: in
snapshotHandler, run whenPERCY_SNAPSHOT === 'true'or when the service did not start Percy (process.env.BROWSERSTACK_PERCY !== 'true'), and let@percy/webdriverio's ownisPercyEnabled()decide; log at debug when skipping so a no-op is not silent. - Or drop the changeset line "Fixed Percy web snapshots" and document in the README that
PercySDK.snapshotis not supported with this service, pointing to@percy/webdriveriodirectly (as the README already does).
This fix is unverified against launcher.ts BROWSERSTACK_PERCY setters on the CLI flow and against @percy/webdriverio isPercyEnabled behaviour under an automate server; confirm it before applying.
Impact / likelihood: medium — The changeset promises 'Fixed Percy web snapshots'; a customer calling PercySDK.snapshot still gets zero web snapshots - silently when percy is unset, or an error log when percy is on - which is the symptom this PR claims to fix. · high — Every service-managed run that calls PercySDK.snapshot: no configuration of the service both opens the PERCY_SNAPSHOT gate and yields a web-type Percy project.
Confidence: 🟢 — The gate chain is visible at HEAD: PERCY_SNAPSHOT is set only at service.ts:139-140 from BEST_PLATFORM_CID, which launcher.ts:240-243 sets only when the percy option is on; with percy on, Percy.ts:132 provisions an 'automate'/'app' project and @percy/webdriverio@3.3.2 index.js:40-41 throws for 'automate'.
— SDK PR Review Agent
| */ | ||
| const percyWebdriverioSnapshot = tryRequire('@percy/webdriverio', null) | ||
|
|
||
| const webSnapshot = percyWebdriverioSnapshot || percySnapshot |
There was a problem hiding this comment.
💡 Suggestion — [CORRECTNESS] snapshot falls back to the Selenium SDK that this PR documents as capturing nothing
Problem
If @percy/webdriverio fails to load, tryRequire logs at debug and returns null, and webSnapshot silently becomes percySnapshot from @percy/selenium-webdriver. The comment directly above (PercySDK.ts:25-28) says that binding "captures nothing and swallows the failure" on a WebdriverIO browser.
The fallback is therefore never a working alternative; it only hides the load failure behind the original Defect B symptom. The PR relies on the <3.3.3 upper bound to keep it unreachable, which the body calls load-bearing.
Suggested Fix
Drop the fallback so a missing @percy/webdriverio takes the existing stub path, which logs at error level:
const webSnapshot = percyWebdriverioSnapshotif (webSnapshot) at PercySDK.ts:64 then leaves the Unsupported driver for percy handler in place. Consider making that message name the package that failed to load.
Impact / likelihood: medium — When the fallback fires, the customer is back to zero snapshots with no error at error level, which re-creates the bug this PR fixes. · low — Needs @percy/webdriverio to fail to load inside the pinned >=3.3.0 <3.3.3 range (a broken install), or the pin to be relaxed onto an unloadable release, as the body says 3.3.3 is.
Confidence: 🟢 — The PR's own comment at PercySDK.ts:25-28 states that percySnapshot from @percy/selenium-webdriver captures nothing on a WebdriverIO browser and swallows the failure, and the PR body states the fallback 'silently restor[es] the exact defect this PR fixes'.
— SDK PR Review Agent
| fails the user's test rather than their visual coverage. PERCY_RAISE_ERROR is Percy's own | ||
| opt-in for the opposite behaviour and is honoured. | ||
| */ | ||
| const runPercy = async (label: string, call: () => unknown) => { |
There was a problem hiding this comment.
💡 Suggestion — [TEST COVERAGE] No test reaches any changed PercySDK branch
Problem
Every behavioural change in PercySDK.ts is verified only by manual runs (Jenkins, a local Percy stub). tests/Percy-Handler.test.ts:168-169 mocks PercySDK.screenshot and PercySDK.screenshotApp wholesale, and no other test imports the module (grep over 149/149 files at HEAD). Untested:
runPercy(PercySDK.ts:48-57): swallows and logs by default, rethrows underPERCY_RAISE_ERROR=true. Cheapest check: unit test with a rejectingcall.snapshotHandler(PercySDK.ts:64-79): binds to@percy/webdriverio, is gated onPERCY_SNAPSHOT, and passestestCase. Cheapest check:vi.mock('@percy/webdriverio')and assert it is called, not the Selenium SDK.webSnapshotfallback (PercySDK.ts:37): Selenium binding when@percy/webdriveriofails to load. Cheapest check: mock the require to throw.tryRequire(PercySDK.ts:16-18): debug log on load failure.screenshotHandler/screenshotAppHandler(PercySDK.ts:117-133): now async and wrapped. Cheapest check: a misuse-guard rejection resolvesundefinedand logs.
Suggested Fix
Add a tests/PercySDK.test.ts that mocks @percy/webdriverio, @percy/selenium-webdriver and @percy/appium-app with vi.mock, re-imports the module per case with vi.resetModules(), and covers the bullets above, including PERCY_RAISE_ERROR=true.
This fix is unverified against the repo's vitest module-reset conventions; confirm it before applying.
Impact / likelihood: low — Missing test evidence only; customer behaviour is covered by the other findings. · high — Applies to every changed function in this PR.
Confidence: 🟢 — No test at HEAD imports PercySDK except tests/Percy-Handler.test.ts, which replaces screenshot and screenshotApp with mocks at L168-169, so no test reaches any changed branch.
— SDK PR Review Agent
| screenshotHandler = (browser: WebdriverIO.Browser | WebdriverIO.MultiRemoteBrowser | string, screenshotName?: string | { [key: string]: unknown }, options?: { [key: string]: unknown }) => { | ||
| return screenshotHelper('web', browser, screenshotName, options) | ||
| screenshotHandler = async (browser: WebdriverIO.Browser | WebdriverIO.MultiRemoteBrowser | string, screenshotName?: string | { [key: string]: unknown }, options?: { [key: string]: unknown }) => { | ||
| return await runPercy('screenshot', () => screenshotHelper('web', browser, screenshotName, options)) |
There was a problem hiding this comment.
💡 Suggestion — [GRACEFUL DEGRADATION] runPercy makes percyAutoCapture's failure branch unreachable for screenshot errors
Problem
screenshot and screenshotApp are now wrapped in runPercy, which swallows the rejection. Their main internal caller, PercyHandler.percyAutoCapture (Percy-Handler.ts:51-75), relied on that rejection: its catch at L67-71 calls this._percyCaptureMap?.decrement(...) and PerformanceTester.end(AUTO_CAPTURE, false, err, ...).
After this change that branch runs only under PERCY_RAISE_ERROR=true. A failed capture is measured as a success and the per-session counter in PercyCaptureMap is not rolled back. The PR body describes the two screenshot entry points as "unchanged in behaviour", which holds for customers but not for this caller.
Suggested Fix
Keep the public exports guarded but let the internal caller see failures: have percyAutoCapture call an unwrapped internal helper (export screenshotHelper-based variants for internal use), or have runPercy return a { ok: false, error } sentinel that percyAutoCapture checks before its success path.
This fix is unverified against tests/Percy-Handler.test.ts spies on PercySDK.screenshot/screenshotApp; confirm it before applying.
Impact / likelihood: low — The customer's tests and Percy results are unaffected and the error is still logged at PercySDK.ts:55; what changes is SDK-internal: a failed auto-capture is recorded as a successful AUTO_CAPTURE/SCREENSHOT performance event and the capture-map counter is not rolled back, so later names in that session skip an index. · low — Only when percyScreenshot or percyAppScreenshot rejects during auto-capture, which needs a Percy-side failure first.
Confidence: 🟢 — Percy-Handler.ts:63 awaits PercySDK.screenshot/screenshotApp inside a try whose catch (L67-71) decrements the capture map and records a failed AUTO_CAPTURE; runPercy now resolves undefined on every error unless PERCY_RAISE_ERROR is set, so that catch is no longer reached for capture failures.
— SDK PR Review Agent
|
🔴 Blocking findings — fix required Findings: 0 critical · 2 warning(s) · 4 suggestion(s) · 0 needing human review — 1 posted in the findings review; 5 already on this PR from an earlier round, not repeated.
Change map (generated deterministically from the diff)graph LR
subgraph nwdio_service["wdio-service"]
npackages_browserstack_service_src_Percy_PercySDK_ts["PercySDK.ts<br/>~59 lines"]
npackages_browserstack_service_README_md["README.md<br/>~34 lines"]
npackages_browserstack_service_package_json["package.json<br/>~11 lines"]
n_changeset_pr_210_md["pr-210.md<br/>~8 lines"]
end
↻ This verdict comment is the review anchor — it's updated in place on each run (the gate posts its status separately). — SDK PR Review Agent |
|
🟢 SDK PR Review gate is green — the SDK PR Review Agent has run on the current head commit (verdict: This gate confirms a review ran on the latest commit. The verdict itself is advisory — read the findings and use your judgement; it does not block merge. A native GitHub reviewer approval is still separately required by branch protection before this PR can merge. |
AakashHotchandani
left a comment
AakashHotchandani
left a comment
There was a problem hiding this comment.
SDK PR Review — findings at 0939a858
Verdict: 🔴 Fix 2 blocking issues
0 critical · 2 warning(s) · 4 suggestion(s) · 0 needing human review
1 finding(s) inline; 5 already posted on this PR, not repeated. The signed verdict comment carries the merge-gate state.
Prior findings
UNRESOLVED 5
— SDK PR Review Agent
| "@grpc/grpc-js": "~1.13.5", | ||
| "@percy/appium-app": "^2.0.9", | ||
| "@percy/selenium-webdriver": "^2.2.2", | ||
| "@percy/webdriverio": ">=3.3.0 <3.3.3", |
There was a problem hiding this comment.
💡 Suggestion — [CONFIG] New @percy/webdriverio dependency declares a peer range that excludes webdriverio 9.1+ · missed in prior round
🟡 Needs human review — this finding is not grounded in code or review knowledge; a reviewer's confirmation (or a reason it is safe) is what moves it.
Problem
This PR makes @percy/webdriverio an unconditional runtime dependency of the service for every customer, Percy users or not. Its published package.json (percy-webdriverio@v3.3.2, L31-33) declares "peerDependencies": { "webdriverio": "~6 || ~7 || ~8 || ~ 9" }; ~ 9 is >=9.0.0 <9.1.0. The service itself requires webdriverio ^9.0.0, and customers run current 9.x (Percy's own devDependency is ^9.2.1), so the peer is unsatisfied on essentially every install. The lock entry added here shows the same range.
Suggested Fix
Confirm what npm install @wdio/browserstack-service does in a project on webdriverio 9.2x (warning only vs. nested copy). If it only warns, note it in the PR/README; if it nests a second webdriverio, consider making @percy/webdriverio an optional peer of the service (the README already tells Percy web users to install it themselves) and ask Percy to widen the peer range to ^9.
This fix is unverified against npm/pnpm/yarn peer resolution on a customer project; confirm it before applying.
Impact / likelihood: low — Expected customer-visible effect is an install-time peer-dependency warning naming a package they did not add; capture itself is not affected. If a package manager instead nests a webdriverio 9.0.x copy to satisfy the peer, the impact would be higher; that is what a human should confirm. · high — Every install of the service on webdriverio 9.1 or later, i.e. essentially every current v9 customer, since the dependency is unconditional.
Confidence: 🟡 — The peer range is read from upstream; the effect on a customer's install (npm ERESOLVE override warning vs. a nested webdriverio 9.0.x copy) was not run here.
— SDK PR Review Agent
4 participants
What is this about?
Percy produced no visual coverage at all on WebdriverIO. Two independent defects were stacked on top of each other, and both were silent — tests kept passing, the only symptom was a
Unsupported driver for percylog line.Defect A —
screenshot(Percy on Automate). Percy's bug; Percy has fixed it.@percy/selenium-webdriver2.2.6 (published 2026-04-01) added a top-levelrequire('selenium-webdriver')for a Selenium-only iframe helper, but ships that package as a devDependency only, so it is never present in a WebdriverIO project and the module throwsMODULE_NOT_FOUNDon load. OurtryRequireswallowed the throw and left every Percy entry point as a no-op stub. Our floating^2.2.2is what admitted it. Percy guarded the require in 2.2.8, so the range stays^2.2.2and the lockfile simply moves2.2.6 → 2.2.8. Not v9-specific: the v8 line's^2.0.3floats into the same window.Defect B —
snapshot(Percy web). Ours, older, and it never worked.snapshotwas bound topercySnapshotfrom@percy/selenium-webdriver, which drives the browser through Selenium client APIs — single-argumentexecuteScript(script),By,switchTo(). A WebdriverIO browser provides none of them, so it failed on the first driver call, the error was swallowed, and zero snapshots were produced. The WebdriverIO-native port is a different package,@percy/webdriverio, andsnapshotnow binds to it.screenshotdeliberately stays on@percy/selenium-webdriver: Percy on Automate is driver-agnostic (it reads session metadata and captures server-side) and already carries an explicit wdio branch in itsDriverMetadata.Changes:
@percy/webdriverioand routesnapshotto it;screenshotandscreenshotAppare unchanged in behaviour.runPercyhelper that logs errors instead of propagating them into the user's test. This is load-bearing, not defensive tidying: now that 2.2.8 loads cleanly, Percy's misuse guards (percySnapshotagainst an Automate build,percyScreenshotagainst anything else) throw ahead of their own try/catch, and all three entry points are public exports — without the wrapper this PR would start failing user tests where they previously only logged.PERCY_RAISE_ERROR=trueopts back into throwing.tryRequirenow logs the underlying load failure instead of discarding it, so a future packaging break surfaces as itself rather than as a misleading driver message.percyandpercyCaptureModeoptions, that this service runs Percy on Automate, and how to drive a Percy web project alongside it.Notes for the reviewer:
@percy/webdriveriois pinned>=3.3.0 <3.3.3deliberately, and the upper bound is load-bearing.3.3.3is unloadable as published:index.js:1requires./_iframe_shim, a file the published tarball does not contain (npm packyields onlyLICENSE,README.md,index.js,package.json,types/index.d.ts). Through 3.3.2 and 3.3.3-beta.0 that line isrequire('@percy/sdk-utils'), an external package; the relative require appears from 3.3.3-beta.1 onward (beta.1, beta.2 and the stable cut are all broken), against afileslist unchanged at["index.js","types/index.d.ts"]throughout. It is stilllateston npm, so a floating^3.3.0lands on it. Worse, a failed load here does not disablesnapshot—webSnapshot = percyWebdriverioSnapshot || percySnapshotfalls through to the Selenium SDK, silently restoring the exact defect this PR fixes. Verified by pinning3.3.3and rebuilding. Do not relax the upper bound until Percy republishes; raised with them separately.@percy/selenium-webdriver@2.2.8pins@percy/sdk-utils@1.32.3-beta.1— a beta, and it is Percy's own pin inside their published package, not something set here. It appears in the lockfile as a nested dependency. Flagging it because a beta transitive arriving via a patch-range float is worth a conscious decision.package-lock.jsondiff is entirely Percy-scoped except one line: npm corrects the lock's stale9.29.1entry for this workspace to the actual9.36.2. Anynpm installonmaindoes this; it is not introduced here.origin/v8still has the barecatch {}and no wrapper, and the samesnapshotmisbinding.snapshotmisbinding exists inbrowserstack-node-sdk(src/helpers/percy/PercySDK.js), affecting its Playwright/Puppeteer/Jest/Cucumber users. Needs its own ticket.Related Jira task/s
https://browserstack.atlassian.net/browse/SDK-7711
Release (mandatory for every PR — required for the
ready-for-reviewlabel)Version bump: (required — tick exactly one)
Release notes type: (optional)
Release notes (customer-facing): (optional but encouraged)
percy: truelogged "Unsupported driver for percy" and produced no screenshots, while the tests themselves continued to pass.PERCY_RAISE_ERROR=trueto fail the build on Percy errors instead.Release notes (internal): (required — engineer-facing; what actually changed / why)
snapshotwas bound topercySnapshotfrom@percy/selenium-webdriver, which drives the browser via Selenium client APIs (1-argexecuteScript,By,switchTo()) that a WebdriverIO browser does not have. It failed on the first driver call and the error was swallowed — 0 snapshots, at every version. Rebound to@percy/webdriverio, pinned>=3.3.0 <3.3.3because the stable3.3.3cut added arequire('./_iframe_shim')atindex.js:1without adding that file tofiles, so it is unloadable as published (the betas and 3.3.2 require the external@percy/sdk-utilsinstead).@percy/selenium-webdriver2.2.6/2.2.7 added an unguarded top-levelrequire('selenium-webdriver')atindex.js:14while shipping that package as a devDependency only, so it threwMODULE_NOT_FOUNDin any WebdriverIO-only install and stubbed out every Percy export. Percy guarded it in 2.2.8; range stays^2.2.2, lock moves to 2.2.8. The v8 line's^2.0.3floats into the same window and still needs the fix.npm install— npm keeps a locked resolution while it satisfies the declared range. Verified by controlled test (pin 2.2.7, widen range, reinstall → stays 2.2.7). Recovery needs an explicitnpm update @percy/selenium-webdriver.runPercyhelper wrappingsnapshot,screenshotandscreenshotApp. Percy's misuse guards throw ahead of their own try/catch, so with the module loading again those rejections would reach the caller — all three entry points are public exports. HonoursPERCY_RAISE_ERROR.tryRequirenow logs the swallowed error. CollapsingMODULE_NOT_FOUNDinto "Unsupported driver for percy" is why Defect A went undiagnosed for ~5 months.screenshotAppwas never affected by either defect (separate@percy/appium-appdependency).Checklist
Defect A — verified on Jenkins (BStackAutomation
SDK-7711-percy-wdio-verify, selection-m sdk_7711). Run #275 againstmain, run #276 against this branch. Both desktop-Automate Percy cases go FAIL → PASS. The App Automate case was tagged as a control and failed in both runs — it is invariant across the change, so it is unaffected by this PR and is a separate pre-existing defect (raised separately).Defect B — verified locally against a local Percy CLI stub, not percy.io. This session has no Percy web project token, so the snapshot path was driven against a local Percy CLI listening on
:5338and reportingtype: "web". The DOM was captured from a real BrowserStack browser session, not a simulated one: sessionb1b9225d74fc7ae292c19665947cb92fad96f68fshows the Selenium SDK posting 0 snapshots (swallowingWrong parameters applied for executeScript) against@percy/webdriverioposting 1 snapshot / 594-byte DOM; session37443af39f547cd3ce0b35d97693593510b0c5b0reproduces that through this service's ownPercySDK.snapshotexport (snapshotsPosted: 1, domBytes: 594). What is not covered: no snapshot has been rendered on a real percy.io web project. That needs a Percy web project token and a new BStackAutomation case — the existing-m sdk_7711cases all runpercy: true, which provisions an automate project wheresnapshotis misuse by design.Build and eslint are clean. The full suite matches the pre-change baseline exactly (same 8 files / 75 failures, all pre-existing and network-dependent in this environment).
PR Validations
Run Tests: Comment RUN_TESTS to trigger sanity tests.