Storybook coverage is the slowest part of the build

[nicksnyder]

In this PR I only changed markdown files, but had to wait 7 minutes for build-storybook to finish.

Can we make these steps not run unless the diff touches files which effect the code being tested (hopefully it would be safe to ignore markdown only changes, and even better if changes to Go files can't cause these tests to fail)?

[keegancsmith]

It would be great to make this step only run if files that could change it where changed. In fact I was frustrated by slower web steps in general and made go only code changes skip everything but go unit tests on PRs #12321

We should have the same logic for doc only changes, in fact my change for go was inspired by that logic.

Taking a step back, why is this step slow? What is it doing?

[nicksnyder]

Another example where this part of the build took 11 minutes and the slowest next step was ~5 minutes:

https://buildkite.com/sourcegraph/sourcegraph/builds/79968#ef7fafe0-91c6-421a-a0f7-ea4f081bf622

[felixfbecker]

Here's the background on why this step takes long: We want to capture the test coverage achieved by Storybook tests (screenshot tests in Chromatic). Chromatic doesn't report this, so we need to locally go through the storybook in Puppeteer similar to how Chromatic does it and capture coverage. We use storyshots for this, which is targetted for taking screenshots, but instead of taking screenshots we just take coverage. Storyshots integrates with Jest and goes through the stories sequentially (with Puppeteer) and needs to wait a bit for each story to load, which is why this step takes long with many stories. There is an issue on the storyshot repo to parallize this process that I've been involved in, and there has recently been a PoC by someone to parallize things and an alternative. Until this is solved there is no easy way to make the step faster though. I'm aware it is slow and it sucks, but it would take significant engineering effort to implement this we currently don't have on the web team if I compare the relative importance of this to to all the other tasks on our backlog.

I agree that it would be nice if CI would generally only run steps for files that changed. It looks like the current system is not a smart one that generally does this for all files and steps, but just a collection of special cases for stuff like Go backend or docs changes where it's easy for a change to not match any special case. I don't see ownership of implementing such a general system on the web team though, especially considering that our CI config is entirely written in Go.

Something that may also be possible configure somehow is that this step doesn't actually block CI, but runs async somehow, given it only reports to Codecov and doesn't test the code in any way. I don't know of a feasible way to do that with Buildkite though.

[keegancsmith]

Something that may also be possible configure somehow is that this step doesn't actually block CI, but runs async somehow, given it only reports to Codecov and doesn't test the code in any way. I don't know of a feasible way to do that with Buildkite though.

This is what we did for e2e tests for a while. Sounds like the correct thing to do.