Skip to content
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

No event loop in the thread 'MainThread' when using test_client fixture #1069

Closed
argaen opened this issue Aug 11, 2016 · 9 comments
Closed

Comments

@argaen
Copy link
Member

argaen commented Aug 11, 2016

Long story short

When using the test_client pytest fixture and calling other functions that make use of the get_event_loop function internally like for example asyncio.Task.current_task() it raises the RuntimeError: There is no current event loop in thread 'MainThread' exception.

This forces the user to pass the loop explicitly to all calls but we are forcing the user to:

  1. Modify the code in order to support testing with it
  2. Passing explicitly the loop to all calls that need it which is not always possible because the request object is not propagated to all the code.

Expected behaviour

Normal calls that require a loop should work without needing to pass the loop explicitly.

@argaen
Copy link
Member Author

argaen commented Aug 11, 2016

Okay, so pulling out what I wrote here: https://groups.google.com/forum/#!topic/aio-libs/sHE53v8UaG0 which was the same case, the issue #939 and some hours of being lost I've got it working.

In order to have the event loop available to the whole code, here is a test example:

import pytest

from aiohttp.test_utils import loop_context

from myapp.api import app_factory

@pytest.yield_fixture
def loop():
    with loop_context() as loop:
        yield loop

@pytest.yield_fixture
def event_loop(loop):
    """
    This is needed for correct functioning of the test_client of aiohttp together with
    pytest.mark.asyncio pytest-asyncio decorator. For more info check the following link:
    https://github.com/KeepSafe/aiohttp/issues/939
    """
    loop._close = loop.close
    loop.close = lambda: None
    yield loop
    loop.close = loop._close

@pytest.mark.asyncio
async def test_valid_get_request(test_client, my_mock_data):
    client = await test_client(app_factory)
    resp = await client.get('/', params=my_mock_data)

    assert resp.status == 200

Things to note:

  • We have to override the event_loop pytest-asyncio fixture (yeah, need to install pytest-asyncio). For more info check Make loop accessible outside of test function pytest-dev/pytest-asyncio#29.
  • Note that you need pytest-asyncio and pytest-aiohttp so you have all the fixtures installed.
  • With that, calls like asyncio.Task.get_current() or aioes calls will work without the need of explicitly having to pass the loop over your code.

As an observation, I think we should try to support this in aiohttp transparently without the need of doing this workarounds (i.e. using pytest-asyncio and overriding the event_loop fixture).

Hope this helps to someone else.

@asvetlov
Copy link
Member

Sorry, but I'm inclining to reject the feature request.
Passing explicit event loop everywhere is definitely possible, at least if you use well-written libraries. aiohttp itself supports it and its test suite uses explicit event loop BTW.
Moreover it is the only way for writing testable applications.

Implicit event loop usage looks seductive but it's really the wrong way I believe.
Unit tests should be isolated from each other, it is the crucial unittest feature.
For forbidding tests mutual interference every test should be executed with own loop.
I believe it's obvious statement.

What's wrong with the following code?

from motor.motor_asyncio import AsyncIOMotorClient
DBNAME = 'testdb'
db = AsyncIOMotorClient()[DBNAME]

It grabs default event loop for db object! If you don't pass a loop to AsyncIOMotorClient() the instance does something like:

if loop is None:
    loop = asyncio.get_event_loop()

That's pretty fine but a call like await db.users.find(...) hangs forever if you execute it with not not default loop at the moment of module import but with a new loop created especially for the test run.

Adding the following lines to your test doesn't help at all:

def test_something():
    loop = asyncio.new_event_loop()
    asyncio.set_event_loop(loop)

The reason is: your new default event loop is not the same as event loop used at module import time.

As result your test will hang forever.

Please bethink about the given example.

@asvetlov
Copy link
Member

P.S. I want to keep the issue opened for a while.
Just for easy searching for reference.
Closed issues are harder to find them.

@argaen
Copy link
Member Author

argaen commented Aug 15, 2016

Yeah, you are right about the issue with the modules that create the loop at import time. Didn't think about that one.

Anyway, I don't think it's a clean way to pass the loop explicitly to all calls. Imagine you have something like that:

example

In order to be able to build the tests, we have to pass the loop instance explicitly from top caller to bottom only because, we want to be able to do the tests.

IMHO, it's cleaner to wrap those modules in a function like setup_motor or use a pytest fixture that inits it after the loop creation than rather having to modify your code on purpose to support them.

Just to finish, if this feature doesn't make it through, I propose a section in the documentation to highlight this. It can save time to others.

@asvetlov
Copy link
Member

Please make a PR for documentation update.

@argaen
Copy link
Member Author

argaen commented Aug 19, 2016

I've come with what I think, it's a clean solution :). You just need to define your low level service with the explicit loop kwarg. Then in the test, you just have to patch it in order to pass the loop from the fixture everytime is called. It would look something like that:

with patch("main.AioESService", MagicMock(return_value=AioESService(loop=loop))):
    client = await test_client(create_app)
    resp = await client.get("/")
    assert resp.status == 200

With that you can just leave the rest of top level services without needing the explicit loop.

I'm gonna upload now the merge request with the documentation updates

@asvetlov
Copy link
Member

Fixed by #1097

@d21d3q
Copy link

d21d3q commented Apr 13, 2018

According to #1097, if well written service should allow to pass loop explicitly, then why loop argument in Application class is deprecated and _set_loop method is prefixed with _ (which suggests that it is meant for internal use)?

@lock
Copy link

lock bot commented Oct 28, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a [new issue] for related bugs.
If you feel like there's important points made in this discussion, please include those exceprts into that [new issue].
[new issue]: https://github.com/aio-libs/aiohttp/issues/new

@lock lock bot added the outdated label Oct 28, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants