Skip to content

Commit

Permalink
fix: use os.environ more directly, handle bin paths
Browse files Browse the repository at this point in the history
Signed-off-by: Henry Schreiner <[email protected]>
  • Loading branch information
henryiii committed Oct 28, 2024
1 parent f1a8046 commit 827c8cf
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 35 deletions.
8 changes: 6 additions & 2 deletions nox/sessions.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,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

Expand Down Expand Up @@ -603,7 +603,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:
Expand Down
16 changes: 2 additions & 14 deletions nox/virtualenv.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,20 +125,8 @@ def __init__(
self._bin_paths = 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:
Expand Down
18 changes: 4 additions & 14 deletions tests/test_sessions.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,6 @@ def make_session_and_runner(self):
)
runner.venv = mock.create_autospec(nox.virtualenv.VirtualEnv)
runner.venv.env = {}
runner.venv.outer_env = dict(os.environ)
runner.venv.bin_paths = ["/no/bin/for/you"]
runner.venv.venv_backend = "venv"
return nox.sessions.Session(runner=runner), runner
Expand Down Expand Up @@ -285,11 +284,12 @@ def test_run_install_only_should_install(self):
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):
Expand Down Expand Up @@ -424,7 +424,6 @@ def test_run_external_with_error_on_external_run_condaenv(self):
session, runner = self.make_session_and_runner()
runner.venv = mock.create_autospec(nox.virtualenv.CondaEnv)
runner.venv.env = {}
runner.venv.outer_env = dict(os.environ)
runner.venv.bin_paths = ["/path/to/env/bin"]

runner.global_config.error_on_external_run = True
Expand Down Expand Up @@ -592,7 +591,6 @@ def test_conda_install(self, auto_offline, offline, conda, channel):
runner.venv = mock.create_autospec(nox.virtualenv.CondaEnv)
runner.venv.location = "/path/to/conda/env"
runner.venv.env = {}
runner.venv.outer_env = dict(os.environ)
runner.venv.is_offline = lambda: offline
runner.venv.conda_cmd = conda

Expand Down Expand Up @@ -637,7 +635,6 @@ def test_conda_venv_reused_with_no_install(self, no_install, reused, run_called)
runner.venv = mock.create_autospec(nox.virtualenv.CondaEnv)
runner.venv.location = "/path/to/conda/env"
runner.venv.env = {}
runner.venv.outer_env = dict(os.environ)
runner.venv.is_offline = lambda: True
runner.venv.conda_cmd = "conda"

Expand Down Expand Up @@ -665,7 +662,6 @@ def test_conda_install_non_default_kwargs(self, version_constraint):
runner.venv = mock.create_autospec(nox.virtualenv.CondaEnv)
runner.venv.location = "/path/to/conda/env"
runner.venv.env = {}
runner.venv.outer_env = dict(os.environ)
runner.venv.is_offline = lambda: False
runner.venv.conda_cmd = "conda"

Expand Down Expand Up @@ -722,7 +718,6 @@ def test_install(self):
)
runner.venv = mock.create_autospec(nox.virtualenv.VirtualEnv)
runner.venv.env = {}
runner.venv.outer_env = dict(os.environ)
runner.venv.venv_backend = "venv"

class SessionNoSlots(nox.sessions.Session):
Expand Down Expand Up @@ -754,7 +749,6 @@ def test_install_non_default_kwargs(self):
)
runner.venv = mock.create_autospec(nox.virtualenv.VirtualEnv)
runner.venv.env = {}
runner.venv.outer_env = dict(os.environ)
runner.venv.venv_backend = "venv"

class SessionNoSlots(nox.sessions.Session):
Expand Down Expand Up @@ -784,7 +778,6 @@ def test_install_no_venv_failure(self):
)
runner.venv = mock.create_autospec(nox.virtualenv.PassthroughEnv)
runner.venv.env = {}
runner.venv.outer_env = dict(os.environ)

class SessionNoSlots(nox.sessions.Session):
pass
Expand Down Expand Up @@ -902,7 +895,6 @@ def test_install_uv(self):
)
runner.venv = mock.create_autospec(nox.virtualenv.VirtualEnv)
runner.venv.env = {}
runner.venv.outer_env = dict(os.environ)
runner.venv.venv_backend = "uv"

class SessionNoSlots(nox.sessions.Session):
Expand Down Expand Up @@ -931,7 +923,6 @@ def test_install_uv_command(self, monkeypatch):
)
runner.venv = mock.create_autospec(nox.virtualenv.VirtualEnv)
runner.venv.env = {}
runner.venv.outer_env = dict(os.environ)
runner.venv.venv_backend = "uv"

class SessionNoSlots(nox.sessions.Session):
Expand Down Expand Up @@ -1182,7 +1173,6 @@ def make_runner_with_mock_venv(self):
runner._create_venv = mock.Mock()
runner.venv = mock.create_autospec(nox.virtualenv.VirtualEnv)
runner.venv.env = {}
runner.venv.outer_env = dict(os.environ)
return runner

def test_execute_noop_success(self, caplog):
Expand Down
11 changes: 6 additions & 5 deletions tests/test_virtualenv.py
Original file line number Diff line number Diff line change
Expand Up @@ -245,9 +245,11 @@ def test_condaenv_detection(make_conda):
venv, dir_ = make_conda()
venv.create()

env = {k: v for k, v in {**os.environ, **venv.env}.items() if v is not None}

proc_result = subprocess.run(
[shutil.which("conda"), "list"],
env={**venv.outer_env, **venv.env},
env=env,
check=True,
capture_output=True,
)
Expand Down Expand Up @@ -293,9 +295,8 @@ def test_constructor_explicit(make_one):
def test_env(monkeypatch, make_one):
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"]


Expand Down Expand Up @@ -372,8 +373,8 @@ def test_create(monkeypatch, make_one):
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()
Expand Down

0 comments on commit 827c8cf

Please sign in to comment.