-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
refactor: Decouple discovery module #2956
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
Merged
gaborbernat
merged 9 commits into
pypa:main
from
esafak:refactor/2074-decouple-discovery
Aug 14, 2025
Merged
refactor: Decouple discovery module #2956
gaborbernat
merged 9 commits into
pypa:main
from
esafak:refactor/2074-decouple-discovery
Aug 14, 2025
+625
−399
Conversation
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
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>
for more information, see https://pre-commit.ci
gaborbernat
requested changes
Aug 13, 2025
* 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>
for more information, see https://pre-commit.ci
* 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>
gaborbernat
approved these changes
Aug 14, 2025
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.
After this refactor it should be possible to extract
discoveryinto 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, andvirtualenv.utilhave been removed. Instead of direct imports, these dependencies are now injected into the discovery components.This was achieved by:
AppDataandCacheprotocols within thediscoverymodule to act as abstract interfaces.cached_py_info.pyandbuiltin.pyto acceptapp_dataandcacheobjects as arguments.virtualenv.run).tox -e fix)docs/changelogfolder