refactor: move instantiateComputedArtifacts to Runner (#2033)

also removes `requestundefined()` from ComputedArtifact base class

lighthouse-core/gather/gather-runner.js

@@ -18,7 +18,6 @@
 
 const log = require('../lib/log.js');
 const Audit = require('../audits/audit');
-const path = require('path');
 const URL = require('../lib/url-shim');
 
 /**
@@ -373,10 +372,8 @@ class GatherRunner {
       .then(_ => GatherRunner.disposeDriver(driver))
       .then(_ => GatherRunner.collectArtifacts(gathererResults))
       .then(artifacts => {
-        // Add tracing data and computed artifacts to artifacts object.
-        const computedArtifacts = this.instantiateComputedArtifacts();
-        Object.assign(artifacts, computedArtifacts, tracingData);
-        return artifacts;
+        // Add tracing data to the artifacts object.
+        return Object.assign(artifacts, tracingData);
       })
       // cleanup on error
       .catch(err => {
@@ -430,22 +427,6 @@ class GatherRunner {
     }
   }
 
-  /**
-   * @return {!ComputedArtifacts}
-   */
-  static instantiateComputedArtifacts() {
-    const computedArtifacts = {};
-    require('fs').readdirSync(path.join(__dirname, 'computed')).forEach(function(file) {
-      // Drop `.js` suffix to keep browserify import happy.
-      file = file.replace(/\.js$/, '');
-      const ArtifactClass = require('./computed/' + file);
-      const artifact = new ArtifactClass(computedArtifacts);
-      // define the request* function that will be exposed on `artifacts`
-      computedArtifacts['request' + artifact.name] = artifact.request.bind(artifact);
-    });
-    return computedArtifacts;
-  }
-
   static instantiateGatherers(passes, rootPath) {
     return passes.map(pass => {
       pass.gatherers = pass.gatherers.map(gatherer => {

lighthouse-core/runner.js

@@ -69,15 +69,18 @@ class Runner {
     // to check that there are artifacts specified in the config, and throw if not.
     if (validPassesAndAudits || validArtifactsAndAudits) {
       if (validPassesAndAudits) {
+        // Set up the driver and run gatherers.
         opts.driver = opts.driverMock || new Driver(connection);
-        // Finally set up the driver to gather.
         run = run.then(_ => GatherRunner.run(config.passes, opts));
       } else if (validArtifactsAndAudits) {
-        run = run.then(_ => {
-          return Object.assign(config.artifacts, GatherRunner.instantiateComputedArtifacts());
-        });
+        run = run.then(_ => config.artifacts);
       }
 
+      // Add computed artifacts.
+      run = run.then(artifacts => {
+        return Object.assign({}, artifacts, Runner.instantiateComputedArtifacts());
+      });
+
       // Basic check that the traces (gathered or loaded) are valid.
       run = run.then(artifacts => {
         for (const passName of Object.keys(artifacts.traces || {})) {
@@ -105,8 +108,8 @@ class Runner {
     } else if (config.auditResults) {
       // If there are existing audit results, surface those here.
       // Instantiate and return artifacts for consistency.
-      const artifacts = Object.assign(config.artifacts || {},
-          GatherRunner.instantiateComputedArtifacts());
+      const artifacts = Object.assign({}, config.artifacts || {},
+          Runner.instantiateComputedArtifacts());
       run = run.then(_ => {
         return {
           artifacts,
@@ -249,6 +252,25 @@ class Runner {
     return fileList.filter(f => /\.js$/.test(f) && f !== 'gatherer.js').sort();
   }
 
+  /**
+   * @return {!ComputedArtifacts}
+   */
+  static instantiateComputedArtifacts() {
+    const computedArtifacts = {};
+    require('fs').readdirSync(__dirname + '/gather/computed').forEach(function(filename) {
+      // Skip base class.
+      if (filename === 'computed-artifact.js') return;
+
+      // Drop `.js` suffix to keep browserify import happy.
+      filename = filename.replace(/\.js$/, '');
+      const ArtifactClass = require('./gather/computed/' + filename);
+      const artifact = new ArtifactClass(computedArtifacts);
+      // define the request* function that will be exposed on `artifacts`
+      computedArtifacts['request' + artifact.name] = artifact.request.bind(artifact);
+    });
+    return computedArtifacts;
+  }
+
   /**
    * Resolves the location of the specified plugin and returns an absolute
    * string path to the file. Used for loading custom audits and gatherers.

lighthouse-core/test/audits/estimated-input-latency-test.js

@@ -16,12 +16,12 @@
 'use strict';
 
 const Audit = require('../../audits/estimated-input-latency.js');
-const GatherRunner = require('../../gather/gather-runner.js');
+const Runner = require('../../runner.js');
 const assert = require('assert');
 
 const pwaTrace = require('../fixtures/traces/progressive-app.json');
 
-const computedArtifacts = GatherRunner.instantiateComputedArtifacts();
+const computedArtifacts = Runner.instantiateComputedArtifacts();
 
 function generateArtifactsWithTrace(trace) {
   return Object.assign({

lighthouse-core/test/audits/first-meaningful-paint-test.js

@@ -25,8 +25,8 @@ const preactTrace = require('../fixtures/traces/preactjs.com_ts_of_undefined.jso
 const noFMPtrace = require('../fixtures/traces/no_fmp_event.json');
 const noFCPtrace = require('../fixtures/traces/airhorner_no_fcp');
 
-const GatherRunner = require('../../gather/gather-runner.js');
-const computedArtifacts = GatherRunner.instantiateComputedArtifacts();
+const Runner = require('../../runner.js');
+const computedArtifacts = Runner.instantiateComputedArtifacts();
 
 function generateArtifactsWithTrace(trace) {
   return Object.assign({

lighthouse-core/test/audits/manifest-short-name-length-test.js

@@ -22,10 +22,10 @@ const manifestParser = require('../../lib/manifest-parser');
 const EXAMPLE_MANIFEST_URL = 'https://example.com/manifest.json';
 const EXAMPLE_DOC_URL = 'https://example.com/index.html';
 
-const GatherRunner = require('../../gather/gather-runner.js');
+const Runner = require('../../runner.js');
 
 function generateMockArtifacts() {
-  const computedArtifacts = GatherRunner.instantiateComputedArtifacts();
+  const computedArtifacts = Runner.instantiateComputedArtifacts();
   const mockArtifacts = Object.assign({}, computedArtifacts, {
     Manifest: null
   });

lighthouse-core/test/audits/splash-screen-test.js

@@ -15,10 +15,10 @@ const EXAMPLE_MANIFEST_URL = 'https://example.com/manifest.json';
 const EXAMPLE_DOC_URL = 'https://example.com/index.html';
 const exampleManifest = noUrlManifestParser(manifestSrc);
 
-const GatherRunner = require('../../gather/gather-runner.js');
+const Runner = require('../../runner.js');
 
 function generateMockArtifacts() {
-  const computedArtifacts = GatherRunner.instantiateComputedArtifacts();
+  const computedArtifacts = Runner.instantiateComputedArtifacts();
   const mockArtifacts = Object.assign({}, computedArtifacts, {
     Manifest: exampleManifest
   });

lighthouse-core/test/audits/themed-omnibox-test.js

@@ -15,10 +15,10 @@ const EXAMPLE_MANIFEST_URL = 'https://example.com/manifest.json';
 const EXAMPLE_DOC_URL = 'https://example.com/index.html';
 const exampleManifest = noUrlManifestParser(manifestSrc);
 
-const GatherRunner = require('../../gather/gather-runner.js');
+const Runner = require('../../runner.js');
 
 function generateMockArtifacts() {
-  const computedArtifacts = GatherRunner.instantiateComputedArtifacts();
+  const computedArtifacts = Runner.instantiateComputedArtifacts();
   const mockArtifacts = Object.assign({}, computedArtifacts, {
     Manifest: exampleManifest,
     ThemeColor: '#bada55'

lighthouse-core/test/audits/time-to-interactive-test.js

@@ -16,7 +16,7 @@
 'use strict';
 
 const TimeToInteractive = require('../../audits/time-to-interactive.js');
-const GatherRunner = require('../../gather/gather-runner.js');
+const Runner = require('../../runner.js');
 const assert = require('assert');
 
 const pwaTrace = require('../fixtures/traces/progressive-app.json');
@@ -30,7 +30,7 @@ describe('Performance: time-to-interactive audit', () => {
           traceEvents: pwaTrace
         }
       }
-    }, GatherRunner.instantiateComputedArtifacts());
+    }, Runner.instantiateComputedArtifacts());
 
     return TimeToInteractive.audit(artifacts).then(output => {
       assert.equal(output.rawValue, 1105.8, output.debugString);

lighthouse-core/test/audits/user-timing-test.js

@@ -19,8 +19,8 @@ const Audit = require('../../audits/user-timings.js');
 const assert = require('assert');
 const traceEvents = require('../fixtures/traces/trace-user-timings.json');
 
-const GatherRunner = require('../../gather/gather-runner.js');
-const computedArtifacts = GatherRunner.instantiateComputedArtifacts();
+const Runner = require('../../runner.js');
+const computedArtifacts = Runner.instantiateComputedArtifacts();
 
 function generateArtifactsWithTrace(trace) {
   return Object.assign({

lighthouse-core/test/audits/webapp-install-banner-test.js

@@ -15,10 +15,10 @@ const EXAMPLE_MANIFEST_URL = 'https://example.com/manifest.json';
 const EXAMPLE_DOC_URL = 'https://example.com/index.html';
 const exampleManifest = manifestParser(manifestSrc, EXAMPLE_MANIFEST_URL, EXAMPLE_DOC_URL);
 
-const GatherRunner = require('../../gather/gather-runner.js');
+const Runner = require('../../runner.js');
 
 function generateMockArtifacts() {
-  const computedArtifacts = GatherRunner.instantiateComputedArtifacts();
+  const computedArtifacts = Runner.instantiateComputedArtifacts();
   const clonedArtifacts = JSON.parse(JSON.stringify({
     Manifest: exampleManifest,
     ServiceWorker: {

lighthouse-core/test/gather/gather-runner-test.js

@@ -561,37 +561,6 @@ describe('GatherRunner', function() {
       /afterPass\(\) method/);
   });
 
-  it('can create computed artifacts', () => {
-    const computedArtifacts = GatherRunner.instantiateComputedArtifacts();
-    assert.ok(Object.keys(computedArtifacts).length, 'there are a few computed artifacts');
-    Object.keys(computedArtifacts).forEach(artifactRequest => {
-      assert.equal(typeof computedArtifacts[artifactRequest], 'function');
-    });
-  });
-
-  it('will instantiate computed artifacts during a run', () => {
-    const passes = [{
-      blankDuration: 0,
-      recordNetwork: true,
-      recordTrace: true,
-      passName: 'firstPass',
-      gatherers: [new TestGatherer()]
-    }];
-    const options = {driver: fakeDriver, url: 'https://example.com', flags: {}, config: {}};
-
-    return GatherRunner.run(passes, options)
-        .then(artifacts => {
-          const networkRecords = artifacts.networkRecords.firstPass;
-          const p = artifacts.requestCriticalRequestChains(networkRecords);
-          return p.then(chains => {
-            // fakeDriver will include networkRecords built from fixtures/perflog.json
-            assert.ok(chains['93149.1']);
-            assert.ok(chains['93149.1'].request);
-            assert.ok(chains['93149.1'].children);
-          });
-        });
-  });
-
   describe('#assertPageLoaded', () => {
     it('passes when the page is loaded', () => {
       const url = 'http://the-page.com';

lighthouse-core/test/runner-test.js

@@ -21,7 +21,7 @@ const Config = require('../config/config');
 const Audit = require('../audits/audit');
 const assert = require('assert');
 const path = require('path');
-const computedArtifacts = require('../gather/gather-runner').instantiateComputedArtifacts();
+const computedArtifacts = Runner.instantiateComputedArtifacts();
 
 /* eslint-env mocha */
 
@@ -410,6 +410,14 @@ describe('Runner', () => {
     });
   });
 
+  it('can create computed artifacts', () => {
+    const computedArtifacts = Runner.instantiateComputedArtifacts();
+    assert.ok(Object.keys(computedArtifacts).length, 'there are a few computed artifacts');
+    Object.keys(computedArtifacts).forEach(artifactRequest => {
+      assert.equal(typeof computedArtifacts[artifactRequest], 'function');
+    });
+  });
+
   it('results include artifacts when given artifacts and audits', () => {
     const url = 'https://example.com';
     const ViewportDimensions = {innerHeight: 10, innerWidth: 10};
@@ -430,10 +438,12 @@ describe('Runner', () => {
     });
   });
 
-  it('results include artifacts when given passes and audits', () => {
+  it('results include artifacts and computedArtifacts when given passes and audits', () => {
     const url = 'https://example.com';
     const config = new Config({
       passes: [{
+        passName: 'firstPass',
+        recordNetwork: true,
         gatherers: ['viewport-dimensions']
       }],
 
@@ -449,6 +459,16 @@ describe('Runner', () => {
       for (const method of Object.keys(computedArtifacts)) {
         assert.ok(results.artifacts.hasOwnProperty(method));
       }
+
+      // Verify a computed artifact. driverMock will include networkRecords
+      // built from fixtures/perflog.json.
+      const networkRecords = results.artifacts.networkRecords.firstPass;
+      const p = results.artifacts.requestCriticalRequestChains(networkRecords);
+      return p.then(chains => {
+        assert.ok(chains['93149.1']);
+        assert.ok(chains['93149.1'].request);
+        assert.ok(chains['93149.1'].children);
+      });
     });
   });