CI Speedup: skip optional install steps in tests

[notnownikki]

Description

During some tests, npm install can run multiple times, and the post install step was also run multiple times when the test didn't require it.

This introduces a new env var, SKIP_POSTINSTALL which lets CI jobs skip the post install step and speed up the test run.

The first job, which runs npm ci keeps the post install step so that coverage is not affected.

This reduces the e2e test run time by a few minutes, from 10-11 minutes to around 8. It also speeds up the other tests by around 45-90 seconds, depending on Travis load.

This should speed up freeing test resources, and make it more bearable to re-run e2e tests that intermittently fail.

The first time this change is merged, the tests will take longer because caches are invalid.

[aduth]

• How well does this work in Windows? I'm imagining a Bash script in postinstall won't work very well.
• Isn't license checker something we should want to occur in CI ?
• Would this cause issue with mobile test-running? And if not, for what reason would npm run mobile-install need to exist at all?
• I'm curious what need there is for building packages in postinstall. Was this meant to be something explicitly targeting CI to catch build errors? cc @gziolo

[notnownikki]

How well does this work in Windows? I'm imagining a Bash script in postinstall won't work very well.

Ah, good point! I'll see if this can be done another way.

Isn't license checker something we should want to occur in CI ?

Yes, and I think that it still runs with this change because the first job doesn't skip postinstall..

Would this cause issue with mobile test-running?

If it does, we can always have those jobs not skip? Those jobs are quick to run anyway, so they don't really need to skip. It's mostly the e2e tests where the install step runs multiple times, although if we can free up resources on other jobs faster, that's nice too.

[tofumatt]

We already have a fair bit of bash scripting in the test runner; I'd be happy to refactor that as much as possible but right now I think you need bash to run our tests.

[aduth]

These are run on simply npm install, which we should support in Windows.

[aduth]

Also, in general, I don't agree with using "it's already not great" as an excuse to perpetuate the not-great.

[notnownikki]

These are run on simply npm install, which we should support in Windows.

Agreed, 100%. I think there should be a way to do the same with a node script instead.

[gziolo]

I see some references to mobile-install which isn't part of Gutenberg anymore.

I'm curious what need there is for building packages in postinstall. Was this meant to be something explicitly targeting CI to catch build errors?

It might be confusing as we have npm run build:packages inside postinstall so it might break some flows which expect it was executed. I think @noisysocks proposed a different approach with caching for packages builds to mitigate that. We also use check-licenses to verify that all dependencies use proper licenses so it needs to be executed at least in one of the nodes.

[noisysocks]

I was working on improving CI build times in December (#13122) and didn't notice npm install running twice on any of our jobs. Is it possible that this was fixed some time in between September and December?

What I did notice was that we run npm run build:packages twice or more in some jobs. That's what inspired me to try and speed up npm run build:packages in #13124.

I spent some considerable time trying to untangle our build scripts so that we weren't duplicating work. It's tough, though, because a lot of our npm scripts do a lot in an effort to make things easy for users. I'd welcome any effort to untangle our mess of build scripts!

[aduth]

Given above, is this pull request still relevant and expected to be revisited in the near future, or should it be closed?

[gziolo]

Let's close it and revisit once we have a clear path for #13124. It might turn out that this change is no longer necessary.