Aiohttp: Rely on pytest-asyncio capabilities

[argaen]

I think it would be healthy to rely some of the asynchronous testing capabilities to https://github.com/pytest-dev/pytest-asyncio/.

For example it provides support for asynchronous fixtures, provides an event_loop fixture, etc.

IMO pytest-aiohttp should dedicate to improve/provide fixtures related to aiohttp itself like test_client and the likes and let the other things more asyncio related to pytest-asyncio. This way we would also avoid problems (https://github.com/pytest-dev/pytest-asyncio/issues/52) where users use pytest-asyncio and pytest-aiohttp for testing (myself included)

If you agree I volunteer to start a MR to see how it would look like.

[asvetlov]

What is MR?

[argaen]

Pull Request/Merge Request. Github/Gitlab terminology :)

[asvetlov]

You could try but I expect pitfalls on this way.

[argaen]

I'll have a look at it and see how it goes

[hynek]

As far as I can tell, currently the testing facilities of aiohttp (like aiohttp_client) are incompatible with pytest-asyncio. I get errors about missing event_loops which suggests that they screw up each others setups:

========================================================================= ERRORS =========================================================================
_____________________________________________ ERROR at setup of TestPool.test_remove_from_pool_simple[None] ______________________________________________

reader = <StreamReader>, writer = FakeWriter(_buf=b'', _closed=False), event_loop = <_UnixSelectorEventLoop running=False closed=False debug=False>
timeouts = BackendConfig.Registry.Timeouts(connect=10, request=100, keep_alive=10)

    @pytest.fixture
    def client(reader, writer, event_loop, timeouts):
        """
        A future of a functional raw client.
        """
>       connector = asyncio.Future()

conftest.py:120:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
../../.pyenv/versions/3.6.4/lib/python3.6/asyncio/events.py:694: in get_event_loop
    return get_event_loop_policy().get_event_loop()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <asyncio.unix_events._UnixDefaultEventLoopPolicy object at 0x10f5cb240>

    def get_event_loop(self):
        """Get the event loop.

            This may be None or an instance of EventLoop.
            """
        if (self._local._loop is None and
            not self._local._set_called and
            isinstance(threading.current_thread(), threading._MainThread)):
            self.set_event_loop(self.new_event_loop())
        if self._local._loop is None:
            raise RuntimeError('There is no current event loop in thread %r.'
>                              % threading.current_thread().name)
E           RuntimeError: There is no current event loop in thread 'MainThread'.

../../.pyenv/versions/3.6.4/lib/python3.6/asyncio/events.py:602: RuntimeError

As you can see this happens even though I’m not using the fixtures at once.

This is a super bummer because one of the reasons I love to using async frameworks is the option of easily mixing a web framework with general purpose network code. While I’m sure interop can be created in other ways, it would be really nice if we had one really good goto solution that everyone relied on. :)

[d21d3q]

According to #1097, well written app allows to passing loop explicitly on init.
I wrote my doubts in #1069 but since that issue is closed, I'll repeat them here.
So why loop argument in Application class is deprecated and _set_loop method is prefixed with _ (which suggests that it is meant for internal use)?
(Edit: Is it only for deprecation period, and in future it will become loop setter?)

I've managed to get rid of 'There is no current event loop in thread %r.' error, by having app.py like this:

import asyncio
from aiohttp.web import Application, run_app

class Foo:
    def __init__(self, app):
        self.queue = asyncio.Queue(loop=app.loop)  # this will crush with pytest without current loop

def make_app(loop=None):
    app = Application()
    if loop is not None:
        app._set_loop(loop)

    app.foo = Foo(app)  # this one will get loop from app 

if __name__ == '__main__':
    run_app(make_app())

and in conftest.py:

import pytest
from myapp import make_app

@pytest.fixture
def cli(loop, aiohttp_client):
    return loop.run_until_complete(aiohttp_client(make_app(loop)))

[asvetlov]

What pytest plugin do you use for running async tests? pytest-asyncio or pytest-aiohttp?

[d21d3q]

I have both installed. Not all tests are related to aiohttp.

[hynek]

Yeah I think this is a key point: if I write a “normal” web app, I just go for Falcon or Pyramid. I specifically use aiohttp only if the application has a good reason to be async. That usually means that it communicates with the world in other ways than just HTTP.

That’s why aiohttp's testing facilities remain useless to me as long as they stop me from testing the usually more important parts of my applications.

[asvetlov]

Well, simple question: What pytest-asyncio feature is missing in pytest-aiohttp?

[hynek]

Excellent question! I kind of assumed it’s lacking something but it doesn’t seem to. It even saves me the @pytest.mark.asnycio marker! I just converted my test suite and everything seems to be working fine, so I think I’m good!

[asvetlov]

Nice to her it.
pytest-asyncio is a pretty good name, pytest-aiohttp should work with it eventually.
But it is not the first priority because pytest-aiohttp just works.
Say again pytest-asyncio and pytest-aiohttp used very different approaches for the loop life-cicle and other low details but now the difference is not such bug (but still exists).

I'll manage a smooth transition someday but keeping backward compatibility is a pain as usual.