From 4b8e1a17718ae1459604cea128c72d46663ad9bd Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Fri, 14 Aug 2020 14:05:00 +0300 Subject: [PATCH] Revert "Move common code between Session and Package to FSCollector" This reverts commit f10ab021e21a44e2f0fa2be66660c4a6d4b7a61a. The commit was good in that it removed a non-trivial amount of code duplication. However it was done in the wrong layer (nodes.py) and split up a major part of the collection (the filesystem traversal) to a separate class making it harder to understand. We should try to reduce the duplication, but in a more appropriate manner. --- src/_pytest/main.py | 37 +++++++++++++++++++++++++++++++++++++ src/_pytest/nodes.py | 39 --------------------------------------- src/_pytest/python.py | 37 +++++++++++++++++++++++++++++++++++++ 3 files changed, 74 insertions(+), 39 deletions(-) diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 06619023e..f71ef86de 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -523,6 +523,43 @@ class Session(nodes.FSCollector): proxy = self.config.hook return proxy + 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()) + if ihook.pytest_ignore_collect(path=path, config=self.config): + return False + norecursepatterns = self.config.getini("norecursedirs") + for pat in norecursepatterns: + if path.check(fnmatch=pat): + return False + return True + + def _collectfile( + self, path: py.path.local, handle_dupes: bool = True + ) -> Sequence[nodes.Collector]: + assert ( + path.isfile() + ), "{!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): + if ihook.pytest_ignore_collect(path=path, config=self.config): + return () + + if handle_dupes: + keepduplicates = self.config.getoption("keepduplicates") + if not keepduplicates: + duplicate_paths = self.config.pluginmanager._duplicatepaths + if path in duplicate_paths: + return () + else: + duplicate_paths.add(path) + + return ihook.pytest_collect_file(path=path, parent=self) # type: ignore[no-any-return] + @overload def perform_collect( self, args: Optional[Sequence[str]] = ..., genitems: "Literal[True]" = ... diff --git a/src/_pytest/nodes.py b/src/_pytest/nodes.py index addeaf1c2..26fab67fe 100644 --- a/src/_pytest/nodes.py +++ b/src/_pytest/nodes.py @@ -8,7 +8,6 @@ from typing import Iterable from typing import Iterator from typing import List from typing import Optional -from typing import Sequence from typing import Set from typing import Tuple from typing import TypeVar @@ -528,8 +527,6 @@ class FSCollector(Collector): super().__init__(name, parent, config, session, nodeid=nodeid, fspath=fspath) - self._norecursepatterns = self.config.getini("norecursedirs") - @classmethod def from_parent(cls, parent, *, fspath, **kw): """The public constructor.""" @@ -543,42 +540,6 @@ class FSCollector(Collector): 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.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 - return True - - def _collectfile( - self, path: py.path.local, handle_dupes: bool = True - ) -> Sequence[Collector]: - assert ( - path.isfile() - ), "{!r} is not a file (isdir={!r}, exists={!r}, islink={!r})".format( - path, path.isdir(), path.exists(), path.islink() - ) - ihook = self.session.gethookproxy(path) - if not self.session.isinitpath(path): - if ihook.pytest_ignore_collect(path=path, config=self.config): - return () - - if handle_dupes: - keepduplicates = self.config.getoption("keepduplicates") - if not keepduplicates: - duplicate_paths = self.config.pluginmanager._duplicatepaths - if path in duplicate_paths: - return () - else: - duplicate_paths.add(path) - - return ihook.pytest_collect_file(path=path, parent=self) # type: ignore[no-any-return] - class File(FSCollector): """Base class for collecting tests from a file.""" diff --git a/src/_pytest/python.py b/src/_pytest/python.py index ae5108e76..63fa539d2 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -634,6 +634,43 @@ class Package(Module): 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.session.gethookproxy(path.dirpath()) + if ihook.pytest_ignore_collect(path=path, config=self.config): + return False + norecursepatterns = self.config.getini("norecursedirs") + for pat in norecursepatterns: + if path.check(fnmatch=pat): + return False + return True + + def _collectfile( + self, path: py.path.local, handle_dupes: bool = True + ) -> typing.Sequence[nodes.Collector]: + assert ( + path.isfile() + ), "{!r} is not a file (isdir={!r}, exists={!r}, islink={!r})".format( + path, path.isdir(), path.exists(), path.islink() + ) + ihook = self.session.gethookproxy(path) + if not self.session.isinitpath(path): + if ihook.pytest_ignore_collect(path=path, config=self.config): + return () + + if handle_dupes: + keepduplicates = self.config.getoption("keepduplicates") + if not keepduplicates: + duplicate_paths = self.config.pluginmanager._duplicatepaths + if path in duplicate_paths: + return () + else: + duplicate_paths.add(path) + + return ihook.pytest_collect_file(path=path, parent=self) # type: ignore[no-any-return] + def collect(self) -> Iterable[Union[nodes.Item, nodes.Collector]]: this_path = self.fspath.dirpath() init_module = this_path.join("__init__.py")