core(lantern): cleanup Simulator construction #4910
Conversation
patrickhulce
force-pushed
the
cleanup_simulator_creation
branch
from
0767228 to
1e2e921
Compare
|
@brendankenny thoughts here? |
|
|
||
| class SimulatorArtifact extends ComputedArtifact { | ||
| get name() { | ||
| return 'Simulator'; |
There was a problem hiding this comment.
LoadSimulator, maybe? Needs more description :)
|
|
||
| switch (throttlingMethod) { | ||
| case 'provided': | ||
| options.rtt = networkAnalysis.rtt; |
There was a problem hiding this comment.
I guess there's the possibility we want to allow a provided user to tell us what throttling they're using, but not sure if anyone will care enough (or if our estimate will be good enough regardless)
There was a problem hiding this comment.
yeah somewhat ambiguous how they would do that for now, let's leave it for another day :)
lighthouse-core/runner.js
Outdated
| @@ -204,10 +204,11 @@ class Runner { | |||
| * Otherwise returns error audit result. | |||
| * @param {!Audit} audit | |||
| * @param {!Artifacts} artifacts | |||
| * @param {*} opts | |||
There was a problem hiding this comment.
which opts is this? Is there an existing type?
There was a problem hiding this comment.
ah, some bastard child of options.settings.opts. Maybe just {settings: LH.ConfigSettings} then? (for future type-adders :)
There was a problem hiding this comment.
you got it boss 👍
| @@ -182,7 +182,7 @@ class Runner { | |||
| let promise = Promise.resolve(); | |||
| for (const auditDefn of opts.config.audits) { | |||
| promise = promise.then(_ => { | |||
| return Runner._runAudit(auditDefn, artifacts).then(ret => auditResults.push(ret)); | |||
| return Runner._runAudit(auditDefn, artifacts, opts).then(ret => auditResults.push(ret)); | |||
There was a problem hiding this comment.
re: conversation on settings === artifact settings, not sure of the best place, but rather than buried in the specific audits that use them (since all audits have access to settings) could be a few lines above here after artifacts is defined?
final touches waiting on #4894 but mostly independenttouched up!this PR cleans up the creation of Simulator objects and their usage by creating a new computed artifact that reads throttling info from the new
settingsobject