From 7dccffc2c9d69dedf36fa26228f2571cf5f705aa Mon Sep 17 00:00:00 2001 From: prklm10 Date: Mon, 25 May 2026 06:15:23 +0530 Subject: [PATCH 1/6] fix(webdriver-utils): propagate labels onto comparisonData for POA snapshots MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- packages/client/test/client.test.js | 19 +++++ packages/core/test/api.test.js | 55 ++++++++++++ packages/webdriver-utils/src/index.js | 1 + packages/webdriver-utils/test/index.test.js | 93 +++++++++++++++++++++ 4 files changed, 168 insertions(+) create mode 100644 packages/webdriver-utils/test/index.test.js diff --git a/packages/client/test/client.test.js b/packages/client/test/client.test.js index 6227423ae..7d7259d98 100644 --- a/packages/client/test/client.test.js +++ b/packages/client/test/client.test.js @@ -2293,6 +2293,25 @@ describe('PercyClient', () => { })).toBeRejectedWithError('sha, filepath or content should be present in tiles object'); }); }); + + describe('when labels are provided on a POA comparison', () => { + beforeEach(async () => { + await client.sendComparison(123, { + name: 'test snapshot name', + tag: { name: 'test tag' }, + tiles: [{ content: base64encode('tile') }], + labels: 'qa, smoke,release' + }); + }); + + it('forwards labels as tags onto the snapshot', () => { + expect(api.requests['/builds/123/snapshots'][0].body.data.attributes.tags).toEqual([ + { id: null, name: 'qa' }, + { id: null, name: 'smoke' }, + { id: null, name: 'release' } + ]); + }); + }); }); describe('#tokenType', () => { diff --git a/packages/core/test/api.test.js b/packages/core/test/api.test.js index a1db6bbe7..a210fda14 100644 --- a/packages/core/test/api.test.js +++ b/packages/core/test/api.test.js @@ -467,6 +467,61 @@ describe('API Server', () => { expect(percy.upload).toHaveBeenCalledOnceWith({ sync: true }, jasmine.objectContaining({}), 'automate'); }); + it('has a /automateScreenshot endpoint that propagates labels through to #upload()', async () => { + let resolve, test = new Promise(r => (resolve = r)); + spyOn(percy, 'upload').and.returnValue(test); + // Simulate what the real WebdriverUtils.captureScreenshot does: + // it must copy options.labels onto the returned comparisonData. + let captureScreenshotSpy = spyOn(WebdriverUtils, 'captureScreenshot').and.callFake(({ options }) => { + return Promise.resolve({ + name: 'Snapshot name', + tag: { name: 'tag-1' }, + tiles: [], + metadata: {}, + sync: options.sync, + testCase: options.testCase, + labels: options.labels, + thTestCaseExecutionId: options.thTestCaseExecutionId + }); + }); + + await percy.start(); + + await expectAsync(request('/percy/automateScreenshot', { + body: { + name: 'Snapshot name', + client_info: 'client', + environment_info: 'environment', + options: { + labels: 'qa,smoke', + testCase: 'tc-1', + thTestCaseExecutionId: 'exec-99' + } + }, + method: 'post' + })).toBeResolvedTo({ success: true }); + + expect(captureScreenshotSpy).toHaveBeenCalledOnceWith(jasmine.objectContaining({ + options: jasmine.objectContaining({ + labels: 'qa,smoke', + testCase: 'tc-1', + thTestCaseExecutionId: 'exec-99' + }) + })); + + expect(percy.upload).toHaveBeenCalledOnceWith( + jasmine.objectContaining({ + labels: 'qa,smoke', + testCase: 'tc-1', + thTestCaseExecutionId: 'exec-99' + }), + null, + 'automate' + ); + + resolve(); + }); + it('has a /events endpoint that calls #sendBuildEvents() async with provided options with clientInfo present', async () => { let { getPackageJSON } = await import('@percy/client/utils'); let pkg = getPackageJSON(import.meta.url); diff --git a/packages/webdriver-utils/src/index.js b/packages/webdriver-utils/src/index.js index 40655fa8e..5b0b9a8b4 100644 --- a/packages/webdriver-utils/src/index.js +++ b/packages/webdriver-utils/src/index.js @@ -39,6 +39,7 @@ export default class WebdriverUtils { comparisonData.metadata.cliScreenshotEndTime = Date.now(); comparisonData.sync = options.sync; comparisonData.testCase = options.testCase; + comparisonData.labels = options.labels; comparisonData.thTestCaseExecutionId = options.thTestCaseExecutionId; log.debug(`[${snapshotName}] : Comparison Data: ${JSON.stringify(comparisonData)}`); return comparisonData; diff --git a/packages/webdriver-utils/test/index.test.js b/packages/webdriver-utils/test/index.test.js new file mode 100644 index 000000000..8ffd99eb5 --- /dev/null +++ b/packages/webdriver-utils/test/index.test.js @@ -0,0 +1,93 @@ +import WebdriverUtils from '../src/index.js'; +import ProviderResolver from '../src/providers/providerResolver.js'; +import PlaywrightProvider from '../src/providers/playwrightProvider.js'; + +describe('WebdriverUtils.captureScreenshot', () => { + let providerStub; + let baseArgs; + let providerResponse; + + beforeEach(() => { + providerResponse = { + name: 'snap', + tag: { name: 'tag-1' }, + tiles: [], + metadata: {} + }; + + providerStub = { + createDriver: jasmine.createSpy('createDriver').and.resolveTo(), + screenshot: jasmine.createSpy('screenshot').and.callFake(() => Promise.resolve({ ...providerResponse })) + }; + + spyOn(ProviderResolver, 'resolve').and.returnValue(providerStub); + + baseArgs = { + sessionId: '1234', + commandExecutorUrl: 'https://localhost/command-executor', + capabilities: {}, + sessionCapabilities: {}, + framework: null, + snapshotName: 'snap', + clientInfo: 'client', + environmentInfo: 'env', + options: {}, + buildInfo: { id: '123' } + }; + }); + + it('forwards labels from options onto comparisonData for POA snapshots', async () => { + baseArgs.options = { + sync: false, + testCase: 'tc', + labels: 'label1,label2', + thTestCaseExecutionId: 'exec-1' + }; + + const result = await WebdriverUtils.captureScreenshot(baseArgs); + + expect(result.labels).toEqual('label1,label2'); + expect(result.testCase).toEqual('tc'); + expect(result.thTestCaseExecutionId).toEqual('exec-1'); + expect(result.sync).toEqual(false); + }); + + it('sets labels to undefined when not provided in options', async () => { + baseArgs.options = { testCase: 'tc' }; + + const result = await WebdriverUtils.captureScreenshot(baseArgs); + + expect(result.labels).toBeUndefined(); + expect(result.testCase).toEqual('tc'); + }); + + it('does not lose labels even when provider response has none', async () => { + baseArgs.options = { labels: 'only-label' }; + + const result = await WebdriverUtils.captureScreenshot(baseArgs); + + expect(result.labels).toEqual('only-label'); + }); + + it('forwards labels through the playwright provider too', async () => { + spyOn(PlaywrightProvider.prototype, 'createDriver').and.resolveTo(); + spyOn(PlaywrightProvider.prototype, 'screenshot').and.resolveTo({ + name: 'snap', tag: { name: 'pw' }, tiles: [], metadata: {} + }); + + baseArgs.framework = 'playwright'; + baseArgs.options = { labels: 'pw-label' }; + + const result = await WebdriverUtils.captureScreenshot(baseArgs); + + expect(result.labels).toEqual('pw-label'); + }); + + it('re-throws errors from the provider without setting labels', async () => { + providerStub.screenshot.and.rejectWith(new Error('boom')); + baseArgs.options = { labels: 'x' }; + + await expectAsync(WebdriverUtils.captureScreenshot(baseArgs)) + .toBeRejectedWithError('boom'); + }); +}); From 923a52ccba85df1b2efc4243185e617220ddb2ca Mon Sep 17 00:00:00 2001 From: prklm10 Date: Mon, 25 May 2026 06:27:21 +0530 Subject: [PATCH 2/6] test(core): remove comprehensive /automateScreenshot labels regression 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) --- packages/core/test/api.test.js | 55 ---------------------------------- 1 file changed, 55 deletions(-) diff --git a/packages/core/test/api.test.js b/packages/core/test/api.test.js index a210fda14..a1db6bbe7 100644 --- a/packages/core/test/api.test.js +++ b/packages/core/test/api.test.js @@ -467,61 +467,6 @@ describe('API Server', () => { expect(percy.upload).toHaveBeenCalledOnceWith({ sync: true }, jasmine.objectContaining({}), 'automate'); }); - it('has a /automateScreenshot endpoint that propagates labels through to #upload()', async () => { - let resolve, test = new Promise(r => (resolve = r)); - spyOn(percy, 'upload').and.returnValue(test); - // Simulate what the real WebdriverUtils.captureScreenshot does: - // it must copy options.labels onto the returned comparisonData. - let captureScreenshotSpy = spyOn(WebdriverUtils, 'captureScreenshot').and.callFake(({ options }) => { - return Promise.resolve({ - name: 'Snapshot name', - tag: { name: 'tag-1' }, - tiles: [], - metadata: {}, - sync: options.sync, - testCase: options.testCase, - labels: options.labels, - thTestCaseExecutionId: options.thTestCaseExecutionId - }); - }); - - await percy.start(); - - await expectAsync(request('/percy/automateScreenshot', { - body: { - name: 'Snapshot name', - client_info: 'client', - environment_info: 'environment', - options: { - labels: 'qa,smoke', - testCase: 'tc-1', - thTestCaseExecutionId: 'exec-99' - } - }, - method: 'post' - })).toBeResolvedTo({ success: true }); - - expect(captureScreenshotSpy).toHaveBeenCalledOnceWith(jasmine.objectContaining({ - options: jasmine.objectContaining({ - labels: 'qa,smoke', - testCase: 'tc-1', - thTestCaseExecutionId: 'exec-99' - }) - })); - - expect(percy.upload).toHaveBeenCalledOnceWith( - jasmine.objectContaining({ - labels: 'qa,smoke', - testCase: 'tc-1', - thTestCaseExecutionId: 'exec-99' - }), - null, - 'automate' - ); - - resolve(); - }); - it('has a /events endpoint that calls #sendBuildEvents() async with provided options with clientInfo present', async () => { let { getPackageJSON } = await import('@percy/client/utils'); let pkg = getPackageJSON(import.meta.url); From 2d4fa87dc258695e1e994ac84257b36269898f37 Mon Sep 17 00:00:00 2001 From: prklm10 Date: Mon, 25 May 2026 06:30:09 +0530 Subject: [PATCH 3/6] Revert "test(core): remove comprehensive /automateScreenshot labels regression test" This reverts commit 923a52ccba85df1b2efc4243185e617220ddb2ca. --- packages/core/test/api.test.js | 55 ++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/packages/core/test/api.test.js b/packages/core/test/api.test.js index a1db6bbe7..a210fda14 100644 --- a/packages/core/test/api.test.js +++ b/packages/core/test/api.test.js @@ -467,6 +467,61 @@ describe('API Server', () => { expect(percy.upload).toHaveBeenCalledOnceWith({ sync: true }, jasmine.objectContaining({}), 'automate'); }); + it('has a /automateScreenshot endpoint that propagates labels through to #upload()', async () => { + let resolve, test = new Promise(r => (resolve = r)); + spyOn(percy, 'upload').and.returnValue(test); + // Simulate what the real WebdriverUtils.captureScreenshot does: + // it must copy options.labels onto the returned comparisonData. + let captureScreenshotSpy = spyOn(WebdriverUtils, 'captureScreenshot').and.callFake(({ options }) => { + return Promise.resolve({ + name: 'Snapshot name', + tag: { name: 'tag-1' }, + tiles: [], + metadata: {}, + sync: options.sync, + testCase: options.testCase, + labels: options.labels, + thTestCaseExecutionId: options.thTestCaseExecutionId + }); + }); + + await percy.start(); + + await expectAsync(request('/percy/automateScreenshot', { + body: { + name: 'Snapshot name', + client_info: 'client', + environment_info: 'environment', + options: { + labels: 'qa,smoke', + testCase: 'tc-1', + thTestCaseExecutionId: 'exec-99' + } + }, + method: 'post' + })).toBeResolvedTo({ success: true }); + + expect(captureScreenshotSpy).toHaveBeenCalledOnceWith(jasmine.objectContaining({ + options: jasmine.objectContaining({ + labels: 'qa,smoke', + testCase: 'tc-1', + thTestCaseExecutionId: 'exec-99' + }) + })); + + expect(percy.upload).toHaveBeenCalledOnceWith( + jasmine.objectContaining({ + labels: 'qa,smoke', + testCase: 'tc-1', + thTestCaseExecutionId: 'exec-99' + }), + null, + 'automate' + ); + + resolve(); + }); + it('has a /events endpoint that calls #sendBuildEvents() async with provided options with clientInfo present', async () => { let { getPackageJSON } = await import('@percy/client/utils'); let pkg = getPackageJSON(import.meta.url); From 2f44f8350fed35c10ed8ccd4084947f268287bab Mon Sep 17 00:00:00 2001 From: prklm10 Date: Mon, 25 May 2026 06:31:16 +0530 Subject: [PATCH 4/6] test: remove unused comprehensive.html regression fixture Co-Authored-By: Claude Opus 4.7 (1M context) --- test/regression/pages/comprehensive.html | 96 ------------------------ 1 file changed, 96 deletions(-) delete mode 100644 test/regression/pages/comprehensive.html diff --git a/test/regression/pages/comprehensive.html b/test/regression/pages/comprehensive.html deleted file mode 100644 index 66f495e7f..000000000 --- a/test/regression/pages/comprehensive.html +++ /dev/null @@ -1,96 +0,0 @@ - - - - - - Comprehensive Regression Test - - - - -

Comprehensive Smoke Test

-

This page combines all major asset discovery features into a single snapshot.

- - -
-
Basic HTML + CSS
-

Static text content with bold and italic formatting.

-
- - -
-
Image
- Test logo -
- - -
-
Web Font
-

This text uses a custom web font loaded via @font-face.

-
- - -
-
Shadow DOM Component
- -

This content is slotted into a shadow DOM component.

- -
- - -
-
Canvas
- -
- - -
-
Video Poster
- -
- - -
-
Base64 Inline Image
- Inline base64 -
- - -
-
Pseudo-class CSS
- - -
- - -
-
SVG Image
- SVG icon -
- - - - - From 0bfd7189f1f0daf7557bd24b90d0595cae102723 Mon Sep 17 00:00:00 2001 From: prklm10 Date: Mon, 25 May 2026 18:50:59 +0530 Subject: [PATCH 5/6] test(webdriver-utils): cover default options/buildInfo branches in captureScreenshot 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) --- packages/webdriver-utils/test/index.test.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/packages/webdriver-utils/test/index.test.js b/packages/webdriver-utils/test/index.test.js index 8ffd99eb5..2ec0da2e0 100644 --- a/packages/webdriver-utils/test/index.test.js +++ b/packages/webdriver-utils/test/index.test.js @@ -90,4 +90,15 @@ describe('WebdriverUtils.captureScreenshot', () => { await expectAsync(WebdriverUtils.captureScreenshot(baseArgs)) .toBeRejectedWithError('boom'); }); + + it('falls back to default options and buildInfo when caller omits them', async () => { + const { options, buildInfo, ...argsWithoutDefaults } = baseArgs; + + const result = await WebdriverUtils.captureScreenshot(argsWithoutDefaults); + + expect(result.labels).toBeUndefined(); + expect(result.testCase).toBeUndefined(); + expect(result.thTestCaseExecutionId).toBeUndefined(); + expect(providerStub.screenshot).toHaveBeenCalledWith('snap', {}); + }); }); From 35b781dc4b511b1a0f7e64c7642f106ccafec4fa Mon Sep 17 00:00:00 2001 From: prklm10 Date: Thu, 28 May 2026 14:32:49 +0530 Subject: [PATCH 6/6] test(core): wait for Fetch.continueResponse spy before asserting in untracked-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) --- packages/core/test/discovery.test.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js index 77fa91fcb..331d5088a 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -3084,6 +3084,14 @@ describe('Discovery', () => { request: { url: 'http://example.com/orphan' } }); + // The event emitter fires `_handleResponsePaused` without awaiting it, + // so `await snap` can resolve before the handler reaches + // Fetch.continueResponse. Poll the spy directly for the assertion. + await waitFor(() => sentMethods.some(c => + c.method === 'Fetch.continueResponse' && + c.params?.requestId === 'untracked-intercept-id' + ), 2000); + await snap; expect(sentMethods.some(c =>