handle bot review comments

script/split_tests.py

@@ -21,6 +21,12 @@ _FAN_OUT_DIRS: Final = frozenset({"components"})
 # caches are ignored rather than misread.
 _CACHE_VERSION: Final = 2
 
+# Fall back from file-level to directory-level pytest collection when
+# misses make up more than this fraction of the tree; past that point
+# the per-file argv overhead pytest pays outweighs the cost of letting
+# it re-walk dirs and re-collect the hits.
+_DIR_LEVEL_MISS_RATIO: Final = 0.3
+
 
 class Bucket:
     """Class to hold bucket."""
@@ -424,6 +430,10 @@ def _run_pytest_collect(paths: list[Path]) -> dict[Path, int]:
             print(stderr)
             print(stdout)
             sys.exit(1)
+        # Surface stderr from successful runs too; pytest puts deprecation
+        # and import warnings here that would otherwise vanish.
+        if stderr.strip():
+            sys.stderr.write(stderr)
         try:
             counts.update(_parse_collect_output(stdout))
         except ValueError as err:
@@ -482,10 +492,14 @@ def _collect_tests_cached(path: Path, cache_path: Path) -> TestFolder:
 
     new_counts: dict[Path, int] = {}
     if miss_hashes:
-        # Cold cache: hand pytest the top-level dirs (much faster than
-        # 5000+ individual file paths).  Once any hit exists, collect
-        # only the diff at file granularity.
-        collect_paths = _enumerate_batch_paths(path) if not hits else list(miss_hashes)
+        # File-level collection saves work when the diff is small.  But
+        # when many files miss (eg a PR adding a new integration with
+        # hundreds of new files) the per-file argv overhead dominates
+        # and dir-level wins, so fall back past _DIR_LEVEL_MISS_RATIO.
+        if not hits or len(miss_hashes) > len(all_test_files) * _DIR_LEVEL_MISS_RATIO:
+            collect_paths = _enumerate_batch_paths(path)
+        else:
+            collect_paths = list(miss_hashes)
         new_counts = _run_pytest_collect(collect_paths)
 
     # One pass over all files: hits keep their entry, misses build a

tests/script/test_split_tests.py

@@ -331,7 +331,9 @@ def test_collect_tests_hashes_each_file_once(tree: Path) -> None:
         counts[path] = counts.get(path, 0) + 1
         return real_hash(path)
 
+    # Pin the threshold so the tiny tree stays on the file-level path.
     with (
+        patch.object(split_tests, "_DIR_LEVEL_MISS_RATIO", 1.0),
         patch.object(split_tests, "_hash_file", side_effect=counting_hash),
         patch.object(
             split_tests, "_run_collect_batches", side_effect=_echo_one_test_each()
@@ -365,9 +367,12 @@ def test_collect_tests_cold_cache_collects_only_missing(tree: Path) -> None:
 
     _prime_cache(cache_path, tree, hits={alpha_one: 1})
 
-    with patch.object(
-        split_tests, "_run_collect_batches", side_effect=_echo_one_test_each()
-    ) as run_batches:
+    with (
+        patch.object(split_tests, "_DIR_LEVEL_MISS_RATIO", 1.0),
+        patch.object(
+            split_tests, "_run_collect_batches", side_effect=_echo_one_test_each()
+        ) as run_batches,
+    ):
         folder = split_tests.collect_tests(tree, cache_path)
 
     assert run_batches.call_count == 1
@@ -384,6 +389,24 @@ def test_collect_tests_cold_cache_collects_only_missing(tree: Path) -> None:
     }
 
 
+def test_collect_tests_falls_back_to_dirs_when_misses_dominate(tree: Path) -> None:
+    """Heavy misses should switch back to dir-level invocation."""
+    cache_path = tree / "cache.json"
+    alpha_one = tree / "components" / "alpha" / "test_one.py"
+    _prime_cache(cache_path, tree, hits={alpha_one: 1})
+    # 1 hit / 3 total = 33% miss, above the 30% default threshold; this
+    # also covers the new-directory PR case (mostly-new test files).
+
+    with patch.object(
+        split_tests, "_run_collect_batches", side_effect=_echo_one_test_each()
+    ) as run_batches:
+        split_tests.collect_tests(tree, cache_path)
+
+    # We expect the dir-level batch paths, not the individual miss files.
+    requested = set(run_batches.call_args.args[0])
+    assert requested == set(split_tests._enumerate_batch_paths(tree))
+
+
 def test_collect_tests_caches_files_with_no_collected_tests(tree: Path) -> None:
     """Files pytest returns nothing for are cached as 0 so we stop re-collecting them.
 
@@ -401,10 +424,13 @@ def test_collect_tests_caches_files_with_no_collected_tests(tree: Path) -> None:
     # rather than individual file paths.
     _prime_cache(cache_path, tree, hits={alpha_one: 1})
 
-    with patch.object(
-        split_tests,
-        "_run_collect_batches",
-        side_effect=_echo_one_test_each(skip={alpha_two}),
+    with (
+        patch.object(split_tests, "_DIR_LEVEL_MISS_RATIO", 1.0),
+        patch.object(
+            split_tests,
+            "_run_collect_batches",
+            side_effect=_echo_one_test_each(skip={alpha_two}),
+        ),
     ):
         split_tests.collect_tests(tree, cache_path)
 
@@ -435,8 +461,11 @@ def test_collect_tests_drops_deleted_files_from_cache(tree: Path) -> None:
         extra_entries={ghost_rel: split_tests._CacheEntry(hash="dead", count=42)},
     )
 
-    with patch.object(
-        split_tests, "_run_collect_batches", side_effect=_echo_one_test_each()
+    with (
+        patch.object(split_tests, "_DIR_LEVEL_MISS_RATIO", 1.0),
+        patch.object(
+            split_tests, "_run_collect_batches", side_effect=_echo_one_test_each()
+        ),
     ):
         split_tests.collect_tests(tree, cache_path)