From b73cb10a7543e1711c30d9fd03ef43791770f9f4 Mon Sep 17 00:00:00 2001 From: Robert DeLuca Date: Mon, 27 Jun 2022 18:00:12 -0500 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20Capture=20different=20pixel=20densi?= =?UTF-8?q?ties=20in=20asset=20discovery=20(#970)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * ✨ Capture different pixel densities in asset discovery With the addition of real native devices in our rendering infrastructure, we now need to capture 2x++ pixel images for pages to render properly (since the real devices have high pixel density screens). With the new `deviceScaleFactor` config option, we now will resize the browser to each specified width at 1x and also whatever the passed value is for `deviceScaleFactor`. We also run both `beforeResize` and `afterResize` events with the additional scale resizes. * 🚧 Implement `before/afterMobile` & move pixel ratio to their own tab * ♻ Trigger resource requests for each pixel ratio Also renamed the new deviceScaleFactor option to devicePixelRatio to better reflect web standards rather than the API option name. * 💡 Add description comment to trigger resource request function Co-authored-by: Wil Wilsman --- packages/core/src/config.js | 4 ++ packages/core/src/page.js | 8 ++-- packages/core/src/snapshot.js | 70 +++++++++++++++++----------- packages/core/test/discovery.test.js | 43 ++++++++++++++++- 4 files changed, 92 insertions(+), 33 deletions(-) diff --git a/packages/core/src/config.js b/packages/core/src/config.js index 3bcd4fe58..9913860fc 100644 --- a/packages/core/src/config.js +++ b/packages/core/src/config.js @@ -28,6 +28,9 @@ export const configSchema = { }, scope: { type: 'string' + }, + devicePixelRatio: { + type: 'integer' } } }, @@ -137,6 +140,7 @@ export const snapshotSchema = { minHeight: { $ref: '/config/snapshot#/properties/minHeight' }, percyCSS: { $ref: '/config/snapshot#/properties/percyCSS' }, enableJavaScript: { $ref: '/config/snapshot#/properties/enableJavaScript' }, + devicePixelRatio: { $ref: '/config/snapshot#/properties/devicePixelRatio' }, discovery: { type: 'object', additionalProperties: false, diff --git a/packages/core/src/page.js b/packages/core/src/page.js index 851358a52..abbeac2db 100644 --- a/packages/core/src/page.js +++ b/packages/core/src/page.js @@ -36,12 +36,12 @@ export class Page { } // Resize the page to the specified width and height - async resize({ width, height }) { - this.log.debug(`Resize page to ${width}x${height}`); + async resize({ width, height, deviceScaleFactor = 1, mobile = false }) { + this.log.debug(`Resize page to ${width}x${height} @${deviceScaleFactor}x`); await this.session.send('Emulation.setDeviceMetricsOverride', { - deviceScaleFactor: 1, - mobile: false, + deviceScaleFactor, + mobile, height, width }); diff --git a/packages/core/src/snapshot.js b/packages/core/src/snapshot.js index 068086503..8f87001fe 100644 --- a/packages/core/src/snapshot.js +++ b/packages/core/src/snapshot.js @@ -258,6 +258,7 @@ function debugSnapshotConfig(snapshot, showInfo) { debugProp(snapshot, 'widths', v => `${v}px`); debugProp(snapshot, 'minHeight', v => `${v}px`); debugProp(snapshot, 'enableJavaScript'); + debugProp(snapshot, 'deviceScaleFactor'); debugProp(snapshot, 'waitForTimeout'); debugProp(snapshot, 'waitForSelector'); debugProp(snapshot, 'execute.afterNavigation'); @@ -323,6 +324,38 @@ function waitForDiscoveryNetworkIdle(page, options) { // Used to cache resources across core instances const RESOURCE_CACHE_KEY = Symbol('resource-cache'); +// Trigger resource requests for a page by iterating over snapshot widths and calling any provided +// execute options. Additional resize options may be provided to capture resources mobile resources +function* triggerResourceRequests(page, snapshot, options) { + // copy widths to prevent mutation later + let [initialWidth, ...widths] = snapshot.widths; + + // set the initial page size + yield page.resize({ + width: initialWidth, + height: snapshot.minHeight, + ...options + }); + + // navigate to the url + yield page.goto(snapshot.url); + + if (snapshot.execute) { + // when any execute options are provided, inject snapshot options + /* istanbul ignore next: cannot detect coverage of injected code */ + yield page.eval((_, s) => (window.__PERCY__.snapshot = s), snapshot); + yield page.evaluate(snapshot.execute.afterNavigation); + } + + // trigger resize events for other widths + for (let width of widths) { + yield page.evaluate(snapshot.execute?.beforeResize); + yield waitForDiscoveryNetworkIdle(page, snapshot.discovery); + yield page.resize({ width, height: snapshot.minHeight, ...options }); + yield page.evaluate(snapshot.execute?.afterResize); + } +} + // Discovers resources for a snapshot using a browser page to intercept requests. The callback // function will be called with the snapshot name (for additional snapshots) and an array of // discovered resources. When additional snapshots are provided, the callback will be called once @@ -336,8 +369,6 @@ export async function* discoverSnapshotResources(percy, snapshot, callback) { // keep a global resource cache across snapshots let cache = percy[RESOURCE_CACHE_KEY] ||= new Map(); - // copy widths to prevent mutation later - let widths = snapshot.widths.slice(); // preload the root resource for existing dom snapshots let resources = new Map(snapshot.domSnapshot && ( @@ -366,28 +397,17 @@ export async function* discoverSnapshotResources(percy, snapshot, callback) { }); try { - // set the initial page size - yield page.resize({ - width: widths.shift(), - height: snapshot.minHeight - }); + yield* triggerResourceRequests(page, snapshot); - // navigate to the url - yield page.goto(snapshot.url); - - if (snapshot.execute) { - // when any execute options are provided, inject snapshot options - /* istanbul ignore next: cannot detect coverage of injected code */ - yield page.eval((_, s) => (window.__PERCY__.snapshot = s), snapshot); - yield page.evaluate(snapshot.execute.afterNavigation); - } - - // trigger resize events for other widths - for (let width of widths) { - yield page.evaluate(snapshot.execute?.beforeResize); + // trigger resource requests for any alternate device pixel ratio + if (snapshot.devicePixelRatio) { + // wait for any existing pending resource requests first yield waitForDiscoveryNetworkIdle(page, snapshot.discovery); - yield page.resize({ width, height: snapshot.minHeight }); - yield page.evaluate(snapshot.execute?.afterResize); + + yield* triggerResourceRequests(page, snapshot, { + deviceScaleFactor: snapshot.devicePixelRatio, + mobile: true + }); } if (snapshot.domSnapshot) { @@ -410,11 +430,7 @@ export async function* discoverSnapshotResources(percy, snapshot, callback) { resources.delete(root.url); } } - - // page clean up - await page.close(); - } catch (error) { + } finally { await page.close(); - throw error; } } diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js index 757a45c58..bbf20d62c 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -516,7 +516,7 @@ describe('Discovery', () => { expect(logger.stderr).toEqual(jasmine.arrayContaining([ '[percy:core:page] Page created', - '[percy:core:page] Resize page to 400x1024', + '[percy:core:page] Resize page to 400x1024 @1x', '[percy:core:page] Navigate to: http://localhost:8000/', '[percy:core:discovery] Handling request: http://localhost:8000/', '[percy:core:discovery] - Serving root resource', @@ -530,7 +530,7 @@ describe('Discovery', () => { '[percy:core:discovery] - mimetype: image/gif', '[percy:core:page] Page navigated', '[percy:core:network] Wait for 100ms idle', - '[percy:core:page] Resize page to 1200x1024', + '[percy:core:page] Resize page to 1200x1024 @1x', '[percy:core:network] Wait for 100ms idle', '[percy:core:page] Page closed' ])); @@ -629,6 +629,45 @@ describe('Discovery', () => { ])); }); + it('captures responsive assets at higher pixel densities', async () => { + let responsiveDOM = dedent` + + + +

Hello Percy!

+ + + + `; + + server.reply('/', () => [200, 'text/html', responsiveDOM]); + server.reply('/img-normal.png', () => [200, 'image/png', pixel]); + server.reply('/img-2x.png', () => new Promise(r => ( + setTimeout(r, 200, [200, 'image/png', pixel])))); + + await percy.snapshot({ + name: 'test responsive', + url: 'http://localhost:8000', + domSnapshot: responsiveDOM, + devicePixelRatio: 2, + widths: [400, 800] + }); + + await percy.idle(); + + let resource = path => jasmine.objectContaining({ + attributes: jasmine.objectContaining({ + 'resource-url': `http://localhost:8000${path}` + }) + }); + + expect(captured[0]).toEqual(jasmine.arrayContaining([ + resource('/img-normal.png'), + resource('/img-2x.png') + ])); + }); + it('captures requests from workers', async () => { // Fetch and Network events are inherently racey because they come from different processes. The // bug we are testing here happens specifically when the Network event comes after the Fetch