Merge pull request #9493 from bluetech/conftesting

Some conftest changes

changelog/9493.bugfix.rst

@@ -0,0 +1,10 @@
+Symbolic link components are no longer resolved in conftest paths.
+This means that if a conftest appears twice in collection tree, using symlinks, it will be executed twice.
+For example, given
+
+    tests/real/conftest.py
+    tests/real/test_it.py
+    tests/link -> tests/real
+
+running ``pytest tests`` now imports the conftest twice, once as ``tests/real/conftest.py`` and once as ``tests/link/conftest.py``.
+This is a fix to match a similar change made to test collection itself in pytest 6.0 (see :pull:`6523` for details).

src/_pytest/config/__init__.py

@@ -1,7 +1,6 @@
 """Command line options, ini-file and conftest.py processing."""
 import argparse
 import collections.abc
-import contextlib
 import copy
 import enum
 import inspect
@@ -345,14 +344,19 @@ def __init__(self) -> None:
         import _pytest.assertion
 
         super().__init__("pytest")
-        # The objects are module objects, only used generically.
-        self._conftest_plugins: Set[types.ModuleType] = set()
 
-        # State related to local conftest plugins.
+        # -- State related to local conftest plugins.
+        # All loaded conftest modules.
+        self._conftest_plugins: Set[types.ModuleType] = set()
+        # All conftest modules applicable for a directory.
+        # This includes the directory's own conftest modules as well
+        # as those of its parent directories.
         self._dirpath2confmods: Dict[Path, List[types.ModuleType]] = {}
-        self._conftestpath2mod: Dict[Path, types.ModuleType] = {}
+        # Cutoff directory above which conftests are no longer discovered.
         self._confcutdir: Optional[Path] = None
+        # If set, conftest loading is skipped.
         self._noconftest = False
+
         self._duplicatepaths: Set[Path] = set()
 
         # plugins that were explicitly skipped with pytest.skip
@@ -514,6 +518,19 @@ def _set_initial_conftests(
         if not foundanchor:
             self._try_load_conftest(current, namespace.importmode, rootpath)
 
+    def _is_in_confcutdir(self, path: Path) -> bool:
+        """Whether a path is within the confcutdir.
+
+        When false, should not load conftest.
+        """
+        if self._confcutdir is None:
+            return True
+        try:
+            path.relative_to(self._confcutdir)
+        except ValueError:
+            return False
+        return True
+
     def _try_load_conftest(
         self, anchor: Path, importmode: Union[str, ImportMode], rootpath: Path
     ) -> None:
@@ -526,7 +543,7 @@ def _try_load_conftest(
 
     def _getconftestmodules(
         self, path: Path, importmode: Union[str, ImportMode], rootpath: Path
-    ) -> List[types.ModuleType]:
+    ) -> Sequence[types.ModuleType]:
         if self._noconftest:
             return []
 
@@ -545,14 +562,12 @@ def _getconftestmodules(
         # and allow users to opt into looking into the rootdir parent
         # directories instead of requiring to specify confcutdir.
         clist = []
-        confcutdir_parents = self._confcutdir.parents if self._confcutdir else []
         for parent in reversed((directory, *directory.parents)):
-            if parent in confcutdir_parents:
-                continue
-            conftestpath = parent / "conftest.py"
-            if conftestpath.is_file():
-                mod = self._importconftest(conftestpath, importmode, rootpath)
-                clist.append(mod)
+            if self._is_in_confcutdir(parent):
+                conftestpath = parent / "conftest.py"
+                if conftestpath.is_file():
+                    mod = self._importconftest(conftestpath, importmode, rootpath)
+                    clist.append(mod)
         self._dirpath2confmods[directory] = clist
         return clist
 
@@ -574,15 +589,9 @@ def _rget_with_confmod(
     def _importconftest(
         self, conftestpath: Path, importmode: Union[str, ImportMode], rootpath: Path
     ) -> types.ModuleType:
-        # Use a resolved Path object as key to avoid loading the same conftest
-        # twice with build systems that create build directories containing
-        # symlinks to actual files.
-        # Using Path().resolve() is better than py.path.realpath because
-        # it resolves to the correct path/drive in case-insensitive file systems (#5792)
-        key = conftestpath.resolve()
-
-        with contextlib.suppress(KeyError):
-            return self._conftestpath2mod[key]
+        existing = self.get_plugin(str(conftestpath))
+        if existing is not None:
+            return cast(types.ModuleType, existing)
 
         pkgpath = resolve_package_path(conftestpath)
         if pkgpath is None:
@@ -598,11 +607,10 @@ def _importconftest(
         self._check_non_top_pytest_plugins(mod, conftestpath)
 
         self._conftest_plugins.add(mod)
-        self._conftestpath2mod[key] = mod
         dirpath = conftestpath.parent
         if dirpath in self._dirpath2confmods:
             for path, mods in self._dirpath2confmods.items():
-                if path and dirpath in path.parents or path == dirpath:
+                if dirpath in path.parents or path == dirpath:
                     assert mod not in mods
                     mods.append(mod)
         self.trace(f"loading conftestmodule {mod!r}")

src/_pytest/main.py

@@ -689,9 +689,8 @@ def collect(self) -> Iterator[Union[nodes.Item, nodes.Collector]]:
             # No point in finding packages when collecting doctests.
             if not self.config.getoption("doctestmodules", False):
                 pm = self.config.pluginmanager
-                confcutdir = pm._confcutdir
                 for parent in (argpath, *argpath.parents):
-                    if confcutdir and parent in confcutdir.parents:
+                    if not pm._is_in_confcutdir(argpath):
                         break
 
                     if parent.is_dir():

testing/test_conftest.py

@@ -146,10 +146,9 @@ def test_issue151_load_all_conftests(pytester: Pytester) -> None:
         p = pytester.mkdir(name)
         p.joinpath("conftest.py").touch()
 
-    conftest = PytestPluginManager()
-    conftest_setinitial(conftest, names)
-    d = list(conftest._conftestpath2mod.values())
-    assert len(d) == len(names)
+    pm = PytestPluginManager()
+    conftest_setinitial(pm, names)
+    assert len(set(pm.get_plugins()) - {pm}) == len(names)
 
 
 def test_conftest_global_import(pytester: Pytester) -> None:
@@ -192,7 +191,7 @@ def test_conftestcutdir(pytester: Pytester) -> None:
         conf.parent, importmode="prepend", rootpath=pytester.path
     )
     assert len(values) == 0
-    assert Path(conf) not in conftest._conftestpath2mod
+    assert not conftest.has_plugin(str(conf))
     # but we can still import a conftest directly
     conftest._importconftest(conf, importmode="prepend", rootpath=pytester.path)
     values = conftest._getconftestmodules(
@@ -226,15 +225,15 @@ def test_setinitial_conftest_subdirs(pytester: Pytester, name: str) -> None:
     sub = pytester.mkdir(name)
     subconftest = sub.joinpath("conftest.py")
     subconftest.touch()
-    conftest = PytestPluginManager()
-    conftest_setinitial(conftest, [sub.parent], confcutdir=pytester.path)
+    pm = PytestPluginManager()
+    conftest_setinitial(pm, [sub.parent], confcutdir=pytester.path)
     key = subconftest.resolve()
     if name not in ("whatever", ".dotdir"):
-        assert key in conftest._conftestpath2mod
-        assert len(conftest._conftestpath2mod) == 1
+        assert pm.has_plugin(str(key))
+        assert len(set(pm.get_plugins()) - {pm}) == 1
     else:
-        assert key not in conftest._conftestpath2mod
-        assert len(conftest._conftestpath2mod) == 0
+        assert not pm.has_plugin(str(key))
+        assert len(set(pm.get_plugins()) - {pm}) == 0
 
 
 def test_conftest_confcutdir(pytester: Pytester) -> None:

testing/test_monkeypatch.py

@@ -50,21 +50,24 @@ class A:
 
 class TestSetattrWithImportPath:
     def test_string_expression(self, monkeypatch: MonkeyPatch) -> None:
-        monkeypatch.setattr("os.path.abspath", lambda x: "hello2")
-        assert os.path.abspath("123") == "hello2"
+        with monkeypatch.context() as mp:
+            mp.setattr("os.path.abspath", lambda x: "hello2")
+            assert os.path.abspath("123") == "hello2"
 
     def test_string_expression_class(self, monkeypatch: MonkeyPatch) -> None:
-        monkeypatch.setattr("_pytest.config.Config", 42)
-        import _pytest
+        with monkeypatch.context() as mp:
+            mp.setattr("_pytest.config.Config", 42)
+            import _pytest
 
-        assert _pytest.config.Config == 42  # type: ignore
+            assert _pytest.config.Config == 42  # type: ignore
 
     def test_unicode_string(self, monkeypatch: MonkeyPatch) -> None:
-        monkeypatch.setattr("_pytest.config.Config", 42)
-        import _pytest
+        with monkeypatch.context() as mp:
+            mp.setattr("_pytest.config.Config", 42)
+            import _pytest
 
-        assert _pytest.config.Config == 42  # type: ignore
-        monkeypatch.delattr("_pytest.config.Config")
+            assert _pytest.config.Config == 42  # type: ignore
+            mp.delattr("_pytest.config.Config")
 
     def test_wrong_target(self, monkeypatch: MonkeyPatch) -> None:
         with pytest.raises(TypeError):
@@ -80,14 +83,16 @@ def test_unknown_attr(self, monkeypatch: MonkeyPatch) -> None:
 
     def test_unknown_attr_non_raising(self, monkeypatch: MonkeyPatch) -> None:
         # https://github.com/pytest-dev/pytest/issues/746
-        monkeypatch.setattr("os.path.qweqwe", 42, raising=False)
-        assert os.path.qweqwe == 42  # type: ignore
+        with monkeypatch.context() as mp:
+            mp.setattr("os.path.qweqwe", 42, raising=False)
+            assert os.path.qweqwe == 42  # type: ignore
 
     def test_delattr(self, monkeypatch: MonkeyPatch) -> None:
-        monkeypatch.delattr("os.path.abspath")
-        assert not hasattr(os.path, "abspath")
-        monkeypatch.undo()
-        assert os.path.abspath
+        with monkeypatch.context() as mp:
+            mp.delattr("os.path.abspath")
+            assert not hasattr(os.path, "abspath")
+            mp.undo()
+            assert os.path.abspath
 
 
 def test_delattr() -> None: