-
-
Notifications
You must be signed in to change notification settings - Fork 378
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.
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
Use retry action, don't retry otherwise #3408
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, sounds great!
This uses a action for retrying steps, which is a bit neater, and lets us more clearly specify what's going on, as well as controlling the number of retries independently. I think it's better to specifically use this on the test suites that we believe to be flaky, rather than adding a lot of noise by doing this on every test invocation.
fecd3ce
to
9c506c2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Let's fix flaky tests properly. I propose we invert this patch to retry all testsuites thrice unconditionally and fail if any run fails. Then we will have a list of most flaky tests. I've started a patch to fix flaky testsuite items in ghcide. |
I'm definitely in favour of fixing flaky tests, I just want our CI to be passing for most people instead of requiring constant restarting as it does right now. Then we can make a ticket for each flaky job to work on it. Having things fail a lot is an incentive to fix the tests for sure, but it also just slows everything down tons. |
That said if you think you can actually fix the flakiness soon then go for it! |
The retry action also seems to add a warning to each run with retries by default ( |
@wz1000 did you get most of the flaky tests? I'd be happy to just remove the retrying on everything if you think it's not needed now. |
No, #3423 seems like an issue that results in quite some flakiness throughout the testsuite but it will require a bit of work to resolve. |
This uses a action for retrying steps, which is a bit neater, and lets
us more clearly specify what's going on, as well as controlling the
number of retries independently.
I think it's better to specifically use this on the test suites that we
believe to be flaky, rather than adding a lot of noise by doing this on
every test invocation.