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

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

Merged
merged 2 commits into from Sep 13, 2017

Conversation

smagafurov
Copy link
Contributor

@smagafurov smagafurov commented Jul 24, 2017

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
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
Copy link

spumer commented Aug 2, 2017

Yea! This is very usefull!

@asvetlov
Copy link
Contributor

asvetlov commented Aug 7, 2017

Looks good at first glance

@argaen
Copy link

argaen commented Sep 8, 2017

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
Copy link
Member

Tinche commented Sep 9, 2017

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
Copy link

argaen commented Sep 9, 2017

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
Contributor Author

test added (copy pasted from #65)

@bugok
Copy link

bugok commented Sep 11, 2017

+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
Copy link
Member

Tinche commented Sep 13, 2017

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

@smagafurov
Copy link
Contributor Author

can we release minor version with this fix?

@Tinche
Copy link
Member

Tinche commented Sep 21, 2017

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.

None yet

6 participants