diff --git a/lighthouse-cli/test/smokehouse/test-definitions/redirects/expectations.js b/lighthouse-cli/test/smokehouse/test-definitions/redirects/expectations.js index e165739c067d..c160bda6f956 100644 --- a/lighthouse-cli/test/smokehouse/test-definitions/redirects/expectations.js +++ b/lighthouse-cli/test/smokehouse/test-definitions/redirects/expectations.js @@ -16,8 +16,18 @@ const expectations = [ requestedUrl: `http://localhost:10200/online-only.html?delay=2000&redirect=%2Fredirects-final.html`, finalUrl: 'http://localhost:10200/redirects-final.html', audits: { + 'first-contentful-paint': { + numericValue: '>=2000', + }, + 'interactive': { + numericValue: '>=2000', + }, + 'speed-index': { + numericValue: '>=2000', + }, 'redirects': { score: 1, + numericValue: '>=2000', details: { items: { length: 2, @@ -36,6 +46,15 @@ const expectations = [ requestedUrl: `http://localhost:10200/online-only.html?delay=1000&redirect_count=3&redirect=%2Fredirects-final.html`, finalUrl: 'http://localhost:10200/redirects-final.html', audits: { + 'first-contentful-paint': { + numericValue: '>=3000', + }, + 'interactive': { + numericValue: '>=3000', + }, + 'speed-index': { + numericValue: '>=3000', + }, 'redirects': { score: '<1', details: { @@ -50,6 +69,38 @@ const expectations = [ ], }, }, + { + // Client-side redirect (2s + 5s), paints at 2s, server-side redirect (1s) + // TODO: Assert performance metrics on client-side redirects, see https://github.com/GoogleChrome/lighthouse/pull/10325 + lhr: { + requestedUrl: `http://localhost:10200/js-redirect.html?delay=2000&jsDelay=5000&jsRedirect=%2Fonline-only.html%3Fdelay%3D1000%26redirect%3D%2Fredirects-final.html`, + finalUrl: 'http://localhost:10200/redirects-final.html', + audits: { + // Just captures the server-side at the moment, should be 8s in the future + 'first-contentful-paint': { + numericValue: '>=1000', + }, + 'interactive': { + numericValue: '>=1000', + }, + 'speed-index': { + numericValue: '>=1000', + }, + 'redirects': { + score: '<1', + numericValue: '>=8000', + details: { + items: { + length: 3, + }, + }, + }, + }, + runWarnings: [ + /The page may not be loading as expected because your test URL \(.*js-redirect.html.*\) was redirected to .*redirects-final.html. Try testing the second URL directly./, + ], + }, + }, { // Client-side redirect (2s + 5s), no paint // TODO: Assert performance metrics on client-side redirects, see https://github.com/GoogleChrome/lighthouse/pull/10325 diff --git a/lighthouse-cli/test/smokehouse/test-definitions/redirects/redirects-config.js b/lighthouse-cli/test/smokehouse/test-definitions/redirects/redirects-config.js index 5c4c475bd89b..1ee850cf27f7 100644 --- a/lighthouse-cli/test/smokehouse/test-definitions/redirects/redirects-config.js +++ b/lighthouse-cli/test/smokehouse/test-definitions/redirects/redirects-config.js @@ -12,7 +12,12 @@ module.exports = { extends: 'lighthouse:default', settings: { onlyAudits: [ + 'first-contentful-paint', + 'interactive', + 'speed-index', 'redirects', ], + // Use provided throttling method to test usage of correct navStart. + throttlingMethod: 'provided', }, }; diff --git a/lighthouse-core/audits/redirects.js b/lighthouse-core/audits/redirects.js index 2ca8b8b61155..5ea85b6c7d52 100644 --- a/lighthouse-core/audits/redirects.js +++ b/lighthouse-core/audits/redirects.js @@ -36,6 +36,47 @@ class Redirects extends Audit { }; } + /** + * This method generates the document request chain including client-side and server-side redirects. + * + * Example: + * GET /initialUrl => 302 /firstRedirect + * GET /firstRedirect => 200 /firstRedirect, window.location = '/secondRedirect' + * GET /secondRedirect => 302 /finalUrl + * GET /finalUrl => 200 /finalUrl + * + * Returns network records [/initialUrl, /firstRedirect, /secondRedirect, /thirdRedirect, /finalUrl] + * + * @param {LH.Artifacts.NetworkRequest} mainResource + * @param {Array} networkRecords + * @param {LH.Artifacts.TraceOfTab} traceOfTab + * @return {Array} + */ + static getDocumentRequestChain(mainResource, networkRecords, traceOfTab) { + /** @type {Array} */ + const documentRequests = []; + + // Find all the document requests by examining navigation events and their redirects + for (const event of traceOfTab.processEvents) { + if (event.name !== 'navigationStart') continue; + + const data = event.args.data || {}; + if (!data.documentLoaderURL || !data.isLoadingMainFrame) continue; + + let networkRecord = networkRecords.find(record => record.url === data.documentLoaderURL); + while (networkRecord) { + documentRequests.push(networkRecord); + networkRecord = networkRecord.redirectDestination; + } + } + + // If we found documents in the trace, just use this directly. + if (documentRequests.length) return documentRequests; + + // Use the main resource as a backup if we didn't find any modern navigationStart events + return (mainResource.redirects || []).concat(mainResource); + } + /** * @param {LH.Artifacts} artifacts * @param {LH.Audit.Context} context @@ -62,26 +103,20 @@ class Redirects extends Audit { } } - // redirects is only available when redirects happens - const redirectRequests = Array.from(mainResource.redirects || []); - - // add main resource to redirectRequests so we can use it to calculate wastedMs - redirectRequests.push(mainResource); + const documentRequests = Redirects.getDocumentRequestChain( + mainResource, networkRecords, traceOfTab); let totalWastedMs = 0; - const pageRedirects = []; + const tableRows = []; - // Kickoff the results table (with the initial request) if there are > 1 redirects - if (redirectRequests.length > 1) { - pageRedirects.push({ - url: `(Initial: ${redirectRequests[0].url})`, - wastedMs: 0, - }); - } + // Iterate through all the document requests and report how much time was wasted until the + // next document request was issued. The final document request will have a `wastedMs` of 0. + for (let i = 0; i < documentRequests.length; i++) { + // If we didn't have enough documents for at least 1 redirect, just skip this loop. + if (documentRequests.length < 2) break; - for (let i = 1; i < redirectRequests.length; i++) { - const initialRequest = redirectRequests[i - 1]; - const redirectedRequest = redirectRequests[i]; + const initialRequest = documentRequests[i]; + const redirectedRequest = documentRequests[i + 1] || initialRequest; const initialTiming = nodeTimingsByUrl.get(initialRequest.url); const redirectedTiming = nodeTimingsByUrl.get(redirectedRequest.url); @@ -89,11 +124,14 @@ class Redirects extends Audit { throw new Error('Could not find redirects in graph'); } - const wastedMs = redirectedTiming.startTime - initialTiming.startTime; + const lanternTimingDeltaMs = redirectedTiming.startTime - initialTiming.startTime; + const observedTimingDeltaS = redirectedRequest.startTime - initialRequest.startTime; + const wastedMs = settings.throttlingMethod === 'simulate' ? + lanternTimingDeltaMs : observedTimingDeltaS * 1000; totalWastedMs += wastedMs; - pageRedirects.push({ - url: redirectedRequest.url, + tableRows.push({ + url: initialRequest.url, wastedMs, }); } @@ -103,11 +141,13 @@ class Redirects extends Audit { {key: 'url', valueType: 'url', label: str_(i18n.UIStrings.columnURL)}, {key: 'wastedMs', valueType: 'timespanMs', label: str_(i18n.UIStrings.columnTimeSpent)}, ]; - const details = Audit.makeOpportunityDetails(headings, pageRedirects, totalWastedMs); + const details = Audit.makeOpportunityDetails(headings, tableRows, totalWastedMs); return { // We award a passing grade if you only have 1 redirect - score: redirectRequests.length <= 2 ? 1 : UnusedBytes.scoreForWastedMs(totalWastedMs), + // TODO(phulce): reconsider if cases like the example in https://github.com/GoogleChrome/lighthouse/issues/8984 + // should fail this audit. + score: documentRequests.length <= 2 ? 1 : UnusedBytes.scoreForWastedMs(totalWastedMs), numericValue: totalWastedMs, numericUnit: 'millisecond', displayValue: totalWastedMs ? diff --git a/lighthouse-core/test/audits/redirects-test.js b/lighthouse-core/test/audits/redirects-test.js index 866e5ed19791..71f345fc65b9 100644 --- a/lighthouse-core/test/audits/redirects-test.js +++ b/lighthouse-core/test/audits/redirects-test.js @@ -20,19 +20,19 @@ const FAILING_THREE_REDIRECTS = [{ timing: {receiveHeadersEnd: 11}, }, { requestId: '1:redirect', - startTime: 11, + startTime: 1, priority: 'VeryHigh', url: 'https://example.com/', timing: {receiveHeadersEnd: 12}, }, { requestId: '1:redirect:redirect', - startTime: 12, + startTime: 2, priority: 'VeryHigh', url: 'https://m.example.com/', timing: {receiveHeadersEnd: 17}, }, { requestId: '1:redirect:redirect:redirect', - startTime: 17, + startTime: 3, priority: 'VeryHigh', url: 'https://m.example.com/final', timing: {receiveHeadersEnd: 19}, @@ -80,6 +80,30 @@ const SUCCESS_NOREDIRECT = [{ timing: {receiveHeadersEnd: 140}, }]; +const FAILING_CLIENTSIDE = [ + { + requestId: '1', + startTime: 445, + priority: 'VeryHigh', + url: 'http://lisairish.com/', + timing: {receiveHeadersEnd: 446}, + }, + { + requestId: '1:redirect', + startTime: 446, + priority: 'VeryHigh', + url: 'https://lisairish.com/', + timing: {receiveHeadersEnd: 447}, + }, + { + requestId: '2', + startTime: 447, + priority: 'VeryHigh', + url: 'https://www.lisairish.com/', + timing: {receiveHeadersEnd: 448}, + }, +]; + describe('Performance: Redirects audit', () => { const mockArtifacts = (networkRecords, finalUrl) => { const devtoolsLog = networkRecordsToDevtoolsLog(networkRecords); @@ -91,13 +115,61 @@ describe('Performance: Redirects audit', () => { }; }; + it('fails when client-side redirects detected', async () => { + const context = {settings: {}, computedCache: new Map()}; + const artifacts = mockArtifacts(FAILING_CLIENTSIDE, 'https://www.lisairish.com/'); + + const traceEvents = artifacts.traces.defaultPass.traceEvents; + const navStart = traceEvents.find(e => e.name === 'navigationStart'); + const secondNavStart = JSON.parse(JSON.stringify(navStart)); + traceEvents.push(secondNavStart); + navStart.args.data.isLoadingMainFrame = true; + navStart.args.data.documentLoaderURL = 'http://lisairish.com/'; + secondNavStart.ts++; + secondNavStart.args.data.isLoadingMainFrame = true; + secondNavStart.args.data.documentLoaderURL = 'https://www.lisairish.com/'; + + const output = await RedirectsAudit.audit(artifacts, context); + expect(output.details.items).toHaveLength(3); + expect(Math.round(output.score * 100) / 100).toMatchInlineSnapshot(`0.35`); + expect(output.numericValue).toMatchInlineSnapshot(`2000`); + }); + + it('uses lantern timings when throttlingMethod is simulate', async () => { + const artifacts = mockArtifacts(FAILING_THREE_REDIRECTS, 'https://m.example.com/final'); + const context = {settings: {throttlingMethod: 'simulate'}, computedCache: new Map()}; + const output = await RedirectsAudit.audit(artifacts, context); + expect(output.details.items).toHaveLength(4); + expect(output.details.items.map(item => [item.url, item.wastedMs])).toMatchInlineSnapshot(` + Array [ + Array [ + "http://example.com/", + 630, + ], + Array [ + "https://example.com/", + 480, + ], + Array [ + "https://m.example.com/", + 780, + ], + Array [ + "https://m.example.com/final", + 0, + ], + ] + `); + expect(output.numericValue).toMatchInlineSnapshot(`1890`); + }); + it('fails when 3 redirects detected', () => { const artifacts = mockArtifacts(FAILING_THREE_REDIRECTS, 'https://m.example.com/final'); const context = {settings: {}, computedCache: new Map()}; return RedirectsAudit.audit(artifacts, context).then(output => { - assert.equal(Math.round(output.score * 100) / 100, 0.37); - assert.equal(output.details.items.length, 4); - assert.equal(output.numericValue, 1890); + expect(output.details.items).toHaveLength(4); + expect(Math.round(output.score * 100) / 100).toMatchInlineSnapshot(`0.24`); + expect(output.numericValue).toMatchInlineSnapshot(`3000`); }); }); @@ -105,9 +177,9 @@ describe('Performance: Redirects audit', () => { const artifacts = mockArtifacts(FAILING_TWO_REDIRECTS, 'https://www.lisairish.com/'); const context = {settings: {}, computedCache: new Map()}; return RedirectsAudit.audit(artifacts, context).then(output => { - assert.equal(Math.round(output.score * 100) / 100, 0.46); - assert.equal(output.details.items.length, 3); - assert.equal(Math.round(output.numericValue), 1110); + expect(output.details.items).toHaveLength(3); + expect(Math.round(output.score * 100) / 100).toMatchInlineSnapshot(`0.35`); + expect(output.numericValue).toMatchInlineSnapshot(`2000`); }); }); @@ -116,10 +188,10 @@ describe('Performance: Redirects audit', () => { const context = {settings: {}, computedCache: new Map()}; return RedirectsAudit.audit(artifacts, context).then(output => { // If === 1 redirect, perfect score is expected, regardless of latency - assert.equal(output.score, 1); // We will still generate a table and show wasted time - assert.equal(output.details.items.length, 2); - assert.equal(Math.round(output.numericValue), 780); + expect(output.details.items).toHaveLength(2); + expect(output.score).toEqual(1); + expect(output.numericValue).toMatchInlineSnapshot(`1000`); }); });