Skip to content

Commit

Permalink
PR Feedback
Browse files Browse the repository at this point in the history
Signed-off-by: Bernát Gábor <[email protected]>
  • Loading branch information
gaborbernat committed Aug 21, 2023
1 parent bd4fa95 commit 3ef1054
Show file tree
Hide file tree
Showing 4 changed files with 104 additions and 35 deletions.
1 change: 0 additions & 1 deletion docs/changelog/3099.bugfix.rst

This file was deleted.

3 changes: 3 additions & 0 deletions docs/changelog/3099.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Change accepted environment name rule: must be made up of factors defined in configuration or match regex
``(pypy|py|cython|)((\d(\.\d+(\.\d+)?)?)|\d+)?``. If an environment name does not match this fail, and if a close match
found suggest that to the user.
66 changes: 44 additions & 22 deletions src/tox/session/env_select.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@
import re
from collections import Counter
from dataclasses import dataclass
from itertools import chain, permutations
from difflib import get_close_matches
from itertools import chain
from typing import TYPE_CHECKING, Dict, Iterable, Iterator, List, cast

from tox.config.loader.str_convert import StrConvert
Expand Down Expand Up @@ -117,6 +118,10 @@ class _ToxEnvInfo:
package_skip: tuple[str, Skip] | None = None #: if set the creation of the packaging environment failed


_DYNAMIC_ENV_FACTORS = re.compile(r"(pypy|py|cython|)((\d(\.\d+(\.\d+)?)?)|\d+)?")
_PY_PRE_RELEASE_FACTOR = re.compile(r"alpha|beta|rc\.\d+")


class EnvSelector:
def __init__(self, state: State) -> None:
# needs core to load the default tox environment list
Expand Down Expand Up @@ -152,23 +157,50 @@ def _collect_names(self) -> Iterator[tuple[Iterable[str], bool]]:
elif self._cli_envs.is_all:
everything_active = True
else:
cli_envs_not_in_config = set(self._cli_envs) - set(self._state.conf)
if cli_envs_not_in_config:
# allow cli_envs matching ".pkg" and starting with "py" to be implicitly created.
disallowed_cli_envs = [env for env in cli_envs_not_in_config if not _is_valid_exception(env)]

# allow disallowed cli envs that match hyphenated combinations
has_match = any(_find_env_match(cli_env, set(self._state.conf)) for cli_env in disallowed_cli_envs)

if disallowed_cli_envs and not has_match:
msg = f"provided environments not found in configuration file: {disallowed_cli_envs}"
raise HandledError(msg)
self._ensure_envs_valid()
yield self._cli_envs, True
yield self._state.conf, everything_active
label_envs = dict.fromkeys(chain.from_iterable(self._state.conf.core["labels"].values()))
if label_envs:
yield label_envs.keys(), False

def _ensure_envs_valid(self) -> None:
valid_factors = set(chain.from_iterable(env.split("-") for env in self._state.conf))
valid_factors.add(".pkg") # packaging factor
invalid_envs: dict[str, str | None] = {}
for env in self._cli_envs or []:
if env.startswith(".pkg_external"): # external package
continue
factors: dict[str, str | None] = {k: None for k in env.split("-")}
found_factors: set[str] = set()
for factor in factors:
if (
_DYNAMIC_ENV_FACTORS.fullmatch(factor)
or _PY_PRE_RELEASE_FACTOR.fullmatch(factor)
or factor in valid_factors
):
found_factors.add(factor)
else:
closest = get_close_matches(factor, valid_factors, n=1)
factors[factor] = closest[0] if closest else None
if set(factors) - found_factors:
invalid_envs[env] = (
None
if any(i is None for i in factors.values())
else "-".join(cast(Iterable[str], factors.values()))
)
if invalid_envs:
msg = "provided environments not found in configuration file:\n"
first = True
for env, suggestion in invalid_envs.items():
if not first:
msg += "\n"
first = False
msg += env
if suggestion:
msg += f" - did you mean {suggestion}?"
raise HandledError(msg)

def _env_name_to_active(self) -> dict[str, bool]:
env_name_to_active_map = {}
for a_collection, is_active in self._collect_names():
Expand Down Expand Up @@ -391,16 +423,6 @@ def _mark_provision(self, on: bool, provision_tox_env: str) -> None: # noqa: FB
self._provision = on, provision_tox_env


def _find_env_match(value: str, state_conf: set[str]) -> bool:
return any(value in conf.split("-") for conf in state_conf) or any(
value == "-".join(combo) for combo in permutations(state_conf, 2)
)


def _is_valid_exception(env: str) -> bool:
return env.startswith("py") or env in (".pkg",)


__all__ = [
"register_env_select_flags",
"EnvSelector",
Expand Down
69 changes: 57 additions & 12 deletions tests/session/test_env_select.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
from __future__ import annotations

import sys
from typing import TYPE_CHECKING

import pytest

from tox.config.cli.parse import get_options
from tox.session.env_select import CliEnv, EnvSelector
from tox.session.env_select import _DYNAMIC_ENV_FACTORS, CliEnv, EnvSelector
from tox.session.state import State

if TYPE_CHECKING:
Expand Down Expand Up @@ -150,13 +151,6 @@ def test_cli_env_can_be_specified_in_additional_environments(tox_project: ToxPro
assert not outcome.err


def test_cli_env_not_in_tox_config_fails(tox_project: ToxProjectCreator) -> None:
proj = tox_project({"tox.ini": ""})
outcome = proj.run("r", "-e", "does_not_exist")
outcome.assert_failed(code=-2)
assert "provided environments not found in configuration file: ['does_not_exist']" in outcome.out, outcome.out


@pytest.mark.parametrize("env_name", ["py", "py310", ".pkg"])
def test_allowed_implicit_cli_envs(env_name: str, tox_project: ToxProjectCreator) -> None:
proj = tox_project({"tox.ini": ""})
Expand All @@ -166,7 +160,7 @@ def test_allowed_implicit_cli_envs(env_name: str, tox_project: ToxProjectCreator
assert not outcome.err


@pytest.mark.parametrize("env_name", ["a", "b", "a-b"])
@pytest.mark.parametrize("env_name", ["a", "b", "a-b", "b-a"])
def test_matches_hyphenated_env(env_name: str, tox_project: ToxProjectCreator) -> None:
tox_ini = """
[tox]
Expand All @@ -185,11 +179,15 @@ def test_matches_hyphenated_env(env_name: str, tox_project: ToxProjectCreator) -
assert not outcome.err


@pytest.mark.parametrize("env_name", ["3.9", "3.9-cov"])
_MINOR = sys.version_info.minor


@pytest.mark.parametrize(
"env_name",
[f"3.{_MINOR}", f"3.{_MINOR}-cov", "3-cov", "3", f"3.{_MINOR}", f"py3{_MINOR}-cov", f"py3.{_MINOR}-cov"],
)
def test_matches_combined_env(env_name: str, tox_project: ToxProjectCreator) -> None:
tox_ini = """
[tox]
env_list=3.9
[testenv]
package=skip
commands =
Expand All @@ -201,3 +199,50 @@ def test_matches_combined_env(env_name: str, tox_project: ToxProjectCreator) ->
outcome.assert_success()
assert env_name in outcome.out
assert not outcome.err


@pytest.mark.parametrize(
"env",
[
"py",
"pypy",
"pypy3",
"pypy3.12",
"pypy312",
"py3",
"py3.12",
"py312",
"3",
"3.12",
"3.12.0",
],
)
def test_dynamic_env_factors_match(env: str) -> None:
assert _DYNAMIC_ENV_FACTORS.fullmatch(env)


@pytest.mark.parametrize(
"env",
[
"cy3",
"cov",
"py10.1",
],
)
def test_dynamic_env_factors_not_match(env: str) -> None:
assert not _DYNAMIC_ENV_FACTORS.fullmatch(env)


def test_suggest_env(tox_project: ToxProjectCreator) -> None:
tox_ini = f"[testenv:release]\n[testenv:py3{_MINOR}]\n[testenv:alpha-py3{_MINOR}]\n"
proj = tox_project({"tox.ini": tox_ini})
outcome = proj.run("r", "-e", f"releas,p3{_MINOR},magic,alph-p{_MINOR}")
outcome.assert_failed(code=-2)

assert not outcome.err
msg = (
"ROOT: HandledError| provided environments not found in configuration file:\n"
f"releas - did you mean release?\np3{_MINOR} - did you mean py{_MINOR}?\nmagic\n"
f"alph-p{_MINOR} - did you mean alpha-py3{_MINOR}?\n"
)
assert outcome.out == msg

0 comments on commit 3ef1054

Please sign in to comment.