Skip to content
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

Only return Python factor on base_python conflict #2840

Merged
merged 1 commit into from
Jan 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions docs/changelog/2838.bugfix.rst
Original file line number Diff line number Diff line change
@@ -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`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"the correct Python version" - that would be which one? The one from base_python or from the factor? And while I could read the source code, I'd prefer to have it here, too.

As you are very involved in that topic... is this properly documented? Or do you think it would make sense to update the documentation, too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be the version derived from the factor. If I run tox -e py310, I expect to get python3.10.

I did have this documented as part of #841 but that seems to have been lost in the rewrite. I can do another PR to (re-)flesh out the documentation. I also need to re-add docs explaining how factors work, which I'd done previously in #1573.

25 changes: 14 additions & 11 deletions src/tox/tox_env/python/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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(
Expand All @@ -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

Expand Down
37 changes: 23 additions & 14 deletions tests/tox_env/python/test_python_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down