From 642c8bb9a7cae5f6855d08da066e3614e788b4ae Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Thu, 16 Jan 2025 10:04:17 -0500 Subject: [PATCH] fix: outer_env issues fixed (#874) * fix: outer_env issues fixed Signed-off-by: Henry Schreiner * fix: use os.environ more directly, handle bin paths Signed-off-by: Henry Schreiner --------- Signed-off-by: Henry Schreiner --- nox/sessions.py | 11 ++++++++--- nox/virtualenv.py | 15 ++------------- tests/test_sessions.py | 36 ++++++++++++++++++++++++++++++------ tests/test_virtualenv.py | 15 ++++++++++----- 4 files changed, 50 insertions(+), 27 deletions(-) diff --git a/nox/sessions.py b/nox/sessions.py index b38fce34..07a78811 100644 --- a/nox/sessions.py +++ b/nox/sessions.py @@ -193,7 +193,7 @@ def name(self) -> str: return self._runner.friendly_name @property - def env(self) -> dict[str, str]: + def env(self) -> dict[str, str | None]: """A dictionary of environment variables to pass into all commands.""" return self.virtualenv.env @@ -618,9 +618,14 @@ def _run( args = (nox.virtualenv.UV, *args[1:]) # Combine the env argument with our virtualenv's env vars. + env = env or {} + env = {**self.env, **env} if include_outer_env: - overlay_env = env or {} - env = {**self.env, **overlay_env} + env = {**os.environ, **env} + if self.virtualenv.bin_paths: + env["PATH"] = os.pathsep.join( + [*self.virtualenv.bin_paths, env.get("PATH") or ""] + ) # If --error-on-external-run is specified, error on external programs. if self._runner.global_config.error_on_external_run and external is None: diff --git a/nox/virtualenv.py b/nox/virtualenv.py index 3048db3f..2d32628e 100644 --- a/nox/virtualenv.py +++ b/nox/virtualenv.py @@ -161,19 +161,8 @@ def __init__( self._bin_paths = None if bin_paths is None else list(bin_paths) self._reused = False - # Filter envs now so `.env` is dict[str, str] (easier to use) - # even though .command's env supports None. - env = env or {} - env = {**os.environ, **env} - self.env = {k: v for k, v in env.items() if v is not None} - - for key in _BLACKLISTED_ENV_VARS: - self.env.pop(key, None) - - if self.bin_paths: - self.env["PATH"] = os.pathsep.join( - [*self.bin_paths, self.env.get("PATH", "")] - ) + # .command's env supports None, meaning don't include value even if in parent + self.env = {**{k: None for k in _BLACKLISTED_ENV_VARS}, **(env or {})} @property def bin_paths(self) -> list[str] | None: diff --git a/tests/test_sessions.py b/tests/test_sessions.py index f96e404f..b8755271 100644 --- a/tests/test_sessions.py +++ b/tests/test_sessions.py @@ -300,9 +300,12 @@ def test_run_install_only_should_install(self) -> None: session.install("spam") session.run("spam", "eggs") + env = dict(os.environ) + env["PATH"] = os.pathsep.join(["/no/bin/for/you", env["PATH"]]) + run.assert_called_once_with( ("python", "-m", "pip", "install", "spam"), - **run_with_defaults(paths=mock.ANY, silent=True, env={}, external="error"), + **run_with_defaults(paths=mock.ANY, silent=True, env=env, external="error"), ) def test_run_success(self) -> None: @@ -342,10 +345,12 @@ def test_run_overly_env(self) -> None: assert result assert result.strip() == "1 3 5" - def test_by_default_all_invocation_env_vars_are_passed(self) -> None: + def test_by_default_all_invocation_env_vars_are_passed( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setenv("I_SHOULD_BE_INCLUDED", "happy") session, runner = self.make_session_and_runner() assert runner.venv - runner.venv.env["I_SHOULD_BE_INCLUDED"] = "happy" runner.venv.env["I_SHOULD_BE_INCLUDED_TOO"] = "happier" runner.venv.env["EVERYONE_SHOULD_BE_INCLUDED_TOO"] = "happiest" result = session.run( @@ -359,11 +364,13 @@ def test_by_default_all_invocation_env_vars_are_passed(self) -> None: assert "happier" in result assert "happiest" in result - def test_no_included_invocation_env_vars_are_passed(self) -> None: + def test_no_included_invocation_env_vars_are_passed( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setenv("I_SHOULD_NOT_BE_INCLUDED", "sad") + monkeypatch.setenv("AND_NEITHER_SHOULD_I", "unhappy") session, runner = self.make_session_and_runner() assert runner.venv - runner.venv.env["I_SHOULD_NOT_BE_INCLUDED"] = "sad" - runner.venv.env["AND_NEITHER_SHOULD_I"] = "unhappy" result = session.run( sys.executable, "-c", @@ -377,6 +384,23 @@ def test_no_included_invocation_env_vars_are_passed(self) -> None: assert "unhappy" not in result assert "happy" in result + def test_no_included_invocation_env_vars_are_passed_empty( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setenv("I_SHOULD_NOT_BE_INCLUDED", "sad") + monkeypatch.setenv("AND_NEITHER_SHOULD_I", "unhappy") + session, runner = self.make_session_and_runner() + result = session.run( + sys.executable, + "-c", + "import os; print(os.environ)", + include_outer_env=False, + silent=True, + ) + assert result + assert "sad" not in result + assert "unhappy" not in result + def test_run_external_not_a_virtualenv(self) -> None: # Non-virtualenv sessions should always allow external programs. session, runner = self.make_session_and_runner() diff --git a/tests/test_virtualenv.py b/tests/test_virtualenv.py index e364ea8d..9f1a6e78 100644 --- a/tests/test_virtualenv.py +++ b/tests/test_virtualenv.py @@ -275,9 +275,11 @@ def test_condaenv_detection(make_conda: Callable[..., tuple[CondaEnv, Path]]) -> conda = shutil.which("conda") assert conda + env = {k: v for k, v in {**os.environ, **venv.env}.items() if v is not None} + proc_result = subprocess.run( [conda, "list"], - env=venv.env, + env=env, check=True, capture_output=True, ) @@ -336,9 +338,8 @@ def test_env( ) -> None: monkeypatch.setenv("SIGIL", "123") venv, _ = make_one() - assert venv.env["SIGIL"] == "123" assert len(venv.bin_paths) == 1 - assert venv.bin_paths[0] in venv.env["PATH"] + assert venv.bin_paths[0] == venv.bin assert venv.bin_paths[0] not in os.environ["PATH"] @@ -439,17 +440,19 @@ def test_create( venv, dir_ = make_one() venv.create() - assert "CONDA_PREFIX" not in venv.env - assert "NOT_CONDA_PREFIX" in venv.env + assert venv.env["CONDA_PREFIX"] is None + assert "NOT_CONDA_PREFIX" not in venv.env if IS_WINDOWS: assert dir_.joinpath("Scripts", "python.exe").exists() assert dir_.joinpath("Scripts", "pip.exe").exists() assert dir_.joinpath("Lib").exists() + assert str(dir_.joinpath("Scripts")) in venv.bin_paths else: assert dir_.joinpath("bin", "python").exists() assert dir_.joinpath("bin", "pip").exists() assert dir_.joinpath("lib").exists() + assert str(dir_.joinpath("bin")) in venv.bin_paths # Test running create on an existing environment. It should be deleted. dir_.joinpath("test.txt").touch() @@ -561,6 +564,8 @@ def test_reuse_conda_environment( ) -> None: venv, _ = make_one(reuse_existing=True, venv_backend="conda") venv.create() + assert venv.bin_paths + assert venv.bin_paths[-1].endswith("bin") venv, _ = make_one(reuse_existing=True, venv_backend="conda") reused = not venv.create()