-
Notifications
You must be signed in to change notification settings - Fork 159
Automatically mark async test functions #61
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
Comments
Hello! There is indeed a reason for not treating async def test functions automatically as asyncio tests: it's that there are other Python async frameworks based on coroutines, like https://github.com/dabeaz/curio and https://github.com/python-trio/trio, and this would make it impossible to support tests for these other frameworks in the same code base (since curio tests would be async def coroutines too). So the current approach is opt-in. The current approach to reducing boilerplate is using pytestmark at the module or class level (see here: https://github.com/pytest-dev/pytest-asyncio#pytestmarkasyncio). This still requires a little bit of boilerplate at the top of the file. I wonder if we could do something with a custom pytest option. For example, by modifying pytest.ini or setup.cfg you could opt into treating all async def tests as pytest-asyncio tests. |
So what determines which of these frameworks gets to run async fixtures then, since they're not specially marked? |
That makes sense. It's easy to forget that writing An opt-in pytest option could work. It may even be preferable to make it opt-out - it depends on how often the scenario with tests requiring multiple async frameworks comes up. With that approach, some kind of @agronholm brings up a good point. The async fixtures appear to use asyncio's event loop to run and they aren't marked in any way. It doesn't look like they would work if another framework was also providing async fixture support. They should probably require a mark of some sort if that scenario is supported, right? |
I've been ruminating on this issue a little. I think we should probably do the reverse of what I suggested earlier:
The rationale is this: simple and common use cases should require a minimum of boilerplate, but more complex use cases should be possible. And code bases using only asyncio are probably a great majority right now. As for fixtures, you're right. When I initially started writing the fixture code, there was a decorator you had to import and apply to fixtures for the plugin to execute them, but I never got to finish this work. Later this feature was contributed by a different contributor (Alexander Gorokhov), but this work supported only function-scoped fixtures. Function scoped fixtures are actually ok, since in non-asyncio tests you just don't use them, and they only got handled by the plugin if they were executing in the context of an asyncio test. Then I finished up the feature adding support for other scopes, and now yes, we will conflict with other frameworks. There are also auto-use fixtures to consider. It'd be nice if we could be intelligent here: for plain function-scoped fixtures we can probably reliably inspect if we're executing for an asyncio test. For module, class and session scoped fixtures we would probably need a marker. |
Please don't make this opt-out. It violates the rule that simply installing a package shouldn't change the behavior of other packages. And it's no big deal to ask folks who are using your package to add one line to their pytest.ini at the same time they add it to their test-requirements.txt or whatever, but if you make it opt-out then you're forcing every package that uses an alternative async library to add a line to their pytest.ini to stop pytest-asyncio from breaking their test suite (but of course they won't even realize this until some random potential contributor gets bitten and wastes however long figuring out what happened). |
I'd say you should be even more stricter with opt-in. Right now For instance, I face |
I'm not sure that pytest actually has a concept of using |
Indeed it doesn't, see the note at the top of the docs for markers. |
Right, I should've said that it shouldn't touch async (generator) fixtures unless a test function that requests them is marked with pytest-asyncio/pytest_asyncio/plugin.py Line 126 in 92b34e5
|
Make db_url work with async databases
I'd like to also request that Either way, I need a way to disable this behavior and while an opt-out mechanism would suffice, I think that it would be more responsible to implement this as an opt-in and let the user decide, with an option for global opt-in as the common case. |
@pipermerriam I invite you to check out my AnyIO project which might solve some of your issues. It comes with its own pytest plugin. |
I believe a cmd-line flag and ini-file option (both opt-in) to enforce the plugin for async functions can satisfy everybody. Opt-in marking for async fixtures makes sense too. |
Yeah, feel free to steal from pytest-trio, we have all the opt-in variants working AFAIK: https://pytest-trio.readthedocs.io/en/latest/reference.html#trio-mode |
I would be happy to have an ini-file option. For now using code snipped from the OP |
Is this still an issue? It seemed to work without a |
That precisely is the issue. It should require a mark, so as not to conflict with other libraries providing a runner for coroutine functions. |
I was just working on a plugin that supports either trio or asyncio. Besides the issues brought up here, it would be nice if when it's made opt-in, support for something like Because with trio, I can enable automatically using pytest-trio by setting |
Do you have a use case where AnyIO's pytest plugin is not enough? |
Do you mean why use |
As opposed to...? |
Done by #125 (use |
I have a test suite that uses pytest-asyncio for some of its tests and it works pretty well. Thank you for creating such a useful plugin.
Enhancement Request
All of my coroutine test functions are declared with
async
. To avoid marking every such function with@pytest.mark.asyncio
, in the rootconftest.py
for that suite I have added the following pytest hook:So far I have not managed to find any drawback to doing this sort of thing - the
async
keyword indicates coroutine functions just as clearly as the decorator does, and this approach seems to correctly mark each async test.Could something like this be a part of pytest-asyncio itself, or is there a rationale for not including such a feature?
I understand that this project existed before Python had
async
/await
semantics; however, you appear to have dropped support for Python < 3.5 (#57), which means that every supported Python version also supports theasync
keyword. Furthermore, I am not suggesting the removal or deprecation of the decorator approach since there are probably valid use cases for creating coroutines without usingasync
.If this kind of feature would be appreciated, I can contribute the changes myself when I can find the time.
The text was updated successfully, but these errors were encountered: