Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for capturing asset in multiple dpr and width combination #1536

Merged
merged 22 commits into from
Mar 6, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
eb9847d
Intial implementation
chinmay-browserstack Feb 27, 2024
a7e10e4
Capture for passed widths
chinmay-browserstack Feb 27, 2024
61113a9
Added support for image srcset capturing
chinmay-browserstack Feb 28, 2024
b62f174
Added tests + addressed comments
chinmay-browserstack Feb 28, 2024
9877cc3
Fix breaking tests
chinmay-browserstack Feb 28, 2024
91e1b17
Merge remote-tracking branch 'origin/capture_srcset_support' into mul…
chinmay-browserstack Feb 28, 2024
b90df23
Capture responsive dom
chinmay-browserstack Feb 28, 2024
dade996
Fix logic
chinmay-browserstack Feb 28, 2024
2c31be6
Remove new flag and use existing + enableJS
chinmay-browserstack Feb 29, 2024
11d8aff
Added endpoint for deviceDetails + deprecate deviceScaleFactor
chinmay-browserstack Feb 29, 2024
3bad053
Fix sdk-utils testcases
chinmay-browserstack Feb 29, 2024
5e46151
Fix breaking test
chinmay-browserstack Feb 29, 2024
45e1625
Add test for getDeviceDetails
chinmay-browserstack Feb 29, 2024
2eeef07
Added tests for captureResponsiveDom
chinmay-browserstack Mar 1, 2024
901d936
Fix edge case
chinmay-browserstack Mar 1, 2024
c9f1b43
Add warning remove tests
chinmay-browserstack Mar 1, 2024
7fe9bdb
Fix tests
chinmay-browserstack Mar 1, 2024
957bc8a
Fix tests
chinmay-browserstack Mar 1, 2024
fe4980f
addressed comments
chinmay-browserstack Mar 4, 2024
9897bad
Added new testcase
chinmay-browserstack Mar 4, 2024
d808e07
handle for snapshot command
chinmay-browserstack Mar 4, 2024
240b409
Merge remote-tracking branch 'origin/master' into multi_dpr_width_cap…
chinmay-browserstack Mar 4, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions packages/client/src/client.js
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,12 @@ export class PercyClient {
return this.get(`job_status?sync=true&type=${type}&id=${ids.join()}`);
}

// Returns device details enabled on project associated with given token
async getDeviceDetails() {
// ToDo: Add endpoint call
return [{ width: 390, deviceScaleFactor: 3, mobile: true }, { width: 375, deviceScaleFactor: 3, mobile: true }, { width: 384, deviceScaleFactor: 2.8125, mobile: true }, { width: 360, deviceScaleFactor: 3, mobile: true }];
}

// Retrieves project builds optionally filtered. Requires a read access token.
async getBuilds(project, filters = {}) {
validateProjectPath(project);
Expand Down
10 changes: 10 additions & 0 deletions packages/core/src/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,14 @@ export const configSchema = {
type: 'boolean',
default: false
},
captureSrcset: {
type: 'boolean',
default: false
},
captureResponsiveAssets: {
type: 'boolean',
default: false
},
requestHeaders: {
type: 'object',
normalize: false,
Expand Down Expand Up @@ -276,6 +284,8 @@ export const snapshotSchema = {
authorization: { $ref: '/config/discovery#/properties/authorization' },
disableCache: { $ref: '/config/discovery#/properties/disableCache' },
captureMockedServiceWorker: { $ref: '/config/discovery#/properties/captureMockedServiceWorker' },
captureSrcset: { $ref: '/config/discovery#/properties/captureSrcset' },
captureResponsiveAssets: { $ref: '/config/discovery#/properties/captureResponsiveAssets' },
userAgent: { $ref: '/config/discovery#/properties/userAgent' },
devicePixelRatio: { $ref: '/config/discovery#/properties/devicePixelRatio' }
}
Expand Down
40 changes: 37 additions & 3 deletions packages/core/src/discovery.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ function debugSnapshotOptions(snapshot) {
debugProp(snapshot, 'discovery.authorization', JSON.stringify);
debugProp(snapshot, 'discovery.disableCache');
debugProp(snapshot, 'discovery.captureMockedServiceWorker');
debugProp(snapshot, 'discovery.captureSrcset');
debugProp(snapshot, 'discovery.captureResponsiveAssets');
debugProp(snapshot, 'discovery.userAgent');
debugProp(snapshot, 'clientInfo');
debugProp(snapshot, 'environmentInfo');
Expand Down Expand Up @@ -137,11 +139,24 @@ function processSnapshotResources({ domSnapshot, resources, ...snapshot }) {
return { ...snapshot, resources };
}

async function responsiveDomParams(snapshot, client) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not something we want to call per snapshot, move it to the snapshot config validation and cache the response in percy.js so that it can be applied to all snapshots in current build

const { discovery, widths } = snapshot;
if (!discovery.captureResponsiveAssets) return false;

const devices = await client.getDeviceDetails();
if (widths.length > 1) {
for (let i = 1; i < widths.length; i++) {
devices.push({ width: widths[i], deviceScaleFactor: 1, mobile: false });
}
}
return devices;
}

// Triggers the capture of resource requests for a page by iterating over snapshot widths to resize
// the page and calling any provided execute options.
async function* captureSnapshotResources(page, snapshot, options) {
let { discovery, additionalSnapshots = [], ...baseSnapshot } = snapshot;
let { capture, captureWidths, deviceScaleFactor, mobile } = options;
let { capture, captureWidths, deviceScaleFactor, mobile, captureResponsiveDom } = options;

// used to take snapshots and remove any discovered root resource
let takeSnapshot = async (options, width) => {
Expand Down Expand Up @@ -171,6 +186,24 @@ async function* captureSnapshotResources(page, snapshot, options) {
yield page.evaluate(snapshot.execute.afterNavigation);
}

// Running before page idle since this will trigger many network calls
// so need to run as early as possible. plus it is just reading urls from dom srcset
// which will be already loaded after navigation complete
if (discovery.captureSrcset) {
await page.insertPercyDom();
yield page.eval('window.PercyDOM.loadAllSrcsetLinks()');
}

if (discovery.captureResponsiveAssets && captureResponsiveDom) {
for (const device of captureResponsiveDom) {
yield waitForDiscoveryNetworkIdle(page, discovery);
yield* captureSnapshotResources(page, { ...snapshot, widths: [device.width] }, {
deviceScaleFactor: device.deviceScaleFactor,
mobile: device.mobile
});
}
}

// iterate over additional snapshots for proper DOM capturing
for (let additionalSnapshot of [baseSnapshot, ...additionalSnapshots]) {
let isBaseSnapshot = additionalSnapshot === baseSnapshot;
Expand Down Expand Up @@ -203,7 +236,7 @@ async function* captureSnapshotResources(page, snapshot, options) {
}

// recursively trigger resource requests for any alternate device pixel ratio
if (deviceScaleFactor !== discovery.devicePixelRatio) {
if (!discovery.captureResponsiveAssets && deviceScaleFactor !== discovery.devicePixelRatio) {
yield waitForDiscoveryNetworkIdle(page, discovery);

yield* captureSnapshotResources(page, snapshot, {
Expand Down Expand Up @@ -308,7 +341,8 @@ export function createDiscoveryQueue(percy) {
try {
yield* captureSnapshotResources(page, snapshot, {
captureWidths: !snapshot.domSnapshot && percy.deferUploads,
capture: callback
capture: callback,
captureResponsiveDom: await responsiveDomParams(snapshot, percy.client)
});
} finally {
// always close the page when done
Expand Down
20 changes: 12 additions & 8 deletions packages/core/src/page.js
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,17 @@ export class Page {
for (let script of scripts) await this.eval(script);
}

async insertPercyDom() {
// inject @percy/dom for serialization by evaluating the file contents which adds a global
// PercyDOM object that we can later check against
/* istanbul ignore next: no instrumenting injected code */
if (await this.eval(() => !window.PercyDOM)) {
this.log.debug('Inject @percy/dom', this.meta);
let script = await fs.promises.readFile(PERCY_DOM, 'utf-8');
await this.eval(new Function(script)); /* eslint-disable-line no-new-func */
}
}

// Takes a snapshot after waiting for any timeout, waiting for any selector, executing any
// scripts, and waiting for the network idle. Returns all other provided snapshot options along
// with the captured URL and DOM snapshot.
Expand Down Expand Up @@ -165,14 +176,7 @@ export class Page {
// wait for any final network activity before capturing the dom snapshot
await this.network.idle();

// inject @percy/dom for serialization by evaluating the file contents which adds a global
// PercyDOM object that we can later check against
/* istanbul ignore next: no instrumenting injected code */
if (await this.eval(() => !window.PercyDOM)) {
this.log.debug('Inject @percy/dom', this.meta);
let script = await fs.promises.readFile(PERCY_DOM, 'utf-8');
await this.eval(new Function(script)); /* eslint-disable-line no-new-func */
}
await this.insertPercyDom();

// serialize and capture a DOM snapshot
this.log.debug('Serialize DOM', this.meta);
Expand Down
2 changes: 2 additions & 0 deletions packages/core/src/snapshot.js
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,8 @@ function getSnapshotOptions(options, { config, meta }) {
authorization: config.discovery.authorization,
disableCache: config.discovery.disableCache,
captureMockedServiceWorker: config.discovery.captureMockedServiceWorker,
captureSrcset: config.discovery.captureSrcset,
captureResponsiveAssets: config.discovery.captureResponsiveAssets,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should not be needed, we should call this explicitly again if there is enableJS true. If we want to keep it then we need confirmation from product.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally we should do this automatically.

userAgent: config.discovery.userAgent
}
}, options], (path, prev, next) => {
Expand Down
103 changes: 103 additions & 0 deletions packages/core/test/discovery.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2039,4 +2039,107 @@ describe('Discovery', () => {
]));
});
});

describe('Capture image srcset =>', () => {
it('make request call to capture resource', async () => {
let responsiveDOM = dedent`
<html>
<head><link href="style.css" rel="stylesheet"/></head>
<body>
<p>Hello Percy!<p>
<img srcset="/img-fromsrcset.png 400w, /img-throwserror.gif 600w, /img-withdifferentcontenttype.gif 800w"
sizes="(max-width: 600px) 400px, (max-width: 800px) 600px, 800px"
src="/img-already-captured.png">
</body>
</html>
`;
server.reply('/img-fromsrcset.png', () => [200, 'image/png', pixel]);
server.reply('/img-already-captured.png', () => [200, 'image/png', pixel]);
server.reply('/img-throwserror.gif', () => [404]);
server.reply('/img-withdifferentcontenttype.gif', () => [200, 'image/gif', pixel]);

let capturedResource = {
url: 'http://localhost:8000/img-already-captured.png',
content: 'R0lGODlhAQABAIAAAP///wAAACH5BAEAAAAALAAAAAABAAEAAAICRAEAOw==',
mimetype: 'image/png'
};
await percy.snapshot({
name: 'test responsive',
url: 'http://localhost:8000',
domSnapshot: {
html: responsiveDOM,
resources: [capturedResource]
},
discovery: {
captureSrcset: true
}
});

await percy.idle();

let resource = path => jasmine.objectContaining({
attributes: jasmine.objectContaining({
'resource-url': `http://localhost:8000${path}`
})
});

let paths = server.requests.map(r => r[0]);
expect(paths).toContain('/img-fromsrcset.png');
expect(paths).toContain('/img-withdifferentcontenttype.gif');
expect(paths).toContain('/img-throwserror.gif');
expect(captured[0]).toEqual(jasmine.arrayContaining([
resource('/img-fromsrcset.png'),
resource('/img-withdifferentcontenttype.gif')
]));
});

it('using snapshot command capture srcset', async () => {
let responsiveDOM = dedent`
<html>
<head></head>
<body>
<p>Hello Percy!<p>
<img srcset="/img-fromsrcset.png 2x, /img-throwserror.jpeg 3x, /img-withdifferentcontenttype.gif 4x, https://remote.resource.com/img-shouldnotcaptured.png 5x"
sizes="(max-width: 600px) 400px, (max-width: 800px) 600px, 800px"
src="/img-already-captured.png">
</body>
</html>
`;

server.reply('/', () => [200, 'text/html', responsiveDOM]);
server.reply('/img-fromsrcset.png', () => [200, 'image/png', pixel]);
server.reply('/img-already-captured.png', () => [200, 'image/png', pixel]);
server.reply('/img-throwserror.jpeg', () => [404]);
server.reply('/img-withdifferentcontenttype.gif', () => [200, 'image/gif', pixel]);

await percy.snapshot({
name: 'image srcset',
url: 'http://localhost:8000',
widths: [1024],
discovery: {
captureSrcset: true
}
});

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-fromsrcset.png'),
resource('/img-withdifferentcontenttype.gif'),
resource('/img-already-captured.png')
]));

expect(captured[0]).not.toContain(jasmine.objectContaining({
attributes: jasmine.objectContaining({
'resource-url': 'https://remote.resource.com/img-shouldnotcaptured.png'
})
}));
});
});
});
2 changes: 2 additions & 0 deletions packages/core/types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ interface DiscoveryOptions {
allowedHostnames?: string[];
disableCache?: boolean;
captureMockedServiceWorker?: boolean;
captureSrcset?: boolean;
captureResponsiveAssets?: boolean;
}

interface ScopeOptions {
Expand Down
2 changes: 2 additions & 0 deletions packages/dom/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,5 @@ export {
// namespace alias
serializeDOM as serialize
} from './serialize-dom';

export { loadAllSrcsetLinks } from './serialize-image-srcset';
42 changes: 42 additions & 0 deletions packages/dom/src/serialize-image-srcset.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
function getSrcsets(dom) {
const links = new Set();

for (let img of dom.querySelectorAll('img[srcset]')) {
handleSrcSet(img.srcset, links);
}

for (let picture of dom.querySelectorAll('picture')) {
for (let source of picture.querySelectorAll('source')) {
handleSrcSet(source.srcset, links);
}
}
return Array.from(links);
}

function handleSrcSet(srcSet, links) {
srcSet = srcSet.split(/,\s+/);
for (let src of srcSet) {
src = src.trim();
src = src.split(' ')[0];
links.add(getFormattedLink(src));
}
}
function getFormattedLink(src) {
const anchor = document.createElement('a');
anchor.href = src;
return anchor.href;
}

export function loadAllSrcsetLinks() {
const allImgTags = [];
const links = getSrcsets(document);
for (const link of links) {
const img = document.createElement('img');
img.src = link;
allImgTags.push(img);
}
// Adding to window so GC won't abort request
window.allImgTags = allImgTags;
return allImgTags;
}
export default loadAllSrcsetLinks;
51 changes: 51 additions & 0 deletions packages/dom/test/serialize-image-srcset.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import { withExample, platforms } from './helpers';

import { loadAllSrcsetLinks } from '@percy/dom';

describe('loadAllSrcsetLinks', () => {
let imgTags;

platforms.forEach((platform) => {
it(`${platform}: capture url from img srcset`, async () => {
withExample(`
<img srcset="base/test/assets/example.webp, base/test/assets/example.png 100px" />
`);

imgTags = loadAllSrcsetLinks();

expect(imgTags.map(s => s.src)).toEqual(['http://localhost:9876/base/test/assets/example.webp', 'http://localhost:9876/base/test/assets/example.png']);
});

it(`${platform}: capture url from source of picture`, async () => {
withExample(`
<picture>
<source srcset='//locahost:9876/base/test/assets/example.webp, //localhost:9876/base/test/assets/example.png 2x' />
<source srcset='//locahost:9876/base/test/assets/example.jpeg 100px, //locahost:9876/base/test/assets/example1.jpeg 200px' />
</picture>
`);

imgTags = loadAllSrcsetLinks();

expect(imgTags.map(s => s.src)).toEqual([
'http://locahost:9876/base/test/assets/example.webp',
'http://localhost:9876/base/test/assets/example.png',
'http://locahost:9876/base/test/assets/example.jpeg',
'http://locahost:9876/base/test/assets/example1.jpeg'
]);
});

it(`${platform}: srcset inside shadowroot`, () => {
withExample(`
<img srcset="/base/test/assets/example.webp, /base/test/assets/example.png 100px, /base/test/assets/example1.png 2x" />
`, { withShadow: true });

imgTags = loadAllSrcsetLinks();

expect(imgTags.map(s => s.src)).toEqual([
'http://localhost:9876/base/test/assets/example.webp',
'http://localhost:9876/base/test/assets/example.png',
'http://localhost:9876/base/test/assets/example1.png'
]);
});
});
});