From 767b00ed5aeaf005746b7267f4cd80b2baeffb86 Mon Sep 17 00:00:00 2001 From: jonahgabriel Date: Sat, 29 Aug 2026 04:19:12 -0400 Subject: [PATCH] fix(OMN-16917): count every configured test filename pattern when sizing splits CodeRabbit review finding on #1614, valid and in-seam. `_contains_collectable_test` admits a directory on EITHER pattern in TEST_FILE_PATTERNS ("test_*.py", "*_test.py" -- mirroring pytest's python_files), and `ci_contract_test_paths` selects tests/ci/ on exactly that predicate. But `_count_test_files` globbed only `test_*.py`, so a directory could be SELECTED on the strength of files the volume counter then did not count: undercounted volume, too few matrix splits, a selection whose own proof was invisible to its sizing. `_count_test_files` now counts every name `is_test_file_name` accepts, so the selection predicate and the sizing predicate are the same predicate. No behavior change on the current tree (0 `*_test.py` files today, 1,586 `test_*.py`) -- this closes the latent divergence rather than reacting to a live miscount. The new test asserts the counter directly against a synthetic tree holding both patterns plus a non-test helper, so the invariant holds regardless of which patterns are populated in the live tree. Readback on the OMN-16625 diff is unchanged: split_count 5, is_full_suite=false, tests/unit/ absent. OMN-16917 --- scripts/ci/detect_test_paths.py | 15 +++++++++-- ..._detect_test_paths_ci_contract_omn16917.py | 27 +++++++++++++++++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/scripts/ci/detect_test_paths.py b/scripts/ci/detect_test_paths.py index b813ee7f7e..3c9317e5f8 100644 --- a/scripts/ci/detect_test_paths.py +++ b/scripts/ci/detect_test_paths.py @@ -557,18 +557,29 @@ def _volume_split_count(selected_paths: list[str], repo_root: Path | None) -> in def _count_test_files(selected_paths: list[str], repo_root: Path) -> int: - """Count ``test_*.py`` files a selection covers. + """Count the collectable test files a selection covers. ``selected_paths`` entries are either a directory (module-grain sentinel, e.g. ``tests/unit/`` or ``tests/unit/cli/`` — walked recursively) or an individual test FILE (OMN-14921 file-grain closure output, e.g. ``tests/unit/cli/test_foo.py`` — counted directly, no walk needed). + + Directory walks count every name :func:`is_test_file_name` accepts — i.e. + the whole of ``TEST_FILE_PATTERNS``, which mirrors pytest's ``python_files`` + — not just the ``test_*.py`` half. Counting only one pattern here while + ``_contains_collectable_test`` admits both would let a directory be SELECTED + on the strength of files this function then does not count, undercounting + the volume and emitting too few matrix splits (OMN-16917 review finding). """ total = 0 for rel in selected_paths: target = repo_root / rel if target.is_dir(): - total += sum(1 for _ in target.rglob("test_*.py")) + total += sum( + 1 + for candidate in target.rglob("*.py") + if is_test_file_name(candidate.name) + ) elif target.is_file(): total += 1 return total diff --git a/tests/unit/scripts/ci/test_detect_test_paths_ci_contract_omn16917.py b/tests/unit/scripts/ci/test_detect_test_paths_ci_contract_omn16917.py index 688a3cf2e9..82e3d232f5 100644 --- a/tests/unit/scripts/ci/test_detect_test_paths_ci_contract_omn16917.py +++ b/tests/unit/scripts/ci/test_detect_test_paths_ci_contract_omn16917.py @@ -24,8 +24,10 @@ from scripts.ci.detect_test_paths import ( CI_CONTRACT_TEST_ROOT, + _count_test_files, ci_contract_test_paths, compute_selection, + is_test_file_name, unnarrowable_test_paths, ) from scripts.ci.test_selection_closure import compute_closure_selection @@ -309,3 +311,28 @@ def test_source_only_diff_is_byte_for_byte_the_pre_existing_closure_path() -> No assert selection.is_full_suite is False assert selection.selected_paths == expected + + +def test_split_count_counts_every_configured_test_filename_pattern( + tmp_path: Path, +) -> None: + """Volume counting honours ALL of ``TEST_FILE_PATTERNS``, not just half. + + ``_contains_collectable_test`` admits a directory on either pattern, so a + directory can be SELECTED on the strength of ``*_test.py`` files. If the + volume counter only globbed ``test_*.py`` it would not count the files that + justified the selection, undercount the volume, and emit too few matrix + splits — a selection whose own proof is invisible to its sizing. + + Asserted against the counter directly so the invariant holds regardless of + which patterns happen to be populated in the live tree today. + """ + suite = tmp_path / "tests" / "suite" + suite.mkdir(parents=True) + (suite / "test_prefix_style.py").write_text("def test_a() -> None: ...\n") + (suite / "suffix_style_test.py").write_text("def test_b() -> None: ...\n") + (suite / "helper.py").write_text("VALUE = 1\n") + + assert _count_test_files(["tests/suite/"], tmp_path) == 2 + assert is_test_file_name("suffix_style_test.py") is True + assert is_test_file_name("helper.py") is False