Add test coverage for pip build environments in find_uv_bin#18095
Draft
Add test coverage for pip build environments in find_uv_bin#18095
find_uv_bin#18095Conversation
acfa299 to
83152c0
Compare
find_uv_binfind_uv_bin
60acc44 to
2d44deb
Compare
zanieb
commented
Feb 20, 2026
zanieb
commented
Feb 20, 2026
zanieb
commented
Feb 20, 2026
zanieb
commented
Feb 23, 2026
Comment on lines
73
to
+90
| @@ -87,6 +87,6 @@ Write-Output ` | |||
| "TEMP=$($Tmp)" ` | |||
| "RUSTUP_HOME=$($Drive)/.rustup" ` | |||
| "CARGO_HOME=$($Drive)/.cargo" ` | |||
| "UV_WORKSPACE=$($Drive)/uv" ` | |||
| "UV_WORKSPACE=$($Drive)/uv/repo" ` | |||
Member
Author
There was a problem hiding this comment.
These were colliding in some filters because the prefix matches
zanieb
commented
Feb 23, 2026
crates/uv/tests/it/python_module.rs
Outdated
| .assert() | ||
| .success() | ||
| .get_output() | ||
| .clone(); |
Member
Author
There was a problem hiding this comment.
This clone seems superfluous?
zanieb
commented
Feb 23, 2026
crates/uv/tests/it/python_module.rs
Outdated
Comment on lines
+481
to
+484
| let found_uv_lines: String = String::from_utf8(output.stdout) | ||
| .unwrap() | ||
| .lines() | ||
| .chain(String::from_utf8(output.stderr).unwrap().lines()) |
Member
Author
There was a problem hiding this comment.
I don't think we need to search both stdout and stderr, where do we expect it?
zanieb
commented
Feb 23, 2026
crates/uv/tests/it/python_module.rs
Outdated
| .collect::<Vec<_>>() | ||
| .join("\n"); | ||
|
|
||
| let snapshot = uv_test::apply_filters(found_uv_lines, &context.filters()); |
Member
Author
There was a problem hiding this comment.
Why not insta settings with_filters like we use elsewhere?
zanieb
commented
Feb 23, 2026
crates/uv/tests/it/python_module.rs
Outdated
|
|
||
| // Extract the FOUND_UV= lines from the combined output and apply filters | ||
| // so paths are platform-independent. | ||
| let found_uv_lines: String = String::from_utf8(output.stdout) |
Member
Author
There was a problem hiding this comment.
If we don't find it we should fail with the stdout / stderr for debugging
Setuptools 69.0.2 on Windows emits `wheel/./project` instead of `wheel/project` in bdist install paths. Add a test filter to normalize redundant `/./` path components so the snapshot matches on all platforms.
The raw setuptools output on Windows uses backslashes (`wheel\.\.\project`), so the filter needs `[\\/]\.[\\/]` to match both slash directions, not just forward slashes.
Move the CI dev drive layout from `D:\uv` + `D:\uv-tmp` to `D:\uv\repo` + `D:\uv\tmp` so the workspace path is never a prefix of the temp dir. This removes the need for the fragile `insert_pos` logic in `with_filtered_system_tmp` that searched for `[TEMP_DIR]/` by replacement string value to maintain filter ordering.
…oc comment Move the many pip-specific filters (temp dirs, progress bars, hashes, sizes, platform tags, etc.) into a reusable `with_filtered_pip_output` method on `TestContext`. Also trim the `with_filtered_system_tmp` doc comment to remove unnecessary implementation details.
Instead of snapshotting the entire verbose pip install output (which is noisy and fragile across platforms), extract just the FOUND_UV= lines and snapshot those. This removes the need for with_filtered_pip_output and all the pip-specific filters.
… assert on missing output - Remove superfluous .clone() by binding the assert result - Only search stderr for FOUND_UV= (that's where setup.py prints it) - Use insta::with_settings! with filters instead of manual apply_filters - Assert FOUND_UV= lines are non-empty with full stderr for debugging
- Replace `.map(|line| line.trim())` with `.map(str::trim)` - Use `apply_filters` instead of `insta::with_settings` to avoid `String`-to-`&str` tuple coercion issues with unstable `str_as_str`
52f3ec6 to
4d22183
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
To ensure parity with
find_ruff_bin(see astral-sh/ruff#13321)It turns out the
_matching_parentsprefix lookup already supports this pattern.