From c15bb5d3de084a10ade0a99e2f6ec1226aa9356a Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Sun, 5 Jul 2020 22:58:47 +0300 Subject: [PATCH 1/2] pathlib: replace py.path.local.visit() with our own function Part of reducing dependency on `py`. Also enables upcoming improvements. In cases where there are simpler alternatives (in tests), I used those. What's left are a couple of uses in `_pytest.main` and `_pytest.python` and they only have modest requirements, so all of the featureful code from py is not needed. --- src/_pytest/main.py | 13 +++++-------- src/_pytest/pathlib.py | 15 +++++++++++++++ src/_pytest/python.py | 3 ++- testing/python/fixtures.py | 4 ++-- testing/test_collection.py | 5 +++-- testing/test_conftest.py | 5 +++-- 6 files changed, 30 insertions(+), 15 deletions(-) diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 7e68165066d..7f81c341de4 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -32,6 +32,7 @@ from _pytest.fixtures import FixtureManager from _pytest.outcomes import exit from _pytest.pathlib import Path +from _pytest.pathlib import visit from _pytest.reports import CollectReport from _pytest.reports import TestReport from _pytest.runner import collect_one_node @@ -617,9 +618,10 @@ def _collect( assert not names, "invalid arg {!r}".format((argpath, names)) seen_dirs = set() # type: Set[py.path.local] - for path in argpath.visit( - fil=self._visit_filter, rec=self._recurse, bf=True, sort=True - ): + for path in visit(argpath, self._recurse): + if not path.check(file=1): + continue + dirpath = path.dirpath() if dirpath not in seen_dirs: # Collect packages first. @@ -668,11 +670,6 @@ def _collect( return yield from m - @staticmethod - def _visit_filter(f: py.path.local) -> bool: - # TODO: Remove type: ignore once `py` is typed. - return f.check(file=1) # type: ignore - def _tryconvertpyarg(self, x: str) -> str: """Convert a dotted module name to path.""" try: diff --git a/src/_pytest/pathlib.py b/src/_pytest/pathlib.py index 92ba32082a5..b78b13ecbd1 100644 --- a/src/_pytest/pathlib.py +++ b/src/_pytest/pathlib.py @@ -16,6 +16,7 @@ from os.path import sep from posixpath import sep as posix_sep from types import ModuleType +from typing import Callable from typing import Iterable from typing import Iterator from typing import Optional @@ -556,3 +557,17 @@ def resolve_package_path(path: Path) -> Optional[Path]: break result = parent return result + + +def visit( + path: py.path.local, recurse: Callable[[py.path.local], bool], +) -> Iterator[py.path.local]: + """Walk path recursively, in breadth-first order. + + Entries at each directory level are sorted. + """ + entries = sorted(path.listdir()) + yield from entries + for entry in entries: + if entry.check(dir=1) and recurse(entry): + yield from visit(entry, recurse) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index aa817148683..2d7060c7845 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -65,6 +65,7 @@ from _pytest.pathlib import import_path from _pytest.pathlib import ImportPathMismatchError from _pytest.pathlib import parts +from _pytest.pathlib import visit from _pytest.reports import TerminalRepr from _pytest.warning_types import PytestCollectionWarning from _pytest.warning_types import PytestUnhandledCoroutineWarning @@ -641,7 +642,7 @@ def collect(self) -> Iterable[Union[nodes.Item, nodes.Collector]]: ): yield Module.from_parent(self, fspath=init_module) pkg_prefixes = set() # type: Set[py.path.local] - for path in this_path.visit(rec=self._recurse, bf=True, sort=True): + for path in visit(this_path, recurse=self._recurse): # We will visit our own __init__.py file, in which case we skip it. is_file = path.isfile() if is_file: diff --git a/testing/python/fixtures.py b/testing/python/fixtures.py index ca3408ece30..119c7dedaf6 100644 --- a/testing/python/fixtures.py +++ b/testing/python/fixtures.py @@ -142,14 +142,14 @@ def test_extend_fixture_conftest_module(self, testdir): p = testdir.copy_example() result = testdir.runpytest() result.stdout.fnmatch_lines(["*1 passed*"]) - result = testdir.runpytest(next(p.visit("test_*.py"))) + result = testdir.runpytest(str(next(Path(str(p)).rglob("test_*.py")))) result.stdout.fnmatch_lines(["*1 passed*"]) def test_extend_fixture_conftest_conftest(self, testdir): p = testdir.copy_example() result = testdir.runpytest() result.stdout.fnmatch_lines(["*1 passed*"]) - result = testdir.runpytest(next(p.visit("test_*.py"))) + result = testdir.runpytest(str(next(Path(str(p)).rglob("test_*.py")))) result.stdout.fnmatch_lines(["*1 passed*"]) def test_extend_fixture_conftest_plugin(self, testdir): diff --git a/testing/test_collection.py b/testing/test_collection.py index 3e01e296b58..f5e8abfd727 100644 --- a/testing/test_collection.py +++ b/testing/test_collection.py @@ -7,6 +7,7 @@ from _pytest.config import ExitCode from _pytest.main import _in_venv from _pytest.main import Session +from _pytest.pathlib import Path from _pytest.pathlib import symlink_or_skip from _pytest.pytester import Testdir @@ -115,8 +116,8 @@ def test_ignored_certain_directories(self, testdir): tmpdir.ensure(".whatever", "test_notfound.py") tmpdir.ensure(".bzr", "test_notfound.py") tmpdir.ensure("normal", "test_found.py") - for x in tmpdir.visit("test_*.py"): - x.write("def test_hello(): pass") + for x in Path(str(tmpdir)).rglob("test_*.py"): + x.write_text("def test_hello(): pass", "utf-8") result = testdir.runpytest("--collect-only") s = result.stdout.str() diff --git a/testing/test_conftest.py b/testing/test_conftest.py index 724e6f464cb..dbafe7dd34b 100644 --- a/testing/test_conftest.py +++ b/testing/test_conftest.py @@ -477,8 +477,9 @@ def test_no_conftest(fxtr): ) ) print("created directory structure:") - for x in testdir.tmpdir.visit(): - print(" " + x.relto(testdir.tmpdir)) + tmppath = Path(str(testdir.tmpdir)) + for x in tmppath.rglob(""): + print(" " + str(x.relative_to(tmppath))) return {"runner": runner, "package": package, "swc": swc, "snc": snc} From 3633b691d88fbe9bf76137d1af25a0893ebefa84 Mon Sep 17 00:00:00 2001 From: Ran Benita Date: Sun, 5 Jul 2020 23:11:47 +0300 Subject: [PATCH 2/2] pathlib: make visit() independent of py.path.local, use os.scandir `os.scandir()`, introduced in Python 3.5, is much faster than `os.listdir()`. See https://www.python.org/dev/peps/pep-0471/. It also has a `DirEntry` which can be used to further reduce syscalls in some cases. --- src/_pytest/main.py | 6 ++++-- src/_pytest/nodes.py | 15 ++++++++------- src/_pytest/pathlib.py | 12 ++++++------ src/_pytest/python.py | 15 ++++++++------- 4 files changed, 26 insertions(+), 22 deletions(-) diff --git a/src/_pytest/main.py b/src/_pytest/main.py index 7f81c341de4..96998830548 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -618,11 +618,13 @@ def _collect( assert not names, "invalid arg {!r}".format((argpath, names)) seen_dirs = set() # type: Set[py.path.local] - for path in visit(argpath, self._recurse): - if not path.check(file=1): + 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) diff --git a/src/_pytest/nodes.py b/src/_pytest/nodes.py index 560548aea64..d53d591e742 100644 --- a/src/_pytest/nodes.py +++ b/src/_pytest/nodes.py @@ -562,17 +562,18 @@ def _gethookproxy(self, fspath: py.path.local): def gethookproxy(self, fspath: py.path.local): raise NotImplementedError() - def _recurse(self, dirpath: py.path.local) -> bool: - if dirpath.basename == "__pycache__": + def _recurse(self, direntry: "os.DirEntry[str]") -> bool: + if direntry.name == "__pycache__": return False - ihook = self._gethookproxy(dirpath.dirpath()) - if ihook.pytest_ignore_collect(path=dirpath, config=self.config): + path = py.path.local(direntry.path) + ihook = self._gethookproxy(path.dirpath()) + if ihook.pytest_ignore_collect(path=path, config=self.config): return False for pat in self._norecursepatterns: - if dirpath.check(fnmatch=pat): + if path.check(fnmatch=pat): return False - ihook = self._gethookproxy(dirpath) - ihook.pytest_collect_directory(path=dirpath, parent=self) + ihook = self._gethookproxy(path) + ihook.pytest_collect_directory(path=path, parent=self) return True def isinitpath(self, path: py.path.local) -> bool: diff --git a/src/_pytest/pathlib.py b/src/_pytest/pathlib.py index b78b13ecbd1..ba7e9948a59 100644 --- a/src/_pytest/pathlib.py +++ b/src/_pytest/pathlib.py @@ -560,14 +560,14 @@ def resolve_package_path(path: Path) -> Optional[Path]: def visit( - path: py.path.local, recurse: Callable[[py.path.local], bool], -) -> Iterator[py.path.local]: - """Walk path recursively, in breadth-first order. + path: str, recurse: Callable[["os.DirEntry[str]"], bool] +) -> Iterator["os.DirEntry[str]"]: + """Walk a directory recursively, in breadth-first order. Entries at each directory level are sorted. """ - entries = sorted(path.listdir()) + entries = sorted(os.scandir(path), key=lambda entry: entry.name) yield from entries for entry in entries: - if entry.check(dir=1) and recurse(entry): - yield from visit(entry, recurse) + if entry.is_dir(follow_symlinks=False) and recurse(entry): + yield from visit(entry.path, recurse) diff --git a/src/_pytest/python.py b/src/_pytest/python.py index 2d7060c7845..e2b6aef9c44 100644 --- a/src/_pytest/python.py +++ b/src/_pytest/python.py @@ -642,23 +642,24 @@ def collect(self) -> Iterable[Union[nodes.Item, nodes.Collector]]: ): yield Module.from_parent(self, fspath=init_module) pkg_prefixes = set() # type: Set[py.path.local] - for path in visit(this_path, recurse=self._recurse): + for direntry in visit(str(this_path), recurse=self._recurse): + path = py.path.local(direntry.path) + # We will visit our own __init__.py file, in which case we skip it. - is_file = path.isfile() - if is_file: - if path.basename == "__init__.py" and path.dirpath() == this_path: + if direntry.is_file(): + if direntry.name == "__init__.py" and path.dirpath() == this_path: continue - parts_ = parts(path.strpath) + parts_ = parts(direntry.path) if any( str(pkg_prefix) in parts_ and pkg_prefix.join("__init__.py") != path for pkg_prefix in pkg_prefixes ): continue - if is_file: + if direntry.is_file(): yield from self._collectfile(path) - elif not path.isdir(): + elif not direntry.is_dir(): # Broken symlink or invalid/missing file. continue elif path.join("__init__.py").check(file=1):