-
-
Notifications
You must be signed in to change notification settings - Fork 610
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
Parallelize pytest runs #1357
Parallelize pytest runs #1357
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1357 +/- ##
=======================================
Coverage 99.65% 99.65%
=======================================
Files 33 33
Lines 2915 2915
Branches 308 308
=======================================
Hits 2905 2905
Misses 5 5
Partials 5 5 Continue to review full report at Codecov.
|
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.
Thanks for bringing this! I remember we made a while ago some preparations to make this possible. See #1022 (review).
Thanks @webknjaz! |
Hey, I wasn't invited to the review party, but don't think this should have gone through without setting unique cache dirs for parallel EDIT: We already had unique cache dirs implemented, and that behavior should indeed have been expected, it just happened to have broken unnoticed, independently of this PR. |
All the class CliRunner(object):
"""The CLI runner provides functionality to invoke a Click command line
script for unittesting purposes in a isolated environment. This only
works in single-threaded systems without any concurrency as it changes the
global interpreter state.
""" |
And I see here we have a no-longer-helpful fixture for this: @pytest.fixture(autouse=True)
def _temp_dep_cache(tmpdir, monkeypatch):
monkeypatch.setenv("PIP_TOOLS_CACHE_DIR", str(tmpdir / "cache")) But that env var was removed in d786232, merged in #1238, which silently broke this fixture. |
I think it's good that this got merged because it reveals problems with tests that may depend on the shared state/order. This gives us a chance to identify the flaky tests and actually fix them over time. Plus, going forward it'll make the test authors more cautious and will encourage everyone to consider the test stability... |
I'm pretty sure pytest-xdist distributes tests across subprocesses, not threads. So this shouldn't be a problem. |
I'm thinking the real bug here is in #1238, where the intent seemed to be only to remove env vars handled by pip itself, and |
Fixes bug introduced in jazzbandGH-1238 and exposed by jazzbandGH-1357
$sbj. This change makes a single pytest run almost 4x faster on my machine:
I'm pretty sure it'll have a positive impact on the CI completion time too. UPD: in GHA it's about 2x faster in this mode.
Changelog-friendly one-liner: (misc/non user facing?) Enabled pytest to run tests in parallel.
Contributor checklist