Make it possible to avoid using the global loop

[leifurhauks]

I'm working on a project that aims not to depend on the implicit global loop; that is, if a client developer explicitly passes an event loop instance to classes and functions in this package, only that loop will be used.

Because many other packages, including asyncio itself, will get the default loop when not passed an instance explicitly, we would like to run our tests against a non-global loop instance to enforce this requirement.

To that end, I tried overriding the event_loop fixture, but I found that the pytest-asyncio marker is implicitly retrieving the default event loop via the call to asyncio.async. This proposed change simply passes event_loop explicitly to asyncio.async.

I wrote the corresponding test in its own module so that I could override the event_loop fixture and verify that asyncio marked test functions run correctly even if there is no default event loop set.

[leifurhauks]

I can't seem to reproduce the 3.4 test failure locally.

[Tinche]

Hi,
I'm at a conference atm so I can only deal with this in a few days, but the
change looks right.

On 17:23, Fri, Apr 29, 2016 Leifur Halldor Asgeirsson <
notifications@github.com> wrote:

I can't seem to reproduce the 3.4 test failure locally.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#21 (comment)

[Tinche]

So here's the thing. When I said overriding the event loop would be the way to go, I actually tested it out first using the following code:

import asyncio

import pytest


class CustomSelectorLoop(asyncio.SelectorEventLoop):
    """A subclass with no overrides, just to test for presence."""
    pass


@pytest.fixture()
def event_loop(request):
    """Create an instance of the default event loop for each test case."""
    policy = asyncio.get_event_loop_policy()

    policy.get_event_loop().close()

    loop = CustomSelectorLoop()
    policy.set_event_loop(loop)

    request.addfinalizer(loop.close)
    return loop

as you can see, the event_loop fixture is a little more complex to write. Your fix might solve the problem for you, but chances are third-party libraries might want to get the current event loop using asyncio anyway, and then they'll get another loop.

What I propose we do instead is move the logic of setting the produced event loop outside of the event_loop fixture and into pytest-asyncio itself.

@pytest.mark.tryfirst
def pytest_pyfunc_call(pyfuncitem):
    """
    Run asyncio marked test functions in an event loop instead of a normal
    function call.
    """
    for marker_name, fixture_name in _markers_2_fixtures.items():
        if marker_name in pyfuncitem.keywords:
            event_loop = pyfuncitem.funcargs[fixture_name]

            policy = asyncio.get_event_loop_policy()
            policy.get_event_loop().close()
            policy.set_event_loop(event_loop)

            funcargs = pyfuncitem.funcargs
            testargs = {arg: funcargs[arg]
                        for arg in pyfuncitem._fixtureinfo.argnames}
            event_loop.run_until_complete(
                asyncio.async(pyfuncitem.obj(**testargs)))

            event_loop.close()
            return True

@pytest.fixture
def event_loop(request):
    """Create an instance of the default event loop for each test case."""
    policy = asyncio.get_event_loop_policy()
    return policy.new_event_loop()

[leifurhauks]

Sorry about the delay getting back to you!

I'm not sure I understand your proposal; I can see that the way I had tried overwriting the event_loop fixture may lead to problems, and these changes would certainly simplify that fixture. I'm trying to enforce that when I run my tests, my application code should never access the global loop. How would I do that with these changes?

If a client developer were to call asyncio.set_event_loop(None) and subsequently call my code, explicitly passing an event loop instance, everything should work correctly. It's easy enough to refrain from calling asyncio.get_event_loop in my code, but it's also very easy to forget to pass a loop instance to a function like asyncio.ensure_future, which will then attempt to get the global loop.

In my local conftest, I overwrote the event_loop fixture and set the global loop as None so that if the tests run any application code that tries to get the global loop, the test should fail with a RuntimeError. This approach wouldn't work if the pytest asyncio marker sets the result of the event_loop fixture as the global loop before running the test method. All my application code would run on the correct loop, but my tests wouldn't pick up on places where my application accesses the global loop.

I did some googling and found that in the asynctest package, if the forbid_get_event_loop attribute is set on a test class, calls to asyncio.get_event_loop will result in an assertion error; is there some way to have a similar option in pytest-asyncio?

For example, maybe a marker pytest.mark.asyncio_forbid_global_loop that runs the test function in an event loop like the regular pytest.mark.asyncio, but uses an event loop policy whose implementation of get_event_loop raises AssertionError.

[Tinche]

Ah, I understand now. Let me dwell on it a little. Ideally we'd both have a simplified way of overriding the event loop, and a way of making get_event_loop throw errors. The asyncio_forbid_global_loop marker might not be a bad idea.

[Tinche]

How about this approach:

@pytest.mark.asyncio(forbid_global_loop=True)
async def test_blabla(event_loop):
    ...

[leifurhauks]

That would be perfect!

[leifurhauks]

I could write a possible implementation for that on Monday.

[Tinche]

Don't worry about it, I've already written it playing around. :)

On Sun, May 8, 2016 at 11:55 PM Leifur Halldor Asgeirsson <
notifications@github.com> wrote:

I could write a possible implementation for that on Monday.

—
You are receiving this because you commented.

Reply to this email directly or view it on GitHub
#21 (comment)

[Tinche]

I'll merge this so you're in the contributors. I'll build on it in the next few days with the changes.

Thanks for contributing!