opt into strict mode by default

[graingert]

Fixes #293

It's looking like a critical mass of projects have set asyncio_mode= if they need it, and an increasing number of projects are getting compatibility issues where they set filterwarnings=error, do not use pytest-asyncio, but are installed into an environment with it already.

In addition pytest has now deprecated returning not-None values https://docs.pytest.org/en/latest/deprecations.html#returning-non-none-value-in-test-functions and so in a future version test suites returning Coros will fail, rather than pass with a warning

[Tinche]

Hi,

I disagree with this. The default mode should be auto.

[graingert]

@Tinche the deprecation warning documents that once the deprecation period of the legacy mode ends the default mode will be strict:

pytest-asyncio/pytest_asyncio/plugin.py

Lines 61 to 65 in db72f25

	LEGACY_MODE = DeprecationWarning(
	"The 'asyncio_mode' default value will change to 'strict' in future, "
	"please explicitly use 'asyncio_mode=strict' or 'asyncio_mode=auto' "
	"in pytest configuration file."
	)

[seifertm]

I've always been a bit torn between which mode should be the default. Strict mode provides the greatest compatibility, but forces users to explicitly mark the tests. On the other hand, auto mode provides the greatest convenience. I believe both have a right to exist and switching between them is just one line away for any new project.

However, I do think that we should stick with the mode that was announced in the deprecation warning. I fear that anything else will create confusion.

[Tinche]

The depreciation warning should definitely not force us to make the wrong call here. I'm sorry I missed this when I approved the initial PR that introduced it, but I seem to remember the consensus being we would default to auto.

auto is the novice user use case, and the most common use case. strict is when your tests need to coexist with other async frameworks - it should definitely exist, but it's an advanced feature only for authors of very complex libraries (if they need to be tested on multiple frameworks).

My opinion is it would be a disservice to the community (especially new users getting into async) to make the simplest and most common use case require configuration, and the super-complex use case the default. Also note that asyncio doesn't really stand on equal footing with the other async frameworks due to it being in the standard library.

@asvetlov what's your take on this?

[graingert]

The PR was created to allow pytest-asyncio to exist installed in a users virtual environment without interacting with other pytest plugins and suites

[njsmith]

The argument for making strict the default is that people aren't careful to only run tests inside a hermetically-constructed venv with only and exactly the packages they need. And the way pytest plugins work, just having the package in your environment at all means pytest will suck it in and start changing the semantics of any tests run in that environment.

So if you make strict the default, then asyncio packages need to install it and add one line of config to turn it on, same as how most pytest plugins work. (E.g. installing pytest-cov doesn't start generating coverage reports unless you use --cov-* options, pytest-xdist doesn't start distributing tests unless pass --numprocesses auto, etc.)

If you make auto the default, then all the packages that aren't using pytest-asyncio will need to add configuration to defensively disable pytest-asyncio, or else get bug reports from folks who tried to run tests in an environment that happened to have pytest-asyncio installed, and got an inscrutable error message.

[Tinche]

You do have a point, but what you're saying is:

A subset of users,

• who have a misconfigured virtual environment, with packages they do not want installed (which is lunacy, by the way, what test harness can work like this?)
• who have async tests, but these tests are for a different async plugin than pytest-asyncio

Will get a strange error unless they apply a little configuration? I'm inclined to treat this as a red herring.

My gut feeling is that situations like these will be an order of magnitude more rare than the alternative, which is people just using pytest-asyncio by itself so I would rather place the burden of configuration on the smaller cohort. To your other point, there are also plenty of pytest plugins that change the behavior just by being present.

But, as I said, I do see your point. If both of my co-maintainers (@seifertm and @asvetlov) also want strict as the default, alright.

[graingert]

Will get a strange error unless they apply a little configuration? I'm inclined to treat this as a red herring.

They do not get an error, for example running the starlette test suite all the tests pass - however the tests against trio never run

[seifertm]

My gut feeling is that situations like these will be an order of magnitude more rare than the alternative, which is people just using pytest-asyncio by itself so I would rather place the burden of configuration on the smaller cohort.

You're right: The most widespread use case is to install pytest-asyncio as part of a virtualenv. To be honest, I didn't even think of global installations until #293 was reported. I also agree that we should make pytest-asyncio easy to use for the standard use case.

However, the problem I see with defaulting to auto mode is that the resulting incompatibilities are much harder to understand. For example, the compatibility issue with pytest-trio (#124) caused the following non-sensical error message:
RuntimeError: must be called from async context

If I understand @graingert's comment correctly and there are cases where no error is raised, but tests are simply swallowed that's even worse.

If I were a user it wouldn't be obvious to me that merely installing a package messes with my test suite. I'd easily lose a day debugging the error message above. On the other hand if the README of a new library tells me need to decorate my tests with @pytest.mark.asyncio or switch to auto mode, that's easy to understand.

This is why I believe switching to --asyncio-mode=strict as the new default is the better choice.

[Tinche]

Alright, since you folks feel so strongly, I can live with the default being strict.

[seifertm]

This PR changes the default mode to "strict". It doesn't remove legacy mode and that's probably a good idea.

Here's my suggestion: We create a new release release (v0.19?) directly after this has been merged. All three modes will stay, but the default will have changed. If anyone needs more time moving to strict or auto, they can temporarily enable legacy mode.

The release after that removes legacy mode entirely. Does that plan sound reasonable?

@graingert Could you add an entry to the Changelog? It should clearly state that this is a breaking change and that users can switch back to the deprecated legacy mode if needed.