Re-introduce pytest-xdist in supported envs
#5431
Conversation
webknjaz
added
infra
bot:chronographer:skip
webknjaz
force-pushed
the
testing/pytest-xdist
branch
3 times, most recently
from
7ed6246
to
392ebbb
Compare
webknjaz
force-pushed
the
testing/pytest-xdist
branch
from
392ebbb
to
edec305
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #5431 +/- ##
=======================================
Coverage 97.64% 97.64%
=======================================
Files 107 107
Lines 33317 33392 +75
Branches 3916 3924 +8
=======================================
+ Hits 32531 32605 +74
- Misses 568 569 +1
Partials 218 218
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
webknjaz
force-pushed
the
testing/pytest-xdist
branch
4 times, most recently
from
7dd330f
to
18dd594
Compare
webknjaz
force-pushed
the
testing/pytest-xdist
branch
from
18dd594
to
c4eab87
Compare
webknjaz
force-pushed
the
testing/pytest-xdist
branch
from
ef79513
to
e1f51b9
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
tests/_pytest_plugin.py
Outdated
| from typing import TYPE_CHECKING, List | ||
|
|
||
| try: | ||
| from asyncio import ThreadedChildWatcher |
Check notice
Code scanning / CodeQL
Unused import Note test
webknjaz
force-pushed
the
testing/pytest-xdist
branch
from
9cc0f30
to
93dc6bd
Compare
webknjaz
added
the
backport-3.10
webknjaz
force-pushed
the
testing/pytest-xdist
branch
from
26d0b16
to
763a477
Compare
webknjaz
force-pushed
the
testing/pytest-xdist
branch
2 times, most recently
from
b6a6440
to
1989754
Compare
|
@Dreamsorcerer @bdraco any ideas why the http parser tests might be failing in with pytest-xdist? |
webknjaz
force-pushed
the
testing/pytest-xdist
branch
from
d297e48
to
92f0b5a
Compare
webknjaz
requested review from
bdraco and
Dreamsorcerer
and removed request for
asvetlov
|
@Dreamsorcerer @bdraco could you check the changes in tests especially carefully? Looks like I made it work, and it makes the CI complete 3x faster. Some cleanup pending. But I'm mostly worried about the changes in tests — perhaps you'd be able to point out better ways of dealing with them. |
| @@ -118,6 +119,7 @@ async def test_secure_https_proxy_absolute_path( | |||
|
|
|||
| await sess.close() | |||
| await conn.close() | |||
| await asyncio.sleep(0.1) | |||
There was a problem hiding this comment.
0.1 seems a bit brittle. Does this need one iteration of the event loop or is it waiting for a specific timer?
| await asyncio.sleep(0.1) | |
| await asyncio.sleep(0) |
There was a problem hiding this comment.
I'm not sure. I was experimenting w/o really debugging in many of the cases. And sometimes 0 wasn't helping… I'll need to take a look again, I suppose.
| @@ -190,6 +192,8 @@ async def test_https_proxy_unsupported_tls_in_tls( | |||
| await sess.close() | |||
| await conn.close() | |||
|
|
|||
| await asyncio.sleep(0.1) | |||
There was a problem hiding this comment.
The test changes seem fine. However |
webknjaz
force-pushed
the
testing/pytest-xdist
branch
from
92f0b5a
to
7333846
Compare
| @@ -1906,6 +2012,8 @@ def check_with_exc(err): | |||
|
|
|||
| assert not conn._waiters | |||
|
|
|||
| yield from conn.close() | |||
There was a problem hiding this comment.
I wonder if this function should just be converted to an async function? Maybe the test just predates async syntax...
The codecov comment says that it's not running though, which makes sense given all we are doing is creating a generator object. So, we never actually run any of the function body...
There was a problem hiding this comment.
That's an interesting idea… I'll have to check. I'm tired, working on this patch on and off for years. So at this point, I'm almost ready to accept any suggestion that would get this merged. I just want to be careful not to break anything. I think the last couple of times (like 6–7 years ago) I attempted integrating xdist here, and it brought a lot of flakiness that would become evident in Travis CI. So I'm both invested and annoyed at this point :)
I was under the impression this should not be needed on master (v4) anymore, so shouldn't be a factor in these tests. Unless it is something different from: https://docs.aiohttp.org/en/stable/client_advanced.html#graceful-shutdown |
bdraco
added
backport-3.11
What do these changes do?
This change resurrects parallel test execution in most of the test envs. On my laptop, the full test run is almost 3x faster: 36.04s vs 105.53s (0:01:45).
Refs:
pytest-xdistbreaks asyncio-based code expecting to be run in the main thread pytest-dev/pytest-xdist#620Are there changes in behavior for the user?
Nope.
Related issue number
#3419 + #3450
Checklist
CONTRIBUTORS.txtCHANGESfolder<issue_id>.<type>for example (588.bugfix)issue_idchange it to the pr id after creating the pr.feature: Signifying a new feature..bugfix: Signifying a bug fix..doc: Signifying a documentation improvement..removal: Signifying a deprecation or removal of public API..misc: A ticket has been closed, but it is not of interest to users.