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.

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

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

Open
okken opened this issue Jan 21, 2024 · 5 comments
Open

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

okken opened this issue Jan 21, 2024 · 5 comments

Comments

@okken
Copy link
Contributor

okken commented Jan 21, 2024

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
Copy link
Member

Full test suite timeout is out of scope here

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

@okken
Copy link
Contributor Author

okken commented Jan 21, 2024

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
Copy link
Member

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
Copy link
Contributor Author

okken commented Jan 22, 2024

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 okken closed this as completed Jan 22, 2024
@okken
Copy link
Contributor Author

okken commented Jan 22, 2024

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.

@okken okken reopened this Jan 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants