-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Add tests for uv python list
#12931
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
base: main
Are you sure you want to change the base?
Add tests for uv python list
#12931
Conversation
1bd5b12
to
da51176
Compare
I added a subset of the Python downloads metadata json file for tests. This PR also fixes a issue where using `--only-downloads` with `--all-arches`, `--all-arches` is not honored.
da51176
to
91137f2
Compare
@@ -0,0 +1,674 @@ | |||
{ |
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.
Should this be here?
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.
Ah I see it's being used for the test case. Interesting 🤔 I'm worried about maintaining these. Do we need a subset? If so, should it be much smaller? Can you say more about the choice?
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 think we need a fixed subset to keep things stable; otherwise, the tests fall apart every time we sync the latest Python versions.
So, I’m making sure the subset includes a prerelease, a freethreaded variant, different patch versions of the same minor version, and different minor versions. This way, we’ve got a pretty thorough test case.
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.
Hm.
I'm not sure if I mind if the snapshots need updating when the available versions change.
My concern is that the snapshot doesn't capture the reality for most users, which makes it less valuable and more likely for us to accidentally ship regressions.
} else if all_arches { | ||
base_download_request.fill_platform()?.with_any_arch() |
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.
Is this a bug fix?
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.
Generally, it'd be better to do this separately so it can be listed in the changelog.
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.
Extracted it to #12953
91137f2
to
b580da0
Compare
b580da0
to
d1ce394
Compare
Note this overlaps with #12381 which I forgot to follow-up on after #12381 (comment). |
Summary
I added a subset of the Python downloads metadata json file for tests.
This PR also fixes a issue where using--only-downloads
with--all-arches
,--all-arches
is not honored.