pytest-dev / pytest-repeat

pytest plugin for repeating test execution
https://pypi.python.org/pypi/pytest-repeat/
Other
169 stars 27 forks source link

Add timeout flag, to timeout test run after some number of minutes #79

Open okken opened 7 months ago

okken commented 7 months ago

Proposal to add --repeat-timeout=minutes, with the type being a float.

This borrowed functionality from pytest-flakefinder that has a --flake-max-minutes.

--repeat-timeout vs --repeat-max-minutes I'm thinking --repeat-timeout seems more obvious, and a bit shorter.

int vs float flakefinder uses an integer. I propose using a float. It doesn't make that much difference in practice. But testing the feature would be way more convenient if we didn't have to wait a minute for a timeout.

skip vs exit flakefinder uses skips to skip remaining tests. That works, but is annoying with tons of skips. I propose using pytest.exit() to avoid all of the pointless skips. Also, then you can just guess a big number. "repeat like 20,000 times, but stop after an hour" kind of thing.

implementation The implementation is fairly simple. At the start of a test run, store an expiration time of time.time() + (timeout_minutes * 60). Then at test time, compare the expiration time to the current time. If timeout, stop testing. This is pretty simple to implement.

affect on existing usage My only concern would be if the functionality slows down test runs not using it. The code added when NOT used is one variable check, and seems to not affect test time if not used.

alternatives This functionality could also be added as a separate plugin entirely, as there are totally reasons to want to ensure a test suite doesn't run too long. There is pytest-timeout, but the timeout there applies to individual tests, not the suite. If added as a new plugin, perhaps something like pytest-suite-timeout or something?

keeping it with pytest-repeat I could see this being a separate plugin. But also, I think it's a pretty common use case especially with pytest-repeat. The idea being some way to implement "repeat it a bunch of times, but not more than like 30 minutes". So, I'm thinking it would be good to include with pytest-repeat.

RonnyPfannschmidt commented 7 months ago

Full test suite timeout is out of scope here

Timeout for a run of repeats of a single test seems potentially tricky

okken commented 7 months ago

Single test is unnecessary, as it's covered by pytest-timeout already. I don't mind putting the functionality into a different plugin, but dismissing it out of hand as "out of scope" seems a bit blunt.

If it's "in scope" for pytest-flakefinder, how is it "out of scope" here?

RonnyPfannschmidt commented 7 months ago

Timeout doesn't handle repeats as each repeat is a new item,so timeout would need to accumulate

Timeout for a complete test suit doesn't fit this plugin, I'd more expected it to be a extension of the timeout plugin

okken commented 7 months ago

I disagree, but don't feel like arguing. I've implemented the functionality in pytest-suite-timeout. We can play with the behavior there.

okken commented 7 months ago

I still think this would be nice to have in repeat, as it seems natural. I guess I'd like more opinions before we kill it outright.