workflows/tests: enable merge group builds. #14964
Conversation
This will enable the GitHub Merge Queue. Don't bother running online tests as they aren't required, use up the rate limit and are flaky.
|
Review period will end on 2023-03-14 at 22:03:39 UTC. |
BrewTestBot
added
the
waiting for feedback
There was a problem hiding this comment.
I'm struggling to see the benefit of this for this repo. We already have auto-merge available at the click of a button, we already have the normal merge button, and there's not a production deployment step with lots of people trying to get lots of changes shipped all at once. 🤔
|
So does this work like “require pull requests branched to be up-to-date” without having to rebase it before merging? I.e. every pull request is rebased in the merge queue before actually being merged? |
If this works like I am thinking it works:
If they are added to a merge queue instead, one of them will be merged and the other will fail once it is rebased on the other. |
| @@ -311,6 +312,7 @@ jobs: | |||
| run: sudo chmod -R g-w,o-w /home/linuxbrew/.linuxbrew/Homebrew | |||
|
|
|||
| - name: Run brew tests | |||
| if: github.event_name == 'pull_request' || matrix.name != 'tests (online)' | |||
There was a problem hiding this comment.
I think it might actually make more sense to flip this condition.
Running online tests only in the merge queue will make it less likely we hit rate limits if we also limit to one concurrent merge group.
There was a problem hiding this comment.
@reitermarkus I'm less concerned about rate limits here than the job being flaky and that being annoying before merge. If it flakes when in the merge queue, it'll get kicked out and you need to try and add and rerun all CI again. If it flakes before the merge queue, you can manually rerun.
There was a problem hiding this comment.
But what does flaky mean exactly? I think we already retry online tests multiple times to make them more robust, so the only thing that can still fail them is the rate limit.
Can you point to an online test that is flaky for a different reason?
There was a problem hiding this comment.
@reitermarkus Flaky meaning will fail intermittently and we've been unable to fix that. That applies with pretty much all our online tests.
It's rare for them to fail consistently outside of the rate limit but e.g. third-party services being down/flaky will make them fail/flake too.
There was a problem hiding this comment.
My only concern is we don't get the benefit of merge queues when two PRs modify code that needs network access. I guess we'll cross that bridge if we ever get there and add offline tests.
There was a problem hiding this comment.
@reitermarkus yeh, agreed 👍🏻
@reitermarkus Yes but with merge commits instead of rebasing.
@issyl0 Will need to figure out how to adjust the CI jobs accordingly so it doesn't take too long/be too annoying but:
@issyl0 exactly what @reitermarkus has pointed out here. |
BrewTestBot
removed
the
waiting for feedback
|
Review period ended. |
This will enable the GitHub Merge Queue. Don't bother running online tests as they aren't required, use up the rate limit and are flaky.