From e76a7272755dcf1b62c4226eb4906cd63ade7e95 Mon Sep 17 00:00:00 2001 From: Adam Raine Date: Mon, 17 Apr 2023 13:36:40 -0700 Subject: [PATCH] feat: use `finalDisplayedUrl` --- packages/cli/src/assert/assert.js | 7 ++- packages/cli/src/collect/node-runner.js | 2 +- packages/cli/src/open/open.js | 11 +++-- packages/cli/src/upload/upload.js | 43 +++++++++++++++---- .../cli/test/fixtures/psi/mock-psi-server.js | 2 +- packages/cli/test/upload-url-hash.test.js | 2 +- packages/cli/test/upload.test.js | 4 +- packages/server/test/server-test-suite.js | 4 +- packages/utils/src/assertions.js | 8 ++-- .../utils/src/seed-data/dataset-generator.js | 2 +- packages/utils/src/seed-data/lhr-generator.js | 1 + packages/utils/test/assertions.test.js | 20 ++++----- packages/utils/test/psi-client.test.js | 4 +- .../test/seed-data/lhr-generator.test.js | 2 +- .../src/ui/components/report-upload-box.jsx | 6 +-- types/lighthouse.d.ts | 1 + types/upload.d.ts | 2 +- 17 files changed, 80 insertions(+), 41 deletions(-) diff --git a/packages/cli/src/assert/assert.js b/packages/cli/src/assert/assert.js index e9bedabd6..ae93bab8f 100644 --- a/packages/cli/src/assert/assert.js +++ b/packages/cli/src/assert/assert.js @@ -54,7 +54,12 @@ async function runCommand(options) { if (budgetsFile) options = await convertBudgetsToAssertions(readBudgets(budgetsFile)); const lhrs = loadSavedLHRs().map(json => JSON.parse(json)); - const uniqueUrls = new Set(lhrs.map(lhr => lhr.finalUrl)); + + // eslint-disable-next-line prettier/prettier + const {upgradeLhrForCompatibility} = await import('lighthouse/core/lib/lighthouse-compatibility.js'); + lhrs.forEach(upgradeLhrForCompatibility); + + const uniqueUrls = new Set(lhrs.map(lhr => lhr.finalDisplayedUrl)); const allResults = getAllAssertionResults(options, lhrs); const groupedResults = _.groupBy(allResults, result => result.url); diff --git a/packages/cli/src/collect/node-runner.js b/packages/cli/src/collect/node-runner.js index 3f1047e7c..d0ffa1b33 100644 --- a/packages/cli/src/collect/node-runner.js +++ b/packages/cli/src/collect/node-runner.js @@ -31,7 +31,7 @@ class LighthouseRunner { /** @type {LH.Result} */ const lhr = JSON.parse(process.env.LHCITEST_MOCK_LHR || '{}'); lhr.requestedUrl = url; - lhr.finalUrl = url; + lhr.finalDisplayedUrl = url; lhr.configSettings = lhr.configSettings || options.settings || {}; return Promise.resolve(JSON.stringify(lhr)); } diff --git a/packages/cli/src/open/open.js b/packages/cli/src/open/open.js index cba5df5ac..ae1dfba22 100644 --- a/packages/cli/src/open/open.js +++ b/packages/cli/src/open/open.js @@ -28,8 +28,13 @@ function buildCommand(yargs) { async function runCommand(options) { /** @type {Array} */ const lhrs = loadSavedLHRs().map(lhr => JSON.parse(lhr)); + + // eslint-disable-next-line prettier/prettier + const {upgradeLhrForCompatibility} = await import('lighthouse/core/lib/lighthouse-compatibility.js'); + lhrs.forEach(upgradeLhrForCompatibility); + /** @type {Array>} */ - const groupedByUrl = _.groupBy(lhrs, lhr => lhr.finalUrl).map(lhrs => + const groupedByUrl = _.groupBy(lhrs, lhr => lhr.finalDisplayedUrl).map(lhrs => lhrs.map(lhr => [lhr, lhr]) ); const representativeLhrs = computeRepresentativeRuns(groupedByUrl); @@ -41,9 +46,9 @@ async function runCommand(options) { const targetUrls = typeof options.url === 'string' ? [options.url] : options.url || []; for (const lhr of representativeLhrs) { - if (targetUrls.length && !targetUrls.includes(lhr.finalUrl)) continue; + if (targetUrls.length && !targetUrls.includes(lhr.finalDisplayedUrl)) continue; - process.stdout.write(`Opening median report for ${lhr.finalUrl}...\n`); + process.stdout.write(`Opening median report for ${lhr.finalDisplayedUrl}...\n`); const tmpFile = tmp.fileSync({postfix: '.html'}); fs.writeFileSync(tmpFile.name, await getHTMLReportForLHR(lhr)); await open(tmpFile.name); diff --git a/packages/cli/src/upload/upload.js b/packages/cli/src/upload/upload.js index f969a4ba6..ed3862761 100644 --- a/packages/cli/src/upload/upload.js +++ b/packages/cli/src/upload/upload.js @@ -273,14 +273,21 @@ async function runGithubStatusCheck(options, targetUrlMap) { } else { /** @type {Array} */ const lhrs = loadSavedLHRs().map(lhr => JSON.parse(lhr)); + + // eslint-disable-next-line prettier/prettier + const {upgradeLhrForCompatibility} = await import('lighthouse/core/lib/lighthouse-compatibility.js'); + lhrs.forEach(upgradeLhrForCompatibility); + /** @type {Array>} */ - const lhrsByUrl = _.groupBy(lhrs, lhr => lhr.finalUrl).map(lhrs => lhrs.map(lhr => [lhr, lhr])); + const lhrsByUrl = _.groupBy(lhrs, lhr => lhr.finalDisplayedUrl).map(lhrs => + lhrs.map(lhr => [lhr, lhr]) + ); const representativeLhrs = computeRepresentativeRuns(lhrsByUrl); if (!representativeLhrs.length) return print('No LHRs for status check, skipping.\n'); for (const lhr of representativeLhrs) { - const rawUrl = lhr.finalUrl; + const rawUrl = lhr.finalDisplayedUrl; const urlLabel = getUrlLabelForGithub(rawUrl, options); const state = 'success'; const context = getGitHubContext(urlLabel, options); @@ -434,7 +441,12 @@ async function runLHCITarget(options) { for (const lhr of lhrs) { const parsedLHR = JSON.parse(lhr); - const url = getUrlForLhciTarget(parsedLHR.finalUrl, options); + + // eslint-disable-next-line prettier/prettier + const {upgradeLhrForCompatibility} = await import('lighthouse/core/lib/lighthouse-compatibility.js'); + upgradeLhrForCompatibility(parsedLHR); + + const url = getUrlForLhciTarget(parsedLHR.finalDisplayedUrl, options); const run = await api.createRun({ projectId: project.id, buildId: build.id, @@ -444,7 +456,7 @@ async function runLHCITarget(options) { }); buildViewUrl.searchParams.set('compareUrl', url); - targetUrlMap.set(parsedLHR.finalUrl, buildViewUrl.href); + targetUrlMap.set(parsedLHR.finalDisplayedUrl, buildViewUrl.href); print(`Saved LHR to ${options.serverBaseUrl} (${run.id})\n`); } @@ -464,15 +476,22 @@ async function runLHCITarget(options) { async function runTemporaryPublicStorageTarget(options) { /** @type {Array} */ const lhrs = loadSavedLHRs().map(lhr => JSON.parse(lhr)); + + // eslint-disable-next-line prettier/prettier + const {upgradeLhrForCompatibility} = await import('lighthouse/core/lib/lighthouse-compatibility.js'); + lhrs.forEach(upgradeLhrForCompatibility); + /** @type {Array>} */ - const lhrsByUrl = _.groupBy(lhrs, lhr => lhr.finalUrl).map(lhrs => lhrs.map(lhr => [lhr, lhr])); + const lhrsByUrl = _.groupBy(lhrs, lhr => lhr.finalDisplayedUrl).map(lhrs => + lhrs.map(lhr => [lhr, lhr]) + ); const representativeLhrs = computeRepresentativeRuns(lhrsByUrl); const targetUrlMap = new Map(); const previousUrlMap = await getPreviousUrlMap(options); for (const lhr of representativeLhrs) { - print(`Uploading median LHR of ${lhr.finalUrl}...`); + print(`Uploading median LHR of ${lhr.finalDisplayedUrl}...`); try { const response = await fetch(TEMPORARY_PUBLIC_STORAGE_URL, { @@ -483,10 +502,13 @@ async function runTemporaryPublicStorageTarget(options) { const {success, url} = await response.json(); if (success && url) { - const urlReplaced = replaceUrlPatterns(lhr.finalUrl, options.urlReplacementPatterns); + const urlReplaced = replaceUrlPatterns( + lhr.finalDisplayedUrl, + options.urlReplacementPatterns + ); const urlToLinkTo = buildTemporaryStorageLink(url, urlReplaced, previousUrlMap); print(`success!\nOpen the report at ${urlToLinkTo}\n`); - targetUrlMap.set(lhr.finalUrl, url); + targetUrlMap.set(lhr.finalDisplayedUrl, url); } else { print(`failed!\n`); } @@ -526,6 +548,11 @@ function getFileOutputPath(pattern, context) { async function runFilesystemTarget(options) { /** @type {Array} */ const lhrs = loadSavedLHRs().map(lhr => JSON.parse(lhr)); + + // eslint-disable-next-line prettier/prettier + const {upgradeLhrForCompatibility} = await import('lighthouse/core/lib/lighthouse-compatibility.js'); + lhrs.forEach(upgradeLhrForCompatibility); + /** @type {Array>} */ const lhrsByUrl = _.groupBy(lhrs, lhr => lhr.requestedUrl).map(lhrs => lhrs.map(lhr => [lhr, lhr]) diff --git a/packages/cli/test/fixtures/psi/mock-psi-server.js b/packages/cli/test/fixtures/psi/mock-psi-server.js index eaf7f8ac7..58d9e9cfd 100644 --- a/packages/cli/test/fixtures/psi/mock-psi-server.js +++ b/packages/cli/test/fixtures/psi/mock-psi-server.js @@ -26,7 +26,7 @@ async function createApp() { const url = req.query.url; const lighthouseResult = JSON.parse(JSON.stringify(LHR)); - lighthouseResult.finalUrl = url; + lighthouseResult.finalDisplayedUrl = url; lighthouseResult.initialUrl = url; setTimeout(() => res.json({lighthouseResult}), 2000); }); diff --git a/packages/cli/test/upload-url-hash.test.js b/packages/cli/test/upload-url-hash.test.js index 03eca79c0..6d4b6bac8 100644 --- a/packages/cli/test/upload-url-hash.test.js +++ b/packages/cli/test/upload-url-hash.test.js @@ -19,7 +19,7 @@ describe('Lighthouse CI upload filesystem reports with url hash', () => { const fakeLhrPath = path.join(lighthouseciDir, 'lhr-12345.json'); const writeLhr = () => { - const fakeLhr = {finalUrl: 'foo.com', categories: {}, audits: {}}; + const fakeLhr = {finalDisplayedUrl: 'foo.com', categories: {}, audits: {}}; fakeLhr.categories.pwa = {score: 0}; fakeLhr.categories.performance = {score: 0}; fakeLhr.audits['performance-budget'] = {score: 0}; diff --git a/packages/cli/test/upload.test.js b/packages/cli/test/upload.test.js index 224d50e57..ac70b6887 100644 --- a/packages/cli/test/upload.test.js +++ b/packages/cli/test/upload.test.js @@ -20,7 +20,7 @@ describe('Lighthouse CI upload CLI', () => { const fakeLhrPath = path.join(lighthouseciDir, 'lhr-12345.json'); const writeLhr = () => { - const fakeLhr = {finalUrl: 'foo.com', categories: {}, audits: {}}; + const fakeLhr = {finalDisplayedUrl: 'foo.com', categories: {}, audits: {}}; fakeLhr.categories.pwa = {score: 0}; fakeLhr.categories.performance = {score: 0}; fakeLhr.audits['performance-budget'] = {score: 0}; @@ -125,7 +125,7 @@ describe('Lighthouse CI upload CLI', () => { it('should upload for a build with very long URL', async () => { const lhr = JSON.parse(fs.readFileSync(fakeLhrPath, 'utf8')); - lhr.finalUrl = `http://localhost/reall${'l'.repeat(256)}y-long-url`; + lhr.finalDisplayedUrl = `http://localhost/reall${'l'.repeat(256)}y-long-url`; fs.writeFileSync(fakeLhrPath, JSON.stringify(lhr)); expect(await apiClient.getBuilds(project.id)).toHaveLength(1); diff --git a/packages/server/test/server-test-suite.js b/packages/server/test/server-test-suite.js index 76d3727a4..beb2888b5 100644 --- a/packages/server/test/server-test-suite.js +++ b/packages/server/test/server-test-suite.js @@ -596,7 +596,7 @@ function runTests(state) { describe('/:projectId/builds/:buildId/runs', () => { const lhr = { lighthouseVersion: '4.1.0', - finalUrl: 'https://example.com/', + finalDisplayedUrl: 'https://example.com/', audits: { interactive: {numericValue: 5000}, 'speed-index': {numericValue: 5000}, @@ -697,7 +697,7 @@ function runTests(state) { buildId: buildA.id, url: 'https://example.com:PORT/blog', lhr: JSON.stringify({ - finalUrl: 'https://example.com/blog', + finalDisplayedUrl: 'https://example.com/blog', lighthouseVersion: '4.2.0', audits: { interactive: {numericValue: 1000}, diff --git a/packages/utils/src/assertions.js b/packages/utils/src/assertions.js index 2a7463eec..28519aca4 100644 --- a/packages/utils/src/assertions.js +++ b/packages/utils/src/assertions.js @@ -284,7 +284,7 @@ function getCategoryAssertionResults(auditProperty, assertionOptions, lhrs) { * @return {boolean} */ function doesLHRMatchPattern(pattern, lhr) { - return new RegExp(pattern).test(lhr.finalUrl); + return new RegExp(pattern).test(lhr.finalDisplayedUrl); } /** @@ -364,7 +364,7 @@ function resolveAssertionOptionsAndLhrs(baseOptions, unfilteredLhrs) { : unfilteredLhrs; // Double-check we've only got one URL to look at that should have been pre-grouped in `getAllAssertionResults`. - const uniqueURLs = new Set(lhrs.map(lhr => lhr.finalUrl)); + const uniqueURLs = new Set(lhrs.map(lhr => lhr.finalDisplayedUrl)); if (uniqueURLs.size > 1) throw new Error('Can only assert one URL at a time!'); const medianLhrs = computeRepresentativeRuns([ @@ -399,7 +399,7 @@ function resolveAssertionOptionsAndLhrs(baseOptions, unfilteredLhrs) { medianLhrs, aggregationMethod, lhrs: lhrs, - url: (lhrs[0] && lhrs[0].finalUrl) || '', + url: (lhrs[0] && lhrs[0].finalDisplayedUrl) || '', }; } @@ -456,7 +456,7 @@ function getAllAssertionResultsForUrl(baseOptions, unfilteredLhrs) { * @return {LHCI.AssertResults.AssertionResult[]} */ function getAllAssertionResults(options, lhrs) { - const groupedByURL = _.groupBy(lhrs, lhr => lhr.finalUrl); + const groupedByURL = _.groupBy(lhrs, lhr => lhr.finalDisplayedUrl); /** @type {LHCI.AssertCommand.BaseOptions[]} */ let arrayOfOptions = [options]; diff --git a/packages/utils/src/seed-data/dataset-generator.js b/packages/utils/src/seed-data/dataset-generator.js index 603005183..7356ddd55 100644 --- a/packages/utils/src/seed-data/dataset-generator.js +++ b/packages/utils/src/seed-data/dataset-generator.js @@ -537,7 +537,7 @@ function createLoadTestDataset() { /** @type {LH.Result} */ const lhr = JSON.parse(sourceLhr); lhr.requestedUrl = url; - lhr.finalUrl = url; + lhr.finalDisplayedUrl = url; for (const auditId of Object.keys(lhr.audits)) { const multiplier = 1 + Math.random() * 0.4 - 0.2; const audit = lhr.audits[auditId]; diff --git a/packages/utils/src/seed-data/lhr-generator.js b/packages/utils/src/seed-data/lhr-generator.js index 3efe1a161..dfcec3368 100644 --- a/packages/utils/src/seed-data/lhr-generator.js +++ b/packages/utils/src/seed-data/lhr-generator.js @@ -186,6 +186,7 @@ function createLHR(pageUrl, auditDefs, prandom) { return { requestedUrl: pageUrl, finalUrl: pageUrl, + finalDisplayedUrl: pageUrl, categories, audits, diff --git a/packages/utils/test/assertions.test.js b/packages/utils/test/assertions.test.js index 72917f701..fc4a3ca21 100644 --- a/packages/utils/test/assertions.test.js +++ b/packages/utils/test/assertions.test.js @@ -17,7 +17,7 @@ describe('getAllAssertionResults', () => { beforeEach(() => { lhrs = [ { - finalUrl: 'http://page-1.com', + finalDisplayedUrl: 'http://page-1.com', categories: {pwa: {score: 0.5}, perf: {score: 0.1}}, audits: { 'first-contentful-paint': { @@ -36,7 +36,7 @@ describe('getAllAssertionResults', () => { }, }, { - finalUrl: 'http://page-1.com', + finalDisplayedUrl: 'http://page-1.com', categories: {pwa: {score: 0.8}, perf: {score: 0.1}}, audits: { 'first-contentful-paint': { @@ -402,7 +402,7 @@ describe('getAllAssertionResults', () => { const lhrs = [ // This is the "median-run" by FCP and interactive. { - finalUrl: 'http://example.com', + finalDisplayedUrl: 'http://example.com', audits: { 'first-contentful-paint': {numericValue: 5000}, interactive: {numericValue: 10000}, @@ -410,7 +410,7 @@ describe('getAllAssertionResults', () => { }, }, { - finalUrl: 'http://example.com', + finalDisplayedUrl: 'http://example.com', audits: { 'first-contentful-paint': {numericValue: 1000}, interactive: {numericValue: 5000}, @@ -418,7 +418,7 @@ describe('getAllAssertionResults', () => { }, }, { - finalUrl: 'http://example.com', + finalDisplayedUrl: 'http://example.com', audits: { 'first-contentful-paint': {numericValue: 10000}, interactive: {numericValue: 15000}, @@ -567,7 +567,7 @@ describe('getAllAssertionResults', () => { beforeEach(() => { lhrWithBudget = { - finalUrl: 'http://page-1.com', + finalDisplayedUrl: 'http://page-1.com', audits: { 'performance-budget': { id: 'performance-budget', @@ -598,7 +598,7 @@ describe('getAllAssertionResults', () => { }; lhrWithResourceSummary = { - finalUrl: 'http://example.com', + finalDisplayedUrl: 'http://example.com', audits: { 'resource-summary': { details: { @@ -897,7 +897,7 @@ describe('getAllAssertionResults', () => { beforeEach(() => { lhrWithUserTimings = { - finalUrl: 'http://example.com', + finalDisplayedUrl: 'http://example.com', audits: { 'user-timings': { details: { @@ -994,7 +994,7 @@ describe('getAllAssertionResults', () => { describe('URL-grouping', () => { beforeEach(() => { for (const lhr of [...lhrs]) { - lhrs.push({...lhr, finalUrl: 'http://page-2.com'}); + lhrs.push({...lhr, finalDisplayedUrl: 'http://page-2.com'}); } }); @@ -1059,7 +1059,7 @@ describe('getAllAssertionResults', () => { describe('assertMatrix', () => { beforeEach(() => { for (const lhr of [...lhrs]) { - lhrs.push({...lhr, finalUrl: 'http://page-2.com'}); + lhrs.push({...lhr, finalDisplayedUrl: 'http://page-2.com'}); } }); diff --git a/packages/utils/test/psi-client.test.js b/packages/utils/test/psi-client.test.js index 4a04d119c..cc3a15647 100644 --- a/packages/utils/test/psi-client.test.js +++ b/packages/utils/test/psi-client.test.js @@ -15,7 +15,7 @@ describe('PSI API Client', () => { const apiKey = 'the-key'; it('should request the url', async () => { - const lighthouseResult = {finalUrl: 'https://example.com/'}; + const lighthouseResult = {finalDisplayedUrl: 'https://example.com/'}; fetchJsonMock = jest.fn().mockResolvedValue({lighthouseResult}); const fetchMock = jest.fn().mockImplementation(fetchMockImpl); const client = new PsiClient({apiKey, fetch: fetchMock}); @@ -39,7 +39,7 @@ describe('PSI API Client', () => { }); it('should respect provided options', async () => { - const lighthouseResult = {finalUrl: 'https://example.com/'}; + const lighthouseResult = {finalDisplayedUrl: 'https://example.com/'}; fetchJsonMock = jest.fn().mockResolvedValue({lighthouseResult}); const fetchMock = jest.fn().mockImplementation(fetchMockImpl); const runOptions = {strategy: 'desktop', categories: ['accessibility', 'pwa', 'seo']}; diff --git a/packages/utils/test/seed-data/lhr-generator.test.js b/packages/utils/test/seed-data/lhr-generator.test.js index a085f1c75..ab58d5039 100644 --- a/packages/utils/test/seed-data/lhr-generator.test.js +++ b/packages/utils/test/seed-data/lhr-generator.test.js @@ -17,7 +17,7 @@ describe('createLHR', () => { expect(lhr).toMatchObject({ requestedUrl: 'http://example.com', - finalUrl: 'http://example.com', + finalDisplayedUrl: 'http://example.com', audits: { 'seo-audit': { title: 'Seo Audit', diff --git a/packages/viewer/src/ui/components/report-upload-box.jsx b/packages/viewer/src/ui/components/report-upload-box.jsx index d9337a39e..87575b4ae 100644 --- a/packages/viewer/src/ui/components/report-upload-box.jsx +++ b/packages/viewer/src/ui/components/report-upload-box.jsx @@ -38,8 +38,8 @@ export function parseStringAsLhr(s) { /** @param {LH.Result} lhrA @param {LH.Result} lhrB @return {DisplayType} */ export function computeBestDisplayType(lhrA, lhrB) { - const urlA = new URL(lhrA.finalUrl); - const urlB = new URL(lhrB.finalUrl); + const urlA = new URL(lhrA.finalDisplayedUrl); + const urlB = new URL(lhrB.finalDisplayedUrl); if (urlA.hostname !== urlB.hostname) return 'hostname'; if (urlA.pathname !== urlB.pathname) return 'pathname'; if (urlA.search !== urlB.search) return 'path'; @@ -50,7 +50,7 @@ export function computeBestDisplayType(lhrA, lhrB) { /** @param {{report: ReportData, displayType: DisplayType}} props */ const FilePill = props => { const {filename, lhr} = props.report; - const url = new URL(lhr.finalUrl); + const url = new URL(lhr.finalDisplayedUrl); const timestamp = new Date(lhr.fetchTime).toLocaleString(); const options = { filename, diff --git a/types/lighthouse.d.ts b/types/lighthouse.d.ts index 7453ee536..faa990f76 100644 --- a/types/lighthouse.d.ts +++ b/types/lighthouse.d.ts @@ -69,6 +69,7 @@ declare global { export interface Result { requestedUrl: string; finalUrl: string; + finalDisplayedUrl: string; fetchTime: string; lighthouseVersion: string; categories: {[categoryId: string]: CategoryResult}; diff --git a/types/upload.d.ts b/types/upload.d.ts index c5936a62b..dbd9874fc 100644 --- a/types/upload.d.ts +++ b/types/upload.d.ts @@ -40,7 +40,7 @@ declare global { } export interface ManifestEntry { - url: string; // finalUrl of the run + url: string; // finalUrlDisplayed of the run isRepresentativeRun: boolean; // whether it was the median run for the URL jsonPath: string; htmlPath: string;