-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
regr_test.py
: Allow non-types dependencies
#9382
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
Conversation
|
tests/utils.py
Outdated
assert dependency.startswith("types-"), f"unrecognized dependency {dependency!r}" | ||
dependencies.append(dependency[6:].split("<")[0]) | ||
return tuple(dependencies) | ||
if dependency.startswith("types-"): |
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.
I don't see a problem here, but there are two cases where this logic may do something slightly surprising / different from stub_uploader which will consider something a typeshed stub iff it's something it's uploaded. So maybe worth adding a comment:
This function may consider things to be typeshed stubs even if they haven't been uploaded to PyPI. If a typeshed stub is removed, this function will consider it to be an external dependency.
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.
Your comment made me realise that there is also an edge case that this function doesn't currently handle: stubs which are uploaded with nonstandard names to PyPI. It'll break, for example, if we ever get a stubs package that depends on types-pika-ts
.
This is unlikely to come up in real life, but I may as well fix it.
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.
How does 42f722f look?
I realised that if 9823065 adds much better logging configurability. You can now run |
I'm planning on merging this in a few days, if there are no objections from anybody, as I'd like to move on to working on |
The approach is pretty similar to the approach I took in #9382 for regr_test.py: dynamically create virtual environments for testing stubs packages in isolation. However, since mypy_test.py tests all of typeshed's stubs in succession (and since lots of typeshed's stubs packages depend on other typeshed stubs packages), the performance issues with creating a virtual environment for testing each stubs package are even more severe than with regr_test.py. I mitigate the performance issues associated with dynamically creating virtual environments in two ways: - Dynamically creating venvs is mostly slow due to I/O bottlenecks. Creating the virtual environments can be sped up dramatically by creating them concurrently in a threadpool. The same goes for pip installing the requirements into the venvs -- though unfortunately, we have to use pip with --no-cache-dir, as the pip cache gets easily corrupted if you try to do concurrent reads and writes to the pip cache. - If types-pycocotools requires numpy>=1 and types-D3DShot also requires numpy>=1 (for example), there's no need to create 2 virtual environments. The same requirements have been specified, so they can share a virtual environment between them. This means we don't have to create nearly so many virtual environments.
Work towards #5768
This PR reworks
tests/regr_test.py
so that the script will still succeed if test cases are added for a stubs package that has non-types dependencies. In the process, it undertakes a lot of the refactoring that will be needed to allow non-types dependencies inmypy_test.py
.The approach is very similar to the approach we take in
stubtest_third_party.py
: dynamically create virtual environments on the fly in order to test stubs in isolation. Unlikestubtest_third_party.py
, however, we only create the venv if it's a package that has non-types dependencies listed. Creating a venv for each package with test cases, even if it's a package that didn't have non-types dependencies, would make the script unacceptably slow.Cc. @Avasam