Skip to content

handle case when no event loop exists (support pytest-aiohttp)#64

Merged
Tinche merged 2 commits into
pytest-dev:masterfrom
smagafurov:master
Sep 13, 2017
Merged

handle case when no event loop exists (support pytest-aiohttp)#64
Tinche merged 2 commits into
pytest-dev:masterfrom
smagafurov:master

Conversation

@smagafurov

@smagafurov smagafurov commented Jul 24, 2017

Copy link
Copy Markdown
Contributor

Solves this:

And this:

Now pytest-asyncio and pytest-aiohttp can act together with this conftest.py:

from aiohttp.test_utils import loop_context


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


@pytest.yield_fixture
def event_loop(loop):
    yield loop

@smagafurov

Copy link
Copy Markdown
Contributor Author

@asvetlov, hi

see conftest.py in my comment above

How do you think, is it ok to run pytest-asyncio and pytest-aiohttp together in such manner?

@spumer

spumer commented Aug 2, 2017

Copy link
Copy Markdown

Yea! This is very usefull!

@asvetlov

asvetlov commented Aug 7, 2017

Copy link
Copy Markdown
Contributor

Looks good at first glance

@argaen

argaen commented Sep 8, 2017

Copy link
Copy Markdown

I really should check open PR first before doing mine 😓 . Looks good to me too, only thing I'm missing there is a test, you can check #65, copy paste for the test should do it.

@Tinche any intentions on merging this? :)

@Tinche

Tinche commented Sep 9, 2017

Copy link
Copy Markdown
Member

The patch looks simple enough, I'd be willing to merge it in. But wouldn't a better course of action be to reach out to pytest-aiohttp and see if they would be willing to depend on pytest-asyncio, instead of merging in workarounds?

@argaen

argaen commented Sep 9, 2017

Copy link
Copy Markdown

Now it's pytest-aiohttp the one having this edge case but anyone can do an asyncio.set_event_loop(None) in a pytest fixture and then pytest-asyncio starts crashing. IMO this should be solved regardless of what other libraries do

@smagafurov

Copy link
Copy Markdown
Contributor Author

test added (copy pasted from #65)

@bugok

bugok commented Sep 11, 2017

Copy link
Copy Markdown

+1

I'm hitting this problem as well. Would be happy to see this getting merged in.

@Tinche Tinche merged commit 54d1d55 into pytest-dev:master Sep 13, 2017
@Tinche

Tinche commented Sep 13, 2017

Copy link
Copy Markdown
Member

Alright, thanks for the contribution. My plea for pytest-asyncio and pytest-aiohttp to work better together still stands :)

@smagafurov

Copy link
Copy Markdown
Contributor Author

can we release minor version with this fix?

@Tinche

Tinche commented Sep 21, 2017

Copy link
Copy Markdown
Member

I'll get it out over the weekend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants