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(config): remove config.artifacts; always use auditMode #4986

Merged
merged 4 commits into from
Apr 18, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ last-run-results.html

!lighthouse-core/test/results/artifacts/*.trace.json
!lighthouse-core/test/results/artifacts/*.devtoolslog.json
!lighthouse-core/test/fixtures/artifacts/**/*.trace.json
!lighthouse-core/test/fixtures/artifacts/**/*.devtoolslog.json

latest-run

Expand Down
16 changes: 5 additions & 11 deletions docs/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -93,28 +93,22 @@ $ lighthouse --port=9222 --disable-device-emulation --disable-cpu-throttling htt

## Lighthouse as trace processor

Lighthouse can be used to analyze trace and performance data collected from other tools (like WebPageTest and ChromeDriver). The `traces` and `devtoolsLogs` artifact items can be provided using a string for the absolute path on disk. The `devtoolsLogs` array is captured from the `Network` and `Page` domains (a la ChromeDriver's [enableNetwork and enablePage options]((https://sites.google.com/a/chromium.org/chromedriver/capabilities#TOC-perfLoggingPrefs-object)).
Lighthouse can be used to analyze trace and performance data collected from other tools (like WebPageTest and ChromeDriver). The `traces` and `devtoolsLogs` artifact items can be provided using a string for the absolute path on disk if they're saved with `.trace.json` and `.devtoolslog.json` file extensions, respectively. The `devtoolsLogs` array is captured from the `Network` and `Page` domains (a la ChromeDriver's [enableNetwork and enablePage options]((https://sites.google.com/a/chromium.org/chromedriver/capabilities#TOC-perfLoggingPrefs-object)).

As an example, here's a trace-only run that's reporting on user timings and critical request chains:
As an example, here's a trace-only run that reports on user timings and critical request chains:

### `config.json`

```json
{
"settings": {
"auditMode": "/User/me/lighthouse/lighthouse-core/test/fixtures/artifacts/perflog/",
},
"audits": [
"user-timings",
"critical-request-chains"
],

"artifacts": {
"traces": {
"defaultPass": "/User/me/lighthouse/lighthouse-core/test/fixtures/traces/trace-user-timings.json"
},
"devtoolsLogs": {
"defaultPass": "/User/me/lighthouse/lighthouse-core/test/fixtures/traces/perflog.json"
}
},

"categories": {
"performance": {
"name": "Performance Metrics",
Expand Down
38 changes: 0 additions & 38 deletions lighthouse-core/config/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -214,38 +214,6 @@ function assertValidGatherer(gathererInstance, gathererName) {
}
}

function expandArtifacts(artifacts) {
if (!artifacts) {
return null;
}
// currently only trace logs and performance logs should be imported
if (artifacts.traces) {
Object.keys(artifacts.traces).forEach(key => {
log.log('info', 'Normalizng trace contents into expected state...');
let trace = require(artifacts.traces[key]);
// Before Chrome 54.0.2816 (codereview.chromium.org/2161583004), trace was
// an array of trace events. After this point, trace is an object with a
// traceEvents property. Normalize to new format.
if (Array.isArray(trace)) {
trace = {
traceEvents: trace,
};
}
trace = cleanTrace(trace);

artifacts.traces[key] = trace;
});
}

if (artifacts.devtoolsLogs) {
Object.keys(artifacts.devtoolsLogs).forEach(key => {
artifacts.devtoolsLogs[key] = require(artifacts.devtoolsLogs[key]);
});
}

return artifacts;
}

/**
* Creates a settings object from potential flags object by dropping all the properties
* that don't exist on Config.Settings.
Expand Down Expand Up @@ -367,7 +335,6 @@ class Config {

this._passes = Config.requireGatherers(configJSON.passes, this._configDir);
this._audits = Config.requireAudits(configJSON.audits, this._configDir);
this._artifacts = expandArtifacts(configJSON.artifacts);
this._categories = configJSON.categories;
this._groups = configJSON.groups;
this._settings = configJSON.settings || {};
Expand Down Expand Up @@ -782,11 +749,6 @@ class Config {
return this._audits;
}

/** @type {Array<!Artifacts>} */
get artifacts() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

return this._artifacts;
}

/** @type {Object<{audits: !Array<{id: string, weight: number}>}>} */
get categories() {
return this._categories;
Expand Down
12 changes: 8 additions & 4 deletions lighthouse-core/lib/asset-saver.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,19 +74,23 @@ img {
/**
* Load artifacts object from files located within basePath
* Also save the traces to their own files
* @param {string} basePath
* @param {string} artifactsPath
* @return {Promise<LH.Artifacts>}
*/
async function loadArtifacts(basePath) {
async function loadArtifacts(artifactsPath) {
const basePath = path.resolve(process.cwd(), artifactsPath);
log.log('Reading artifacts from disk:', basePath);

if (!fs.existsSync(basePath)) return Promise.reject(new Error('No saved artifacts found'));
if (!fs.existsSync(basePath)) {
throw new Error('No saved artifacts found at ' + basePath)
}

// load artifacts.json
const filenames = fs.readdirSync(basePath);
/** @type {LH.Artifacts} */
const artifacts = JSON.parse(fs.readFileSync(path.join(basePath, artifactsFilename), 'utf8'));

const filenames = fs.readdirSync(basePath);

// load devtoolsLogs
artifacts.devtoolsLogs = {};
filenames.filter(f => f.endsWith(devtoolsLogSuffix)).map(filename => {
Expand Down
9 changes: 4 additions & 5 deletions lighthouse-core/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,18 +70,16 @@ class Runner {
opts.url = parsedURL.href;

// User can run -G solo, -A solo, or -GA together
// -G and -A will do run partial lighthouse pipelines,
// -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, from config, or get from the browser
// Either load saved artifacts off disk or from the browser
let artifacts;
if (settings.auditMode && !settings.gatherMode) {
// No browser required, just load the artifacts from disk.
const path = Runner._getArtifactsPath(settings);
artifacts = await assetSaver.loadArtifacts(path);
} else if (opts.config.artifacts) {
artifacts = opts.config.artifacts;
} else {
artifacts = await Runner._gatherArtifactsFromBrowser(opts, connection);
// -G means save these to ./latest-run, etc.
Expand Down Expand Up @@ -115,7 +113,8 @@ class Runner {
const resultsById = {};
for (const audit of auditResults) resultsById[audit.name] = audit;

let reportCategories;
/** @type {Array<LH.Result.Category>} */
let reportCategories = [];
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

part of the discussed change to default to having an empty categories array, even if none in the config (we could maybe start defaulting to [] in the config, but we'll see)

if (opts.config.categories) {
reportCategories = ReportScoring.scoreAllCategories(opts.config.categories, resultsById);
}
Expand Down
62 changes: 0 additions & 62 deletions lighthouse-core/test/config/config-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -473,68 +473,6 @@ describe('Config', () => {
assert.ok(config.settings.disableDeviceEmulation, 'missing setting from flags');
});

describe('artifact loading', () => {
it('expands artifacts', () => {
const config = new Config({
artifacts: {
traces: {
defaultPass: path.resolve(__dirname, '../fixtures/traces/trace-user-timings.json'),
},
devtoolsLogs: {
defaultPass: path.resolve(__dirname, '../fixtures/perflog.json'),
},
},
});
const computed = Runner.instantiateComputedArtifacts();

const traceUserTimings = require('../fixtures/traces/trace-user-timings.json');
assert.deepStrictEqual(config.artifacts.traces.defaultPass.traceEvents, traceUserTimings);
const devtoolsLogs = config.artifacts.devtoolsLogs.defaultPass;
assert.equal(devtoolsLogs.length, 555);

return computed.requestNetworkRecords(devtoolsLogs).then(records => {
assert.equal(records.length, 76);
});
});

it('expands artifacts with multiple named passes', () => {
const config = new Config({
artifacts: {
traces: {
defaultPass: path.resolve(__dirname, '../fixtures/traces/trace-user-timings.json'),
otherPass: path.resolve(__dirname, '../fixtures/traces/trace-user-timings.json'),
},
devtoolsLogs: {
defaultPass: path.resolve(__dirname, '../fixtures/perflog.json'),
otherPass: path.resolve(__dirname, '../fixtures/perflog.json'),
},
},
});
const traceUserTimings = require('../fixtures/traces/trace-user-timings.json');
assert.deepStrictEqual(config.artifacts.traces.defaultPass.traceEvents, traceUserTimings);
assert.deepStrictEqual(config.artifacts.traces.otherPass.traceEvents, traceUserTimings);
assert.equal(config.artifacts.devtoolsLogs.defaultPass.length, 555);
assert.equal(config.artifacts.devtoolsLogs.otherPass.length, 555);
});

it('handles traces with no TracingStartedInPage events', () => {
const config = new Config({
artifacts: {
traces: {
defaultPass: path.resolve(__dirname,
'../fixtures/traces/trace-user-timings-no-tracingstartedinpage.json'),
},
devtoolsLogs: {
defaultPass: path.resolve(__dirname, '../fixtures/perflog.json'),
},
},
});

assert.ok(config.artifacts.traces.defaultPass.traceEvents.find(
e => e.name === 'TracingStartedInPage' && e.args.data.page === '0xhad00p'));
});
});

describe('#extendConfigJSON', () => {
it('should merge passes', () => {
const configA = {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
{}
20 changes: 20 additions & 0 deletions lighthouse-core/test/fixtures/artifacts/perflog/artifacts.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{
"LighthouseRunWarnings": [
"I'm a warning!",
"Also a warning"
],
"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",
"fetchedAt": "2018-03-13T00:55:45.840Z",
"URL": {
"initialUrl": "https://www.reddit.com/r/nba",
"finalUrl": "https://www.reddit.com/r/nba"
},
"Viewport": null,
"ViewportDimensions": {
"innerWidth": 412,
"innerHeight": 732,
"outerWidth": 412,
"outerHeight": 732,
"devicePixelRatio": 2.625
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
[
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

copied over from fixtures/traces/trace-user-timings.json cause it was small and we really only need to test the loading part :)

{"pid":41904,"tid":1295,"ts":1676836141,"ph":"I","cat":"disabled-by-default-devtools.timeline","name":"TracingStartedInPage","args":{"data":{"page":"0xf5fc2501e08","sessionId":"9331.8"}},"tts":314881,"s":"t"},
{"pid":41904,"tid":1295,"ts":506085991145,"ph":"R","cat":"blink.user_timing","name":"navigationStart","args":{"frame": "0xf5fc2501e08"},"tts":314882},
{"pid":41904,"tid":1295,"ts":506085991146,"ph":"R","cat":"blink.user_timing","name":"firstPaint","args":{"frame": "0xf5fc2501e08"},"tts":314883},
{"pid":41904,"tid":1295,"ts":506085991146,"ph":"R","cat":"blink.user_timing","name":"firstContentfulPaint","args":{"frame": "0xf5fc2501e08"},"tts":314883},
{"pid":41904,"tid":1295,"ts":506085991146,"ph":"R","cat":"blink.user_timing","name":"paintNonDefaultBackgroundColor","args":{},"tts":314883},
{"pid":41904,"tid":1295,"ts":506086992099,"ph":"R","cat":"blink.user_timing","name":"mark_test","args":{},"tts":331149},
{"pid":41904,"tid":1295,"ts":506086992100,"ph":"R","cat":"blink.user_timing","name":"goog_123_3_1_start","args":{},"tts":331150},
{"pid":41904,"tid":1295,"ts":506086992101,"ph":"R","cat":"blink.user_timing","name":"goog_123_3_1_end","args":{},"tts":331151},
{"pid":41904,"tid":1295,"ts":506085991147,"ph":"b","cat":"blink.user_timing","name":"measure_test","args":{},"tts":331184,"id":"0x73b016"},
{"pid":41904,"tid":1295,"ts":506086992112,"ph":"e","cat":"blink.user_timing","name":"measure_test","args":{},"tts":331186,"id":"0x73b016"},
{"pid":41904,"tid":1295,"ts":506085991148,"ph":"b","cat":"blink.user_timing","name":"goog_123_3_1","args":{},"tts":331184,"id":"0x73b016"},
{"pid":41904,"tid":1295,"ts":506086992113,"ph":"e","cat":"blink.user_timing","name":"goog_123_3_1","args":{},"tts":331186,"id":"0x73b016"}
]
2 changes: 1 addition & 1 deletion lighthouse-core/test/gather/fake-driver.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ module.exports = {
},
beginDevtoolsLog() {},
endDevtoolsLog() {
return require('../fixtures/perflog.json');
return require('../fixtures/artifacts/perflog/defaultPass.devtoolslog.json');
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was big, so just moved over to the artifacts fixtures and moved the two tests using it to point at the new path

},
blockUrlPatterns() {
return Promise.resolve();
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/test/gather/network-recorder-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

const NetworkRecorder = require('../../lib/network-recorder');
const assert = require('assert');
const devtoolsLogItems = require('../fixtures/perflog.json');
const devtoolsLogItems = require('../fixtures/artifacts/perflog/defaultPass.devtoolslog.json');

/* eslint-env mocha */
describe('network recorder', function() {
Expand Down
17 changes: 9 additions & 8 deletions lighthouse-core/test/index-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,18 +87,17 @@ describe('Module Tests', function() {
});
});

it.skip('should return formatted audit results when given no categories', function() {
it.only('should return formatted LHR when given no categories', function() {
const exampleUrl = 'https://example.com/';
return lighthouse(exampleUrl, {
output: 'json',
}, {
auditResults: [{
score: 0,
displayValue: '',
rawValue: true,
name: 'viewport',
description: 'HTML has a viewport <meta>',
}],
settings: {
auditMode: __dirname + '/fixtures/artifacts/perflog/',
},
audits: [
'viewport',
],
}).then(results => {
assert.ok(results.lighthouseVersion);
assert.ok(results.fetchedAt);
Expand All @@ -107,6 +106,8 @@ describe('Module Tests', function() {
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');
});
Expand Down
Loading