From 0f5073744833e657e3ddd4702a22c883767a3ff8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Gia=20Phong?= Date: Sat, 7 Mar 2020 15:53:15 +0700 Subject: [PATCH] Add unit tests for user install decision Functional tests for conflict install locations are removed in favor of the unittest which handle more cases. --- src/pip/_internal/commands/install.py | 28 ++++--- tests/functional/test_install.py | 36 --------- tests/unit/test_command_install.py | 110 +++++++++++++++++++------- 3 files changed, 98 insertions(+), 76 deletions(-) diff --git a/src/pip/_internal/commands/install.py b/src/pip/_internal/commands/install.py index 450b83ef06b..15c6564f065 100644 --- a/src/pip/_internal/commands/install.py +++ b/src/pip/_internal/commands/install.py @@ -536,7 +536,7 @@ def site_packages_writable(**kwargs): def decide_user_install( - use_user_site, # type: Optional[bool] + use_user_site=None, # type: Optional[bool] prefix_path=None, # type: Optional[str] target_dir=None, # type: Optional[str] root_path=None, # type: Optional[str] @@ -577,12 +577,19 @@ def decide_user_install( return False if use_user_site is not None: + use_user_site = bool(use_user_site) logger.debug("{} install by explicit request".format( "User" if use_user_site else "Non-user")) - use_user_site = bool(use_user_site) - elif site.ENABLE_USER_SITE is False: # This never seems to be reached. - logger.debug("Non-user install due to disabled user site-packages by " - "user request (with 'python -s' or PYTHONNOUSERSITE)") + if use_user_site is True and site.ENABLE_USER_SITE is None: + raise InstallationError("User site-packages are disabled for " + "security reasons or by an administrator.") + elif site.ENABLE_USER_SITE is None: + logger.debug("Non-user install since user site-packages are disabled " + "for security reasons or by an administrator.") + use_user_site = False + elif site.ENABLE_USER_SITE is False: + logger.debug("Non-user install since user site-packages are disabled " + "by user request (with 'python -s' or PYTHONNOUSERSITE)") use_user_site = False elif root_path: logger.debug( @@ -596,14 +603,13 @@ def decide_user_install( "normal site-packages is not writeable") use_user_site = True - if use_user_site: - if site.ENABLE_USER_SITE is None: - raise InstallationError("User site-packages are disabled for " - "security reasons or by an administrator.") + if use_user_site is True: if virtualenv_no_global(): - raise InstallationError("User site-packages are not visible " - "in this virtualenv.") + raise InstallationError( + "Can not perform a '--user' install. " + "User site-packages are not visible in this virtualenv.") else: + assert use_user_site is False if not site_packages_writable(root=root_path, isolated=isolated_mode): raise InstallationError("Cannot write to global site-packages.") return use_user_site diff --git a/tests/functional/test_install.py b/tests/functional/test_install.py index 1ea9db26e4b..99b629a77f8 100644 --- a/tests/functional/test_install.py +++ b/tests/functional/test_install.py @@ -847,27 +847,6 @@ def test_install_package_with_target(script): assert singlemodule_py in result.files_updated, str(result) -def test_install_package_to_usersite_with_target_must_fail(script): - """ - Test that installing package to usersite with target - must raise error - """ - target_dir = script.scratch_path / 'target' - result = script.pip_install_local( - '--user', '-t', target_dir, "simple==1.0", expect_error=True - ) - assert "Can not combine '--user' and '--target'" in result.stderr, ( - str(result) - ) - - result = script.pip_install_local( - '--user', '--target', target_dir, "simple==1.0", expect_error=True - ) - assert "Can not combine '--user' and '--target'" in result.stderr, ( - str(result) - ) - - def test_install_nonlocal_compatible_wheel(script, data): target_dir = script.scratch_path / 'target' @@ -1031,21 +1010,6 @@ def test_install_editable_with_prefix(script): assert install_path in result.files_created, str(result) -def test_install_package_conflict_prefix_and_user(script, data): - """ - Test installing a package using pip install --prefix --user errors out - """ - prefix_path = script.scratch_path / 'prefix' - result = script.pip( - 'install', '-f', data.find_links, '--no-index', '--user', - '--prefix', prefix_path, 'simple==1.0', - expect_error=True, quiet=True, - ) - assert ( - "Can not combine '--user' and '--prefix'" in result.stderr - ) - - def test_install_package_that_emits_unicode(script, data): """ Install a package with a setup.py that emits UTF-8 output and then fails. diff --git a/tests/unit/test_command_install.py b/tests/unit/test_command_install.py index 80aaf4ae8b3..31b33ce6792 100644 --- a/tests/unit/test_command_install.py +++ b/tests/unit/test_command_install.py @@ -1,4 +1,5 @@ import errno +from itertools import product import pytest from mock import patch @@ -9,39 +10,90 @@ decide_user_install, warn_deprecated_install_options, ) +from pip._internal.exceptions import CommandError, InstallationError from pip._internal.req.req_install import InstallRequirement +ENABLE_USER_SITE = 'site.ENABLE_USER_SITE' +WRITABLE = 'pip._internal.commands.install.test_writable_dir' +ISDIR = 'pip._internal.commands.install.os.path.isdir' +EXISTS = 'pip._internal.commands.install.os.path.exists' + +def false(*args, **kwargs): + """Return False.""" + return False + + +def true(*args, **kwargs): + """Return True.""" + return True + + +# virtualenv_no_global is tested by functional test +@patch('pip._internal.commands.install.virtualenv_no_global', false) class TestDecideUserInstall: - @patch('site.ENABLE_USER_SITE', True) - @patch('pip._internal.commands.install.site_packages_writable') - def test_prefix_and_target(self, sp_writable): - sp_writable.return_value = False - - assert decide_user_install( - use_user_site=None, prefix_path='foo' - ) is False - - assert decide_user_install( - use_user_site=None, target_dir='bar' - ) is False - - @pytest.mark.parametrize( - "enable_user_site,site_packages_writable,result", [ - (True, True, False), - (True, False, True), - (False, True, False), - (False, False, False), - ]) - def test_most_cases( - self, enable_user_site, site_packages_writable, result, monkeypatch, - ): - monkeypatch.setattr('site.ENABLE_USER_SITE', enable_user_site) - monkeypatch.setattr( - 'pip._internal.commands.install.site_packages_writable', - lambda **kw: site_packages_writable - ) - assert decide_user_install(use_user_site=None) is result + @pytest.mark.parametrize('use_user_site,prefix_path,target_dir,root_path', + filter(lambda args: sum(map(bool, args)) > 1, + product((False, True), (None, 'foo'), + (None, 'bar'), (None, 'baz')))) + def test_conflicts(self, use_user_site, + prefix_path, target_dir, root_path): + with pytest.raises(CommandError): + decide_user_install( + use_user_site=use_user_site, prefix_path=prefix_path, + target_dir=target_dir, root_path=root_path) + + def test_target_dir(self): + with patch(WRITABLE, true): + with patch(EXISTS, true), patch(ISDIR, false): + with pytest.raises(InstallationError): + decide_user_install(target_dir='bar') + for exists, isdir in (false, false), (false, true), (true, true): + with patch(EXISTS, exists), patch(ISDIR, isdir): + assert decide_user_install(target_dir='bar') is False + + def test_target_writable(self): + with patch(EXISTS, false): + with patch(WRITABLE, false), pytest.raises(InstallationError): + decide_user_install(target_dir='bar') + with patch(WRITABLE, true): + assert decide_user_install(target_dir='bar') is False + + def test_prefix_writable(self): + with patch(WRITABLE, false), pytest.raises(InstallationError): + decide_user_install(prefix_path='foo') + with patch(WRITABLE, true): + assert decide_user_install(prefix_path='foo') is False + + def test_global_site_writable(self): + with patch(ENABLE_USER_SITE, True): + with patch(WRITABLE, false): + with pytest.raises(InstallationError): + decide_user_install(use_user_site=False) + with pytest.raises(InstallationError): + decide_user_install(root_path='baz') + assert decide_user_install() is True + with patch(WRITABLE, true): + assert decide_user_install(use_user_site=True) is True + assert decide_user_install(root_path='baz') is False + assert decide_user_install() is False + + def test_enable_user_site(self): + with patch(WRITABLE, true): + with patch(ENABLE_USER_SITE, None): + assert decide_user_install() is False + assert decide_user_install(use_user_site=False) is False + with pytest.raises(InstallationError): + decide_user_install(use_user_site=True) + with patch(ENABLE_USER_SITE, False): + assert decide_user_install() is False + assert decide_user_install(use_user_site=False) is False + assert decide_user_install(use_user_site=True) is True + with patch(ENABLE_USER_SITE, True): + assert decide_user_install(use_user_site=False) is False + assert decide_user_install(use_user_site=True) is True + with patch(WRITABLE, false), patch(ENABLE_USER_SITE, True): + assert decide_user_install() is True def test_deprecation_notice_for_pip_install_options(recwarn):