Repository navigation
CI: Reduce unnecessary jobs #182
Description
We currently have 3 workflows that run as part of the CI, and they run on both push and pull-request.
-
codeql.yml:
- Basic sanity check of the CodeQL behavior.
- Splits into 3 jobs of up to 9 minutes.
- Hopefully this will improve soon with Use smaller Plarform-Language packages when analyzing only one language #172 and similar changes
-
pr-checks.yml:
- Code quality checks
- Splits into 5 jobs of less than 30 seconds
-
integration-testing.yml:
- Extensive coverage of cross-feature behavior.
- Splits into 19 jobs of between 5 and 15 minutes.
- Use smaller Plarform-Language packages when analyzing only one language #172 is planning to add 21 additional jobs
All those parallel jobs compete for the same resources. Running them twice means that each push we are running 27x2 -> 54 jobs, filling up the queue.
Proposal
- Change
pr-checks.ymlto be only onpush(maybe rename tolinting?) - Change
codeql.ymlto be onpushonly formainandv1, andpull-requestfor all. - Change
integration-testingto be onpull-requestand only on push formainandv1 - Ideally sort the jobs in
integration-testingfrom the more likely to fail to the less likely to fail, and make the latter conditional on the former.- This is the strategy I am hoping to adopt for Use smaller Plarform-Language packages when analyzing only one language #172
This would avoid duplication of jobs without reducing the coverage we get.
Metadata
Metadata
Assignees
Labels
No labels
Activity
robertbrignull commentedon Sep 11, 2020
Thanks for coming up with a plan for this. I agree the number of jobs is high at the moment and there's some unnecessarily duplication.
I think your description of the current situation is entirely correct.
For the proposed changes, I worry that would create too much of a difference between PRs raised by us, and PRs raised from forks. I like your suggestion on some of the cases of
pushonly formainandv1, andpull-requestfor all. I think that would be a good system to use for all workflows in fact.I don't have any opinion on this except that won't this cause things to slow down by delaying the start of some jobs? Or does it start them all but then kill some jobs if others fail?
marcogario commentedon Sep 15, 2020
I was thinking that it would delay them. I believe the total runtime would not change by much (maybe a few minutes), but it would ensure that we have enough capacity to run multiple workflows in parallel, thus allowing multiple people to get a fair share of the compute resource.
I know we can fail the jobs within a matrix build (
strategy: fail-fast), but I would have to take a look for cross-job failure. I think there is no option across workflows.This applies only to
Change pr-checks.yml to be only on push (maybe rename to linting?), right? In all other cases we should be able to analyze the forks since we runon: pull-request.As you say, maybe having consistent triggers for all workflows might be easier, as we keep it simple and consistent.
robertbrignull commentedon Sep 15, 2020
I think let's try it and see what change it makes to the overall runtime.
Yes, that comment was only applying to
pr-checks.yml. I think having consistent triggers for all of them makes sense, for ease and because I think they all have the same requirements.Reduce triggers in workflows
Reduce triggers in workflows
marcogario commentedon Sep 21, 2020
I would probably additionally copy these two jobs
check-node-modules:andcheck-jsas pre-condition for all theIntegration Testingjobs. These are good sanity checks that, and if they fail, we cannot rely on the results of the integration testing.