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
henryiii committed Nov 23, 2024
1 parent daf5d58 commit 356ff7e
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 41 deletions.
8 changes: 6 additions & 2 deletions nox/sessions.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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:
Expand Down
16 changes: 2 additions & 14 deletions nox/virtualenv.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
33 changes: 13 additions & 20 deletions tests/test_sessions.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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(
Expand All @@ -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",
Expand Down Expand Up @@ -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]

Expand All @@ -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
Expand Down Expand Up @@ -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]

Expand Down Expand Up @@ -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]

Expand Down Expand Up @@ -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]

Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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:
Expand Down
15 changes: 10 additions & 5 deletions tests/test_virtualenv.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down Expand Up @@ -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"]


Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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()
Expand Down

0 comments on commit 356ff7e

Please sign in to comment.