Skip to content

Commit

Permalink
✨ Capture different pixel densities in asset discovery (#970)
Browse files Browse the repository at this point in the history
* ✨ 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 <wil@wilwilsman.com>
  • Loading branch information
Robdel12 and wwilsman authored Jun 27, 2022
1 parent 341362f commit b73cb10
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 33 deletions.
4 changes: 4 additions & 0 deletions packages/core/src/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ export const configSchema = {
},
scope: {
type: 'string'
},
devicePixelRatio: {
type: 'integer'
}
}
},
Expand Down Expand Up @@ -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,
Expand Down
8 changes: 4 additions & 4 deletions packages/core/src/page.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
});
Expand Down
70 changes: 43 additions & 27 deletions packages/core/src/snapshot.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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
Expand All @@ -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 && (
Expand Down Expand Up @@ -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) {
Expand All @@ -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;
}
}
43 changes: 41 additions & 2 deletions packages/core/test/discovery.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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'
]));
Expand Down Expand Up @@ -629,6 +629,45 @@ describe('Discovery', () => {
]));
});

it('captures responsive assets at higher pixel densities', async () => {
let responsiveDOM = dedent`
<html>
<head></head>
<body>
<p>Hello Percy!<p>
<img srcset="/img-normal.png, /img-2x.png 2x"
src="img-normal.png">
</body>
</html>
`;

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
Expand Down

0 comments on commit b73cb10

Please sign in to comment.