core(load-fast-4-pwa): use computed artifacts (#4981)

patrickhulce · web-flow · commit 9771117d08f0 · 2018-04-18T14:00:13.000-07:00
diff --git a/lighthouse-core/audits/load-fast-enough-for-pwa.js b/lighthouse-core/audits/load-fast-enough-for-pwa.js
@@ -11,17 +11,14 @@
  *  Afterwards, we report if the TTFI is less than 10 seconds.
  */
 
+const isDeepEqual = require('lodash.isequal');
 const Audit = require('./audit');
-const URL = require('../lib/url-shim');
-const targetLatencyMs = require('../config/constants').throttling.mobile3G.rttMs;
-const Sentry = require('../lib/sentry');
+const mobile3GThrottling = require('../config/constants').throttling.mobile3G;
 const Util = require('../report/v2/renderer/util.js');
 
-// Maximum TTFI to be considered "fast" for PWA baseline checklist
+// Maximum TTI to be considered "fast" for PWA baseline checklist
 //   https://developers.google.com/web/progressive-web-apps/checklist
-const MAXIMUM_TTFI = 10 * 1000;
-
-const WHITELISTED_STATUS_CODES = [307];
+const MAXIMUM_TTI = 10 * 1000;
 
 class LoadFastEnough4Pwa extends Audit {
   /**
@@ -32,116 +29,47 @@ class LoadFastEnough4Pwa extends Audit {
       name: 'load-fast-enough-for-pwa',
       description: 'Page load is fast enough on 3G',
       failureDescription: 'Page load is not fast enough on 3G',
-      helpText: 'A fast page load over a 3G network ensures a good mobile user experience. ' +
-          '[Learn more](https://developers.google.com/web/tools/lighthouse/audits/fast-3g).',
+      helpText:
+        'A fast page load over a 3G network ensures a good mobile user experience. ' +
+        '[Learn more](https://developers.google.com/web/tools/lighthouse/audits/fast-3g).',
       requiredArtifacts: ['traces', 'devtoolsLogs'],
     };
   }
 
   /**
-   * @param {!Artifacts} artifacts
-   * @return {!AuditResult}
+   * @param {LH.Artifacts} artifacts
+   * @param {LH.Audit.Context} context
+   * @return {LH.AuditResult}
    */
-  static audit(artifacts) {
-    const devtoolsLogs = artifacts.devtoolsLogs[Audit.DEFAULT_PASS];
-    return artifacts.requestNetworkRecords(devtoolsLogs).then(networkRecords => {
-      const firstRequestLatenciesByOrigin = new Map();
-      networkRecords.forEach(record => {
-        // Ignore requests that don't have valid origin, timing data, came from the cache, were
-        // redirected by Chrome without going to the network, or are not finished.
-        const fromCache = record._fromDiskCache || record._fromMemoryCache;
-        const origin = URL.getOrigin(record._url);
-        if (!origin || !record._timing || fromCache ||
-            WHITELISTED_STATUS_CODES.includes(record.statusCode) || !record.finished) {
-          return;
-        }
-
-        // Disregard requests with an invalid start time, (H2 request start times are sometimes less
-        // than issue time and even negative which throws off timing)
-        if (record._startTime < record._issueTime) {
-          return;
-        }
-
-        // Use DevTools' definition of Waiting latency: https://github.com/ChromeDevTools/devtools-frontend/blob/66595b8a73a9c873ea7714205b828866630e9e82/front_end/network/RequestTimingView.js#L164
-        const latency = record._timing.receiveHeadersEnd - record._timing.sendEnd;
-        const latencyInfo = {
-          url: record._url,
-          startTime: record._startTime,
-          origin,
-          latency,
-        };
-
-        // Only examine the first request per origin to reduce noisiness from cases like H2 push
-        // where individual request latency may not apply.
-        const existing = firstRequestLatenciesByOrigin.get(origin);
-        if (!existing || latencyInfo.startTime < existing.startTime) {
-          firstRequestLatenciesByOrigin.set(origin, latencyInfo);
-        }
-      });
-
-      let firstRequestLatencies = Array.from(firstRequestLatenciesByOrigin.values());
-      const latency3gMin = targetLatencyMs - 10;
-      const areLatenciesAll3G = firstRequestLatencies.every(val => val.latency > latency3gMin);
-      firstRequestLatencies = firstRequestLatencies.map(item => ({
-        url: item.url,
-        latency: Util.formatNumber(item.latency, 0.01),
-      }));
-
-      const trace = artifacts.traces[Audit.DEFAULT_PASS];
-      return artifacts.requestFirstInteractive(trace).then(firstInteractive => {
-        const timeToFirstInteractive = firstInteractive.timeInMs;
-        const isFast = timeToFirstInteractive < MAXIMUM_TTFI;
-
-        const extendedInfo = {
-          value: {areLatenciesAll3G, firstRequestLatencies, isFast, timeToFirstInteractive},
-        };
+  static async audit(artifacts, context) {
+    const trace = artifacts.traces[Audit.DEFAULT_PASS];
+    const devtoolsLog = artifacts.devtoolsLogs[Audit.DEFAULT_PASS];
+
+    // If throttling was default devtools or lantern 3G throttling, then reuse the given settings
+    // Otherwise, we'll force the usage of lantern 3G.
+    const settingOverrides = {throttlingMethod: 'simulate', throttling: mobile3GThrottling};
+    const settings =
+      context.settings.throttlingMethod !== 'provided' &&
+      isDeepEqual(context.settings.throttling, mobile3GThrottling)
+        ? context.settings
+        : Object.assign({}, context.settings, settingOverrides);
+
+    const metricComputationData = {trace, devtoolsLog, settings};
+    const tti = await artifacts.requestConsistentlyInteractive(metricComputationData);
+
+    const score = Number(tti.timing < MAXIMUM_TTI);
+
+    let debugString;
+    if (!score) {
+      // eslint-disable-next-line max-len
+      debugString = `First Interactive was ${Util.formatMilliseconds(tti.timing)}. More details in the "Performance" section.`;
+    }
 
-        const details = Audit.makeTableDetails([
-          {key: 'url', itemType: 'url', text: 'URL'},
-          {key: 'latency', itemType: 'text', text: 'Latency (ms)'},
-        ], firstRequestLatencies);
-
-        if (!isFast) {
-          return {
-            rawValue: false,
-            // eslint-disable-next-line max-len
-            debugString: `First Interactive was at ${Util.formatMilliseconds(timeToFirstInteractive)}. More details in the "Performance" section.`,
-            extendedInfo,
-          };
-        }
-
-        if (!areLatenciesAll3G) {
-          const sentryContext = Sentry.getContext();
-          const hadThrottlingEnabled = sentryContext && sentryContext.extra &&
-              sentryContext.extra.networkThrottling;
-
-          if (hadThrottlingEnabled) {
-            // Track these instances in Sentry since there should be no requests that are fast when
-            // throttling is enabled, and it's likely a throttling bug we should look into.
-            const violatingLatency = firstRequestLatencies
-                .find(item => Number(item.latency) < latency3gMin);
-            Sentry.captureMessage('Network request latencies were not realistic', {
-              tags: {audit: this.meta.name},
-              extra: {violatingLatency},
-              level: 'warning',
-            });
-          }
-
-          return {
-            rawValue: true,
-            // eslint-disable-next-line max-len
-            debugString: `First Interactive was found at ${Util.formatMilliseconds(timeToFirstInteractive)}; however, the network request latencies were not sufficiently realistic, so the performance measurements cannot be trusted.`,
-            extendedInfo,
-            details,
-          };
-        }
-
-        return {
-          rawValue: true,
-          extendedInfo,
-        };
-      });
-    });
+    return {
+      score,
+      debugString,
+      rawValue: tti.timing,
+    };
   }
 }
 
diff --git a/lighthouse-core/gather/computed/metrics/metric.js b/lighthouse-core/gather/computed/metrics/metric.js
@@ -48,6 +48,10 @@ class ComputedMetric extends ComputedArtifact {
    */
   async compute_(data, artifacts) {
     const {trace, devtoolsLog, settings} = data;
+    if (!trace || !devtoolsLog || !settings) {
+      throw new Error('Did not provide necessary metric computation data');
+    }
+
     const augmentedData = Object.assign({
       networkRecords: await artifacts.requestNetworkRecords(devtoolsLog),
       traceOfTab: await artifacts.requestTraceOfTab(trace),
diff --git a/lighthouse-core/test/audits/load-fast-enough-for-pwa-test.js b/lighthouse-core/test/audits/load-fast-enough-for-pwa-test.js
@@ -6,88 +6,66 @@
 'use strict';
 
 const FastPWAAudit = require('../../audits/load-fast-enough-for-pwa');
+const Runner = require('../../runner.js');
 const Audit = require('../../audits/audit.js');
+const mobile3GThrottling = require('../../config/constants').throttling.mobile3G;
 const assert = require('assert');
 
-function generateArtifacts(firstInteractiveValue, networkRecords = []) {
+const trace = require('../fixtures/traces/progressive-app-m60.json');
+const devtoolsLog = require('../fixtures/traces/progressive-app-m60.devtools.log.json');
+
+function generateArtifacts(ttiValue) {
   return {
     devtoolsLogs: {
       [Audit.DEFAULT_PASS]: [],
     },
-    requestNetworkRecords: () => {
-      return Promise.resolve(networkRecords);
-    },
     traces: {
       [Audit.DEFAULT_PASS]: {traceEvents: []},
     },
-    requestFirstInteractive: () => Promise.resolve({
-      timeInMs: firstInteractiveValue,
+    requestConsistentlyInteractive: () => Promise.resolve({
+      timing: ttiValue,
     }),
   };
 }
 
 /* eslint-env mocha */
 describe('PWA: load-fast-enough-for-pwa audit', () => {
   it('returns boolean based on TTI value', () => {
-    return FastPWAAudit.audit(generateArtifacts(5000)).then(result => {
-      assert.equal(result.rawValue, true, 'fixture trace is not passing audit');
+    const settings = {throttlingMethod: 'devtools', throttling: mobile3GThrottling};
+    return FastPWAAudit.audit(generateArtifacts(5000), {settings}).then(result => {
+      assert.equal(result.score, true, 'fixture trace is not passing audit');
+      assert.equal(result.rawValue, 5000);
     });
   });
 
   it('fails a bad TTI value', () => {
-    return FastPWAAudit.audit(generateArtifacts(15000)).then(result => {
-      assert.equal(result.rawValue, false, 'not failing a long TTI value');
+    const settings = {throttlingMethod: 'devtools', throttling: mobile3GThrottling};
+    return FastPWAAudit.audit(generateArtifacts(15000), {settings}).then(result => {
+      assert.equal(result.score, false, 'not failing a long TTI value');
+      assert.equal(result.rawValue, 15000);
       assert.ok(result.debugString);
     });
   });
 
-  it('warns on a good TTI value with no throttling', () => {
-    // latencies are very short
-    const mockNetworkRecords = [
-      {_timing: {sendEnd: 0, receiveHeadersEnd: 50}, finished: true, _url: 'https://google.com/'},
-      {_timing: {sendEnd: 0, receiveHeadersEnd: 75}, finished: true, _url: 'https://google.com/a'},
-      { },
-      {_timing: {sendEnd: 0, receiveHeadersEnd: 50}, finished: true, _url: 'https://google.com/b'},
-    ];
-    return FastPWAAudit.audit(generateArtifacts(5000, mockNetworkRecords)).then(result => {
-      assert.equal(result.rawValue, true);
-      assert.ok(result.debugString.includes('network request latencies'));
-      assert.ok(result.details, 'contains details when latencies were not realistic');
-    });
-  });
+  it('respects the observed result when throttling is preset', async () => {
+    const artifacts = Object.assign({
+      traces: {defaultPass: trace},
+      devtoolsLogs: {defaultPass: devtoolsLog},
+    }, Runner.instantiateComputedArtifacts());
 
-  it('ignores resources coming from cache', () => {
-    const mockNetworkRecords = [
-      {_timing: {sendEnd: 0, receiveHeadersEnd: 50}, _fromDiskCache: true},
-    ];
-    return FastPWAAudit.audit(generateArtifacts(5000, mockNetworkRecords)).then(result => {
-      assert.equal(result.rawValue, true);
-      assert.strictEqual(result.debugString, undefined);
-    });
+    const settings = {throttlingMethod: 'devtools', throttling: mobile3GThrottling};
+    const result = await FastPWAAudit.audit(artifacts, {settings});
+    assert.equal(Math.round(result.rawValue), 1582);
   });
 
-  it('passes a good TTI value and WITH throttling', () => {
-    // latencies are very long
-    const urlA = 'https://google.com';
-    const urlB = 'https://example.com';
-    const urlC = 'https://example-c.com';
-    const mockNetworkRecords = [
-      {_timing: {sendEnd: 0, receiveHeadersEnd: 250}, finished: true, _url: urlA, _startTime: 0},
-      {_timing: {sendEnd: 0, receiveHeadersEnd: 250}, finished: true, _url: urlB},
-      // ignored for not having timing
-      { },
-      // ignored for not being the first of the origin
-      {_timing: {sendEnd: 0, receiveHeadersEnd: 100}, finished: true, _url: urlA, _startTime: 100},
-      // ignored for being redirected internally
-      {_timing: {sendEnd: 0, receiveHeadersEnd: 100}, finished: true, _url: urlC, _startTime: 0,
-        statusCode: 307},
-      // ignored for not finishing
-      {_timing: {sendEnd: 0, receiveHeadersEnd: -1}, finished: false},
-    ];
-    return FastPWAAudit.audit(generateArtifacts(5000, mockNetworkRecords)).then(result => {
-      assert.equal(result.rawValue, true);
-      assert.strictEqual(result.debugString, undefined);
-      assert.ok(!result.details, 'does not contain details when latencies are realistic');
-    });
+  it('overrides with simulated result when throttling is modified', async () => {
+    const artifacts = Object.assign({
+      traces: {defaultPass: trace},
+      devtoolsLogs: {defaultPass: devtoolsLog},
+    }, Runner.instantiateComputedArtifacts());
+
+    const settings = {throttlingMethod: 'provided', throttling: {rttMs: 40, throughput: 100000}};
+    const result = await FastPWAAudit.audit(artifacts, {settings});
+    assert.equal(Math.round(result.rawValue), 5308);
   });
 });
diff --git a/lighthouse-core/test/results/sample_v2.json b/lighthouse-core/test/results/sample_v2.json
@@ -105,24 +105,7 @@
     "load-fast-enough-for-pwa": {
       "score": 1,
       "displayValue": "",
-      "rawValue": true,
-      "extendedInfo": {
-        "value": {
-          "areLatenciesAll3G": true,
-          "firstRequestLatencies": [
-            {
-              "url": "http://localhost:10200/dobetterweb/dbw_tester.html",
-              "latency": "570.56"
-            },
-            {
-              "url": "http://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js",
-              "latency": "564.12"
-            }
-          ],
-          "isFast": true,
-          "timeToFirstInteractive": 4927.278
-        }
-      },
+      "rawValue": 4927.278,
       "scoreDisplayMode": "binary",
       "name": "load-fast-enough-for-pwa",
       "description": "Page load is fast enough on 3G",
@@ -5403,6 +5386,6 @@
     }
   },
   "timing": {
-    "total": 877
+    "total": 788
   }
 }
