-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,89 +17,6 @@ const path = require('path'); | |
const Audit = require('../audits/audit'); | ||
const Runner = require('../runner'); | ||
|
||
// cleanTrace is run to remove duplicate TracingStartedInPage events, | ||
// and to change TracingStartedInBrowser events into TracingStartedInPage. | ||
// This is done by searching for most occuring threads and basing new events | ||
// off of those. | ||
function cleanTrace(trace) { | ||
const traceEvents = trace.traceEvents; | ||
// Keep track of most occuring threads | ||
const threads = []; | ||
const countsByThread = {}; | ||
const traceStartEvents = []; | ||
const makeMockEvent = (evt, ts) => { | ||
return { | ||
pid: evt.pid, | ||
tid: evt.tid, | ||
ts: ts || 0, // default to 0 for now | ||
ph: 'I', | ||
cat: 'disabled-by-default-devtools.timeline', | ||
name: 'TracingStartedInPage', | ||
args: { | ||
data: { | ||
page: evt.frame, | ||
}, | ||
}, | ||
s: 't', | ||
}; | ||
}; | ||
|
||
let frame; | ||
let data; | ||
let name; | ||
let counter; | ||
|
||
traceEvents.forEach((evt, idx) => { | ||
if (evt.name.startsWith('TracingStartedIn')) { | ||
traceStartEvents.push(idx); | ||
} | ||
|
||
// find the event's frame | ||
data = evt.args && (evt.args.data || evt.args.beginData || evt.args.counters); | ||
frame = (evt.args && evt.args.frame) || data && (data.frame || data.page); | ||
|
||
if (!frame) { | ||
return; | ||
} | ||
|
||
// Increase occurences count of the frame | ||
name = `pid${evt.pid}-tid${evt.tid}-frame${frame}`; | ||
counter = countsByThread[name]; | ||
if (!counter) { | ||
counter = { | ||
pid: evt.pid, | ||
tid: evt.tid, | ||
frame: frame, | ||
count: 0, | ||
}; | ||
countsByThread[name] = counter; | ||
threads.push(counter); | ||
} | ||
counter.count++; | ||
}); | ||
|
||
// find most active thread (and frame) | ||
threads.sort((a, b) => b.count - a.count); | ||
const mostActiveFrame = threads[0]; | ||
|
||
// Remove all current TracingStartedIn* events, storing | ||
// the first events ts. | ||
const ts = traceEvents[traceStartEvents[0]] && traceEvents[traceStartEvents[0]].ts; | ||
|
||
// account for offset after removing items | ||
let i = 0; | ||
for (const dup of traceStartEvents) { | ||
traceEvents.splice(dup - i, 1); | ||
i++; | ||
} | ||
|
||
// Add a new TracingStartedInPage event based on most active thread | ||
// and using TS of first found TracingStartedIn* event | ||
traceEvents.unshift(makeMockEvent(mostActiveFrame, ts)); | ||
|
||
return trace; | ||
} | ||
|
||
function validatePasses(passes, audits) { | ||
if (!Array.isArray(passes)) { | ||
return; | ||
|
@@ -214,38 +131,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. | ||
|
@@ -367,7 +252,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 || {}; | ||
|
@@ -782,11 +666,6 @@ class Config { | |
return this._audits; | ||
} | ||
|
||
/** @type {Array<!Artifacts>} */ | ||
get artifacts() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. | ||
|
@@ -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 = []; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. part of the discussed change to default to having an empty |
||
if (opts.config.categories) { | ||
reportCategories = ReportScoring.scoreAllCategories(opts.config.categories, resultsById); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
{} |
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 @@ | ||
[ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. copied over from |
||
{"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"} | ||
] |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -60,7 +60,7 @@ module.exports = { | |
}, | ||
beginDevtoolsLog() {}, | ||
endDevtoolsLog() { | ||
return require('../fixtures/perflog.json'); | ||
return require('../fixtures/artifacts/perflog/defaultPass.devtoolslog.json'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what a beautiful sight :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aye. this removes support for traces captured by webdriver, but at this point, i'm just fine with that. :)