core(runner): split lhr, artifacts return, respect output type

lighthouse-cli/bin.js

@@ -84,7 +84,7 @@ if (cliFlags.extraHeaders) {
 }
 
 /**
- * @return {Promise<LH.Results|void>}
+ * @return {Promise<LH.RunnerResult|void>}
  */
 function run() {
   return Promise.resolve()

lighthouse-cli/printer.js

@@ -6,7 +6,6 @@
 'use strict';
 
 const fs = require('fs');
-const ReportGenerator = require('../lighthouse-core/report/v2/report-generator');
 const log = require('lighthouse-logger');
 
 /**
@@ -34,65 +33,6 @@ function checkOutputPath(path) {
   return path;
 }
 
-/**
- * Converts the results to a CSV formatted string
- * Each row describes the result of 1 audit with
- *  - the name of the category the audit belongs to
- *  - the name of the audit
- *  - a description of the audit
- *  - the score type that is used for the audit
- *  - the score value of the audit
- *
- * @param {LH.Results} results
- * @returns {string}
- */
-function toCSVReport(results) {
-  // To keep things "official" we follow the CSV specification (RFC4180)
-  // The document describes how to deal with escaping commas and quotes etc.
-  const CRLF = '\r\n';
-  const separator = ',';
-  /** @param {string} value @returns {string} */
-  const escape = (value) => `"${value.replace(/"/g, '""')}"`;
-
-
-  // Possible TODO: tightly couple headers and row values
-  const header = ['category', 'name', 'title', 'type', 'score'];
-  const table = results.reportCategories.map(category => {
-    return category.audits.map(catAudit => {
-      const audit = results.audits[catAudit.id];
-      return [category.name, audit.name, audit.description, audit.scoreDisplayMode, audit.score]
-        .map(value => value.toString())
-        .map(escape);
-    });
-  });
-
-  // @ts-ignore TS loses track of type Array
-  const flattedTable = [].concat(...table);
-  return [header, ...flattedTable].map(row => row.join(separator)).join(CRLF);
-}
-
-/**
- * Creates the results output in a format based on the `mode`.
- * @param {!LH.Results} results
- * @param {string} outputMode
- * @return {string}
- */
-function createOutput(results, outputMode) {
-  // HTML report.
-  if (outputMode === OutputMode.html) {
-    return new ReportGenerator().generateReportHtml(results);
-  }
-  // JSON report.
-  if (outputMode === OutputMode.json) {
-    return JSON.stringify(results, null, 2);
-  }
-  // CSV report.
-  if (outputMode === OutputMode.csv) {
-    return toCSVReport(results);
-  }
-  throw new Error('Invalid output mode: ' + outputMode);
-}
-
 /**
  * Writes the output to stdout.
  * @param {string} output
@@ -130,15 +70,15 @@ function writeFile(filePath, output, outputMode) {
 
 /**
  * Writes the results.
- * @param {!LH.Results} results
+ * @param {LH.RunnerResult} results
  * @param {string} mode
  * @param {string} path
- * @return {Promise<LH.Results>}
+ * @return {Promise<LH.RunnerResult>}
  */
 function write(results, mode, path) {
   return new Promise((resolve, reject) => {
     const outputPath = checkOutputPath(path);
-    const output = createOutput(results, mode);
+    const output = results.formattedReport;
 
     if (outputPath === 'stdout') {
       return writeToStdout(output).then(_ => resolve(results));
@@ -161,7 +101,6 @@ function getValidOutputOptions() {
 
 module.exports = {
   checkOutputPath,
-  createOutput,
   write,
   OutputMode,
   getValidOutputOptions,

lighthouse-cli/run.js

@@ -95,12 +95,12 @@ function handleError(err) {
 }
 
 /**
- * @param {!LH.Results} results
- * @param {!Object} artifacts
+ * @param {!LH.RunnerResult} results
  * @param {!LH.Flags} flags
  * @return {Promise<void>}
  */
-function saveResults(results, artifacts, flags) {
+function saveResults(results, flags) {
+  const {lhr, artifacts} = results;
   const cwd = process.cwd();
   let promise = Promise.resolve();
 
@@ -110,12 +110,12 @@ function saveResults(results, artifacts, flags) {
   // Use the output path as the prefix for all generated files.
   // If no output path is set, generate a file prefix using the URL and date.
   const configuredPath = !flags.outputPath || flags.outputPath === 'stdout' ?
-      getFilenamePrefix(results) :
+      getFilenamePrefix(lhr) :
       flags.outputPath.replace(/\.\w{2,4}$/, '');
   const resolvedPath = path.resolve(cwd, configuredPath);
 
   if (flags.saveAssets) {
-    promise = promise.then(_ => assetSaver.saveAssets(artifacts, results.audits, resolvedPath));
+    promise = promise.then(_ => assetSaver.saveAssets(artifacts, lhr.audits, resolvedPath));
   }
 
   return promise.then(_ => {
@@ -149,7 +149,7 @@ function saveResults(results, artifacts, flags) {
  * @param {string} url
  * @param {LH.Flags} flags
  * @param {LH.Config.Json|undefined} config
- * @return {Promise<LH.Results|void>}
+ * @return {Promise<LH.RunnerResult|void>}
  */
 function runLighthouse(url, flags, config) {
   /** @type {!LH.LaunchedChrome} */
@@ -170,9 +170,7 @@ function runLighthouse(url, flags, config) {
     return lighthouse(url, flags, config).then(results => {
       return potentiallyKillChrome().then(_ => results);
     }).then(results => {
-      const artifacts = results.artifacts;
-      delete results.artifacts;
-      return saveResults(results, artifacts, flags).then(_ => results);
+      return saveResults(results, flags).then(_ => results);
     });
   });
 

lighthouse-core/config/config.js

@@ -153,6 +153,8 @@ function merge(base, extension) {
   // If the default value doesn't exist or is explicitly null, defer to the extending value
   if (typeof base === 'undefined' || base === null) {
     return extension;
+  } else if (typeof extension === 'undefined') {
+    return base;
   } else if (Array.isArray(extension)) {
     if (!Array.isArray(base)) throw new TypeError(`Expected array but got ${typeof base}`);
     const merged = base.slice();

lighthouse-core/config/constants.js

@@ -28,6 +28,7 @@ const throttling = {
 
 /** @type {LH.Config.Settings} */
 const defaultSettings = {
+  output: 'json',
   maxWaitForLoad: 45 * 1000,
   throttlingMethod: 'devtools',
   throttling: throttling.mobile3G,

lighthouse-core/index.js

@@ -30,7 +30,7 @@ const Config = require('./config/config');
  * @param {string} url
  * @param {LH.Flags} flags
  * @param {LH.Config.Json|undefined} configJSON
- * @return {Promise<LH.Results>}
+ * @return {Promise<LH.RunnerResult>}
  */
 function lighthouse(url, flags = {}, configJSON) {
   const startTime = Date.now();
@@ -46,10 +46,12 @@ function lighthouse(url, flags = {}, configJSON) {
     // kick off a lighthouse run
     return Runner.run(connection, {url, config})
       .then((lighthouseResults = {}) => {
+        lighthouseResults.lhr = lighthouseResults.lhr || {};
+
         // Annotate with time to run lighthouse.
         const endTime = Date.now();
-        lighthouseResults.timing = lighthouseResults.timing || {};
-        lighthouseResults.timing.total = endTime - startTime;
+        lighthouseResults.lhr.timing = lighthouseResults.lhr.timing || {};
+        lighthouseResults.lhr.timing.total = endTime - startTime;
 
         return lighthouseResults;
       });

lighthouse-core/lib/file-namer.js

@@ -12,12 +12,12 @@
  * Generate a filenamePrefix of hostname_YYYY-MM-DD_HH-MM-SS
  * Date/time uses the local timezone, however Node has unreliable ICU
  * support, so we must construct a YYYY-MM-DD date format manually. :/
- * @param {{url: string, fetchedAt: string}} results
+ * @param {LH.Result} lhr
  * @return {string}
  */
-function getFilenamePrefix(results) {
-  const hostname = new (URLConstructor || URL)(results.url).hostname;
-  const date = (results.fetchedAt && new Date(results.fetchedAt)) || new Date();
+function getFilenamePrefix(lhr) {
+  const hostname = new (URLConstructor || URL)(lhr.url).hostname;
+  const date = (lhr.fetchedAt && new Date(lhr.fetchedAt)) || new Date();
 
   const timeStr = date.toLocaleTimeString('en-US', {hour12: false});
   const dateParts = date.toLocaleDateString('en-US', {

lighthouse-core/lib/format-output.js

@@ -0,0 +1,72 @@
+/**
+ * @license Copyright 2018 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';
+
+const ReportGenerator = require('../report/v2/report-generator');
+
+/**
+ * Converts the results to a CSV formatted string
+ * Each row describes the result of 1 audit with
+ *  - the name of the category the audit belongs to
+ *  - the name of the audit
+ *  - a description of the audit
+ *  - the score type that is used for the audit
+ *  - the score value of the audit
+ *
+ * @param {LH.Result} lhr
+ * @returns {string}
+ */
+function toCSVReport(lhr) {
+  // To keep things "official" we follow the CSV specification (RFC4180)
+  // The document describes how to deal with escaping commas and quotes etc.
+  const CRLF = '\r\n';
+  const separator = ',';
+  /** @param {string} value @returns {string} */
+  const escape = (value) => `"${value.replace(/"/g, '""')}"`;
+
+
+  // Possible TODO: tightly couple headers and row values
+  const header = ['category', 'name', 'title', 'type', 'score'];
+  const table = lhr.reportCategories.map(category => {
+    return category.audits.map(catAudit => {
+      const audit = lhr.audits[catAudit.id];
+      return [category.name, audit.name, audit.description, audit.scoreDisplayMode, audit.score]
+        .map(value => value.toString())
+        .map(escape);
+    });
+  });
+
+  // @ts-ignore TS loses track of type Array
+  const flattedTable = [].concat(...table);
+  return [header, ...flattedTable].map(row => row.join(separator)).join(CRLF);
+}
+
+/**
+ * Creates the results output in a format based on the `mode`.
+ * @param {LH.Result} lhr
+ * @param {string} outputMode
+ * @return {string}
+ */
+function createOutput(lhr, outputMode) {
+  // HTML report.
+  if (outputMode === 'html') {
+    return new ReportGenerator().generateReportHtml(lhr);
+  }
+  // JSON report.
+  if (outputMode === 'json') {
+    return JSON.stringify(lhr, null, 2);
+  }
+  // CSV report.
+  if (outputMode === 'csv') {
+    return toCSVReport(lhr);
+  }
+
+  throw new Error('Invalid output mode: ' + outputMode);
+}
+
+module.exports = {
+  createOutput,
+};

lighthouse-core/runner.js

@@ -17,16 +17,15 @@ const fs = require('fs');
 const path = require('path');
 const URL = require('./lib/url-shim');
 const Sentry = require('./lib/sentry');
+const getFormattedReport = require('./lib/format-output').createOutput;
 
 const Connection = require('./gather/connections/connection.js'); // eslint-disable-line no-unused-vars
 
-/** @typedef {LH.Result & {artifacts: LH.Artifacts}} RunnerResult */
-
 class Runner {
   /**
    * @param {Connection} connection
    * @param {{config: LH.Config, url: string, initialUrl?: string, driverMock?: Driver}} opts
-   * @return {Promise<RunnerResult|undefined>}
+   * @return {Promise<LH.RunnerResult|undefined>}
    */
   static async run(connection, opts) {
     try {
@@ -118,7 +117,8 @@ class Runner {
       if (opts.config.categories) {
         reportCategories = ReportScoring.scoreAllCategories(opts.config.categories, resultsById);
       }
-      return {
+
+      const lhr = {
         userAgent: artifacts.UserAgent,
         lighthouseVersion,
         fetchedAt: artifacts.fetchedAt,
@@ -127,11 +127,13 @@ class Runner {
         url: opts.url,
         runWarnings: lighthouseRunWarnings,
         audits: resultsById,
-        artifacts,
         runtimeConfig: Runner.getRuntimeConfig(settings),
         reportCategories,
         reportGroups: opts.config.groups,
       };
+
+      const formattedReport = getFormattedReport(lhr, settings.output);
+      return {lhr, formattedReport, artifacts};
     } catch (err) {
       // @ts-ignore TODO(bckenny): Sentry type checking
       await Sentry.captureException(err, {level: 'fatal'});

lighthouse-core/test/config/config-test.js

@@ -472,6 +472,11 @@ describe('Config', () => {
     assert.ok(config.settings.disableDeviceEmulation, 'missing setting from flags');
   });
 
+  it('inherits default settings when undefined', () => {
+    const config = new Config({settings: undefined});
+    assert.ok(typeof config.settings.maxWaitForLoad === 'number', 'missing setting from default');
+  });
+
   describe('#extendConfigJSON', () => {
     it('should merge passes', () => {
       const configA = {

lighthouse-core/test/index-test.js

@@ -90,7 +90,7 @@ describe('Module Tests', function() {
   it('should return formatted LHR when given no categories', function() {
     const exampleUrl = 'https://example.com/';
     return lighthouse(exampleUrl, {
-      output: 'json',
+      output: 'html',
     }, {
       settings: {
         auditMode: __dirname + '/fixtures/artifacts/perflog/',
@@ -99,17 +99,19 @@ describe('Module Tests', function() {
         'viewport',
       ],
     }).then(results => {
-      assert.ok(results.lighthouseVersion);
-      assert.ok(results.fetchedAt);
-      assert.equal(results.url, exampleUrl);
-      assert.equal(results.initialUrl, exampleUrl);
-      assert.ok(Array.isArray(results.reportCategories));
-      assert.equal(results.reportCategories.length, 0);
-      assert.ok(results.audits.viewport);
-      assert.strictEqual(results.audits.viewport.score, 0);
-      assert.ok(results.audits.viewport.debugString);
-      assert.ok(results.timing);
-      assert.equal(typeof results.timing.total, 'number');
+      assert.ok(/<html/.test(results.formattedReport), 'did not create html report');
+      assert.ok(results.artifacts.ViewportDimensions, 'did not set artifacts');
+      assert.ok(results.lhr.lighthouseVersion);
+      assert.ok(results.lhr.fetchedAt);
+      assert.equal(results.lhr.url, exampleUrl);
+      assert.equal(results.lhr.initialUrl, exampleUrl);
+      assert.ok(Array.isArray(results.lhr.reportCategories));
+      assert.equal(results.lhr.reportCategories.length, 0);
+      assert.ok(results.lhr.audits.viewport);
+      assert.strictEqual(results.lhr.audits.viewport.score, 0);
+      assert.ok(results.lhr.audits.viewport.debugString);
+      assert.ok(results.lhr.timing);
+      assert.equal(typeof results.lhr.timing.total, 'number');
     });
   });