From 01ac13a77d6cd56a5ac20756542e59a7b14129a0 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Sun, 9 Jan 2022 13:58:29 +0200 Subject: [PATCH] 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. --- src/_pytest/config/__init__.py | 32 +++++++--------- src/_pytest/main.py | 11 ++++-- testing/python/fixtures.py | 4 +- testing/test_config.py | 17 +++------ testing/test_conftest.py | 70 +++++++++++----------------------- 5 files changed, 50 insertions(+), 84 deletions(-) diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index 2b6f250f316..8dbaf7c70e5 100644 --- a/src/_pytest/config/__init__.py +++ b/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 diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 4c3c9aed439..fd3836736ed 100644 --- a/src/_pytest/main.py +++ b/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. diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index 191689d1c2e..7c028277253 100644 --- a/testing/python/fixtures.py +++ b/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: diff --git a/testing/test_config.py b/testing/test_config.py index 43561000c91..04161f238d8 100644 --- a/testing/test_config.py +++ b/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: diff --git a/testing/test_conftest.py b/testing/test_conftest.py index 4278315078f..cfc2d577b53 100644 --- a/testing/test_conftest.py +++ b/testing/test_conftest.py @@ -62,28 +62,22 @@ 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 @@ -91,33 +85,23 @@ def test_immediate_initialiation_and_incremental_are_the_same( 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