-
Notifications
You must be signed in to change notification settings - Fork 21
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 integration tests for pip #60
Conversation
da07a91
to
9cdefcd
Compare
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.
Great!
Are you sure the tests are running using the truststore checkout and not with an installation of the released version? The test is explicitly pip installing truststore here: https://github.com/pypa/pip/pull/11082/files#diff-a5047544edabab77bcc833c6db13a484c0911014e9b2fd71c429c238aa106ab1R59 -- I guess that should find that the package installed from the checkout is already there and do nothing, but maybe worth double checking?
Should we put this in a separate github workflow that is scheduled to run periodically? That way if pip changes in a way that breaks truststore, we find out proactively rather than only when we push to truststore.
@davisagli Thanks for the review, great ideas! I'll try pushing a broken commit and see that the integration tests fail. |
@sethmlarson oh no, it succeeded! |
Your hunch was spot-on, now to figure out how we can somehow circumvent pip installing us during their test suite... 🤔 |
Hmm, nox is going to make that difficult for us, isn't it. Maybe apply a patch to test_truststore.py in between checking out pip and running the tests, to make it install truststore from a relative path instead of from PyPI? |
@davisagli Thanks for the fix, review, and merge!! 💪 |
Closes #49