Skip to content

Commit

Permalink
Only define gethookproxy, isinitpath on Session
Browse files Browse the repository at this point in the history
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.

See commits e2934c3 and
f10ab02 for reference.
  • Loading branch information
bluetech committed Aug 15, 2020
1 parent d426a79 commit 0da3385
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 45 deletions.
1 change: 1 addition & 0 deletions changelog/7591.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
pylint shouldn't complain anymore about unimplemented abstract methods when inheriting from :ref:`File <non-python tests>`.
27 changes: 26 additions & 1 deletion src/_pytest/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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."""

Expand Down Expand Up @@ -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(
Expand Down
42 changes: 4 additions & 38 deletions src/_pytest/nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
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 NODE_USE_FROM_PARENT
from _pytest.fixtures import FixtureDef
from _pytest.fixtures import FixtureLookupError
Expand Down Expand Up @@ -495,17 +494,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,
Expand Down Expand Up @@ -542,42 +530,20 @@ 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()

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]:
Expand All @@ -586,8 +552,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 ()

Expand Down
6 changes: 0 additions & 6 deletions src/_pytest/python.py
Original file line number Diff line number Diff line change
Expand Up @@ -626,12 +626,6 @@ def setup(self) -> None:
func = partial(_call_with_optional_argument, teardown_module, self.obj)
self.addfinalizer(func)

def gethookproxy(self, fspath: py.path.local):
return super()._gethookproxy(fspath)

def isinitpath(self, path: py.path.local) -> bool:
return path in self.session._initialpaths

def collect(self) -> Iterable[Union[nodes.Item, nodes.Collector]]:
this_path = self.fspath.dirpath()
init_module = this_path.join("__init__.py")
Expand Down

0 comments on commit 0da3385

Please sign in to comment.