fix: outer_env issues fixed (#874)

* fix: outer_env issues fixed

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>

* fix: use os.environ more directly, handle bin paths

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>

---------

Signed-off-by: Henry Schreiner <henryschreineriii@gmail.com>

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:

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:

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()

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()