From 356ff7e1d45218a2d49a46cd32d1ce5e012720fe Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Mon, 28 Oct 2024 00:17:30 -0400 Subject: [PATCH] fix: use os.environ more directly, handle bin paths Signed-off-by: Henry Schreiner --- nox/sessions.py | 8 ++++++-- nox/virtualenv.py | 16 ++-------------- tests/test_sessions.py | 33 +++++++++++++-------------------- tests/test_virtualenv.py | 15 ++++++++++----- 4 files changed, 31 insertions(+), 41 deletions(-) diff --git a/nox/sessions.py b/nox/sessions.py index 388d51b0..60309598 100644 --- a/nox/sessions.py +++ b/nox/sessions.py @@ -187,7 +187,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 @@ -615,7 +615,11 @@ def _run( env = env or {} env = {**self.env, **env} if include_outer_env: - env = {**self.virtualenv.outer_env, **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 359606ba..6e71c1f5 100644 --- a/nox/virtualenv.py +++ b/nox/virtualenv.py @@ -131,20 +131,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 {} - self.env = {k: v for k, v in env.items() if v is not None} - self.outer_env = { - k: v - for k, v in os.environ.items() - if k not in _BLACKLISTED_ENV_VARS and k not in env - } - - 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 82d70f70..b8dce002 100644 --- a/tests/test_sessions.py +++ b/tests/test_sessions.py @@ -128,7 +128,6 @@ def make_session_and_runner( runner.venv = mock.create_autospec(nox.virtualenv.VirtualEnv) assert runner.venv runner.venv.env = {} - runner.venv.outer_env = dict(os.environ) runner.venv.bin_paths = ["/no/bin/for/you"] # type: ignore[misc] runner.venv.venv_backend = "venv" # type: ignore[misc] return nox.sessions.Session(runner=runner), runner @@ -300,11 +299,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=dict(os.environ), external="error" - ), + **run_with_defaults(paths=mock.ANY, silent=True, env=env, external="error"), ) def test_run_success(self) -> None: @@ -344,10 +344,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( @@ -361,11 +363,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", @@ -417,7 +421,6 @@ def test_run_external_condaenv(self) -> None: assert runner.venv runner.venv.allowed_globals = ("conda",) # type: ignore[misc] runner.venv.env = {} - runner.venv.outer_env = dict(os.environ) runner.venv.bin_paths = ["/path/to/env/bin"] # type: ignore[misc] runner.venv.create.return_value = True # type: ignore[attr-defined] @@ -444,7 +447,6 @@ def test_run_external_with_error_on_external_run_condaenv(self) -> None: runner.venv = mock.create_autospec(nox.virtualenv.CondaEnv) assert runner.venv runner.venv.env = {} - runner.venv.outer_env = dict(os.environ) runner.venv.bin_paths = ["/path/to/env/bin"] # type: ignore[misc] runner.global_config.error_on_external_run = True @@ -618,7 +620,6 @@ def test_conda_install( assert runner.venv runner.venv.location = "/path/to/conda/env" runner.venv.env = {} - runner.venv.outer_env = dict(os.environ) runner.venv.is_offline = lambda: offline # type: ignore[attr-defined] runner.venv.conda_cmd = conda # type: ignore[attr-defined] @@ -666,7 +667,6 @@ def test_conda_venv_reused_with_no_install( assert runner.venv runner.venv.location = "/path/to/conda/env" runner.venv.env = {} - runner.venv.outer_env = dict(os.environ) runner.venv.is_offline = lambda: True # type: ignore[attr-defined] runner.venv.conda_cmd = "conda" # type: ignore[attr-defined] @@ -695,7 +695,6 @@ def test_conda_install_non_default_kwargs(self, version_constraint: str) -> None assert runner.venv runner.venv.location = "/path/to/conda/env" runner.venv.env = {} - runner.venv.outer_env = dict(os.environ) runner.venv.is_offline = lambda: False # type: ignore[attr-defined] runner.venv.conda_cmd = "conda" # type: ignore[attr-defined] @@ -753,7 +752,6 @@ def test_install(self) -> None: runner.venv = mock.create_autospec(nox.virtualenv.VirtualEnv) assert runner.venv runner.venv.env = {} - runner.venv.outer_env = dict(os.environ) runner.venv.venv_backend = "venv" # type: ignore[misc] class SessionNoSlots(nox.sessions.Session): @@ -786,7 +784,6 @@ def test_install_non_default_kwargs(self) -> None: runner.venv = mock.create_autospec(nox.virtualenv.VirtualEnv) assert runner.venv runner.venv.env = {} - runner.venv.outer_env = dict(os.environ) runner.venv.venv_backend = "venv" # type: ignore[misc] class SessionNoSlots(nox.sessions.Session): @@ -817,7 +814,6 @@ def test_install_no_venv_failure(self) -> None: runner.venv = mock.create_autospec(nox.virtualenv.PassthroughEnv) assert runner.venv runner.venv.env = {} - runner.venv.outer_env = dict(os.environ) class SessionNoSlots(nox.sessions.Session): pass @@ -942,7 +938,6 @@ def test_install_uv(self) -> None: runner.venv = mock.create_autospec(nox.virtualenv.VirtualEnv) assert runner.venv runner.venv.env = {} - runner.venv.outer_env = dict(os.environ) runner.venv.venv_backend = "uv" # type: ignore[misc] class SessionNoSlots(nox.sessions.Session): @@ -972,7 +967,6 @@ def test_install_uv_command(self, monkeypatch: pytest.MonkeyPatch) -> None: runner.venv = mock.create_autospec(nox.virtualenv.VirtualEnv) assert runner.venv runner.venv.env = {} - runner.venv.outer_env = dict(os.environ) runner.venv.venv_backend = "uv" # type: ignore[misc] class SessionNoSlots(nox.sessions.Session): @@ -1249,7 +1243,6 @@ def make_runner_with_mock_venv(self) -> nox.sessions.SessionRunner: runner.venv = mock.create_autospec(nox.virtualenv.VirtualEnv) assert runner.venv runner.venv.env = {} - runner.venv.outer_env = dict(os.environ) return runner def test_execute_noop_success(self, caplog: pytest.LogCaptureFixture) -> None: diff --git a/tests/test_virtualenv.py b/tests/test_virtualenv.py index 77e3b8b6..4ce0af1c 100644 --- a/tests/test_virtualenv.py +++ b/tests/test_virtualenv.py @@ -279,9 +279,11 @@ def test_condaenv_detection(make_conda: Callable[..., tuple[CondaEnv, Any]]) -> 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.outer_env, **venv.env}, + env=env, check=True, capture_output=True, ) @@ -340,9 +342,8 @@ def test_env( ) -> None: monkeypatch.setenv("SIGIL", "123") venv, _ = make_one() - assert venv.outer_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"] @@ -434,17 +435,19 @@ def test_create( venv, dir_ = make_one() venv.create() - assert "CONDA_PREFIX" not in venv.outer_env - assert venv.outer_env["NOT_CONDA_PREFIX"] == "something-else" + assert venv.env["CONDA_PREFIX"] is None + assert "NOT_CONDA_PREFIX" not in venv.env if IS_WINDOWS: assert dir_.join("Scripts", "python.exe").check() assert dir_.join("Scripts", "pip.exe").check() assert dir_.join("Lib").check() + assert str(dir_.join("Scripts")) in venv.bin_paths else: assert dir_.join("bin", "python").check() assert dir_.join("bin", "pip").check() assert dir_.join("lib").check() + assert str(dir_.join("bin")) in venv.bin_paths # Test running create on an existing environment. It should be deleted. dir_.ensure("test.txt") @@ -556,6 +559,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()