core: expose LighthouseRunWarnings on audit context

[brendankenny]

I was thinking about #5666 this morning and realized we shouldn't be altering artifacts in audits. Instead, this exposes a way to push top-level warnings from audits via the auditContext, similar to how gatherers can do so from the passContext.

happy to bikeshed on anything

[patrickhulce]

this works for me :)

I'm not really sure I care about the distinction though. Is there a user-facing downside of mutating the artifact?

similar to how gatherers can do so from the passContext.

AFAICT, the gatherers are directly pushing onto the same array that gets exposed as an artifact, so it already seemed pretty much the same to me.

[brendankenny]

Is there a user-facing downside of mutating the artifact?

In -A mode we still pass the artifacts back out, so you'd get the artifacts with an additional warning from what you passed in. In the CLI we don't save the artifacts if it's just -A mode, but if used as a node module, repeated processing would add more and more extension bootup time warnings to the run warnings. It is admittedly a minor problem :)

Mostly it's the conceptual cleanliness, which is why I mentioned being open to bikeshedding on the approach.

AFAICT, the gatherers are directly pushing onto the same array that gets exposed as an artifact, so it already seemed pretty much the same to me.

Right, and here it parallels that with audits pushing onto the warnings array that gets exposed on the LHR (which also includes the warnings copied from the artifact). Keeps artifact generation solely in gatherRunner and gatherers, but yes, in the end result functionally equivalent.

[brendankenny]

In the CLI

actually, maybe it's observable in the CLI if run with -GA and then again with -A you'd get two copies of the warning in the resulting report, one saved from the original -A and one added by the second -A (you'd just max out at two warnings as the artifacts wouldn't be re-saved from the second run)

[patrickhulce]

Ah gotcha -A + node module, yeah makes sense! In CLI -GA, we should be saving the artifacts before audit mode so the saved artifacts should never have results of auditing, right?

lighthouse/lighthouse-core/runner.js

Lines 84 to 87 in 098d281

	if (settings.gatherMode) {
	const path = Runner._getArtifactsPath(settings);
	await assetSaver.saveArtifacts(artifacts, path);
	}

either way, this seems good 👍

[brendankenny]

In CLI -GA, we should be saving the artifacts before audit mode

oh riiiight, yeah, good call