Replace `strtobool` by a simpler check in `shell` command (#6191)

* test(commands): add tests for `shell`

* refactor(commands/shell): move venv check to dedicated function

* feat(commands/shell): replace `strtobool` with simpler check

Co-authored-by: Xinyuan Chen <45612704+tddschn@users.noreply.github.com>

* test(commands/shell): add tests for `_is_venv_activated`

* chore(cirrus): explicitly set `SHELL` env var

Co-authored-by: Xinyuan Chen <45612704+tddschn@users.noreply.github.com>
Co-authored-by: Bartosz Sokorski <b.sokorski@gmail.com>

.cirrus.yml

@@ -8,6 +8,9 @@ test_task:
   only_if: $CIRRUS_TAG == ''
   only_if: $CIRRUS_TAG == ''
   skip: "!changesInclude('.cirrus.yml', 'poetry.lock', 'pyproject.toml', '**.json','**.py')"
   skip: "!changesInclude('.cirrus.yml', 'poetry.lock', 'pyproject.toml', '**.json','**.py')"
   env:
   env:
+    # `SHELL` environment variable is not set by default, so we explicitly set it to
+    # avoid failures on tests that depend on it.
+    SHELL: sh
     matrix:
     matrix:
       - PYTHON: python3.7
       - PYTHON: python3.7
       - PYTHON: python3.8
       - PYTHON: python3.8

src/poetry/console/commands/shell.py

@@ -2,7 +2,6 @@ from __future__ import annotations
 
 
 import sys
 import sys
 
 
-from distutils.util import strtobool
 from os import environ
 from os import environ
 from typing import TYPE_CHECKING
 from typing import TYPE_CHECKING
 from typing import cast
 from typing import cast
@@ -27,10 +26,7 @@ If one doesn't exist yet, it will be created.
         from poetry.utils.shell import Shell
         from poetry.utils.shell import Shell
 
 
         # Check if it's already activated or doesn't exist and won't be created
         # Check if it's already activated or doesn't exist and won't be created
-        venv_activated = strtobool(environ.get("POETRY_ACTIVE", "0")) or getattr(
-            sys, "real_prefix", sys.prefix
-        ) == str(self.env.path)
-        if venv_activated:
+        if self._is_venv_activated():
             self.line(
             self.line(
                 f"Virtual environment already activated: <info>{self.env.path}</>"
                 f"Virtual environment already activated: <info>{self.env.path}</>"
             )
             )
@@ -51,3 +47,8 @@ If one doesn't exist yet, it will be created.
         environ.pop("POETRY_ACTIVE")
         environ.pop("POETRY_ACTIVE")
 
 
         return 0
         return 0
+
+    def _is_venv_activated(self) -> bool:
+        return bool(environ.get("POETRY_ACTIVE")) or getattr(
+            sys, "real_prefix", sys.prefix
+        ) == str(self.env.path)

tests/console/commands/test_shell.py

+from __future__ import annotations
+
+import os
+
+from pathlib import Path
+from typing import TYPE_CHECKING
+
+import pytest
+
+
+if TYPE_CHECKING:
+    from cleo.testers.command_tester import CommandTester
+    from pytest_mock import MockerFixture
+
+    from tests.types import CommandTesterFactory
+
+
+@pytest.fixture
+def tester(command_tester_factory: CommandTesterFactory) -> CommandTester:
+    return command_tester_factory("shell")
+
+
+def test_shell(tester: CommandTester, mocker: MockerFixture):
+    shell_activate = mocker.patch("poetry.utils.shell.Shell.activate")
+
+    tester.execute()
+
+    expected_output = f"Spawning shell within {tester.command.env.path}\n"
+
+    shell_activate.assert_called_once_with(tester.command.env)
+    assert tester.io.fetch_output() == expected_output
+    assert tester.status_code == 0
+
+
+def test_shell_already_active(tester: CommandTester, mocker: MockerFixture):
+    os.environ["POETRY_ACTIVE"] = "1"
+    shell_activate = mocker.patch("poetry.utils.shell.Shell.activate")
+
+    tester.execute()
+
+    expected_output = (
+        f"Virtual environment already activated: {tester.command.env.path}\n"
+    )
+
+    shell_activate.assert_not_called()
+    assert tester.io.fetch_output() == expected_output
+    assert tester.status_code == 0
+
+
+@pytest.mark.parametrize(
+    ("poetry_active", "real_prefix", "prefix", "expected"),
+    [
+        (None, None, "", False),
+        ("", None, "", False),
+        (" ", None, "", True),
+        ("0", None, "", True),
+        ("1", None, "", True),
+        ("foobar", None, "", True),
+        ("1", "foobar", "foobar", True),
+        (None, "foobar", "foobar", True),
+        (None, "foobar", "foo", True),
+        (None, None, "foobar", True),
+        (None, "foo", "foobar", False),
+        (None, "foo", "foo", False),
+    ],
+)
+def test__is_venv_activated(
+    tester: CommandTester,
+    mocker: MockerFixture,
+    poetry_active: str | None,
+    real_prefix: str | None,
+    prefix: str,
+    expected: bool,
+):
+    mocker.patch.object(tester.command.env, "_path", Path("foobar"))
+    mocker.patch("sys.prefix", prefix)
+
+    if real_prefix is not None:
+        mocker.patch("sys.real_prefix", real_prefix, create=True)
+
+    if poetry_active is not None:
+        os.environ["POETRY_ACTIVE"] = poetry_active
+
+    assert tester.command._is_venv_activated() is expected