From 8ee61984ff06950c4bb7d2f4cd1d856a599cca1e Mon Sep 17 00:00:00 2001 From: Chinmay Maheshwari Date: Wed, 18 Sep 2024 11:05:42 +0530 Subject: [PATCH 01/12] [Defer Uploads v2] Added support for handling multiple root resource in single asset discovery phase --- packages/core/src/api.js | 7 ++++ packages/core/src/discovery.js | 71 +++++++++++++++++++++++++++------- packages/core/src/network.js | 4 +- packages/core/src/page.js | 2 + packages/core/src/utils.js | 4 +- 5 files changed, 70 insertions(+), 18 deletions(-) diff --git a/packages/core/src/api.js b/packages/core/src/api.js index 49abc1f29..a1ee0577e 100644 --- a/packages/core/src/api.js +++ b/packages/core/src/api.js @@ -78,6 +78,13 @@ export function createPercyServer(percy, port) { .route('get', '/percy/dom.js', (req, res) => { return res.file(200, PERCY_DOM); }) + // returns widths used in config and of devices enabled + .route('get', '/percy/widths', (req, res) => res.json(200, { + // This is always needed even if width is passed + mobile: percy.deviceDetails ? percy.deviceDetails.map((d) => d.width) : [], + // This will only be used if width is not passed in options + config: percy.config.snapshot.widths + })) // legacy agent wrapper for @percy/dom .route('get', '/percy-agent.js', async (req, res) => { logger('core:server').deprecated([ diff --git a/packages/core/src/discovery.js b/packages/core/src/discovery.js index 743d90e44..8031470c5 100644 --- a/packages/core/src/discovery.js +++ b/packages/core/src/discovery.js @@ -88,13 +88,34 @@ function waitForDiscoveryNetworkIdle(page, options) { // Creates an initial resource map for a snapshot containing serialized DOM function parseDomResources({ url, domSnapshot }) { - if (!domSnapshot) return new Map(); - let isHTML = typeof domSnapshot === 'string'; - let { html, resources = [] } = isHTML ? { html: domSnapshot } : domSnapshot; - let rootResource = createRootResource(url, html); + const map = new Map(); + if (!domSnapshot) return map; + let allResources = new Set(); + + if (Array.isArray(domSnapshot)) { + let allRootResources = new Set(); + for (let snapshot of domSnapshot) { + const dom = snapshot.domSnapshot; + const width = snapshot.width; + let isHTML = typeof dom === 'string'; + let { html, resources = [] } = isHTML ? { html: dom } : dom; + resources.forEach(r => allResources.add(r)); + let rootResource = createRootResource(url, html, { widths: [width] }); + allRootResources.add(rootResource); + } + allRootResources = Array.from(allRootResources); + map.set(allRootResources[0].url, allRootResources); + allResources = Array.from(allResources); + } else { + let isHTML = typeof domSnapshot === 'string'; + let { html, resources = [] } = isHTML ? { html: domSnapshot } : domSnapshot; + allResources = resources; + let rootResource = createRootResource(url, html); + map.set(rootResource.url, rootResource); + } // reduce the array of resources into a keyed map - return resources.reduce((map, { url, content, mimetype }) => { + return allResources.reduce((map, { url, content, mimetype }) => { // serialized resource contents are base64 encoded content = Buffer.from(content, mimetype.includes('text') ? 'utf8' : 'base64'); // specify the resource as provided to prevent overwriting during asset discovery @@ -102,7 +123,7 @@ function parseDomResources({ url, domSnapshot }) { // key the resource by its url and return the map return map.set(resource.url, resource); // the initial map is created with at least a root resource - }, new Map([[rootResource.url, rootResource]])); + }, map); } // Calls the provided callback with additional resources @@ -114,11 +135,21 @@ function processSnapshotResources({ domSnapshot, resources, ...snapshot }) { let rootContent = domSnapshot?.html ?? domSnapshot; let root = resources.find(r => r.content === rootContent); + let roots = root ? [root] : []; + if (!root && Array.isArray(domSnapshot)) { + // Only root resources are stored as array + roots = resources.find(r => Array.isArray(r)); + } + // initialize root resources if needed - if (!root) { + if (roots.length === 0) { let domResources = parseDomResources({ ...snapshot, domSnapshot }); resources = [...domResources.values(), ...resources]; - root = resources[0]; + if (Array.isArray(domSnapshot)) { + roots = resources.find(r => Array.isArray(r)); + } else { + roots = [resources[0]]; + } } // inject Percy CSS @@ -129,16 +160,21 @@ function processSnapshotResources({ domSnapshot, resources, ...snapshot }) { log.warn('DOM elements found outside , percyCSS might not work'); } - let css = createPercyCSSResource(root.url, snapshot.percyCSS); + let css = createPercyCSSResource(roots[0].url, snapshot.percyCSS); resources.push(css); // replace root contents and associated properties - Object.assign(root, createRootResource(root.url, ( - root.content.replace(/(<\/body>)(?!.*\1)/is, ( - `` - ) + '$&')))); + roots.forEach(root => { + Object.assign(root, createRootResource(root.url, ( + root.content.replace(/(<\/body>)(?!.*\1)/is, ( + `` + ) + '$&')))); + }); } + // For multi dom root resources are stored as array + resources = resources.flat(); + // include associated snapshot logs matched by meta information resources.push(createLogResource(logger.query(log => ( log.meta.snapshot?.testCase === snapshot.meta.snapshot.testCase && log.meta.snapshot?.name === snapshot.meta.snapshot.name @@ -365,7 +401,14 @@ export function createDiscoveryQueue(percy) { disableCache: snapshot.discovery.disableCache, allowedHostnames: snapshot.discovery.allowedHostnames, disallowedHostnames: snapshot.discovery.disallowedHostnames, - getResource: u => snapshot.resources.get(u) || cache.get(u), + getResource: (u, width = null) => { + let resource = snapshot.resources.get(u) || cache.get(u); + if (resource && Array.isArray(resource) && resource[0].root) { + const rootResource = resource.find(r => r.widths.includes(width)); + resource = rootResource || resource[0]; + } + return resource; + }, saveResource: r => { snapshot.resources.set(r.url, r); if (!r.root) { cache.set(r.url, r); } } } }); diff --git a/packages/core/src/network.js b/packages/core/src/network.js index 711518fd7..1aa542f42 100644 --- a/packages/core/src/network.js +++ b/packages/core/src/network.js @@ -354,7 +354,7 @@ async function sendResponseResource(network, request, session) { let send = (method, params) => network.send(session, method, params); try { - let resource = network.intercept.getResource(url); + let resource = network.intercept.getResource(url, network.page.width); network.log.debug(`Handling request: ${url}`, meta); if (!resource?.root && hostnameMatches(disallowedHostnames, url)) { @@ -495,7 +495,7 @@ async function saveResponseResource(network, request) { } } - if (resource) { + if (resource && !resource.root) { network.intercept.saveResource(resource); } } diff --git a/packages/core/src/page.js b/packages/core/src/page.js index 80a936e31..8ed367b90 100644 --- a/packages/core/src/page.js +++ b/packages/core/src/page.js @@ -18,6 +18,7 @@ export class Page { this.session = session; this.browser = session.browser; this.enableJavaScript = options.enableJavaScript ?? true; + this.width = null; this.network = new Network(this, options); this.meta = options.meta; this._initializeLoadTimeout(); @@ -40,6 +41,7 @@ export class Page { async resize({ width, height, deviceScaleFactor = 1, mobile = false }) { this.log.debug(`Resize page to ${width}x${height} @${deviceScaleFactor}x`); + this.width = width; await this.session.send('Emulation.setDeviceMetricsOverride', { deviceScaleFactor, mobile, diff --git a/packages/core/src/utils.js b/packages/core/src/utils.js index 163fe7702..cac47e84f 100644 --- a/packages/core/src/utils.js +++ b/packages/core/src/utils.js @@ -113,8 +113,8 @@ export function createResource(url, content, mimetype, attrs) { // Creates a root resource object with an additional `root: true` property. The URL is normalized // here as a convenience since root resources are usually created outside of asset discovery. -export function createRootResource(url, content) { - return createResource(normalizeURL(url), content, 'text/html', { root: true }); +export function createRootResource(url, content, attrs = {}) { + return createResource(normalizeURL(url), content, 'text/html', { ...attrs, root: true }); } // Creates a Percy CSS resource object. From fb2afdeb04b2d1bd250a594b86da7b2d76cb8cf8 Mon Sep 17 00:00:00 2001 From: Chinmay Maheshwari Date: Wed, 18 Sep 2024 18:00:35 +0530 Subject: [PATCH 02/12] update domSnapshot structure in config + reload page in case on multidom --- packages/core/src/config.js | 14 ++++++++++++++ packages/core/src/discovery.js | 3 ++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/packages/core/src/config.js b/packages/core/src/config.js index 017e042cd..78feed7f5 100644 --- a/packages/core/src/config.js +++ b/packages/core/src/config.js @@ -78,6 +78,10 @@ export const configSchema = { sync: { type: 'boolean' }, + multiDOM: { + type: 'boolean', + default: false + }, testCase: { type: 'string' }, @@ -291,6 +295,7 @@ export const snapshotSchema = { domTransformation: { $ref: '/config/snapshot#/properties/domTransformation' }, enableLayout: { $ref: '/config/snapshot#/properties/enableLayout' }, sync: { $ref: '/config/snapshot#/properties/sync' }, + multiDOM: { $ref: '/config/snapshot#/properties/multiDOM' }, testCase: { $ref: '/config/snapshot#/properties/testCase' }, labels: { $ref: '/config/snapshot#/properties/labels' }, thTestCaseExecutionId: { $ref: '/config/snapshot#/properties/thTestCaseExecutionId' }, @@ -473,6 +478,15 @@ export const snapshotSchema = { items: { type: 'string' } } } + }, { + type: 'array', + items: { + type: 'object', + properties: { + domSnapshot: { $ref: '/snapshot#/$defs/dom/properties/domSnapshot/oneOf/1' }, + width: { $ref: '/config/snapshot#/properties/widths/items' } + } + } }] } }, diff --git a/packages/core/src/discovery.js b/packages/core/src/discovery.js index 8031470c5..901f86357 100644 --- a/packages/core/src/discovery.js +++ b/packages/core/src/discovery.js @@ -264,7 +264,7 @@ async function* captureSnapshotResources(page, snapshot, options) { // 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) { + if (!snapshot.multiDOM && discovery.captureSrcset) { await page.insertPercyDom(); yield page.eval('window.PercyDOM.loadAllSrcsetLinks()'); } @@ -283,6 +283,7 @@ async function* captureSnapshotResources(page, snapshot, options) { yield page.evaluate(execute?.beforeResize); yield waitForDiscoveryNetworkIdle(page, discovery); yield resizePage(width = widths[i + 1]); + if (snapshot.multiDOM) { yield page.goto(snapshot.url, { cookies }); } yield page.evaluate(execute?.afterResize); } } From 9c25c5be60f9813dee60f3c83dad205410582202 Mon Sep 17 00:00:00 2001 From: Chinmay Maheshwari Date: Wed, 18 Sep 2024 18:14:09 +0530 Subject: [PATCH 03/12] Fix test --- packages/core/test/percy.test.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/core/test/percy.test.js b/packages/core/test/percy.test.js index 4228e73c1..6d71bf183 100644 --- a/packages/core/test/percy.test.js +++ b/packages/core/test/percy.test.js @@ -74,7 +74,8 @@ describe('Percy', () => { percyCSS: '', enableJavaScript: false, disableShadowDOM: false, - cliEnableJavaScript: true + cliEnableJavaScript: true, + multiDOM: false }); }); From 539c49b3d8da561aec4c65bf1369a0f1de3528e1 Mon Sep 17 00:00:00 2001 From: Chinmay Maheshwari Date: Wed, 18 Sep 2024 19:09:09 +0530 Subject: [PATCH 04/12] Added tests for widths endpoint + multiple root resources --- packages/core/test/api.test.js | 20 ++++++++++ packages/core/test/discovery.test.js | 57 ++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+) diff --git a/packages/core/test/api.test.js b/packages/core/test/api.test.js index fa6c1c789..ecfcd6ad7 100644 --- a/packages/core/test/api.test.js +++ b/packages/core/test/api.test.js @@ -69,6 +69,26 @@ describe('API Server', () => { }); }); + it('has a /widths endpoint that returns widths present in config', async () => { + await percy.start(); + percy.config = PercyConfig.getDefaults({ snapshot: { widths: [1000] } }); + + await expectAsync(request('/percy/widths')).toBeResolvedTo({ + mobile: [], + config: [1000] + }); + }); + + it('has a /widths endpoint that returns widths present in config and fetch widths for devices', async () => { + await percy.start(); + percy.deviceDetails = [{ width: 390, devicePixelRatio: 2 }]; + + await expectAsync(request('/percy/widths')).toBeResolvedTo({ + mobile: [390], + config: PercyConfig.getDefaults().snapshot.widths + }); + }); + it('can set config options via the /config endpoint', async () => { let expected = PercyConfig.getDefaults({ snapshot: { widths: [1000] } }); await percy.start(); diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js index d4e2ca9d8..0d56808a3 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -2885,4 +2885,61 @@ describe('Discovery', () => { })); }); }); + + describe('Handles multiple root resources', () => { + it('gathers resources for a snapshot', async () => { + let DOM1 = testDOM.replace('Percy!', 'Percy! at 370'); + let DOM2 = testDOM.replace('Percy!', 'Percy! at 765'); + + await percy.snapshot({ + name: 'test snapshot', + url: 'http://localhost:8000', + domSnapshot: [{ + domSnapshot: testDOM, + width: 1280 + }, { + domSnapshot: DOM1, + width: 370 + }, { + domSnapshot: DOM2, + width: 765 + }] + }); + + await percy.idle(); + + let paths = server.requests.map(r => r[0]); + // does not request the root url (serves domSnapshot instead) + expect(paths).not.toContain('/'); + expect(paths).toContain('/style.css'); + expect(paths).toContain('/img.gif'); + + expect(captured[0]).toEqual(jasmine.arrayContaining([ + jasmine.objectContaining({ + id: sha256hash(testDOM), + attributes: jasmine.objectContaining({ + 'resource-url': 'http://localhost:8000/', + 'is-root': true, + 'for-widths': [1280] + }) + }), + jasmine.objectContaining({ + id: sha256hash(DOM1), + attributes: jasmine.objectContaining({ + 'resource-url': 'http://localhost:8000/', + 'is-root': true, + 'for-widths': [370] + }) + }), + jasmine.objectContaining({ + id: sha256hash(DOM2), + attributes: jasmine.objectContaining({ + 'resource-url': 'http://localhost:8000/', + 'is-root': true, + 'for-widths': [765] + }) + }) + ])); + }); + }); }); From 78c7678f5b42a9ec0ade3d683ca4e717d46501d7 Mon Sep 17 00:00:00 2001 From: Chinmay Maheshwari Date: Wed, 18 Sep 2024 19:10:33 +0530 Subject: [PATCH 05/12] add multi dom case --- packages/core/test/discovery.test.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js index 0d56808a3..02dc59705 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -2894,6 +2894,7 @@ describe('Discovery', () => { await percy.snapshot({ name: 'test snapshot', url: 'http://localhost:8000', + multiDOM: true, domSnapshot: [{ domSnapshot: testDOM, width: 1280 From ebed72a6728739669f07ad1ae780af94f4d58dd6 Mon Sep 17 00:00:00 2001 From: Chinmay Maheshwari Date: Wed, 18 Sep 2024 19:26:28 +0530 Subject: [PATCH 06/12] Added case for percyCSS with multiple root resource --- packages/core/test/discovery.test.js | 55 ++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js index 02dc59705..461722156 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -2942,5 +2942,60 @@ describe('Discovery', () => { }) ])); }); + + it('injects the percy-css resource into all dom snapshots', async () => { + const simpleDOM = dedent` + + + +

Hello Percy!

+ + + `; + let DOM1 = simpleDOM.replace('Percy!', 'Percy! at 370'); + + await percy.snapshot({ + name: 'test snapshot', + url: 'http://localhost:8000', + multiDOM: true, + percyCSS: 'body { color: purple; }', + domSnapshot: [{ + domSnapshot: simpleDOM, + width: 1280 + }, { + domSnapshot: DOM1, + width: 370 + }] + }); + + await percy.idle(); + + let cssURL = new URL((api.requests['/builds/123/snapshots'][0].body.data.relationships.resources.data).find(r => r.attributes['resource-url'].endsWith('.css')).attributes['resource-url']); + let injectedDOM = simpleDOM.replace('', ( + `` + ) + ''); + let injectedDOM1 = DOM1.replace('', ( + `` + ) + ''); + + expect(captured[0]).toEqual(jasmine.arrayContaining([ + jasmine.objectContaining({ + id: sha256hash(injectedDOM), + attributes: jasmine.objectContaining({ + 'resource-url': 'http://localhost:8000/', + 'is-root': true, + 'for-widths': [1280] + }) + }), + jasmine.objectContaining({ + id: sha256hash(injectedDOM1), + attributes: jasmine.objectContaining({ + 'resource-url': 'http://localhost:8000/', + 'is-root': true, + 'for-widths': [370] + }) + }) + ])); + }); }); }); From d487b6a75344b989542752f6d992a1cf945d823b Mon Sep 17 00:00:00 2001 From: Chinmay Maheshwari Date: Thu, 19 Sep 2024 00:15:51 +0530 Subject: [PATCH 07/12] Remove extra code and improve coverage --- packages/core/src/discovery.js | 6 +----- packages/core/test/discovery.test.js | 3 ++- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/packages/core/src/discovery.js b/packages/core/src/discovery.js index 901f86357..4c4252f7f 100644 --- a/packages/core/src/discovery.js +++ b/packages/core/src/discovery.js @@ -145,11 +145,7 @@ function processSnapshotResources({ domSnapshot, resources, ...snapshot }) { if (roots.length === 0) { let domResources = parseDomResources({ ...snapshot, domSnapshot }); resources = [...domResources.values(), ...resources]; - if (Array.isArray(domSnapshot)) { - roots = resources.find(r => Array.isArray(r)); - } else { - roots = [resources[0]]; - } + roots = [resources[0]]; } // inject Percy CSS diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js index 461722156..7a8483ef1 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -2895,11 +2895,12 @@ describe('Discovery', () => { name: 'test snapshot', url: 'http://localhost:8000', multiDOM: true, + widths: [365, 1280], domSnapshot: [{ domSnapshot: testDOM, width: 1280 }, { - domSnapshot: DOM1, + domSnapshot: { html: DOM1 }, width: 370 }, { domSnapshot: DOM2, From 70f370f0344798a8e7aa24c3e808dbfc94546101 Mon Sep 17 00:00:00 2001 From: Chinmay Maheshwari Date: Thu, 19 Sep 2024 00:26:39 +0530 Subject: [PATCH 08/12] Fix coverage --- packages/core/src/discovery.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/src/discovery.js b/packages/core/src/discovery.js index 4c4252f7f..60f7c278a 100644 --- a/packages/core/src/discovery.js +++ b/packages/core/src/discovery.js @@ -406,7 +406,7 @@ export function createDiscoveryQueue(percy) { } return resource; }, - saveResource: r => { snapshot.resources.set(r.url, r); if (!r.root) { cache.set(r.url, r); } } + saveResource: r => { snapshot.resources.set(r.url, r); cache.set(r.url, r); } } }); From d7de0d93837ebf53c107d7565e653b3ca039214a Mon Sep 17 00:00:00 2001 From: Chinmay Maheshwari Date: Thu, 19 Sep 2024 14:12:41 +0530 Subject: [PATCH 09/12] Coverage resolved --- packages/core/test/discovery.test.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js index 7a8483ef1..40cc81edc 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -2890,6 +2890,11 @@ describe('Discovery', () => { it('gathers resources for a snapshot', async () => { let DOM1 = testDOM.replace('Percy!', 'Percy! at 370'); let DOM2 = testDOM.replace('Percy!', 'Percy! at 765'); + const capturedResource = { + url: 'http://localhost:8000/img-already-captured.png', + content: 'R0lGODlhAQABAIAAAP///wAAACH5BAEAAAAALAAAAAABAAEAAAICRAEAOw==', + mimetype: 'image/png' + }; await percy.snapshot({ name: 'test snapshot', @@ -2900,7 +2905,7 @@ describe('Discovery', () => { domSnapshot: testDOM, width: 1280 }, { - domSnapshot: { html: DOM1 }, + domSnapshot: { html: DOM1, resources: [capturedResource] }, width: 370 }, { domSnapshot: DOM2, From fc065a2a29df56175b6bf343c0624b13175e6554 Mon Sep 17 00:00:00 2001 From: Chinmay Maheshwari Date: Thu, 19 Sep 2024 19:48:57 +0530 Subject: [PATCH 10/12] Addressed comments --- packages/core/src/api.js | 13 ++--- packages/core/src/discovery.js | 100 +++++++++++++++++---------------- packages/core/src/network.js | 2 +- packages/core/src/page.js | 9 ++- packages/core/test/api.test.js | 24 +++----- 5 files changed, 73 insertions(+), 75 deletions(-) diff --git a/packages/core/src/api.js b/packages/core/src/api.js index a1ee0577e..47544f445 100644 --- a/packages/core/src/api.js +++ b/packages/core/src/api.js @@ -62,6 +62,12 @@ export function createPercyServer(percy, port) { build: percy.testing?.build ?? percy.build, loglevel: percy.loglevel(), config: percy.config, + widths: { + // This is always needed even if width is passed + mobile: percy.deviceDetails ? percy.deviceDetails.map((d) => d.width) : [], + // This will only be used if width is not passed in options + config: percy.config.snapshot.widths + }, success: true, type: percy.client.tokenType() })) @@ -78,13 +84,6 @@ export function createPercyServer(percy, port) { .route('get', '/percy/dom.js', (req, res) => { return res.file(200, PERCY_DOM); }) - // returns widths used in config and of devices enabled - .route('get', '/percy/widths', (req, res) => res.json(200, { - // This is always needed even if width is passed - mobile: percy.deviceDetails ? percy.deviceDetails.map((d) => d.width) : [], - // This will only be used if width is not passed in options - config: percy.config.snapshot.widths - })) // legacy agent wrapper for @percy/dom .route('get', '/percy-agent.js', async (req, res) => { logger('core:server').deprecated([ diff --git a/packages/core/src/discovery.js b/packages/core/src/discovery.js index 60f7c278a..1c9ebf7e7 100644 --- a/packages/core/src/discovery.js +++ b/packages/core/src/discovery.js @@ -90,29 +90,25 @@ function waitForDiscoveryNetworkIdle(page, options) { function parseDomResources({ url, domSnapshot }) { const map = new Map(); if (!domSnapshot) return map; + let allRootResources = new Set(); let allResources = new Set(); - if (Array.isArray(domSnapshot)) { - let allRootResources = new Set(); - for (let snapshot of domSnapshot) { - const dom = snapshot.domSnapshot; - const width = snapshot.width; - let isHTML = typeof dom === 'string'; - let { html, resources = [] } = isHTML ? { html: dom } : dom; - resources.forEach(r => allResources.add(r)); - let rootResource = createRootResource(url, html, { widths: [width] }); - allRootResources.add(rootResource); - } - allRootResources = Array.from(allRootResources); - map.set(allRootResources[0].url, allRootResources); - allResources = Array.from(allResources); - } else { - let isHTML = typeof domSnapshot === 'string'; - let { html, resources = [] } = isHTML ? { html: domSnapshot } : domSnapshot; - allResources = resources; - let rootResource = createRootResource(url, html); - map.set(rootResource.url, rootResource); + if (!Array.isArray(domSnapshot)) { + domSnapshot = [{ domSnapshot }]; + } + + for (let snapshot of domSnapshot) { + const dom = snapshot.domSnapshot; + let isHTML = typeof dom === 'string'; + let { html, resources = [] } = isHTML ? { html: dom } : dom; + resources.forEach(r => allResources.add(r)); + const attrs = snapshot.width ? { widths: [snapshot.width] } : {}; + let rootResource = createRootResource(url, html, attrs); + allRootResources.add(rootResource); } + allRootResources = Array.from(allRootResources); + map.set(allRootResources[0].url, allRootResources); + allResources = Array.from(allResources); // reduce the array of resources into a keyed map return allResources.reduce((map, { url, content, mimetype }) => { @@ -126,26 +122,34 @@ function parseDomResources({ url, domSnapshot }) { }, map); } +function createAndApplyPercyCSS({ percyCSS, roots }) { + let css = createPercyCSSResource(roots[0].url, percyCSS); + + // replace root contents and associated properties + roots.forEach(root => { + Object.assign(root, createRootResource(root.url, ( + root.content.replace(/(<\/body>)(?!.*\1)/is, ( + `` + ) + '$&')))); + }); + + return css; +} + // Calls the provided callback with additional resources function processSnapshotResources({ domSnapshot, resources, ...snapshot }) { let log = logger('core:snapshot'); resources = [...(resources?.values() ?? [])]; // find any root resource matching the provided dom snapshot - let rootContent = domSnapshot?.html ?? domSnapshot; - let root = resources.find(r => r.content === rootContent); - - let roots = root ? [root] : []; - if (!root && Array.isArray(domSnapshot)) { - // Only root resources are stored as array - roots = resources.find(r => Array.isArray(r)); - } + // since root resources are stored as array + let roots = resources.find(r => Array.isArray(r)); // initialize root resources if needed - if (roots.length === 0) { + if (!roots) { let domResources = parseDomResources({ ...snapshot, domSnapshot }); resources = [...domResources.values(), ...resources]; - roots = [resources[0]]; + roots = resources.find(r => Array.isArray(r)); } // inject Percy CSS @@ -156,16 +160,8 @@ function processSnapshotResources({ domSnapshot, resources, ...snapshot }) { log.warn('DOM elements found outside , percyCSS might not work'); } - let css = createPercyCSSResource(roots[0].url, snapshot.percyCSS); - resources.push(css); - - // replace root contents and associated properties - roots.forEach(root => { - Object.assign(root, createRootResource(root.url, ( - root.content.replace(/(<\/body>)(?!.*\1)/is, ( - `` - ) + '$&')))); - }); + const percyCSSReource = createAndApplyPercyCSS({ percyCSS: snapshot.percyCSS, roots }); + resources.push(percyCSSReource); } // For multi dom root resources are stored as array @@ -227,16 +223,21 @@ async function* captureSnapshotResources(page, snapshot, options) { }; // used to resize the using capture options - let resizePage = width => page.resize({ - height: snapshot.minHeight, - deviceScaleFactor, - mobile, - width - }); + let resizePage = width => { + page.network.currentWidth = width; + return page.resize({ + height: snapshot.minHeight, + deviceScaleFactor, + mobile, + width + }); + }; // navigate to the url yield resizePage(snapshot.widths[0]); - yield page.goto(snapshot.url, { cookies }); + let forceReload = false; + if (discovery.captureResponsiveAssetsEnabled) { forceReload = true; } + yield page.goto(snapshot.url, { cookies, forceReload }); // wait for any specified timeout if (snapshot.discovery.waitForTimeout && page.enableJavaScript) { @@ -260,6 +261,7 @@ async function* captureSnapshotResources(page, snapshot, options) { // 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 + // Don't run incase of multiDOM since we are running discovery for all widths so images will get captured in all required widths if (!snapshot.multiDOM && discovery.captureSrcset) { await page.insertPercyDom(); yield page.eval('window.PercyDOM.loadAllSrcsetLinks()'); @@ -279,7 +281,7 @@ async function* captureSnapshotResources(page, snapshot, options) { yield page.evaluate(execute?.beforeResize); yield waitForDiscoveryNetworkIdle(page, discovery); yield resizePage(width = widths[i + 1]); - if (snapshot.multiDOM) { yield page.goto(snapshot.url, { cookies }); } + if (snapshot.multiDOM) { yield page.goto(snapshot.url, { cookies, forceReload: true }); } yield page.evaluate(execute?.afterResize); } } @@ -401,7 +403,7 @@ export function createDiscoveryQueue(percy) { getResource: (u, width = null) => { let resource = snapshot.resources.get(u) || cache.get(u); if (resource && Array.isArray(resource) && resource[0].root) { - const rootResource = resource.find(r => r.widths.includes(width)); + const rootResource = resource.find(r => r.widths?.includes(width)); resource = rootResource || resource[0]; } return resource; diff --git a/packages/core/src/network.js b/packages/core/src/network.js index 1aa542f42..b66f79060 100644 --- a/packages/core/src/network.js +++ b/packages/core/src/network.js @@ -354,7 +354,7 @@ async function sendResponseResource(network, request, session) { let send = (method, params) => network.send(session, method, params); try { - let resource = network.intercept.getResource(url, network.page.width); + let resource = network.intercept.getResource(url, network.currentWidth); network.log.debug(`Handling request: ${url}`, meta); if (!resource?.root && hostnameMatches(disallowedHostnames, url)) { diff --git a/packages/core/src/page.js b/packages/core/src/page.js index 8ed367b90..8f778d7be 100644 --- a/packages/core/src/page.js +++ b/packages/core/src/page.js @@ -18,7 +18,6 @@ export class Page { this.session = session; this.browser = session.browser; this.enableJavaScript = options.enableJavaScript ?? true; - this.width = null; this.network = new Network(this, options); this.meta = options.meta; this._initializeLoadTimeout(); @@ -41,7 +40,6 @@ export class Page { async resize({ width, height, deviceScaleFactor = 1, mobile = false }) { this.log.debug(`Resize page to ${width}x${height} @${deviceScaleFactor}x`); - this.width = width; await this.session.send('Emulation.setDeviceMetricsOverride', { deviceScaleFactor, mobile, @@ -70,9 +68,14 @@ export class Page { } // Go to a URL and wait for navigation to occur - async goto(url, { waitUntil = 'load', cookies } = {}) { + async goto(url, { waitUntil = 'load', cookies, forceReload } = {}) { this.log.debug(`Navigate to: ${url}`, this.meta); + if (forceReload) { + this.log.debug('Navigating to blank page', this.meta); + await this.session.send('Page.navigate', { url: 'about:blank' }); + } + let navigate = async () => { const userPassedCookie = this.session.browser.cookies; // set cookies before navigation so we can default the domain to this hostname diff --git a/packages/core/test/api.test.js b/packages/core/test/api.test.js index ecfcd6ad7..26c476625 100644 --- a/packages/core/test/api.test.js +++ b/packages/core/test/api.test.js @@ -51,6 +51,7 @@ describe('API Server', () => { success: true, loglevel: 'info', config: PercyConfig.getDefaults(), + widths: { mobile: [], config: PercyConfig.getDefaults().snapshot.widths }, build: { id: '123', number: 1, @@ -69,24 +70,17 @@ describe('API Server', () => { }); }); - it('has a /widths endpoint that returns widths present in config', async () => { - await percy.start(); - percy.config = PercyConfig.getDefaults({ snapshot: { widths: [1000] } }); - - await expectAsync(request('/percy/widths')).toBeResolvedTo({ - mobile: [], - config: [1000] - }); - }); - - it('has a /widths endpoint that returns widths present in config and fetch widths for devices', async () => { + it('should return widths present in config and fetch widths for devices', async () => { await percy.start(); percy.deviceDetails = [{ width: 390, devicePixelRatio: 2 }]; + percy.config = PercyConfig.getDefaults({ snapshot: { widths: [1000] } }); - await expectAsync(request('/percy/widths')).toBeResolvedTo({ - mobile: [390], - config: PercyConfig.getDefaults().snapshot.widths - }); + await expectAsync(request('/percy/healthcheck')).toBeResolvedTo(jasmine.objectContaining({ + widths: { + mobile: [390], + config: [1000] + } + })); }); it('can set config options via the /config endpoint', async () => { From af0989e380518164510ced0657b0bd0879f0fad6 Mon Sep 17 00:00:00 2001 From: Chinmay Maheshwari Date: Fri, 20 Sep 2024 11:07:00 +0530 Subject: [PATCH 11/12] Minor improvement --- packages/core/src/discovery.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/core/src/discovery.js b/packages/core/src/discovery.js index 1c9ebf7e7..a90efc644 100644 --- a/packages/core/src/discovery.js +++ b/packages/core/src/discovery.js @@ -235,9 +235,7 @@ async function* captureSnapshotResources(page, snapshot, options) { // navigate to the url yield resizePage(snapshot.widths[0]); - let forceReload = false; - if (discovery.captureResponsiveAssetsEnabled) { forceReload = true; } - yield page.goto(snapshot.url, { cookies, forceReload }); + yield page.goto(snapshot.url, { cookies, forceReload: discovery.captureResponsiveAssetsEnabled }); // wait for any specified timeout if (snapshot.discovery.waitForTimeout && page.enableJavaScript) { From dffcee04e5aefc3cc7f9f84ceaa655ad5adf9972 Mon Sep 17 00:00:00 2001 From: Chinmay Maheshwari Date: Fri, 20 Sep 2024 17:46:45 +0530 Subject: [PATCH 12/12] Addressed comments --- packages/core/src/config.js | 18 ++++++------------ packages/core/src/discovery.js | 15 +++++++-------- packages/core/src/network.js | 2 +- packages/core/test/discovery.test.js | 17 +++++++++-------- packages/core/test/percy.test.js | 2 +- 5 files changed, 24 insertions(+), 30 deletions(-) diff --git a/packages/core/src/config.js b/packages/core/src/config.js index 78feed7f5..68d090244 100644 --- a/packages/core/src/config.js +++ b/packages/core/src/config.js @@ -78,7 +78,7 @@ export const configSchema = { sync: { type: 'boolean' }, - multiDOM: { + responsiveSnapshotCapture: { type: 'boolean', default: false }, @@ -295,7 +295,7 @@ export const snapshotSchema = { domTransformation: { $ref: '/config/snapshot#/properties/domTransformation' }, enableLayout: { $ref: '/config/snapshot#/properties/enableLayout' }, sync: { $ref: '/config/snapshot#/properties/sync' }, - multiDOM: { $ref: '/config/snapshot#/properties/multiDOM' }, + responsiveSnapshotCapture: { $ref: '/config/snapshot#/properties/responsiveSnapshotCapture' }, testCase: { $ref: '/config/snapshot#/properties/testCase' }, labels: { $ref: '/config/snapshot#/properties/labels' }, thTestCaseExecutionId: { $ref: '/config/snapshot#/properties/thTestCaseExecutionId' }, @@ -460,6 +460,7 @@ export const snapshotSchema = { items: { type: 'string' } }, cookies: { type: 'string' }, + width: { $ref: '/config/snapshot#/properties/widths/items' }, resources: { type: 'array', items: { @@ -478,16 +479,9 @@ export const snapshotSchema = { items: { type: 'string' } } } - }, { - type: 'array', - items: { - type: 'object', - properties: { - domSnapshot: { $ref: '/snapshot#/$defs/dom/properties/domSnapshot/oneOf/1' }, - width: { $ref: '/config/snapshot#/properties/widths/items' } - } - } - }] + }, + { type: 'array', items: { $ref: '/snapshot#/$defs/dom/properties/domSnapshot/oneOf/1' } } + ] } }, errors: { diff --git a/packages/core/src/discovery.js b/packages/core/src/discovery.js index a90efc644..d03d89200 100644 --- a/packages/core/src/discovery.js +++ b/packages/core/src/discovery.js @@ -94,15 +94,14 @@ function parseDomResources({ url, domSnapshot }) { let allResources = new Set(); if (!Array.isArray(domSnapshot)) { - domSnapshot = [{ domSnapshot }]; + domSnapshot = [domSnapshot]; } - for (let snapshot of domSnapshot) { - const dom = snapshot.domSnapshot; + for (let dom of domSnapshot) { let isHTML = typeof dom === 'string'; let { html, resources = [] } = isHTML ? { html: dom } : dom; resources.forEach(r => allResources.add(r)); - const attrs = snapshot.width ? { widths: [snapshot.width] } : {}; + const attrs = dom.width ? { widths: [dom.width] } : {}; let rootResource = createRootResource(url, html, attrs); allRootResources.add(rootResource); } @@ -224,7 +223,7 @@ async function* captureSnapshotResources(page, snapshot, options) { // used to resize the using capture options let resizePage = width => { - page.network.currentWidth = width; + page.network.intercept.currentWidth = width; return page.resize({ height: snapshot.minHeight, deviceScaleFactor, @@ -259,8 +258,8 @@ async function* captureSnapshotResources(page, snapshot, options) { // 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 - // Don't run incase of multiDOM since we are running discovery for all widths so images will get captured in all required widths - if (!snapshot.multiDOM && discovery.captureSrcset) { + // Don't run incase of responsiveSnapshotCapture since we are running discovery for all widths so images will get captured in all required widths + if (!snapshot.responsiveSnapshotCapture && discovery.captureSrcset) { await page.insertPercyDom(); yield page.eval('window.PercyDOM.loadAllSrcsetLinks()'); } @@ -279,7 +278,7 @@ async function* captureSnapshotResources(page, snapshot, options) { yield page.evaluate(execute?.beforeResize); yield waitForDiscoveryNetworkIdle(page, discovery); yield resizePage(width = widths[i + 1]); - if (snapshot.multiDOM) { yield page.goto(snapshot.url, { cookies, forceReload: true }); } + if (snapshot.responsiveSnapshotCapture) { yield page.goto(snapshot.url, { cookies, forceReload: true }); } yield page.evaluate(execute?.afterResize); } } diff --git a/packages/core/src/network.js b/packages/core/src/network.js index b66f79060..0c81fa4b1 100644 --- a/packages/core/src/network.js +++ b/packages/core/src/network.js @@ -354,7 +354,7 @@ async function sendResponseResource(network, request, session) { let send = (method, params) => network.send(session, method, params); try { - let resource = network.intercept.getResource(url, network.currentWidth); + let resource = network.intercept.getResource(url, network.intercept.currentWidth); network.log.debug(`Handling request: ${url}`, meta); if (!resource?.root && hostnameMatches(disallowedHostnames, url)) { diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js index 40cc81edc..dfcd3b460 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -2432,7 +2432,7 @@ describe('Discovery', () => { describe('waitForSelector/waitForTimeout at the time of discovery when Js is enabled =>', () => { it('calls waitForTimeout, waitForSelector and page.eval when their respective arguments are given', async () => { - const page = await percy.browser.page(); + const page = await percy.browser.page({ intercept: { getResource: () => {} } }); spyOn(percy.browser, 'page').and.returnValue(page); spyOn(page, 'eval').and.callThrough(); percy.loglevel('debug'); @@ -2899,16 +2899,17 @@ describe('Discovery', () => { await percy.snapshot({ name: 'test snapshot', url: 'http://localhost:8000', - multiDOM: true, + responsiveSnapshotCapture: true, widths: [365, 1280], domSnapshot: [{ - domSnapshot: testDOM, + html: testDOM, width: 1280 }, { - domSnapshot: { html: DOM1, resources: [capturedResource] }, + html: DOM1, + resources: [capturedResource], width: 370 }, { - domSnapshot: DOM2, + html: DOM2, width: 765 }] }); @@ -2963,13 +2964,13 @@ describe('Discovery', () => { await percy.snapshot({ name: 'test snapshot', url: 'http://localhost:8000', - multiDOM: true, + responsiveSnapshotCapture: true, percyCSS: 'body { color: purple; }', domSnapshot: [{ - domSnapshot: simpleDOM, + html: simpleDOM, width: 1280 }, { - domSnapshot: DOM1, + html: DOM1, width: 370 }] }); diff --git a/packages/core/test/percy.test.js b/packages/core/test/percy.test.js index 6d71bf183..07767c198 100644 --- a/packages/core/test/percy.test.js +++ b/packages/core/test/percy.test.js @@ -75,7 +75,7 @@ describe('Percy', () => { enableJavaScript: false, disableShadowDOM: false, cliEnableJavaScript: true, - multiDOM: false + responsiveSnapshotCapture: false }); });