From 71ed52075a33b0797fb3e31418868cc82a3dd4bc Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Wed, 28 Dec 2022 22:39:38 +0000 Subject: [PATCH 1/9] Enable multiple uses of '-f' meaning 'OR' Previously, when `-f` was passed, it overwrote the last value. The result was that `-f foo -f bar` was equivalent to only passing `-f bar`. Under the new behavior, `-f foo -f bar` combines `foo` and `bar` as selection criteria, using OR-semantics. Envs matching `foo OR bar` will be selected. The existing multi-value argument behavior for `-f` is retained, in which `-f foo bar` means `foo AND bar`. The behaviors can be combined to express a variety of environment selections which were not previously possible in a single invocation. e.g. `-f foo bar -f baz` meaning `(foo AND bar) OR baz`. No existing tests fail, and the new behavior is checked by a new test. The help message for `-f` is updated. --- src/tox/session/env_select.py | 15 +++++++++------ tests/session/test_env_select.py | 11 +++++++++++ 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/src/tox/session/env_select.py b/src/tox/session/env_select.py index c746c25b5..bcf4f0dd8 100644 --- a/src/tox/session/env_select.py +++ b/src/tox/session/env_select.py @@ -91,8 +91,8 @@ def register_env_select_flags( if multiple: help_msg = "labels to evaluate" add_to.add_argument("-m", dest="labels", metavar="label", help=help_msg, default=[], type=str, nargs="+") - help_msg = "factors to evaluate" - add_to.add_argument("-f", dest="factors", metavar="factor", help=help_msg, default=[], type=str, nargs="+") + help_msg = "factors to evaluate (passing multiple factors means 'AND', passing this option multiple times means 'OR')" + add_to.add_argument("-f", dest="factors", metavar="factor", help=help_msg, default=[], type=str, nargs="+", action="append") help_msg = "exclude all environments selected that match this regular expression" add_to.add_argument("--skip-env", dest="skip_env", metavar="re", help=help_msg, default="", type=str) return add_to @@ -290,7 +290,9 @@ def _get_package_env(self, packager: str, name: str, is_active: bool) -> Package def _mark_active(self) -> None: labels = set(getattr(self._state.conf.options, "labels", [])) - factors = set(getattr(self._state.conf.options, "factors", [])) + # factors is a list of lists, from the combination of nargs="+" and action="append" + factors = [set(factor_list) for factor_list in getattr(self._state.conf.options, "factors", [])] + assert self._defined_envs_ is not None if labels or factors: for env_info in self._defined_envs_.values(): @@ -302,10 +304,11 @@ def _mark_active(self) -> None: for env_info in self._defined_envs_.values(): if labels.intersection(env_info.env.conf["labels"]): env_info.is_active = True - if self._state.conf.options.factors: # if matches mark it active + if factors: # if matches mark it active for name, env_info in self._defined_envs_.items(): - if factors.issubset(set(name.split("-"))): - env_info.is_active = True + for factor_set in factors: + if factor_set.issubset(set(name.split("-"))): + env_info.is_active = True def __getitem__(self, item: str) -> RunToxEnv | PackageToxEnv: """ diff --git a/tests/session/test_env_select.py b/tests/session/test_env_select.py index b6683541f..d4560cb8e 100644 --- a/tests/session/test_env_select.py +++ b/tests/session/test_env_select.py @@ -72,6 +72,17 @@ def test_factor_select(tox_project: ToxProjectCreator) -> None: outcome.assert_out_err("py310-django20-cov\npy39-django20-cov\n", "") +def test_factor_select_multiple_factors(tox_project: ToxProjectCreator) -> None: + ini = """ + [tox] + env_list = py3{10,9}-{django20,django21}{-cov,} + """ + project = tox_project({"tox.ini": ini}) + outcome = project.run("l", "--no-desc", "-f", "py39", "django20", "-f", "py310", "django21") + outcome.assert_success() + outcome.assert_out_err("py310-django21-cov\npy310-django21\npy39-django20-cov\npy39-django20\n", "") + + def test_tox_skip_env(tox_project: ToxProjectCreator, monkeypatch: MonkeyPatch) -> None: monkeypatch.setenv("TOX_SKIP_ENV", "m[y]py") project = tox_project({"tox.ini": "[tox]\nenv_list = py3{10,9},mypy"}) From a20160d6aa6f5e1b59e5a2095bc1e0426a480fbe Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Wed, 28 Dec 2022 22:55:15 +0000 Subject: [PATCH 2/9] Allow factors to be passed hyphenated The existing parsing of factors allows multiple factors to be selected by passing them as multiple arguments to the `-f` flag. For example, `-f foo bar` to pass both `foo` and `bar` as factors. This can now be passed equivalently using `-f foo-bar`. The meaning of this usage is identical to `-f foo bar`. A new test checks the behavior, and very closely mirrors the existing `-f` selection test so that their outputs are exactly equivalent. --- src/tox/session/env_select.py | 18 ++++++++++++++---- tests/session/test_env_select.py | 11 +++++++++++ 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/src/tox/session/env_select.py b/src/tox/session/env_select.py index bcf4f0dd8..1aa70ca51 100644 --- a/src/tox/session/env_select.py +++ b/src/tox/session/env_select.py @@ -91,8 +91,12 @@ def register_env_select_flags( if multiple: help_msg = "labels to evaluate" add_to.add_argument("-m", dest="labels", metavar="label", help=help_msg, default=[], type=str, nargs="+") - help_msg = "factors to evaluate (passing multiple factors means 'AND', passing this option multiple times means 'OR')" - add_to.add_argument("-f", dest="factors", metavar="factor", help=help_msg, default=[], type=str, nargs="+", action="append") + help_msg = ( + "factors to evaluate (passing multiple factors means 'AND', passing this option multiple times means 'OR')" + ) + add_to.add_argument( + "-f", dest="factors", metavar="factor", help=help_msg, default=[], type=str, nargs="+", action="append" + ) help_msg = "exclude all environments selected that match this regular expression" add_to.add_argument("--skip-env", dest="skip_env", metavar="re", help=help_msg, default="", type=str) return add_to @@ -288,10 +292,16 @@ def _get_package_env(self, packager: str, name: str, is_active: bool) -> Package self._manager.tox_add_env_config(pkg_conf, self._state) return pkg_env + def _parse_factors(self) -> list[set[str]]: + # factors is a list of lists, from the combination of nargs="+" and action="append" + # also parse hyphenated factors into lists of factors + # so that `-f foo-bar` and `-f foo bar` are treated equivalently + raw_factors = getattr(self._state.conf.options, "factors", []) + return [set(f for factor in factor_list for f in factor.split("-")) for factor_list in raw_factors] + def _mark_active(self) -> None: labels = set(getattr(self._state.conf.options, "labels", [])) - # factors is a list of lists, from the combination of nargs="+" and action="append" - factors = [set(factor_list) for factor_list in getattr(self._state.conf.options, "factors", [])] + factors = self._parse_factors() assert self._defined_envs_ is not None if labels or factors: diff --git a/tests/session/test_env_select.py b/tests/session/test_env_select.py index d4560cb8e..65d1ef6e8 100644 --- a/tests/session/test_env_select.py +++ b/tests/session/test_env_select.py @@ -72,6 +72,17 @@ def test_factor_select(tox_project: ToxProjectCreator) -> None: outcome.assert_out_err("py310-django20-cov\npy39-django20-cov\n", "") +def test_factor_select_containing_hyphenate(tox_project: ToxProjectCreator) -> None: + ini = """ + [tox] + env_list = py3{10,9}-{django20,django21}{-cov,} + """ + project = tox_project({"tox.ini": ini}) + outcome = project.run("l", "--no-desc", "-f", "cov-django20") + outcome.assert_success() + outcome.assert_out_err("py310-django20-cov\npy39-django20-cov\n", "") + + def test_factor_select_multiple_factors(tox_project: ToxProjectCreator) -> None: ini = """ [tox] From 08092bfc5a5ee5b08b0c8c2a7f53d30d812bce36 Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Wed, 28 Dec 2022 23:03:16 +0000 Subject: [PATCH 3/9] Make factor tests parametrized & apply pre-commit These three tests are nearly identical in structure, and rely upon the same project configuration. Convert from three distinct test cases to a single parametrized test. Also apply pre-commit, which does some mild reformatting. --- src/tox/session/env_select.py | 11 ++++++-- tests/session/test_env_select.py | 47 +++++++++++++++----------------- 2 files changed, 31 insertions(+), 27 deletions(-) diff --git a/src/tox/session/env_select.py b/src/tox/session/env_select.py index 1aa70ca51..30a413229 100644 --- a/src/tox/session/env_select.py +++ b/src/tox/session/env_select.py @@ -95,7 +95,14 @@ def register_env_select_flags( "factors to evaluate (passing multiple factors means 'AND', passing this option multiple times means 'OR')" ) add_to.add_argument( - "-f", dest="factors", metavar="factor", help=help_msg, default=[], type=str, nargs="+", action="append" + "-f", + dest="factors", + metavar="factor", + help=help_msg, + default=[], + type=str, + nargs="+", + action="append", ) help_msg = "exclude all environments selected that match this regular expression" add_to.add_argument("--skip-env", dest="skip_env", metavar="re", help=help_msg, default="", type=str) @@ -297,7 +304,7 @@ def _parse_factors(self) -> list[set[str]]: # also parse hyphenated factors into lists of factors # so that `-f foo-bar` and `-f foo bar` are treated equivalently raw_factors = getattr(self._state.conf.options, "factors", []) - return [set(f for factor in factor_list for f in factor.split("-")) for factor_list in raw_factors] + return [{f for factor in factor_list for f in factor.split("-")} for factor_list in raw_factors] def _mark_active(self) -> None: labels = set(getattr(self._state.conf.options, "labels", [])) diff --git a/tests/session/test_env_select.py b/tests/session/test_env_select.py index 65d1ef6e8..06a1f4bea 100644 --- a/tests/session/test_env_select.py +++ b/tests/session/test_env_select.py @@ -1,5 +1,7 @@ from __future__ import annotations +import pytest + from tox.pytest import MonkeyPatch, ToxProjectCreator @@ -61,37 +63,32 @@ def test_label_core_and_trait(tox_project: ToxProjectCreator) -> None: outcome.assert_out_err("py310\npy39\nflake8\ntype\n", "") -def test_factor_select(tox_project: ToxProjectCreator) -> None: - ini = """ - [tox] - env_list = py3{10,9}-{django20,django21}{-cov,} - """ - project = tox_project({"tox.ini": ini}) - outcome = project.run("l", "--no-desc", "-f", "cov", "django20") - outcome.assert_success() - outcome.assert_out_err("py310-django20-cov\npy39-django20-cov\n", "") - - -def test_factor_select_containing_hyphenate(tox_project: ToxProjectCreator) -> None: - ini = """ - [tox] - env_list = py3{10,9}-{django20,django21}{-cov,} - """ - project = tox_project({"tox.ini": ini}) - outcome = project.run("l", "--no-desc", "-f", "cov-django20") - outcome.assert_success() - outcome.assert_out_err("py310-django20-cov\npy39-django20-cov\n", "") - - -def test_factor_select_multiple_factors(tox_project: ToxProjectCreator) -> None: +@pytest.mark.parametrize( + ("selection_arguments", "expect_envs"), + [ + ( + ["-f", "cov", "django20"], + ["py310-django20-cov", "py39-django20-cov"], + ), + ( + ["-f", "cov-django20"], + ["py310-django20-cov", "py39-django20-cov"], + ), + ( + ["-f", "py39", "django20", "-f", "py310", "django21"], + ["py310-django21-cov", "py310-django21", "py39-django20-cov", "py39-django20"], + ), + ], +) +def test_factor_select(tox_project: ToxProjectCreator, selection_arguments, expect_envs) -> None: ini = """ [tox] env_list = py3{10,9}-{django20,django21}{-cov,} """ project = tox_project({"tox.ini": ini}) - outcome = project.run("l", "--no-desc", "-f", "py39", "django20", "-f", "py310", "django21") + outcome = project.run("l", "--no-desc", *selection_arguments) outcome.assert_success() - outcome.assert_out_err("py310-django21-cov\npy310-django21\npy39-django20-cov\npy39-django20\n", "") + outcome.assert_out_err("\n".join(expect_envs) + "\n", "") def test_tox_skip_env(tox_project: ToxProjectCreator, monkeypatch: MonkeyPatch) -> None: From a8e9b1513a3e4d04ce44d4ab5d440239beafa447 Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Wed, 28 Dec 2022 23:16:02 +0000 Subject: [PATCH 4/9] Add changelog entry for #2766 --- docs/changelog/2766.feature.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 docs/changelog/2766.feature.rst diff --git a/docs/changelog/2766.feature.rst b/docs/changelog/2766.feature.rst new file mode 100644 index 000000000..700d19714 --- /dev/null +++ b/docs/changelog/2766.feature.rst @@ -0,0 +1,2 @@ +* Allow ``-f`` to be used multiple times and to be used on hyphenated factors + (e.g. ``-f py311-django -f py39``) From 826e2a923a3278dcaab4c2a476317286cbcdc1a9 Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Thu, 29 Dec 2022 01:25:02 +0000 Subject: [PATCH 5/9] Fix missing annotation in tests --- tests/session/test_env_select.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/session/test_env_select.py b/tests/session/test_env_select.py index 06a1f4bea..2a40f85d6 100644 --- a/tests/session/test_env_select.py +++ b/tests/session/test_env_select.py @@ -80,7 +80,7 @@ def test_label_core_and_trait(tox_project: ToxProjectCreator) -> None: ), ], ) -def test_factor_select(tox_project: ToxProjectCreator, selection_arguments, expect_envs) -> None: +def test_factor_select(tox_project: ToxProjectCreator, selection_arguments: list[str], expect_envs: list[str]) -> None: ini = """ [tox] env_list = py3{10,9}-{django20,django21}{-cov,} From 2be500b34f35e5ea1f633054177ffda1b9c1c1cf Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Thu, 29 Dec 2022 01:25:09 +0000 Subject: [PATCH 6/9] Fix changelog entry for #2766 --- docs/changelog/2766.feature.rst | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/changelog/2766.feature.rst b/docs/changelog/2766.feature.rst index 700d19714..3e4d67104 100644 --- a/docs/changelog/2766.feature.rst +++ b/docs/changelog/2766.feature.rst @@ -1,2 +1 @@ -* Allow ``-f`` to be used multiple times and to be used on hyphenated factors - (e.g. ``-f py311-django -f py39``) +``-f`` can be used multiple times and on hyphenated factors (e.g. ``-f py311-django -f py39``) - by :user:`sirosen`. From 6c69190c87794d87a9e559717fe619b4e676cb93 Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Thu, 29 Dec 2022 02:03:26 +0000 Subject: [PATCH 7/9] Improve env selection with factors: perf and types - use tuple instead of list for immutable data - use `continue` and `break` to skip unnecessary loop iterations --- src/tox/session/env_select.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/tox/session/env_select.py b/src/tox/session/env_select.py index 30a413229..0f17f82be 100644 --- a/src/tox/session/env_select.py +++ b/src/tox/session/env_select.py @@ -299,12 +299,12 @@ def _get_package_env(self, packager: str, name: str, is_active: bool) -> Package self._manager.tox_add_env_config(pkg_conf, self._state) return pkg_env - def _parse_factors(self) -> list[set[str]]: + def _parse_factors(self) -> tuple[set[str], ...]: # factors is a list of lists, from the combination of nargs="+" and action="append" # also parse hyphenated factors into lists of factors # so that `-f foo-bar` and `-f foo bar` are treated equivalently raw_factors = getattr(self._state.conf.options, "factors", []) - return [{f for factor in factor_list for f in factor.split("-")} for factor_list in raw_factors] + return tuple({f for factor in factor_list for f in factor.split("-")} for factor_list in raw_factors) def _mark_active(self) -> None: labels = set(getattr(self._state.conf.options, "labels", [])) @@ -323,9 +323,12 @@ def _mark_active(self) -> None: env_info.is_active = True if factors: # if matches mark it active for name, env_info in self._defined_envs_.items(): + if env_info.is_active: + continue for factor_set in factors: if factor_set.issubset(set(name.split("-"))): env_info.is_active = True + break def __getitem__(self, item: str) -> RunToxEnv | PackageToxEnv: """ From b398ccfd1b269eae9130d0e6d70adc267991319c Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Thu, 29 Dec 2022 04:48:02 +0000 Subject: [PATCH 8/9] Cleanup factor selection tests - convert args from list[str] to tuple[str, ...] - reformat str concat into a `.format()` usage --- tests/session/test_env_select.py | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/tests/session/test_env_select.py b/tests/session/test_env_select.py index 2a40f85d6..496107a37 100644 --- a/tests/session/test_env_select.py +++ b/tests/session/test_env_select.py @@ -67,20 +67,24 @@ def test_label_core_and_trait(tox_project: ToxProjectCreator) -> None: ("selection_arguments", "expect_envs"), [ ( - ["-f", "cov", "django20"], - ["py310-django20-cov", "py39-django20-cov"], + ("-f", "cov", "django20"), + ("py310-django20-cov", "py39-django20-cov"), ), ( - ["-f", "cov-django20"], - ["py310-django20-cov", "py39-django20-cov"], + ("-f", "cov-django20"), + ("py310-django20-cov", "py39-django20-cov"), ), ( - ["-f", "py39", "django20", "-f", "py310", "django21"], - ["py310-django21-cov", "py310-django21", "py39-django20-cov", "py39-django20"], + ("-f", "py39", "django20", "-f", "py310", "django21"), + ("py310-django21-cov", "py310-django21", "py39-django20-cov", "py39-django20"), ), ], ) -def test_factor_select(tox_project: ToxProjectCreator, selection_arguments: list[str], expect_envs: list[str]) -> None: +def test_factor_select( + tox_project: ToxProjectCreator, + selection_arguments: tuple[str, ...], + expect_envs: tuple[str, ...], +) -> None: ini = """ [tox] env_list = py3{10,9}-{django20,django21}{-cov,} @@ -88,7 +92,7 @@ def test_factor_select(tox_project: ToxProjectCreator, selection_arguments: list project = tox_project({"tox.ini": ini}) outcome = project.run("l", "--no-desc", *selection_arguments) outcome.assert_success() - outcome.assert_out_err("\n".join(expect_envs) + "\n", "") + outcome.assert_out_err("{}\n".format("\n".join(expect_envs)), "") def test_tox_skip_env(tox_project: ToxProjectCreator, monkeypatch: MonkeyPatch) -> None: From 9984d91510bd82ef2d83e2ca73689d262d8cac6f Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Thu, 29 Dec 2022 04:50:38 +0000 Subject: [PATCH 9/9] Remove unreachable factor selection check This check cannot be reached because it relies on an impossible combination of factors and labels. --- src/tox/session/env_select.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/tox/session/env_select.py b/src/tox/session/env_select.py index 0f17f82be..f926beda3 100644 --- a/src/tox/session/env_select.py +++ b/src/tox/session/env_select.py @@ -323,8 +323,6 @@ def _mark_active(self) -> None: env_info.is_active = True if factors: # if matches mark it active for name, env_info in self._defined_envs_.items(): - if env_info.is_active: - continue for factor_set in factors: if factor_set.issubset(set(name.split("-"))): env_info.is_active = True