Skip to content

Make pep561 tests parrallel and sandboxed #5060

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

Merged
merged 5 commits into from
Jun 15, 2018

Conversation

emmatyping
Copy link
Member

@emmatyping emmatyping commented May 16, 2018

Each test is run in a seperate virtualenv which is deleted after the
tests. This means they can be run in parrallel, and caused about a 2x
speedup for pep561 on my machine (4x parrallel). This also will allow for the tests
to scale better with egg support, and bypass issues with uninstalling
some types of packages.

Each test is run in a seperate virtualenv which is deleted after the
tests. This means they can be run in parrallel, and caused about a 30%
speedup on my machine (4x parrallel). This also will allow for the tests
to scale better with egg support, and bypass issues with uninstalling
some types of packages.
@emmatyping emmatyping changed the title Make pep561 tests parrallel and sandboxed [WIP] Make pep561 tests parrallel and sandboxed May 16, 2018
@emmatyping emmatyping changed the title [WIP] Make pep561 tests parrallel and sandboxed Make pep561 tests parrallel and sandboxed May 16, 2018
@emmatyping
Copy link
Member Author

This is ready for review now. It isn't as important as I decided egg support serves little purpose, but it provides a nice speedup, and makes things cleaner.

@emmatyping
Copy link
Member Author

I just realized I might have been a bit vague in my last message. I'm hoping to get this merged soonish so that I can work on adding support for editable installs of packages (which I already have an implementation of), then support for partial stub packages. If someone finds the time I'd appreciate a review.

Copy link
Member

@ilevkivskyi ilevkivskyi 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. Just have one question.

'-y', pkg], cwd=package_path)
if returncode != 0:
self.fail('\n'.join(lines))
yield
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure what is the benefit of context manager which has yield as a last statement. Can't be this just made a function?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. I will refactor things into just using this as a function.

@emmatyping emmatyping merged commit 595545f into python:master Jun 15, 2018
ilevkivskyi pushed a commit that referenced this pull request Jun 17, 2018
When working on partial stub packages I realized that the changes in #5060 were flaking. This change runs mypy in each temporary directory, which avoids parrallelism/cache issues (sometimes the multiple processes would try to read/write the same cache file). To test this thoroughly, I ran the test 25 times in a row, with no failures.
@emmatyping emmatyping deleted the pep561test branch October 9, 2018 18:37
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.

2 participants