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 18 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
12 changes: 12 additions & 0 deletions packages/client/src/client.js
Expand Up @@ -217,6 +217,18 @@ 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(buildId) {
try {
let url = 'device-details';
if (buildId) url += `?build_id=${buildId}`;
const { data } = await this.get(url);
Copy link
Contributor

Choose a reason for hiding this comment

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

This returns list only correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes

return data;
} catch (e) {
return [];
}
}

// Retrieves project builds optionally filtered. Requires a read access token.
async getBuilds(project, filters = {}) {
validateProjectPath(project);
Expand Down
14 changes: 14 additions & 0 deletions packages/client/test/client.test.js
Expand Up @@ -353,6 +353,20 @@ describe('PercyClient', () => {
});
});

describe('#getDeviceDetails()', () => {
it('in case of error return []', async () => {
api.reply('/device-details', () => [500]);
await expectAsync(client.getDeviceDetails()).toBeResolvedTo([]);
});

it('gets device details', async () => {
api.reply('/device-details', () => [200, { data: '<<device-data-without-build-id>>' }]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
api.reply('/device-details', () => [200, { data: '<<device-data-without-build-id>>' }]);
api.reply('/device-details', () => [200, { data: ['<<device-data-without-build-id>>' ]}]);

api.reply('/device-details?build_id=123', () => [200, { data: '<<device-data-with-build-id>>' }]);
await expectAsync(client.getDeviceDetails()).toBeResolvedTo('<<device-data-without-build-id>>');
await expectAsync(client.getDeviceDetails(123)).toBeResolvedTo('<<device-data-with-build-id>>');
});
});

describe('#getBuilds()', () => {
it('throws when missing a project path', async () => {
await expectAsync(client.getBuilds())
Expand Down
6 changes: 6 additions & 0 deletions packages/client/test/helpers.js
Expand Up @@ -139,6 +139,12 @@ export const api = {
'/snapshots/4567?sync=true&response_format=sync-cli': () => [201, {
name: 'test snapshot',
diff_ratio: 0
}],
'/device-details?build_id=123': () => [201, {
data: []
}],
'/device-details': () => [201, {
data: []
}]
},

Expand Down
5 changes: 5 additions & 0 deletions packages/core/src/config.js
Expand Up @@ -190,6 +190,10 @@ export const configSchema = {
type: 'boolean',
default: false
},
captureSrcset: {
type: 'boolean',
default: false
},
requestHeaders: {
type: 'object',
normalize: false,
Expand Down Expand Up @@ -276,6 +280,7 @@ 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' },
userAgent: { $ref: '/config/discovery#/properties/userAgent' },
devicePixelRatio: { $ref: '/config/discovery#/properties/devicePixelRatio' }
}
Expand Down
35 changes: 26 additions & 9 deletions packages/core/src/discovery.js
Expand Up @@ -55,6 +55,7 @@ function debugSnapshotOptions(snapshot) {
debugProp(snapshot, 'discovery.authorization', JSON.stringify);
debugProp(snapshot, 'discovery.disableCache');
debugProp(snapshot, 'discovery.captureMockedServiceWorker');
debugProp(snapshot, 'discovery.captureSrcset');
debugProp(snapshot, 'discovery.userAgent');
debugProp(snapshot, 'clientInfo');
debugProp(snapshot, 'environmentInfo');
Expand Down Expand Up @@ -141,7 +142,7 @@ function processSnapshotResources({ domSnapshot, resources, ...snapshot }) {
// 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 +172,14 @@ 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()');
}

// iterate over additional snapshots for proper DOM capturing
for (let additionalSnapshot of [baseSnapshot, ...additionalSnapshots]) {
let isBaseSnapshot = additionalSnapshot === baseSnapshot;
Expand All @@ -189,6 +198,16 @@ async function* captureSnapshotResources(page, snapshot, options) {
}
}

if (captureResponsiveDom) {
for (const device of captureResponsiveDom) {
yield waitForDiscoveryNetworkIdle(page, discovery);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
yield waitForDiscoveryNetworkIdle(page, discovery);
// We are not adding these widths and pixels ratios in loop above because we want to explicitly reload the page after resize which we dont do above
yield waitForDiscoveryNetworkIdle(page, discovery);

yield* captureSnapshotResources(page, { ...snapshot, widths: [device.width] }, {
deviceScaleFactor: device.deviceScaleFactor,
mobile: true
});
}
}

if (capture && !snapshot.domSnapshot) {
// capture this snapshot and update the base snapshot after capture
let captured = yield takeSnapshot(snap, width);
Expand All @@ -203,13 +222,10 @@ async function* captureSnapshotResources(page, snapshot, options) {
}

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

yield* captureSnapshotResources(page, snapshot, {
deviceScaleFactor: discovery.devicePixelRatio,
mobile: true
});
if (discovery.devicePixelRatio) {
const log = logger('core:discovery');
log.warn('discovery.devicePixelRatio is deprecated percy will now auto capture resource in all devicePixelRatio, Ignoring configuration');
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
log.warn('discovery.devicePixelRatio is deprecated percy will now auto capture resource in all devicePixelRatio, Ignoring configuration');
log.deprecate('discovery.devicePixelRatio is deprecated percy will now auto capture resource in all devicePixelRatio, Ignoring configuration');

discovery.devicePixelRatio = null;
}

// wait for final network idle when not capturing DOM
Expand Down Expand Up @@ -308,7 +324,8 @@ export function createDiscoveryQueue(percy) {
try {
yield* captureSnapshotResources(page, snapshot, {
captureWidths: !snapshot.domSnapshot && percy.deferUploads,
capture: callback
capture: callback,
captureResponsiveDom: percy.deviceDetails || []
Copy link
Contributor

Choose a reason for hiding this comment

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

captureForDevices

});
} finally {
// always close the page when done
Expand Down
20 changes: 12 additions & 8 deletions packages/core/src/page.js
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
1 change: 1 addition & 0 deletions packages/core/src/percy.js
Expand Up @@ -157,6 +157,7 @@ export class Percy {
if (!this.skipDiscovery) yield this.#discovery.start();
// start a local API server for SDK communication
if (this.server) yield this.server.listen();
if (this.projectType === 'web') this.deviceDetails = yield this.client.getDeviceDetails(this.build?.id);
const snapshotType = this.projectType === 'web' ? 'snapshot' : 'comparison';
this.syncQueue = new WaitForJob(snapshotType, this);
// log and mark this instance as started
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/snapshot.js
Expand Up @@ -117,6 +117,7 @@ function getSnapshotOptions(options, { config, meta }) {
authorization: config.discovery.authorization,
disableCache: config.discovery.disableCache,
captureMockedServiceWorker: config.discovery.captureMockedServiceWorker,
captureSrcset: config.discovery.captureSrcset,
userAgent: config.discovery.userAgent
}
}, options], (path, prev, next) => {
Expand Down