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 all 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
3 changes: 2 additions & 1 deletion packages/cli-snapshot/src/snapshot.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ export const snapshot = command('snapshot', {
],

percy: {
delayUploads: true
delayUploads: true,
projectType: 'web'
},

config: {
Expand Down
12 changes: 12 additions & 0 deletions packages/client/src/client.js
Original file line number Diff line number Diff line change
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 = 'discovery/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
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,20 @@ describe('PercyClient', () => {
});
});

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

it('gets device details', async () => {
api.reply('/discovery/device-details', () => [200, { data: ['<<device-data-without-build-id>>'] }]);
api.reply('/discovery/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
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,12 @@ export const api = {
'/snapshots/4567?sync=true&response_format=sync-cli': () => [201, {
name: 'test snapshot',
diff_ratio: 0
}],
'/discovery/device-details?build_id=123': () => [201, {
data: []
}],
'/discovery/device-details': () => [201, {
data: []
}]
},

Expand Down
27 changes: 18 additions & 9 deletions packages/core/src/discovery.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,9 @@ function processSnapshotResources({ domSnapshot, resources, ...snapshot }) {
// 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) {
const log = logger('core:discovery');
let { discovery, additionalSnapshots = [], ...baseSnapshot } = snapshot;
let { capture, captureWidths, deviceScaleFactor, mobile } = options;
let { capture, captureWidths, deviceScaleFactor, mobile, captureForDevices } = options;

// used to take snapshots and remove any discovered root resource
let takeSnapshot = async (options, width) => {
Expand Down Expand Up @@ -187,6 +188,18 @@ async function* captureSnapshotResources(page, snapshot, options) {
let { widths, execute } = snap;
let [width] = widths;

// iterate over device to trigger reqeusts and capture other dpr width
if (captureForDevices) {
for (const device of captureForDevices) {
yield waitForDiscoveryNetworkIdle(page, discovery);
// We are not adding these widths and pixels ratios in loop below because we want to explicitly reload the page after resize which we dont do below
yield* captureSnapshotResources(page, { ...snapshot, widths: [device.width] }, {
deviceScaleFactor: device.deviceScaleFactor,
mobile: true
});
}
}

// iterate over widths to trigger reqeusts and capture other widths
if (isBaseSnapshot || captureWidths) {
for (let i = 0; i < widths.length - 1; i++) {
Expand All @@ -212,13 +225,8 @@ 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) {
log.deprecated('discovery.devicePixelRatio is deprecated percy will now auto capture resource in all devicePixelRatio, Ignoring configuration');
}

// wait for final network idle when not capturing DOM
Expand Down Expand Up @@ -317,7 +325,8 @@ export function createDiscoveryQueue(percy) {
try {
yield* captureSnapshotResources(page, snapshot, {
captureWidths: !snapshot.domSnapshot && percy.deferUploads,
capture: callback
capture: callback,
captureForDevices: percy.deviceDetails || []
});
} finally {
// always close the page when done
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/percy.js
Original file line number Diff line number Diff line change
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