report(redirects): reformat results, incl all requests and wasted tim…

…e, (#3492)

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,
           },
         },
       },

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 = [

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,

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);
     });
   });