Support python 3.7 and 3.8 in travis CI #493
Conversation
MateuszCzubak
commented
May 28, 2020
•
MateuszCzubak
commented
- Fix tests to pass on python 3.7 and 3.8 versions
- Support python 3.7 and 3.8 in Travis CI
This comment has been minimized.
This comment has been minimized.
MateuszCzubak
force-pushed
the
support-python-3.7-and-3.8-in-travis
branch
6 times, most recently
from
3d2b1c9
to
e2064b7
Compare
|
@terrycain @webknjaz can you please take a look? Hopefully these changes will make possible to release the next version. |
MateuszCzubak
force-pushed
the
support-python-3.7-and-3.8-in-travis
branch
from
0240e2a
to
95aeac5
Compare
| @@ -131,7 +131,7 @@ async def test_create_pool_deprecations(mysql_params, loop): | |||
| warnings.simplefilter("always") | |||
| async with pool.get() as conn: | |||
| pass | |||
| assert issubclass(w[-1].category, DeprecationWarning) | |||
| assert issubclass(w[0].category, DeprecationWarning) | |||
There was a problem hiding this comment.
This is necessary for tests to pass on P3.7 with PYTHONASYNCIODEBUG=1, for which there are two warnings emitted:
(Pdb) print([x.category for x in w])
[<class 'DeprecationWarning'>, <class 'ResourceWarning'>]
It doesn't happen on P3.8 though. Here's related build result:
https://travis-ci.com/github/aio-libs/aiomysql/jobs/341264313
There was a problem hiding this comment.
The ResourceWarning msg:
ResourceWarning('unclosed event loop <_UnixSelectorEventLoop running=False closed=False debug=True>')
coming from asyncio/base_events.py line 623 (def __del__(self))
There was a problem hiding this comment.
Okay, in this case, we should add a code comment explaining this magic of choosing which element to compare.
There was a problem hiding this comment.
I have added a code comment. I have also added some explanation in the commit msg.
There was a problem hiding this comment.
Honestly that ResourceWarning is a bug that may mean that our tests don't close the loop properly, upon teardown. Does this happen under Python 3.8?
There was a problem hiding this comment.
No, this happens only under 3.7
MateuszCzubak
force-pushed
the
support-python-3.7-and-3.8-in-travis
branch
4 times, most recently
from
7500b11
to
2fd34f2
Compare
| @@ -131,7 +131,9 @@ async def test_create_pool_deprecations(mysql_params, loop): | |||
| warnings.simplefilter("always") | |||
There was a problem hiding this comment.
Looking at this test closer, we could probably just suppress all the warnings except for the DeprecationWarning instead of trying to guess on what position it'll be under the next Python version.
Ref: https://docs.python.org/3/library/warnings.html#overriding-the-default-filter.
@MateuszCzubak WDYT?
There was a problem hiding this comment.
@webknjaz I think that makes a lot of sense but it will look weird to limit to DeprecationWarning just in this one place and not in the others. Maybe it would be better to switch all related tests like this in a "refactoring commit", before the actual p3.7/p3.8 compatibility commit?
There was a problem hiding this comment.
Hm actually the are only 2 invokations of warnings.simplefilter("always") - I think it should be enough to change them both within the same commit - WDYT?
There was a problem hiding this comment.
Well, if you want to switch all, this needs to go into a separate PR. Let me merge this one and then we can do a follow-up.
MateuszCzubak
force-pushed
the
support-python-3.7-and-3.8-in-travis
branch
from
2fd34f2
to
165ef15
Compare
Some changes in tests were needed to make them pass: - `issubclass(w[0].category, DeprecationWarning)` - previous version breaks on python 3.7 executed with PYTHONASYNCIODEBUG=1 - `disable_gc` fixture - I can only suspect that in p3.7 and p3.8 the connection object was automatically garbage collected before the `gc.collect()` was invoked. - curosor `async for` usage was incorrect
MateuszCzubak
force-pushed
the
support-python-3.7-and-3.8-in-travis
branch
from
165ef15
to
e4cba8f
Compare
