-
Notifications
You must be signed in to change notification settings - Fork 164
Loop Factory parameter for asyncio mark #1164
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
base: main
Are you sure you want to change the base?
Conversation
Can't seem to figure out where I should inject this new function I made. When I would go to run tests they would fail. |
Testing with different event loops is definitely something we want to support in pytest-asyncio. Currently, there's an event_loop_policy fixture for this purpose. Can you explain why the existing approach is insufficient for your use cases? |
@seifertm I would love to, as of right now eventloop policies are deprecated and planned for removal in 3.16 my goal is to provide a workaround that doesn't involve silencing warnings and allows for the EventLoopPolicy to remain temporarly but redirect users to using a non-deprecated approach sooner than later. I'm trying to catch up with these changes by allowing for a different fixture method to be used I would like to save this library from simply having to keep a warning silent which I would argue is a bad approach to solving a problem. Sooner it's changed the better. |
I agree that the existing approach with the I'd like to avoid configuring the loop type through a fixture, though, for two reasons:
I'm currently leaning towards a configuration setting in pyproject.toml dor a global parametrization of loop types. I don't really like it, so I'm open for alternative suggestions :) It definitely needs some more thought. Besides parametrizing globally, we also want users to be able to override the loop type for specific tests (see #1032 #1101). Users should be able to pass a @pytest.mark.asyncio(loop_factory=uvloop.new_event_loop)
def test_run_with_uvloop():
... If you're interested in working on the |
That would be a great solution actually. Thanks for helping me come up with a compromise, I'll see what I can do to make that happen. |
Let me revert my changes and then I'll make new ones. |
Guess the only thing I will have to figure out is where loop-factory should get executed I wanted to make it so that both Sequences of factories and singular factories could be accepted. |
Now I think about it I am having trouble finding where to inject loop factories. I'll upload what I have so far and take a temporary break. But I'm trying to make it so that loop_factory can pass multiple factories and run them all. |
This puzzle turned out to be bigger than I thought let me push what I have now so far... |
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
…e a comment explaining it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the effort. I rebased the PR.
I think this is a good start, but there are some edge cases that we didn't think of at first (see my previous PR comment). I added some tests. I think their code explains the issues best.
pytest_asyncio/plugin.py
Outdated
print(f"FACTORY {factory}") | ||
return factory | ||
else: | ||
return request.obj.__dict__.get("_loop_factory", None) # type: ignore[attr-defined] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding is that the else branch is supposed to retrieve the loop factory from a fixture that's requested by the test. This seems reasonable, but not quite enough. It's possible that the fixture pulls in another fixture transitively, which can also have a loop factory.
Therefore, we need to walk the request's _iter_chain
to find loop factories in transitive fixtures.
That still leaves the problem that a test can request multiple fixtures, each of which can define their own loop_factory and pull in other fixtures transitively. Effectively, that means we would have to walk the whole fixture dependency tree upwards. All of that makes me question if we should allow fixtures to set the loop factory.
It gets even more complicated than that: Let's assume all tests and fixtures run with loop_scope="module"
. If two tests in the same module define different loop factories, it's a user error and we have to bail out. However, get_loop_factory
can't really identify the issue, because the function is executed as part of the fixture setup for an individual test. It has no knowledge whatsoever about the loop_factories defined in other tests.
dfa3a50
to
e1af98a
Compare
After mentally thinking about this back and forth. I decided to step to the plate and try and fix this problem once and for all.
The aiohttp developers have told me they had a bit of intrest utilizing this library and so would I with my plans to migrate from unittest to pytest for winloop and a new project coming that aims to upgrade & migrate pycares to cython, which I've added my own deadline that I would move winloop to pytest by 2026. And yes aiothreading which aims to be an alternative to aiomultiprocess for users with slower hardware or providing safer solutions for heavier loads.
While talking with the aiohttp devs on the matrix about the problem. I decided to see if I could have a crack at providing a way to run eventloops without needing to use deprecated setups. Unfortunately I'm extremely picky about deprecated setups This might be a good thing however because the sooner someone does something to solve the problem the better.