pytest-dev / pytest-timeout

MIT License
213 stars 64 forks source link

feature request: add suite timeout #163

Closed okken closed 7 months ago

okken commented 9 months ago

As far as I understand, the timeout in pytest-timeout is per test item. There are times when a full suite timeout is also appropriate. Example: Sure, I don't want any single test to take longer than 2 minutes. But I also don't want the suite to take longer than 20 min.

This is implemented in a "between tests, best attempt" way with pytest-suite-timeout, but it might make more sense to incorporate it into pytest-timeout.

flub commented 9 months ago

Maybe, maybe not. I'm pretty sure you're not the first to ask for this. I think usually I respond with something that this isn't the goal of pytest-timeout.

If you'd like to see this could you dig though the issues and PRs for previous discussions around this and write a summary of the for and against arguments?

Cheers

On Mon 22 Jan 2024 at 07:49 -0800, Brian Okken wrote:

As far as I understand, the timeout in pytest-timeout is per test item. There are times when a full suite timeout is also appropriate. Example: Sure, I don't want any single test to take longer than 2 minutes. But I also don't want the suite to take longer than 20 min.

This is implemented in a "between tests, best attempt" way with pytest-suite-timeout, but it might make more sense to incorporate it into pytest-timeout.

okken commented 9 months ago

I found one discussion, which included this comment: "this essentially asks for graceful termination and graceful termination is explicitly not the domain of pytest-timeout. It's purpose is last-resort termination." https://github.com/pytest-dev/pytest-timeout/issues/60#issuecomment-693631163

I am looking for graceful termination, and I think lots of people are. I don't actually see the problem with expanding the concept of pytest-timeout to include graceful termination of the suite/session if it runs too long.

Earlier in the thread, you said " And I also believe that this should not happen normally and is a serious bug in the test suite if it does. " I don't think that's true in a lot of situations.

When a test suite includes communication channels (external services, hardware resources, embedded devices, network connections, etc.), slowdowns are always possible. A suite that usually takes 10 minutes can easily crawl to over 30 if there is a network or communication slowdown, or an error in a device, etc. In those cases, a test will usually complete, but everything just slows down, and just giving up on a suite can be appropriate.

There's also the pytest-repeat use case. I'm looking at a flaky test failure, and want to run it over lunch or overnight or whatever and get a report afterwards. I could run the test like 10 times and then guess/calculate how many iterations I can fit into an hour, but it's often mentally simpler to say "run it like 10,000 times or at most 90 minutes".

Perhaps in the pytest-repeat case, it does belong in pytest-repeat and not pytest-timeout. But I think it's useful for other cases as well. And I think the most obvious place for people to look for the feature is to pytest-timeout.

However, pytest-timeout is already reasonably complex as far as plugins go, so maybe it does make sense to leave it in a separate plugin. So feel free to close this request.

flub commented 9 months ago

Given how many times it comes up I'm wondering if I should finally give in and it should be added. Though a major caveat is that I'm mostly in maintenance-mode here and mostly merge PRs from other people. So someone would have to come up with a simple enough design carefully considering the tradeoffs and be willing to drive the implementation.

okken commented 9 months ago

I think that's fair. I'll prepare a PR when I can.

image

okken commented 9 months ago

PR on its way

flub commented 7 months ago

done in #165