str0zzapreti / pytest-retry

A simple plugin for retrying flaky tests in CI environments
MIT License
29 stars 6 forks source link

feat: Add max-global-retries option #38

Closed str0zzapreti closed 3 months ago

str0zzapreti commented 7 months ago

Created based on PR https://github.com/str0zzapreti/pytest-retry/pull/37

As a user, I would like the ability to set an optional global maximum number of retries for tests. If the total count of retried tests due to failures exceeds this number, all subsequent tests will not be retried in the event of a failure. For example:

I have a suite of 10 tests which are all marked retries=2 and global max retries set to 5 The first test passes The second test fails and is retried twice The third test fails and is retried twice The fourth test fails and although it is configured to retry twice, after the first retry the global limit is reached Now if test 6, 7, 8, 9, and/or 10 fails, it will not be retried because of the global limit

This option should be configurable either via a command line option or through any valid pytest config file. If not specified, there is assumed to be no limit (i.e. the existing default behavior)

str0zzapreti commented 7 months ago

@deronnax I created an enhancement ticket for you to track the request facilitate discussion.

str0zzapreti commented 7 months ago

After reviewing https://github.com/str0zzapreti/pytest-retry/pull/37, my first thought is this implementation might not work as expected when using pytext-xdist for parallelization. Specifically, unless the retries are tracked on the server, each node would be individually counting and limiting retries. On the other hand, server-based tracking would make this more complicated and would probably introduce some slight loss of precision due to the asynchronous processing, so one or two extra retries might occur before each node received the signal to stop performing additional retries.

Another thought: your stated goal was to "better control the flakiness of the whole test suite", but I'm not sure I understand how this feature would achieve that goal, exactly. Would you mind elaborating a bit? To me, the purpose of this plugin is to help produce consistent, accurate results from test suites which must contend with unstable environments, 3rd party services, or other factors out of the tester's control. Ideally, allowing the tester or their colleagues to work on mitigating those factors in better ways. How would globally limiting retires help?

With that in mind, Pytest has a maxfail parameter which will halt all further test execution after a maximum number of test failures. I haven't tested that with pytest-retry myself, but I was curious to know if you'd tried this option already and what the result was. Any chance it would meet your needs?

Thanks again for the suggested feature, looking forward to hearing your thoughts! 😃

deronnax commented 5 months ago

Hello, I am back. No, --maxfail would not meet our needs because we want to run a maximum possible of tests as long as they only flake once. With maxfail, the test run would stop as soon as a test flakes twice. We are actually OK than the max-retries is not global but per process, so in the current state, the current implementation is correct, no?

deronnax commented 5 months ago

Hello. I slightly updated the PR. We will test the current implementation, and use it for us if it works as expected. I understand that this might be too specific, so you are free to close the PR. Just a quick review for you would be very nice. The PR is short is simple. Thank you.

str0zzapreti commented 5 months ago

Hey, just catching up after a busy few weeks. My instinct says this might be a bit too niche as well, but I'll happily take another look at your PR and decide if it's something I want to incorporate. Either way, thank you!

deronnax commented 3 months ago

@str0zzapreti I think you can close ;)

str0zzapreti commented 3 months ago

Sure thing! It looks like you had a working prototype for adding this functionality to your own repo, but let me know if there's another way I can help either with suggested implementation or by making this plugin more extensible if necessary.