-
-
Notifications
You must be signed in to change notification settings - Fork 504
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
Changes from all commits
07a77e4
e01b8e3
8457a43
ac5dcb3
5505c82
3039214
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 |
---|---|---|
@@ -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`. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
Fix an attribute error when ``use_develop = true`` is set and an unsupported interpreter version is requested - by | ||
:user:`stephenfin`. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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`. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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: | ||
|
@@ -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: | ||
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. 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. |
||
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}") | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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) | ||
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. 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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -142,6 +142,7 @@ replacer | |
repo | ||
reqs | ||
retann | ||
retcode | ||
rfind | ||
rpartition | ||
rreq | ||
|
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.
Note for reviewers: This is a new source of errors but we handle it below...