I'm totally into this and think it rationalizes things quite a bit. The churn doesnt feel too bad from here.

i can't speak to how this plays into the FR world, but it feels like a partial win there. i'll defer to patrick, clearly.

general 👍 from me

Other things that could be good but not currently included:

• rename runner.js to audit-runner.js

eh. i expect the churn is a bit much. i'm fine punting this for a while

• move html/json/csv report generation to index.js. Runner.run() would return just an lhr, and lighthouse() in index.js would be the one to put together the {lhr, artifacts, report} RunnerResult. This would require a bit more test changes due to the Runner.run() return type change.

yeah... in fact.. this PR lays perfect groundwork to finally do cli(lifecycle): introduce report-only runs · #4210! we'd then have core/index calling gather-runner, then runner, then report-generator. and the last could absorb some of the generateReport() fn you're talking about here.

Other considered ideas…

• move -GA artifact loading/saving out of lighthouse-core/ altogether and move them to lighthouse-cli/

yeah i'm a 👎 on this. i like our CLI being as dumb as we can allow.

Things I love:

• Straightforward FR flow without the confusing callback
• Creating the concept of audit-runner

Things I don't love:

• Loss of -GA / latest-run functionality for FR requiring a new refactor to bring it back
• The growth of logic in index.js, in general I find heavy lifting in an unhelpfully labeled "index.js"
• Merge conflicts with other ongoing FR work (super minor, but I don't love it 😄 )

I think those are all resolved while preserving my favorite parts if instead of moving the logic directly to index.js, it stayed in runner.js decomposed into a few functions that index.js and FR could use, and the runner.js in this PR were renamed audit-runner.js like you suggested.

but it stretches the definition of "CLI" since we'd want node modules to still be able to use it.

I've been thinking we really want 3 modules:

• core
• node
• cli

Much of the CLI work belongs in a node package and these core utilities you mention belong in a node package, but I definitely wouldn't want to remove capability from node module use, in general I think we should be aligning even more the CLI and node capabilities.

move lhr generation out of runner. Doesn't seem worth it because the LHR is the only audit output we ever care about, and there's some internal details (like handling ICU types before i18n.replaceIcuMessages() is called) that are currently encapsulated but would have to be exposed

I agree not worth it.

expose an audit() method in index.js. This would let FR runners use our core entry point instead of calling Runner directly. Something like this seems worthwhile but probably better to think about when we start nailing down the FR module API.

This is kinda close to my counter-proposal above.

move html/json/csv report generation to index.js. Runner.run() would return just an lhr, and lighthouse() in index.js would be the one to put together the {lhr, artifacts, report} RunnerResult. This would require a bit more test changes due to the Runner.run() return type change.

This seems annoying from a shared FR perspective and not sure it's that valuable in the short term, especially given the test surface changes. Maybe worth tackling as part of a legacy winddown once FR is default? Paul seems excited about this, but I'm not sure what need we've really had for report only mode lately :)

rename runner.js to audit-runner.js

👍 see above :D

The growth of logic in index.js, in general I find heavy lifting in an unhelpfully labeled "index.js"

yeah, i was thinking the same. it might as well get a proper name. possibilities: coordinator.. orchestrater.. director..

Paul seems excited about this, but I'm not sure what need we've really had for report only mode lately :)

tbh the only need is when we're hacking on the report. superfast iteration of just JSON->HTML repeatedly. (my hack has been find lighthouse-core/report | entr node lighthouse-core/scripts/build-report-for-autodeployment.js)..
but outside of us working on the report renderer.... i don't think its a compelling user feature. so yeah not a big deal if we never make -R a first-class thing.

Should we take another look at this during next eng sync?

This reorg still seem good, but so much is changed since this was opened that we likely wouldn't use any of the code here so closing