Updates tests and fixes bugs found by doing so

lighthouse-core/core.js

@@ -26,6 +26,12 @@ class Core {
   }
 
   static expandAudits(audits) {
+    // It's possible we didn't get given any audits (but existing audit results), in which case
+    // there is no need to do any expansion work.
+    if (!audits) {
+      return;
+    }
+
     return audits.map(audit => {
       // If this is already instantiated, don't do anything else.
       if (typeof audit !== 'string') {
@@ -41,6 +47,12 @@ class Core {
   }
 
   static filterAudits(audits, whitelist) {
+    // It's possible we didn't get given any audits (but existing audit results), in which case
+    // there is no need to do any filter work.
+    if (!audits) {
+      return;
+    }
+
     const rejected = [];
     const filteredAudits = audits.filter(a => {
       // If there is no whitelist, assume all.

lighthouse-core/runner.js

@@ -24,8 +24,13 @@ const fs = require('fs');
 const path = require('path');
 
 class Runner {
-
   static getGatherersNeededByAudits(audits) {
+    // It's possible we didn't get given any audits (but existing audit results), in which case
+    // there is no need to do any work here.
+    if (!audits) {
+      return new Set();
+    }
+
     audits = Core.expandAudits(audits);
 
     return audits.reduce((list, audit) => {
@@ -40,71 +45,80 @@ class Runner {
     // Filter out any audits by the whitelist.
     config.audits = Core.filterAudits(config.audits, opts.flags.auditWhitelist);
 
+    // Check that there are passes & audits...
+    const validPassesAndAudits = (
+        typeof config.passes !== 'undefined' &&
+        typeof config.audits !== 'undefined');
+
+    // ... or that there are artifacts & audits.
+    const validArtifactsAndAudits = (
+        typeof config.artifacts !== 'undefined' &&
+        typeof config.audits !== 'undefined');
+
+    // And we need either of those _or_ existing audit results.
+    if (!(validPassesAndAudits || validArtifactsAndAudits || config.auditResults)) {
+      throw new Error(
+          'The config must provide passes and audits, artifacts and audits, or auditResults');
+    }
+
     // Make a run, which can be .then()'d with whatever needs to run (based on the config).
     let run = Promise.resolve();
 
     // If there are passes run the Driver and gather the artifacts. If not, we will need
     // to check that there are artifacts specified in the config, and throw if not.
-    if (config.passes) {
-      const requiredGatherers = this.getGatherersNeededByAudits(config.audits);
-
-      // Make sure we only have the gatherers that are needed by the audits
-      // that have been listed in the config.
-      const filteredPasses = config.passes.map(pass => {
-        pass.gatherers = pass.gatherers.filter(gatherer => {
-          if (typeof gatherer !== 'string') {
-            return requiredGatherers.has(gatherer.name);
-          }
-
-          try {
-            const GathererClass = Driver.getGathererClass(gatherer);
-            return requiredGatherers.has(GathererClass.name);
-          } catch (requireError) {
-            throw new Error(`Unable to locate gatherer: ${gatherer}`);
-          }
-        });
+    if (validPassesAndAudits || validArtifactsAndAudits) {
+      if (validPassesAndAudits) {
+        const requiredGatherers = this.getGatherersNeededByAudits(config.audits);
+
+        // Make sure we only have the gatherers that are needed by the audits
+        // that have been listed in the config.
+        const filteredPasses = config.passes.map(pass => {
+          pass.gatherers = pass.gatherers.filter(gatherer => {
+            if (typeof gatherer !== 'string') {
+              return requiredGatherers.has(gatherer.name);
+            }
+
+            try {
+              const GathererClass = Driver.getGathererClass(gatherer);
+              return requiredGatherers.has(GathererClass.name);
+            } catch (requireError) {
+              throw new Error(`Unable to locate gatherer: ${gatherer}`);
+            }
+          });
 
-        return pass;
-      })
+          return pass;
+        })
 
-      // Now remove any passes which no longer have gatherers.
-      .filter(p => p.gatherers.length > 0);
+        // Now remove any passes which no longer have gatherers.
+        .filter(p => p.gatherers.length > 0);
 
-      // Finally set up the driver to gather.
-      run = run.then(_ => Driver.run(filteredPasses, Object.assign({}, opts, {driver})));
-    } else {
-      if (!config.artifacts) {
-        throw new Error('Neither passes nor artifacts has been included in the config.');
+        // Finally set up the driver to gather.
+        run = run.then(_ => Driver.run(filteredPasses, Object.assign({}, opts, {driver})));
+      } else if (validArtifactsAndAudits) {
+        run = run.then(_ => config.artifacts);
       }
 
-      run = run.then(_ => config.artifacts);
-    }
-
-    // Ignoring these two flags since this functionality is not exposed by the module.
-    /* istanbul ignore next */
-    if (opts.flags.saveArtifacts) {
-      run = run.then(artifacts => {
-        assetSaver.saveArtifacts(artifacts);
-        return artifacts;
-      });
-    }
-
-    /* istanbul ignore next */
-    if (opts.flags.saveAssets) {
-      run = run.then(artifacts => {
-        assetSaver.saveAssets(opts, artifacts);
-        return artifacts;
-      });
-    }
+      // Ignoring these two flags since this functionality is not exposed by the module.
+      /* istanbul ignore next */
+      if (opts.flags.saveArtifacts) {
+        run = run.then(artifacts => {
+          assetSaver.saveArtifacts(artifacts);
+          return artifacts;
+        });
+      }
 
-    // Now either run the audits, or use the supplied auditResults.
-    if (config.audits) {
-      run = run.then(artifacts => Core.audit(artifacts, config.audits));
-    } else {
-      if (!config.auditResults) {
-        throw new Error('Neither audits nor auditResults has been included in the config.');
+      /* istanbul ignore next */
+      if (opts.flags.saveAssets) {
+        run = run.then(artifacts => {
+          assetSaver.saveAssets(opts, artifacts);
+          return artifacts;
+        });
       }
 
+      // Now run the audits.
+      run.then(artifacts => Core.audit(artifacts, config.audits));
+    } else if (config.auditResults) {
+      // If there are existing audit results, surface those here.
       run = run.then(_ => config.auditResults);
     }
 

lighthouse-core/test/core.js

@@ -13,18 +13,53 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-const Auditor = require('../core');
+const Core = require('../core');
 const assert = require('assert');
 
 /* global describe, it*/
 
 describe('Core', () => {
   it('maps all audits to an array of Promises', () => {
-    return Auditor
+    return Core
       .audit([{}], ['is-on-https'])
       .then(modifiedResults => {
         assert.ok(Array.isArray(modifiedResults));
         assert.equal(modifiedResults.length, 1);
       });
   });
+
+  it('handles non-existent audits when expanding', () => {
+    const modifiedResults = Core.expandAudits();
+
+    return assert.equal(modifiedResults, undefined);
+  });
+
+  it('expands audits', () => {
+    const modifiedResults = Core.expandAudits(['is-on-https']);
+
+    assert.ok(Array.isArray(modifiedResults));
+    assert.equal(modifiedResults.length, 1);
+    return assert.equal(typeof modifiedResults[0], 'function');
+  });
+
+  it('handles non-existent audits when filtering', () => {
+    const modifiedResults = Core.filterAudits(undefined, ['a']);
+
+    return assert.equal(modifiedResults, undefined);
+  });
+
+  it('returns unfiltered audits when no whitelist is given', () => {
+    const modifiedResults = Core.filterAudits(['is-on-https']);
+
+    assert.ok(Array.isArray(modifiedResults));
+    assert.equal(modifiedResults.length, 1);
+    return assert.equal(modifiedResults[0], 'is-on-https');
+  });
+
+  it('returns filtered audits when a whitelist is given', () => {
+    const modifiedResults = Core.filterAudits(['is-on-https'], new Set(['b']));
+
+    assert.ok(Array.isArray(modifiedResults));
+    return assert.equal(modifiedResults.length, 0);
+  });
 });

lighthouse-core/test/driver/fake-driver.js

@@ -0,0 +1,51 @@
+/**
+ * Copyright 2016 Google Inc. All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+'use strict';
+
+module.exports = {
+  connect() {
+    return Promise.resolve();
+  },
+  disconnect() {},
+  gotoURL() {
+    return Promise.resolve();
+  },
+
+  beginEmulation() {
+    return Promise.resolve();
+  },
+
+  cleanAndDisableBrowserCaches() {},
+  forceUpdateServiceWorkers() {},
+  beginTrace() {
+    return Promise.resolve();
+  },
+  endTrace() {
+    return Promise.resolve(
+      require('../fixtures/traces/progressive-app.json')
+    );
+  },
+  beginNetworkCollect() {},
+  endNetworkCollect() {
+    return Promise.resolve();
+  },
+  getSecurityState() {
+    return Promise.resolve({
+      schemeIsCryptographic: true
+    });
+  }
+};

lighthouse-core/test/driver/index.js

@@ -37,34 +37,7 @@ class TestGatherer extends Gather {
   }
 }
 
-const fakeDriver = {
-  connect() {
-    return Promise.resolve();
-  },
-  disconnect() {},
-  gotoURL() {
-    return Promise.resolve();
-  },
-
-  beginEmulation() {
-    return Promise.resolve();
-  },
-
-  cleanAndDisableBrowserCaches() {},
-  forceUpdateServiceWorkers() {},
-  beginTrace() {
-    return Promise.resolve();
-  },
-  endTrace() {
-    return Promise.resolve(
-      require('../fixtures/traces/progressive-app.json')
-    );
-  },
-  beginNetworkCollect() {},
-  endNetworkCollect() {
-    return Promise.resolve();
-  }
-};
+const fakeDriver = require('./fake-driver');
 
 describe('Driver', function() {
   it('loads a page', () => {
@@ -83,6 +56,16 @@ describe('Driver', function() {
     });
   });
 
+  it('creates flags if needed', () => {
+    const url = 'https://example.com';
+    const driver = fakeDriver;
+    const options = {url, driver};
+
+    return Driver.run([], options).then(_ => {
+      assert.equal(typeof options.flags, 'object');
+    });
+  });
+
   it('reloads a page via about:blank', () => {
     const expected = [
       'https://example.com',

lighthouse-core/test/formatter/formatter.js

@@ -28,6 +28,12 @@ describe('Formatter', () => {
     assert.notEqual(Formatter.SUPPORTED_FORMATS, undefined);
   });
 
+  it('returns supported formats when called by name', () => {
+    // Force the internal _formatters to not exist
+    Formatter._formatters = null;
+    assert.notEqual(Formatter.getByName('accessibility'), undefined);
+  });
+
   it('throws when invalid format is provided', () => {
     assert.throws(_ => Formatter.getByName('invalid-format'), Error);
   });