-
Notifications
You must be signed in to change notification settings - Fork 2.5k
fix(env): prevent crash when invalid Python or Python 2.x is in PATH #10962
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
493f381
4e5e5ab
7770eb2
d83c7bb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -141,3 +141,77 @@ def test_python_find_compatible( | |
| python = Python.get_compatible_python(poetry) | ||
|
|
||
| assert Version.from_parts(3, 4) <= python.version <= Version.from_parts(4, 0) | ||
|
|
||
|
|
||
| def test_python_ignores_broken_installations(mocker: MockerFixture) -> None: | ||
| from subprocess import CalledProcessError | ||
|
|
||
| mock_good_pv = mocker.MagicMock(spec=findpython.PythonVersion) | ||
| mock_good_pv.executable = Path("/usr/bin/python3.12") | ||
| mock_good_pv.interpreter = Path("/usr/bin/python3.12") | ||
| mock_good_pv.version = packaging.version.Version("3.12.0") | ||
|
|
||
| mock_bad_pv = mocker.MagicMock(spec=findpython.PythonVersion) | ||
| mock_bad_pv.executable = Path("/usr/bin/python2.7") | ||
| type(mock_bad_pv).interpreter = mocker.PropertyMock( | ||
| side_effect=CalledProcessError(2, ["/usr/bin/python2.7", "-Ic", "..."]) | ||
| ) | ||
|
|
||
| mocker.patch("findpython.find_all", return_value=[mock_bad_pv, mock_good_pv]) | ||
|
|
||
| pythons = list(Python.find_all()) | ||
| assert len(pythons) == 1 | ||
| assert pythons[0].executable == Path("/usr/bin/python3.12") | ||
|
|
||
|
|
||
| def test_get_active_python_ignores_broken_installations(mocker: MockerFixture) -> None: | ||
| from subprocess import CalledProcessError | ||
|
|
||
| mock_bad_pv = mocker.MagicMock(spec=findpython.PythonVersion) | ||
| mock_bad_pv.executable = Path("/usr/bin/python2.7") | ||
| type(mock_bad_pv).interpreter = mocker.PropertyMock( | ||
| side_effect=CalledProcessError(2, ["/usr/bin/python2.7", "-Ic", "..."]) | ||
| ) | ||
|
|
||
| mocker.patch( | ||
| "poetry.utils.env.python.providers.ShutilWhichPythonProvider.find_pythons", | ||
| return_value=[mock_bad_pv], | ||
| ) | ||
| mocker.patch("findpython.find", return_value=None) | ||
|
|
||
| assert Python.get_active_python() is None | ||
|
|
||
|
|
||
| def test_get_by_name_ignores_broken_installations(mocker: MockerFixture) -> None: | ||
| from subprocess import CalledProcessError | ||
|
|
||
| mock_bad_pv = mocker.MagicMock(spec=findpython.PythonVersion) | ||
| mock_bad_pv.executable = Path("/usr/bin/python2.7") | ||
| type(mock_bad_pv).interpreter = mocker.PropertyMock( | ||
| side_effect=CalledProcessError(2, ["/usr/bin/python2.7", "-Ic", "..."]) | ||
| ) | ||
|
|
||
| mocker.patch( | ||
| "poetry.utils.env.python.providers.ShutilWhichPythonProvider.find_python_by_name", | ||
| return_value=mock_bad_pv, | ||
| ) | ||
| mocker.patch("findpython.find", return_value=None) | ||
|
|
||
| assert Python.get_by_name("python") is None | ||
|
|
||
|
|
||
| def test_python_properties_raise_value_error_on_subprocess_failure( | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (testing): Expand this test (or add another) to cover Since the implementation now wraps |
||
| mocker: MockerFixture, | ||
| ) -> None: | ||
| from subprocess import CalledProcessError | ||
|
|
||
| mock_bad_pv = mocker.MagicMock(spec=findpython.PythonVersion) | ||
| mock_bad_pv.executable = Path("/usr/bin/python2.7") | ||
| type(mock_bad_pv).interpreter = mocker.PropertyMock( | ||
| side_effect=CalledProcessError(2, ["/usr/bin/python2.7", "-Ic", "..."]) | ||
| ) | ||
|
|
||
| python = Python(python=mock_bad_pv) | ||
| with pytest.raises(ValueError, match="Invalid Python executable"): | ||
| _ = python.executable | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (testing): Add a positive-path test for
get_by_namewhere a broken primary result is ignored but a valid fallback fromfindpython.findis used.The existing test only covers the case where both
ShutilWhichPythonProvider.find_python_by_nameandfindpython.findfail andget_by_namereturnsNone. Please also add a test wherefind_python_by_nameyields a brokenPythonVersion(raisingCalledProcessError/ValueErroroninterpreter), butfindpython.find(python_name)returns a validPythonVersion, and assert thatPython.get_by_name(python_name)returns a non-NonePythonto cover the new fallback behavior.