core(audit-mode): do not require a URL

lighthouse-cli/cli-flags.js

@@ -136,9 +136,13 @@ function getFlags(manualArgv) {
       .default('port', 0)
       .default('hostname', 'localhost')
       .check(/** @param {!LH.Flags} argv */ (argv) => {
-        // Make sure lighthouse has been passed a url, or at least one of --list-all-audits
-        // or --list-trace-categories. If not, stop the program and ask for a url
-        if (!argv.listAllAudits && !argv.listTraceCategories && argv._.length === 0) {
+        // Lighthouse doesn't need a URL if...
+        //   - We're in auditMode (and we have artifacts already)
+        //   - We're just listing the available options.
+        // If one of these don't apply, stop the program and ask for a url.
+        const isListMode = argv.listAllAudits || argv.listTraceCategories;
+        const isOnlyAuditMode = !!argv.auditMode && !argv.gatherMode;
+        if (!isListMode && !isOnlyAuditMode && argv._.length === 0) {
           throw new Error('Please provide a url');
         }
 

lighthouse-core/runner.js

@@ -23,7 +23,7 @@ const Connection = require('./gather/connections/connection.js'); // eslint-disa
 class Runner {
   /**
    * @param {Connection} connection
-   * @param {{config: LH.Config, url: string, driverMock?: Driver}} opts
+   * @param {{config: LH.Config, url?: string, driverMock?: Driver}} opts
    * @return {Promise<LH.RunnerResult|undefined>}
    */
   static async run(connection, opts) {
@@ -37,19 +37,6 @@ class Runner {
        */
       const lighthouseRunWarnings = [];
 
-      // save the requestedUrl provided by the user
-      const rawRequestedUrl = opts.url;
-      if (typeof rawRequestedUrl !== 'string' || rawRequestedUrl.length === 0) {
-        throw new Error(`You must provide a url to the runner. '${rawRequestedUrl}' provided.`);
-      }
-
-      let parsedURL;
-      try {
-        parsedURL = new URL(opts.url);
-      } catch (e) {
-        throw new Error('The url provided should have a proper protocol and hostname.');
-      }
-
       const sentryContext = Sentry.getContext();
       // @ts-ignore TODO(bckenny): Sentry type checking
       Sentry.captureBreadcrumb({
@@ -59,27 +46,38 @@ class Runner {
         data: sentryContext && sentryContext.extra,
       });
 
-      // If the URL isn't https and is also not localhost complain to the user.
-      if (parsedURL.protocol !== 'https:' && parsedURL.hostname !== 'localhost') {
-        log.warn('Lighthouse', 'The URL provided should be on HTTPS');
-        log.warn('Lighthouse', 'Performance stats will be skewed redirecting from HTTP to HTTPS.');
-      }
-
-      // canonicalize URL with any trailing slashes neccessary
-      const requestedUrl = parsedURL.href;
-
       // User can run -G solo, -A solo, or -GA together
       // -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 or from the browser
       let artifacts;
+      let requestedUrl;
       if (settings.auditMode && !settings.gatherMode) {
         // No browser required, just load the artifacts from disk.
         const path = Runner._getArtifactsPath(settings);
         artifacts = await assetSaver.loadArtifacts(path);
+        requestedUrl = artifacts.URL.requestedUrl;
+
+        if (!requestedUrl) {
+          throw new Error('Cannot run audit mode on empty URL');
+        }
+        if (opts.url && opts.url !== requestedUrl) {
+          throw new Error('Cannot run audit mode on different URL');
+        }
       } else {
+        if (typeof opts.url !== 'string' || opts.url.length === 0) {
+          throw new Error(`You must provide a url to the runner. '${opts.url}' provided.`);
+        }
+
+        try {
+          // Use canonicalized URL (with trailing slashes and such)
+          requestedUrl = new URL(opts.url).href;
+        } catch (e) {
+          throw new Error('The url provided should have a proper protocol and hostname.');
+        }
+
         artifacts = await Runner._gatherArtifactsFromBrowser(requestedUrl, opts, connection);
         // -G means save these to ./latest-run, etc.
         if (settings.gatherMode) {

lighthouse-core/scripts/assert-golden-lhr-unchanged.sh

@@ -29,7 +29,7 @@ trap teardown EXIT
 colorText "Generating a fresh LHR..." "$purple"
 set -x
 # TODO(phulce): add a lantern LHR-differ
-node "$lhroot_path/lighthouse-cli" -A="$lhroot_path/lighthouse-core/test/results/artifacts" --throttling-method=devtools --quiet --output=json --output-path="$freshLHRPath" http://localhost/dobetterweb/dbw_tester.html
+node "$lhroot_path/lighthouse-cli" -A="$lhroot_path/lighthouse-core/test/results/artifacts" --throttling-method=devtools --quiet --output=json --output-path="$freshLHRPath"
 set +x
 
 # remove timing from both

lighthouse-core/test/results/sample_v2.json

@@ -2,7 +2,7 @@
   "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",
   "lighthouseVersion": "3.0.0-beta.0",
   "fetchTime": "2018-03-13T00:55:45.840Z",
-  "requestedUrl": "http://localhost/dobetterweb/dbw_tester.html",
+  "requestedUrl": "http://localhost:10200/dobetterweb/dbw_tester.html",
   "finalUrl": "http://localhost:10200/dobetterweb/dbw_tester.html",
   "runWarnings": [],
   "audits": {

lighthouse-core/test/runner-test.js

@@ -72,7 +72,7 @@ describe('Runner', () => {
 
     // uses the files on disk from the -G test. ;)
     it('-A audits from saved artifacts and doesn\'t gather', () => {
-      const opts = {url, config: generateConfig({auditMode: artifactsPath}), driverMock};
+      const opts = {config: generateConfig({auditMode: artifactsPath}), driverMock};
       return Runner.run(null, opts).then(_ => {
         assert.equal(loadArtifactsSpy.called, true, 'loadArtifacts was not called');
         assert.equal(gatherRunnerRunSpy.called, false, 'GatherRunner.run was called');
@@ -83,7 +83,7 @@ describe('Runner', () => {
 
     it('-A throws if the settings change', async () => {
       const settings = {auditMode: artifactsPath, disableDeviceEmulation: true};
-      const opts = {url, config: generateConfig(settings), driverMock};
+      const opts = {config: generateConfig(settings), driverMock};
       try {
         await Runner.run(null, opts);
         assert.fail('should have thrown');
@@ -92,6 +92,17 @@ describe('Runner', () => {
       }
     });
 
+    it('-A throws if the URL changes', async () => {
+      const settings = {auditMode: artifactsPath, disableDeviceEmulation: true};
+      const opts = {url: 'https://example.com', config: generateConfig(settings), driverMock};
+      try {
+        await Runner.run(null, opts);
+        assert.fail('should have thrown');
+      } catch (err) {
+        assert.ok(/different URL/.test(err.message), 'should have prevented run');
+      }
+    });
+
     it('-GA is a normal run but it saves artifacts to disk', () => {
       const settings = {auditMode: artifactsPath, gatherMode: artifactsPath};
       const opts = {url, config: generateConfig(settings), driverMock};
@@ -187,8 +198,6 @@ describe('Runner', () => {
   });
 
   it('accepts trace artifacts as paths and outputs appropriate data', () => {
-    const url = 'https://example.com/';
-
     const config = new Config({
       settings: {
         auditMode: __dirname + '/fixtures/artifacts/perflog/',
@@ -198,7 +207,7 @@ describe('Runner', () => {
       ],
     });
 
-    return Runner.run({}, {url, config}).then(results => {
+    return Runner.run({}, {config}).then(results => {
       const audits = results.lhr.audits;
       assert.equal(audits['user-timings'].displayValue[1], 2);
       assert.equal(audits['user-timings'].rawValue, false);
@@ -233,7 +242,6 @@ describe('Runner', () => {
 
   describe('Bad required artifact handling', () => {
     it('outputs an error audit result when trace required but not provided', () => {
-      const url = 'https://example.com';
       const config = new Config({
         settings: {
           auditMode: __dirname + '/fixtures/artifacts/empty-artifacts/',
@@ -244,7 +252,7 @@ describe('Runner', () => {
         ],
       });
 
-      return Runner.run({}, {url, config}).then(results => {
+      return Runner.run({}, {config}).then(results => {
         const auditResult = results.lhr.audits['user-timings'];
         assert.strictEqual(auditResult.rawValue, null);
         assert.strictEqual(auditResult.scoreDisplayMode, 'error');
@@ -253,7 +261,6 @@ describe('Runner', () => {
     });
 
     it('outputs an error audit result when missing a required artifact', () => {
-      const url = 'https://example.com';
       const config = new Config({
         settings: {
           auditMode: __dirname + '/fixtures/artifacts/empty-artifacts/',
@@ -264,7 +271,7 @@ describe('Runner', () => {
         ],
       });
 
-      return Runner.run({}, {url, config}).then(results => {
+      return Runner.run({}, {config}).then(results => {
         const auditResult = results.lhr.audits['content-width'];
         assert.strictEqual(auditResult.rawValue, null);
         assert.strictEqual(auditResult.scoreDisplayMode, 'error');
@@ -312,7 +319,6 @@ describe('Runner', () => {
 
     it('produces an error audit result when an audit throws a non-fatal Error', () => {
       const errorMessage = 'Audit yourself';
-      const url = 'https://example.com';
       const config = new Config({
         settings: {
           auditMode: __dirname + '/fixtures/artifacts/empty-artifacts/',
@@ -329,7 +335,7 @@ describe('Runner', () => {
         ],
       });
 
-      return Runner.run({}, {url, config}).then(results => {
+      return Runner.run({}, {config}).then(results => {
         const auditResult = results.lhr.audits['throwy-audit'];
         assert.strictEqual(auditResult.rawValue, null);
         assert.strictEqual(auditResult.scoreDisplayMode, 'error');
@@ -339,7 +345,6 @@ describe('Runner', () => {
 
     it('rejects if an audit throws a fatal error', () => {
       const errorMessage = 'Uh oh';
-      const url = 'https://example.com';
       const config = new Config({
         settings: {
           auditMode: __dirname + '/fixtures/artifacts/empty-artifacts/',
@@ -358,14 +363,13 @@ describe('Runner', () => {
         ],
       });
 
-      return Runner.run({}, {url, config}).then(
+      return Runner.run({}, {config}).then(
         _ => assert.ok(false),
         err => assert.strictEqual(err.message, errorMessage));
     });
   });
 
   it('accepts devtoolsLog in artifacts', () => {
-    const url = 'https://example.com';
     const config = new Config({
       settings: {
         auditMode: __dirname + '/fixtures/artifacts/perflog/',
@@ -375,7 +379,7 @@ describe('Runner', () => {
       ],
     });
 
-    return Runner.run({}, {url, config}).then(results => {
+    return Runner.run({}, {config}).then(results => {
       const audits = results.lhr.audits;
       assert.equal(audits['critical-request-chains'].displayValue, '5 chains found');
       assert.equal(audits['critical-request-chains'].rawValue, false);
@@ -487,7 +491,6 @@ describe('Runner', () => {
   });
 
   it('results include artifacts when given artifacts and audits', () => {
-    const url = 'https://example.com';
     const config = new Config({
       settings: {
         auditMode: __dirname + '/fixtures/artifacts/perflog/',
@@ -497,7 +500,7 @@ describe('Runner', () => {
       ],
     });
 
-    return Runner.run({}, {url, config}).then(results => {
+    return Runner.run({}, {config}).then(results => {
       assert.strictEqual(results.artifacts.ViewportDimensions.innerWidth, 412);
       assert.strictEqual(results.artifacts.ViewportDimensions.innerHeight, 732);
     });
@@ -528,15 +531,14 @@ describe('Runner', () => {
   });
 
   it('includes any LighthouseRunWarnings from artifacts in output', () => {
-    const url = 'https://example.com';
     const config = new Config({
       settings: {
         auditMode: __dirname + '/fixtures/artifacts/perflog/',
       },
       audits: [],
     });
 
-    return Runner.run(null, {url, config, driverMock}).then(results => {
+    return Runner.run(null, {config, driverMock}).then(results => {
       assert.deepStrictEqual(results.lhr.runWarnings, [
         'I\'m a warning!',
         'Also a warning',

package.json

@@ -59,7 +59,7 @@
     "changelog": "conventional-changelog --config ./build/changelog-generator/index.js --infile changelog.md --same-file",
     "type-check": "tsc -p . && cd ./lighthouse-viewer && yarn type-check",
     "update:sample-artifacts": "node lighthouse-core/scripts/update-report-fixtures.js -G",
-    "update:sample-json": "node ./lighthouse-cli -A=./lighthouse-core/test/results/artifacts --throttling-method=devtools --output=json --output-path=./lighthouse-core/test/results/sample_v2.json http://localhost/dobetterweb/dbw_tester.html && node lighthouse-core/scripts/cleanup-LHR-for-diff.js ./lighthouse-core/test/results/sample_v2.json --only-remove-timing",
+    "update:sample-json": "node ./lighthouse-cli -A=./lighthouse-core/test/results/artifacts --throttling-method=devtools --output=json --output-path=./lighthouse-core/test/results/sample_v2.json && node lighthouse-core/scripts/cleanup-LHR-for-diff.js ./lighthouse-core/test/results/sample_v2.json --only-remove-timing",
     "diff:sample-json": "bash lighthouse-core/scripts/assert-golden-lhr-unchanged.sh",
     "update:crdp-typings": "node lighthouse-core/scripts/extract-crdp-mapping.js",
     "mixed-content": "./lighthouse-cli/index.js --chrome-flags='--headless' --config-path=./lighthouse-core/config/mixed-content.js",