From 5e3b6a893985bf247173a7d85f4698a7965764f4 Mon Sep 17 00:00:00 2001 From: Paul Irish Date: Thu, 5 Oct 2017 16:45:48 -0700 Subject: [PATCH 1/4] Redirects: formatting --- .../dobetterweb/geolocation-on-start.js | 1 + lighthouse-core/audits/redirects.js | 38 +++++++----- lighthouse-core/test/audits/redirects-test.js | 58 +++++++++++++------ 3 files changed, 65 insertions(+), 32 deletions(-) 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..e6e94cc0e189 100644 --- a/lighthouse-core/audits/redirects.js +++ b/lighthouse-core/audits/redirects.js @@ -32,38 +32,48 @@ 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 = []; - for (let i = 1; i < redirectRequests.length; i++) { - const request = redirectRequests[i - 1]; - const nextRequest = redirectRequests[i]; - const wastedMs = (nextRequest.startTime - request.startTime) * 1000; - // 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; + for (let i = 0; i < redirectRequests.length; i++) { + const initialRequest = redirectRequests[i - 1]; + const redirectedRequest = redirectRequests[i]; + + // Only create a table (with the initial request) if there are > 1 redirects + if (!initialRequest) { + if (redirectRequests.length > 1) { + pageRedirects.push({ + url: `(Initial: ${redirectedRequest.url})`, + wastedMs: 'n/a', + }); + } + continue; } + 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); }); }); From dcb24dff96fbf87d6e4eb32985792519a6142d33 Mon Sep 17 00:00:00 2001 From: Paul Irish Date: Mon, 9 Oct 2017 15:57:58 -0700 Subject: [PATCH 2/4] extract conditional. thx brendan --- lighthouse-core/audits/redirects.js | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/lighthouse-core/audits/redirects.js b/lighthouse-core/audits/redirects.js index e6e94cc0e189..1c2d8340fccd 100644 --- a/lighthouse-core/audits/redirects.js +++ b/lighthouse-core/audits/redirects.js @@ -39,21 +39,18 @@ class Redirects extends Audit { let totalWastedMs = 0; const pageRedirects = []; - for (let i = 0; i < redirectRequests.length; i++) { + // 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 initialRequest = redirectRequests[i - 1]; const redirectedRequest = redirectRequests[i]; - // Only create a table (with the initial request) if there are > 1 redirects - if (!initialRequest) { - if (redirectRequests.length > 1) { - pageRedirects.push({ - url: `(Initial: ${redirectedRequest.url})`, - wastedMs: 'n/a', - }); - } - continue; - } - const wastedMs = (redirectedRequest.startTime - initialRequest.startTime) * 1000; totalWastedMs += wastedMs; From 0b032a9c175e471c239045620923d16b31e334a5 Mon Sep 17 00:00:00 2001 From: Paul Irish Date: Mon, 9 Oct 2017 15:58:03 -0700 Subject: [PATCH 3/4] rebaseline smoke test. --- lighthouse-cli/test/smokehouse/redirects/expectations.js | 6 +++--- 1 file changed, 3 insertions(+), 3 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, }, }, }, From e50ffc43e8cf02a775d1ae2661bed221259967dd Mon Sep 17 00:00:00 2001 From: Paul Irish Date: Tue, 31 Oct 2017 16:31:29 -0700 Subject: [PATCH 4/4] logic <= --- lighthouse-core/audits/redirects.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lighthouse-core/audits/redirects.js b/lighthouse-core/audits/redirects.js index 1c2d8340fccd..0cb94cb5af98 100644 --- a/lighthouse-core/audits/redirects.js +++ b/lighthouse-core/audits/redirects.js @@ -68,7 +68,7 @@ class Redirects extends Audit { return { // We award a passing grade if you only have 1 redirect - score: redirectRequests.length === 2 ? 100 : UnusedBytes.scoreForWastedMs(totalWastedMs), + score: redirectRequests.length <= 2 ? 100 : UnusedBytes.scoreForWastedMs(totalWastedMs), rawValue: totalWastedMs, displayValue: Util.formatMilliseconds(totalWastedMs, 1), extendedInfo: {