Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

workflows/tests: enable merge group builds. #14964

Merged
merged 1 commit into from Mar 15, 2023
Merged

workflows/tests: enable merge group builds. #14964

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/tests.yml
View file
Open in desktop
Expand Up @@ -5,6 +5,7 @@ on:
branches:
- master
pull_request:
merge_group:

permissions:
contents: read
Expand Down Expand Up @@ -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)'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@reitermarkus yeh, agreed 👍🏻

run: |
# brew tests

Expand Down