From 026a9f5cf54335a5baa78d3da24552b613a4f164 Mon Sep 17 00:00:00 2001 From: Eric L Date: Sat, 10 Dec 2022 09:38:57 +0100 Subject: [PATCH 1/2] Fail on blanks in passed env vars Avoids to oversee pass_env/passenv going from blank to comma separated list Resolves https://github.com/tox-dev/tox/issues/2658 --- docs/changelog/2658.feature.rst | 1 + src/tox/tox_env/api.py | 8 +++++++- tests/tox_env/test_tox_env_api.py | 20 ++++++++++++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 docs/changelog/2658.feature.rst diff --git a/docs/changelog/2658.feature.rst b/docs/changelog/2658.feature.rst new file mode 100644 index 000000000..85a5d4787 --- /dev/null +++ b/docs/changelog/2658.feature.rst @@ -0,0 +1 @@ +Fail on ''pass_env/passenv'' entries containing blanks, detecting missing commas diff --git a/src/tox/tox_env/api.py b/src/tox/tox_env/api.py index abcdc78b4..34b04a3fd 100644 --- a/src/tox/tox_env/api.py +++ b/src/tox/tox_env/api.py @@ -21,7 +21,7 @@ from tox.execute.request import ExecuteRequest from tox.journal import EnvJournal from tox.report import OutErr, ToxHandler -from tox.tox_env.errors import Recreate, Skip +from tox.tox_env.errors import Fail, Recreate, Skip from tox.tox_env.info import Info from tox.tox_env.installer import Installer from tox.util.path import ensure_empty_dir @@ -131,6 +131,12 @@ def register_config(self) -> None: ) def pass_env_post_process(values: list[str]) -> list[str]: + blank_values = [v for v in values if " " in v or "\t" in v] + if blank_values: + raise Fail( + f"pass_env/passenv variable can't have values containing " + f"blanks like {blank_values}; a comma is possibly missing", + ) values.extend(self._default_pass_env()) return sorted({k: None for k in values}.keys()) diff --git a/tests/tox_env/test_tox_env_api.py b/tests/tox_env/test_tox_env_api.py index 2c467bb6b..32c71bdb1 100644 --- a/tests/tox_env/test_tox_env_api.py +++ b/tests/tox_env/test_tox_env_api.py @@ -80,6 +80,26 @@ def test_tox_env_pass_env_literal_miss() -> None: assert not env +def test_tox_env_pass_env_fail(tox_project: ToxProjectCreator) -> None: + prj = tox_project( + { + "tox.ini": """[testenv] + passenv = MYENV YOURENV, THEIRENV + commands=python -c 'import os; print("MYENV", os.getenv("MYENV"))'""", + }, + ) + + result = prj.run("r") + + result.assert_failed(1) + out = ( + r"py: failed with pass_env/passenv variable can't have values " + r"containing blanks like \['MYENV YOURENV'\]; " + r"a comma is possibly missing.*" + ) + result.assert_out_err(out=out, err="", regex=True) + + @pytest.mark.parametrize("glob", ["*", "?"]) @pytest.mark.parametrize("char", ["a", "A"]) def test_tox_env_pass_env_match_ignore_case(char: str, glob: str) -> None: From d8dce1f1354e748113e412f951b77ad55a6b9d93 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bern=C3=A1t=20G=C3=A1bor?= Date: Sat, 10 Dec 2022 18:59:15 -0800 Subject: [PATCH 2/2] PR Feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Bernát Gábor --- docs/changelog/2658.feature.rst | 2 +- src/tox/tox_env/api.py | 16 ++++++++++------ tests/tox_env/test_tox_env_api.py | 26 +++++++++++++------------- 3 files changed, 24 insertions(+), 20 deletions(-) diff --git a/docs/changelog/2658.feature.rst b/docs/changelog/2658.feature.rst index 85a5d4787..92dcaf752 100644 --- a/docs/changelog/2658.feature.rst +++ b/docs/changelog/2658.feature.rst @@ -1 +1 @@ -Fail on ''pass_env/passenv'' entries containing blanks, detecting missing commas +Fail on :ref:`pass_env`/:ref:`passenv` entries containing whitespace - by :user:`ericzolf`. diff --git a/src/tox/tox_env/api.py b/src/tox/tox_env/api.py index 34b04a3fd..da042f523 100644 --- a/src/tox/tox_env/api.py +++ b/src/tox/tox_env/api.py @@ -7,6 +7,7 @@ import logging import os import re +import string import sys from abc import ABC, abstractmethod from contextlib import contextmanager @@ -131,14 +132,17 @@ def register_config(self) -> None: ) def pass_env_post_process(values: list[str]) -> list[str]: - blank_values = [v for v in values if " " in v or "\t" in v] - if blank_values: + values.extend(self._default_pass_env()) + result = sorted({k: None for k in values}.keys()) + invalid_chars = set(string.whitespace) + invalid = [v for v in result if any(c in invalid_chars for c in v)] + if invalid: + invalid_repr = ", ".join(repr(i) for i in invalid) raise Fail( - f"pass_env/passenv variable can't have values containing " - f"blanks like {blank_values}; a comma is possibly missing", + f"pass_env values cannot contain whitespace, use comma to have multiple values in a single line, " + f"invalid values found {invalid_repr}", ) - values.extend(self._default_pass_env()) - return sorted({k: None for k in values}.keys()) + return result self.conf.add_config( keys=["pass_env", "passenv"], diff --git a/tests/tox_env/test_tox_env_api.py b/tests/tox_env/test_tox_env_api.py index 32c71bdb1..700974430 100644 --- a/tests/tox_env/test_tox_env_api.py +++ b/tests/tox_env/test_tox_env_api.py @@ -80,24 +80,24 @@ def test_tox_env_pass_env_literal_miss() -> None: assert not env -def test_tox_env_pass_env_fail(tox_project: ToxProjectCreator) -> None: - prj = tox_project( - { - "tox.ini": """[testenv] - passenv = MYENV YOURENV, THEIRENV - commands=python -c 'import os; print("MYENV", os.getenv("MYENV"))'""", - }, +def test_tox_env_pass_env_fails_on_whitespace(tox_project: ToxProjectCreator) -> None: + first, second = "A B", "C D" + prj = tox_project({"tox.ini": f"[testenv]\npackage=skip\npass_env = {first}\n {second}\n E"}) + result = prj.run("c", "-k", "pass_env") + result.assert_success() + msg = ( + '[testenv:py]\npass_env = # Exception: Fail("pass_env values cannot contain whitespace, use comma to have ' + f'multiple values in a single line, invalid values found {first!r}, {second!r}")\n\n[tox]\n' ) + assert result.out == msg result = prj.run("r") - result.assert_failed(1) - out = ( - r"py: failed with pass_env/passenv variable can't have values " - r"containing blanks like \['MYENV YOURENV'\]; " - r"a comma is possibly missing.*" + msg = ( + "py: failed with pass_env values cannot contain whitespace, use comma to have multiple values in a single line," + " invalid values found 'A B', 'C D'" ) - result.assert_out_err(out=out, err="", regex=True) + assert msg in result.out @pytest.mark.parametrize("glob", ["*", "?"])