Skip to content

fix: live test for [githubpipenv] wrong validation #11053

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

jNullj
Copy link
Member

@jNullj jNullj commented May 4, 2025

Change the message for the locked version of the VCS dependency badge to utilize version validation instead of commit hash validation.

Looked at past pypiserver version names and i think isVPlusDottedVersionNClausesWithOptionalSuffix is the best fit here

@jNullj jNullj added the keep-service-tests-green Related to fixing failing tests of the services label May 4, 2025
Copy link
Contributor

github-actions bot commented May 4, 2025

Messages
📖 ✨ Thanks for your contribution to Shields, @jNullj!

Generated by 🚫 dangerJS against 602023d

@chris48s
Copy link
Member

chris48s commented May 5, 2025

The intent of this test is to cover the VCS dependency path.

The reason we picked this example is because pypiserver used to be a VCS dependency in this project:
https://github.com/pypa/pipenv/blob/5b8d3f22ae323338521311f19e4d1e2a17ddf6ca/Pipfile#L31
and hence locked to a commit hash, rather than a version number.

The reason why this test is now failing is because they've switched it to a package requirement
https://github.com/pypa/pipenv/blob/149a003a87e3349ed08b4014357cdfe167377c70/Pipfile#L30
(in pypa/pipenv@a5a6069 )

so this isn't really the right fix.

One option would be to find another example of a VCS dependency "in the wild", but this is a test that has a habit of breaking (in fact, we've been here before #10658 (review) 😄 ). Another option would be to mock this condition, or pull the logic out and unit test it.

jNullj added 2 commits May 5, 2025 22:39
picked `https://github.com/ykdojo/editdojo/` as it seems to be real world example of somewhat popular repo with a VCS deps that is not often updated
fix from prev commit
@jNullj
Copy link
Member Author

jNullj commented May 5, 2025

Sorry about that. I learn from this that

  1. I should be more careful
  2. I should pick something less prune to changes
    Picked a new example, its somewhat popular repo but has 7 years without new commits.
    So we still keep using live example, which i think is best while mitigating the risk of often live test breaks drastically.

Maybe i should search from time to time with git blame the latest live tests changes when i fix them to get a feel for how long our last path survived.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keep-service-tests-green Related to fixing failing tests of the services
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants