-
Notifications
You must be signed in to change notification settings - Fork 35
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Switch pytest-asyncio #14
Conversation
e738fb0
to
f7660a4
Compare
Current coverage is 97.43% (diff: 100%)@@ master #14 diff @@
==========================================
Files 1 1
Lines 78 78
Methods 0 0
Messages 0 0
Branches 13 13
==========================================
Hits 76 76
Misses 1 1
Partials 1 1
|
flake8==3.2.1 | ||
ipdb==0.10.1 | ||
pytest==3.0.5 | ||
pytest-cov==2.4.0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please leave it as it is, I added pyup (https://pyup.io/) bot to this repo we will have automatic PR with dependency update, see example here aio-libs/aiomonitor#10 Other aio-libs do the same
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
61c6fb3
to
7cd69ab
Compare
@@ -6,8 +6,8 @@ | |||
from aiohttp_sse import EventSourceResponse | |||
|
|||
|
|||
@pytest.mark.run_loop | |||
def test_func(loop, unused_port): | |||
@pytest.mark.asyncio |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we use pytest.mark.asyncio(forbid_global_loop=True)
instead? Previous implementation had asyncio.set_event_loop(None)
so we can get handy error if loop is not passed explicitly somewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point
I believe loop issue should be resolved, other then that LGTM |
And cleanup pending tasks before exiting tests
b523d14
to
f434545
Compare
@jettify thx for the review. That was fast |
Please merge PR, I believe you have access rights |
I can't. |
I do belong to the team aiohttp-sse-admins. But it seems this group doesn't have write access. |
it does... weird |
alright this is sorted out. |
@ticosax Thanks! |
No description provided.