From d71b1898148641a4ac0437873d030f08b710b559 Mon Sep 17 00:00:00 2001 From: Paul Irish Date: Tue, 31 Oct 2017 16:31:07 -0700 Subject: [PATCH] report(redirects): reformat results, incl all requests and wasted time, (#3492) --- .../test/smokehouse/redirects/expectations.js | 6 +- .../dobetterweb/geolocation-on-start.js | 1 + lighthouse-core/audits/redirects.js | 35 ++++++----- lighthouse-core/test/audits/redirects-test.js | 58 +++++++++++++------ 4 files changed, 65 insertions(+), 35 deletions(-) diff --git a/lighthouse-cli/test/smokehouse/redirects/expectations.js b/lighthouse-cli/test/smokehouse/redirects/expectations.js index ecf85d98fdc0..9010d61156b4 100644 --- a/lighthouse-cli/test/smokehouse/redirects/expectations.js +++ b/lighthouse-cli/test/smokehouse/redirects/expectations.js @@ -20,7 +20,7 @@ module.exports = [ rawValue: '>=500', details: { items: { - length: 2, + length: 3, }, }, }, @@ -32,10 +32,10 @@ module.exports = [ audits: { 'redirects': { score: 100, - rawValue: 0, + rawValue: '>=250', details: { items: { - length: 1, + length: 2, }, }, }, diff --git a/lighthouse-core/audits/dobetterweb/geolocation-on-start.js b/lighthouse-core/audits/dobetterweb/geolocation-on-start.js index d4501db6ad58..af9f8f5cda0d 100644 --- a/lighthouse-core/audits/dobetterweb/geolocation-on-start.js +++ b/lighthouse-core/audits/dobetterweb/geolocation-on-start.js @@ -34,6 +34,7 @@ class GeolocationOnStart extends ViolationAudit { * @return {!AuditResult} */ static audit(artifacts) { + // 'Only request geolocation information in response to a user gesture.' const results = ViolationAudit.getViolationResults(artifacts, /geolocation/); const headings = [ diff --git a/lighthouse-core/audits/redirects.js b/lighthouse-core/audits/redirects.js index dd9b67dd0bf9..0cb94cb5af98 100644 --- a/lighthouse-core/audits/redirects.js +++ b/lighthouse-core/audits/redirects.js @@ -32,38 +32,45 @@ class Redirects extends Audit { .then(mainResource => { // 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); - let totalWastedMs = 0; + let totalWastedMs = 0; const pageRedirects = []; + + // 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: 'n/a', + }); + } + for (let i = 1; i < redirectRequests.length; i++) { - const request = redirectRequests[i - 1]; - const nextRequest = redirectRequests[i]; - const wastedMs = (nextRequest.startTime - request.startTime) * 1000; + const initialRequest = redirectRequests[i - 1]; + const redirectedRequest = redirectRequests[i]; - // We skip the first redirect in our calculations but show it in the table below. - // We allow 1 redirect (www. => m.) - if (i > 1) { - totalWastedMs += wastedMs; - } + const wastedMs = (redirectedRequest.startTime - initialRequest.startTime) * 1000; + totalWastedMs += wastedMs; pageRedirects.push({ - url: request.url, - wastedMs: Util.formatMilliseconds(wastedMs), + url: redirectedRequest.url, + wastedMs: Util.formatMilliseconds(wastedMs, 1), }); } const headings = [ - {key: 'url', itemType: 'text', text: 'URL'}, + {key: 'url', itemType: 'text', text: 'Redirected URL'}, {key: 'wastedMs', itemType: 'text', text: 'Time for Redirect'}, ]; const details = Audit.makeTableDetails(headings, pageRedirects); return { - score: UnusedBytes.scoreForWastedMs(totalWastedMs), + // We award a passing grade if you only have 1 redirect + score: redirectRequests.length <= 2 ? 100 : UnusedBytes.scoreForWastedMs(totalWastedMs), rawValue: totalWastedMs, - displayValue: Util.formatMilliseconds(totalWastedMs), + displayValue: Util.formatMilliseconds(totalWastedMs, 1), extendedInfo: { value: { wastedMs: totalWastedMs, diff --git a/lighthouse-core/test/audits/redirects-test.js b/lighthouse-core/test/audits/redirects-test.js index bdffe4b28186..0575a59f58fd 100644 --- a/lighthouse-core/test/audits/redirects-test.js +++ b/lighthouse-core/test/audits/redirects-test.js @@ -9,41 +9,54 @@ const Audit = require('../../audits/redirects.js'); const assert = require('assert'); /* eslint-env mocha */ -const FAILING_REDIRECTS = { +const FAILING_THREE_REDIRECTS = { startTime: 17, + url: 'http://exampel.com/', redirects: [ { - endTime: 1, - responseReceivedTime: 5, startTime: 0, url: 'http://example.com/', }, { - endTime: 16, - responseReceivedTime: 14, startTime: 11, url: 'https://example.com/', }, { - endTime: 17, - responseReceivedTime: 15, startTime: 12, url: 'https://m.example.com/', }, ], }; +const FAILING_TWO_REDIRECTS = { + startTime: 446.286, + url: 'http://lisairish.com/', + redirects: [ + { + startTime: 445.648, + url: 'https://lisairish.com/', + }, + { + startTime: 445.757, + url: 'https://www.lisairish.com/', + }, + ], +}; + const SUCCESS_ONE_REDIRECT = { - startTime: 0.7, + startTime: 136.383, + url: 'https://lisairish.com/', redirects: [{ - endTime: 0.7, - responseReceivedTime: 5, - startTime: 0, - url: 'https://example.com/', + startTime: 135.873, + url: 'https://www.lisairish.com/', }], }; -const SUCCESS_NOREDIRECT = {}; +const SUCCESS_NOREDIRECT = { + startTime: 135.873, + url: 'https://www.google.com/', + redirects: [], +}; const mockArtifacts = (mockChain) => { return { @@ -60,19 +73,28 @@ const mockArtifacts = (mockChain) => { }; describe('Performance: Redirects audit', () => { - it('fails when more than one redirect detected', () => { - return Audit.audit(mockArtifacts(FAILING_REDIRECTS)).then(output => { + it('fails when 3 redirects detected', () => { + return Audit.audit(mockArtifacts(FAILING_THREE_REDIRECTS)).then(output => { assert.equal(output.score, 0); + assert.equal(output.details.items.length, 4); + assert.equal(output.rawValue, 17000); + }); + }); + it('fails when 2 redirects detected', () => { + return Audit.audit(mockArtifacts(FAILING_TWO_REDIRECTS)).then(output => { + assert.equal(output.score, 65); assert.equal(output.details.items.length, 3); - assert.equal(output.rawValue, 6000); + assert.equal(Math.round(output.rawValue), 638); }); }); it('passes when one redirect detected', () => { return Audit.audit(mockArtifacts(SUCCESS_ONE_REDIRECT)).then(output => { + // If === 1 redirect, perfect score is expected, regardless of latency assert.equal(output.score, 100); - assert.equal(output.details.items.length, 1); - assert.equal(output.rawValue, 0); + // We will still generate a table and show wasted time + assert.equal(output.details.items.length, 2); + assert.equal(Math.round(output.rawValue), 510); }); });