diff --git a/AUTHORS b/AUTHORS index bd1e27cb37d..11a1e24d693 100644 --- a/AUTHORS +++ b/AUTHORS @@ -455,6 +455,7 @@ Segev Finer SemTiOne Serhii Mozghovyi Seth Junot +Shahab Ahmed Shantanu Jain Sharad Nair Shaygan Hooshyari diff --git a/changelog/15221.improvement.rst b/changelog/15221.improvement.rst new file mode 100644 index 00000000000..06db95f272e --- /dev/null +++ b/changelog/15221.improvement.rst @@ -0,0 +1,9 @@ +pytest now uses a single collection of a level for explicit file arguments +of the same parent directory, instead of collecting that level again for +every file argument, which made collection time grow with the multiplication +of those counts (pytest a/test_1.py a/test_2.py …). + +The previous behaviour of running the same explicitly given file multiple +times with ``--keepduplicates``, or when the same file is selected with +different ``::`` parts, is preserved: those occurrences keep being collected +fresh per occurrence. diff --git a/src/_pytest/main.py b/src/_pytest/main.py index d43a68b4679..a1b34dd7cfc 100644 --- a/src/_pytest/main.py +++ b/src/_pytest/main.py @@ -623,6 +623,7 @@ def __init__(self, config: Config) -> None: self._notfound: list[tuple[str, Sequence[nodes.Collector]]] = [] self._initial_parts: list[CollectionArgument] = [] self._collection_cache: dict[nodes.Collector, CollectReport] = {} + self._repeated_file_arguments: set[Path] = set() self.items: list[nodes.Item] = [] self._bestrelpathcache: dict[Path, str] = _bestrelpath_cache(config.rootpath) @@ -816,6 +817,7 @@ def perform_collect( self._notfound = [] self._initial_parts = [] self._collection_cache = {} + self._repeated_file_arguments = set() self.items = [] items: Sequence[nodes.Item | nodes.Collector] = self.items consider_namespace_packages: bool = self.config.getini( @@ -842,6 +844,21 @@ def perform_collect( else: self._initial_parts = collection_args + # Explicit file argument paths that occur in more than one of the + # initial arguments (--keepduplicates, or the same file selected + # with different parts) re-collect their parent level fresh, per + # occurrence, in Session.collect. Identical path-sharing of file + # arguments collected exactly once reuses the level's collection + # report instead (#15221). + file_argument_counts: dict[Path, int] = {} + for collection_argument in self._initial_parts: + if collection_argument.path.is_file(): + path = collection_argument.path + file_argument_counts[path] = file_argument_counts.get(path, 0) + 1 + self._repeated_file_arguments = { + path for path, count in file_argument_counts.items() if count > 1 + } + for collection_argument in self._initial_parts: initialpaths.append(collection_argument.path) initialpaths_with_parents.append(collection_argument.path) @@ -879,6 +896,7 @@ def perform_collect( self._notfound = [] self._initial_parts = [] self._collection_cache = {} + self._repeated_file_arguments = set() hook.pytest_collection_finish(session=self) if genitems: @@ -992,12 +1010,46 @@ def collect(self) -> Iterator[nodes.Item | nodes.Collector]: else: # For backward compat, files given directly multiple # times on the command line should not be deduplicated. - handle_dupes = not ( - len(matchparts) == 1 - and isinstance(matchparts[0], Path) - and matchparts[0].is_file() + file_argument: Path | None = ( + matchparts[0] + if ( + len(matchparts) == 1 + and isinstance(matchparts[0], Path) + and matchparts[0].is_file() + ) + else None ) - rep, duplicate = self._collect_one_node(matchnode, handle_dupes) + if file_argument is None: + rep, duplicate = self._collect_one_node(matchnode) + elif file_argument in self._repeated_file_arguments: + # The file path occurs in more than one initial + # argument (--keepduplicates, or the same file + # selected with different parts): collect this + # occurrence from scratch, as before, so the file is + # not deduplicated (#15221). + rep, duplicate = self._collect_one_node( + matchnode, handle_dupes=False + ) + else: + # This file path is collected exactly once: reuse + # this level's cached report instead of re-collecting + # the whole level for each explicit file argument. + # The node ids and their order are unchanged, and the + # file itself is not deduplicated (#15221). + if ( + matchnode in self._collection_cache + and not self._collection_cache[matchnode].passed + ): + # A previously cached report for this level + # failed: re-collect from scratch, as it would + # have been before successful reports were + # reused, so the failure and its report are + # unchanged. + rep, duplicate = self._collect_one_node( + matchnode, handle_dupes=False + ) + else: + rep, duplicate = self._collect_one_node(matchnode) if not duplicate and not rep.passed: # Report collection failures here to avoid failing to # run some test specified in the command line because diff --git a/testing/test_collection.py b/testing/test_collection.py index f7cdc60a1c3..bdc995e432a 100644 --- a/testing/test_collection.py +++ b/testing/test_collection.py @@ -2807,3 +2807,102 @@ def test1(x): "Duplicates: a", ] ) + + +def test_file_arguments_scan_parent_level_once(pytester: Pytester) -> None: + """Explicit file arguments sharing a parent must not re-collect that + parent per file argument (#15221). + + Each file argument used to walk its parents from the root again, and since + an explicit file at the end of a path re-collects its level fresh, the + whole parent level was re-scanned for each argument: after this change a + parent level is scanned once and reused for the remaining file arguments, + which keeps the same node ids and order. + """ + tests = pytester.mkdir("tests") + for i in (1, 2, 3): + ensure_file(tests / f"test_{i}.py").write_text( + f"def test_{i}(): pass\n", encoding="utf-8" + ) + ensure_file(tests / "other.py").write_text("value = 1\n", encoding="utf-8") + + args = ["tests/test_1.py", "tests/test_2.py", "tests/test_3.py"] + items, hookrec = pytester.inline_genitems(*args) + + assert [item.nodeid for item in items] == [ + "tests/test_1.py::test_1", + "tests/test_2.py::test_2", + "tests/test_3.py::test_3", + ] + scanned = [ + call.collection_path.name + for call in hookrec.getcalls("pytest_ignore_collect") + if call.collection_path.parent == tests + ] + # The three argument files are initial paths and skip the + # pytest_ignore_collect consult; only non-argument files of the parent + # level are consulted, and only as often as the level is scanned. + assert scanned == ["other.py"] + + +def test_keepduplicates_runs_the_same_file_twice(pytester: Pytester) -> None: + tests = pytester.mkdir("tests") + ensure_file(tests / "test_dup.py").write_text( + "def test_dup(): pass\n", encoding="utf-8" + ) + + hookrec = pytester.inline_run( + "--collect-only", "--keepduplicates", "tests/test_dup.py", "tests/test_dup.py" + ) + items = [call.item for call in hookrec.getcalls("pytest_itemcollected")] + assert [item.nodeid for item in items] == [ + "tests/test_dup.py::test_dup", + "tests/test_dup.py::test_dup", + ] + + +def test_same_file_with_different_parts_is_collected_twice(pytester: Pytester) -> None: + tests = pytester.mkdir("tests") + ensure_file(tests / "test_dup.py").write_text( + "def test_a(): pass\ndef test_b(): pass\n", encoding="utf-8" + ) + + items, _hookrec = pytester.inline_genitems( + "tests/test_dup.py::test_a", "tests/test_dup.py::test_b" + ) + assert [item.nodeid for item in items] == [ + "tests/test_dup.py::test_a", + "tests/test_dup.py::test_b", + ] + + +def test_file_arguments_recollect_after_failed_parent_collection( + pytester: Pytester, +) -> None: + """A failing parent-level collection is re-collected per file argument, + as it was before its successful report reuse (--keepduplicates aside), + so each failure stays reported and unchanged.""" + tests = pytester.mkdir("tests") + for name in ("test_1.py", "test_2.py"): + ensure_file(tests / name).write_text( + f"def test_{name[5]}(): pass\n", encoding="utf-8" + ) + ensure_file(tests / "test_neighbor.py").write_text( + "def test_neighbor(): pass\n", encoding="utf-8" + ) + pytester.makeconftest( + textwrap.dedent( + """\ + def pytest_collect_file(file_path, parent): + # A failing collect of one non-argument sibling during the + # directory scan fails the whole Directory collection report. + if file_path.name == "test_neighbor.py": + raise RuntimeError("collect_neighbor_boom") + return None + """ + ) + ) + result = pytester.runpytest("tests/test_1.py", "tests/test_2.py") + assert result.ret != 0 + combined = result.stdout.str() + result.stderr.str() + assert combined.count("ERROR tests - RuntimeError: collect_neighbor_boom") == 2