Skip to content

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

Closed
lawsonjl opened this issue Jul 6, 2017 · 21 comments
Closed

Automatically mark async test functions #61

lawsonjl opened this issue Jul 6, 2017 · 21 comments

Comments

@lawsonjl
Copy link

lawsonjl commented Jul 6, 2017

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 root conftest.py for that suite I have added the following pytest hook:

import pytest
import inspect

def pytest_collection_modifyitems(session, config, items):
    for item in items:
        if isinstance(item, pytest.Function) and inspect.iscoroutinefunction(item.function):
            item.add_marker(pytest.mark.asyncio)

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 the async 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 using async.

If this kind of feature would be appreciated, I can contribute the changes myself when I can find the time.

@Tinche
Copy link
Member

Tinche commented Jul 6, 2017

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.

@agronholm
Copy link
Contributor

So what determines which of these frameworks gets to run async fixtures then, since they're not specially marked?

@lawsonjl
Copy link
Author

lawsonjl commented Jul 8, 2017

That makes sense. It's easy to forget that writing async def functions doesn't necessarily imply that you are using asyncio.

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 @pytest.mark.asyncio_ignore decorator may be appropriate for users who have special cases.

@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?

@Tinche
Copy link
Member

Tinche commented Jul 9, 2017

I've been ruminating on this issue a little.

I think we should probably do the reverse of what I suggested earlier:

  • treat test coroutines as pytest-asyncio tests automatically
  • treat coroutine fixtures as pytest-asyncio fixtures automatically (which we kinda do now)
  • offer an opt-out setting that only treats test coroutines and fixtures with the marker as ours

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.

@njsmith
Copy link

njsmith commented Jul 26, 2017

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).

@malinoff
Copy link

malinoff commented Aug 8, 2017

I'd say you should be even more stricter with opt-in. Right now pytest_fixture_setup hook is too greedy: it always awaits async (generator) fixtures using the asyncio loop. But it shouldn't be the case when running tests for, say, curio. I think pytest_fixture_setup shouldn't touch async (generator) fixtures at all if they aren't explicitly marked with @pytest.mark.asyncio.

For instance, I face Fatal Python error: GC object already tracked crashes when trying to run tests for curio adapter while having both pytest-curio and pytest-asyncio installed, so that's a high priority issue for me.

@njsmith
Copy link

njsmith commented Aug 8, 2017

I'm not sure that pytest actually has a concept of using pytest.mark on fixtures. Fixing the fixture issue might require switching to a new decorator, like @pytest_asyncio.fixture...

@nicoddemus
Copy link
Member

I'm not sure that pytest actually has a concept of using pytest.mark on fixtures.

Indeed it doesn't, see the note at the top of the docs for markers.

@malinoff
Copy link

malinoff commented Aug 9, 2017

Right, I should've said that it shouldn't touch async (generator) fixtures unless a test function that requests them is marked with @pytest.mark.asyncio (i.e.

if fixturedef.argname == "event_loop" and 'asyncio' in request.keywords:
).

smagafurov pushed a commit to smagafurov/pytest-asyncio that referenced this issue Apr 4, 2018
Make db_url work with async databases
@pipermerriam
Copy link
Contributor

I'd like to also request that pytest-asyncio stop automatically treating async def fixtures as if they wanted to be run by asyncio. I have a codebase that exposes two versions of an API, one that uses asyncio and one that uses trio. As it stands it is impossible for me to run these within the same test run because pytest-asyncio greedily chooses to run my trio based fixtures from an event loop.

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.

@agronholm
Copy link
Contributor

@pipermerriam I invite you to check out my AnyIO project which might solve some of your issues. It comes with its own pytest plugin.

@asvetlov
Copy link
Contributor

asvetlov commented May 7, 2019

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.

@njsmith
Copy link

njsmith commented May 7, 2019

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

@belkka
Copy link

belkka commented Mar 19, 2020

I would be happy to have an ini-file option. For now using code snipped from the OP

@KimiNewt
Copy link

Is this still an issue? It seemed to work without a mark now.

@agronholm
Copy link
Contributor

Is this still an issue? It seemed to work without a mark now.

That precisely is the issue. It should require a mark, so as not to conflict with other libraries providing a runner for coroutine functions.

@matham
Copy link

matham commented Dec 21, 2020

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 asyncio_mode, like trio's trio_mode is also added.

Because with trio, I can enable automatically using pytest-trio by setting trio_mode in config.ini or by doing from pytest_trio.enable_trio_mode import * in conftest.py. With asyncio, you'd need to mark every test module or test function.

@agronholm
Copy link
Contributor

I was just working on a plugin that supports either trio or asyncio.

Do you have a use case where AnyIO's pytest plugin is not enough?

@matham
Copy link

matham commented Dec 23, 2020

Do you have a use case where AnyIO's pytest plugin is not enough?

Do you mean why use pytest-trio or pytest-asyncio instead of pytest-anyio so it works with either async framework? I guess no real reason, other than I didn't think of anyio and I also tend to like working with bare metal.

@agronholm
Copy link
Contributor

I didn't think of anyio and I also tend to like working with bare metal.

As opposed to...?

@asvetlov
Copy link
Contributor

asvetlov commented Jan 8, 2022

Done by #125 (use asyncio_mode=auto for autodiscovering)

@asvetlov asvetlov closed this as completed Jan 8, 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

No branches or pull requests