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 options #216

Closed
wants to merge 12 commits into from
Closed

Add timeout options #216

wants to merge 12 commits into from

Conversation

fjetter
Copy link

@fjetter fjetter commented Jun 8, 2021

This adds the option to add timeouts to tests either via marker kwargs or via options/CLI arguments.

The naming of the options is intentionally prefixes with asyncio to avoid clashes with pytest-timeout. Marker kwargs do not have this ambiguity and I therefore allowed both.

I don't have a good sense on how to test the options via config and CLI. If somebody got an idea, I'd be happy to implement it.

Closes #215
Depends on pytest-dev/pytest-timeout#117

@asvetlov
Copy link
Contributor

asvetlov commented Jan 6, 2022

Please read https://docs.pytest.org/en/6.2.x/writing_plugins.html#testing-plugins

pytester fixture can be used for testing command-line and ini files.

The plugin has no documentation, IMHO.
I can suggest only the source code and example of usage in aiohttp.

Here is an example of calling pytest with custom cmdline arguments: https://github.com/aio-libs/aiohttp/blob/master/tests/test_pytest_plugin.py#L140-L150

@codecov-commenter
Copy link

codecov-commenter commented Jan 9, 2022

Codecov Report

Merging #216 (1009cc0) into master (4c7da65) will increase coverage by 0.36%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #216      +/-   ##
==========================================
+ Coverage   95.43%   95.79%   +0.36%     
==========================================
  Files           2        2              
  Lines         219      238      +19     
==========================================
+ Hits          209      228      +19     
  Misses         10       10              
Impacted Files Coverage Δ
pytest_asyncio/plugin.py 95.74% <100.00%> (+0.37%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4c7da65...1009cc0. Read the comment docs.

@fjetter
Copy link
Author

fjetter commented Jan 10, 2022

Didn't know about pytester. Thanks for adding the commits!

@asvetlov
Copy link
Contributor

The PR is not complete until timeout disabling on debugging is implemented, as in pytest-timeout.
Otherwise, any tests debug session ends with TimeoutError quickly.
I'm not sure is it worth to copy-paste the whole implementation from pytest-timeout plugin?
Maybe adding timeout_setup()/timeout_teardown() hooks for implementing them in pytest-asyncio makes the code easier and stablier?
Should take time for experimenting.

flub pushed a commit to pytest-dev/pytest-timeout that referenced this pull request Jan 16, 2022
…down_timeout methods (#117)

It seems pytest-asyncio is interested in this.  See e.g.
pytest-dev/pytest-asyncio#216
@altendky
Copy link
Member

I'm interested in timeouts for my asyncio tests and it looks like this might be the path to that. Is that correct? What is needed here?

@johnthagen
Copy link

Any chance that this PR could make progress? This seems like a really important fundamental tool for testing async functions.

@seifertm
Copy link
Contributor

seifertm commented Jun 6, 2023

This PR introduces a "runner" abstraction and adds the timeout functionality. This is also the general way forward for pytest-asyncio. Preferably, these two things should happen in separate PRs, one that introduces the Runner and another one that adds timeout functionality.

PR #312 is draft that does both. Moreover, it adds a dependency on aioloop-proxy which is completely separate from this feature. Since it's unclear who will be able to maintain aioloop-proxy, please try to keep the loop proxy topic separate from #312 if you decide to rebase the draft.

Does that answer your questions?

@fjetter
Copy link
Author

fjetter commented Jun 19, 2023

I will actually go ahead and close this PR. The code here drifted significantly from my much simpler initial contribution attempt and I do not intend to pick this up again. If somebody wants to finish this up, please open a new PR.

@fjetter fjetter closed this Jun 19, 2023
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.

Test timeout via marker kwargs
6 participants