Skip to content
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

Use pytest-xdist #3707

Merged
merged 1 commit into from
Apr 17, 2019
Merged

Use pytest-xdist #3707

merged 1 commit into from
Apr 17, 2019

Conversation

Gallaecio
Copy link
Member

@Gallaecio Gallaecio commented Mar 29, 2019

In summary:

  • Add pytest-xdist to tests/requirements*.txt and remove versions from pytest* packages.
  • In the tox configuration, add -n auto for all environments running pytest.
  • Because Windows does not seem to support pytest-xdist cleanly, create a new tox environment specifically for Windows that does not use pytest-xdist (no -n auto).

@Gallaecio Gallaecio force-pushed the pytest-xdist branch 2 times, most recently from 00298da to f1aff2a Compare March 29, 2019 16:30
@codecov
Copy link

codecov bot commented Mar 29, 2019

Codecov Report

Merging #3707 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #3707   +/-   ##
=======================================
  Coverage   85.42%   85.42%           
=======================================
  Files         169      169           
  Lines        9635     9635           
  Branches     1433     1433           
=======================================
  Hits         8231     8231           
  Misses       1156     1156           
  Partials      248      248

@Gallaecio Gallaecio changed the title [WIP] Use pytest-xdist Use pytest-xdist Mar 29, 2019
@kmike
Copy link
Member

kmike commented Apr 15, 2019

Hey! I'm surprised tests pass, I was thinking they won't work out of box 👍

What do you think about adding pytest-xdist to tox, documenting how to use it, but not having -n auto by default, or at least allowing to override it? In some cases using all CPUs may be not desirable, as it may use too much RAM, and make tests slower, not faster; also, with pytest-xdist sometimes it is harder to stop tests cleanly. It could also make things slower if you're just running a few tests, selected by -k, as setup of these processes may take longer than just running everything in a single process.

@Gallaecio
Copy link
Member Author

Then, what about simply installing pytest-xdist and documenting its usage?

If you have multiple CPU cores, you can use ``-n <number>`` to use a specific
number of CPU cores to run the tests, or ``-n auto`` to use all of them::

tox -- scrapy tests -n auto
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think in this example tests are executed for all environments; there is a different way to parallelize it (https://tox.readthedocs.io/en/latest/example/basic.html#parallel-mode). I'm not sure what is better, to run individual environments with several processes, or to run environments in parallel in this case. What do you think about having e.g. tox -e py36 -- scrapy tests -n auto as an example?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can actually use both in combination, as it turns out, but I guess it’s better to cover them separately, and let developers do what works best for them.

@kmike
Copy link
Member

kmike commented Apr 17, 2019

@Gallaecio Thanks, the PR looks great!

@kmike
Copy link
Member

kmike commented Apr 17, 2019

Thanks @Gallaecio!

@kmike kmike merged commit 5b667b6 into scrapy:master Apr 17, 2019
@kmike kmike added this to the v1.7 milestone Jun 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants