config: split _getconftestmodules and _loadconftestmodules

Previously, the `_getconftestmodules` function was used both to load
conftest modules for a path (during `pytest_load_initial_conftests`),
and to retrieve conftest modules for a path (during hook dispatch and
for fetching `collect_ignore`). This made things muddy - it is usually
nicer to have clear separation between "command" and "query" functions,
when they occur in separate phases.

So split into "load" and "get".

Currently, `gethookproxy` still loads conftest itself. I hope to change
this in the future.

Author: bluetech

src/_pytest/config/__init__.py

@@ -581,26 +581,25 @@ def _is_in_confcutdir(self, path: Path) -> bool:
     def _try_load_conftest(
         self, anchor: Path, importmode: Union[str, ImportMode], rootpath: Path
     ) -> None:
-        self._getconftestmodules(anchor, importmode, rootpath)
+        self._loadconftestmodules(anchor, importmode, rootpath)
         # let's also consider test* subdirs
         if anchor.is_dir():
             for x in anchor.glob("test*"):
                 if x.is_dir():
-                    self._getconftestmodules(x, importmode, rootpath)
+                    self._loadconftestmodules(x, importmode, rootpath)
 
-    def _getconftestmodules(
+    def _loadconftestmodules(
         self, path: Path, importmode: Union[str, ImportMode], rootpath: Path
-    ) -> Sequence[types.ModuleType]:
+    ) -> None:
         if self._noconftest:
-            return []
+            return
 
         directory = self._get_directory(path)
 
         # Optimization: avoid repeated searches in the same directory.
         # Assumes always called with same importmode and rootpath.
-        existing_clist = self._dirpath2confmods.get(directory)
-        if existing_clist is not None:
-            return existing_clist
+        if directory in self._dirpath2confmods:
+            return
 
         # XXX these days we may rather want to use config.rootpath
         # and allow users to opt into looking into the rootdir parent
@@ -613,16 +612,17 @@ def _getconftestmodules(
                     mod = self._importconftest(conftestpath, importmode, rootpath)
                     clist.append(mod)
         self._dirpath2confmods[directory] = clist
-        return clist
+
+    def _getconftestmodules(self, path: Path) -> Sequence[types.ModuleType]:
+        directory = self._get_directory(path)
+        return self._dirpath2confmods.get(directory, ())
 
     def _rget_with_confmod(
         self,
         name: str,
         path: Path,
-        importmode: Union[str, ImportMode],
-        rootpath: Path,
     ) -> Tuple[types.ModuleType, Any]:
-        modules = self._getconftestmodules(path, importmode, rootpath=rootpath)
+        modules = self._getconftestmodules(path)
         for mod in reversed(modules):
             try:
                 return mod, getattr(mod, name)
@@ -1562,13 +1562,9 @@ def _getini(self, name: str):
         else:
             return self._getini_unknown_type(name, type, value)
 
-    def _getconftest_pathlist(
-        self, name: str, path: Path, rootpath: Path
-    ) -> Optional[List[Path]]:
+    def _getconftest_pathlist(self, name: str, path: Path) -> Optional[List[Path]]:
         try:
-            mod, relroots = self.pluginmanager._rget_with_confmod(
-                name, path, self.getoption("importmode"), rootpath
-            )
+            mod, relroots = self.pluginmanager._rget_with_confmod(name, path)
         except KeyError:
             return None
         assert mod.__file__ is not None

src/_pytest/main.py

@@ -376,7 +376,7 @@ def _in_venv(path: Path) -> bool:
 
 def pytest_ignore_collect(collection_path: Path, config: Config) -> Optional[bool]:
     ignore_paths = config._getconftest_pathlist(
-        "collect_ignore", path=collection_path.parent, rootpath=config.rootpath
+        "collect_ignore", path=collection_path.parent
     )
     ignore_paths = ignore_paths or []
     excludeopt = config.getoption("ignore")
@@ -387,7 +387,7 @@ def pytest_ignore_collect(collection_path: Path, config: Config) -> Optional[boo
         return True
 
     ignore_globs = config._getconftest_pathlist(
-        "collect_ignore_glob", path=collection_path.parent, rootpath=config.rootpath
+        "collect_ignore_glob", path=collection_path.parent
     )
     ignore_globs = ignore_globs or []
     excludeglobopt = config.getoption("ignore_glob")
@@ -551,11 +551,16 @@ def gethookproxy(self, fspath: "os.PathLike[str]"):
         pm = self.config.pluginmanager
         # Check if we have the common case of running
         # hooks with all conftest.py files.
-        my_conftestmodules = pm._getconftestmodules(
+        #
+        # TODO: pytest relies on this call to load non-initial conftests. This
+        # is incidental. It will be better to load conftests at a more
+        # well-defined place.
+        pm._loadconftestmodules(
             path,
             self.config.getoption("importmode"),
             rootpath=self.config.rootpath,
         )
+        my_conftestmodules = pm._getconftestmodules(path)
         remove_mods = pm._conftest_plugins.difference(my_conftestmodules)
         if remove_mods:
             # One or more conftests are not in use at this fspath.

testing/python/fixtures.py

@@ -2103,9 +2103,7 @@ def test_2(self):
         reprec = pytester.inline_run("-v", "-s", "--confcutdir", pytester.path)
         reprec.assertoutcome(passed=8)
         config = reprec.getcalls("pytest_unconfigure")[0].config
-        values = config.pluginmanager._getconftestmodules(
-            p, importmode="prepend", rootpath=pytester.path
-        )[0].values
+        values = config.pluginmanager._getconftestmodules(p)[0].values
         assert values == ["fin_a1", "fin_a2", "fin_b1", "fin_b2"] * 2
 
     def test_scope_ordering(self, pytester: Pytester) -> None:

testing/test_config.py

@@ -642,18 +642,11 @@ def test_getconftest_pathlist(self, pytester: Pytester, tmp_path: Path) -> None:
         p = tmp_path.joinpath("conftest.py")
         p.write_text(f"mylist = {['.', str(somepath)]}", encoding="utf-8")
         config = pytester.parseconfigure(p)
-        assert (
-            config._getconftest_pathlist("notexist", path=tmp_path, rootpath=tmp_path)
-            is None
-        )
-        pl = (
-            config._getconftest_pathlist("mylist", path=tmp_path, rootpath=tmp_path)
-            or []
-        )
-        print(pl)
-        assert len(pl) == 2
-        assert pl[0] == tmp_path
-        assert pl[1] == somepath
+        assert config._getconftest_pathlist("notexist", path=tmp_path) is None
+        assert config._getconftest_pathlist("mylist", path=tmp_path) == [
+            tmp_path,
+            somepath,
+        ]
 
     @pytest.mark.parametrize("maybe_type", ["not passed", "None", '"string"'])
     def test_addini(self, pytester: Pytester, maybe_type: str) -> None:

testing/test_conftest.py

@@ -62,62 +62,46 @@ def basedir(
     def test_basic_init(self, basedir: Path) -> None:
         conftest = PytestPluginManager()
         p = basedir / "adir"
-        assert (
-            conftest._rget_with_confmod("a", p, importmode="prepend", rootpath=basedir)[
-                1
-            ]
-            == 1
-        )
+        conftest._loadconftestmodules(p, importmode="prepend", rootpath=basedir)
+        assert conftest._rget_with_confmod("a", p)[1] == 1
 
     def test_immediate_initialiation_and_incremental_are_the_same(
         self, basedir: Path
     ) -> None:
         conftest = PytestPluginManager()
         assert not len(conftest._dirpath2confmods)
-        conftest._getconftestmodules(
-            basedir, importmode="prepend", rootpath=Path(basedir)
-        )
+        conftest._loadconftestmodules(basedir, importmode="prepend", rootpath=basedir)
         snap1 = len(conftest._dirpath2confmods)
         assert snap1 == 1
-        conftest._getconftestmodules(
+        conftest._loadconftestmodules(
             basedir / "adir", importmode="prepend", rootpath=basedir
         )
         assert len(conftest._dirpath2confmods) == snap1 + 1
-        conftest._getconftestmodules(
+        conftest._loadconftestmodules(
             basedir / "b", importmode="prepend", rootpath=basedir
         )
         assert len(conftest._dirpath2confmods) == snap1 + 2
 
     def test_value_access_not_existing(self, basedir: Path) -> None:
         conftest = ConftestWithSetinitial(basedir)
         with pytest.raises(KeyError):
-            conftest._rget_with_confmod(
-                "a", basedir, importmode="prepend", rootpath=Path(basedir)
-            )
+            conftest._rget_with_confmod("a", basedir)
 
     def test_value_access_by_path(self, basedir: Path) -> None:
         conftest = ConftestWithSetinitial(basedir)
         adir = basedir / "adir"
-        assert (
-            conftest._rget_with_confmod(
-                "a", adir, importmode="prepend", rootpath=basedir
-            )[1]
-            == 1
-        )
-        assert (
-            conftest._rget_with_confmod(
-                "a", adir / "b", importmode="prepend", rootpath=basedir
-            )[1]
-            == 1.5
+        conftest._loadconftestmodules(adir, importmode="prepend", rootpath=basedir)
+        assert conftest._rget_with_confmod("a", adir)[1] == 1
+        conftest._loadconftestmodules(
+            adir / "b", importmode="prepend", rootpath=basedir
         )
+        assert conftest._rget_with_confmod("a", adir / "b")[1] == 1.5
 
     def test_value_access_with_confmod(self, basedir: Path) -> None:
         startdir = basedir / "adir" / "b"
         startdir.joinpath("xx").mkdir()
         conftest = ConftestWithSetinitial(startdir)
-        mod, value = conftest._rget_with_confmod(
-            "a", startdir, importmode="prepend", rootpath=Path(basedir)
-        )
+        mod, value = conftest._rget_with_confmod("a", startdir)
         assert value == 1.5
         assert mod.__file__ is not None
         path = Path(mod.__file__)
@@ -143,9 +127,7 @@ def test_doubledash_considered(pytester: Pytester) -> None:
     conf.joinpath("conftest.py").touch()
     conftest = PytestPluginManager()
     conftest_setinitial(conftest, [conf.name, conf.name])
-    values = conftest._getconftestmodules(
-        conf, importmode="prepend", rootpath=pytester.path
-    )
+    values = conftest._getconftestmodules(conf)
     assert len(values) == 1
 
 
@@ -192,26 +174,22 @@ def test_conftestcutdir(pytester: Pytester) -> None:
     p = pytester.mkdir("x")
     conftest = PytestPluginManager()
     conftest_setinitial(conftest, [pytester.path], confcutdir=p)
-    values = conftest._getconftestmodules(
-        p, importmode="prepend", rootpath=pytester.path
-    )
+    conftest._loadconftestmodules(p, importmode="prepend", rootpath=pytester.path)
+    values = conftest._getconftestmodules(p)
     assert len(values) == 0
-    values = conftest._getconftestmodules(
+    conftest._loadconftestmodules(
         conf.parent, importmode="prepend", rootpath=pytester.path
     )
+    values = conftest._getconftestmodules(conf.parent)
     assert len(values) == 0
     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(
-        conf.parent, importmode="prepend", rootpath=pytester.path
-    )
+    values = conftest._getconftestmodules(conf.parent)
     assert values[0].__file__ is not None
     assert values[0].__file__.startswith(str(conf))
     # and all sub paths get updated properly
-    values = conftest._getconftestmodules(
-        p, importmode="prepend", rootpath=pytester.path
-    )
+    values = conftest._getconftestmodules(p)
     assert len(values) == 1
     assert values[0].__file__ is not None
     assert values[0].__file__.startswith(str(conf))
@@ -221,9 +199,7 @@ def test_conftestcutdir_inplace_considered(pytester: Pytester) -> None:
     conf = pytester.makeconftest("")
     conftest = PytestPluginManager()
     conftest_setinitial(conftest, [conf.parent], confcutdir=conf.parent)
-    values = conftest._getconftestmodules(
-        conf.parent, importmode="prepend", rootpath=pytester.path
-    )
+    values = conftest._getconftestmodules(conf.parent)
     assert len(values) == 1
     assert values[0].__file__ is not None
     assert values[0].__file__.startswith(str(conf))
@@ -433,10 +409,8 @@ def impct(p, importmode, root):
     conftest = PytestPluginManager()
     conftest._confcutdir = pytester.path
     monkeypatch.setattr(conftest, "_importconftest", impct)
-    mods = cast(
-        List[Path],
-        conftest._getconftestmodules(sub, importmode="prepend", rootpath=pytester.path),
-    )
+    conftest._loadconftestmodules(sub, importmode="prepend", rootpath=pytester.path)
+    mods = cast(List[Path], conftest._getconftestmodules(sub))
     expected = [ct1, ct2]
     assert mods == expected