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

Disallow command line environments which are not explicitly specified in the config file #3089

Merged
merged 5 commits into from
Aug 14, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/changelog/2858.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Disallow command line environments which are not explicitly specified in the config file - by :user:`tjsmart`.
16 changes: 16 additions & 0 deletions docs/user_guide.rst
Original file line number Diff line number Diff line change
Expand Up @@ -541,3 +541,19 @@ create your virtual env for the developers.
py310-lint -> [no description]
py311-black -> [no description]
py311-lint -> [no description]

Disallow command line environments which are not explicitly specified in the config file
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Previously, any environment would be implicitly created even if no such environment was specified in the configuration
file.For example, given this config:

.. code-block:: ini

[testenv:unit]
deps = pytest
commands = pytest

Running ``tox -e unit`` would run our tests but running ``tox -e unt`` or ``tox -e unti`` would ultimately succeed
without running any tests. A special exception is made for environments starting in ``py*``. In the above example
running ``tox -e py310`` would still function as intended.
9 changes: 9 additions & 0 deletions src/tox/session/env_select.py
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,15 @@ 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 env.startswith("py") and env not in (".pkg",)
]
if disallowed_cli_envs:
msg = f"provided environments not found in configuration file: {disallowed_cli_envs}"
raise HandledError(msg)
yield self._cli_envs, True
yield self._state.conf, everything_active
label_envs = dict.fromkeys(chain.from_iterable(self._state.conf.core["labels"].values()))
Expand Down
11 changes: 10 additions & 1 deletion tests/plugin/test_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,16 @@ def tox_env_teardown(tox_env: ToxEnv) -> None:
plugins = tuple(v for v in locals().values() if callable(v) and hasattr(v, "tox_impl"))
assert len(plugins) == 8
register_inline_plugin(mocker, *plugins)
project = tox_project({"tox.ini": "[testenv]\npackage=skip\ncommands=python -c 'print(1)'"})

tox_ini = """
[tox]
env_list=a,b
[testenv]
package=skip
commands=python -c 'print(1)'
env_list=a,b
"""
project = tox_project({"tox.ini": tox_ini})
result = project.run("r", "-e", "a,b")
result.assert_success()
cmd = "print(1)" if sys.platform == "win32" else "'print(1)'"
Expand Down
2 changes: 1 addition & 1 deletion tests/session/cmd/test_devenv.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@


def test_devenv_fail_multiple_target(tox_project: ToxProjectCreator) -> None:
outcome = tox_project({"tox.ini": ""}).run("d", "-e", "a,b")
outcome = tox_project({"tox.ini": "[tox]\nenv_list=a,b"}).run("d", "-e", "a,b")
outcome.assert_failed()
msg = "ROOT: HandledError| exactly one target environment allowed in devenv mode but found a, b\n"
outcome.assert_out_err(msg, "")
Expand Down
2 changes: 1 addition & 1 deletion tests/session/cmd/test_parallel.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ def test_keyboard_interrupt(tox_project: ToxProjectCreator, demo_pkg_inline: Pat
)
cmd = ["-c", str(proj.path / "tox.ini"), "p", "-p", "1", "-e", f"py,py{sys.version_info[0]},dep"]
process = Popen([sys.executable, "-m", "tox", *cmd], stdout=PIPE, stderr=PIPE, universal_newlines=True)
while not marker.exists():
while not marker.exists() and (process.poll() is None):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

While in a bit of broken state, this test was hanging for me. Suggesting that we use Popen.poll to check if the child process has already terminated (likely with an error if marker does not exist).

sleep(0.05)
process.send_signal(SIGINT)
out, err = process.communicate()
Expand Down
4 changes: 2 additions & 2 deletions tests/session/cmd/test_sequential.py
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ def test_platform_matches_run_env(tox_project: ToxProjectCreator) -> None:
def test_platform_does_not_match_package_env(tox_project: ToxProjectCreator, demo_pkg_inline: Path) -> None:
toml = (demo_pkg_inline / "pyproject.toml").read_text()
build = (demo_pkg_inline / "build.py").read_text()
ini = "[testenv]\npackage=wheel\n[testenv:.pkg]\nplatform=wrong_platform"
ini = "[tox]\nenv_list=a,b\n[testenv]\npackage=wheel\n[testenv:.pkg]\nplatform=wrong_platform"
proj = tox_project({"tox.ini": ini, "pyproject.toml": toml, "build.py": build})
result = proj.run("r", "-e", "a,b")
result.assert_failed() # tox run fails as all envs are skipped
Expand Down Expand Up @@ -430,7 +430,7 @@ def test_sequential_help(tox_project: ToxProjectCreator) -> None:


def test_sequential_clears_pkg_at_most_once(tox_project: ToxProjectCreator, demo_pkg_inline: Path) -> None:
project = tox_project({"tox.ini": ""})
project = tox_project({"tox.ini": "[tox]\nenv_list=a,b"})
result = project.run("r", "--root", str(demo_pkg_inline), "-e", "a,b", "-r")
result.assert_success()

Expand Down
32 changes: 32 additions & 0 deletions tests/session/test_env_select.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,3 +132,35 @@ def test_env_select_lazily_looks_at_envs() -> None:
# late-assigning env should be reflected in env_selector
state.conf.options.env = CliEnv("py")
assert set(env_selector.iter()) == {"py"}


def test_cli_env_can_be_specified_in_default(tox_project: ToxProjectCreator) -> None:
proj = tox_project({"tox.ini": "[tox]\nenv_list=exists"})
outcome = proj.run("r", "-e", "exists")
outcome.assert_success()
assert "exists" in outcome.out
assert not outcome.err


def test_cli_env_can_be_specified_in_additional_environments(tox_project: ToxProjectCreator) -> None:
proj = tox_project({"tox.ini": "[testenv:exists]"})
outcome = proj.run("r", "-e", "exists")
outcome.assert_success()
assert "exists" in outcome.out
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"])
Copy link
Contributor

Choose a reason for hiding this comment

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

test_allowed_implicit_cli_envs[py310] now fails when Python 3.10 is not installed. Is that expected, or should I open an issue?

Copy link
Member

Choose a reason for hiding this comment

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

PR welcome, likely should be pyx.y where x.y is the current Python interpreter.

Copy link
Contributor

Choose a reason for hiding this comment

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

def test_allowed_implicit_cli_envs(env_name: str, tox_project: ToxProjectCreator) -> None:
proj = tox_project({"tox.ini": ""})
outcome = proj.run("r", "-e", env_name)
outcome.assert_success()
assert env_name in outcome.out
assert not outcome.err
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ def test_pyproject_deps_static_with_dynamic( # noqa: PLR0913


def test_pyproject_no_build_editable_fallback(tox_project: ToxProjectCreator, demo_pkg_inline: Path) -> None:
proj = tox_project({"tox.ini": ""}, base=demo_pkg_inline)
proj = tox_project({"tox.ini": "[tox]\nenv_list=a,b"}, base=demo_pkg_inline)
execute_calls = proj.patch_execute(lambda r: 0 if "install" in r.run_id else None)
result = proj.run("r", "-e", "a,b", "--notest", "--develop")
result.assert_success()
Expand Down