From d121d7c917fda743aa4ce4459eb0beac5e3ce3c7 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Sat, 15 Aug 2020 11:58:44 +0300 Subject: [PATCH 01/18] main: inline Session._perform_collect() into perform_collect() It doesn't add much, mostly just an eye sore, particularly with the overloads. --- src/_pytest/main.py | 86 ++++++++++++++++++++------------------------- 1 file changed, 38 insertions(+), 48 deletions(-) diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 479f34cdc..7281ff9d1 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -552,65 +552,55 @@ class Session(nodes.FSCollector): in which case the return value contains these collectors unexpanded, and ``session.items`` is empty. """ + if args is None: + args = self.config.args + + self.trace("perform_collect", self, args) + self.trace.root.indent += 1 + + self._notfound = [] # type: List[Tuple[str, NoMatch]] + self._initial_parts = [] # type: List[Tuple[py.path.local, List[str]]] + self.items = items = [] # type: List[nodes.Item] + hook = self.config.hook + try: - items = self._perform_collect(args, genitems) + initialpaths = [] # type: List[py.path.local] + for arg in args: + fspath, parts = resolve_collection_argument( + self.config.invocation_dir, arg, as_pypath=self.config.option.pyargs + ) + self._initial_parts.append((fspath, parts)) + initialpaths.append(fspath) + self._initialpaths = frozenset(initialpaths) + rep = collect_one_node(self) + self.ihook.pytest_collectreport(report=rep) + self.trace.root.indent -= 1 + if self._notfound: + errors = [] + for arg, exc in self._notfound: + line = "(no name {!r} in any of {!r})".format(arg, exc.args[0]) + errors.append("not found: {}\n{}".format(arg, line)) + raise UsageError(*errors) + if not genitems: + # Type ignored because genitems=False is only used by tests. We don't + # want to change the type of `session.items` for this case. + items = rep.result # type: ignore[assignment] + else: + if rep.passed: + for node in rep.result: + self.items.extend(self.genitems(node)) + self.config.pluginmanager.check_pending() hook.pytest_collection_modifyitems( session=self, config=self.config, items=items ) finally: hook.pytest_collection_finish(session=self) + self.testscollected = len(items) return items - @overload - def _perform_collect( - self, args: Optional[Sequence[str]], genitems: "Literal[True]" - ) -> List[nodes.Item]: - ... - - @overload # noqa: F811 - def _perform_collect( # noqa: F811 - self, args: Optional[Sequence[str]], genitems: bool - ) -> Union[List[Union[nodes.Item]], List[Union[nodes.Item, nodes.Collector]]]: - ... - - def _perform_collect( # noqa: F811 - self, args: Optional[Sequence[str]], genitems: bool - ) -> Union[List[Union[nodes.Item]], List[Union[nodes.Item, nodes.Collector]]]: - if args is None: - args = self.config.args - self.trace("perform_collect", self, args) - self.trace.root.indent += 1 - self._notfound = [] # type: List[Tuple[str, NoMatch]] - initialpaths = [] # type: List[py.path.local] - self._initial_parts = [] # type: List[Tuple[py.path.local, List[str]]] - self.items = items = [] # type: List[nodes.Item] - for arg in args: - fspath, parts = resolve_collection_argument( - self.config.invocation_dir, arg, as_pypath=self.config.option.pyargs - ) - self._initial_parts.append((fspath, parts)) - initialpaths.append(fspath) - self._initialpaths = frozenset(initialpaths) - rep = collect_one_node(self) - self.ihook.pytest_collectreport(report=rep) - self.trace.root.indent -= 1 - if self._notfound: - errors = [] - for arg, exc in self._notfound: - line = "(no name {!r} in any of {!r})".format(arg, exc.args[0]) - errors.append("not found: {}\n{}".format(arg, line)) - raise UsageError(*errors) - if not genitems: - return rep.result - else: - if rep.passed: - for node in rep.result: - self.items.extend(self.genitems(node)) - return items - def collect(self) -> Iterator[Union[nodes.Item, nodes.Collector]]: for fspath, parts in self._initial_parts: self.trace("processing argument", (fspath, parts)) From 32edc4655cc3cff79a935fbd4e9f474fa3001ffd Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Sat, 15 Aug 2020 12:38:57 +0300 Subject: [PATCH 02/18] main: inline Session._matchnodes() into Session.matchnodes() Similar to the previous commit, this makes things more straightforward. --- src/_pytest/main.py | 85 ++++++++++++++++++++++----------------------- 1 file changed, 42 insertions(+), 43 deletions(-) diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 7281ff9d1..06619023e 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -709,52 +709,51 @@ class Session(nodes.FSCollector): ) -> Sequence[Union[nodes.Item, nodes.Collector]]: self.trace("matchnodes", matching, names) self.trace.root.indent += 1 - nodes = self._matchnodes(matching, names) - num = len(nodes) - self.trace("matchnodes finished -> ", num, "nodes") - self.trace.root.indent -= 1 - if num == 0: - raise NoMatch(matching, names[:1]) - return nodes - def _matchnodes( - self, matching: Sequence[Union[nodes.Item, nodes.Collector]], names: List[str], - ) -> Sequence[Union[nodes.Item, nodes.Collector]]: if not matching or not names: - return matching - name = names[0] - assert name - nextnames = names[1:] - resultnodes = [] # type: List[Union[nodes.Item, nodes.Collector]] - for node in matching: - if isinstance(node, nodes.Item): - if not names: - resultnodes.append(node) - continue - assert isinstance(node, nodes.Collector) - key = (type(node), node.nodeid) - if key in self._collection_node_cache3: - rep = self._collection_node_cache3[key] - else: - rep = collect_one_node(node) - self._collection_node_cache3[key] = rep - if rep.passed: - has_matched = False - for x in rep.result: - # TODO: Remove parametrized workaround once collection structure contains parametrization. - if x.name == name or x.name.split("[")[0] == name: + result = matching + else: + name = names[0] + assert name + nextnames = names[1:] + resultnodes = [] # type: List[Union[nodes.Item, nodes.Collector]] + for node in matching: + if isinstance(node, nodes.Item): + if not names: + resultnodes.append(node) + continue + assert isinstance(node, nodes.Collector) + key = (type(node), node.nodeid) + if key in self._collection_node_cache3: + rep = self._collection_node_cache3[key] + else: + rep = collect_one_node(node) + self._collection_node_cache3[key] = rep + if rep.passed: + has_matched = False + for x in rep.result: + # TODO: Remove parametrized workaround once collection structure contains parametrization. + if x.name == name or x.name.split("[")[0] == name: + resultnodes.extend(self.matchnodes([x], nextnames)) + has_matched = True + # XXX Accept IDs that don't have "()" for class instances. + if not has_matched and len(rep.result) == 1 and x.name == "()": + nextnames.insert(0, name) resultnodes.extend(self.matchnodes([x], nextnames)) - has_matched = True - # XXX Accept IDs that don't have "()" for class instances. - if not has_matched and len(rep.result) == 1 and x.name == "()": - nextnames.insert(0, name) - resultnodes.extend(self.matchnodes([x], nextnames)) - else: - # Report collection failures here to avoid failing to run some test - # specified in the command line because the module could not be - # imported (#134). - node.ihook.pytest_collectreport(report=rep) - return resultnodes + else: + # Report collection failures here to avoid failing to run some test + # specified in the command line because the module could not be + # imported (#134). + node.ihook.pytest_collectreport(report=rep) + result = resultnodes + + self.trace("matchnodes finished -> ", len(result), "nodes") + self.trace.root.indent -= 1 + + if not result: + raise NoMatch(matching, names[:1]) + else: + return result def genitems( self, node: Union[nodes.Item, nodes.Collector] From 4b8e1a17718ae1459604cea128c72d46663ad9bd Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Fri, 14 Aug 2020 14:05:00 +0300 Subject: [PATCH 03/18] 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") From 57aca11d4a468cf9a40b93dcb3bdd0f253ead017 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Thu, 20 Aug 2020 23:51:08 +0300 Subject: [PATCH 04/18] hookspec: type annotate parent argument to pytest_collect_file --- src/_pytest/doctest.py | 3 ++- src/_pytest/hookspec.py | 8 +++++--- src/_pytest/python.py | 4 +++- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/_pytest/doctest.py b/src/_pytest/doctest.py index acedd389b..c744bb369 100644 --- a/src/_pytest/doctest.py +++ b/src/_pytest/doctest.py @@ -32,6 +32,7 @@ from _pytest.compat import TYPE_CHECKING from _pytest.config import Config from _pytest.config.argparsing import Parser from _pytest.fixtures import FixtureRequest +from _pytest.nodes import Collector from _pytest.outcomes import OutcomeException from _pytest.pathlib import import_path from _pytest.python_api import approx @@ -118,7 +119,7 @@ def pytest_unconfigure() -> None: def pytest_collect_file( - path: py.path.local, parent + path: py.path.local, parent: Collector, ) -> Optional[Union["DoctestModule", "DoctestTextfile"]]: config = parent.config if path.ext == ".py": diff --git a/src/_pytest/hookspec.py b/src/_pytest/hookspec.py index 8e6be7d56..d4aaba1ec 100644 --- a/src/_pytest/hookspec.py +++ b/src/_pytest/hookspec.py @@ -274,10 +274,12 @@ def pytest_ignore_collect(path: py.path.local, config: "Config") -> Optional[boo """ -def pytest_collect_file(path: py.path.local, parent) -> "Optional[Collector]": - """Return collection Node or None for the given path. +def pytest_collect_file( + path: py.path.local, parent: "Collector" +) -> "Optional[Collector]": + """Create a Collector for the given path, or None if not relevant. - Any new node needs to have the specified ``parent`` as a parent. + The new node needs to have the specified ``parent`` as a parent. :param py.path.local path: The path to collect. """ diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 63fa539d2..7aa5b4222 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -184,7 +184,9 @@ def pytest_pyfunc_call(pyfuncitem: "Function") -> Optional[object]: return True -def pytest_collect_file(path: py.path.local, parent) -> Optional["Module"]: +def pytest_collect_file( + path: py.path.local, parent: nodes.Collector +) -> Optional["Module"]: ext = path.ext if ext == ".py": if not parent.session.isinitpath(path): From 0b41b79dcb985f2fbb56772ddedcfc6f0e210748 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Fri, 21 Aug 2020 10:47:29 +0300 Subject: [PATCH 05/18] main: better solution to a type ignore --- src/_pytest/main.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/_pytest/main.py b/src/_pytest/main.py index f71ef86de..f31defe67 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -597,10 +597,11 @@ class Session(nodes.FSCollector): self._notfound = [] # type: List[Tuple[str, NoMatch]] self._initial_parts = [] # type: List[Tuple[py.path.local, List[str]]] - self.items = items = [] # type: List[nodes.Item] + self.items = [] # type: List[nodes.Item] hook = self.config.hook + items = self.items # type: Sequence[Union[nodes.Item, nodes.Collector]] try: initialpaths = [] # type: List[py.path.local] for arg in args: @@ -620,9 +621,7 @@ class Session(nodes.FSCollector): errors.append("not found: {}\n{}".format(arg, line)) raise UsageError(*errors) if not genitems: - # Type ignored because genitems=False is only used by tests. We don't - # want to change the type of `session.items` for this case. - items = rep.result # type: ignore[assignment] + items = rep.result else: if rep.passed: for node in rep.result: From 5356a0979a4b18a111cf5ad4c53484864369652e Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Fri, 21 Aug 2020 13:39:46 +0300 Subject: [PATCH 06/18] main: small code simplification in matchnodes --- src/_pytest/main.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/_pytest/main.py b/src/_pytest/main.py index f31defe67..d214f0c29 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -656,7 +656,7 @@ class Session(nodes.FSCollector): self._collection_pkg_roots.clear() def _collect( - self, argpath: py.path.local, names: List[str] + self, argpath: py.path.local, names: Sequence[str] ) -> Iterator[Union[nodes.Item, nodes.Collector]]: from _pytest.python import Package @@ -741,7 +741,9 @@ class Session(nodes.FSCollector): yield from m def matchnodes( - self, matching: Sequence[Union[nodes.Item, nodes.Collector]], names: List[str], + self, + matching: Sequence[Union[nodes.Item, nodes.Collector]], + names: Sequence[str], ) -> Sequence[Union[nodes.Item, nodes.Collector]]: self.trace("matchnodes", matching, names) self.trace.root.indent += 1 @@ -751,7 +753,6 @@ class Session(nodes.FSCollector): else: name = names[0] assert name - nextnames = names[1:] resultnodes = [] # type: List[Union[nodes.Item, nodes.Collector]] for node in matching: if isinstance(node, nodes.Item): @@ -770,12 +771,11 @@ class Session(nodes.FSCollector): for x in rep.result: # TODO: Remove parametrized workaround once collection structure contains parametrization. if x.name == name or x.name.split("[")[0] == name: - resultnodes.extend(self.matchnodes([x], nextnames)) + resultnodes.extend(self.matchnodes([x], names[1:])) has_matched = True # XXX Accept IDs that don't have "()" for class instances. if not has_matched and len(rep.result) == 1 and x.name == "()": - nextnames.insert(0, name) - resultnodes.extend(self.matchnodes([x], nextnames)) + resultnodes.extend(self.matchnodes([x], names)) else: # Report collection failures here to avoid failing to run some test # specified in the command line because the module could not be From 1b2de81404172d8b48d93e192b59e4c397e08e8d Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Fri, 21 Aug 2020 13:50:13 +0300 Subject: [PATCH 07/18] main: remove unneeded condition in matchnodes The end result in the `else` branch is the same, but flows naturally. --- src/_pytest/main.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/_pytest/main.py b/src/_pytest/main.py index d214f0c29..71c230077 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -748,7 +748,7 @@ class Session(nodes.FSCollector): self.trace("matchnodes", matching, names) self.trace.root.indent += 1 - if not matching or not names: + if not names: result = matching else: name = names[0] From adaec2da90c006e4c3317e49b8cbb3f5b6a8dc72 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Fri, 21 Aug 2020 13:53:05 +0300 Subject: [PATCH 08/18] main: remove impossible condition in matchnodes Already covered in a condition above. --- src/_pytest/main.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 71c230077..602f5fbd2 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -756,8 +756,6 @@ class Session(nodes.FSCollector): resultnodes = [] # type: List[Union[nodes.Item, nodes.Collector]] for node in matching: if isinstance(node, nodes.Item): - if not names: - resultnodes.append(node) continue assert isinstance(node, nodes.Collector) key = (type(node), node.nodeid) From a2c919d350e5f52287a42187853ed0e090168fe1 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Fri, 21 Aug 2020 14:19:14 +0300 Subject: [PATCH 09/18] main: refactor a bit to reduce indentation --- src/_pytest/main.py | 60 +++++++++++++++++++++------------------------ 1 file changed, 28 insertions(+), 32 deletions(-) diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 602f5fbd2..1f7340720 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -748,38 +748,34 @@ class Session(nodes.FSCollector): self.trace("matchnodes", matching, names) self.trace.root.indent += 1 - if not names: - result = matching - else: - name = names[0] - assert name - resultnodes = [] # type: List[Union[nodes.Item, nodes.Collector]] - for node in matching: - if isinstance(node, nodes.Item): - continue - assert isinstance(node, nodes.Collector) - key = (type(node), node.nodeid) - if key in self._collection_node_cache3: - rep = self._collection_node_cache3[key] - else: - rep = collect_one_node(node) - self._collection_node_cache3[key] = rep - if rep.passed: - has_matched = False - for x in rep.result: - # TODO: Remove parametrized workaround once collection structure contains parametrization. - if x.name == name or x.name.split("[")[0] == name: - resultnodes.extend(self.matchnodes([x], names[1:])) - has_matched = True - # XXX Accept IDs that don't have "()" for class instances. - if not has_matched and len(rep.result) == 1 and x.name == "()": - resultnodes.extend(self.matchnodes([x], names)) - else: - # Report collection failures here to avoid failing to run some test - # specified in the command line because the module could not be - # imported (#134). - node.ihook.pytest_collectreport(report=rep) - result = resultnodes + result = [] + for node in matching: + if not names: + result.append(node) + continue + if not isinstance(node, nodes.Collector): + continue + key = (type(node), node.nodeid) + if key in self._collection_node_cache3: + rep = self._collection_node_cache3[key] + else: + rep = collect_one_node(node) + self._collection_node_cache3[key] = rep + if rep.passed: + has_matched = False + for x in rep.result: + # TODO: Remove parametrized workaround once collection structure contains parametrization. + if x.name == names[0] or x.name.split("[")[0] == names[0]: + result.extend(self.matchnodes([x], names[1:])) + has_matched = True + # XXX Accept IDs that don't have "()" for class instances. + if not has_matched and len(rep.result) == 1 and x.name == "()": + result.extend(self.matchnodes([x], names)) + else: + # Report collection failures here to avoid failing to run some test + # specified in the command line because the module could not be + # imported (#134). + node.ihook.pytest_collectreport(report=rep) self.trace("matchnodes finished -> ", len(result), "nodes") self.trace.root.indent -= 1 From 0c6b2f39b2fa4532813b5995038af6d143405c88 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Fri, 21 Aug 2020 14:45:03 +0300 Subject: [PATCH 10/18] main: move NoMatch raising to _collect() This is a more sensible interface for matchnodes. This also fixes a sort-of bug where a recursive call to matchnodes raises NoMatch which would terminate the entire tree, even if other branches may find a match. Though I don't think it's actually possible. --- src/_pytest/main.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 1f7340720..8d898426c 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -724,6 +724,8 @@ class Session(nodes.FSCollector): if col: self._collection_node_cache1[argpath] = col m = self.matchnodes(col, names) + if not m: + raise NoMatch(col) # If __init__.py was the only file requested, then the matched node will be # the corresponding Package, and the first yielded item will be the __init__ # Module itself, so just use that. If this special case isn't taken, then all @@ -780,10 +782,7 @@ class Session(nodes.FSCollector): self.trace("matchnodes finished -> ", len(result), "nodes") self.trace.root.indent -= 1 - if not result: - raise NoMatch(matching, names[:1]) - else: - return result + return result def genitems( self, node: Union[nodes.Item, nodes.Collector] From 841521fedb9e04a67479b286773a38f0fc4c1c90 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Fri, 21 Aug 2020 15:05:54 +0300 Subject: [PATCH 11/18] main: only perform one recursive matchnodes call per node --- src/_pytest/main.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 8d898426c..e47752f91 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -764,15 +764,16 @@ class Session(nodes.FSCollector): rep = collect_one_node(node) self._collection_node_cache3[key] = rep if rep.passed: - has_matched = False + submatching = [] for x in rep.result: # TODO: Remove parametrized workaround once collection structure contains parametrization. if x.name == names[0] or x.name.split("[")[0] == names[0]: - result.extend(self.matchnodes([x], names[1:])) - has_matched = True + submatching.append(x) + if submatching: + result.extend(self.matchnodes(submatching, names[1:])) # XXX Accept IDs that don't have "()" for class instances. - if not has_matched and len(rep.result) == 1 and x.name == "()": - result.extend(self.matchnodes([x], names)) + elif len(rep.result) == 1 and rep.result[0].name == "()": + result.extend(self.matchnodes(rep.result, names)) else: # Report collection failures here to avoid failing to run some test # specified in the command line because the module could not be From c2256189ae2bd1cd0b5e2c34b5861b6f9255c028 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Fri, 21 Aug 2020 16:50:28 +0300 Subject: [PATCH 12/18] main: make matchnodes non-recursive It's a little more sane this way. --- src/_pytest/main.py | 68 +++++++++++++++++++++++---------------------- 1 file changed, 35 insertions(+), 33 deletions(-) diff --git a/src/_pytest/main.py b/src/_pytest/main.py index e47752f91..7b5272666 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -747,42 +747,44 @@ class Session(nodes.FSCollector): matching: Sequence[Union[nodes.Item, nodes.Collector]], names: Sequence[str], ) -> Sequence[Union[nodes.Item, nodes.Collector]]: - self.trace("matchnodes", matching, names) - self.trace.root.indent += 1 - result = [] - for node in matching: - if not names: - result.append(node) - continue - if not isinstance(node, nodes.Collector): - continue - key = (type(node), node.nodeid) - if key in self._collection_node_cache3: - rep = self._collection_node_cache3[key] - else: - rep = collect_one_node(node) - self._collection_node_cache3[key] = rep - if rep.passed: - submatching = [] - for x in rep.result: - # TODO: Remove parametrized workaround once collection structure contains parametrization. - if x.name == names[0] or x.name.split("[")[0] == names[0]: - submatching.append(x) - if submatching: - result.extend(self.matchnodes(submatching, names[1:])) - # XXX Accept IDs that don't have "()" for class instances. - elif len(rep.result) == 1 and rep.result[0].name == "()": - result.extend(self.matchnodes(rep.result, names)) - else: - # Report collection failures here to avoid failing to run some test - # specified in the command line because the module could not be - # imported (#134). - node.ihook.pytest_collectreport(report=rep) + work = [(matching, names)] + while work: + self.trace("matchnodes", matching, names) + self.trace.root.indent += 1 - self.trace("matchnodes finished -> ", len(result), "nodes") - self.trace.root.indent -= 1 + matching, names = work.pop() + for node in matching: + if not names: + result.append(node) + continue + if not isinstance(node, nodes.Collector): + continue + key = (type(node), node.nodeid) + if key in self._collection_node_cache3: + rep = self._collection_node_cache3[key] + else: + rep = collect_one_node(node) + self._collection_node_cache3[key] = rep + if rep.passed: + submatching = [] + for x in rep.result: + # TODO: Remove parametrized workaround once collection structure contains parametrization. + if x.name == names[0] or x.name.split("[")[0] == names[0]: + submatching.append(x) + if submatching: + work.append((submatching, names[1:])) + # XXX Accept IDs that don't have "()" for class instances. + elif len(rep.result) == 1 and rep.result[0].name == "()": + work.append((rep.result, names)) + else: + # Report collection failures here to avoid failing to run some test + # specified in the command line because the module could not be + # imported (#134). + node.ihook.pytest_collectreport(report=rep) + self.trace("matchnodes finished -> ", len(result), "nodes") + self.trace.root.indent -= 1 return result def genitems( From c4fd4616176a685f25ba79249cd78574b4a6fba2 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Fri, 21 Aug 2020 17:03:52 +0300 Subject: [PATCH 13/18] main: better name for _collection_node_cache3 The weird name was due to f3967333a145d8a793a0ab53ac5e0cb0b6c87cac, now that I understand it a bit better can give it a more descriptive name. --- src/_pytest/main.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 7b5272666..ec069808b 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -454,7 +454,10 @@ class Session(nodes.FSCollector): self._collection_node_cache2 = ( {} ) # type: Dict[Tuple[Type[nodes.Collector], py.path.local], nodes.Collector] - self._collection_node_cache3 = ( + + # Keep track of any collected collectors in matchnodes paths, so they + # are not collected more than once. + self._collection_matchnodes_cache = ( {} ) # type: Dict[Tuple[Type[nodes.Collector], str], CollectReport] @@ -652,7 +655,7 @@ class Session(nodes.FSCollector): self.trace.root.indent -= 1 self._collection_node_cache1.clear() self._collection_node_cache2.clear() - self._collection_node_cache3.clear() + self._collection_matchnodes_cache.clear() self._collection_pkg_roots.clear() def _collect( @@ -677,7 +680,6 @@ class Session(nodes.FSCollector): if col: if isinstance(col[0], Package): self._collection_pkg_roots[str(parent)] = col[0] - # Always store a list in the cache, matchnodes expects it. self._collection_node_cache1[col[0].fspath] = [col[0]] # If it's a directory argument, recurse and look for any Subpackages. @@ -761,11 +763,11 @@ class Session(nodes.FSCollector): if not isinstance(node, nodes.Collector): continue key = (type(node), node.nodeid) - if key in self._collection_node_cache3: - rep = self._collection_node_cache3[key] + if key in self._collection_matchnodes_cache: + rep = self._collection_matchnodes_cache[key] else: rep = collect_one_node(node) - self._collection_node_cache3[key] = rep + self._collection_matchnodes_cache[key] = rep if rep.passed: submatching = [] for x in rep.result: From eec13ba57ed812fe094b2ba12b57cabd08739a34 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Fri, 21 Aug 2020 19:29:13 +0300 Subject: [PATCH 14/18] main: get rid of NoMatch Things are easier to understand without the weird exception. --- src/_pytest/main.py | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/src/_pytest/main.py b/src/_pytest/main.py index ec069808b..274266faf 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -402,10 +402,6 @@ class FSHookProxy: return x -class NoMatch(Exception): - """Matching cannot locate matching names.""" - - class Interrupted(KeyboardInterrupt): """Signals that the test run was interrupted.""" @@ -598,7 +594,7 @@ class Session(nodes.FSCollector): self.trace("perform_collect", self, args) self.trace.root.indent += 1 - self._notfound = [] # type: List[Tuple[str, NoMatch]] + self._notfound = [] # type: List[Tuple[str, Sequence[nodes.Collector]]] self._initial_parts = [] # type: List[Tuple[py.path.local, List[str]]] self.items = [] # type: List[nodes.Item] @@ -619,8 +615,8 @@ class Session(nodes.FSCollector): self.trace.root.indent -= 1 if self._notfound: errors = [] - for arg, exc in self._notfound: - line = "(no name {!r} in any of {!r})".format(arg, exc.args[0]) + for arg, cols in self._notfound: + line = "(no name {!r} in any of {!r})".format(arg, cols) errors.append("not found: {}\n{}".format(arg, line)) raise UsageError(*errors) if not genitems: @@ -644,14 +640,7 @@ class Session(nodes.FSCollector): for fspath, parts in self._initial_parts: self.trace("processing argument", (fspath, parts)) self.trace.root.indent += 1 - try: - yield from self._collect(fspath, parts) - except NoMatch as exc: - report_arg = "::".join((str(fspath), *parts)) - # we are inside a make_report hook so - # we cannot directly pass through the exception - self._notfound.append((report_arg, exc)) - + yield from self._collect(fspath, parts) self.trace.root.indent -= 1 self._collection_node_cache1.clear() self._collection_node_cache2.clear() @@ -727,7 +716,10 @@ class Session(nodes.FSCollector): self._collection_node_cache1[argpath] = col m = self.matchnodes(col, names) if not m: - raise NoMatch(col) + report_arg = "::".join((str(argpath), *names)) + self._notfound.append((report_arg, col)) + return + # If __init__.py was the only file requested, then the matched node will be # the corresponding Package, and the first yielded item will be the __init__ # Module itself, so just use that. If this special case isn't taken, then all From d0e8b71404efc19554dbe5b38a226c31b7e281b9 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Sat, 22 Aug 2020 11:15:10 +0300 Subject: [PATCH 15/18] main: inline _collect() into collect() This removes an unhelpful level of indirection and enables some upcoming upcoming simplifications. --- src/_pytest/main.py | 183 ++++++++++++++++++++++---------------------- 1 file changed, 91 insertions(+), 92 deletions(-) diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 274266faf..a581cbe23 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -637,105 +637,104 @@ class Session(nodes.FSCollector): return items def collect(self) -> Iterator[Union[nodes.Item, nodes.Collector]]: - for fspath, parts in self._initial_parts: - self.trace("processing argument", (fspath, parts)) + from _pytest.python import Package + + for argpath, names in self._initial_parts: + self.trace("processing argument", (argpath, names)) self.trace.root.indent += 1 - yield from self._collect(fspath, parts) + + # Start with a Session root, and delve to argpath item (dir or file) + # and stack all Packages found on the way. + # No point in finding packages when collecting doctests. + if not self.config.getoption("doctestmodules", False): + pm = self.config.pluginmanager + for parent in reversed(argpath.parts()): + if pm._confcutdir and pm._confcutdir.relto(parent): + break + + if parent.isdir(): + pkginit = parent.join("__init__.py") + if pkginit.isfile(): + if pkginit not in self._collection_node_cache1: + col = self._collectfile(pkginit, handle_dupes=False) + if col: + if isinstance(col[0], Package): + self._collection_pkg_roots[str(parent)] = col[0] + self._collection_node_cache1[col[0].fspath] = [ + col[0] + ] + + # If it's a directory argument, recurse and look for any Subpackages. + # Let the Package collector deal with subnodes, don't collect here. + if argpath.check(dir=1): + assert not names, "invalid arg {!r}".format((argpath, names)) + + seen_dirs = set() # type: Set[py.path.local] + for direntry in visit(str(argpath), self._recurse): + if not direntry.is_file(): + continue + + path = py.path.local(direntry.path) + dirpath = path.dirpath() + + if dirpath not in seen_dirs: + # Collect packages first. + seen_dirs.add(dirpath) + pkginit = dirpath.join("__init__.py") + if pkginit.exists(): + for x in self._collectfile(pkginit): + yield x + if isinstance(x, Package): + self._collection_pkg_roots[str(dirpath)] = x + if str(dirpath) in self._collection_pkg_roots: + # Do not collect packages here. + continue + + for x in self._collectfile(path): + key = (type(x), x.fspath) + if key in self._collection_node_cache2: + yield self._collection_node_cache2[key] + else: + self._collection_node_cache2[key] = x + yield x + else: + assert argpath.check(file=1) + + if argpath in self._collection_node_cache1: + col = self._collection_node_cache1[argpath] + else: + collect_root = self._collection_pkg_roots.get(argpath.dirname, self) + col = collect_root._collectfile(argpath, handle_dupes=False) + if col: + self._collection_node_cache1[argpath] = col + m = self.matchnodes(col, names) + if not m: + report_arg = "::".join((str(argpath), *names)) + self._notfound.append((report_arg, col)) + continue + + # If __init__.py was the only file requested, then the matched node will be + # the corresponding Package, and the first yielded item will be the __init__ + # Module itself, so just use that. If this special case isn't taken, then all + # the files in the package will be yielded. + if argpath.basename == "__init__.py": + assert isinstance(m[0], nodes.Collector) + try: + yield next(iter(m[0].collect())) + except StopIteration: + # The package collects nothing with only an __init__.py + # file in it, which gets ignored by the default + # "python_files" option. + pass + continue + yield from m + self.trace.root.indent -= 1 self._collection_node_cache1.clear() self._collection_node_cache2.clear() self._collection_matchnodes_cache.clear() self._collection_pkg_roots.clear() - def _collect( - self, argpath: py.path.local, names: Sequence[str] - ) -> Iterator[Union[nodes.Item, nodes.Collector]]: - from _pytest.python import Package - - # Start with a Session root, and delve to argpath item (dir or file) - # and stack all Packages found on the way. - # No point in finding packages when collecting doctests. - if not self.config.getoption("doctestmodules", False): - pm = self.config.pluginmanager - for parent in reversed(argpath.parts()): - if pm._confcutdir and pm._confcutdir.relto(parent): - break - - if parent.isdir(): - pkginit = parent.join("__init__.py") - if pkginit.isfile(): - if pkginit not in self._collection_node_cache1: - col = self._collectfile(pkginit, handle_dupes=False) - if col: - if isinstance(col[0], Package): - self._collection_pkg_roots[str(parent)] = col[0] - self._collection_node_cache1[col[0].fspath] = [col[0]] - - # If it's a directory argument, recurse and look for any Subpackages. - # Let the Package collector deal with subnodes, don't collect here. - if argpath.check(dir=1): - assert not names, "invalid arg {!r}".format((argpath, names)) - - seen_dirs = set() # type: Set[py.path.local] - for direntry in visit(str(argpath), self._recurse): - if not direntry.is_file(): - continue - - path = py.path.local(direntry.path) - dirpath = path.dirpath() - - if dirpath not in seen_dirs: - # Collect packages first. - seen_dirs.add(dirpath) - pkginit = dirpath.join("__init__.py") - if pkginit.exists(): - for x in self._collectfile(pkginit): - yield x - if isinstance(x, Package): - self._collection_pkg_roots[str(dirpath)] = x - if str(dirpath) in self._collection_pkg_roots: - # Do not collect packages here. - continue - - for x in self._collectfile(path): - key = (type(x), x.fspath) - if key in self._collection_node_cache2: - yield self._collection_node_cache2[key] - else: - self._collection_node_cache2[key] = x - yield x - else: - assert argpath.check(file=1) - - if argpath in self._collection_node_cache1: - col = self._collection_node_cache1[argpath] - else: - collect_root = self._collection_pkg_roots.get(argpath.dirname, self) - col = collect_root._collectfile(argpath, handle_dupes=False) - if col: - self._collection_node_cache1[argpath] = col - m = self.matchnodes(col, names) - if not m: - report_arg = "::".join((str(argpath), *names)) - self._notfound.append((report_arg, col)) - return - - # If __init__.py was the only file requested, then the matched node will be - # the corresponding Package, and the first yielded item will be the __init__ - # Module itself, so just use that. If this special case isn't taken, then all - # the files in the package will be yielded. - if argpath.basename == "__init__.py": - assert isinstance(m[0], nodes.Collector) - try: - yield next(iter(m[0].collect())) - except StopIteration: - # The package collects nothing with only an __init__.py - # file in it, which gets ignored by the default - # "python_files" option. - pass - return - yield from m - def matchnodes( self, matching: Sequence[Union[nodes.Item, nodes.Collector]], From c867452488659729c2caef82cd323c79a2a19b08 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Sat, 22 Aug 2020 11:35:54 +0300 Subject: [PATCH 16/18] main: inline matchnodes() into collect() Now all of the logic is in one place and may be simplified and refactored in more sensible way. --- src/_pytest/main.py | 101 ++++++++++++++++++++++---------------------- 1 file changed, 51 insertions(+), 50 deletions(-) diff --git a/src/_pytest/main.py b/src/_pytest/main.py index a581cbe23..69ea46a40 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -707,8 +707,53 @@ class Session(nodes.FSCollector): col = collect_root._collectfile(argpath, handle_dupes=False) if col: self._collection_node_cache1[argpath] = col - m = self.matchnodes(col, names) - if not m: + + matching = [] + work = [ + (col, names) + ] # type: List[Tuple[Sequence[Union[nodes.Item, nodes.Collector]], Sequence[str]]] + while work: + self.trace("matchnodes", col, names) + self.trace.root.indent += 1 + + matchnodes, matchnames = work.pop() + for node in matchnodes: + if not matchnames: + matching.append(node) + continue + if not isinstance(node, nodes.Collector): + continue + key = (type(node), node.nodeid) + if key in self._collection_matchnodes_cache: + rep = self._collection_matchnodes_cache[key] + else: + rep = collect_one_node(node) + self._collection_matchnodes_cache[key] = rep + if rep.passed: + submatchnodes = [] + for r in rep.result: + # TODO: Remove parametrized workaround once collection structure contains + # parametrization. + if ( + r.name == matchnames[0] + or r.name.split("[")[0] == matchnames[0] + ): + submatchnodes.append(r) + if submatchnodes: + work.append((submatchnodes, matchnames[1:])) + # XXX Accept IDs that don't have "()" for class instances. + elif len(rep.result) == 1 and rep.result[0].name == "()": + work.append((rep.result, matchnames)) + else: + # Report collection failures here to avoid failing to run some test + # specified in the command line because the module could not be + # imported (#134). + node.ihook.pytest_collectreport(report=rep) + + self.trace("matchnodes finished -> ", len(matching), "nodes") + self.trace.root.indent -= 1 + + if not matching: report_arg = "::".join((str(argpath), *names)) self._notfound.append((report_arg, col)) continue @@ -718,16 +763,17 @@ class Session(nodes.FSCollector): # Module itself, so just use that. If this special case isn't taken, then all # the files in the package will be yielded. if argpath.basename == "__init__.py": - assert isinstance(m[0], nodes.Collector) + assert isinstance(matching[0], nodes.Collector) try: - yield next(iter(m[0].collect())) + yield next(iter(matching[0].collect())) except StopIteration: # The package collects nothing with only an __init__.py # file in it, which gets ignored by the default # "python_files" option. pass continue - yield from m + + yield from matching self.trace.root.indent -= 1 self._collection_node_cache1.clear() @@ -735,51 +781,6 @@ class Session(nodes.FSCollector): self._collection_matchnodes_cache.clear() self._collection_pkg_roots.clear() - def matchnodes( - self, - matching: Sequence[Union[nodes.Item, nodes.Collector]], - names: Sequence[str], - ) -> Sequence[Union[nodes.Item, nodes.Collector]]: - result = [] - work = [(matching, names)] - while work: - self.trace("matchnodes", matching, names) - self.trace.root.indent += 1 - - matching, names = work.pop() - for node in matching: - if not names: - result.append(node) - continue - if not isinstance(node, nodes.Collector): - continue - key = (type(node), node.nodeid) - if key in self._collection_matchnodes_cache: - rep = self._collection_matchnodes_cache[key] - else: - rep = collect_one_node(node) - self._collection_matchnodes_cache[key] = rep - if rep.passed: - submatching = [] - for x in rep.result: - # TODO: Remove parametrized workaround once collection structure contains parametrization. - if x.name == names[0] or x.name.split("[")[0] == names[0]: - submatching.append(x) - if submatching: - work.append((submatching, names[1:])) - # XXX Accept IDs that don't have "()" for class instances. - elif len(rep.result) == 1 and rep.result[0].name == "()": - work.append((rep.result, names)) - else: - # Report collection failures here to avoid failing to run some test - # specified in the command line because the module could not be - # imported (#134). - node.ihook.pytest_collectreport(report=rep) - - self.trace("matchnodes finished -> ", len(result), "nodes") - self.trace.root.indent -= 1 - return result - def genitems( self, node: Union[nodes.Item, nodes.Collector] ) -> Iterator[nodes.Item]: From 023f0510afcd2a182423b45fa79da47c275ab446 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Sat, 22 Aug 2020 11:43:23 +0300 Subject: [PATCH 17/18] main: move collection cache attributes to local variables in collect() They are only used for the duration of this function. --- src/_pytest/main.py | 70 +++++++++++++++++++-------------------------- 1 file changed, 30 insertions(+), 40 deletions(-) diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 69ea46a40..c4d1ba85e 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -45,8 +45,6 @@ if TYPE_CHECKING: from typing import Type from typing_extensions import Literal - from _pytest.python import Package - def pytest_addoption(parser: Parser) -> None: parser.addini( @@ -443,23 +441,6 @@ class Session(nodes.FSCollector): self.startdir = config.invocation_dir self._initialpaths = frozenset() # type: FrozenSet[py.path.local] - # Keep track of any collected nodes in here, so we don't duplicate fixtures. - self._collection_node_cache1 = ( - {} - ) # type: Dict[py.path.local, Sequence[nodes.Collector]] - self._collection_node_cache2 = ( - {} - ) # type: Dict[Tuple[Type[nodes.Collector], py.path.local], nodes.Collector] - - # Keep track of any collected collectors in matchnodes paths, so they - # are not collected more than once. - self._collection_matchnodes_cache = ( - {} - ) # type: Dict[Tuple[Type[nodes.Collector], str], CollectReport] - - # Dirnames of pkgs with dunder-init files. - self._collection_pkg_roots = {} # type: Dict[str, Package] - self._bestrelpathcache = _bestrelpath_cache( config.rootdir ) # type: Dict[py.path.local, str] @@ -639,6 +620,21 @@ class Session(nodes.FSCollector): def collect(self) -> Iterator[Union[nodes.Item, nodes.Collector]]: from _pytest.python import Package + # Keep track of any collected nodes in here, so we don't duplicate fixtures. + node_cache1 = {} # type: Dict[py.path.local, Sequence[nodes.Collector]] + node_cache2 = ( + {} + ) # type: Dict[Tuple[Type[nodes.Collector], py.path.local], nodes.Collector] + + # Keep track of any collected collectors in matchnodes paths, so they + # are not collected more than once. + matchnodes_cache = ( + {} + ) # type: Dict[Tuple[Type[nodes.Collector], str], CollectReport] + + # Dirnames of pkgs with dunder-init files. + pkg_roots = {} # type: Dict[str, Package] + for argpath, names in self._initial_parts: self.trace("processing argument", (argpath, names)) self.trace.root.indent += 1 @@ -655,14 +651,12 @@ class Session(nodes.FSCollector): if parent.isdir(): pkginit = parent.join("__init__.py") if pkginit.isfile(): - if pkginit not in self._collection_node_cache1: + if pkginit not in node_cache1: col = self._collectfile(pkginit, handle_dupes=False) if col: if isinstance(col[0], Package): - self._collection_pkg_roots[str(parent)] = col[0] - self._collection_node_cache1[col[0].fspath] = [ - col[0] - ] + pkg_roots[str(parent)] = col[0] + node_cache1[col[0].fspath] = [col[0]] # If it's a directory argument, recurse and look for any Subpackages. # Let the Package collector deal with subnodes, don't collect here. @@ -685,28 +679,28 @@ class Session(nodes.FSCollector): for x in self._collectfile(pkginit): yield x if isinstance(x, Package): - self._collection_pkg_roots[str(dirpath)] = x - if str(dirpath) in self._collection_pkg_roots: + pkg_roots[str(dirpath)] = x + if str(dirpath) in pkg_roots: # Do not collect packages here. continue for x in self._collectfile(path): key = (type(x), x.fspath) - if key in self._collection_node_cache2: - yield self._collection_node_cache2[key] + if key in node_cache2: + yield node_cache2[key] else: - self._collection_node_cache2[key] = x + node_cache2[key] = x yield x else: assert argpath.check(file=1) - if argpath in self._collection_node_cache1: - col = self._collection_node_cache1[argpath] + if argpath in node_cache1: + col = node_cache1[argpath] else: - collect_root = self._collection_pkg_roots.get(argpath.dirname, self) + collect_root = pkg_roots.get(argpath.dirname, self) col = collect_root._collectfile(argpath, handle_dupes=False) if col: - self._collection_node_cache1[argpath] = col + node_cache1[argpath] = col matching = [] work = [ @@ -724,11 +718,11 @@ class Session(nodes.FSCollector): if not isinstance(node, nodes.Collector): continue key = (type(node), node.nodeid) - if key in self._collection_matchnodes_cache: - rep = self._collection_matchnodes_cache[key] + if key in matchnodes_cache: + rep = matchnodes_cache[key] else: rep = collect_one_node(node) - self._collection_matchnodes_cache[key] = rep + matchnodes_cache[key] = rep if rep.passed: submatchnodes = [] for r in rep.result: @@ -776,10 +770,6 @@ class Session(nodes.FSCollector): yield from matching self.trace.root.indent -= 1 - self._collection_node_cache1.clear() - self._collection_node_cache2.clear() - self._collection_matchnodes_cache.clear() - self._collection_pkg_roots.clear() def genitems( self, node: Union[nodes.Item, nodes.Collector] From c1f975668ec94ec6e93718abd696b7eb8450360b Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Mon, 24 Aug 2020 11:14:57 +0300 Subject: [PATCH 18/18] main: couple of code simplifications --- src/_pytest/main.py | 18 ++++++++---------- src/_pytest/python.py | 5 ++--- 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/_pytest/main.py b/src/_pytest/main.py index c4d1ba85e..29b1e5f70 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -511,9 +511,8 @@ class Session(nodes.FSCollector): 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 + if any(path.check(fnmatch=pat) for pat in norecursepatterns): + return False return True def _collectfile( @@ -650,13 +649,12 @@ class Session(nodes.FSCollector): if parent.isdir(): pkginit = parent.join("__init__.py") - if pkginit.isfile(): - if pkginit not in node_cache1: - col = self._collectfile(pkginit, handle_dupes=False) - if col: - if isinstance(col[0], Package): - pkg_roots[str(parent)] = col[0] - node_cache1[col[0].fspath] = [col[0]] + if pkginit.isfile() and pkginit not in node_cache1: + col = self._collectfile(pkginit, handle_dupes=False) + if col: + if isinstance(col[0], Package): + pkg_roots[str(parent)] = col[0] + node_cache1[col[0].fspath] = [col[0]] # If it's a directory argument, recurse and look for any Subpackages. # Let the Package collector deal with subnodes, don't collect here. diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 7aa5b4222..f792acbd8 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -644,9 +644,8 @@ class Package(Module): 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 + if any(path.check(fnmatch=pat) for pat in norecursepatterns): + return False return True def _collectfile(