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

Support running tests with a timeout. #34

Closed
wants to merge 2 commits into from
Closed

Support running tests with a timeout. #34

wants to merge 2 commits into from

Conversation

sqwishy
Copy link

@sqwishy sqwishy commented Aug 27, 2016

This just lets you pass a timeout with the asyncio mark so that the marked test will error if it takes too long.

I'm not sure if this is a good feature but I didn't see any discussion of it in pytest-asyncio about timeouts so I thought I'd try it and see what happens.

(I only tested this on 3.5 as that's the only python 3 interpreter I have on hand.)

@sqwishy
Copy link
Author

sqwishy commented Aug 27, 2016

This would be nicer if it handled TimeoutErrors a bit better; maybe by showing the loop's current tasks to show at what point coroutines were at when it the test timed out. By default I think the traceback for the TimeoutError is pretty verbose and not very informative.

@Tinche
Copy link
Member

Tinche commented Aug 27, 2016

Hello!

First of all, thanks for contributing to pytest-asyncio.

I think this would be a very useful feature for obvious reasons. However, I notice there's already a separate plugin for timing out tests, pytest-timeout (https://pypi.python.org/pypi/pytest-timeout). So if that works, I'd rather not add this functionality directly in pytest-asyncio. Could you check?

@sqwishy
Copy link
Author

sqwishy commented Aug 27, 2016

I didn't see that earlier, that's neat. It seems to work okay. They do the timeouts differently. Either by scheduling a signal (where supported) or by using a thread that calls os._exit (which quits the interpreter instead of causing the individual test to fail). The output for either isn't very nice; it's about as ugly as what is in this pull request.

Maybe adding an asyncio method to pytest-timeout would be an alternative to doing test timeouts here. But I don't know what that would look like. Maybe it would just wrap the coroutine test in a asyncio.wait_for() and you would decorate it before the @pytest.mark.asyncio decorator so that pytest-asyncio ends up run_until_complete()ing the wait_for()?

But I think it would be ideal to use asyncio's timeout features for coroutine tests instead of signals or having a thread jump in and do something. pytest-timeout doesn't do that right now, and I'm not sure how easy it would be to support it there instead of here.

Also, I didn't check this but I imagine the signal and thread methods of timing out would break debugging with pdb.

@Tinche
Copy link
Member

Tinche commented Aug 28, 2016

The thing is, their way is more robust, and this matters a great deal in tests (because people throw all kinds of bad code at tests, that's what they're for). For example, if you call a non-asyncio function in your test (say, by accident) and it blocks, the loop will never get a chance to even run the timeout coroutine.

What we should do is see if there's a way to integrate with pytest-timeout, either here or in their project. And by integrate I mean maybe printing out a list of running tasks on the current event loop?

smagafurov pushed a commit to smagafurov/pytest-asyncio that referenced this pull request Apr 4, 2018
@asvetlov
Copy link
Contributor

asvetlov commented Jan 6, 2022

#216 provides more advanced approach (while still better test coverage is desired)

@asvetlov asvetlov closed this Jan 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants