Add status check when target=filesystem

[idbartosz]

Currently when target is set to filesystem there's no report back to Github statuses api. Is there a reason why runGithubStatusCheck wasn't added for runFilesystemTarget? Maybe it also would be worth to add pending state before Lighthouse collect is ran 🙂

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

Lines 566 to 569 in daac2d1

	const manifestPath = path.join(targetDir, 'manifest.json');
	fs.writeFileSync(manifestPath, JSON.stringify(manifest, null, 2));
	print('Done writing reports to disk.\n');
	}

[patrickhulce]

Thanks for filing @idbartosz !

Is there a reason why runGithubStatusCheck wasn't added for runFilesystemTarget?

We don't run the status check because there's nothing for the status check to link to. Filesystem target was meant as a workaround for the user to do whatever they want with the results.

I'm not terribly opposed to running the status check for the filesystem target, but it's definitely a less than ideal experience to not be able to click details and see the report or any additional information.

[idbartosz]

but it's definitely a less than ideal experience to not be able to click details and see the report or any additional information.

Thanks for quick response! Actually Jenkins CI has a nice publishHTML plugin which is very handy for exposing static assets generated during build steps. And those are accessible via URL which could be set as a status details link, maybe even with combination of LHCI_BUILD_CONTEXT__EXTERNAL_BUILD_URL constant.

Filesystem target was meant as a workaround for the user to do whatever they want with the results.

And thank you for this option 😀 not everyone needs advanced features like comparison with historical data, or wants to expose their results in some external storage solution. The status check has some handy automation behind it, like assertions or generating checks for individual URLs - it's a bit wasteful to do those on your own when they are already here 🙂

[patrickhulce]

And those are accessible via URL which could be set as a status details link

I don't think there's any way for LHCI to figure this out in a way that isn't just much more work than the end user posting a status to GitHub 😆 We need to know the URL per URL per run at the time of upload. The URLs won't be known to userland until after filesystem upload. Just leave this up to

maybe even with combination of LHCI_BUILD_CONTEXT__EXTERNAL_BUILD_URL constant.

This is a good fallback though. If GitHub token is found and this is set, we'll run github status check for filesystem 👍

[paigevogie]

I had to do a bit of troubleshooting to figure out that the github status checks don't post when the target is set to filesystem. I was hoping to add the status checks because even if they don't link to anything, they would be helpful to provide more visibility into the lighthouse audits being run.

Unless this changes, it might be helpful to update the upload documentation and the app docs to clarify that a target other than filesystem is required.