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

Fix various issues with missing interpreters #2828

Merged
merged 6 commits into from Jan 6, 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/2811.bugfix.rst
@@ -0,0 +1,2 @@
The combination of ``usedevelop = true`` and ``--skip-missing-interpreters=false`` will no longer fail for environments
that were *not* invoked - by :user:`stephenfin`.
2 changes: 2 additions & 0 deletions docs/changelog/2826.bugfix.rst
@@ -0,0 +1,2 @@
Fix an attribute error when ``use_develop = true`` is set and an unsupported interpreter version is requested - by
:user:`stephenfin`.
2 changes: 2 additions & 0 deletions docs/changelog/2827.bugfix.rst
@@ -0,0 +1,2 @@
tox returns a non-zero error code if all envs are skipped. It will now correctly do this if only a single env was
requested and this was skipped - by :user:`stephenfin`.
13 changes: 12 additions & 1 deletion docs/upgrading.rst
Expand Up @@ -88,6 +88,18 @@ moving to the newly added ``py_impl`` and ``py_dot_ver`` variables, for example:

deps = -r{py_impl}{py_dot_ver}-req.txt

Failure when all environments are skipped
-----------------------------------------

A run that results in all environments being skipped will no longer result in success. Instead, a failure will be
reported. For example, consider a host that does not support Python 3.5:

.. code-block:: bash

tox run --skip-missing-interpreters=true -e py35

This will now result in a failure.

Substitutions removed
---------------------

Expand Down Expand Up @@ -134,7 +146,6 @@ Re-use of environments
[testenv:b]
deps = pytest<7


CLI command compatibility
-------------------------

Expand Down
4 changes: 3 additions & 1 deletion src/tox/session/cmd/run/common.py
Expand Up @@ -187,7 +187,9 @@ def _print(color_: int, message: str) -> None:
_print(Fore.GREEN, f" congratulations :) ({duration:.2f} seconds)")
return Outcome.OK
_print(Fore.RED, f" evaluation failed :( ({duration:.2f} seconds)")
return runs[0].code if len(runs) == 1 else -1
if len(runs) == 1:
return runs[0].code if not runs[0].skipped else -1
return -1


def _get_outcome_message(run: ToxEnvRunResult) -> tuple[str, int]:
Expand Down
24 changes: 13 additions & 11 deletions src/tox/tox_env/python/api.py
Expand Up @@ -231,27 +231,29 @@ def python_cache(self) -> dict[str, Any]:
@property
def base_python(self) -> PythonInfo:
"""Resolve base python"""
base_pythons: list[str] = self.conf["base_python"]

if self._base_python_searched is False:
base_pythons: list[str] = self.conf["base_python"]
self._base_python_searched = True
self._base_python = self._get_python(base_pythons)
if self._base_python is None:
if self.core["skip_missing_interpreters"]:
raise Skip(f"could not find python interpreter with spec(s): {', '.join(base_pythons)}")
raise NoInterpreter(base_pythons)
if self.journal:
if self._base_python is not None and self.journal:
value = self._get_env_journal_python()
self.journal["python"] = value

if self._base_python is None:
if self.core["skip_missing_interpreters"]:
raise Skip(f"could not find python interpreter with spec(s): {', '.join(base_pythons)}")
raise NoInterpreter(base_pythons)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note for reviewers: This is a new source of errors but we handle it below...


return cast(PythonInfo, self._base_python)

def _get_env_journal_python(self) -> dict[str, Any]:
assert self._base_python is not None
return {
"implementation": self._base_python.implementation,
"implementation": self.base_python.implementation,
"version_info": tuple(self.base_python.version_info),
"version": self._base_python.version,
"is_64": self._base_python.is_64,
"sysplatform": self._base_python.platform,
"version": self.base_python.version,
"is_64": self.base_python.is_64,
"sysplatform": self.base_python.platform,
"extra_version_info": None,
}

Expand Down
8 changes: 6 additions & 2 deletions src/tox/tox_env/python/package.py
Expand Up @@ -14,7 +14,7 @@
from ..errors import Skip
from ..package import Package, PackageToxEnv, PathPackage
from ..runner import RunToxEnv
from .api import Python
from .api import NoInterpreter, Python
from .pip.req_file import PythonDeps

if TYPE_CHECKING:
Expand Down Expand Up @@ -87,7 +87,11 @@ def default_wheel_tag(conf: Config, env_name: str | None) -> str: # noqa: U100
# python only code are often compatible at major level (unless universal wheel in which case both 2/3)
# c-extension codes are trickier, but as of today both poetry/setuptools uses pypa/wheels logic
# https://github.com/pypa/wheel/blob/master/src/wheel/bdist_wheel.py#L234-L280
run_py = cast(Python, run_env).base_python
try:
run_py = cast(Python, run_env).base_python
except NoInterpreter:
Copy link
Contributor Author

@stephenfin stephenfin Jan 6, 2023

Choose a reason for hiding this comment

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

Note for reviewers: This is where we handle those new errors I mentioned above. We also handle "old" errors: you could previously get here by running e.g. tox -e py33 on an environment that didn't provide python3.3.

run_py = None

if run_py is None:
base = ",".join(run_env.conf["base_python"])
raise Skip(f"could not resolve base python with {base}")
Expand Down
6 changes: 3 additions & 3 deletions tests/session/cmd/test_depends.py
Expand Up @@ -34,18 +34,18 @@ def test_depends(tox_project: ToxProjectCreator, patch_prev_py: Callable[[bool],
py ~ .pkg
{py} ~ .pkg
{prev_py} ~ .pkg | .pkg-{impl}{prev_ver}
py31 ~ .pkg | ... (could not resolve base python with py31)
py31 ~ .pkg | ... (could not find python interpreter with spec(s): py31)
cov2
cov
py ~ .pkg
{py} ~ .pkg
{prev_py} ~ .pkg | .pkg-{impl}{prev_ver}
py31 ~ .pkg | ... (could not resolve base python with py31)
py31 ~ .pkg | ... (could not find python interpreter with spec(s): py31)
cov
py ~ .pkg
{py} ~ .pkg
{prev_py} ~ .pkg | .pkg-{impl}{prev_ver}
py31 ~ .pkg | ... (could not resolve base python with py31)
py31 ~ .pkg | ... (could not find python interpreter with spec(s): py31)
"""
assert outcome.out == dedent(expected).lstrip()

Expand Down
4 changes: 2 additions & 2 deletions tests/session/cmd/test_sequential.py
Expand Up @@ -224,7 +224,7 @@ def test_missing_interpreter_skip_on(tox_project: ToxProjectCreator) -> None:
proj = tox_project({"tox.ini": ini})

result = proj.run("r")
result.assert_success()
result.assert_failed()
assert "py: SKIP" in result.out


Expand Down Expand Up @@ -367,7 +367,7 @@ def test_platform_does_not_match_run_env(tox_project: ToxProjectCreator) -> None
proj = tox_project({"tox.ini": ini})

result = proj.run("r")
result.assert_success()
result.assert_failed()
exp = f"py: skipped because platform {sys.platform} does not match wrong_platform"
assert exp in result.out

Expand Down
37 changes: 34 additions & 3 deletions tests/tox_env/python/test_python_runner.py
Expand Up @@ -128,8 +128,39 @@ def test_extras_are_normalized(
("config", "cli", "expected"),
[("false", "true", True), ("true", "false", False), ("false", "config", False), ("true", "config", True)],
)
def test_config_skip_missing_interpreters(tox_project: ToxProjectCreator, config: str, cli: str, expected: str) -> None:
def test_config_skip_missing_interpreters(
tox_project: ToxProjectCreator,
config: str,
cli: str,
expected: bool,
) -> None:
py_ver = ".".join(str(i) for i in sys.version_info[0:2])
project = tox_project({"tox.ini": f"[tox]\nenvlist=py4,py{py_ver}\nskip_missing_interpreters={config}"})
result = project.run("--skip-missing-interpreters", cli)
assert result.code == 0 if expected else 1
result = project.run(f"--skip-missing-interpreters={cli}")
assert result.code == (0 if expected else -1)
Copy link
Contributor Author

@stephenfin stephenfin Jan 6, 2023

Choose a reason for hiding this comment

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

Note for reviewers: I've called this out in the commit message but this was a bug I only spotted when I copied the logic and found it wasn't working as expected. I suspect there are probably other places that we do this around here but I only fixed this one to keep the PR sane.



@pytest.mark.parametrize(
("skip", "env", "retcode"),
[
("true", f"py{''.join(str(i) for i in sys.version_info[0:2])}", 0),
("false", f"py{''.join(str(i) for i in sys.version_info[0:2])}", 0),
("true", "py31", -1),
("false", "py31", 1),
("true", None, 0),
("false", None, -1),
],
)
def test_skip_missing_interpreters_specified_env(
tox_project: ToxProjectCreator,
skip: str,
env: str | None,
retcode: int,
) -> None:
py_ver = "".join(str(i) for i in sys.version_info[0:2])
project = tox_project({"tox.ini": f"[tox]\nenvlist=py31,py{py_ver}\n[testenv]\nusedevelop=true"})
args = [f"--skip-missing-interpreters={skip}"]
if env:
args += ["-e", env]
result = project.run(*args)
assert result.code == retcode
1 change: 1 addition & 0 deletions whitelist.txt
Expand Up @@ -142,6 +142,7 @@ replacer
repo
reqs
retann
retcode
rfind
rpartition
rreq
Expand Down