Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 7 commits into from
Apr 23, 2018
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/hacking-tips.md
Original file line number Diff line number Diff line change
Expand Up @@ -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);
```
Expand All @@ -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

Expand Down
2 changes: 1 addition & 1 deletion lighthouse-cli/bin.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ if (cliFlags.extraHeaders) {
}

/**
* @return {Promise<LH.Results|void>}
* @return {Promise<LH.RunnerResult|void>}
*/
function run() {
return Promise.resolve()
Expand Down
67 changes: 3 additions & 64 deletions lighthouse-cli/printer.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
'use strict';

const fs = require('fs');
const ReportGenerator = require('../lighthouse-core/report/v2/report-generator');
const log = require('lighthouse-logger');

/**
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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));
Expand All @@ -161,7 +101,6 @@ function getValidOutputOptions() {

module.exports = {
checkOutputPath,
createOutput,
write,
OutputMode,
getValidOutputOptions,
Expand Down
26 changes: 12 additions & 14 deletions lighthouse-cli/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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});
Expand All @@ -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} */
Expand All @@ -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);
});
});

Expand Down
59 changes: 7 additions & 52 deletions lighthouse-cli/test/cli/printer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -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 formattedReport = JSON.stringify(sampleResults);
return Printer.write({formattedReport}, '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 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));
Expand Down
11 changes: 6 additions & 5 deletions lighthouse-cli/test/cli/run-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/closure/typedefs/viewer-externs.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,4 @@ function ReportGenerator() {}
* @param {!ReportRenderer.ReportJSON} reportJson
* @return {string}
*/
ReportGenerator.prototype.generateReportHtml = function(reportJson) {};
ReportGenerator.generateReportHtml = function(reportJson) {};
2 changes: 2 additions & 0 deletions lighthouse-core/config/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
1 change: 1 addition & 0 deletions lighthouse-core/config/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const throttling = {

/** @type {LH.Config.Settings} */
const defaultSettings = {
output: 'json',
maxWaitForLoad: 45 * 1000,
throttlingMethod: 'devtools',
throttling: throttling.mobile3G,
Expand Down
13 changes: 2 additions & 11 deletions lighthouse-core/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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});
});
}

Expand Down
8 changes: 4 additions & 4 deletions lighthouse-core/lib/file-namer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', {
Expand Down
Loading