Add unit tests for user install decision

Functional tests for conflict install locations are removed in favor of
the unittest which handle more cases.

Author: McSinyx

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

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.

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):