core(config): remove config.artifacts; always use auditMode (#4986)

GoogleChrome · Apr 18, 2018 · 84debe2 · 84debe2
1 parent 3ce82a1
commit 84debe2
Show file tree

Hide file tree

Showing 16 changed files with 99 additions and 287 deletions.
diff --git a/.gitignore b/.gitignore
@@ -26,6 +26,8 @@ last-run-results.html
 
 !lighthouse-core/test/results/artifacts/*.trace.json
 !lighthouse-core/test/results/artifacts/*.devtoolslog.json
+!lighthouse-core/test/fixtures/artifacts/**/*.trace.json
+!lighthouse-core/test/fixtures/artifacts/**/*.devtoolslog.json
 
 latest-run
 

diff --git a/docs/readme.md b/docs/readme.md
@@ -93,28 +93,22 @@ $ lighthouse --port=9222 --disable-device-emulation --disable-cpu-throttling htt
 
 ## Lighthouse as trace processor
 
-Lighthouse can be used to analyze trace and performance data collected from other tools (like WebPageTest and ChromeDriver). The `traces` and `devtoolsLogs` artifact items can be provided using a string for the absolute path on disk. The `devtoolsLogs` array is captured from the `Network` and `Page` domains (a la ChromeDriver's [enableNetwork and enablePage options]((https://sites.google.com/a/chromium.org/chromedriver/capabilities#TOC-perfLoggingPrefs-object)).
+Lighthouse can be used to analyze trace and performance data collected from other tools (like WebPageTest and ChromeDriver). The `traces` and `devtoolsLogs` artifact items can be provided using a string for the absolute path on disk if they're saved with `.trace.json` and `.devtoolslog.json` file extensions, respectively. The `devtoolsLogs` array is captured from the `Network` and `Page` domains (a la ChromeDriver's [enableNetwork and enablePage options]((https://sites.google.com/a/chromium.org/chromedriver/capabilities#TOC-perfLoggingPrefs-object)).
 
-As an example, here's a trace-only run that's reporting on user timings and critical request chains:
+As an example, here's a trace-only run that reports on user timings and critical request chains:
 
 ### `config.json`
 
 ```json
 {
+  "settings": {
+    "auditMode": "/User/me/lighthouse/lighthouse-core/test/fixtures/artifacts/perflog/",
+  },
   "audits": [
     "user-timings",
     "critical-request-chains"
   ],
 
-  "artifacts": {
-    "traces": {
-      "defaultPass": "/User/me/lighthouse/lighthouse-core/test/fixtures/traces/trace-user-timings.json"
-    },
-    "devtoolsLogs": {
-      "defaultPass": "/User/me/lighthouse/lighthouse-core/test/fixtures/traces/perflog.json"
-    }
-  },
-
   "categories": {
     "performance": {
       "name": "Performance Metrics",

diff --git a/lighthouse-core/config/config.js b/lighthouse-core/config/config.js
@@ -17,89 +17,6 @@ const path = require('path');
 const Audit = require('../audits/audit');
 const Runner = require('../runner');
 
-// cleanTrace is run to remove duplicate TracingStartedInPage events,
-// and to change TracingStartedInBrowser events into TracingStartedInPage.
-// This is done by searching for most occuring threads and basing new events
-// off of those.
-function cleanTrace(trace) {
-  const traceEvents = trace.traceEvents;
-  // Keep track of most occuring threads
-  const threads = [];
-  const countsByThread = {};
-  const traceStartEvents = [];
-  const makeMockEvent = (evt, ts) => {
-    return {
-      pid: evt.pid,
-      tid: evt.tid,
-      ts: ts || 0, // default to 0 for now
-      ph: 'I',
-      cat: 'disabled-by-default-devtools.timeline',
-      name: 'TracingStartedInPage',
-      args: {
-        data: {
-          page: evt.frame,
-        },
-      },
-      s: 't',
-    };
-  };
-
-  let frame;
-  let data;
-  let name;
-  let counter;
-
-  traceEvents.forEach((evt, idx) => {
-    if (evt.name.startsWith('TracingStartedIn')) {
-      traceStartEvents.push(idx);
-    }
-
-    // find the event's frame
-    data = evt.args && (evt.args.data || evt.args.beginData || evt.args.counters);
-    frame = (evt.args && evt.args.frame) || data && (data.frame || data.page);
-
-    if (!frame) {
-      return;
-    }
-
-    // Increase occurences count of the frame
-    name = `pid${evt.pid}-tid${evt.tid}-frame${frame}`;
-    counter = countsByThread[name];
-    if (!counter) {
-      counter = {
-        pid: evt.pid,
-        tid: evt.tid,
-        frame: frame,
-        count: 0,
-      };
-      countsByThread[name] = counter;
-      threads.push(counter);
-    }
-    counter.count++;
-  });
-
-  // find most active thread (and frame)
-  threads.sort((a, b) => b.count - a.count);
-  const mostActiveFrame = threads[0];
-
-  // Remove all current TracingStartedIn* events, storing
-  // the first events ts.
-  const ts = traceEvents[traceStartEvents[0]] && traceEvents[traceStartEvents[0]].ts;
-
-  // account for offset after removing items
-  let i = 0;
-  for (const dup of traceStartEvents) {
-    traceEvents.splice(dup - i, 1);
-    i++;
-  }
-
-  // Add a new TracingStartedInPage event based on most active thread
-  // and using TS of first found TracingStartedIn* event
-  traceEvents.unshift(makeMockEvent(mostActiveFrame, ts));
-
-  return trace;
-}
-
 function validatePasses(passes, audits) {
   if (!Array.isArray(passes)) {
     return;
@@ -214,38 +131,6 @@ function assertValidGatherer(gathererInstance, gathererName) {
   }
 }
 
-function expandArtifacts(artifacts) {
-  if (!artifacts) {
-    return null;
-  }
-  // currently only trace logs and performance logs should be imported
-  if (artifacts.traces) {
-    Object.keys(artifacts.traces).forEach(key => {
-      log.log('info', 'Normalizng trace contents into expected state...');
-      let trace = require(artifacts.traces[key]);
-      // Before Chrome 54.0.2816 (codereview.chromium.org/2161583004), trace was
-      // an array of trace events. After this point, trace is an object with a
-      // traceEvents property. Normalize to new format.
-      if (Array.isArray(trace)) {
-        trace = {
-          traceEvents: trace,
-        };
-      }
-      trace = cleanTrace(trace);
-
-      artifacts.traces[key] = trace;
-    });
-  }
-
-  if (artifacts.devtoolsLogs) {
-    Object.keys(artifacts.devtoolsLogs).forEach(key => {
-      artifacts.devtoolsLogs[key] = require(artifacts.devtoolsLogs[key]);
-    });
-  }
-
-  return artifacts;
-}
-
 /**
  * Creates a settings object from potential flags object by dropping all the properties
  * that don't exist on Config.Settings.
@@ -367,7 +252,6 @@ class Config {
 
     this._passes = Config.requireGatherers(configJSON.passes, this._configDir);
     this._audits = Config.requireAudits(configJSON.audits, this._configDir);
-    this._artifacts = expandArtifacts(configJSON.artifacts);
     this._categories = configJSON.categories;
     this._groups = configJSON.groups;
     this._settings = configJSON.settings || {};
@@ -782,11 +666,6 @@ class Config {
     return this._audits;
   }
 
-  /** @type {Array<!Artifacts>} */
-  get artifacts() {
-    return this._artifacts;
-  }
-
   /** @type {Object<{audits: !Array<{id: string, weight: number}>}>} */
   get categories() {
     return this._categories;

diff --git a/lighthouse-core/lib/asset-saver.js b/lighthouse-core/lib/asset-saver.js
@@ -80,13 +80,16 @@ img {
 async function loadArtifacts(basePath) {
   log.log('Reading artifacts from disk:', basePath);
 
-  if (!fs.existsSync(basePath)) return Promise.reject(new Error('No saved artifacts found'));
+  if (!fs.existsSync(basePath)) {
+    throw new Error('No saved artifacts found at ' + basePath);
+  }
 
   // load artifacts.json
-  const filenames = fs.readdirSync(basePath);
   /** @type {LH.Artifacts} */
   const artifacts = JSON.parse(fs.readFileSync(path.join(basePath, artifactsFilename), 'utf8'));
 
+  const filenames = fs.readdirSync(basePath);
+
   // load devtoolsLogs
   artifacts.devtoolsLogs = {};
   filenames.filter(f => f.endsWith(devtoolsLogSuffix)).map(filename => {

diff --git a/lighthouse-core/runner.js b/lighthouse-core/runner.js
@@ -70,18 +70,16 @@ class Runner {
       opts.url = parsedURL.href;
 
       // User can run -G solo, -A solo, or -GA together
-      // -G and -A will do run partial lighthouse pipelines,
+      // -G and -A will run partial lighthouse pipelines,
       // and -GA will run everything plus save artifacts to disk
 
       // Gather phase
-      // Either load saved artifacts off disk, from config, or get from the browser
+      // Either load saved artifacts off disk or from the browser
       let artifacts;
       if (settings.auditMode && !settings.gatherMode) {
         // No browser required, just load the artifacts from disk.
         const path = Runner._getArtifactsPath(settings);
         artifacts = await assetSaver.loadArtifacts(path);
-      } else if (opts.config.artifacts) {
-        artifacts = opts.config.artifacts;
       } else {
         artifacts = await Runner._gatherArtifactsFromBrowser(opts, connection);
         // -G means save these to ./latest-run, etc.
@@ -115,7 +113,8 @@ class Runner {
       const resultsById = {};
       for (const audit of auditResults) resultsById[audit.name] = audit;
 
-      let reportCategories;
+      /** @type {Array<LH.Result.Category>} */
+      let reportCategories = [];
       if (opts.config.categories) {
         reportCategories = ReportScoring.scoreAllCategories(opts.config.categories, resultsById);
       }

diff --git a/lighthouse-core/test/config/config-test.js b/lighthouse-core/test/config/config-test.js
@@ -12,7 +12,6 @@ const defaultConfig = require('../../config/default-config.js');
 const log = require('lighthouse-logger');
 const Gatherer = require('../../gather/gatherers/gatherer');
 const Audit = require('../../audits/audit');
-const Runner = require('../../runner');
 
 /* eslint-env mocha */
 
@@ -473,68 +472,6 @@ describe('Config', () => {
     assert.ok(config.settings.disableDeviceEmulation, 'missing setting from flags');
   });
 
-  describe('artifact loading', () => {
-    it('expands artifacts', () => {
-      const config = new Config({
-        artifacts: {
-          traces: {
-            defaultPass: path.resolve(__dirname, '../fixtures/traces/trace-user-timings.json'),
-          },
-          devtoolsLogs: {
-            defaultPass: path.resolve(__dirname, '../fixtures/perflog.json'),
-          },
-        },
-      });
-      const computed = Runner.instantiateComputedArtifacts();
-
-      const traceUserTimings = require('../fixtures/traces/trace-user-timings.json');
-      assert.deepStrictEqual(config.artifacts.traces.defaultPass.traceEvents, traceUserTimings);
-      const devtoolsLogs = config.artifacts.devtoolsLogs.defaultPass;
-      assert.equal(devtoolsLogs.length, 555);
-
-      return computed.requestNetworkRecords(devtoolsLogs).then(records => {
-        assert.equal(records.length, 76);
-      });
-    });
-
-    it('expands artifacts with multiple named passes', () => {
-      const config = new Config({
-        artifacts: {
-          traces: {
-            defaultPass: path.resolve(__dirname, '../fixtures/traces/trace-user-timings.json'),
-            otherPass: path.resolve(__dirname, '../fixtures/traces/trace-user-timings.json'),
-          },
-          devtoolsLogs: {
-            defaultPass: path.resolve(__dirname, '../fixtures/perflog.json'),
-            otherPass: path.resolve(__dirname, '../fixtures/perflog.json'),
-          },
-        },
-      });
-      const traceUserTimings = require('../fixtures/traces/trace-user-timings.json');
-      assert.deepStrictEqual(config.artifacts.traces.defaultPass.traceEvents, traceUserTimings);
-      assert.deepStrictEqual(config.artifacts.traces.otherPass.traceEvents, traceUserTimings);
-      assert.equal(config.artifacts.devtoolsLogs.defaultPass.length, 555);
-      assert.equal(config.artifacts.devtoolsLogs.otherPass.length, 555);
-    });
-
-    it('handles traces with no TracingStartedInPage events', () => {
-      const config = new Config({
-        artifacts: {
-          traces: {
-            defaultPass: path.resolve(__dirname,
-                            '../fixtures/traces/trace-user-timings-no-tracingstartedinpage.json'),
-          },
-          devtoolsLogs: {
-            defaultPass: path.resolve(__dirname, '../fixtures/perflog.json'),
-          },
-        },
-      });
-
-      assert.ok(config.artifacts.traces.defaultPass.traceEvents.find(
-            e => e.name === 'TracingStartedInPage' && e.args.data.page === '0xhad00p'));
-    });
-  });
-
   describe('#extendConfigJSON', () => {
     it('should merge passes', () => {
       const configA = {

diff --git a/lighthouse-core/test/fixtures/artifacts/empty-artifacts/artifacts.json b/lighthouse-core/test/fixtures/artifacts/empty-artifacts/artifacts.json
@@ -0,0 +1 @@
+{}
diff --git a/lighthouse-core/test/fixtures/artifacts/perflog/artifacts.json b/lighthouse-core/test/fixtures/artifacts/perflog/artifacts.json
@@ -0,0 +1,20 @@
+{
+  "LighthouseRunWarnings": [
+    "I'm a warning!",
+    "Also a warning"
+  ],
+  "UserAgent": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/66.0.3358.0 Safari/537.36",
+  "fetchedAt": "2018-03-13T00:55:45.840Z",
+  "URL": {
+    "initialUrl": "https://www.reddit.com/r/nba",
+    "finalUrl": "https://www.reddit.com/r/nba"
+  },
+  "Viewport": null,
+  "ViewportDimensions": {
+    "innerWidth": 412,
+    "innerHeight": 732,
+    "outerWidth": 412,
+    "outerHeight": 732,
+    "devicePixelRatio": 2.625
+  }
+}
diff --git a/lighthouse-core/test/fixtures/perflog.json → ...acts/perflog/defaultPass.devtoolslog.json b/lighthouse-core/test/fixtures/perflog.json → ...acts/perflog/defaultPass.devtoolslog.json
diff --git a/lighthouse-core/test/fixtures/artifacts/perflog/defaultPass.trace.json b/lighthouse-core/test/fixtures/artifacts/perflog/defaultPass.trace.json
@@ -0,0 +1,14 @@
+[
+{"pid":41904,"tid":1295,"ts":1676836141,"ph":"I","cat":"disabled-by-default-devtools.timeline","name":"TracingStartedInPage","args":{"data":{"page":"0xf5fc2501e08","sessionId":"9331.8"}},"tts":314881,"s":"t"},
+{"pid":41904,"tid":1295,"ts":506085991145,"ph":"R","cat":"blink.user_timing","name":"navigationStart","args":{"frame": "0xf5fc2501e08"},"tts":314882},
+{"pid":41904,"tid":1295,"ts":506085991146,"ph":"R","cat":"blink.user_timing","name":"firstPaint","args":{"frame": "0xf5fc2501e08"},"tts":314883},
+{"pid":41904,"tid":1295,"ts":506085991146,"ph":"R","cat":"blink.user_timing","name":"firstContentfulPaint","args":{"frame": "0xf5fc2501e08"},"tts":314883},
+{"pid":41904,"tid":1295,"ts":506085991146,"ph":"R","cat":"blink.user_timing","name":"paintNonDefaultBackgroundColor","args":{},"tts":314883},
+{"pid":41904,"tid":1295,"ts":506086992099,"ph":"R","cat":"blink.user_timing","name":"mark_test","args":{},"tts":331149},
+{"pid":41904,"tid":1295,"ts":506086992100,"ph":"R","cat":"blink.user_timing","name":"goog_123_3_1_start","args":{},"tts":331150},
+{"pid":41904,"tid":1295,"ts":506086992101,"ph":"R","cat":"blink.user_timing","name":"goog_123_3_1_end","args":{},"tts":331151},
+{"pid":41904,"tid":1295,"ts":506085991147,"ph":"b","cat":"blink.user_timing","name":"measure_test","args":{},"tts":331184,"id":"0x73b016"},
+{"pid":41904,"tid":1295,"ts":506086992112,"ph":"e","cat":"blink.user_timing","name":"measure_test","args":{},"tts":331186,"id":"0x73b016"},
+{"pid":41904,"tid":1295,"ts":506085991148,"ph":"b","cat":"blink.user_timing","name":"goog_123_3_1","args":{},"tts":331184,"id":"0x73b016"},
+{"pid":41904,"tid":1295,"ts":506086992113,"ph":"e","cat":"blink.user_timing","name":"goog_123_3_1","args":{},"tts":331186,"id":"0x73b016"}
+]
diff --git a/lighthouse-core/test/gather/fake-driver.js b/lighthouse-core/test/gather/fake-driver.js
@@ -60,7 +60,7 @@ module.exports = {
   },
   beginDevtoolsLog() {},
   endDevtoolsLog() {
-    return require('../fixtures/perflog.json');
+    return require('../fixtures/artifacts/perflog/defaultPass.devtoolslog.json');
   },
   blockUrlPatterns() {
     return Promise.resolve();

diff --git a/lighthouse-core/test/gather/network-recorder-test.js b/lighthouse-core/test/gather/network-recorder-test.js
@@ -7,7 +7,7 @@
 
 const NetworkRecorder = require('../../lib/network-recorder');
 const assert = require('assert');
-const devtoolsLogItems = require('../fixtures/perflog.json');
+const devtoolsLogItems = require('../fixtures/artifacts/perflog/defaultPass.devtoolslog.json');
 
 /* eslint-env mocha */
 describe('network recorder', function() {