From fa2e154b419238532cba9664fd444bcc00dfb787 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Skytt=C3=A4?= Date: Thu, 15 Aug 2019 08:36:06 +0300 Subject: [PATCH 1/5] Stabilize python default version lookup For example, for sys.executable: /usr/bin/python3 -> python3.7 ...the default lookup may return either python3 or python3.7. Make the order deterministic by iterating over tuple, not set, of candidates. --- pre_commit/languages/python.py | 12 +++++++++--- tests/languages/python_test.py | 15 +++++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/pre_commit/languages/python.py b/pre_commit/languages/python.py index 5d48fb89..df00a071 100644 --- a/pre_commit/languages/python.py +++ b/pre_commit/languages/python.py @@ -43,14 +43,13 @@ def _find_by_py_launcher(version): # pragma: no cover (windows only) pass -def _get_default_version(): # pragma: no cover (platform dependent) +def _find_by_sys_executable(): def _norm(path): _, exe = os.path.split(path.lower()) exe, _, _ = exe.partition('.exe') if find_executable(exe) and exe not in {'python', 'pythonw'}: return exe - # First attempt from `sys.executable` (or the realpath) # On linux, I see these common sys.executables: # # system `python`: /usr/bin/python -> python2.7 @@ -59,10 +58,17 @@ def _get_default_version(): # pragma: no cover (platform dependent) # virtualenv v -ppython2: v/bin/python -> python2 # virtualenv v -ppython2.7: v/bin/python -> python2.7 # virtualenv v -ppypy: v/bin/python -> v/bin/pypy - for path in {sys.executable, os.path.realpath(sys.executable)}: + for path in (sys.executable, os.path.realpath(sys.executable)): exe = _norm(path) if exe: return exe + return None + + +def _get_default_version(): # pragma: no cover (platform dependent) + + # First attempt from `sys.executable` (or the realpath) + exe = _find_by_sys_executable() # Next try the `pythonX.X` executable exe = 'python{}.{}'.format(*sys.version_info) diff --git a/tests/languages/python_test.py b/tests/languages/python_test.py index 426d3ec6..52e0e85c 100644 --- a/tests/languages/python_test.py +++ b/tests/languages/python_test.py @@ -32,3 +32,18 @@ def test_sys_executable_matches(v): def test_sys_executable_matches_does_not_match(v): with mock.patch.object(sys, 'version_info', (3, 6, 7)): assert not python._sys_executable_matches(v) + + +@pytest.mark.parametrize( + 'exe,realpath,expected', ( + ('/usr/bin/python3', '/usr/bin/python3.7', 'python3'), + ('/usr/bin/python', '/usr/bin/python3.7', 'python3.7'), + ('/usr/bin/python', '/usr/bin/python', None), + ('/usr/bin/python3.6m', '/usr/bin/python3.6m', 'python3.6m'), + ('v/bin/python', 'v/bin/pypy', 'pypy'), + ), +) +def test_find_by_sys_executable(exe, realpath, expected): + with mock.patch.object(sys, 'executable', exe): + with mock.patch('os.path.realpath', return_value=realpath): + assert python._find_by_sys_executable() == expected From c3778308980be22f0d2708b7ffeed2d3e11e60fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Skytt=C3=A4?= Date: Thu, 15 Aug 2019 18:30:43 +0300 Subject: [PATCH 2/5] Mock find_executable for find_by_sys_executable test --- tests/languages/python_test.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/languages/python_test.py b/tests/languages/python_test.py index 52e0e85c..4506f9f0 100644 --- a/tests/languages/python_test.py +++ b/tests/languages/python_test.py @@ -44,6 +44,12 @@ def test_sys_executable_matches_does_not_match(v): ), ) def test_find_by_sys_executable(exe, realpath, expected): + def mocked_find_executable(exe): + return exe.rpartition('/')[2] with mock.patch.object(sys, 'executable', exe): with mock.patch('os.path.realpath', return_value=realpath): - assert python._find_by_sys_executable() == expected + with mock.patch( + 'pre_commit.parse_shebang.find_executable', + side_effect=mocked_find_executable, + ): + assert python._find_by_sys_executable() == expected From 38da98d2d65d9df37671aba3f10fbbd080fadd4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Skytt=C3=A4?= Date: Thu, 15 Aug 2019 18:43:31 +0300 Subject: [PATCH 3/5] Address @asottile's review comments --- pre_commit/languages/python.py | 1 - tests/languages/python_test.py | 9 +++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pre_commit/languages/python.py b/pre_commit/languages/python.py index df00a071..1585a7fc 100644 --- a/pre_commit/languages/python.py +++ b/pre_commit/languages/python.py @@ -66,7 +66,6 @@ def _find_by_sys_executable(): def _get_default_version(): # pragma: no cover (platform dependent) - # First attempt from `sys.executable` (or the realpath) exe = _find_by_sys_executable() diff --git a/tests/languages/python_test.py b/tests/languages/python_test.py index 4506f9f0..3634fa4f 100644 --- a/tests/languages/python_test.py +++ b/tests/languages/python_test.py @@ -7,6 +7,7 @@ import sys import mock import pytest +import pre_commit.parse_shebang from pre_commit.languages import python @@ -35,7 +36,7 @@ def test_sys_executable_matches_does_not_match(v): @pytest.mark.parametrize( - 'exe,realpath,expected', ( + ('exe', 'realpath', 'expected'), ( ('/usr/bin/python3', '/usr/bin/python3.7', 'python3'), ('/usr/bin/python', '/usr/bin/python3.7', 'python3.7'), ('/usr/bin/python', '/usr/bin/python', None), @@ -47,9 +48,9 @@ def test_find_by_sys_executable(exe, realpath, expected): def mocked_find_executable(exe): return exe.rpartition('/')[2] with mock.patch.object(sys, 'executable', exe): - with mock.patch('os.path.realpath', return_value=realpath): - with mock.patch( - 'pre_commit.parse_shebang.find_executable', + with mock.patch.object(os.path, 'realpath', return_value=realpath): + with mock.patch.object( + pre_commit.parse_shebang, 'find_executable', side_effect=mocked_find_executable, ): assert python._find_by_sys_executable() == expected From 562276098c5c42f364cdf836e1842d30265fd4ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Skytt=C3=A4?= Date: Thu, 15 Aug 2019 18:54:08 +0300 Subject: [PATCH 4/5] Address more @asottile's review comments --- pre_commit/languages/python.py | 2 ++ tests/languages/python_test.py | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/pre_commit/languages/python.py b/pre_commit/languages/python.py index 1585a7fc..6d125a43 100644 --- a/pre_commit/languages/python.py +++ b/pre_commit/languages/python.py @@ -68,6 +68,8 @@ def _find_by_sys_executable(): def _get_default_version(): # pragma: no cover (platform dependent) # First attempt from `sys.executable` (or the realpath) exe = _find_by_sys_executable() + if exe: + return exe # Next try the `pythonX.X` executable exe = 'python{}.{}'.format(*sys.version_info) diff --git a/tests/languages/python_test.py b/tests/languages/python_test.py index 3634fa4f..debf9753 100644 --- a/tests/languages/python_test.py +++ b/tests/languages/python_test.py @@ -7,7 +7,7 @@ import sys import mock import pytest -import pre_commit.parse_shebang +from pre_commit import parse_shebang from pre_commit.languages import python @@ -50,7 +50,7 @@ def test_find_by_sys_executable(exe, realpath, expected): with mock.patch.object(sys, 'executable', exe): with mock.patch.object(os.path, 'realpath', return_value=realpath): with mock.patch.object( - pre_commit.parse_shebang, 'find_executable', + parse_shebang, 'find_executable', side_effect=mocked_find_executable, ): assert python._find_by_sys_executable() == expected From f84b19748d7d0dfda496b73ab365a2e64b377696 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Skytt=C3=A4?= Date: Thu, 15 Aug 2019 19:28:07 +0300 Subject: [PATCH 5/5] Patch the correct find_executable --- tests/languages/python_test.py | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/tests/languages/python_test.py b/tests/languages/python_test.py index debf9753..d9d8ecd5 100644 --- a/tests/languages/python_test.py +++ b/tests/languages/python_test.py @@ -7,7 +7,6 @@ import sys import mock import pytest -from pre_commit import parse_shebang from pre_commit.languages import python @@ -45,12 +44,7 @@ def test_sys_executable_matches_does_not_match(v): ), ) def test_find_by_sys_executable(exe, realpath, expected): - def mocked_find_executable(exe): - return exe.rpartition('/')[2] with mock.patch.object(sys, 'executable', exe): with mock.patch.object(os.path, 'realpath', return_value=realpath): - with mock.patch.object( - parse_shebang, 'find_executable', - side_effect=mocked_find_executable, - ): + with mock.patch.object(python, 'find_executable', lambda x: x): assert python._find_by_sys_executable() == expected