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

…eChrome#4999)

docs/hacking-tips.md

@@ -41,7 +41,7 @@ node generate_report_v2.js > temp.report.html; open temp.report.html
 
 const ReportGeneratorV2 = require('./lighthouse-core/report/v2/report-generator');
 const results = require('./temp.report.json');
-const html = new ReportGeneratorV2().generateReportHtml(results);
+const html = ReportGeneratorV2.generateReportHtml(results);
 
 console.log(html);
 ```
@@ -64,7 +64,7 @@ su - travis
 ```
 
 ```sh
-# you may also want to mount a local folder into your docker instance. 
+# you may also want to mount a local folder into your docker instance.
 # This will mount your local machines's ~/temp/trav folder into the container's /home/travis/mountpoint folder
 docker run -v $HOME/temp/trav:/home/travis/mountpoint --name travis-debug -dit travisci/ci-garnet:packer-1496954857 /sbin/init
 

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.report;
 
     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} runnerResult
  * @param {!LH.Flags} flags
  * @return {Promise<void>}
  */
-function saveResults(results, artifacts, flags) {
+function saveResults(runnerResult, flags) {
+  const {lhr, artifacts} = runnerResult;
   const cwd = process.cwd();
   let promise = Promise.resolve();
 
@@ -110,26 +110,26 @@ 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(_ => {
     if (Array.isArray(flags.output)) {
       return flags.output.reduce((innerPromise, outputType) => {
         const extension = outputType;
         const outputPath = `${resolvedPath}.report.${extension}`;
-        return innerPromise.then(() => Printer.write(results, outputType, outputPath));
+        return innerPromise.then(() => Printer.write(runnerResult, outputType, outputPath));
       }, Promise.resolve());
     } else {
       const extension = flags.output;
       const outputPath =
           flags.outputPath || `${resolvedPath}.report.${extension}`;
-      return Printer.write(results, flags.output, outputPath).then(_ => {
+      return Printer.write(runnerResult, flags.output, outputPath).then(_ => {
         if (flags.output === Printer.OutputMode[Printer.OutputMode.html]) {
           if (flags.view) {
             opn(outputPath, {wait: false});
@@ -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} */
@@ -167,12 +167,10 @@ function runLighthouse(url, flags, config) {
   }
 
   const resultsP = chromeP.then(_ => {
-    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 lighthouse(url, flags, config).then(runnerResult => {
+      return potentiallyKillChrome().then(_ => runnerResult);
+    }).then(runnerResult => {
+      return saveResults(runnerResult, flags).then(_ => runnerResult);
     });
   });
 

lighthouse-cli/test/cli/printer-test.js

@@ -10,7 +10,6 @@ const Printer = require('../../printer.js');
 const assert = require('assert');
 const fs = require('fs');
 const sampleResults = require('../../../lighthouse-core/test/results/sample_v2.json');
-const csvValidator = require('csv-validator');
 
 describe('Printer', () => {
   it('accepts valid output paths', () => {
@@ -23,68 +22,24 @@ describe('Printer', () => {
     assert.notEqual(Printer.checkOutputPath(path), path);
   });
 
-  it('creates JSON for results', () => {
-    const mode = Printer.OutputMode.json;
-    const jsonOutput = Printer.createOutput(sampleResults, mode);
-    assert.doesNotThrow(_ => JSON.parse(jsonOutput));
-  });
-
-  it('creates HTML for results', () => {
-    const mode = Printer.OutputMode.html;
-    const htmlOutput = Printer.createOutput(sampleResults, mode);
-    assert.ok(/<!doctype/gim.test(htmlOutput));
-    assert.ok(/<html lang="en"/gim.test(htmlOutput));
-  });
-
-  it('creates CSV for results', async () => {
-    const mode = Printer.OutputMode.csv;
-    const path = './.results-as-csv.csv';
-    const headers = {
-      category: '',
-      name: '',
-      title: '',
-      type: '',
-      score: 42,
-    };
-    await Printer.write(sampleResults, mode, path);
-
-    try {
-      await csvValidator(path, headers);
-    } catch (err) {
-      assert.fail('CSV parser error:\n' + err.join('\n'));
-    } finally {
-      fs.unlinkSync(path);
-    }
-  });
-
   it('writes file for results', () => {
-    const mode = 'html';
-    const path = './.test-file.html';
-
-    // Now do a second pass where the file is written out.
-    return Printer.write(sampleResults, mode, path).then(_ => {
+    const path = './.test-file.json';
+    const report = JSON.stringify(sampleResults);
+    return Printer.write({report}, 'json', path).then(_ => {
       const fileContents = fs.readFileSync(path, 'utf8');
-      assert.ok(/<!doctype/gim.test(fileContents));
+      assert.ok(/lighthouseVersion/gim.test(fileContents));
       fs.unlinkSync(path);
     });
   });
 
   it('throws for invalid paths', () => {
-    const mode = 'html';
-    const path = '!/#@.html';
-    return Printer.write(sampleResults, mode, path).catch(err => {
+    const path = '!/#@.json';
+    const report = JSON.stringify(sampleResults);
+    return Printer.write({report}, 'html', path).catch(err => {
       assert.ok(err.code === 'ENOENT');
     });
   });
 
-  it('writes extended info', () => {
-    const mode = Printer.OutputMode.html;
-    const htmlOutput = Printer.createOutput(sampleResults, mode);
-    const outputCheck = new RegExp('dobetterweb/dbw_tester.css', 'i');
-
-    assert.ok(outputCheck.test(htmlOutput));
-  });
-
   it('returns output modes', () => {
     const modes = Printer.getValidOutputOptions();
     assert.ok(Array.isArray(modes));

lighthouse-cli/test/cli/run-test.js

@@ -28,18 +28,19 @@ describe('CLI run', function() {
     const timeoutFlag = `--max-wait-for-load=${9000}`;
     const flags = getFlags(`--output=json --output-path=${filename} ${timeoutFlag} ${url}`);
     return run.runLighthouse(url, flags, fastConfig).then(passedResults => {
+      const {lhr} = passedResults;
       assert.ok(fs.existsSync(filename));
       const results = JSON.parse(fs.readFileSync(filename, 'utf-8'));
       assert.equal(results.audits.viewport.rawValue, false);
 
       // passed results match saved results
-      assert.strictEqual(results.fetchedAt, passedResults.fetchedAt);
-      assert.strictEqual(results.url, passedResults.url);
-      assert.strictEqual(results.audits.viewport.rawValue, passedResults.audits.viewport.rawValue);
+      assert.strictEqual(results.fetchedAt, lhr.fetchedAt);
+      assert.strictEqual(results.url, lhr.url);
+      assert.strictEqual(results.audits.viewport.rawValue, lhr.audits.viewport.rawValue);
       assert.strictEqual(
           Object.keys(results.audits).length,
-          Object.keys(passedResults.audits).length);
-      assert.deepStrictEqual(results.timing, passedResults.timing);
+          Object.keys(lhr.audits).length);
+      assert.deepStrictEqual(results.timing, lhr.timing);
 
       fs.unlinkSync(filename);
     });

lighthouse-core/closure/typedefs/viewer-externs.js

@@ -20,4 +20,4 @@ function ReportGenerator() {}
  * @param {!ReportRenderer.ReportJSON} reportJson
  * @return {string}
  */
-ReportGenerator.prototype.generateReportHtml = function(reportJson) {};
+ReportGenerator.generateReportHtml = function(reportJson) {};

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,10 +30,9 @@ 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();
   return Promise.resolve().then(_ => {
     // set logging preferences, assume quiet
     flags.logLevel = flags.logLevel || 'error';
@@ -44,15 +43,7 @@ function lighthouse(url, flags = {}, configJSON) {
     const connection = new ChromeProtocol(flags.port, flags.hostname);
 
     // kick off a lighthouse run
-    return Runner.run(connection, {url, config})
-      .then((lighthouseResults = {}) => {
-        // Annotate with time to run lighthouse.
-        const endTime = Date.now();
-        lighthouseResults.timing = lighthouseResults.timing || {};
-        lighthouseResults.timing.total = endTime - startTime;
-
-        return lighthouseResults;
-      });
+    return Runner.run(connection, {url, config});
   });
 }
 

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 {{url: string, fetchedAt: string}} 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', {