Heavy improvements, add async yield fixture, fix bugs, add tests etc.#17
Heavy improvements, add async yield fixture, fix bugs, add tests etc.#17touilleMan merged 5 commits intomasterfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## master #17 +/- ##
===========================================
+ Coverage 30.76% 71.81% +41.04%
===========================================
Files 5 9 +4
Lines 91 220 +129
Branches 16 18 +2
===========================================
+ Hits 28 158 +130
- Misses 56 61 +5
+ Partials 7 1 -6
Continue to review full report at Codecov.
|
|
It seems this code trigger a core dump with Python 3.5.0 and 3.5.2 on travis: I cannot reproduce this behavior on my laptop with Python 3.5.2 @ ubuntu 16.04. Funny thing is the trouble is not present with Python-3.5-dev. My guess is travis doesn't like my use of pytester module (which allows to test a pytest environment from whithin pytest), but not sure what to do to fix this... |
|
It's not pytester. There's some unawaited coroutine somewhere (a routine
called 'fix1', apparently), which causes a warning when it gets garbage
collected. That warning gets issued in a weird context (the coroutine
__del__ method). We're using '-W error' to convert warnings into errors.
And it turns out that 3.5 had a bug where the __del__ method forgot to
account for the possibility that the warning it's trying to issue could
turn into an error, and it segfaults. Eventually someone noticed, but not
until after 3.5.2 was out :-).
Probably the solution is to make sure the coroutine is awaited? Unawaited
coroutines are basically always a bug. It's possible it's needed here b/c
you're intentionally testing a case where things are broken, in which case
there are some other options (trio has some tests like that too).
…On Wed, Dec 13, 2017 at 1:40 AM, Emmanuel Leblond ***@***.***> wrote:
It seems this code trigger a core dump
<https://travis-ci.org/python-trio/pytest-trio/jobs/315786846#L670> with
Python 3.5.0 and 3.5.2 on travis:
Exception ignored in: 'garbage collection'
RuntimeWarning: coroutine 'fix1' was never awaited
ci/travis.sh: line 98: 2437 Aborted (core dumped) pytest -W error -ra -v --pyargs pytest_trio --cov=pytest_trio --cov-config=../.coveragerc --verbose
I cannot reproduce this behavior on my laptop with Python 3.5.2 @ ubuntu
16.04.
Funny thing is the trouble is not present with Python-3.5-dev.
My guess is travis doesn't like my use of pytester module (which allows to
test a pytest environment from whithin pytest), but not sure what to do to
fix this...
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#17 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAlOaLdn8ATgLhHF_oPBVhrKxzPmWPrYks5s_5uFgaJpZM4Q_z1->
.
--
Nathaniel J. Smith -- https://vorpus.org <http://vorpus.org>
|
|
I use the This is from this context the leaking coroutine is, so the warning issued there should not be provided to the main pytest process. The trouble is core dump cannot be ignored as easily as warnings !
Totally agree ! In fact the trick is this test is marked as xfail (it tests that pytest-trio should raise an error when user try to pass an async fixture with a broader scope than the default So in the end we got a xfail test leaking an async coroutine in a sub pytest environment, with warning considered as error configuration (provided to the main pytest environment) passed to the sub environment causing a core dump... Our solutions are:
I would say 1) is the easiest |
|
I'd be fine with moving the test into an issue. Another quick hack option
would be to add an 'await fix1' to the test body; the body isn't supposed
to be executed, but if it is then that would silence the warning...
…On Dec 13, 2017 03:17, "Emmanuel Leblond" ***@***.***> wrote:
I use the testdir fixture provided by pytest to create a new pytest
context (see #17/
files#diff-48851f0efdd4792c878e675e38e301e1R6)
This is from this context the leaking coroutine is, so the warning issued
there should not be provided to the main pytest process. The trouble is
core dump cannot be ignored as easily as warnings !
Unawaited coroutines are basically always a bug.
Totally agree ! In fact the trick is this test is marked as xfail (it
tests that pytest-trio should raise an error when user try to pass an async
fixture with a broader scope than the default function one). Given I
didn't find a clean way to do this so far, I left the test as
documentation/discussion topic in the PR.
So in the end we got a xfail test leaking an async coroutine in a sub
pytest environment, with warning considered as error configuration
(provided to the main pytest environment) passed to the sub environment
causing a core dump...
Our solutions are:
1. remove the test (maybe take the test snippet and put it in an issue
on github about adding a bad fixture scope detection feature)
2. disable the error on warning config passed to the sub pytest
environment, not sure how to do that though
3. remove tests for python 3.5.x [image:
 ]
I would say 1) is the easiest
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#17 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAlOaGLYnGqVG-oJTI-cdfkuimcyluAyks5s_7IygaJpZM4Q_z1->
.
|
|
Issue #18 created, I've removed the test from this PR |
2 participants
This should address #4 and #16
I'm still not happy by the fact an erroring fixture will be considered as a FAILURE instead of an ERROR (this is due to the fact the async fixture is run where the function should be run from the pytest point of view).