From 5f23157b39143f25560ee0dfa52a2df49a7cb6c3 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Sat, 8 Jan 2022 12:36:43 +0200 Subject: [PATCH 1/7] config: remove always truthy condition --- src/_pytest/config/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index 174d80a01ca..5ec4099140e 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -602,7 +602,7 @@ def _importconftest( 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}") From 1c7644cc7a796dda13c8ee56c3d65d99bd35a39b Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Sat, 8 Jan 2022 13:50:52 +0200 Subject: [PATCH 2/7] config: some comments --- src/_pytest/config/__init__.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index 5ec4099140e..79b2ad59862 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -345,14 +345,21 @@ 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]] = {} + # The conftest module of a conftest path. 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 From 0c98f1923101e5905c54ba07650a043fca374f4b Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Sat, 8 Jan 2022 22:41:14 +0200 Subject: [PATCH 3/7] config: make confcutdir check a bit more clear & correct I think this named function makes the code a bit easier to understand. Also change the check to explicitly check for "is a sub-path of" instead of the previous check which only worked assuming that path is within confcutdir or a direct parent of it. --- src/_pytest/config/__init__.py | 25 ++++++++++++++++++------- src/_pytest/main.py | 3 +-- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index 79b2ad59862..a30ee6cee00 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -521,6 +521,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: @@ -552,14 +565,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 diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 952c703509d..afe613fd0d9 100644 --- a/src/_pytest/main.py +++ b/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(): From ed83efaf4bd802a0c96d8be0c841d1d508fbffd5 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Sun, 9 Jan 2022 00:12:33 +0200 Subject: [PATCH 4/7] testing/test_monkeypatch: fix some patches leaking into pytest code The tests patch `os.path.abspath` which can break some pytest internal code since the patching is not undone immediately. --- testing/test_monkeypatch.py | 35 ++++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/testing/test_monkeypatch.py b/testing/test_monkeypatch.py index 95521818021..49635f95e79 100644 --- a/testing/test_monkeypatch.py +++ b/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: From d98b695fecd8761ba600cc5e30fb239be5221253 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Sun, 9 Jan 2022 00:28:44 +0200 Subject: [PATCH 5/7] config: return Sequence instead of List from _getconftestmodules Nothing should mutate the internal data structure here. --- src/_pytest/config/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index a30ee6cee00..a6a22d2b913 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -546,7 +546,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 [] From 0ef882364e9249f46d21d1e94867de77096b7c62 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Sun, 9 Jan 2022 01:40:57 +0200 Subject: [PATCH 6/7] config: stop resolving symlinks in conftest paths This became the wrong thing to do since 322190fd84e1b86d7b9a2d71f086445ca80c39b3. --- changelog/9493.bugfix.rst | 10 ++++++++++ src/_pytest/config/__init__.py | 11 ++--------- 2 files changed, 12 insertions(+), 9 deletions(-) create mode 100644 changelog/9493.bugfix.rst diff --git a/changelog/9493.bugfix.rst b/changelog/9493.bugfix.rst new file mode 100644 index 00000000000..d99c80b7d2c --- /dev/null +++ b/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). diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index a6a22d2b913..b75a675b6d8 100644 --- a/src/_pytest/config/__init__.py +++ b/src/_pytest/config/__init__.py @@ -592,15 +592,8 @@ 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] + return self._conftestpath2mod[conftestpath] pkgpath = resolve_package_path(conftestpath) if pkgpath is None: @@ -616,7 +609,7 @@ def _importconftest( self._check_non_top_pytest_plugins(mod, conftestpath) self._conftest_plugins.add(mod) - self._conftestpath2mod[key] = mod + self._conftestpath2mod[conftestpath] = mod dirpath = conftestpath.parent if dirpath in self._dirpath2confmods: for path, mods in self._dirpath2confmods.items(): From 161bc481178c7dbd59b3ee3de92606a006a269aa Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Sun, 9 Jan 2022 00:38:40 +0200 Subject: [PATCH 7/7] config: get rid of _conftestpath2mod It duplicates what PluginManager already knows, and no longer needed now that symlinks are not resolved (see previous commit). --- src/_pytest/config/__init__.py | 9 +++------ testing/test_conftest.py | 21 ++++++++++----------- 2 files changed, 13 insertions(+), 17 deletions(-) diff --git a/src/_pytest/config/__init__.py b/src/_pytest/config/__init__.py index b75a675b6d8..3896313e033 100644 --- a/src/_pytest/config/__init__.py +++ b/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 @@ -353,8 +352,6 @@ def __init__(self) -> None: # This includes the directory's own conftest modules as well # as those of its parent directories. self._dirpath2confmods: Dict[Path, List[types.ModuleType]] = {} - # The conftest module of a conftest path. - 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. @@ -592,8 +589,9 @@ def _rget_with_confmod( def _importconftest( self, conftestpath: Path, importmode: Union[str, ImportMode], rootpath: Path ) -> types.ModuleType: - with contextlib.suppress(KeyError): - return self._conftestpath2mod[conftestpath] + 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: @@ -609,7 +607,6 @@ def _importconftest( self._check_non_top_pytest_plugins(mod, conftestpath) self._conftest_plugins.add(mod) - self._conftestpath2mod[conftestpath] = mod dirpath = conftestpath.parent if dirpath in self._dirpath2confmods: for path, mods in self._dirpath2confmods.items(): diff --git a/testing/test_conftest.py b/testing/test_conftest.py index 92a5ffb72cf..4cbc2d14c36 100644 --- a/testing/test_conftest.py +++ b/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: