Skip to content

fix(webdriver-utils): propagate labels onto comparisonData for POA snapshots - #2239

Merged
prklm10 merged 7 commits into
masterfrom
fix/poa-labels-propagation
May 28, 2026
Merged

fix(webdriver-utils): propagate labels onto comparisonData for POA snapshots#2239
prklm10 merged 7 commits into
masterfrom
fix/poa-labels-propagation

Conversation

@prklm10

@prklm10 prklm10 commented May 25, 2026

Copy link
Copy Markdown
Contributor

Summary

  • POA (Percy on Automate) snapshots silently dropped per-screenshot labelsWebdriverUtils.captureScreenshot forwarded sync, testCase, and thTestCaseExecutionId from options onto the returned comparisonData but not labels, so labels never reached percy.uploadsendComparisoncreateSnapshot (which converts labels to tags on the snapshot POST). The result: POA tags were always empty regardless of SDK input.
  • One-line fix at packages/webdriver-utils/src/index.js:42.
  • Tests added at three layers to lock the contract end-to-end.

Root cause

packages/webdriver-utils/src/index.js

comparisonData.sync = options.sync;
comparisonData.testCase = options.testCase;
comparisonData.labels = options.labels;            // <- added
comparisonData.thTestCaseExecutionId = options.thTestCaseExecutionId;

Tests

  • packages/webdriver-utils/test/index.test.js (new) — 5 specs for WebdriverUtils.captureScreenshot:
    • forwards labels onto comparisonData
    • undefined when not provided
    • does not lose labels when provider response has none
    • works for playwright provider
    • re-throws provider errors
  • packages/client/test/client.test.jssendComparison spec asserts labels: 'qa, smoke,release' becomes tags: [{id: null, name: 'qa'}, ...] on the snapshot POST body.
  • packages/core/test/api.test.js/percy/automateScreenshot spec asserts labels survive percyAutomateRequestHandler and reach percy.upload.

Test plan

  • cd packages/webdriver-utils && yarn test — 231/231 pass (incl. 5 new specs)
  • cd packages/client && yarn test — new sendComparison labels spec passes (one pre-existing unrelated PAC-proxy test fails on Node URL message format)
  • cd packages/core && yarn test — full suite has pre-existing Chromium install failures unrelated to this change; new API spec was added following the existing /automateScreenshot test patterns
  • Manual verification: run any POA SDK with labels set and confirm tags are populated on the resulting snapshot in the Percy dashboard

🤖 Generated with Claude Code

…apshots

POA snapshots dropped any `labels` set on per-screenshot options because
WebdriverUtils.captureScreenshot only forwarded `sync`, `testCase`, and
`thTestCaseExecutionId` from options onto the returned comparisonData.
Labels never reached percy.upload → sendComparison → createSnapshot
(which converts labels to `tags` for the API), so POA tags were always
empty regardless of SDK input.

Adds the missing forward and tests at three layers:
- webdriver-utils: new unit specs for captureScreenshot covering label
  propagation, missing labels, playwright provider, and error rethrow.
- client: sendComparison spec asserting labels become `tags` on the
  snapshot POST body.
- core: /percy/automateScreenshot spec asserting labels survive the API
  handler and reach percy.upload.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@prklm10
prklm10 requested a review from a team as a code owner May 25, 2026 00:46
prklm10 and others added 4 commits May 25, 2026 06:27
…n test

Drop the full-stack /automateScreenshot labels propagation spec. The
labels behavior is already covered by the focused unit specs in
webdriver-utils (captureScreenshot) and client (sendComparison).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ptureScreenshot

Branch coverage for captureScreenshot dropped to 66.67% (lines 17-18)
because every existing spec passes both `options` and `buildInfo`,
leaving the default-parameter branches untaken. Adds a spec that
calls captureScreenshot with both omitted to exercise the defaults.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
prklm10 and others added 2 commits May 26, 2026 12:44
…agation

# Conflicts:
#	packages/core/test/api.test.js
…ntracked-request spec

The event emitter fires `_handleResponsePaused` without awaiting it, so
`await snap` can resolve before the handler reaches Fetch.continueResponse
and the spy records nothing. The race was masked on master and exposed on
this PR by the extra api.test.js spec shifting timing. Poll the spy with
`waitFor` so the assertion is deterministic.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@prklm10
prklm10 merged commit fbabcb6 into master May 28, 2026
44 of 45 checks passed
@prklm10
prklm10 deleted the fix/poa-labels-propagation branch May 28, 2026 10:02
pankaj443 added a commit that referenced this pull request Jun 5, 2026
* test: re-add comprehensive.html regression fixture

Restore the comprehensive regression test page that was removed in
PR #2239. The fixture exercises fonts, pseudo-class CSS rules, and
other web features used by the regression suite.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test: comment out video element to verify issue

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test: drop unplayable mp4 source from video poster fixture

The <video> carried a header-only base64 mp4 <source> (a 32-byte ftyp
box with no moov/mdat). It's unplayable and serves no purpose for this
fixture, whose goal is exercising video *poster* handling — serialize-
video.js skips elements that already have a poster attribute, so the
source was never part of any capture path.

The stub engages the browser media pipeline, which the upgraded (v143-
era) Firefox renderer handles differently than v126, causing the
Comprehensive snapshot to fail rendering on Firefox. Removing the
source leaves networkState=EMPTY (no load attempt) in both Chromium and
Firefox while keeping the poster — the actual thing under test.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test: use a valid mp4 source in video poster fixture

Replace the header-only/truncated base64 mp4 (a 32-byte ftyp box with no
moov/mdat) with a complete, spec-valid minimal mp4 (ftyp+free+mdat+moov).

The original stub engaged the media pipeline in a way the upgraded
Firefox 146 renderer (percy-renderer #1775, FF 139.0.4 -> 146.0.1) failed
to render, causing the Comprehensive snapshot to time out on Firefox. The
discovery/serialization path is identical across CLI versions (verified:
old v126 and new v143 CLI both timed out), so the fixture must carry media
the renderer can actually parse.

Using a valid source keeps the data-URI video serialization path under
test (serialize-video / serializeBase64) while no longer hanging the
renderer. Locally the video now reports networkState=IDLE with no decode
error in both Chromium and Firefox (the stub reported NO_SOURCE).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants