GitHub status check score description doesn't use median run when target=lhci

[patrickhulce]

From #235 (comment)

Currently just the last LHR wins for the summary description

lighthouse-ci/packages/cli/src/upload/upload.js

Lines 415 to 429 in 5380a23

	for (const lhr of lhrs) {
	const parsedLHR = JSON.parse(lhr);
	const url = getUrlForLhciTarget(parsedLHR.finalUrl, options);
	const run = await api.createRun({
	projectId: project.id,
	buildId: build.id,
	representative: false,
	url,
	lhr,
	});
	buildViewUrl.searchParams.set('compareUrl', url);
	targetUrlMap.set(parsedLHR.finalUrl, buildViewUrl.href);
	print(`Saved LHR to ${options.serverBaseUrl} (${run.id})\n`);
	}

[jflayhart]

Anything I can do to help with this? Would be awesome if we could release this along with whatever else is in master (not sure what your release strategy is).

[patrickhulce]

if you want to try a PR for it, be my guest! :) otherwise I'll get to it next week

[jflayhart]

Hmm ok. I just started looking into this and immediately noticed that code snippet and PR descr don't apply to my comment, which was referring to temporary-public-storage target. I did not specify, sorry.

I didn't realize it applied to both though, so that's good to know (I use both upload targets separately).

Looks like we will need to handle for both upload targets? runLHCITarget and runTemporaryPublicStorageTarget unless there's a shared util method used for calc.

[patrickhulce]

Hm, there must be something else going on then. The only bug I see is that the link for the LHCI target is linking to the incorrect report the labels and links for temporary-public-storage should be using the representative ones.

lighthouse-ci/packages/cli/src/upload/upload.js

Lines 271 to 284 in 5380a23

	const representativeLhrs = computeRepresentativeRuns(lhrsByUrl);
	if (!representativeLhrs.length) return print('No LHRs for status check, skipping.\n');
	for (const lhr of representativeLhrs) {
	const rawUrl = lhr.finalUrl;
	const urlLabel = getUrlLabelForGithub(rawUrl, options);
	const state = 'success';
	const context = getGitHubContext(urlLabel, options);
	const categoriesDescription = Object.values(lhr.categories)
	.map(category => `${category.title}: ${Math.round(category.score * 100)}`)
	.join(', ');
	const description = `${categoriesDescription}`;
	const targetUrl = targetUrlMap.get(rawUrl) || rawUrl;

[patrickhulce]

Actually now that you're mentioning it, I don't see any bug at all here. The only place where we don't use the representative run is for LHCI target link but that doesn't matter because the links are all identical for all the runs because the median is always surfaced there.

Is it possible that the last one actually was the median of all the metrics?

[jflayhart]

Even so, then why is it passing with a median performance score of 58 (of 5 runs, 65/61/60/58/58) when I set assertions to fail <=60? My point was that I am seeing Github check/link not adding up to my score assertions, since it links to the perf score of 58, which doesn't look like a median to me.

Are you saying this is expected behavior? That's fine I guess, just want to understand why 🤔

Again, my expectation was that my performance assertion (<=60) should pass/fail accordingly and the link to the pass/fail LHR should correspond. Maybe I am assuming incorrectly, based on LHCI calculating median FCP score, that it should reflect scores per my assertions?

[jflayhart]

Here's an example where it fails, but it's not getting the median failed score either:

categories.performance failure for minScore assertion
 
        expected: >=0.6
        found: 0.59
 
      all values: 0.46, 0.48, 0.53, 0.54, 0.59

again, im operating under the assumption that assertions determine which scores should be weighed by the median score. So maybe i'm wrong, but then it's confusing that a passing score (in my original comment) of 58 is shown when I asserted that it must be >=60.

[patrickhulce]

It's really hard without the lighthouse reports themselves to answer if this is working as intended. One important thing to note here...

The median selection algorithm for assertions is not the same as the median report selection algorithm for the GitHub status check and it's definitely possible for them not to match.

Hypothetical example:

Run 1 - Perf 60, FCP 4000, TTI 6000
Run 2 - Perf 61, FCP 1000, TTI 10000
Run 3 - Perf 62 FCP 3990, TTI 5900

Run 2 will be used for the median performance score in assertions.
Run 1 will be used for the "representative run" (it has the median TTI and is only 10ms away from median FCP).

The disconnect in the LHCI server UI and the github status checks with assertion summary is understood, undesirable, and on the backlog for improvement (related #232 #100)

[jflayhart]

Ok thanks, I think your explanation clears up what I'm seeing and that it's not necessarily desired behavior but it is currently expected 👍 Feel free to close this ticket or relate it to the others, whichever you think is best.

[patrickhulce]

Thanks @jflayhart! I'll go ahead and close this out since #232 (assertions UI) is largely what blocks syncing all of this up.