Repository: wntrblm/nox
Pull request #786 (2024-02-28 20:00:45)

fix: warn user when first argument to session.run is a list
They probably meant to unpack the list.

Closes https://github.com/wntrblm/nox/issues/746


Patch:
diff --git a/nox/sessions.py b/nox/sessions.py
index a98916f..1ecacf0 100644
--- a/nox/sessions.py
+++ b/nox/sessions.py
@@ -379,6 +379,10 @@ class Session:
         if not args:
             raise ValueError("At least one argument required to run().")
 
+        if len(args) == 1 and isinstance(args[0], (list, tuple)):
+            msg = "First argument to `session.run` is a list. Did you mean to use `session.run(*args)`?"
+            raise ValueError(msg)
+
         if self._runner.global_config.install_only:
             logger.info(f"Skipping {args[0]} run, as --install-only is set.")
             return None
diff --git a/nox/virtualenv.py b/nox/virtualenv.py
index d11db74..1895f84 100644
--- a/nox/virtualenv.py
+++ b/nox/virtualenv.py
@@ -35,7 +35,6 @@ _BLACKLISTED_ENV_VARS = frozenset(
     ["PIP_RESPECT_VIRTUALENV", "PIP_REQUIRE_VIRTUALENV", "__PYVENV_LAUNCHER__"]
 )
 _SYSTEM = platform.system()
-_ENABLE_STALENESS_CHECK = "NOX_ENABLE_STALENESS_CHECK" in os.environ
 
 
 class InterpreterNotFound(OSError):
@@ -214,9 +213,12 @@ class CondaEnv(ProcessEnv):
 
     def _clean_location(self) -> bool:
         """Deletes existing conda environment"""
+        is_conda = os.path.isdir(os.path.join(self.location, "conda-meta"))
         if os.path.exists(self.location):
-            if self.reuse_existing:
+            if self.reuse_existing and is_conda:
                 return False
+            if not is_conda:
+                shutil.rmtree(self.location)
             else:
                 cmd = [
                     self.conda_cmd,
@@ -227,9 +229,9 @@ class CondaEnv(ProcessEnv):
                     "--all",
                 ]
                 nox.command.run(cmd, silent=True, log=False)
-                # Make sure that location is clean
-                with contextlib.suppress(FileNotFoundError):
-                    shutil.rmtree(self.location)
+            # Make sure that location is clean
+            with contextlib.suppress(FileNotFoundError):
+                shutil.rmtree(self.location)
 
         return True
 
@@ -330,45 +332,78 @@ class VirtualEnv(ProcessEnv):
         self.reuse_existing = reuse_existing
         self.venv_backend = venv_backend
         self.venv_params = venv_params or []
+        if venv_backend not in {"virtualenv", "venv", "uv"}:
+            msg = f"venv_backend {venv_backend} not recognized"
+            raise ValueError(msg)
         super().__init__(env={"VIRTUAL_ENV": self.location})
 
     def _clean_location(self) -> bool:
         """Deletes any existing virtual environment"""
         if os.path.exists(self.location):
-            if self.reuse_existing and not _ENABLE_STALENESS_CHECK:
-                return False
             if (
                 self.reuse_existing
                 and self._check_reused_environment_type()
                 and self._check_reused_environment_interpreter()
             ):
                 return False
-            else:
-                shutil.rmtree(self.location)
-
+            shutil.rmtree(self.location)
         return True
 
+    def _read_pyvenv_cfg(self) -> dict[str, str] | None:
+        """Read a pyvenv.cfg file into dict, returns None if missing."""
+        path = os.path.join(self.location, "pyvenv.cfg")
+        with contextlib.suppress(FileNotFoundError), open(path) as fp:
+            parts = (x.partition("=") for x in fp if "=" in x)
+            return {k.strip(): v.strip() for k, _, v in parts}
+        return None
+
     def _check_reused_environment_type(self) -> bool:
-        """Check if reused environment type is the same."""
-        try:
-            with open(os.path.join(self.location, "pyvenv.cfg")) as fp:
-                parts = (x.partition("=") for x in fp if "=" in x)
-                config = {k.strip(): v.strip() for k, _, v in parts}
-            if "uv" in config or "gourgeist" in config:
-                old_env = "uv"
-            elif "virtualenv" in config:
-                old_env = "virtualenv"
-            else:
-                old_env = "venv"
-        except FileNotFoundError:  # pragma: no cover
-            # virtualenv < 20.0 does not create pyvenv.cfg
+        """Check if reused environment type is the same or equivalent."""
+
+        config = self._read_pyvenv_cfg()
+        # virtualenv < 20.0 does not create pyvenv.cfg
+        if config is None:
             old_env = "virtualenv"
+        elif "uv" in config or "gourgeist" in config:
+            old_env = "uv"
+        elif "virtualenv" in config:
+            old_env = "virtualenv"
+        else:
+            old_env = "venv"
+
+        # Can't detect mamba separately, but shouldn't matter
+        if os.path.isdir(os.path.join(self.location, "conda-meta")):
+            return False
 
-        return old_env == self.venv_backend
+        # Matching is always true
+        if old_env == self.venv_backend:
+            return True
+
+        # venv family with pip installed
+        if {old_env, self.venv_backend} <= {"virtualenv", "venv"}:
+            return True
+
+        # Switching to "uv" is safe, but not the other direction (no pip)
+        if old_env in {"virtualenv", "venv"} and self.venv_backend == "uv":
+            return True
+
+        return False
 
     def _check_reused_environment_interpreter(self) -> bool:
-        """Check if reused environment interpreter is the same."""
-        original = self._read_base_prefix_from_pyvenv_cfg()
+        """
+        Check if reused environment interpreter is the same. Currently only checks if
+        NOX_ENABLE_STALENESS_CHECK is set in the environment. See
+
+        * https://github.com/wntrblm/nox/issues/449#issuecomment-860030890
+        * https://github.com/wntrblm/nox/issues/441
+        * https://github.com/pypa/virtualenv/issues/2130
+        """
+        if not os.environ.get("NOX_ENABLE_STALENESS_CHECK", ""):
+            return True
+
+        config = self._read_pyvenv_cfg() or {}
+        original = config.get("base-prefix", None)
+
         program = (
             "import sys; sys.stdout.write(getattr(sys, 'real_prefix', sys.base_prefix))"
         )
@@ -384,18 +419,11 @@ class VirtualEnv(ProcessEnv):
             ["python", "-c", program], silent=True, log=False, paths=self.bin_paths
         )
 
-        return original == created
-
-    def _read_base_prefix_from_pyvenv_cfg(self) -> str | None:
-        """Return the base-prefix entry from pyvenv.cfg, if present."""
-        path = os.path.join(self.location, "pyvenv.cfg")
-        if os.path.isfile(path):
-            with open(path) as io:
-                for line in io:
-                    key, _, value = line.partition("=")
-                    if key.strip() == "base-prefix":
-                        return value.strip()
-        return None
+        return (
+            os.path.exists(original)
+            and os.path.exists(created)
+            and os.path.samefile(original, created)
+        )
 
     @property
     def _resolved_interpreter(self) -> str:


Test patch:
diff --git a/tests/test_sessions.py b/tests/test_sessions.py
index 24ebd63..3b8c53d 100644
--- a/tests/test_sessions.py
+++ b/tests/test_sessions.py
@@ -842,6 +842,12 @@ class TestSession:
         expected = {name: getattr(session, name) for name in session.__slots__}
         assert session.__dict__ == expected
 
+    def test_first_arg_list(self):
+        session, _ = self.make_session_and_runner()
+
+        with pytest.raises(ValueError):
+            session.run(["ls", "-al"])
+
 
 class TestSessionRunner:
     def make_runner(self):
diff --git a/tests/test_virtualenv.py b/tests/test_virtualenv.py
index d3973b2..708de0d 100644
--- a/tests/test_virtualenv.py
+++ b/tests/test_virtualenv.py
@@ -39,6 +39,9 @@ HAS_UV = shutil.which("uv") is not None
 RAISE_ERROR = "RAISE_ERROR"
 VIRTUALENV_VERSION = metadata.version("virtualenv")
 
+has_uv = pytest.mark.skipif(not HAS_UV, reason="Missing uv command.")
+has_conda = pytest.mark.skipif(not HAS_CONDA, reason="Missing conda command.")
+
 
 class TextProcessResult(NamedTuple):
     stdout: str
@@ -47,9 +50,16 @@ class TextProcessResult(NamedTuple):
 
 @pytest.fixture
 def make_one(tmpdir):
-    def factory(*args, **kwargs):
+    def factory(*args, venv_backend: str = "virtualenv", **kwargs):
         location = tmpdir.join("venv")
-        venv = nox.virtualenv.VirtualEnv(location.strpath, *args, **kwargs)
+        if venv_backend in {"mamba", "conda"}:
+            venv = nox.virtualenv.CondaEnv(
+                location.strpath, *args, conda_cmd=venv_backend, **kwargs
+            )
+        else:
+            venv = nox.virtualenv.VirtualEnv(
+                location.strpath, *args, venv_backend=venv_backend, **kwargs
+            )
         return (venv, location)
 
     return factory
@@ -123,6 +133,11 @@ def test_process_env_create():
         penv.create()
 
 
+def test_invalid_venv_create(make_one):
+    with pytest.raises(ValueError):
+        make_one(venv_backend="invalid")
+
+
 def test_condaenv_constructor_defaults(make_conda):
     venv, _ = make_conda()
     assert venv.location
@@ -137,7 +152,7 @@ def test_condaenv_constructor_explicit(make_conda):
     assert venv.reuse_existing is True
 
 
-@pytest.mark.skipif(not HAS_CONDA, reason="Missing conda command.")
+@has_conda
 def test_condaenv_create(make_conda):
     venv, dir_ = make_conda()
     venv.create()
@@ -166,7 +181,7 @@ def test_condaenv_create(make_conda):
     assert venv._reused
 
 
-@pytest.mark.skipif(not HAS_CONDA, reason="Missing conda command.")
+@has_conda
 def test_condaenv_create_with_params(make_conda):
     venv, dir_ = make_conda(venv_params=["--verbose"])
     venv.create()
@@ -178,7 +193,7 @@ def test_condaenv_create_with_params(make_conda):
         assert dir_.join("bin", "pip").check()
 
 
-@pytest.mark.skipif(not HAS_CONDA, reason="Missing conda command.")
+@has_conda
 def test_condaenv_create_interpreter(make_conda):
     venv, dir_ = make_conda(interpreter="3.7")
     venv.create()
@@ -192,7 +207,7 @@ def test_condaenv_create_interpreter(make_conda):
         assert dir_.join("bin", "python3.7").check()
 
 
-@pytest.mark.skipif(not HAS_CONDA, reason="Missing conda command.")
+@has_conda
 def test_conda_env_create_verbose(make_conda):
     venv, dir_ = make_conda()
     with mock.patch("nox.virtualenv.nox.command.run") as mock_run:
@@ -222,13 +237,13 @@ def test_condaenv_bin_windows(make_conda):
     ] == venv.bin_paths
 
 
-@pytest.mark.skipif(not HAS_CONDA, reason="Missing conda command.")
+@has_conda
 def test_condaenv_(make_conda):
     venv, dir_ = make_conda()
     assert not venv.is_offline()
 
 
-@pytest.mark.skipif(not HAS_CONDA, reason="Missing conda command.")
+@has_conda
 def test_condaenv_detection(make_conda):
     venv, dir_ = make_conda()
     venv.create()
@@ -245,7 +260,7 @@ def test_condaenv_detection(make_conda):
     assert path_regex.search(output).group("env_dir") == dir_.strpath
 
 
-@pytest.mark.skipif(not HAS_UV, reason="Missing uv command.")
+@has_uv
 def test_uv_creation(make_one):
     venv, _ = make_one(venv_backend="uv")
     assert venv.location
@@ -387,16 +402,10 @@ def test_create_reuse_environment(make_one):
     assert reused
 
 
-@pytest.fixture
-def _enable_staleness_check(monkeypatch):
-    monkeypatch.setattr("nox.virtualenv._ENABLE_STALENESS_CHECK", True)
-
-
-enable_staleness_check = pytest.mark.usefixtures("_enable_staleness_check")
-
-
-@enable_staleness_check
 def test_create_reuse_environment_with_different_interpreter(make_one, monkeypatch):
+    # Making the reuse requirement more strict
+    monkeypatch.setenv("NOX_ENABLE_STALENESS_CHECK", "1")
+
     venv, location = make_one(reuse_existing=True)
     venv.create()
 
@@ -412,37 +421,95 @@ def test_create_reuse_environment_with_different_interpreter(make_one, monkeypat
     assert not location.join("marker").check()
 
 
-@enable_staleness_check
+@has_uv
 def test_create_reuse_stale_venv_environment(make_one):
     venv, location = make_one(reuse_existing=True)
     venv.create()
 
-    # Drop a venv-style pyvenv.cfg into the environment.
+    # Drop a uv-style pyvenv.cfg into the environment.
     pyvenv_cfg = """\
     home = /usr/bin
     include-system-site-packages = false
     version = 3.9.6
+    uv = 0.1.9
     """
     location.join("pyvenv.cfg").write(dedent(pyvenv_cfg))
 
     reused = not venv.create()
 
-    # The environment is not reused because it does not look like a
-    # virtualenv-style environment.
     assert not reused
 
 
-@enable_staleness_check
-def test_create_reuse_stale_virtualenv_environment(make_one):
+def test_not_stale_virtualenv_environment(make_one, monkeypatch):
+    # Making the reuse requirement more strict
+    monkeypatch.setenv("NOX_ENABLE_STALENESS_CHECK", "1")
+
+    venv, location = make_one(reuse_existing=True, venv_backend="virtualenv")
+    venv.create()
+
+    venv, location = make_one(reuse_existing=True, venv_backend="virtualenv")
+    reused = not venv.create()
+
+    assert reused
+
+
+@has_conda
+def test_stale_virtualenv_to_conda_environment(make_one):
+    venv, location = make_one(reuse_existing=True, venv_backend="virtualenv")
+    venv.create()
+
+    venv, location = make_one(reuse_existing=True, venv_backend="conda")
+    reused = not venv.create()
+
+    # The environment is not reused because it is now conda style
+    # environment.
+    assert not reused
+
+
+@has_conda
+def test_reuse_conda_environment(make_one):
+    venv, _ = make_one(reuse_existing=True, venv_backend="conda")
+    venv.create()
+
+    venv, _ = make_one(reuse_existing=True, venv_backend="conda")
+    reused = not venv.create()
+
+    assert reused
+
+
+@pytest.mark.parametrize(
+    ("frm", "to", "result"),
+    [
+        ("virtualenv", "venv", True),
+        ("venv", "virtualenv", True),
+        ("virtualenv", "uv", True),
+        pytest.param("uv", "virtualenv", False, marks=has_uv),
+        pytest.param("conda", "virtualenv", False, marks=has_conda),
+    ],
+)
+def test_stale_environment(make_one, frm, to, result, monkeypatch):
+    monkeypatch.setenv("NOX_ENABLE_STALENESS_CHECK", "1")
+    venv, _ = make_one(reuse_existing=True, venv_backend=frm)
+    venv.create()
+
+    venv, _ = make_one(reuse_existing=True, venv_backend=to)
+    reused = venv._check_reused_environment_type()
+
+    assert reused == result
+
+
+@has_uv
+def test_create_reuse_stale_virtualenv_environment(make_one, monkeypatch):
+    monkeypatch.setenv("NOX_ENABLE_STALENESS_CHECK", "1")
     venv, location = make_one(reuse_existing=True, venv_backend="venv")
     venv.create()
 
-    # Drop a virtualenv-style pyvenv.cfg into the environment.
+    # Drop a uv-style pyvenv.cfg into the environment.
     pyvenv_cfg = """\
     home = /usr
     implementation = CPython
     version_info = 3.9.6.final.0
-    virtualenv = 20.4.6
+    uv = 0.1.9
     include-system-site-packages = false
     base-prefix = /usr
     base-exec-prefix = /usr
@@ -457,8 +524,25 @@ def test_create_reuse_stale_virtualenv_environment(make_one):
     assert not reused
 
 
-@enable_staleness_check
-def test_create_reuse_venv_environment(make_one):
+@has_uv
+def test_create_reuse_uv_environment(make_one):
+    venv, location = make_one(reuse_existing=True, venv_backend="uv")
+    venv.create()
+
+    # Place a spurious occurrence of "uv" in the pyvenv.cfg.
+    pyvenv_cfg = location.join("pyvenv.cfg")
+    pyvenv_cfg.write(pyvenv_cfg.read() + "bogus = uv\n")
+
+    reused = not venv.create()
+
+    # The environment is reused because it looks like a uv environment
+    assert reused
+
+
+def test_create_reuse_venv_environment(make_one, monkeypatch):
+    # Making the reuse requirement more strict
+    monkeypatch.setenv("NOX_ENABLE_STALENESS_CHECK", "1")
+
     venv, location = make_one(reuse_existing=True, venv_backend="venv")
     venv.create()
 
@@ -472,7 +556,6 @@ def test_create_reuse_venv_environment(make_one):
     assert reused
 
 
-@enable_staleness_check
 @pytest.mark.skipif(IS_WINDOWS, reason="Avoid 'No pyvenv.cfg file' error on Windows.")
 def test_create_reuse_oldstyle_virtualenv_environment(make_one):
     venv, location = make_one(reuse_existing=True)
@@ -491,9 +574,9 @@ def test_create_reuse_oldstyle_virtualenv_environment(make_one):
     assert reused
 
 
-@enable_staleness_check
 @pytest.mark.skipif(IS_WINDOWS, reason="Avoid 'No pyvenv.cfg file' error on Windows.")
-def test_inner_functions_reusing_venv(make_one):
+def test_inner_functions_reusing_venv(make_one, monkeypatch):
+    monkeypatch.setenv("NOX_ENABLE_STALENESS_CHECK", "1")
     venv, location = make_one(reuse_existing=True)
     venv.create()
 
@@ -506,7 +589,7 @@ def test_inner_functions_reusing_venv(make_one):
     """
     location.join("pyvenv.cfg").write(dedent(pyvenv_cfg))
 
-    base_prefix = venv._read_base_prefix_from_pyvenv_cfg()
+    base_prefix = venv._read_pyvenv_cfg()["base-prefix"]
     assert base_prefix == "foo"
 
     reused_interpreter = venv._check_reused_environment_interpreter()
@@ -514,7 +597,6 @@ def test_inner_functions_reusing_venv(make_one):
     assert not reused_interpreter
 
 
-@enable_staleness_check
 @pytest.mark.skipif(
     version.parse(VIRTUALENV_VERSION) >= version.parse("20.22.0"),
     reason="Python 2.7 unsupported for virtualenv>=20.22.0",