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

Attempt to speed up tests with xdist. #73

Closed
wants to merge 1 commit into from

Conversation

jsirois
Copy link
Contributor

@jsirois jsirois commented Feb 23, 2020

No description provided.

@jsirois jsirois marked this pull request as ready for review February 23, 2020 20:23
@jsirois
Copy link
Contributor Author

jsirois commented Feb 23, 2020

This does speed up tests on OSX modestly, perhaps 10-15%. On my machine it speeds things up by ~60%. Perhaps more importantly this caught a bug in Pants pre-Pex 2.x:

...
New virtual environment successfully created at /Users/travis/tmp/popen-gw2/test_pants_1_260/PANTS_HOME/bootstrap-Darwin-x86_64/1.26.0.dev0_py36.
20:14:56 [INFO] Resolving new plugins...:
  pantsbuild.pants.contrib.go==1.26.0.dev0
/Users/travis/tmp/popen-gw2/test_pants_1_260/PANTS_HOME/bootstrap-Darwin-x86_64/1.26.0.dev0_py36/lib/python3.6/site-packages/pex/pep425tags.py:274: DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses
  import imp
ERROR: [Errno 66] Directory not empty: '/Users/travis/.cache/pants/plugins/tmpubx60nc2' -> '/Users/travis/.cache/pants/plugins/twitter.common.log-0.3.11-py3-none-any.whl-install'
...

This should not be a problem in pantsbuild.pants>=1.26.0.dev1 since it uses Pex 2+ which includes pre-installation of wheels in directory chroots and it does so via a multi-process concurrency safe atomic directory primitive.

Another - not sure it's worth it change.

@jsirois
Copy link
Contributor Author

jsirois commented Feb 23, 2020

I'll leave the CI shard with the concurrency error red until you review. If you like it, then I can click retry and we'll have known flaky <=1.26.0.dev0 Pants executions.

Copy link
Contributor

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

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

Looks good. Thanks for making so many improvements to this repo recently!

@Eric-Arellano
Copy link
Contributor

then I can click retry and we'll have known flaky <=1.26.0.dev0 Pants executions.

Your call on if we're okay with the flakes. I was already planning on no longer testing <= 1.26.0 so that we can guarantee pants.toml support.

@jsirois
Copy link
Contributor Author

jsirois commented Feb 23, 2020

@Eric-Arellano it's probably best you try this on OSX 1st. /tmp/... appears to get realpathed by the time watchman sees it and under OSX that leads to a too long pathname. So if tox -etest fails for you with too long a pathname, this PR is no good without a solution to OSX shenanigans.

@benjyw
Copy link
Contributor

benjyw commented Feb 8, 2023

Closing this due to #142 - we will get test concurrency via Pants once that is merged.

@benjyw benjyw closed this Feb 8, 2023
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.

3 participants