Only return Python factor on base_python conflict

Consider the following 'tox.ini' file:

  [tox]
  ignore_base_python_conflict = true

  [testenv]
  base_python = python3
  commands = ...

  [testenv:functional{,-py38,-py39,-py310}]
  commands = ...

There is a conflict between the base_python value specified in
'[testenv] base_python' and the value implied by the 'pyXY' factors in
the 'functional-pyXY' test envs. The 'Python._validate_base_python'
function is supposed to resolve this for us and either (a) raise an
error if '[tox] ignore_base_python_conflict' is set to 'false' (default)
or (b) ignore the value of '[testenv] base_python' in favour of the
value implied by the 'pyXY' factor for the given test env if '[tox]
ignore_base_python_conflict' is set to 'true'. There's a bug though.
Rather than returning the 'pyXY' factor, we were returning the entire
test env name ('functional-pyXY'). There is no Python version
corresponding to e.g. 'functional-py39' so this (correctly) fails.

We can correct the issue by only returning the factor that modified the
base_python value, i.e. the 'pyXY' factor. To ensure we do this, we need
some additional logic. It turns out this logic is already present in
another helper method on the 'Python' class, 'extract_base_python', so
we also take the opportunity to de-duplicate and reuse some logic.

Note that this change breaks the ability of users to use a testenv name
like 'py38-64' (to get the 64 bit version of a Python 3.8 interpreter).
Continuing to support this would require much larger change since we'd
no longer be able to strictly delimit factors by hyphens (in this case,
the entirety of 'py38-64' becomes a factor).

Also note that this change emphasises issue #2657, as this will now be
raised for a factor like 'py38-64' since 'tox' (or rather, virtualenv)
is falsely identifying '64' as a valid Python interpreter identifier. We
will fix this separately so the offending test are skipped for now.

Signed-off-by: Stephen Finucane <stephen@that.guru>
Fixes: #2838

docs/changelog/2838.bugfix.rst

@@ -0,0 +1,2 @@
+A testenv with multiple factors, one of which conflicts with a ``base_python`` setting in ``tox.ini``, will now use the
+correct Python interpreter version - by :user:`stephenfin`.

src/tox/tox_env/python/api.py

@@ -127,8 +127,8 @@ def default_base_python(self, conf: Config, env_name: str | None) -> list[str]:
         base_python = None if env_name is None else self.extract_base_python(env_name)
         return [sys.executable if base_python is None else base_python]
 
-    @staticmethod
-    def extract_base_python(env_name: str) -> str | None:
+    @classmethod
+    def extract_base_python(cls, env_name: str) -> str | None:
         candidates: list[str] = []
         for factor in env_name.split("-"):
             spec = PythonSpec.from_string_spec(factor)
@@ -141,14 +141,16 @@ def extract_base_python(env_name: str) -> str | None:
             return next(iter(candidates))
         return None
 
-    @staticmethod
-    def _validate_base_python(env_name: str, base_pythons: list[str], ignore_base_python_conflict: bool) -> list[str]:
-        elements = {env_name}  # match with full env-name
-        elements.update(env_name.split("-"))  # and also any factor
-        for candidate in elements:
-            spec_name = PythonSpec.from_string_spec(candidate)
-            if spec_name.implementation and spec_name.implementation.lower() not in INTERPRETER_SHORT_NAMES:
-                continue
+    @classmethod
+    def _validate_base_python(
+        cls,
+        env_name: str,
+        base_pythons: list[str],
+        ignore_base_python_conflict: bool,
+    ) -> list[str]:
+        env_base_python = cls.extract_base_python(env_name)
+        if env_base_python is not None:
+            spec_name = PythonSpec.from_string_spec(env_base_python)
             for base_python in base_pythons:
                 spec_base = PythonSpec.from_string_spec(base_python)
                 if any(
@@ -158,7 +160,8 @@ def _validate_base_python(env_name: str, base_pythons: list[str], ignore_base_py
                 ):
                     msg = f"env name {env_name} conflicting with base python {base_python}"
                     if ignore_base_python_conflict:
-                        return [env_name]  # ignore the base python settings
+                        # ignore the base python settings and return the thing that looks like a Python version
+                        return [env_base_python]
                     raise Fail(msg)
         return base_pythons
 

tests/tox_env/python/test_python_api.py

@@ -87,26 +87,35 @@ def test_base_python_env_no_conflict(env: str, base_python: list[str], ignore_co
 
 @pytest.mark.parametrize("ignore_conflict", [True, False])
 @pytest.mark.parametrize(
-    ("env", "base_python", "conflict"),
+    ("env", "base_python", "expected", "conflict"),
     [
-        ("cpython", ["pypy"], ["pypy"]),
-        ("pypy", ["cpython"], ["cpython"]),
-        ("pypy2", ["pypy3"], ["pypy3"]),
-        ("py3", ["py2"], ["py2"]),
-        ("py38", ["py39"], ["py39"]),
-        ("py38", ["py38", "py39"], ["py39"]),
-        ("py38", ["python3"], ["python3"]),
-        ("py310", ["py38", "py39"], ["py38", "py39"]),
-        ("py3.11.1", ["py3.11.2"], ["py3.11.2"]),
-        ("py3-64", ["py3-32"], ["py3-32"]),
-        ("py310-magic", ["py39"], ["py39"]),
+        ("cpython", ["pypy"], "cpython", ["pypy"]),
+        ("pypy", ["cpython"], "pypy", ["cpython"]),
+        ("pypy2", ["pypy3"], "pypy2", ["pypy3"]),
+        ("py3", ["py2"], "py3", ["py2"]),
+        ("py38", ["py39"], "py38", ["py39"]),
+        ("py38", ["py38", "py39"], "py38", ["py39"]),
+        ("py38", ["python3"], "py38", ["python3"]),
+        ("py310", ["py38", "py39"], "py310", ["py38", "py39"]),
+        ("py3.11.1", ["py3.11.2"], "py3.11.1", ["py3.11.2"]),
+        ("py3-64", ["py3-32"], "py3-64", ["py3-32"]),
+        ("py310-magic", ["py39"], "py310", ["py39"]),
     ],
     ids=lambda a: "|".join(a) if isinstance(a, list) else str(a),
 )
-def test_base_python_env_conflict(env: str, base_python: list[str], conflict: list[str], ignore_conflict: bool) -> None:
+def test_base_python_env_conflict(
+    env: str,
+    base_python: list[str],
+    expected: str,
+    conflict: list[str],
+    ignore_conflict: bool,
+) -> None:
+    if env == "py3-64":
+        raise pytest.skip("bug #2657")
+
     if ignore_conflict:
         result = Python._validate_base_python(env, base_python, ignore_conflict)
-        assert result == [env]
+        assert result == [expected]
     else:
         msg = f"env name {env} conflicting with base python {conflict[0]}"
         with pytest.raises(Fail, match=msg):