From eddd993cf469df33268097f4bdaf60ccb8f50d3b Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Sat, 15 Aug 2020 11:35:54 +0300 Subject: [PATCH] Only define gethookproxy, isinitpath on Session This fixes an issue where pylint complains about missing implementations of abstract methods in subclasses of `File` which only override `collect()` (as they should). It is also cleaner and makes sense, these methods really don't need to be overridden. The previous methods defined directly on `FSCollector` and `Package` are deprecated, to be removed in pytest 7. See commits e2934c3f8c03c83469f4c6670c207773a6e02df4 and f10ab021e21a44e2f0fa2be66660c4a6d4b7a61a for reference. --- changelog/7591.bugfix.rst | 1 + changelog/7648.deprecation.rst | 3 +++ src/_pytest/deprecated.py | 5 ++++ src/_pytest/main.py | 27 ++++++++++++++++++- src/_pytest/nodes.py | 47 ++++++++-------------------------- src/_pytest/python.py | 7 +++-- testing/deprecated_test.py | 27 +++++++++++++++++++ 7 files changed, 78 insertions(+), 39 deletions(-) create mode 100644 changelog/7591.bugfix.rst create mode 100644 changelog/7648.deprecation.rst diff --git a/changelog/7591.bugfix.rst b/changelog/7591.bugfix.rst new file mode 100644 index 00000000000..10de43a96a5 --- /dev/null +++ b/changelog/7591.bugfix.rst @@ -0,0 +1 @@ +pylint shouldn't complain anymore about unimplemented abstract methods when inheriting from :ref:`File `. diff --git a/changelog/7648.deprecation.rst b/changelog/7648.deprecation.rst new file mode 100644 index 00000000000..440b1114116 --- /dev/null +++ b/changelog/7648.deprecation.rst @@ -0,0 +1,3 @@ +The ``gethookproxy()`` and ``isinitpath()`` methods of ``FSCollector`` and ``Package`` are deprecated; +use ``self.session.gethookproxy()`` and ``self.session.isinitpath()`` instead. +This should work on all pytest versions. diff --git a/src/_pytest/deprecated.py b/src/_pytest/deprecated.py index bd2574ba769..7481473fdae 100644 --- a/src/_pytest/deprecated.py +++ b/src/_pytest/deprecated.py @@ -84,3 +84,8 @@ "The pytest_warning_captured is deprecated and will be removed in a future release.\n" "Please use pytest_warning_recorded instead." ) + +FSCOLLECTOR_GETHOOKPROXY_ISINITPATH = PytestDeprecationWarning( + "The gethookproxy() and isinitpath() methods of FSCollector and Package are deprecated; " + "use self.session.gethookproxy() and self.session.isinitpath() instead. " +) diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 0baa22a6aaf..58d45ebd196 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -27,6 +27,7 @@ from _pytest.config import directory_arg from _pytest.config import ExitCode from _pytest.config import hookimpl +from _pytest.config import PytestPluginManager from _pytest.config import UsageError from _pytest.config.argparsing import Parser from _pytest.fixtures import FixtureManager @@ -389,6 +390,17 @@ def pytest_collection_modifyitems(items: List[nodes.Item], config: Config) -> No items[:] = remaining +class FSHookProxy: + def __init__(self, pm: PytestPluginManager, remove_mods) -> None: + self.pm = pm + self.remove_mods = remove_mods + + def __getattr__(self, name: str): + x = self.pm.subset_hook_caller(name, remove_plugins=self.remove_mods) + self.__dict__[name] = x + return x + + class NoMatch(Exception): """Matching cannot locate matching names.""" @@ -495,7 +507,20 @@ def isinitpath(self, path: py.path.local) -> bool: return path in self._initialpaths def gethookproxy(self, fspath: py.path.local): - return super()._gethookproxy(fspath) + # Check if we have the common case of running + # hooks with all conftest.py files. + pm = self.config.pluginmanager + my_conftestmodules = pm._getconftestmodules( + fspath, self.config.getoption("importmode") + ) + remove_mods = pm._conftest_plugins.difference(my_conftestmodules) + if remove_mods: + # One or more conftests are not in use at this fspath. + proxy = FSHookProxy(pm, remove_mods) + else: + # All plugins are active for this fspath. + proxy = self.config.hook + return proxy @overload def perform_collect( diff --git a/src/_pytest/nodes.py b/src/_pytest/nodes.py index 9d2365c4d5e..79e0914438b 100644 --- a/src/_pytest/nodes.py +++ b/src/_pytest/nodes.py @@ -25,7 +25,7 @@ from _pytest.compat import TYPE_CHECKING from _pytest.config import Config from _pytest.config import ConftestImportFailure -from _pytest.config import PytestPluginManager +from _pytest.deprecated import FSCOLLECTOR_GETHOOKPROXY_ISINITPATH from _pytest.deprecated import NODE_USE_FROM_PARENT from _pytest.fixtures import FixtureDef from _pytest.fixtures import FixtureLookupError @@ -495,17 +495,6 @@ def _check_initialpaths_for_relpath(session, fspath): return fspath.relto(initial_path) -class FSHookProxy: - def __init__(self, pm: PytestPluginManager, remove_mods) -> None: - self.pm = pm - self.remove_mods = remove_mods - - def __getattr__(self, name: str): - x = self.pm.subset_hook_caller(name, remove_plugins=self.remove_mods) - self.__dict__[name] = x - return x - - class FSCollector(Collector): def __init__( self, @@ -542,42 +531,28 @@ def from_parent(cls, parent, *, fspath, **kw): """The public constructor.""" return super().from_parent(parent=parent, fspath=fspath, **kw) - def _gethookproxy(self, fspath: py.path.local): - # Check if we have the common case of running - # hooks with all conftest.py files. - pm = self.config.pluginmanager - my_conftestmodules = pm._getconftestmodules( - fspath, self.config.getoption("importmode") - ) - remove_mods = pm._conftest_plugins.difference(my_conftestmodules) - if remove_mods: - # One or more conftests are not in use at this fspath. - proxy = FSHookProxy(pm, remove_mods) - else: - # All plugins are active for this fspath. - proxy = self.config.hook - return proxy - def gethookproxy(self, fspath: py.path.local): - raise NotImplementedError() + warnings.warn(FSCOLLECTOR_GETHOOKPROXY_ISINITPATH, stacklevel=2) + return self.session.gethookproxy(fspath) + + def isinitpath(self, path: py.path.local) -> bool: + warnings.warn(FSCOLLECTOR_GETHOOKPROXY_ISINITPATH, stacklevel=2) + return self.session.isinitpath(path) def _recurse(self, direntry: "os.DirEntry[str]") -> bool: if direntry.name == "__pycache__": return False path = py.path.local(direntry.path) - ihook = self._gethookproxy(path.dirpath()) + ihook = self.session.gethookproxy(path.dirpath()) if ihook.pytest_ignore_collect(path=path, config=self.config): return False for pat in self._norecursepatterns: if path.check(fnmatch=pat): return False - ihook = self._gethookproxy(path) + ihook = self.session.gethookproxy(path) ihook.pytest_collect_directory(path=path, parent=self) return True - def isinitpath(self, path: py.path.local) -> bool: - raise NotImplementedError() - def _collectfile( self, path: py.path.local, handle_dupes: bool = True ) -> Sequence[Collector]: @@ -586,8 +561,8 @@ def _collectfile( ), "{!r} is not a file (isdir={!r}, exists={!r}, islink={!r})".format( path, path.isdir(), path.exists(), path.islink() ) - ihook = self.gethookproxy(path) - if not self.isinitpath(path): + ihook = self.session.gethookproxy(path) + if not self.session.isinitpath(path): if ihook.pytest_ignore_collect(path=path, config=self.config): return () diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 0661f340209..820b7e86c91 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -52,6 +52,7 @@ from _pytest.config import ExitCode from _pytest.config import hookimpl from _pytest.config.argparsing import Parser +from _pytest.deprecated import FSCOLLECTOR_GETHOOKPROXY_ISINITPATH from _pytest.deprecated import FUNCARGNAMES from _pytest.fixtures import FuncFixtureInfo from _pytest.main import Session @@ -627,10 +628,12 @@ def setup(self) -> None: self.addfinalizer(func) def gethookproxy(self, fspath: py.path.local): - return super()._gethookproxy(fspath) + warnings.warn(FSCOLLECTOR_GETHOOKPROXY_ISINITPATH, stacklevel=2) + return self.session.gethookproxy(fspath) def isinitpath(self, path: py.path.local) -> bool: - return path in self.session._initialpaths + warnings.warn(FSCOLLECTOR_GETHOOKPROXY_ISINITPATH, stacklevel=2) + return self.session.isinitpath(path) def collect(self) -> Iterable[Union[nodes.Item, nodes.Collector]]: this_path = self.fspath.dirpath() diff --git a/testing/deprecated_test.py b/testing/deprecated_test.py index f4de3b5d9c5..9507d990230 100644 --- a/testing/deprecated_test.py +++ b/testing/deprecated_test.py @@ -1,11 +1,13 @@ import copy import inspect +import warnings from unittest import mock import pytest from _pytest import deprecated from _pytest import nodes from _pytest.config import Config +from _pytest.pytester import Testdir @pytest.mark.filterwarnings("default") @@ -151,3 +153,28 @@ def test_three(): assert 1 ) result = testdir.runpytest("-k", "test_two:", threepass) result.stdout.fnmatch_lines(["*The `-k 'expr:'` syntax*deprecated*"]) + + +def test_fscollector_gethookproxy_isinitpath(testdir: Testdir) -> None: + module = testdir.getmodulecol( + """ + def test_foo(): pass + """, + withinit=True, + ) + assert isinstance(module, pytest.Module) + package = module.parent + assert isinstance(package, pytest.Package) + + with pytest.warns(pytest.PytestDeprecationWarning, match="gethookproxy"): + package.gethookproxy(testdir.tmpdir) + + with pytest.warns(pytest.PytestDeprecationWarning, match="isinitpath"): + package.isinitpath(testdir.tmpdir) + + # The methods on Session are *not* deprecated. + session = module.session + with warnings.catch_warnings(record=True) as rec: + session.gethookproxy(testdir.tmpdir) + session.isinitpath(testdir.tmpdir) + assert len(rec) == 0