Skip to content

Comments

[ty] Add test coverage for PythonEnvironment::System variants#17996

Merged
zanieb merged 2 commits intomainfrom
zb/python-env-test
May 10, 2025
Merged

[ty] Add test coverage for PythonEnvironment::System variants#17996
zanieb merged 2 commits intomainfrom
zb/python-env-test

Conversation

@zanieb
Copy link
Member

@zanieb zanieb commented May 9, 2025

Adds test coverage for #17991, which includes some minor refactoring of the virtual environment test infrastructure.

I tried to minimize stylistic changes, but there are still a few because I was a little confused by the setup. I could see this evolving more in the future, as I don't think the existing model can capture all the test coverage I'm looking for.

@github-actions
Copy link
Contributor

github-actions bot commented May 9, 2025

mypy_primer results

No ecosystem changes detected ✅

@zanieb zanieb force-pushed the zb/python-env-test branch 3 times, most recently from 91e6571 to 2f9a189 Compare May 9, 2025 22:00
@zanieb zanieb added ty Multi-file analysis & type inference internal An internal refactor or improvement testing Related to testing Ruff itself labels May 9, 2025
@zanieb zanieb changed the title Add test coverage for PythonEnvironment::System variants [ty] Add test coverage for PythonEnvironment::System variants May 10, 2025
Comment on lines +841 to +854
let expected_site_packages = if cfg!(target_os = "windows") {
SystemPathBuf::from(&*format!(
r"\Python3.{}\Lib\site-packages",
self.minor_version
))
} else if self.free_threaded {
SystemPathBuf::from(&*format!(
"/Python3.{minor_version}/lib/python3.{minor_version}t/site-packages",
minor_version = self.minor_version
))
} else {
SystemPathBuf::from(&*format!(
"/Python3.{minor_version}/lib/python3.{minor_version}/site-packages",
minor_version = self.minor_version
))
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks repeated from the virtual environment branch -- worth factoring it out into a helper function?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels low priority to me in a test setup, but yeah we could.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I just don't like how these strings are pretty complicated and are duplicated in both branches -- it feels kinda fragile. But I don't feel strongly; feel free to disregard if you don't think it's worth spending time on!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test setup feels fragile to me, I think that's why I wasn't motivated to do more consolidation. I can look at doing so in a follow-up though.

Copy link
Member

@AlexWaygood AlexWaygood May 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is fragile, yeah, agreed. improvements very much welcome :-)

Base automatically changed from zb/python-env to main May 10, 2025 20:17
zanieb added a commit that referenced this pull request May 10, 2025
This adds basic support for non-virtual Python environments by accepting
a directory without a `pyvenv.cfg` which allows existing, subsequent
site-packages discovery logic to succeed. We can do better here in the
long-term, by adding more eager validation (for error messages) and
parsing the Python version from the discovered site-packages directory
(which isn't relevant yet, because we don't use the discovered Python
version from virtual environments as the default `--python-version` yet
either).

Related

- astral-sh/ty#265
- astral-sh/ty#193

You can review this commit by commit if it makes you happy.

I tested this manually; I think refactoring the test setup is going to
be a bit more invasive so I'll stack it on top (see
#17996).

```
❯ uv run ty check --python /Users/zb/.local/share/uv/python/cpython-3.10.17-macos-aarch64-none/ -vv example
2025-05-09 12:06:33.685911 DEBUG Version: 0.0.0-alpha.7 (f9c4c8999 2025-05-08)
2025-05-09 12:06:33.685987 DEBUG Architecture: aarch64, OS: macos, case-sensitive: case-insensitive
2025-05-09 12:06:33.686002 DEBUG Searching for a project in '/Users/zb/workspace/ty'
2025-05-09 12:06:33.686123 DEBUG Resolving requires-python constraint: `>=3.8`
2025-05-09 12:06:33.686129 DEBUG Resolved requires-python constraint to: 3.8
2025-05-09 12:06:33.686142 DEBUG Project without `tool.ty` section: '/Users/zb/workspace/ty'
2025-05-09 12:06:33.686147 DEBUG Searching for a user-level configuration at `/Users/zb/.config/ty/ty.toml`
2025-05-09 12:06:33.686156 INFO Defaulting to python-platform `darwin`
2025-05-09 12:06:33.68636 INFO Python version: Python 3.8, platform: darwin
2025-05-09 12:06:33.686375 DEBUG Adding first-party search path '/Users/zb/workspace/ty'
2025-05-09 12:06:33.68638 DEBUG Using vendored stdlib
2025-05-09 12:06:33.686634 DEBUG Discovering site-packages paths from sys-prefix `/Users/zb/.local/share/uv/python/cpython-3.10.17-macos-aarch64-none` (`--python` argument')
2025-05-09 12:06:33.686667 DEBUG Attempting to parse virtual environment metadata at '/Users/zb/.local/share/uv/python/cpython-3.10.17-macos-aarch64-none/pyvenv.cfg'
2025-05-09 12:06:33.686671 DEBUG Searching for site-packages directory in `sys.prefix` path `/Users/zb/.local/share/uv/python/cpython-3.10.17-macos-aarch64-none`
2025-05-09 12:06:33.686702 DEBUG Resolved site-packages directories for this environment are: ["/Users/zb/.local/share/uv/python/cpython-3.10.17-macos-aarch64-none/lib/python3.10/site-packages"]
2025-05-09 12:06:33.686706 DEBUG Adding site-packages search path '/Users/zb/.local/share/uv/python/cpython-3.10.17-macos-aarch64-none/lib/python3.10/site-packages'
...

❯ uv run ty check --python /tmp -vv example
2025-05-09 15:36:10.819416 DEBUG Version: 0.0.0-alpha.7 (f9c4c8999 2025-05-08)
2025-05-09 15:36:10.819708 DEBUG Architecture: aarch64, OS: macos, case-sensitive: case-insensitive
2025-05-09 15:36:10.820118 DEBUG Searching for a project in '/Users/zb/workspace/ty'
2025-05-09 15:36:10.821652 DEBUG Resolving requires-python constraint: `>=3.8`
2025-05-09 15:36:10.821667 DEBUG Resolved requires-python constraint to: 3.8
2025-05-09 15:36:10.8217 DEBUG Project without `tool.ty` section: '/Users/zb/workspace/ty'
2025-05-09 15:36:10.821888 DEBUG Searching for a user-level configuration at `/Users/zb/.config/ty/ty.toml`
2025-05-09 15:36:10.822072 INFO Defaulting to python-platform `darwin`
2025-05-09 15:36:10.822439 INFO Python version: Python 3.8, platform: darwin
2025-05-09 15:36:10.822773 DEBUG Adding first-party search path '/Users/zb/workspace/ty'
2025-05-09 15:36:10.822929 DEBUG Using vendored stdlib
2025-05-09 15:36:10.829872 DEBUG Discovering site-packages paths from sys-prefix `/tmp` (`--python` argument')
2025-05-09 15:36:10.829911 DEBUG Attempting to parse virtual environment metadata at '/private/tmp/pyvenv.cfg'
2025-05-09 15:36:10.829917 DEBUG Searching for site-packages directory in `sys.prefix` path `/private/tmp`
ty failed
  Cause: Invalid search path settings
  Cause: Failed to discover the site-packages directory: Failed to search the `lib` directory of the Python installation at `sys.prefix` path `/private/tmp` for `site-packages`
```
@zanieb zanieb force-pushed the zb/python-env-test branch from 2f9a189 to c9653c0 Compare May 10, 2025 20:25
@zanieb zanieb enabled auto-merge (squash) May 10, 2025 20:25
@zanieb zanieb merged commit 2923c55 into main May 10, 2025
31 checks passed
@zanieb zanieb deleted the zb/python-env-test branch May 10, 2025 20:28
Glyphack pushed a commit to Glyphack/ruff that referenced this pull request May 21, 2025
…#17991)

This adds basic support for non-virtual Python environments by accepting
a directory without a `pyvenv.cfg` which allows existing, subsequent
site-packages discovery logic to succeed. We can do better here in the
long-term, by adding more eager validation (for error messages) and
parsing the Python version from the discovered site-packages directory
(which isn't relevant yet, because we don't use the discovered Python
version from virtual environments as the default `--python-version` yet
either).

Related

- astral-sh/ty#265
- astral-sh/ty#193

You can review this commit by commit if it makes you happy.

I tested this manually; I think refactoring the test setup is going to
be a bit more invasive so I'll stack it on top (see
astral-sh#17996).

```
❯ uv run ty check --python /Users/zb/.local/share/uv/python/cpython-3.10.17-macos-aarch64-none/ -vv example
2025-05-09 12:06:33.685911 DEBUG Version: 0.0.0-alpha.7 (f9c4c8999 2025-05-08)
2025-05-09 12:06:33.685987 DEBUG Architecture: aarch64, OS: macos, case-sensitive: case-insensitive
2025-05-09 12:06:33.686002 DEBUG Searching for a project in '/Users/zb/workspace/ty'
2025-05-09 12:06:33.686123 DEBUG Resolving requires-python constraint: `>=3.8`
2025-05-09 12:06:33.686129 DEBUG Resolved requires-python constraint to: 3.8
2025-05-09 12:06:33.686142 DEBUG Project without `tool.ty` section: '/Users/zb/workspace/ty'
2025-05-09 12:06:33.686147 DEBUG Searching for a user-level configuration at `/Users/zb/.config/ty/ty.toml`
2025-05-09 12:06:33.686156 INFO Defaulting to python-platform `darwin`
2025-05-09 12:06:33.68636 INFO Python version: Python 3.8, platform: darwin
2025-05-09 12:06:33.686375 DEBUG Adding first-party search path '/Users/zb/workspace/ty'
2025-05-09 12:06:33.68638 DEBUG Using vendored stdlib
2025-05-09 12:06:33.686634 DEBUG Discovering site-packages paths from sys-prefix `/Users/zb/.local/share/uv/python/cpython-3.10.17-macos-aarch64-none` (`--python` argument')
2025-05-09 12:06:33.686667 DEBUG Attempting to parse virtual environment metadata at '/Users/zb/.local/share/uv/python/cpython-3.10.17-macos-aarch64-none/pyvenv.cfg'
2025-05-09 12:06:33.686671 DEBUG Searching for site-packages directory in `sys.prefix` path `/Users/zb/.local/share/uv/python/cpython-3.10.17-macos-aarch64-none`
2025-05-09 12:06:33.686702 DEBUG Resolved site-packages directories for this environment are: ["/Users/zb/.local/share/uv/python/cpython-3.10.17-macos-aarch64-none/lib/python3.10/site-packages"]
2025-05-09 12:06:33.686706 DEBUG Adding site-packages search path '/Users/zb/.local/share/uv/python/cpython-3.10.17-macos-aarch64-none/lib/python3.10/site-packages'
...

❯ uv run ty check --python /tmp -vv example
2025-05-09 15:36:10.819416 DEBUG Version: 0.0.0-alpha.7 (f9c4c8999 2025-05-08)
2025-05-09 15:36:10.819708 DEBUG Architecture: aarch64, OS: macos, case-sensitive: case-insensitive
2025-05-09 15:36:10.820118 DEBUG Searching for a project in '/Users/zb/workspace/ty'
2025-05-09 15:36:10.821652 DEBUG Resolving requires-python constraint: `>=3.8`
2025-05-09 15:36:10.821667 DEBUG Resolved requires-python constraint to: 3.8
2025-05-09 15:36:10.8217 DEBUG Project without `tool.ty` section: '/Users/zb/workspace/ty'
2025-05-09 15:36:10.821888 DEBUG Searching for a user-level configuration at `/Users/zb/.config/ty/ty.toml`
2025-05-09 15:36:10.822072 INFO Defaulting to python-platform `darwin`
2025-05-09 15:36:10.822439 INFO Python version: Python 3.8, platform: darwin
2025-05-09 15:36:10.822773 DEBUG Adding first-party search path '/Users/zb/workspace/ty'
2025-05-09 15:36:10.822929 DEBUG Using vendored stdlib
2025-05-09 15:36:10.829872 DEBUG Discovering site-packages paths from sys-prefix `/tmp` (`--python` argument')
2025-05-09 15:36:10.829911 DEBUG Attempting to parse virtual environment metadata at '/private/tmp/pyvenv.cfg'
2025-05-09 15:36:10.829917 DEBUG Searching for site-packages directory in `sys.prefix` path `/private/tmp`
ty failed
  Cause: Invalid search path settings
  Cause: Failed to discover the site-packages directory: Failed to search the `lib` directory of the Python installation at `sys.prefix` path `/private/tmp` for `site-packages`
```
Glyphack pushed a commit to Glyphack/ruff that referenced this pull request May 21, 2025
…al-sh#17996)

Adds test coverage for astral-sh#17991,
which includes some minor refactoring of the virtual environment test
infrastructure.

I tried to minimize stylistic changes, but there are still a few because
I was a little confused by the setup. I could see this evolving more in
the future, as I don't think the existing model can capture all the test
coverage I'm looking for.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal An internal refactor or improvement testing Related to testing Ruff itself ty Multi-file analysis & type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants