From 6c6b0b14a31547a965292e6ce46eeea0ed716eb6 Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Thu, 19 Apr 2018 14:10:22 -0700 Subject: [PATCH 1/6] core(runner): respect output setting --- lighthouse-cli/bin.js | 2 +- lighthouse-cli/printer.js | 67 +------------------- lighthouse-cli/run.js | 16 +++-- lighthouse-core/config/config.js | 2 + lighthouse-core/config/constants.js | 1 + lighthouse-core/index.js | 8 ++- lighthouse-core/lib/file-namer.js | 8 +-- lighthouse-core/lib/format-output.js | 72 ++++++++++++++++++++++ lighthouse-core/runner.js | 12 ++-- lighthouse-core/test/config/config-test.js | 5 ++ lighthouse-core/test/index-test.js | 26 ++++---- lighthouse-core/test/runner-test.js | 40 ++++++------ typings/externs.d.ts | 13 ++-- 13 files changed, 146 insertions(+), 126 deletions(-) create mode 100644 lighthouse-core/lib/format-output.js diff --git a/lighthouse-cli/bin.js b/lighthouse-cli/bin.js index 93703cc674cf..a8bd557c2161 100644 --- a/lighthouse-cli/bin.js +++ b/lighthouse-cli/bin.js @@ -84,7 +84,7 @@ if (cliFlags.extraHeaders) { } /** - * @return {Promise} + * @return {Promise} */ function run() { return Promise.resolve() diff --git a/lighthouse-cli/printer.js b/lighthouse-cli/printer.js index 23f1988b4c09..e8bd0c2c1a8b 100644 --- a/lighthouse-cli/printer.js +++ b/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} + * @return {Promise} */ 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, diff --git a/lighthouse-cli/run.js b/lighthouse-cli/run.js index 95b276a372e1..b9bf2ef8eb48 100644 --- a/lighthouse-cli/run.js +++ b/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} */ -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} + * @return {Promise} */ 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); }); }); diff --git a/lighthouse-core/config/config.js b/lighthouse-core/config/config.js index 6cd53996ea43..4f2a891d7b0b 100644 --- a/lighthouse-core/config/config.js +++ b/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(); diff --git a/lighthouse-core/config/constants.js b/lighthouse-core/config/constants.js index 9836b5344086..a667d3efc983 100644 --- a/lighthouse-core/config/constants.js +++ b/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, diff --git a/lighthouse-core/index.js b/lighthouse-core/index.js index b5128da9f3de..2b2a2e1ee44a 100644 --- a/lighthouse-core/index.js +++ b/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} + * @return {Promise} */ 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; }); diff --git a/lighthouse-core/lib/file-namer.js b/lighthouse-core/lib/file-namer.js index d455dd4feadd..a2b25b7d0c1f 100644 --- a/lighthouse-core/lib/file-namer.js +++ b/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', { diff --git a/lighthouse-core/lib/format-output.js b/lighthouse-core/lib/format-output.js new file mode 100644 index 000000000000..ec50975ba59a --- /dev/null +++ b/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, +}; diff --git a/lighthouse-core/runner.js b/lighthouse-core/runner.js index 358bd0feafd1..fb02c41d4f18 100644 --- a/lighthouse-core/runner.js +++ b/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} + * @return {Promise} */ 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'}); diff --git a/lighthouse-core/test/config/config-test.js b/lighthouse-core/test/config/config-test.js index d5a69bdbf9f2..e47d1c81b269 100644 --- a/lighthouse-core/test/config/config-test.js +++ b/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 = { diff --git a/lighthouse-core/test/index-test.js b/lighthouse-core/test/index-test.js index b2fd7765cd56..7a6cf63015b2 100644 --- a/lighthouse-core/test/index-test.js +++ b/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(/ { }); return Runner.run({}, {url, config}).then(results => { - assert.equal(results.initialUrl, url); - assert.equal(results.audits['eavesdrop-audit'].rawValue, true); + assert.equal(results.lhr.initialUrl, url); + assert.equal(results.lhr.audits['eavesdrop-audit'].rawValue, true); // assert that the options we received matched expectations assert.deepEqual(calls, [{x: 1}, {x: 2}]); }); @@ -199,7 +199,7 @@ describe('Runner', () => { }); return Runner.run({}, {url, config}).then(results => { - const audits = results.audits; + const audits = results.lhr.audits; assert.equal(audits['user-timings'].displayValue, 2); assert.equal(audits['user-timings'].rawValue, false); }); @@ -245,7 +245,7 @@ describe('Runner', () => { }); return Runner.run({}, {url, config}).then(results => { - const auditResult = results.audits['user-timings']; + const auditResult = results.lhr.audits['user-timings']; assert.strictEqual(auditResult.rawValue, null); assert.strictEqual(auditResult.error, true); assert.ok(auditResult.debugString.includes('traces')); @@ -265,7 +265,7 @@ describe('Runner', () => { }); return Runner.run({}, {url, config}).then(results => { - const auditResult = results.audits['content-width']; + const auditResult = results.lhr.audits['content-width']; assert.strictEqual(auditResult.rawValue, null); assert.strictEqual(auditResult.error, true); assert.ok(auditResult.debugString.includes('ViewportDimensions')); @@ -293,7 +293,7 @@ describe('Runner', () => { config.artifacts.ViewportDimensions = artifactError; return Runner.run({}, {url, config}).then(results => { - const auditResult = results.audits['content-width']; + const auditResult = results.lhr.audits['content-width']; assert.strictEqual(auditResult.rawValue, null); assert.strictEqual(auditResult.error, true); assert.ok(auditResult.debugString.includes(errorMessage)); @@ -330,7 +330,7 @@ describe('Runner', () => { }); return Runner.run({}, {url, config}).then(results => { - const auditResult = results.audits['throwy-audit']; + const auditResult = results.lhr.audits['throwy-audit']; assert.strictEqual(auditResult.rawValue, null); assert.strictEqual(auditResult.error, true); assert.ok(auditResult.debugString.includes(errorMessage)); @@ -376,7 +376,7 @@ describe('Runner', () => { }); return Runner.run({}, {url, config}).then(results => { - const audits = results.audits; + const audits = results.lhr.audits; assert.equal(audits['critical-request-chains'].displayValue, '5 chains found'); assert.equal(audits['critical-request-chains'].rawValue, false); }); @@ -410,11 +410,11 @@ describe('Runner', () => { }); return Runner.run(null, {url, config, driverMock}).then(results => { - assert.ok(results.lighthouseVersion); - assert.ok(results.fetchedAt); - assert.equal(results.initialUrl, url); + assert.ok(results.lhr.lighthouseVersion); + assert.ok(results.lhr.fetchedAt); + assert.equal(results.lhr.initialUrl, url); assert.equal(gatherRunnerRunSpy.called, true, 'GatherRunner.run was not called'); - assert.equal(results.audits['content-width'].name, 'content-width'); + assert.equal(results.lhr.audits['content-width'].name, 'content-width'); }); }); @@ -440,14 +440,14 @@ describe('Runner', () => { }); return Runner.run(null, {url, config, driverMock}).then(results => { - assert.ok(results.lighthouseVersion); - assert.ok(results.fetchedAt); - assert.equal(results.initialUrl, url); + assert.ok(results.lhr.lighthouseVersion); + assert.ok(results.lhr.fetchedAt); + assert.equal(results.lhr.initialUrl, url); assert.equal(gatherRunnerRunSpy.called, true, 'GatherRunner.run was not called'); - assert.equal(results.audits['content-width'].name, 'content-width'); - assert.equal(results.audits['content-width'].score, 1); - assert.equal(results.reportCategories[0].score, 1); - assert.equal(results.reportCategories[0].audits[0].id, 'content-width'); + assert.equal(results.lhr.audits['content-width'].name, 'content-width'); + assert.equal(results.lhr.audits['content-width'].score, 1); + assert.equal(results.lhr.reportCategories[0].score, 1); + assert.equal(results.lhr.reportCategories[0].audits[0].id, 'content-width'); }); }); @@ -537,7 +537,7 @@ describe('Runner', () => { }); return Runner.run(null, {url, config, driverMock}).then(results => { - assert.deepStrictEqual(results.runWarnings, [ + assert.deepStrictEqual(results.lhr.runWarnings, [ 'I\'m a warning!', 'Also a warning', ]); diff --git a/typings/externs.d.ts b/typings/externs.d.ts index d7396097474d..ca3ae6d321cd 100644 --- a/typings/externs.d.ts +++ b/typings/externs.d.ts @@ -34,6 +34,7 @@ declare global { } interface SharedFlagsSettings { + output?: 'json' | 'html' | 'csv'; maxWaitForLoad?: number; blockedUrlPatterns?: string[] | null; additionalTraceCategories?: string | null; @@ -70,14 +71,10 @@ declare global { extraHeaders?: string; } - export interface Results { - url: string; - audits: Audit.Results; - lighthouseVersion: string; - artifacts?: Object; - initialUrl: string; - fetchedAt: string; - reportCategories: ReportCategory[]; + export interface RunnerResult { + lhr: Result; + formattedReport: string; + artifacts: Artifacts; } export interface ReportCategory { From 774d3d3fdfe9e5f826bbfcd6e002bad116b60b16 Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Fri, 20 Apr 2018 09:48:04 -0700 Subject: [PATCH 2/6] feedback --- lighthouse-cli/run.js | 18 ++--- lighthouse-core/index.js | 10 +-- lighthouse-core/lib/format-output.js | 72 ------------------- lighthouse-core/report/v2/report-formatter.js | 33 +++++++++ lighthouse-core/report/v2/report-generator.js | 50 ++++++++++--- lighthouse-core/runner.js | 4 +- .../app/src/lighthouse-ext-background.js | 35 ++++----- lighthouse-extension/gulpfile.js | 5 ++ 8 files changed, 108 insertions(+), 119 deletions(-) delete mode 100644 lighthouse-core/lib/format-output.js create mode 100644 lighthouse-core/report/v2/report-formatter.js diff --git a/lighthouse-cli/run.js b/lighthouse-cli/run.js index b9bf2ef8eb48..32e47e83ca37 100644 --- a/lighthouse-cli/run.js +++ b/lighthouse-cli/run.js @@ -95,12 +95,12 @@ function handleError(err) { } /** - * @param {!LH.RunnerResult} results + * @param {!LH.RunnerResult} runnerResult * @param {!LH.Flags} flags * @return {Promise} */ -function saveResults(results, flags) { - const {lhr, artifacts} = results; +function saveResults(runnerResult, flags) { + const {lhr, artifacts} = runnerResult; const cwd = process.cwd(); let promise = Promise.resolve(); @@ -123,13 +123,13 @@ function saveResults(results, flags) { 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}); @@ -167,10 +167,10 @@ function runLighthouse(url, flags, config) { } const resultsP = chromeP.then(_ => { - return lighthouse(url, flags, config).then(results => { - return potentiallyKillChrome().then(_ => results); - }).then(results => { - return saveResults(results, flags).then(_ => results); + return lighthouse(url, flags, config).then(runnerResult => { + return potentiallyKillChrome().then(_ => runnerResult); + }).then(runnerResult => { + return saveResults(runnerResult, flags).then(_ => runnerResult); }); }); diff --git a/lighthouse-core/index.js b/lighthouse-core/index.js index 2b2a2e1ee44a..8610c171c8ac 100644 --- a/lighthouse-core/index.js +++ b/lighthouse-core/index.js @@ -45,15 +45,15 @@ function lighthouse(url, flags = {}, configJSON) { // kick off a lighthouse run return Runner.run(connection, {url, config}) - .then((lighthouseResults = {}) => { - lighthouseResults.lhr = lighthouseResults.lhr || {}; + .then((runnerResult = {}) => { + runnerResult.lhr = runnerResult.lhr || {}; // Annotate with time to run lighthouse. const endTime = Date.now(); - lighthouseResults.lhr.timing = lighthouseResults.lhr.timing || {}; - lighthouseResults.lhr.timing.total = endTime - startTime; + runnerResult.lhr.timing = runnerResult.lhr.timing || {}; + runnerResult.lhr.timing.total = endTime - startTime; - return lighthouseResults; + return runnerResult; }); }); } diff --git a/lighthouse-core/lib/format-output.js b/lighthouse-core/lib/format-output.js deleted file mode 100644 index ec50975ba59a..000000000000 --- a/lighthouse-core/lib/format-output.js +++ /dev/null @@ -1,72 +0,0 @@ -/** - * @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, -}; diff --git a/lighthouse-core/report/v2/report-formatter.js b/lighthouse-core/report/v2/report-formatter.js new file mode 100644 index 000000000000..46b3f76d9c2c --- /dev/null +++ b/lighthouse-core/report/v2/report-formatter.js @@ -0,0 +1,33 @@ +/** + * @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-generator'); + +module.exports = { + /** + * Creates the results output in a format based on the `mode`. + * @param {LH.Result} lhr + * @param {string} outputMode + * @return {string} + */ + formatReport(lhr, outputMode) { + // HTML report. + if (outputMode === 'html') { + return ReportGenerator.generateReportHtml(lhr); + } + // CSV report. + if (outputMode === 'csv') { + return ReportGenerator.generateReportCSV(lhr); + } + // JSON report. + if (outputMode === 'json') { + return JSON.stringify(lhr, null, 2); + } + + throw new Error('Invalid output mode: ' + outputMode); + }, +}; diff --git a/lighthouse-core/report/v2/report-generator.js b/lighthouse-core/report/v2/report-generator.js index 26203aa89d74..6da295d69820 100644 --- a/lighthouse-core/report/v2/report-generator.js +++ b/lighthouse-core/report/v2/report-generator.js @@ -45,7 +45,6 @@ class ReportGeneratorV2 { return REPORT_TEMPLATES; } - /** * Replaces all the specified strings in source without serial replacements. * @param {string} source @@ -60,19 +59,18 @@ class ReportGeneratorV2 { const firstReplacement = replacements[0]; const nextReplacements = replacements.slice(1); return source - .split(firstReplacement.search) - .map(part => ReportGeneratorV2.replaceStrings(part, nextReplacements)) - .join(firstReplacement.replacement); + .split(firstReplacement.search) + .map(part => ReportGeneratorV2.replaceStrings(part, nextReplacements)) + .join(firstReplacement.replacement); } - /** * Returns the report HTML as a string with the report JSON and renderer JS inlined. - * @param {!Object} reportAsJson + * @param {LH.Result} lhr * @return {string} */ - generateReportHtml(reportAsJson) { - const sanitizedJson = JSON.stringify(reportAsJson) + static generateReportHtml(lhr) { + const sanitizedJson = JSON.stringify(lhr) .replace(/ `"${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); + } } module.exports = ReportGeneratorV2; diff --git a/lighthouse-core/runner.js b/lighthouse-core/runner.js index fb02c41d4f18..c39228611fcc 100644 --- a/lighthouse-core/runner.js +++ b/lighthouse-core/runner.js @@ -17,7 +17,7 @@ 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 formatReport = require('./report/v2/report-formatter').formatReport; const Connection = require('./gather/connections/connection.js'); // eslint-disable-line no-unused-vars @@ -132,7 +132,7 @@ class Runner { reportGroups: opts.config.groups, }; - const formattedReport = getFormattedReport(lhr, settings.output); + const formattedReport = formatReport(lhr, settings.output); return {lhr, formattedReport, artifacts}; } catch (err) { // @ts-ignore TODO(bckenny): Sentry type checking diff --git a/lighthouse-extension/app/src/lighthouse-ext-background.js b/lighthouse-extension/app/src/lighthouse-ext-background.js index 97b24b2de54d..f1b3168c2f72 100644 --- a/lighthouse-extension/app/src/lighthouse-ext-background.js +++ b/lighthouse-extension/app/src/lighthouse-ext-background.js @@ -11,8 +11,6 @@ const ExtensionProtocol = require('../../../lighthouse-core/gather/connections/e const log = require('lighthouse-logger'); const assetSaver = require('../../../lighthouse-core/lib/asset-saver.js'); -const ReportGeneratorV2 = require('../../../lighthouse-core/report/v2/report-generator'); - const STORAGE_KEY = 'lighthouse_audits'; const SETTINGS_KEY = 'lighthouse_settings'; @@ -71,15 +69,6 @@ function updateBadgeUI(optUrl) { } } -/** - * Removes artifacts from the result object for portability - * @param {!Object} result Lighthouse results object - */ -function filterOutArtifacts(result) { - // strip them out, as the networkRecords artifact has circular structures - result.artifacts = undefined; -} - /** * @param {!Object} options Lighthouse options. * @param {!Array} categoryIDs Name values of categories to include. @@ -89,15 +78,16 @@ window.runLighthouseInExtension = function(options, categoryIDs) { // Default to 'info' logging level. log.setLevel('info'); const connection = new ExtensionProtocol(); + options.flags = Object.assign({}, options.flags, {output: 'html'}); + // return enableOtherChromeExtensions(false) // .then(_ => connection.getCurrentTabURL()) return connection.getCurrentTabURL() .then(url => window.runLighthouseForConnection(connection, url, options, categoryIDs, updateBadgeUI)) - .then(results => { - filterOutArtifacts(results); + .then(runnerResult => { // return enableOtherChromeExtensions(true).then(_ => { - const blobURL = window.createReportPageAsBlob(results, 'extension'); + const blobURL = window.createReportPageAsBlob(runnerResult, 'extension'); chrome.windows.create({url: blobURL}); // }); }).catch(err => { @@ -112,39 +102,38 @@ window.runLighthouseInExtension = function(options, categoryIDs) { * @param {!Connection} connection * @param {string} url * @param {!Object} options Lighthouse options. - Specify lightriderFormat to change the output format. + Specify outputFormat to change the output format. * @param {!Array} categoryIDs Name values of categories to include. * @return {!Promise} */ window.runLighthouseAsInCLI = function(connection, url, options, categoryIDs) { log.setLevel('info'); const startTime = Date.now(); + options.flags = Object.assign({}, options.flags, {output: options.outputFormat}); + return window.runLighthouseForConnection(connection, url, options, categoryIDs) .then(results => { const endTime = Date.now(); results.timing = {total: endTime - startTime}; let promise = Promise.resolve(); if (options && options.logAssets) { - promise = promise.then(_ => assetSaver.logAssets(results.artifacts, results.audits)); + promise = promise.then(_ => assetSaver.logAssets(results.artifacts, results.lhr.audits)); } return promise.then( _ => { - filterOutArtifacts(results); - const json = options && options.outputFormat === 'json'; - return json ? JSON.stringify(results) : new ReportGeneratorV2().generateReportHtml(results); + return results.formattedReport; }); }); }; /** - * @param {!Object} results Lighthouse results object + * @param {LH.RunnerResult} runnerResult Lighthouse results object * @param {!string} reportContext Where the report is going * @return {!string} Blob URL of the report (or error page) HTML */ -window.createReportPageAsBlob = function(results) { +window.createReportPageAsBlob = function(runnerResult) { performance.mark('report-start'); - const html = new ReportGeneratorV2().generateReportHtml(results); - + const html = runnerResult.formattedReport; const blob = new Blob([html], {type: 'text/html'}); const blobURL = window.URL.createObjectURL(blob); diff --git a/lighthouse-extension/gulpfile.js b/lighthouse-extension/gulpfile.js index 45f3b7cc96f8..d9b92914c9c3 100644 --- a/lighthouse-extension/gulpfile.js +++ b/lighthouse-extension/gulpfile.js @@ -123,6 +123,11 @@ gulp.task('browserify-lighthouse', () => { .ignore('rimraf') .ignore('pako/lib/zlib/inflate.js'); + // Prevent the DevTools/LR background script from getting the HTML report generator. + if (/lighthouse-background/.test(file.path)) { + bundle.ignore(require.resolve('../lighthouse-core/report/v2/report-generator.js')); + } + // Expose the audits, gatherers, and computed artifacts so they can be dynamically loaded. const corePath = '../lighthouse-core/'; const driverPath = `${corePath}gather/`; From f6e9dbc087d7fcb2b0808c5baaadf970f7558bcc Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Fri, 20 Apr 2018 10:55:19 -0700 Subject: [PATCH 3/6] feedback --- docs/hacking-tips.md | 4 +- lighthouse-cli/test/cli/printer-test.js | 59 +++---------------- lighthouse-cli/test/cli/run-test.js | 11 ++-- .../test/smokehouse/perf/expectations.js | 9 +-- .../closure/typedefs/viewer-externs.js | 2 +- lighthouse-core/index.js | 2 + lighthouse-core/lib/file-namer.js | 2 +- lighthouse-core/runner.js | 4 +- .../test/report/v2/report-formatter-test.js | 54 +++++++++++++++++ .../test/report/v2/report-generator-test.js | 10 ++-- .../app/src/viewer-ui-features.js | 2 +- 11 files changed, 81 insertions(+), 78 deletions(-) create mode 100644 lighthouse-core/test/report/v2/report-formatter-test.js diff --git a/docs/hacking-tips.md b/docs/hacking-tips.md index 04dec4441e53..f20101e3a744 100644 --- a/docs/hacking-tips.md +++ b/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 diff --git a/lighthouse-cli/test/cli/printer-test.js b/lighthouse-cli/test/cli/printer-test.js index 523f687b0606..c3e0a5932840 100644 --- a/lighthouse-cli/test/cli/printer-test.js +++ b/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(/ { - 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 formattedReport = JSON.stringify(sampleResults); + return Printer.write({formattedReport}, 'json', path).then(_ => { const fileContents = fs.readFileSync(path, 'utf8'); - assert.ok(/ { - const mode = 'html'; - const path = '!/#@.html'; - return Printer.write(sampleResults, mode, path).catch(err => { + const path = '!/#@.json'; + const formattedReport = JSON.stringify(sampleResults); + return Printer.write({formattedReport}, '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)); diff --git a/lighthouse-cli/test/cli/run-test.js b/lighthouse-cli/test/cli/run-test.js index f1cbd956bf22..48fef6e3047a 100644 --- a/lighthouse-cli/test/cli/run-test.js +++ b/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); }); diff --git a/lighthouse-cli/test/smokehouse/perf/expectations.js b/lighthouse-cli/test/smokehouse/perf/expectations.js index c2fc4486ba38..e033d622a66c 100644 --- a/lighthouse-cli/test/smokehouse/perf/expectations.js +++ b/lighthouse-cli/test/smokehouse/perf/expectations.js @@ -13,15 +13,8 @@ module.exports = [ initialUrl: 'http://localhost:10200/preload.html', url: 'http://localhost:10200/preload.html', audits: { - 'speed-index-metric': { + 'speed-index': { score: '>=0.80', - extendedInfo: { - value: { - timings: {}, - timestamps: {}, - frames: [], - }, - }, }, 'first-meaningful-paint': { score: '>=0.90', diff --git a/lighthouse-core/closure/typedefs/viewer-externs.js b/lighthouse-core/closure/typedefs/viewer-externs.js index 1f15a9e44145..a70b5fed86f6 100644 --- a/lighthouse-core/closure/typedefs/viewer-externs.js +++ b/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) {}; diff --git a/lighthouse-core/index.js b/lighthouse-core/index.js index 8610c171c8ac..88e5e50daff3 100644 --- a/lighthouse-core/index.js +++ b/lighthouse-core/index.js @@ -9,6 +9,7 @@ const Runner = require('./runner'); const log = require('lighthouse-logger'); const ChromeProtocol = require('./gather/connections/cri.js'); +const formatReport = require('./report/v2/report-formatter').formatReport; const Config = require('./config/config'); /* @@ -53,6 +54,7 @@ function lighthouse(url, flags = {}, configJSON) { runnerResult.lhr.timing = runnerResult.lhr.timing || {}; runnerResult.lhr.timing.total = endTime - startTime; + runnerResult.formattedReport = formatReport(runnerResult.lhr, config.settings.output); return runnerResult; }); }); diff --git a/lighthouse-core/lib/file-namer.js b/lighthouse-core/lib/file-namer.js index a2b25b7d0c1f..ee5395f2cedc 100644 --- a/lighthouse-core/lib/file-namer.js +++ b/lighthouse-core/lib/file-namer.js @@ -12,7 +12,7 @@ * 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 {LH.Result} lhr + * @param {{url: string, fetchedAt: string}} lhr * @return {string} */ function getFilenamePrefix(lhr) { diff --git a/lighthouse-core/runner.js b/lighthouse-core/runner.js index c39228611fcc..76590db2fee8 100644 --- a/lighthouse-core/runner.js +++ b/lighthouse-core/runner.js @@ -17,7 +17,6 @@ const fs = require('fs'); const path = require('path'); const URL = require('./lib/url-shim'); const Sentry = require('./lib/sentry'); -const formatReport = require('./report/v2/report-formatter').formatReport; const Connection = require('./gather/connections/connection.js'); // eslint-disable-line no-unused-vars @@ -132,8 +131,7 @@ class Runner { reportGroups: opts.config.groups, }; - const formattedReport = formatReport(lhr, settings.output); - return {lhr, formattedReport, artifacts}; + return {lhr, artifacts, formattedReport: ''}; } catch (err) { // @ts-ignore TODO(bckenny): Sentry type checking await Sentry.captureException(err, {level: 'fatal'}); diff --git a/lighthouse-core/test/report/v2/report-formatter-test.js b/lighthouse-core/test/report/v2/report-formatter-test.js new file mode 100644 index 000000000000..e35849419164 --- /dev/null +++ b/lighthouse-core/test/report/v2/report-formatter-test.js @@ -0,0 +1,54 @@ +/** + * @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'; + +/* eslint-env mocha */ +const formatReport = require('../../../report/v2/report-formatter').formatReport; +const assert = require('assert'); +const fs = require('fs'); +const sampleResults = require('../../../../lighthouse-core/test/results/sample_v2.json'); +const csvValidator = require('csv-validator'); + +describe('formatReport', () => { + it('creates JSON for results', () => { + const jsonOutput = formatReport(sampleResults, 'json'); + assert.doesNotThrow(_ => JSON.parse(jsonOutput)); + }); + + it('creates HTML for results', () => { + const htmlOutput = formatReport(sampleResults, 'html'); + assert.ok(/ { + const path = './.results-as-csv.csv'; + const headers = { + category: '', + name: '', + title: '', + type: '', + score: 42, + }; + + const csvOutput = formatReport(sampleResults, 'csv'); + fs.writeFileSync(path, csvOutput); + + try { + await csvValidator(path, headers); + } catch (err) { + assert.fail('CSV parser error:\n' + err.join('\n')); + } finally { + fs.unlinkSync(path); + } + }); + + it('writes extended info', () => { + const htmlOutput = formatReport(sampleResults, 'html'); + const outputCheck = new RegExp('dobetterweb/dbw_tester.css', 'i'); + assert.ok(outputCheck.test(htmlOutput)); + }); +}); diff --git a/lighthouse-core/test/report/v2/report-generator-test.js b/lighthouse-core/test/report/v2/report-generator-test.js index ac56a513822c..0da5b4cf8cdd 100644 --- a/lighthouse-core/test/report/v2/report-generator-test.js +++ b/lighthouse-core/test/report/v2/report-generator-test.js @@ -37,34 +37,34 @@ describe('ReportGeneratorV2', () => { describe('#generateHtmlReport', () => { it('should return html', () => { - const result = new ReportGeneratorV2().generateReportHtml({}); + const result = ReportGeneratorV2.generateReportHtml({}); assert.ok(result.includes('doctype html'), 'includes doctype'); assert.ok(result.trim().match(/<\/html>$/), 'ends with HTML tag'); }); it('should inject the report JSON', () => { const code = 'hax\u2028hax