-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Remove commented out pytest-rerunfailures test dependency #3263
Remove commented out pytest-rerunfailures test dependency #3263
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #3263 +/- ##
========================================
Coverage 79.53% 79.53%
========================================
Files 68 68
Lines 11781 11781
========================================
Hits 9370 9370
Misses 2411 2411
Continue to review full report at Codecov.
|
What's the benefit of re-running failures? In my opinion, if a test keeps blinking, it's probably a bad test. We should improve the test so that it does not blink instead of re-running it and hoping that we get lucky on one of the attempts. @pabs3 What are your thoughts? |
I'm not sure about the benefit, I just figured it was used for a reason
before and only got disabled due to a bug somewhere else and that bug
got fixed so now it could get enabled again.
What you say sounds correct to me. If it isn't useful then I can turn
this PR into a commit dropping the commented out dependency.
…--
bye,
pabs
https://bonedaddy.net/pabs3/
|
@menshikh-iv @piskvorky What do you guys think? Can we get rid of this and run tests once only? |
Do we still experience flaky tests? How bad is it – if not bad, then no reason to automate "re-runs" IMO. |
I don't think it's bad. One of the wheel builds fails occasionally, but there are ways around that other than automatic re-runs (plus the re-runs don't always help). |
If I remember correctly, rerun plugin help us to avoid "blinking" test failures, if we didn't see these tests now - we can just don't use this plugin.
@mpenkov just curious, would you mind to add details? |
In order of decreasing preference, we could:
|
Personally, I'd rather we default to no automatic rerunning of tests. It slows down detection of reliable failures, & might hide other stability issues better solved another way. |
5bca10c
to
c43b365
Compare
The consensus seems to be to not re-add pytest-rerunfailures, so I have added a commit removing the commented out dependency. |
Can someone re-run the checks? Seems like one of them timed out and another I cannot tell why the failure happened. |
The wheel-build failures are likely unrelated. @mpenkov is looking into it – thanks for your PR! |
It is no longer needed.
c43b365
to
6f7e9f5
Compare
Merged. Thank you @pabs3 ! |
pytest-rerunfailures is now compatible with pytest 6.1.0 and later.
Reverts: commit 8687e7f.
Reported-in: pytest-dev/pytest-rerunfailures#128
Fixed-by: pytest-dev/pytest-rerunfailures#129