Skip to content

Add integration tests for pip#60

Merged
davisagli merged 2 commits intomainfrom
pip-integ
Jul 3, 2022
Merged

Add integration tests for pip#60
davisagli merged 2 commits intomainfrom
pip-integ

Conversation

@sethmlarson
Copy link
Copy Markdown
Owner

@sethmlarson sethmlarson commented Jul 1, 2022 •
edited
Loading

Closes #49

@sethmlarson sethmlarson force-pushed the pip-integ branch 3 times, most recently from da07a91 to 9cdefcd Compare July 1, 2022 19:03
@sethmlarson sethmlarson requested a review from davisagli July 1, 2022 19:09
davisagli
davisagli reviewed Jul 1, 2022
View reviewed changes
Copy link
Copy Markdown
Collaborator

@davisagli davisagli left a comment •
edited
Loading

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.

@sethmlarson
Copy link
Copy Markdown
Owner Author

sethmlarson commented Jul 1, 2022

@davisagli Thanks for the review, great ideas! I'll try pushing a broken commit and see that the integration tests fail.

@davisagli
Copy link
Copy Markdown
Collaborator

davisagli commented Jul 1, 2022

@sethmlarson oh no, it succeeded!

@sethmlarson
Copy link
Copy Markdown
Owner Author

sethmlarson commented Jul 1, 2022

Your hunch was spot-on, now to figure out how we can somehow circumvent pip installing us during their test suite... 🤔

@davisagli
Copy link
Copy Markdown
Collaborator

davisagli commented Jul 1, 2022

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?

@sethmlarson sethmlarson requested a review from davisagli July 3, 2022 20:42
@davisagli davisagli merged commit c1da996 into main Jul 3, 2022
@davisagli davisagli deleted the pip-integ branch July 3, 2022 23:21
@sethmlarson
Copy link
Copy Markdown
Owner Author

sethmlarson commented Jul 4, 2022

@davisagli Thanks for the fix, review, and merge!! 💪

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@davisagli davisagli davisagli approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add integration test using pip's --use-feature=truststore

2 participants

@sethmlarson @davisagli