-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Add a --use-zipapp option to the test suite #11250
Conversation
Wow that slow down the test suite a lot. |
Hmm, it did, didn't it? I mainly wanted to add it to CI to test on a "proper" Linux box, as I was getting a single test failure locally on WSL, which I think is due to it being WSL ( It's possible that I've got the zipapp fixture wrong, and it's building the zipapp multiple times. I've no idea how to check that or how to fix it if it is the case. I'd really appreciate someone who understands pytest and our test suite taking a look at that code. It's also possible that running from a zipapp is just really slow - the zipapp doesn't contain |
I'm going to take the "draft" status off this PR, as all the tests are passing so the zipapp approach seems pretty solid. I'm happy to remove zipapp from CI, if the performance is too much of a problem (I'm inclined to think that it is) and it can't be fixed. I did some checks running various versions of pip, using hyperfine. The basic conclusions were:
I'm inclined to think that the overall implication here is that the slowness is down to Python's "zipimport" mechanism, and not something we can address in pip. Which is disappointing, as it may have implications for zipapp as a distribution mechanism for pip. I still think it's a viable approach, and if we add it initially as "experimental", we can get some feedback from users as to whether the performance hit is a problem in real world situations - I suspect that normally, pip's startup time is swamped by things like downloading, builds, etc. |
Hmm, from the latest test run, the duration difference is actually not that much (zipapp took 29m, the second slowest was 26m); maybe it’d a good enough if we simply split that job into two, like we do for tests on Windows. |
Merging with main to test the new cc @pradyunsg |
Aha. I just found https://github.com/pypa/pip/blob/main/src/pip/_internal/build_env.py#L50. Cool. |
I’ll leave splitting the ci run into two for |
This adds a new
--use-zipapp
option to the test suite, which builds pip as a zipapp and uses that when running the integration tests.The implementation is complete, but I'm leaving this as draft for now as not all of the tests support being run with a zipapp yet.