Skip to content

Conversation

@esafak
Copy link
Contributor

@esafak esafak commented Aug 13, 2025

After this refactor it should be possible to extract discovery into its own package (#2074). There is some coupling in tests, but this can be considered integration testing.

The primary dependencies on virtualenv.info, virtualenv.app_data, virtualenv.cache, and virtualenv.util have been removed. Instead of direct imports, these dependencies are now injected into the discovery components.

This was achieved by:

  • Introducing AppData and Cache protocols within the discovery module to act as abstract interfaces.
  • Modifying the functions in cached_py_info.py and builtin.py to accept app_data and cache objects as arguments.
  • Plumbing these new arguments down from the main entry points (e.g., virtualenv.run).
  • Refactoring the test suite extensively to provide the necessary mock or real objects to the refactored functions, ensuring all tests pass.
  • ran the linter to address style issues (tox -e fix)
  • wrote descriptive pull request text
  • ensured there are test(s) validating the fix
  • added news fragment in docs/changelog folder
  • updated/extended the documentation

esafak and others added 2 commits August 13, 2025 16:34
After this refactor it should be possible to extract `discovery` into its own package. There is some coupling in tests, but this can be considered integration testing.

The primary dependencies on `virtualenv.info`, `virtualenv.app_data`, `virtualenv.cache`, and `virtualenv.util` have been removed. Instead of direct imports, these dependencies are now injected into the discovery components.

This was achieved by:
- Introducing `AppData` and `Cache` protocols within the `discovery` module to act as abstract interfaces.
- Modifying the functions in `cached_py_info.py` and `builtin.py` to accept `app_data` and `cache` objects as arguments.
- Plumbing these new arguments down from the main entry points (e.g., `virtualenv.run`).
- Refactoring the test suite extensively to provide the necessary mock or real objects to the refactored functions, ensuring all tests pass.

Signed-off-by: Emre Şafak <3928300+esafak@users.noreply.github.com>
@gaborbernat gaborbernat marked this pull request as draft August 13, 2025 20:53
esafak and others added 7 commits August 13, 2025 17:26
* Remove unused `Protocol` import from `src/virtualenv/discovery/cache.py`
* Remove unused `Protocol` and `typing_extensions.Protocol` from `src/virtualenv/discovery/app_data.py`
* Replace `sysconfig.get_path("include")` with `current.system_include` in test file
* Update `cache_dir` argument to `cache` in Windows discovery method

Signed-off-by: Emre Şafak <3928300+esafak@users.noreply.github.com>
* Replace `cache_dir` parameter with `app_data` in `propose_interpreters`
* Update test case to use `MockAppData` instead of `None`
* Modify `Pep514PythonInfo.from_exe()` call to use `app_data` and `app_data.cache`
* Improve interpreter discovery method for Windows Python installations

Signed-off-by: Emre Şafak <3928300+esafak@users.noreply.github.com>
* Adjust `current_fastest` fixture to correctly use `key_to_class` for creator selection.
* Update `test_can_build_c_extensions` with a slow marker and xfail condition for CI.
* Modify `activation_python` fixture to pass the correct creator name.
* Introduce `MockCache` in `test_windows.py` for improved discovery tests.
* Update `propose_interpreters` function signature in `src/virtualenv/discovery/windows/__init__.py` to accept `cache`.
* Pass `cache` to `Pep514PythonInfo.from_exe`.

Signed-off-by: Emre Şafak <3928300+esafak@users.noreply.github.com>
…erly

* Pass the cache object to the `propose_interpreters` function on Windows.
* Update tests to pass the `current_fastest` directly instead of iterating over it.

Signed-off-by: Emre Şafak <3928300+esafak@users.noreply.github.com>
* Renamed `current_creators` fixture to `current_creator_keys`.
* Updated `create_methods` fixture to use `current_creator_keys`.
* Updated `test_create_clear_resets` to use `current_creator_keys`.
* Updated `test_prompt_set` to use `current_creator_keys`.
* Updated `test_home_path_is_exe_parent` to use `current_creator_keys`.
* Updated `test_create_distutils_cfg` to use `current_creator_keys`.
* Updated `test_venv_creator_without_write_perms` to use `current_creator_keys`.

Signed-off-by: Emre Şafak <3928300+esafak@users.noreply.github.com>
Signed-off-by: Emre Şafak <3928300+esafak@users.noreply.github.com>
@esafak esafak marked this pull request as ready for review August 14, 2025 02:12
@gaborbernat gaborbernat merged commit 1d7548d into pypa:main Aug 14, 2025
45 checks passed
@esafak esafak deleted the refactor/2074-decouple-discovery branch August 14, 2025 04:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants