From 50e194f1070733d0af66904001a89e4d603387b4 Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Thu, 27 Oct 2022 21:34:17 +0800 Subject: [PATCH] Selectively enable user site The modern virtual environment structure does not allow us to enable "fake user site" while disabling the global site, so we need to do more fine-grained configuration to correctly set up test environments for each test case. With this done, we can also properly support the stdlib venv ad the test environment backend, since it basically works identically with modern virtualenv. The incompatible_with_test_venv is thus removed. --- setup.cfg | 1 - tests/conftest.py | 12 ++- tests/functional/test_build_env.py | 2 +- tests/functional/test_freeze.py | 4 +- tests/functional/test_install.py | 4 +- tests/functional/test_install_reqs.py | 3 +- tests/functional/test_install_user.py | 9 +-- tests/functional/test_install_wheel.py | 3 +- tests/functional/test_list.py | 6 +- tests/functional/test_new_resolver_user.py | 17 ++--- tests/functional/test_uninstall_user.py | 2 +- tests/lib/venv.py | 85 ++++++++++++---------- 12 files changed, 70 insertions(+), 78 deletions(-) diff --git a/setup.cfg b/setup.cfg index dae2f21b1..1502abfc8 100644 --- a/setup.cfg +++ b/setup.cfg @@ -63,7 +63,6 @@ xfail_strict = True markers = network: tests that need network incompatible_with_sysconfig - incompatible_with_test_venv incompatible_with_venv no_auto_tempdir_manager unit: unit tests diff --git a/tests/conftest.py b/tests/conftest.py index 44aa56026..46975b29b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -108,10 +108,6 @@ def pytest_collection_modifyitems(config: Config, items: List[pytest.Function]) if item.get_closest_marker("network") is not None: item.add_marker(pytest.mark.flaky(reruns=3, reruns_delay=2)) - if item.get_closest_marker("incompatible_with_test_venv") and config.getoption( - "--use-venv" - ): - item.add_marker(pytest.mark.skip("Incompatible with test venv")) if ( item.get_closest_marker("incompatible_with_venv") and sys.prefix != sys.base_prefix @@ -474,9 +470,6 @@ def virtualenv_template( ): (venv.bin / exe).unlink() - # Enable user site packages. - venv.user_site_packages = True - # Rename original virtualenv directory to make sure # it's not reused by mistake from one of the copies. venv_template = tmpdir / "venv_template" @@ -742,3 +735,8 @@ def mock_server() -> Iterator[MockServer]: @pytest.fixture def proxy(request: pytest.FixtureRequest) -> str: return request.config.getoption("proxy") + + +@pytest.fixture +def enable_user_site(virtualenv: VirtualEnvironment) -> None: + virtualenv.user_site_packages = True diff --git a/tests/functional/test_build_env.py b/tests/functional/test_build_env.py index 869e8ad92..93a6b930f 100644 --- a/tests/functional/test_build_env.py +++ b/tests/functional/test_build_env.py @@ -204,7 +204,7 @@ def test_build_env_overlay_prefix_has_priority(script: PipTestEnvironment) -> No assert result.stdout.strip() == "2.0", str(result) -@pytest.mark.incompatible_with_test_venv +@pytest.mark.usefixtures("enable_user_site") def test_build_env_isolation(script: PipTestEnvironment) -> None: # Create dummy `pkg` wheel. diff --git a/tests/functional/test_freeze.py b/tests/functional/test_freeze.py index 535581121..49b362d7e 100644 --- a/tests/functional/test_freeze.py +++ b/tests/functional/test_freeze.py @@ -862,7 +862,7 @@ def test_freeze_with_requirement_option_package_repeated_multi_file( @pytest.mark.network -@pytest.mark.incompatible_with_test_venv +@pytest.mark.usefixtures("enable_user_site") def test_freeze_user( script: PipTestEnvironment, virtualenv: VirtualEnvironment, data: TestData ) -> None: @@ -900,7 +900,7 @@ def test_freeze_path(tmpdir: Path, script: PipTestEnvironment, data: TestData) - @pytest.mark.network -@pytest.mark.incompatible_with_test_venv +@pytest.mark.usefixtures("enable_user_site") def test_freeze_path_exclude_user( tmpdir: Path, script: PipTestEnvironment, data: TestData ) -> None: diff --git a/tests/functional/test_install.py b/tests/functional/test_install.py index 876f2e12a..f61137268 100644 --- a/tests/functional/test_install.py +++ b/tests/functional/test_install.py @@ -171,7 +171,7 @@ def test_pep518_allows_missing_requires( assert result.files_created -@pytest.mark.incompatible_with_test_venv +@pytest.mark.usefixtures("enable_user_site") def test_pep518_with_user_pip( script: PipTestEnvironment, pip_src: Path, data: TestData, common_wheels: Path ) -> None: @@ -2106,7 +2106,7 @@ def test_target_install_ignores_distutils_config_install_prefix( result.did_not_create(relative_script_base) -@pytest.mark.incompatible_with_test_venv +@pytest.mark.usefixtures("enable_user_site") def test_user_config_accepted(script: PipTestEnvironment) -> None: # user set in the config file is parsed as 0/1 instead of True/False. # Check that this doesn't cause a problem. diff --git a/tests/functional/test_install_reqs.py b/tests/functional/test_install_reqs.py index 19c526aab..14e1056ae 100644 --- a/tests/functional/test_install_reqs.py +++ b/tests/functional/test_install_reqs.py @@ -305,8 +305,7 @@ def test_install_local_with_subdirectory(script: PipTestEnvironment) -> None: result.assert_installed("version_subpkg.py", editable=False) -@pytest.mark.incompatible_with_test_venv -@pytest.mark.usefixtures("with_wheel") +@pytest.mark.usefixtures("enable_user_site", "with_wheel") def test_wheel_user_with_prefix_in_pydistutils_cfg( script: PipTestEnvironment, data: TestData ) -> None: diff --git a/tests/functional/test_install_user.py b/tests/functional/test_install_user.py index e34b35431..c960d0de4 100644 --- a/tests/functional/test_install_user.py +++ b/tests/functional/test_install_user.py @@ -35,9 +35,9 @@ def _patch_dist_in_site_packages(virtualenv: VirtualEnvironment) -> None: ) +@pytest.mark.usefixtures("enable_user_site") class Tests_UserSite: @pytest.mark.network - @pytest.mark.incompatible_with_test_venv def test_reset_env_system_site_packages_usersite( self, script: PipTestEnvironment ) -> None: @@ -57,7 +57,6 @@ class Tests_UserSite: @pytest.mark.xfail @pytest.mark.network @need_svn - @pytest.mark.incompatible_with_test_venv def test_install_subversion_usersite_editable_with_distribute( self, script: PipTestEnvironment, tmpdir: Path ) -> None: @@ -77,7 +76,6 @@ class Tests_UserSite: ) result.assert_installed("INITools", use_user_site=True) - @pytest.mark.incompatible_with_test_venv @pytest.mark.usefixtures("with_wheel") def test_install_from_current_directory_into_usersite( self, script: PipTestEnvironment, data: TestData @@ -123,7 +121,6 @@ class Tests_UserSite: ) @pytest.mark.network - @pytest.mark.incompatible_with_test_venv def test_install_user_conflict_in_usersite( self, script: PipTestEnvironment ) -> None: @@ -148,7 +145,6 @@ class Tests_UserSite: result2.did_create(egg_info_folder) assert not isfile(initools_v3_file), initools_v3_file - @pytest.mark.incompatible_with_test_venv def test_install_user_conflict_in_globalsite( self, virtualenv: VirtualEnvironment, script: PipTestEnvironment ) -> None: @@ -191,7 +187,6 @@ class Tests_UserSite: assert isdir(dist_info_folder) assert isdir(initools_folder) - @pytest.mark.incompatible_with_test_venv def test_upgrade_user_conflict_in_globalsite( self, virtualenv: VirtualEnvironment, script: PipTestEnvironment ) -> None: @@ -235,7 +230,6 @@ class Tests_UserSite: assert isdir(dist_info_folder), result2.stdout assert isdir(initools_folder) - @pytest.mark.incompatible_with_test_venv def test_install_user_conflict_in_globalsite_and_usersite( self, virtualenv: VirtualEnvironment, script: PipTestEnvironment ) -> None: @@ -294,7 +288,6 @@ class Tests_UserSite: assert isdir(initools_folder) @pytest.mark.network - @pytest.mark.incompatible_with_test_venv def test_install_user_in_global_virtualenv_with_conflict_fails( self, script: PipTestEnvironment ) -> None: diff --git a/tests/functional/test_install_wheel.py b/tests/functional/test_install_wheel.py index e988e7419..49c2d1d6d 100644 --- a/tests/functional/test_install_wheel.py +++ b/tests/functional/test_install_wheel.py @@ -406,8 +406,7 @@ def test_wheel_record_lines_have_updated_hash_for_scripts( ] -@pytest.mark.incompatible_with_test_venv -@pytest.mark.usefixtures("with_wheel") +@pytest.mark.usefixtures("enable_user_site", "with_wheel") def test_install_user_wheel( script: PipTestEnvironment, shared_data: TestData, tmpdir: Path ) -> None: diff --git a/tests/functional/test_list.py b/tests/functional/test_list.py index c7fdec2f2..d05fe9dce 100644 --- a/tests/functional/test_list.py +++ b/tests/functional/test_list.py @@ -129,7 +129,7 @@ def test_multiple_exclude_and_normalization( @pytest.mark.network -@pytest.mark.incompatible_with_test_venv +@pytest.mark.usefixtures("enable_user_site") def test_user_flag(script: PipTestEnvironment, data: TestData) -> None: """ Test the behavior of --user flag in the list command @@ -144,7 +144,7 @@ def test_user_flag(script: PipTestEnvironment, data: TestData) -> None: @pytest.mark.network -@pytest.mark.incompatible_with_test_venv +@pytest.mark.usefixtures("enable_user_site") def test_user_columns_flag(script: PipTestEnvironment, data: TestData) -> None: """ Test the behavior of --user --format=columns flags in the list command @@ -656,7 +656,7 @@ def test_list_path(tmpdir: Path, script: PipTestEnvironment, data: TestData) -> assert {"name": "simple", "version": "2.0"} in json_result -@pytest.mark.incompatible_with_test_venv +@pytest.mark.usefixtures("enable_user_site") def test_list_path_exclude_user( tmpdir: Path, script: PipTestEnvironment, data: TestData ) -> None: diff --git a/tests/functional/test_new_resolver_user.py b/tests/functional/test_new_resolver_user.py index 2f9fb65ba..4578c3114 100644 --- a/tests/functional/test_new_resolver_user.py +++ b/tests/functional/test_new_resolver_user.py @@ -7,7 +7,7 @@ from tests.lib import PipTestEnvironment, create_basic_wheel_for_package from tests.lib.venv import VirtualEnvironment -@pytest.mark.incompatible_with_test_venv +@pytest.mark.usefixtures("enable_user_site") def test_new_resolver_install_user(script: PipTestEnvironment) -> None: create_basic_wheel_for_package(script, "base", "0.1.0") result = script.pip( @@ -22,7 +22,7 @@ def test_new_resolver_install_user(script: PipTestEnvironment) -> None: result.did_create(script.user_site / "base") -@pytest.mark.incompatible_with_test_venv +@pytest.mark.usefixtures("enable_user_site") def test_new_resolver_install_user_satisfied_by_global_site( script: PipTestEnvironment, ) -> None: @@ -53,7 +53,7 @@ def test_new_resolver_install_user_satisfied_by_global_site( result.did_not_create(script.user_site / "base") -@pytest.mark.incompatible_with_test_venv +@pytest.mark.usefixtures("enable_user_site") def test_new_resolver_install_user_conflict_in_user_site( script: PipTestEnvironment, ) -> None: @@ -91,7 +91,7 @@ def test_new_resolver_install_user_conflict_in_user_site( result.did_not_create(base_2_dist_info) -@pytest.mark.incompatible_with_test_venv +@pytest.mark.usefixtures("enable_user_site") def test_new_resolver_install_user_in_virtualenv_with_conflict_fails( script: PipTestEnvironment, ) -> None: @@ -141,8 +141,7 @@ def patch_dist_in_site_packages(virtualenv: VirtualEnvironment) -> None: ) -@pytest.mark.incompatible_with_test_venv -@pytest.mark.usefixtures("patch_dist_in_site_packages") +@pytest.mark.usefixtures("enable_user_site", "patch_dist_in_site_packages") def test_new_resolver_install_user_reinstall_global_site( script: PipTestEnvironment, ) -> None: @@ -177,8 +176,7 @@ def test_new_resolver_install_user_reinstall_global_site( assert "base" in site_packages_content -@pytest.mark.incompatible_with_test_venv -@pytest.mark.usefixtures("patch_dist_in_site_packages") +@pytest.mark.usefixtures("enable_user_site", "patch_dist_in_site_packages") def test_new_resolver_install_user_conflict_in_global_site( script: PipTestEnvironment, ) -> None: @@ -215,8 +213,7 @@ def test_new_resolver_install_user_conflict_in_global_site( assert "base-1.0.0.dist-info" in site_packages_content -@pytest.mark.incompatible_with_test_venv -@pytest.mark.usefixtures("patch_dist_in_site_packages") +@pytest.mark.usefixtures("enable_user_site", "patch_dist_in_site_packages") def test_new_resolver_install_user_conflict_in_global_and_user_sites( script: PipTestEnvironment, ) -> None: diff --git a/tests/functional/test_uninstall_user.py b/tests/functional/test_uninstall_user.py index 1ef65dd16..0bf2e6d41 100644 --- a/tests/functional/test_uninstall_user.py +++ b/tests/functional/test_uninstall_user.py @@ -11,7 +11,7 @@ from tests.lib.venv import VirtualEnvironment from tests.lib.wheel import make_wheel -@pytest.mark.incompatible_with_test_venv +@pytest.mark.usefixtures("enable_user_site") class Tests_UninstallUserSite: @pytest.mark.network def test_uninstall_from_usersite(self, script: PipTestEnvironment) -> None: diff --git a/tests/lib/venv.py b/tests/lib/venv.py index 42ec5082a..e65a32912 100644 --- a/tests/lib/venv.py +++ b/tests/lib/venv.py @@ -20,9 +20,6 @@ else: VirtualEnvironmentType = str -LEGACY_VIRTUALENV = int(_virtualenv.__version__.split(".", 1)[0]) < 20 - - class VirtualEnvironment: """ An abstraction around virtual environments, currently it only uses @@ -34,7 +31,7 @@ class VirtualEnvironment: location: Path, template: Optional["VirtualEnvironment"] = None, venv_type: Optional[VirtualEnvironmentType] = None, - ): + ) -> None: self.location = location assert template is None or venv_type is None self._venv_type: VirtualEnvironmentType @@ -44,13 +41,18 @@ class VirtualEnvironment: self._venv_type = venv_type else: self._venv_type = "virtualenv" - assert self._venv_type in ("virtualenv", "venv") self._user_site_packages = False self._template = template self._sitecustomize: Optional[str] = None self._update_paths() self._create() + @property + def _legacy_virtualenv(self) -> bool: + if self._venv_type != "virtualenv": + return False + return int(_virtualenv.__version__.split(".", 1)[0]) < 20 + def __update_paths_legacy(self) -> None: home, lib, inc, bin = _virtualenv.path_locations(self.location) self.bin = Path(bin) @@ -63,7 +65,7 @@ class VirtualEnvironment: self.lib = Path(lib) def _update_paths(self) -> None: - if LEGACY_VIRTUALENV: + if self._legacy_virtualenv: self.__update_paths_legacy() return bases = { @@ -86,7 +88,11 @@ class VirtualEnvironment: if self._template: # On Windows, calling `_virtualenv.path_locations(target)` # will have created the `target` directory... - if LEGACY_VIRTUALENV and sys.platform == "win32" and self.location.exists(): + if ( + self._legacy_virtualenv + and sys.platform == "win32" + and self.location.exists() + ): self.location.rmdir() # Clone virtual environment from template. shutil.copytree(self._template.location, self.location, symlinks=True) @@ -94,35 +100,36 @@ class VirtualEnvironment: self._user_site_packages = self._template.user_site_packages else: # Create a new virtual environment. - if self._venv_type == "virtualenv": - if LEGACY_VIRTUALENV: - subprocess.check_call( - [ - sys.executable, - "-m", - "virtualenv", - "--no-pip", - "--no-wheel", - "--no-setuptools", - os.fspath(self.location), - ] - ) - self._fix_legacy_virtualenv_site_module() - else: - _virtualenv.cli_run( - [ - "--no-pip", - "--no-wheel", - "--no-setuptools", - os.fspath(self.location), - ], - ) + if self._legacy_virtualenv: + subprocess.check_call( + [ + sys.executable, + "-m", + "virtualenv", + "--no-pip", + "--no-wheel", + "--no-setuptools", + os.fspath(self.location), + ] + ) + self._fix_legacy_virtualenv_site_module() + elif self._venv_type == "virtualenv": + _virtualenv.cli_run( + [ + "--no-pip", + "--no-wheel", + "--no-setuptools", + os.fspath(self.location), + ], + ) elif self._venv_type == "venv": builder = _venv.EnvBuilder() context = builder.ensure_directories(self.location) builder.create_configuration(context) builder.setup_python(context) self.site.mkdir(parents=True, exist_ok=True) + else: + raise RuntimeError(f"Unsupported venv type {self._venv_type!r}") self.sitecustomize = self._sitecustomize self.user_site_packages = self._user_site_packages @@ -161,7 +168,9 @@ class VirtualEnvironment: assert compileall.compile_file(str(site_py), quiet=1, force=True) def _customize_site(self) -> None: - if not LEGACY_VIRTUALENV or self._venv_type == "venv": + if self._legacy_virtualenv: + contents = "" + else: # Enable user site (before system). contents = textwrap.dedent( f""" @@ -186,8 +195,6 @@ class VirtualEnvironment: site.addsitedir(path) """ ).strip() - else: - contents = "" if self._sitecustomize is not None: contents += "\n" + self._sitecustomize sitecustomize = self.site / "sitecustomize.py" @@ -234,14 +241,14 @@ class VirtualEnvironment: @user_site_packages.setter def user_site_packages(self, value: bool) -> None: self._user_site_packages = value - if not LEGACY_VIRTUALENV or self._venv_type == "venv": - self._rewrite_pyvenv_cfg( - {"include-system-site-packages": str(bool(value)).lower()} - ) - self._customize_site() - else: + if self._legacy_virtualenv: marker = self.lib / "no-global-site-packages.txt" if self._user_site_packages: marker.unlink() else: marker.touch() + else: + self._rewrite_pyvenv_cfg( + {"include-system-site-packages": str(bool(value)).lower()} + ) + self._customize_site()