Async fixtures, explicit user-defined loops and more

[malinoff]

Hi, this PR is not supposed to be merged. Instead, I want to discuss these changes, fix what is wrong, add backwards compatibility layer if needed, and then if everybody is okay I want to create separate PRs with small changes.

Please, refer to the readme to see the major changes.

[axsapronov]

+1

[malinoff]

@Tinche sorry for disrupting, but is this project maintained?

[Tinche]

Hi,

yes it is, we had a new release 10 days ago.

This is hard for me to review because there is a bunch of changes and no rationale for any of them.

For one, I think an important goal of this library is that ordinary, non-complex usage should be the default, and complex features should be available if you want them. So, accept_global_loop should default to True. Also, since we already have this parameter under the name forbid_global_loop, I'd prefer to keep that name to not break an existing API (defaulting to False).

I like the current approach of having a special fixture named event_loop better than yours of inspecting fixtures for a specific type. Dependency injection based on fixture names is a usual pytest convention, but based on types is not. Fixtures can also be overriden by their names, etc.

As for changing the scope of the event loop used, I think the existing mechanism of overriding the event_loop fixture should be good for that, but it needs to be refactored a little. Currently the event_loop fixture is responsible for producing the event loop instance, but not for closing it (the plugin closes it); this should be refactored so the fixture itself closes the loop when done. This is more in the spirit of Pytest and enables some additional use cases, but it makes for a more complex event_loop fixture so the readme should be updated with a proper recipe for overriding it.

Async fixtures look interesting, those would be valuable to support.

If you want to contribute, a good first step would be to fix the default event_loop fixture so that it's a normal yield_fixture (function scoped) that closes itself, and remove the close from the plugin itself. Also update the readme with a good recipe for overriding the event_loop fixture, both for using a different loop and for using a different scope.

[malinoff]

@Tinche rationale is in the readme, it's literally a bunch of features allowing more flexibility.

So, accept_global_loop should default to True

Unfortunately, this leads to very weird errors for a library developer who decided to use a custom user-provided event loop instead of always getting the global one due to programming errors when the loop wasn't explicitly passed to asyncio functions, like sleep.

The rest is okay to me.

[Tinche]

Well, you can write simple to moderate asyncio applications using just a single loop in the main thread, and asyncio itself supports it with the asyncio.get_event_loop function. So people relatively new to asyncio are likely to just write code that makes asyncio use the global loop, and those are the people that we should support out of the box.

Now, we can talk about ways to document this and make it easier to test without a global loop. For example, we could have a command line parameter that changes the default from False to True. Then you could just add this setting to your pytest.ini.

[joac]

Great work @malinoff!
Async fixtures is must have.
In my opinion, small and focused Pull Request are more likely to be merged than ones with lots of changes. Let me know if you need help to do that.

[asvetlov]

I'm with @malinoff
Global loop is good for toy examples only, real app should use explicit loops.
The rule violation leads to weird errors and bad code.

[vortec]

Any chance to get this merged and released?

[julienatshopify]

Would love to see the async fixtures to be added to this project.

[Tinche]

Hi, I've recently merged in support for function-scoped async fixtures, but haven't yet made a release. (#45)

Please try it out directly from git if you have the time and willpower, and report any bugs.

(Closing this PR.)