Fix regression in check for stale environments

nox/virtualenv.py

@@ -293,7 +293,7 @@ def __init__(
         reuse_existing: bool = False,
         *,
         venv: bool = False,
-        venv_params: Any = None
+        venv_params: Any = None,
     ):
         self.location_name = location
         self.location = os.path.abspath(location)
@@ -307,19 +307,39 @@ def __init__(
     def _clean_location(self) -> bool:
         """Deletes any existing virtual environment"""
         if os.path.exists(self.location):
-            if self.reuse_existing:
-                self._check_reused_environment()
+            if (
+                self.reuse_existing
+                and self._check_reused_environment_type()
+                and self._check_reused_environment_interpreter()
+            ):
                 return False
             else:
                 shutil.rmtree(self.location)
 
         return True
 
-    def _check_reused_environment(self) -> None:
+    def _check_reused_environment_type(self) -> bool:
         """Check if reused environment type is the same."""
-        with open(os.path.join(self.location, "pyvenv.cfg")) as fp:
-            old_env = "virtualenv" if "virtualenv" in fp.read() else "venv"
-        assert old_env == self.venv_or_virtualenv
+        path = os.path.join(self.location, "pyvenv.cfg")
+        if not os.path.isfile(path):
+            # virtualenv < 20.0 does not create pyvenv.cfg
+            old_env = "virtualenv"
+        else:
+            pattern = re.compile(f"virtualenv[ \t]*=")
+            with open(path) as fp:
+                old_env = (
+                    "virtualenv" if any(pattern.match(line) for line in fp) else "venv"
+                )
+        return old_env == self.venv_or_virtualenv
+
+    def _check_reused_environment_interpreter(self) -> bool:
+        """Check if reused environment interpreter is the same."""
+        program = "import sys; print(getattr(sys, 'real_prefix', sys.base_prefix))"
+        original = nox.command.run(
+            [self._resolved_interpreter, "-c", program], silent=True
+        )
+        created = nox.command.run(["python", "-c", program], silent=True)
+        return original == created
 
     @property
     def _resolved_interpreter(self) -> str:
@@ -396,25 +416,12 @@ def bin_paths(self) -> List[str]:
     def create(self) -> bool:
         """Create the virtualenv or venv."""
         if not self._clean_location():
-            original = nox.command.run(
-                [self._resolved_interpreter, "-c", "import sys; print(sys.prefix)"],
-                silent=True,
-            )
-            created = nox.command.run(
-                [
-                    os.path.join(self.location, "bin", "python"),
-                    "-c",
-                    "import sys; print(sys.real_prefix)",
-                ],
-                silent=True,
-            )
-            if original == created:
-                logger.debug(
-                    "Re-using existing virtual environment at {}.".format(
-                        self.location_name
-                    )
+            logger.debug(
+                "Re-using existing virtual environment at {}.".format(
+                    self.location_name
                 )
-                return False
+            )
+            return False
 
         if self.venv_or_virtualenv == "virtualenv":
             cmd = [sys.executable, "-m", "virtualenv", self.location]

tests/test_virtualenv.py

@@ -15,6 +15,7 @@
 import os
 import shutil
 import sys
+from textwrap import dedent
 from unittest import mock
 
 import nox.virtualenv
@@ -257,7 +258,12 @@ def test__clean_location(monkeypatch, make_one):
     # Don't re-use existing, but doesn't currently exist.
     # Should return True indicating that the venv needs to be created.
     monkeypatch.setattr(
-        nox.virtualenv.VirtualEnv, "_check_reused_environment", mock.MagicMock()
+        nox.virtualenv.VirtualEnv, "_check_reused_environment_type", mock.MagicMock()
+    )
+    monkeypatch.setattr(
+        nox.virtualenv.VirtualEnv,
+        "_check_reused_environment_interpreter",
+        mock.MagicMock(),
     )
     monkeypatch.delattr(nox.virtualenv.shutil, "rmtree")
     assert not dir_.check()
@@ -332,19 +338,107 @@ def test_create(monkeypatch, make_one):
     assert dir_.join("test.txt").check()
 
 
-def test_create_check_interpreter(make_one, monkeypatch, tmpdir):
-    cmd_mock = mock.MagicMock(
-        side_effect=["python-1", "python-1", "python-2", "python-3", "python-4"]
-    )
-    monkeypatch.setattr(nox.virtualenv.nox.command, "run", cmd_mock)
-    test_dir = tmpdir.mkdir("pytest")
-    fp = test_dir.join("pyvenv.cfg")
-    fp.write("virtualenv")
-    venv, dir_ = make_one()
-    venv.reuse_existing = True
-    venv.location = test_dir.strpath
-    assert not venv.create()
-    assert venv.create()
+def test_create_reuse_environment(make_one):
+    venv, location = make_one(reuse_existing=True)
+    venv.create()
+
+    reused = not venv.create()
+
+    assert reused
+
+
+def test_create_reuse_environment_with_different_interpreter(make_one, monkeypatch):
+    venv, location = make_one(reuse_existing=True)
+    venv.create()
+
+    # Pretend that the environment was created with a different interpreter.
+    monkeypatch.setattr(venv, "_check_reused_environment_interpreter", lambda: False)
+
+    # Create a marker file. It should be gone after the environment is re-created.
+    location.join("marker").ensure()
+
+    reused = not venv.create()
+
+    assert not reused
+    assert not location.join("marker").check()
+
+
+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.
+    pyvenv_cfg = """\
+    home = /usr/bin
+    include-system-site-packages = false
+    version = 3.9.6
+    """
+    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
+
+
+def test_create_reuse_stale_virtualenv_environment(make_one):
+    venv, location = make_one(reuse_existing=True, venv=True)
+    venv.create()
+
+    # Drop a virtualenv-style pyvenv.cfg into the environment.
+    pyvenv_cfg = """\
+    home = /usr
+    implementation = CPython
+    version_info = 3.9.6.final.0
+    virtualenv = 20.4.6
+    include-system-site-packages = false
+    base-prefix = /usr
+    base-exec-prefix = /usr
+    base-executable = /usr/bin/python3.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
+    # venv-style environment.
+    assert not reused
+
+
+def test_create_reuse_venv_environment(make_one):
+    venv, location = make_one(reuse_existing=True, venv=True)
+    venv.create()
+
+    # Use a spurious occurrence of "virtualenv" in the pyvenv.cfg.
+    pyvenv_cfg = """\
+    home = /opt/virtualenv/bin
+    include-system-site-packages = false
+    version = 3.9.6
+    """
+    location.join("pyvenv.cfg").write(dedent(pyvenv_cfg))
+
+    reused = not venv.create()
+
+    # The environment should be detected as venv-style and reused.
+    assert reused
+
+
+def test_create_reuse_oldstyle_virtualenv_environment(make_one):
+    venv, location = make_one(reuse_existing=True)
+    venv.create()
+
+    pyvenv_cfg = location.join("pyvenv.cfg")
+    if not pyvenv_cfg.check():
+        pytest.skip("Requires virtualenv >= 20.0.0.")
+
+    # virtualenv < 20.0.0 does not create a pyvenv.cfg file.
+    pyvenv_cfg.remove()
+
+    reused = not venv.create()
+
+    # The environment is detected as virtualenv-style and reused.
+    assert reused
 
 
 def test_create_venv_backend(make_one):