-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
fix [githubpipenv] service tests #10658
fix [githubpipenv] service tests #10658
Conversation
45af635
to
c27637c
Compare
replaced tested package which is missing with another example
c27637c
to
9a05c6e
Compare
@@ -83,9 +82,9 @@ t.create('Locked version of unknown dependency') | |||
|
|||
t.create('Locked version of VCS dependency') | |||
.get( | |||
'/locked/dependency-version/thorn-oss/perception/dev/videoalignment.json', | |||
'/locked/dependency-version/metabolize/rq-dashboard-on-heroku/dev/black.json', |
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.
This isn't a VCS dependency it is a pypi dependency, so doing this just makes this replicate another test. We need to find an example of a project using a VCS dependency. Let me see if I can find one.
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.
So here's a few examples of what we are looking for:
I have no idea how much we can expect any of those examples to remain stable. If this is something that keeps breaking, we could consider converting this test case to a mock or unit test.
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 ended up picking pipenv's locked vcs dependency pypiserver, i assume its a big project and is stable.
also very related to our tests here ;)
The version ref is a commit hash so i added a text-validation for commit hash, i bet it might be useful in the future.
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.
reasonable choices all round 👍
replaced tested package that changed on remote repo to avoid failure.
as discovered in pr #10656 at this ci test