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.

The previous methods defined directly on `FSCollector` and `Package` are
deprecated, to be removed in pytest 7.

See commits e2934c3 and
f10ab02 for reference.
  • Loading branch information
bluetech committed Aug 15, 2020
1 parent d426a79 commit eddd993
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 39 deletions.
1 change: 1 addition & 0 deletions changelog/7591.bugfix.rst
@@ -0,0 +1 @@
pylint shouldn't complain anymore about unimplemented abstract methods when inheriting from :ref:`File <non-python tests>`.
3 changes: 3 additions & 0 deletions 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.
5 changes: 5 additions & 0 deletions src/_pytest/deprecated.py
Expand Up @@ -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. "
)
27 changes: 26 additions & 1 deletion src/_pytest/main.py
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
47 changes: 11 additions & 36 deletions src/_pytest/nodes.py
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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]:
Expand All @@ -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 ()

Expand Down
7 changes: 5 additions & 2 deletions src/_pytest/python.py
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down
27 changes: 27 additions & 0 deletions 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")
Expand Down Expand Up @@ -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

0 comments on commit eddd993

Please sign in to comment.