Merge pull request #1219 from pypa/add-ci

gaborbernat · web-flow · commit 6974665f6eae · 2018-10-25T12:25:15.000+01:00
* update detecting Windows Python installations

- now correctly detects current user installation and not just
  those installed at the system level
- for Python versions 3.5+ (which track 32-bit &amp; 64-bit installations
  separately), recognize version tags X.Y, X.Y-32 &amp; X.Y-64 where X.Y
  represents the 64-bit installation if available or 32-bit otherwise

* touch up &amp; improve `resolve_interpreter()` tests

- tests no longer permanently modify the `virtualenv` module under test
  (`virtualenv.is_executable` changes were leaking from some tests)
- tests now check that `get_installed_python()` results unrelated to the
  given version tag do not affect the result
- simple patch function's return values now given in `@patch` decorators
  to make the test code more compact

* test resolve_interpreter() with registered python installations

* test `get_installed_pythons()` on and off Windows platform

* add embed tox env

* disable Python 3.4

* fix coverage reporter
diff --git a/azure-pipelines.yml b/azure-pipelines.yml
@@ -20,6 +20,8 @@ trigger:
     - .github/*
 
 jobs:
+- template: azure-run-tox-env.yml
+  parameters: {tox: embed, python: 3.7}
 - template: azure-run-tox-env.yml
   parameters: {tox: docs, python: 3.7}
 - template: azure-run-tox-env.yml
diff --git a/bin/rebuild-script.py b/bin/rebuild-script.py
@@ -16,7 +16,7 @@ def crc32(data):
 
 
 here = os.path.dirname(__file__)
-script = os.path.join(here, '..', 'virtualenv.py')
+script = os.path.join(here, '..', 'src', 'virtualenv.py')
 
 gzip = codecs.lookup('zlib')
 b64 = codecs.lookup('base64')
@@ -28,6 +28,7 @@ def crc32(data):
 
 
 def rebuild(script_path):
+    exit_code = 0
     with open(script_path, 'rb') as f:
         script_content = f.read()
     parts = []
@@ -52,6 +53,7 @@ def rebuild(script_path):
             print('  File up to date (crc: %08x)' % new_crc)
             parts += [match.group(0)]
             continue
+        exit_code = 1
         # Else: content has changed
         crc = crc32(gzip.decode(b64.decode(data)[0])[0])
         print('  Content changed (crc: %08x -> %08x)' %
@@ -75,6 +77,7 @@ def rebuild(script_path):
         print('No changes in content')
     if match is None:
         print('No variables were matched/found')
+    raise SystemExit(exit_code)
 
 if __name__ == '__main__':
     rebuild(script)
diff --git a/src/virtualenv.py b/src/virtualenv.py
@@ -85,33 +85,51 @@ def get_installed_pythons():
         import _winreg as winreg
 
     def get_installed_pythons():
-        try:
-            python_core = winreg.CreateKey(winreg.HKEY_LOCAL_MACHINE,
-                                           "Software\\Python\\PythonCore")
-        except WindowsError:
-            # No registered Python installations
-            return {}
-        i = 0
-        versions = []
-        while True:
-            try:
-                versions.append(winreg.EnumKey(python_core, i))
-                i = i + 1
-            except WindowsError:
-                break
         exes = dict()
-        for ver in versions:
+        # If both system and current user installations are found for a
+        # particular Python version, the current user one is used
+        for key in (winreg.HKEY_LOCAL_MACHINE, winreg.HKEY_CURRENT_USER):
             try:
-                path = winreg.QueryValue(python_core, "%s\\InstallPath" % ver)
+                python_core = winreg.CreateKey(key,
+                                               "Software\\Python\\PythonCore")
             except WindowsError:
+                # No registered Python installations
                 continue
-            exes[ver] = join(path, "python.exe")
-
-        winreg.CloseKey(python_core)
+            i = 0
+            while True:
+                try:
+                    version = winreg.EnumKey(python_core, i)
+                    i += 1
+                    try:
+                        path = winreg.QueryValue(python_core,
+                                                 "%s\\InstallPath" % version)
+                    except WindowsError:
+                        continue
+                    exes[version] = join(path, "python.exe")
+                except WindowsError:
+                    break
+            winreg.CloseKey(python_core)
+
+        # For versions that track separate 32-bit (`X.Y-32`) & 64-bit (`X-Y`)
+        # installation registrations, add a `X.Y-64` version tag and make the
+        # extensionless `X.Y` version tag represent the 64-bit installation if
+        # available or 32-bit if it is not
+        updated = {}
+        for ver in exes:
+            if ver < '3.5':
+                continue
+            if ver.endswith('-32'):
+                base_ver = ver[:-3]
+                if base_ver not in exes:
+                    updated[base_ver] = exes[ver]
+            else:
+                updated[ver + '-64'] = exes[ver]
+        exes.update(updated)
 
         # Add the major versions
         # Sort the keys, then repeatedly update the major version entry
-        # Last executable (i.e., highest version) wins with this approach
+        # Last executable (i.e., highest version) wins with this approach,
+        # 64-bit over 32-bit if both are found
         for ver in sorted(exes):
             exes[ver[0]] = exes[ver]
 
diff --git a/tests/test_cmdline.py b/tests/test_cmdline.py
@@ -27,7 +27,7 @@ def test_commandline_explicit_interp(tmpdir):
 # registry layout is not well documented, and it's not clear that the feature
 # is sufficiently widely used to be worth fixing.
 # See https://github.com/pypa/virtualenv/issues/864
-@pytest.mark.skipif("sys.platform == 'win32' and sys.version_info[:2] >= (3,5)")
+@pytest.mark.skipif("sys.platform == 'win32' and sys.version_info[:1] >= (3,)")
 def test_commandline_abbrev_interp(tmpdir):
     """Specifying abbreviated forms of the Python interpreter should work"""
     if sys.platform == 'win32':
diff --git a/tests/test_virtualenv.py b/tests/test_virtualenv.py
@@ -7,52 +7,224 @@
 import pytest
 import platform  # noqa
 
-from mock import patch, Mock
+from mock import call, Mock, NonCallableMock, patch
 
 
 def test_version():
     """Should have a version string"""
     assert virtualenv.virtualenv_version, "Should have version"
 
 
-@patch('os.path.exists')
-def test_resolve_interpreter_with_absolute_path(mock_exists):
+class TestGetInstalledPythons:
+    key_local_machine = 'key-local-machine'
+    key_current_user = 'key-current-user'
+
+    @classmethod
+    def mock_virtualenv_winreg(cls, monkeypatch, data):
+        def enum_key(key, index):
+            try:
+                return data.get(key, [])[index]
+            except IndexError:
+                raise WindowsError
+
+        def query_value(key, path):
+            installed_version_tags = data.get(key, [])
+            suffix = '\\InstallPath'
+            if path.endswith(suffix):
+                version_tag = path[:-len(suffix)]
+                if version_tag in installed_version_tags:
+                    return '{}-{}-path'.format(key, version_tag)
+            raise WindowsError
+
+        mock_winreg = NonCallableMock(spec_set=[
+            'HKEY_LOCAL_MACHINE',
+            'HKEY_CURRENT_USER',
+            'CreateKey',
+            'EnumKey',
+            'QueryValue',
+            'CloseKey'])
+        mock_winreg.HKEY_LOCAL_MACHINE = 'HKEY_LOCAL_MACHINE'
+        mock_winreg.HKEY_CURRENT_USER = 'HKEY_CURRENT_USER'
+        mock_winreg.CreateKey.side_effect = [cls.key_local_machine,
+                                             cls.key_current_user]
+        mock_winreg.EnumKey.side_effect = enum_key
+        mock_winreg.QueryValue.side_effect = query_value
+        mock_winreg.CloseKey.return_value = None
+        monkeypatch.setattr(virtualenv, 'winreg', mock_winreg)
+        return mock_winreg
+
+    @pytest.mark.skipif(sys.platform == 'win32',
+                        reason='non-windows specific test')
+    def test_on_non_windows(self, monkeypatch):
+        assert not virtualenv.is_win
+        assert not hasattr(virtualenv, 'winreg')
+        assert virtualenv.get_installed_pythons() == {}
+
+    @pytest.mark.skipif(sys.platform != 'win32',
+                        reason='non-windows specific test')
+    def test_on_windows(self, monkeypatch):
+        assert virtualenv.is_win
+        mock_winreg = self.mock_virtualenv_winreg(monkeypatch, {
+            self.key_local_machine: (
+                '2.4',
+                '2.7',
+                '3.2',
+                '3.4',
+                '3.5',  # 64-bit only
+                '3.6-32',  # 32-bit only
+                '3.7', '3.7-32',  # both 32 & 64-bit with a 64-bit user install
+                '3.8'),  # 64-bit with a 32-bit user install
+            self.key_current_user: (
+                '2.5',
+                '2.7',
+                '3.7',
+                '3.8-32')})
+        monkeypatch.setattr(virtualenv, 'join', '{}\\{}'.format)
+
+        installed_pythons = virtualenv.get_installed_pythons()
+
+        assert installed_pythons == {
+            '2': self.key_current_user + '-2.7-path\\python.exe',
+            '2.4': self.key_local_machine + '-2.4-path\\python.exe',
+            '2.5': self.key_current_user + '-2.5-path\\python.exe',
+            '2.7': self.key_current_user + '-2.7-path\\python.exe',
+            '3': self.key_local_machine + '-3.8-path\\python.exe',
+            '3.2': self.key_local_machine + '-3.2-path\\python.exe',
+            '3.4': self.key_local_machine + '-3.4-path\\python.exe',
+            '3.5': self.key_local_machine + '-3.5-path\\python.exe',
+            '3.5-64': self.key_local_machine + '-3.5-path\\python.exe',
+            '3.6': self.key_local_machine + '-3.6-32-path\\python.exe',
+            '3.6-32': self.key_local_machine + '-3.6-32-path\\python.exe',
+            '3.7': self.key_current_user + '-3.7-path\\python.exe',
+            '3.7-32': self.key_local_machine + '-3.7-32-path\\python.exe',
+            '3.7-64': self.key_current_user + '-3.7-path\\python.exe',
+            '3.8': self.key_local_machine + '-3.8-path\\python.exe',
+            '3.8-32': self.key_current_user + '-3.8-32-path\\python.exe',
+            '3.8-64': self.key_local_machine + '-3.8-path\\python.exe'}
+        assert mock_winreg.mock_calls == [
+            call.CreateKey(mock_winreg.HKEY_LOCAL_MACHINE,
+                           'Software\\Python\\PythonCore'),
+            call.EnumKey(self.key_local_machine, 0),
+            call.QueryValue(self.key_local_machine, '2.4\\InstallPath'),
+            call.EnumKey(self.key_local_machine, 1),
+            call.QueryValue(self.key_local_machine, '2.7\\InstallPath'),
+            call.EnumKey(self.key_local_machine, 2),
+            call.QueryValue(self.key_local_machine, '3.2\\InstallPath'),
+            call.EnumKey(self.key_local_machine, 3),
+            call.QueryValue(self.key_local_machine, '3.4\\InstallPath'),
+            call.EnumKey(self.key_local_machine, 4),
+            call.QueryValue(self.key_local_machine, '3.5\\InstallPath'),
+            call.EnumKey(self.key_local_machine, 5),
+            call.QueryValue(self.key_local_machine, '3.6-32\\InstallPath'),
+            call.EnumKey(self.key_local_machine, 6),
+            call.QueryValue(self.key_local_machine, '3.7\\InstallPath'),
+            call.EnumKey(self.key_local_machine, 7),
+            call.QueryValue(self.key_local_machine, '3.7-32\\InstallPath'),
+            call.EnumKey(self.key_local_machine, 8),
+            call.QueryValue(self.key_local_machine, '3.8\\InstallPath'),
+            call.EnumKey(self.key_local_machine, 9),
+            call.CloseKey(self.key_local_machine),
+            call.CreateKey(mock_winreg.HKEY_CURRENT_USER,
+                           'Software\\Python\\PythonCore'),
+            call.EnumKey(self.key_current_user, 0),
+            call.QueryValue(self.key_current_user, '2.5\\InstallPath'),
+            call.EnumKey(self.key_current_user, 1),
+            call.QueryValue(self.key_current_user, '2.7\\InstallPath'),
+            call.EnumKey(self.key_current_user, 2),
+            call.QueryValue(self.key_current_user, '3.7\\InstallPath'),
+            call.EnumKey(self.key_current_user, 3),
+            call.QueryValue(self.key_current_user, '3.8-32\\InstallPath'),
+            call.EnumKey(self.key_current_user, 4),
+            call.CloseKey(self.key_current_user)]
+
+    @pytest.mark.skipif(sys.platform != 'win32',
+                        reason='windows specific test')
+    def test_on_windows_with_no_installations(self, monkeypatch):
+        assert virtualenv.is_win
+        mock_winreg = self.mock_virtualenv_winreg(monkeypatch, {})
+
+        installed_pythons = virtualenv.get_installed_pythons()
+
+        assert installed_pythons == {}
+        assert mock_winreg.mock_calls == [
+            call.CreateKey(mock_winreg.HKEY_LOCAL_MACHINE,
+                           'Software\\Python\\PythonCore'),
+            call.EnumKey(self.key_local_machine, 0),
+            call.CloseKey(self.key_local_machine),
+            call.CreateKey(mock_winreg.HKEY_CURRENT_USER,
+                           'Software\\Python\\PythonCore'),
+            call.EnumKey(self.key_current_user, 0),
+            call.CloseKey(self.key_current_user)]
+
+
+@patch('distutils.spawn.find_executable')
+@patch('virtualenv.is_executable', return_value=True)
+@patch('virtualenv.get_installed_pythons')
+@patch('os.path.exists', return_value=True)
+@patch('os.path.abspath')
+def test_resolve_interpreter_with_installed_python(mock_abspath, mock_exists,
+        mock_get_installed_pythons, mock_is_executable, mock_find_executable):
+    test_tag = 'foo'
+    test_path = '/path/to/foo/python.exe'
+    test_abs_path = 'some-abs-path'
+    test_found_path = 'some-found-path'
+    mock_get_installed_pythons.return_value = {
+        test_tag: test_path,
+        test_tag + '2': test_path + '2'}
+    mock_abspath.return_value = test_abs_path
+    mock_find_executable.return_value = test_found_path
+
+    exe = virtualenv.resolve_interpreter('foo')
+
+    assert exe == test_found_path, \
+        "installed python should be accessible by key"
+
+    mock_get_installed_pythons.assert_called_once_with()
+    mock_abspath.assert_called_once_with(test_path)
+    mock_find_executable.assert_called_once_with(test_path)
+    mock_exists.assert_called_once_with(test_found_path)
+    mock_is_executable.assert_called_once_with(test_found_path)
+
+
+@patch('virtualenv.is_executable', return_value=True)
+@patch('virtualenv.get_installed_pythons', return_value={'foo': 'bar'})
+@patch('os.path.exists', return_value=True)
+def test_resolve_interpreter_with_absolute_path(
+        mock_exists, mock_get_installed_pythons, mock_is_executable):
     """Should return absolute path if given and exists"""
-    mock_exists.return_value = True
-    virtualenv.is_executable = Mock(return_value=True)
     test_abs_path = os.path.abspath("/usr/bin/python53")
 
     exe = virtualenv.resolve_interpreter(test_abs_path)
 
     assert exe == test_abs_path, "Absolute path should return as is"
 
     mock_exists.assert_called_with(test_abs_path)
-    virtualenv.is_executable.assert_called_with(test_abs_path)
+    mock_is_executable.assert_called_with(test_abs_path)
 
 
-@patch('os.path.exists')
-def test_resolve_interpreter_with_nonexistent_interpreter(mock_exists):
+@patch('virtualenv.get_installed_pythons', return_value={'foo': 'bar'})
+@patch('os.path.exists', return_value=False)
+def test_resolve_interpreter_with_nonexistent_interpreter(
+        mock_exists, mock_get_installed_pythons):
     """Should SystemExit with an nonexistent python interpreter path"""
-    mock_exists.return_value = False
-
     with pytest.raises(SystemExit):
         virtualenv.resolve_interpreter("/usr/bin/python53")
 
     mock_exists.assert_called_with("/usr/bin/python53")
 
 
-@patch('os.path.exists')
-def test_resolve_interpreter_with_invalid_interpreter(mock_exists):
+@patch('virtualenv.is_executable', return_value=False)
+@patch('os.path.exists', return_value=True)
+def test_resolve_interpreter_with_invalid_interpreter(mock_exists,
+                                                      mock_is_executable):
     """Should exit when with absolute path if not exists"""
-    mock_exists.return_value = True
-    virtualenv.is_executable = Mock(return_value=False)
     invalid = os.path.abspath("/usr/bin/pyt_hon53")
 
     with pytest.raises(SystemExit):
         virtualenv.resolve_interpreter(invalid)
 
     mock_exists.assert_called_with(invalid)
-    virtualenv.is_executable.assert_called_with(invalid)
+    mock_is_executable.assert_called_with(invalid)
 
 
 def test_activate_after_future_statements():
diff --git a/tox.ini b/tox.ini
