diff --git a/.buildkite/test_areas/misc.yaml b/.buildkite/test_areas/misc.yaml index 759f3900c5..6709dbb565 100644 --- a/.buildkite/test_areas/misc.yaml +++ b/.buildkite/test_areas/misc.yaml @@ -525,6 +525,9 @@ steps: - tests/renderers - tests/standalone_tests/lazy_imports.py - tests/reasoning + - tools/pre_commit/check_test_tethering.py + - tools/pre_commit/test_tethering_allowlist.txt + - tests/tools/test_check_test_tethering.py - tests/tool_parsers - tests/tokenizers_ - tests/parser @@ -552,6 +555,7 @@ steps: commands: - pytest -v -s -m 'cpu_test' multimodal - pytest -v -s config + - pytest -v -s tools/test_check_test_tethering.py - label: ":computer: (CPU) Reasoning + Renderers" key: cpu-reasoning-renderers diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index b12c8ce508..4823f788dc 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -184,6 +184,14 @@ repos: files: ^\.buildkite/.*\.ya?ml$ pass_filenames: false additional_dependencies: [PyYAML] + - id: check-test-tethering + name: Check new tests are wired into Buildkite CI + language: python + entry: python tools/pre_commit/check_test_tethering.py + files: ^(tests/.*\.py|\.buildkite/(test_areas|hardware_tests|intel_jobs)/.*\.yaml|\.buildkite/(test-amd|ci_config[^/]*)\.yaml|tools/pre_commit/test_tethering_allowlist\.txt)$ + additional_dependencies: [PyYAML, regex] + verbose: true # full-scan `note:` lines are advisory, so the hook passes + require_serial: true # one full scan, not one per pre-commit file batch - id: mypy-3.10 # TODO: Use https://github.com/pre-commit/mirrors-mypy when mypy setup is less awkward name: Run mypy for Python 3.10 entry: python tools/pre_commit/mypy.py "3.10" diff --git a/docker/Dockerfile.cpu b/docker/Dockerfile.cpu index 7898e57cf3..d61e186dac 100644 --- a/docker/Dockerfile.cpu +++ b/docker/Dockerfile.cpu @@ -471,6 +471,10 @@ ADD ./benchmarks/ ./benchmarks/ ADD ./vllm/collect_env.py . ADD ./docker/ ./docker/ ADD ./.buildkite/ ./.buildkite/ +# tools/ carries repo-tooling code that a few tests exercise directly +# (e.g. tests/tools/test_check_test_tethering.py imports +# tools.pre_commit.check_test_tethering). +ADD ./tools/ ./tools/ # install development dependencies (for testing) RUN --mount=type=cache,target=/root/.cache/uv \ diff --git a/tests/tools/test_check_test_tethering.py b/tests/tools/test_check_test_tethering.py new file mode 100644 index 0000000000..34f4fd179c --- /dev/null +++ b/tests/tools/test_check_test_tethering.py @@ -0,0 +1,879 @@ +# SPDX-License-Identifier: Apache-2.0 +# SPDX-FileCopyrightText: Copyright contributors to the vLLM project +"""Unit tests for the test-tethering pre-commit check. + +The check answers one question - "does any Buildkite job (in +``.buildkite/test_areas/`` or the legacy ``.buildkite/test-amd.yaml``) collect +this test file?" - by parsing job ``commands`` as shell. The risk that +matters is a *false tether*: a misparse that invents coverage makes the gate +pass while the test still never runs, which is the exact failure the check +exists to catch. So the parser cases below pin both directions, and +:func:`test_no_selection_matches_everything` guards the catastrophic version of +it. +""" + +import textwrap +from pathlib import Path + +import pytest + +import tools.pre_commit.check_test_tethering as checker +from tools.pre_commit.check_test_tethering import ( + FindSelection, + PytestSelection, + _parse_command, + _to_repo_relative, + all_test_modules, + allowlist_entries_missing_reason, + is_test_module, + is_tethered, + load_allowlist, + load_selections, + main, + normalize_test_path, + run_changed_files_check, + run_full_scan, +) + +REPO_ROOT = Path(__file__).resolve().parents[2] + + +def runs(command: str, test_file: str) -> bool: + """True if `command` collects `test_file` (path relative to ``tests/``).""" + return any(sel.runs(test_file) for sel in _parse_command(command)) + + +# --------------------------------------------------------------------------- # +# pytest path arguments +# --------------------------------------------------------------------------- # + + +@pytest.mark.parametrize( + ("command", "test_file", "expected"), + [ + # Directory sweeps cover everything nested underneath. + ("pytest -v -s kernels", "kernels/test_attention.py", True), + ("pytest -v -s kernels/", "kernels/moe/test_moe.py", True), + ("pytest -v -s kernels", "lora/test_lora.py", False), + # An exact file argument covers that file and nothing beside it. + ("pytest -v -s kernels/test_a.py", "kernels/test_a.py", True), + ("pytest -v -s kernels/test_a.py", "kernels/test_b.py", False), + # Commands run from the repo root and from tests/ alike. + ("pytest -v -s tests/kernels/test_a.py", "kernels/test_a.py", True), + ("pytest -v -s ./kernels/test_a.py", "kernels/test_a.py", True), + ( + "pytest -v -s /vllm-workspace/tests/kernels/test_a.py", + "kernels/test_a.py", + True, + ), + # A ::nodeid selector still collects the file. + ("pytest -v -s kernels/test_a.py::test_one", "kernels/test_a.py", True), + ("pytest -v -s kernels/test_a.py::Cls::test_one", "kernels/test_a.py", True), + # Several positional paths, including a bare root-level file. + ("pytest -v -s test_envs.py test_outputs.py", "test_outputs.py", True), + ("pytest -v -s test_envs.py test_outputs.py", "test_regression.py", False), + # A bare word is a path only when it is really a directory under tests/. + ("pytest -v -s samplers", "samplers/test_sampler.py", True), + # Glob path args do not cross a directory separator, matching the shell. + ("pytest -v -s kernels/test_*.py", "kernels/test_a.py", True), + ("pytest -v -s kernels/test_*.py", "kernels/moe/test_a.py", False), + ("pytest -v -s kernels/test_*.py", "kernels/helper.py", False), + ], +) +def test_pytest_path_arguments(command, test_file, expected): + assert runs(command, test_file) is expected + + +# --------------------------------------------------------------------------- # +# --ignore / --deselect +# --------------------------------------------------------------------------- # + + +@pytest.mark.parametrize( + ("command", "test_file", "expected"), + [ + # --ignore removes a file from an otherwise covering sweep, in both the + # `=` and space-separated spellings. + ("pytest -v -s kernels --ignore=kernels/test_a.py", "kernels/test_a.py", False), + ("pytest -v -s kernels --ignore kernels/test_a.py", "kernels/test_a.py", False), + # ...and only that file. + ("pytest -v -s kernels --ignore=kernels/test_a.py", "kernels/test_b.py", True), + # --ignore of a directory removes the whole subtree. + ("pytest -v -s kernels --ignore=kernels/moe", "kernels/moe/test_moe.py", False), + ( + "pytest -v -s lora --ignore-glob=lora/*_tp.py", + "lora/test_llama_tp.py", + False, + ), + # --ignore-glob is a whole-path fnmatch (pytest semantics): its `*` + # crosses `/`, so a bare `*_tp.py` drops a nested match too. A positional + # path glob would not - that is the distinction the parser has to keep. + ("pytest -v -s kernels --ignore-glob=*_tp.py", "kernels/test_a_tp.py", False), + ( + "pytest -v -s kernels --ignore-glob=*_tp.py", + "kernels/nested/test_a_tp.py", + False, + ), + ("pytest -v -s kernels --ignore-glob '*_tp.py'", "kernels/test_a.py", True), + # A node-id deselect leaves the file collected; a bare-path one does not. + ( + "pytest -v -s kernels --deselect kernels/test_a.py::test_one", + "kernels/test_a.py", + True, + ), + ( + "pytest -v -s kernels --deselect kernels/test_a.py", + "kernels/test_a.py", + False, + ), + ], +) +def test_ignore_and_deselect(command, test_file, expected): + assert runs(command, test_file) is expected + + +# --------------------------------------------------------------------------- # +# Options, env prefixes and other token noise +# --------------------------------------------------------------------------- # + + +@pytest.mark.parametrize( + ("command", "test_file", "expected"), + [ + # A -k / -m filter narrows which tests run but does not un-run the file. + ("pytest -v -s kernels -k 'not slow'", "kernels/test_a.py", True), + ("pytest -v -s kernels -m core_model", "kernels/test_a.py", True), + # A value-taking option's value is never mistaken for a path. + ("pytest -m core_model", "core_model", False), + ("pytest -k kernels", "kernels/test_a.py", False), + ("pytest -n 4 kernels/test_a.py", "kernels/test_a.py", True), + # Buildkite shard flags carry a $$VAR and must not swallow the path. + ( + ( + "pytest -v -s kernels --shard-id=$$BUILDKITE_PARALLEL_JOB " + "--num-shards=$$BUILDKITE_PARALLEL_JOB_COUNT" + ), + "kernels/test_a.py", + True, + ), + # A leading env-var assignment is not a path. + ( + "VLLM_TEST_FORCE_LOAD_FORMAT=auto pytest -v -s kernels", + "kernels/test_a.py", + True, + ), + # `python -m pytest` is the same invocation. + ("python3 -m pytest -v -s kernels/test_a.py", "kernels/test_a.py", True), + # Quoted paths, wrapper commands and redirects all survive tokenizing. + ("pytest -v -s 'kernels/test_a.py'", "kernels/test_a.py", True), + ("timeout 600 pytest kernels/test_a.py", "kernels/test_a.py", True), + ("pytest kernels/test_a.py > out.log 2>&1", "kernels/test_a.py", True), + # Each sub-command of a compound line is classified on its own. + ("pytest kernels/test_a.py && pytest lora/test_b.py", "lora/test_b.py", True), + ("pytest kernels/test_a.py ; pytest lora/test_b.py", "lora/test_b.py", True), + # An unresolvable $VAR path yields no coverage rather than a guess. + ("pytest -v -s $TEST_TARGET", "kernels/test_a.py", False), + ], +) +def test_option_and_token_handling(command, test_file, expected): + assert runs(command, test_file) is expected + + +# --------------------------------------------------------------------------- # +# find | xargs pytest pipelines +# --------------------------------------------------------------------------- # + + +@pytest.mark.parametrize( + ("command", "test_file", "expected"), + [ + ("find kernels -name 'test_*.py' | xargs pytest -v", "kernels/test_a.py", True), + ( + "find kernels -name 'test_*.py' | xargs pytest -v", + "kernels/moe/test_b.py", + True, + ), + # A non-matching filename is not collected by the find expression. + ( + "find kernels -name 'test_*.py' | xargs pytest -v", + "kernels/helper.py", + False, + ), + # -maxdepth 1 keeps direct children and drops nested ones. + ( + "find kernels -maxdepth 1 -name 'test_*.py' | xargs pytest", + "kernels/test_a.py", + True, + ), + ( + "find kernels -maxdepth 1 -name 'test_*.py' | xargs pytest", + "kernels/moe/test_b.py", + False, + ), + # -not -name excludes its target and leaves siblings alone. + ( + "find kernels -name 'test_*.py' -not -name 'test_skip.py' | xargs pytest", + "kernels/test_skip.py", + False, + ), + ( + "find kernels -name 'test_*.py' -not -name 'test_skip.py' | xargs pytest", + "kernels/test_a.py", + True, + ), + # The root confines the sweep. + ("find kernels -name 'test_*.py' | xargs pytest", "lora/test_a.py", False), + # -maxdepth is honored even when the find root is the tests/ root itself + # ("." or "tests/"), not just a subdir. + ( + "find . -maxdepth 1 -name 'test_*.py' | xargs pytest", + "test_regression.py", + True, + ), + ( + "find . -maxdepth 1 -name 'test_*.py' | xargs pytest", + "v1/test_scheduler.py", + False, + ), + ( + "find tests/ -maxdepth 1 -name 'test_*.py' | xargs pytest", + "v1/test_scheduler.py", + False, + ), + # -exec pytest wires the sweep just as `| xargs pytest` does. + ( + "find compile -name 'test_*.py' -exec pytest -s -v {} \\;", + "compile/test_a.py", + True, + ), + # A pytest in a *separate* command on the line (not fed by the find) does + # not arm the sweep - the find here is piped to `wc`. + ( + "find kernels -name 'test_*.py' | wc -l ; pytest test_regression.py", + "kernels/test_a.py", + False, + ), + ( + "find kernels -name 'test_*.py' -delete && pytest test_regression.py", + "kernels/test_a.py", + False, + ), + # ...but only a *non-filtering* intermediate stage. `sort` reorders, + # every match still reaches pytest. + ( + "find kernels -name 'test_*.py' | sort | xargs pytest", + "kernels/test_a.py", + True, + ), + # `head` drops all but one match - the sweep is not fully tethered. + ( + "find kernels -name 'test_*.py' | head -n 1 | xargs pytest", + "kernels/test_a.py", + False, + ), + ( + "find kernels -name 'test_*.py' | grep attention | xargs pytest", + "kernels/test_a.py", + False, + ), + # `xargs` runs `echo` here, not pytest - the trailing `pytest` is echo's + # argument. + ( + "find kernels -name 'test_*.py' | xargs echo pytest", + "kernels/test_a.py", + False, + ), + # An `-exec` with no `\;` / `+` terminator is a `find` syntax error - it + # never runs pytest. + ( + "find kernels -name 'test_*.py' -exec pytest {}", + "kernels/test_a.py", + False, + ), + # An `-exec pytest` with no `{}` runs only the named file for every + # match - the matched files never reach pytest. + ( + "find kernels -name 'test_*.py' -exec pytest test_regression.py \\;", + "kernels/test_a.py", + False, + ), + # `{}` as an option *value* (`--ignore {}`) doesn't reach pytest as a + # test path either - only `test_regression.py` runs. + ( + ( + "find kernels -name 'test_*.py' -exec pytest test_regression.py " + "--ignore {} \\;" + ), + "kernels/test_a.py", + False, + ), + # xargs value-flags with a space don't swallow the command. + ( + "find kernels -name 'test_*.py' | xargs -0 -I {} pytest {}", + "kernels/test_a.py", + True, + ), + # `xargs env FOO=1 pytest` - xargs runs `env`, which runs pytest. + ( + "find kernels -name 'test_*.py' | xargs env FOO=1 pytest", + "kernels/test_a.py", + True, + ), + ], +) +def test_find_pipelines(command, test_file, expected): + assert runs(command, test_file) is expected + + +def test_find_pipeline_parses_as_find_selection(): + (selection,) = [ + s + for s in _parse_command("find kernels -name 'test_*.py' | xargs pytest -v") + if isinstance(s, FindSelection) + ] + assert selection.root == "kernels" + assert selection.name_globs == ["test_*.py"] + + +# --------------------------------------------------------------------------- # +# Direct file runners +# --------------------------------------------------------------------------- # + + +@pytest.mark.parametrize( + ("command", "test_file", "expected"), + [ + ( + "python3 standalone_tests/lazy_imports.py", + "standalone_tests/lazy_imports.py", + True, + ), + ( + "python standalone_tests/lazy_imports.py", + "standalone_tests/lazy_imports.py", + True, + ), + # `timeout -k ` wrapping a runner: the grace value must not + # be consumed as the duration (which would hide `python`). + ( + "timeout -k 5s 30s python standalone_tests/lazy_imports.py", + "standalone_tests/lazy_imports.py", + True, + ), + ( + "torchrun --nproc-per-node 2 distributed/test_a.py", + "distributed/test_a.py", + True, + ), + # The option's value is a count, not a path. + ("torchrun --nproc-per-node 2 distributed/test_a.py", "2", False), + ( + "VLLM_X=1 python3 standalone_tests/lazy_imports.py | grep -q ok", + "standalone_tests/lazy_imports.py", + True, + ), + # Only the executed script counts; the tokens after it are that script's + # own argv. `--suffix v1` must not tether the whole tests/v1 subtree. + ( + ( + "python3 features/tensorize_vllm_model.py --model facebook/opt-125m " + "serialize --serialized-directory /tmp/ --suffix v1" + ), + "v1/test_scheduler.py", + False, + ), + ( + ( + "python3 features/tensorize_vllm_model.py --model facebook/opt-125m " + "serialize --serialized-directory /tmp/ --suffix v1" + ), + "features/tensorize_vllm_model.py", + True, + ), + # A script named by an absolute `$VAR` path still resolves. + ( + 'python3 -m pytest -s -x "${GIT_ROOT}/tests/v1/e2e/test_x.py"', + "v1/e2e/test_x.py", + True, + ), + # `python -m ` runs the module, not a script - only `-m pytest` + # collects tests. `compileall kernels` byte-compiles, it does not run. + ("python -m compileall kernels", "kernels/test_a.py", False), + ("python -m mypy kernels/test_a.py", "kernels/test_a.py", False), + # `coverage run -m pytest` still resolves through to pytest. + ("coverage run -m pytest kernels/test_a.py", "kernels/test_a.py", True), + # `python -c '' file.py` runs the code string; `file.py` is argv. + ("python -c 'pass' kernels/test_a.py", "kernels/test_a.py", False), + ("python -c 'import sys' kernels/test_a.py", "kernels/test_a.py", False), + # a `-m pytest` *after* the `-c` code operand is that program's argv too. + ("python -c 'pass' -m pytest kernels/test_a.py", "kernels/test_a.py", False), + # ...but `-c` as the *script's* own arg still tethers the script. + ("python kernels/test_a.py -c config.py", "kernels/test_a.py", True), + # ...and pytest's own `-c ` (after `-m pytest`) is fine. + ("python -m pytest kernels/test_a.py -c setup.cfg", "kernels/test_a.py", True), + ], +) +def test_direct_runners(command, test_file, expected): + assert runs(command, test_file) is expected + + +# --------------------------------------------------------------------------- # +# Commands that must contribute no coverage +# --------------------------------------------------------------------------- # + + +@pytest.mark.parametrize( + "command", + [ + "export VLLM_WORKER_MULTIPROC_METHOD=spawn", + "pip install pytest-timeout pytest-forked", + "uv pip install -r requirements/test/cuda.in", + "bash tests/does_not_exist.sh", + "echo 'pytest kernels/test_a.py'", + # `pytest` as an argument to another command - `echo` runs, not pytest. + "echo pytest kernels/test_a.py", + 'pytest -v -s "unbalanced', + # A `.sh` that is only an *argument* (Buildkite printing / copying a + # filename) is not a script invocation. + "echo tests/foo.sh", + "cp scripts/run_tests.sh /tmp/", + # A `find` whose output never reaches pytest is not a test selection. + "find kernels -name 'test_*.py'", + "find kernels -name 'test_*.py' | wc -l", + "find kernels -name 'test_*.py' -delete", + "find kernels -name 'test_*.py' | xargs echo pytest", + "find kernels -name 'test_*.py' -exec pytest {}", + ], +) +def test_non_test_commands_contribute_nothing(command): + assert _parse_command(command) == [] or not any( + sel.runs("kernels/test_a.py") for sel in _parse_command(command) + ) + + +# --------------------------------------------------------------------------- # +# Job-level yaml parsing +# --------------------------------------------------------------------------- # + + +def _write_area(tmp_path, monkeypatch, body: str): + mod = checker + area_dir = tmp_path / "test_areas" + area_dir.mkdir(exist_ok=True) + (area_dir / "synthetic.yaml").write_text(textwrap.dedent(body)) + monkeypatch.setattr(mod, "TEST_AREAS_DIR", area_dir) + # Keep the real test-amd.yaml out of these isolated parsing tests. + monkeypatch.setattr(mod, "TEST_AMD_YAML", tmp_path / "no-such-test-amd.yaml") + # The yaml-error path reports a repo-relative name, so REPO_ROOT has to + # contain the synthetic area too. + monkeypatch.setattr(mod, "REPO_ROOT", tmp_path) + return load_selections() + + +def test_plain_step_commands_are_collected(tmp_path, monkeypatch): + selections = _write_area( + tmp_path, + monkeypatch, + """ + steps: + - label: ":nvidia: (H200) Kernels" + commands: + - pytest -v -s kernels + """, + ) + assert is_tethered("kernels/test_a.py", selections) + + +def test_singular_command_key_is_collected(tmp_path, monkeypatch): + selections = _write_area( + tmp_path, + monkeypatch, + """ + steps: + - label: "Singular" + command: pytest -v -s kernels/test_a.py + """, + ) + assert is_tethered("kernels/test_a.py", selections) + + +def test_test_amd_yaml_coverage_counts_as_tethered(tmp_path, monkeypatch): + """A test wired only into the legacy test-amd.yaml is still run by CI.""" + mod = checker + (tmp_path / "test_areas").mkdir() + amd_yaml = tmp_path / "test-amd.yaml" + amd_yaml.write_text( + textwrap.dedent( + """ + steps: + - label: ":amd: (MI300) Quantization" + commands: + - VLLM_TEST_FORCE_LOAD_FORMAT=auto pytest -v -s quantization/ + - pytest -v -s rocm/test_moe_weight_replay.py + """ + ) + ) + monkeypatch.setattr(mod, "TEST_AREAS_DIR", tmp_path / "test_areas") + monkeypatch.setattr(mod, "TEST_AMD_YAML", amd_yaml) + monkeypatch.setattr(mod, "REPO_ROOT", tmp_path) + selections = load_selections() + assert is_tethered("rocm/test_moe_weight_replay.py", selections) + assert is_tethered("quantization/test_foo.py", selections) + + +def test_missing_test_amd_yaml_is_not_fatal(tmp_path, monkeypatch): + """When the migration finally deletes test-amd.yaml, the checker still runs.""" + mod = checker + (tmp_path / "test_areas").mkdir() + monkeypatch.setattr(mod, "TEST_AREAS_DIR", tmp_path / "test_areas") + monkeypatch.setattr(mod, "TEST_AMD_YAML", tmp_path / "gone.yaml") + monkeypatch.setattr(mod, "REPO_ROOT", tmp_path) + assert load_selections() == [] + + +def test_job_dirs_come_from_ci_config(tmp_path, monkeypatch): + """The pipeline directories are whatever `.buildkite/ci_config*.yaml` lists + under `job_dirs:` - so a test wired only into e.g. `hardware_tests/` counts, + and an unreadable config falls back to `test_areas/` alone.""" + mod = checker + bk = tmp_path / ".buildkite" + (bk / "test_areas").mkdir(parents=True) + (bk / "hardware_tests").mkdir() + (bk / "ci_config.yaml").write_text("job_dirs:\n - .buildkite/hardware_tests\n") + (bk / "ci_config_broken.yaml").write_text("job_dirs: [oops\n") # ignored + (bk / "hardware_tests" / "cpu.yaml").write_text( + textwrap.dedent( + """ + steps: + - label: "CPU" + commands: + - pytest -v -s v1/e2e/test_cpu_thing.py + """ + ) + ) + monkeypatch.setattr(mod, "REPO_ROOT", tmp_path) + monkeypatch.setattr(mod, "TEST_AREAS_DIR", bk / "test_areas") + monkeypatch.setattr(mod, "TEST_AMD_YAML", tmp_path / "no-amd.yaml") + + assert (bk / "hardware_tests") in checker._pipeline_job_dirs() + assert is_tethered("v1/e2e/test_cpu_thing.py", load_selections()) + + +def test_step_level_ignore_is_honored(tmp_path, monkeypatch): + selections = _write_area( + tmp_path, + monkeypatch, + """ + steps: + - label: "Kernels" + commands: + - pytest -v -s kernels --ignore=kernels/test_a.py + """, + ) + assert not is_tethered("kernels/test_a.py", selections) + assert is_tethered("kernels/test_b.py", selections) + + +def test_amd_mirror_only_command_is_collected(tmp_path, monkeypatch): + """A mirror that overrides `commands` runs tests the base step never does.""" + selections = _write_area( + tmp_path, + monkeypatch, + """ + steps: + - label: ":nvidia: (H200) Base" + commands: + - pytest -v -s kernels/test_base.py + mirror: + amd: + label: ":amd: (MI300) Base" + commands: + - pytest -v -s kernels/test_amd_only.py + """, + ) + assert is_tethered("kernels/test_base.py", selections) + assert is_tethered("kernels/test_amd_only.py", selections) + + +def test_shell_script_continuation_vars_and_nesting(tmp_path, monkeypatch): + """The v1 P/D integration scripts name their test through a `\\`-continued + line, a literal shell var, and a nested `bash "$SCRIPT"`. Each indirection + has to be followed or the check falsely reports the test untethered.""" + scripts = tmp_path / "tests" / "v1" / "kv_connector" / "nixl_integration" + scripts.mkdir(parents=True) + (scripts / "inner.sh").write_text( + "python3 -m pytest -s -v \\\n" + ' "${GIT_ROOT}/tests/v1/kv_connector/nixl_integration/test_inner.py"\n' + ) + (scripts / "sweep.sh").write_text( + 'SCRIPT="v1/kv_connector/nixl_integration/inner.sh"\n' + 'IMPORT_CANARY="v1/kv_connector/nixl_integration/test_canary.py"\n' + 'LITERAL="v1/kv_connector/nixl_integration/test_not_run.py"\n' + 'python3 -m pytest -s -x "${IMPORT_CANARY}"\n' + # single-quoted: the shell passes a literal $LITERAL, collecting nothing + "python3 -m pytest -s -x '${LITERAL}'\n" + 'if ! env FOO=1 bash "${SCRIPT}"; then exit 1; fi\n' + ) + monkeypatch.setattr(checker, "REPO_ROOT", tmp_path) + monkeypatch.setattr(checker, "TESTS_DIR", tmp_path / "tests") + + selections = _parse_command("bash v1/kv_connector/nixl_integration/sweep.sh") + + def tethered(path): + return any(s.runs(path) for s in selections) + + # `\`-continuation + $VAR-prefixed absolute path, resolved through a nested + # `env FOO=1 bash "$SCRIPT"`: + assert tethered("v1/kv_connector/nixl_integration/test_inner.py") + # literal `VAR=path` + double-quoted `"$VAR"` (the import-canary pattern): + assert tethered("v1/kv_connector/nixl_integration/test_canary.py") + # single-quoted `'$VAR'` is NOT expanded by the shell, so not tethered: + assert not tethered("v1/kv_connector/nixl_integration/test_not_run.py") + + +def test_bash_dash_c_does_not_follow_a_script_operand(tmp_path, monkeypatch): + """`bash -c '' foo.sh` runs the `-c` string; `foo.sh` is `$0` and is + never executed, so its contents must not count as coverage.""" + (tmp_path / "tests").mkdir() + (tmp_path / "tests" / "decoy.sh").write_text("pytest -v -s lora/test_decoy.py\n") + monkeypatch.setattr(checker, "REPO_ROOT", tmp_path) + monkeypatch.setattr(checker, "TESTS_DIR", tmp_path / "tests") + + with_c = _parse_command("bash -c 'echo hi' tests/decoy.sh") + assert not any(s.runs("lora/test_decoy.py") for s in with_c) + # ...but a plain `bash tests/decoy.sh` still follows it. + plain = _parse_command("bash tests/decoy.sh") + assert any(s.runs("lora/test_decoy.py") for s in plain) + # ...and a `-c` that is the *script's* own flag (not bash's) is not `bash -c`. + script_flag = _parse_command("bash tests/decoy.sh -c models.txt") + assert any(s.runs("lora/test_decoy.py") for s in script_flag) + # A `bash -c` trailing arg that looks like a command string is `$0`, not run. + cmd_string_arg = _parse_command("bash -c 'echo ok' 'pytest lora/test_x.py'") + assert not any(s.runs("lora/test_x.py") for s in cmd_string_arg) + # ...but the `-c` code string *itself* is parsed - it is what runs. + code_runs = _parse_command("bash -c 'pytest lora/test_x.py'") + assert any(s.runs("lora/test_x.py") for s in code_runs) + # a combined short-option group resolves the `-c` operand with `c` in any + # position (`-ec`, `-ce`), and its trailing args are still `$0`, not run. + for grp in ("-ec", "-ce", "-euxc"): + assert any( + s.runs("lora/test_x.py") + for s in _parse_command(f"bash {grp} 'pytest lora/test_x.py'") + ) + assert not any( + s.runs("lora/test_x.py") + for s in _parse_command("bash -ce 'echo ok' 'pytest lora/test_x.py'") + ) + + +def test_unparsable_yaml_is_fatal(tmp_path, monkeypatch): + """Silently skipping a bad yaml would drop its coverage and report false + untethered files, so it must fail loudly instead.""" + with pytest.raises(SystemExit): + _write_area(tmp_path, monkeypatch, "steps: [oops\n") + + +def test_non_mapping_yaml_is_fatal(tmp_path, monkeypatch): + """A yaml that parses but isn't a pipeline mapping (a bare list or scalar) + must raise the actionable error, not crash on ``.get``.""" + with pytest.raises(SystemExit): + _write_area(tmp_path, monkeypatch, "- just\n- a\n- list\n") + + +# --------------------------------------------------------------------------- # +# is_test_module / path normalization +# --------------------------------------------------------------------------- # + + +@pytest.mark.parametrize( + ("path", "expected"), + [ + ("tests/kernels/test_a.py", True), + ("tests/kernels/a_test.py", True), + ("tests/kernels/conftest.py", False), + ("tests/kernels/__init__.py", False), + ("tests/kernels/utils.py", False), + ("tests/kernels/test_a.txt", False), + ("vllm/kernels/test_a.py", False), + ], +) +def test_is_test_module(path, expected): + assert is_test_module(path) is expected + + +@pytest.mark.parametrize( + ("raw", "expected"), + [ + ("tests/kernels/test_a.py", "kernels/test_a.py"), + ("./kernels/test_a.py", "kernels/test_a.py"), + ("/vllm-workspace/tests/kernels/test_a.py", "kernels/test_a.py"), + ("'kernels/test_a.py'", "kernels/test_a.py"), + ("kernels/test_a.py::test_one", "kernels/test_a.py"), + ("kernels/", "kernels"), + ("${GIT_ROOT}/tests/kernels/test_a.py", "kernels/test_a.py"), + ("$GIT_ROOT/tests/kernels/test_a.py", "kernels/test_a.py"), + ], +) +def test_normalize_test_path(raw, expected): + assert normalize_test_path(raw) == expected + + +# --------------------------------------------------------------------------- # +# Whole-tree invariants +# --------------------------------------------------------------------------- # + + +def test_real_tree_parses_into_selections(): + assert len(load_selections()) > 200 + assert len(all_test_modules()) > 1000 + + +def test_no_selection_matches_everything(): + """A path arg that normalizes to '' or '.' would tether every test in the + repo and silently disarm the gate - the worst possible misparse.""" + for selection in load_selections(): + if isinstance(selection, PytestSelection): + offenders = [ + p + for p in selection.included_paths + if normalize_test_path(p) in ("", ".") + ] + assert not offenders, f"match-everything pytest arg: {offenders}" + else: + assert normalize_test_path(selection.root) not in ("", ".") + + +def test_tethering_is_deterministic(): + selections = load_selections() + modules = all_test_modules()[:200] + first = [is_tethered(m, selections) for m in modules] + assert first == [is_tethered(m, load_selections()) for m in modules] + + +def test_allowlist_entries_are_well_formed(): + allowlist = load_allowlist() + assert allowlist, "allowlist should not be empty while gaps remain" + assert all(is_test_module(path) for path in allowlist) + assert all(not path.startswith("/") for path in allowlist) + + +def test_real_allowlist_entries_all_have_a_reason(): + assert allowlist_entries_missing_reason() == [] + + +def test_bare_allowlist_entry_is_flagged(tmp_path, monkeypatch): + allowlist = tmp_path / "allowlist.txt" + allowlist.write_text( + "tests/a/test_ok.py # torchrun multi-GPU, no job\n" + "tests/b/test_bare.py\n" + " # a standalone comment line is fine\n" + ) + monkeypatch.setattr(checker, "ALLOWLIST_PATH", allowlist) + assert allowlist_entries_missing_reason() == ["tests/b/test_bare.py"] + + +def test_real_tree_has_no_unallowlisted_gaps(): + """Every test module is either collected by a job or allowlisted. + + Deliberately non-strict: a stale allowlist entry is advisory by design, so + asserting `strict=True` here would make unrelated allowlist cleanup block + CI - the exact behaviour the two-mode design avoids. + """ + assert run_full_scan(load_selections(), load_allowlist(), strict=False) == 0 + + +# --------------------------------------------------------------------------- # +# CLI behaviour +# --------------------------------------------------------------------------- # + + +@pytest.fixture +def fake_repo(tmp_path, monkeypatch): + """Point the module at a throwaway tree so the changed-files path can be + exercised without writing test files into the real repo.""" + mod = checker + (tmp_path / "tests" / "kernels").mkdir(parents=True) + (tmp_path / "tests" / "kernels" / "test_new.py").write_text("def test_x(): pass\n") + monkeypatch.setattr(mod, "REPO_ROOT", tmp_path) + monkeypatch.setattr(mod, "TESTS_DIR", tmp_path / "tests") + monkeypatch.setattr(mod, "ALLOWLIST_PATH", tmp_path / "allowlist.txt") + # The throwaway tree is not a git checkout, so the tracked-file inventory + # has to be stubbed rather than shelled out. + monkeypatch.setattr(mod, "all_test_modules", lambda: ["tests/kernels/test_new.py"]) + return tmp_path + + +def test_untethered_changed_file_fails(fake_repo, capsys): + rc = run_changed_files_check(["tests/kernels/test_new.py"], [], set()) + assert rc == 1 + assert "is not run by any Buildkite job" in capsys.readouterr().out + + +def test_tethered_changed_file_passes(fake_repo): + selections = _parse_command("pytest -v -s kernels") + assert ( + run_changed_files_check(["tests/kernels/test_new.py"], selections, set()) == 0 + ) + + +def test_allowlisted_changed_file_passes(fake_repo): + allowlist = {"tests/kernels/test_new.py"} + assert run_changed_files_check(["tests/kernels/test_new.py"], [], allowlist) == 0 + + +def test_non_test_and_missing_paths_are_ignored(fake_repo): + paths = ["tests/kernels/conftest.py", "vllm/config.py", "tests/kernels/gone.py"] + assert run_changed_files_check(paths, [], set()) == 0 + + +def test_stale_allowlist_entries_are_advisory_but_reported(fake_repo, capsys): + """Dangling entries must not block an unrelated PR, but they have to be + printed - the hook sets `verbose: true` so a passing run still shows them.""" + allowlist = {"tests/kernels/gone.py"} + rc = run_full_scan(_parse_command("pytest -v -s kernels"), allowlist, strict=False) + out = capsys.readouterr().out + assert rc == 0 + assert "no longer exists" in out + + +def test_stale_allowlist_entries_are_fatal_under_all(fake_repo, capsys): + allowlist = {"tests/kernels/gone.py"} + rc = run_full_scan(_parse_command("pytest -v -s kernels"), allowlist, strict=True) + assert rc == 1 + assert "no longer exists" in capsys.readouterr().out + + +def test_now_tethered_allowlist_entry_is_reported(fake_repo, capsys): + allowlist = {"tests/kernels/test_new.py"} + rc = run_full_scan(_parse_command("pytest -v -s kernels"), allowlist, strict=True) + assert rc == 1 + assert "is now tethered" in capsys.readouterr().out + + +def test_main_passes_a_tethered_file(monkeypatch): + """The CLI's normal pre-commit path: a tethered file exits 0. + + `--all` is not asserted here on purpose - it implies `strict=True`, so a + stale allowlist entry would turn unrelated cleanup into a CI failure. + """ + monkeypatch.setattr( + "sys.argv", + ["check_test_tethering.py", "tests/tools/test_check_test_tethering.py"], + ) + assert main() == 0 + + +def test_main_flags_an_untethered_file(monkeypatch): + # Must be named like a test module, or the check correctly skips it. + orphan = REPO_ROOT / "tests" / "test_tethering_probe_orphan.py" + orphan.write_text("def test_x(): pass\n") + monkeypatch.setattr("sys.argv", ["check_test_tethering.py", str(orphan)]) + try: + assert main() == 1 + finally: + orphan.unlink() + + +def test_to_repo_relative_accepts_absolute_paths(): + absolute = str(REPO_ROOT / "tests" / "kernels" / "test_a.py") + assert _to_repo_relative(absolute) == "tests/kernels/test_a.py" diff --git a/tools/__init__.py b/tools/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tools/pre_commit/__init__.py b/tools/pre_commit/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tools/pre_commit/check_test_tethering.py b/tools/pre_commit/check_test_tethering.py new file mode 100644 index 0000000000..ee080a6757 --- /dev/null +++ b/tools/pre_commit/check_test_tethering.py @@ -0,0 +1,925 @@ +# SPDX-License-Identifier: Apache-2.0 +# SPDX-FileCopyrightText: Copyright contributors to the vLLM project +"""Check that test files are wired into ("tethered to") the Buildkite CI. + +A test file is *tethered* when at least one Buildkite job actually collects it - +some job's ``commands`` run ``pytest`` (or ``python`` / ``torchrun``, or a +``find ... | xargs pytest`` pipeline) over a path that includes the file. A test +that is collected by no job silently never runs: it passes review, merges, and +rots. + +How it works +------------ +1. Parse every job in every Buildkite pipeline yaml (and each ``mirror.amd`` + sub-step), and turn each shell command into a :class:`Selection` describing + which test files it runs. The pipeline directories are read from + ``.buildkite/ci_config*.yaml`` (``job_dirs:``) - currently ``test_areas/``, + ``hardware_tests/`` and ``intel_jobs/`` across the CUDA, ROCm and Intel + pipelines - plus the legacy ``.buildkite/test-amd.yaml``. +2. Build the inventory of test files under ``tests/``. +3. A file is a problem if no selection runs it and it is not in the allowlist + (``tools/pre_commit/test_tethering_allowlist.txt`` - the set of pre-existing + gaps, which is only ever meant to shrink). Every allowlist entry must carry a + trailing ``# ``; a bare path line is itself an error. + +Modes +----- +* **changed-files** (default): pre-commit passes the changed test files as + arguments; each one must be tethered or allowlisted. +* **full scan**: ``--all``, or automatically whenever the change set touches a + pipeline yaml or the allowlist itself - because editing a job command can + orphan a test that is not otherwise in the diff. The full scan also reports + allowlist entries that have since become tethered, or whose file is gone, so + the list stays honest. + +Coverage from *any* of those pipelines counts - the question is "does some +Buildkite job run this test", not "which pipeline". ``test-amd.yaml`` is being +folded into ``test_areas/`` a group at a time; when it is gone the checker stops +parsing it automatically. + +Usage:: + + python tools/pre_commit/check_test_tethering.py [FILES...] + python tools/pre_commit/check_test_tethering.py --all +""" + +import argparse +import fnmatch +import shlex +import subprocess +import sys +from dataclasses import dataclass, field +from pathlib import Path + +import regex as re +import yaml + +REPO_ROOT = Path(__file__).resolve().parents[2] +BUILDKITE_DIR = REPO_ROOT / ".buildkite" +# Fallback pipeline dir, always parsed even if ``ci_config*.yaml`` can't be read. +TEST_AREAS_DIR = BUILDKITE_DIR / "test_areas" +# The legacy hand-maintained AMD pipeline. It is being migrated into +# ``test_areas/`` piecemeal; until that finishes, a test wired only into this +# file is still genuinely run by CI, so it counts as tethered. When the file is +# removed, ``_pipeline_yaml_paths()`` stops appending it automatically. +TEST_AMD_YAML = BUILDKITE_DIR / "test-amd.yaml" +TESTS_DIR = REPO_ROOT / "tests" +ALLOWLIST_PATH = Path(__file__).resolve().parent / "test_tethering_allowlist.txt" + +# `pytest` options that take a value in the *next* token, so that token must not +# be mistaken for a test path (e.g. `-k expr`, `--shard-id 3`). Options that only +# ever take their value with `=` (`--cov=...`) don't belong here - a bare +# `--cov` takes no value and would wrongly swallow the following test path. +PYTEST_OPTIONS_WITH_VALUE = { + "-k", "-m", "-p", "-n", "-c", "-o", "-W", "-r", + "--shard-id", "--num-shards", "--dist", "--durations", "--maxfail", + "--timeout", "--rootdir", "--junitxml", "--tb", +} # fmt: skip + +# `pytest` options whose value is a path that is *removed* from collection at +# file granularity. ``--ignore`` takes a path prefix; ``--ignore-glob`` takes a +# glob that pytest matches against the whole path with ``fnmatch`` (so its ``*`` +# *does* cross a ``/``), which is why the two are modelled separately. +PYTEST_IGNORE_OPTIONS = {"--ignore", "--ignore-glob"} + +# `--deselect` takes a value too, but it removes individual node IDs - a file +# with one parametrization deselected is still collected and still "run". +PYTEST_DESELECT_OPTION = "--deselect" + +PYTEST_COMMANDS = {"pytest", "py.test"} + +# A leading ``$VAR/`` or ``${VAR}/`` path prefix - CI shell scripts routinely +# name a test by an absolute path rooted at a shell var +# (``"${GIT_ROOT}/tests/v1/..."``). The prefix is dropped before the path is +# interpreted; an unresolved ``$`` anywhere else still makes the token unusable. +_LEADING_VAR_PREFIX = re.compile(r"^\$\{?\w+\}?/") + +# Commands that execute a Python file directly. Only the *first* path argument is +# the script that runs; everything after it is that script's own argv, not more +# test files (``python tensorize.py --suffix v1`` runs one file, not ``tests/v1``). +FILE_RUNNER_COMMANDS = {"python", "python3", "torchrun", "coverage"} + + +# --------------------------------------------------------------------------- # +# Shell-token helpers +# --------------------------------------------------------------------------- # + + +def _is_env_assignment(token: str) -> bool: + """True for a leading ``FOO=bar`` shell env-var assignment - the kind some + commands put before ``pytest`` (``VLLM_TEST_FORCE_LOAD_FORMAT=auto pytest``). + """ + if "=" not in token: + return False + name = token.split("=", 1)[0] + return bool(name) and name.replace("_", "").isupper() + + +def normalize_test_path(raw: str) -> str: + """Reduce a path as it appears in a command to one comparable form: + relative to ``tests/``, no surrounding quotes, no ``::nodeid`` selector, no + trailing slash. + + Commands run from either the repo root or from ``tests/``, so both a + ``tests/`` and a ``/vllm-workspace/tests/`` prefix are stripped, as is a + leading ``$VAR/`` (``"${GIT_ROOT}/tests/..."``). + """ + path = raw.strip().strip("'\"") + path = path.split("::", 1)[0] + path = _LEADING_VAR_PREFIX.sub("", path) + for prefix in ("./", "tests/", "/vllm-workspace/tests/"): + if path.startswith(prefix): + path = path[len(prefix) :] + return path.rstrip("/") + + +def _token_is_test_path(token: str) -> bool: + """True if a shell token is a path argument rather than a flag, an env-var + assignment, a Buildkite ``$VAR``, or a ``-m`` / ``-k`` expression value. + + A token with a ``/`` or a ``.py`` ending is always a path. A bare word + (``samplers``) only counts if it is really a directory under ``tests/``. A + leading ``$VAR/`` is dropped first, but a ``$`` anywhere else means the token + can't be resolved to a known path. + """ + if not token or token.startswith("-"): + return False + if _is_env_assignment(token): + return False + token = _LEADING_VAR_PREFIX.sub("", token) + if "$" in token: + return False + if "/" in token or token.endswith(".py"): + return True + return (TESTS_DIR / normalize_test_path(token)).is_dir() + + +def _path_is_within(parent: str, path: str) -> bool: + """True if ``path`` is ``parent`` itself or nested underneath it. A ``''`` / + ``'.'`` parent (a command run with no path arg) contains everything.""" + if parent in ("", "."): + return True + return path == parent or path.startswith(parent + "/") + + +def _glob_match_path(pattern: str, path: str) -> bool: + """Like :func:`fnmatch.fnmatch`, but ``*`` and ``?`` do not match across a + ``/`` - the way the shell and ``pytest`` treat an unexpanded glob path + argument (``kernels/test_foo_*.py`` selects one directory, not the tree). + A ``**`` component keeps the cross-separator meaning.""" + if not any(ch in pattern for ch in "*?["): + return pattern == path + if "**" in pattern: + return fnmatch.fnmatch(path, pattern) + pattern_parts = pattern.split("/") + path_parts = path.split("/") + return len(pattern_parts) == len(path_parts) and all( + fnmatch.fnmatch(part, glob) for part, glob in zip(path_parts, pattern_parts) + ) + + +# --------------------------------------------------------------------------- # +# Selections - "which test files does this command run?" +# --------------------------------------------------------------------------- # + + +@dataclass +class PytestSelection: + """A ``pytest``-style command: run everything under ``included_paths`` + except what an ``--ignore`` (path prefix) or ``--ignore-glob`` (whole-path + ``fnmatch``) removes.""" + + included_paths: list[str] = field(default_factory=list) + ignored_paths: list[str] = field(default_factory=list) + ignored_globs: list[str] = field(default_factory=list) + + def runs(self, test_file: str) -> bool: + included = any(self._arg_covers(arg, test_file) for arg in self.included_paths) + if not included: + return False + if any(self._arg_covers(arg, test_file) for arg in self.ignored_paths): + return False + # `--ignore-glob` is a plain `fnmatch` over the whole path - `*_tp.py` + # drops `kernels/nested/test_a_tp.py`, unlike a positional glob arg. + return not any( + fnmatch.fnmatch(test_file, normalize_test_path(glob)) + for glob in self.ignored_globs + ) + + @staticmethod + def _arg_covers(path_arg: str, test_file: str) -> bool: + """True if a single pytest path argument selects ``test_file`` - by + exact match, by being a parent directory, or as a glob.""" + path_arg = normalize_test_path(path_arg) + if _path_is_within(path_arg, test_file): + return True + return _glob_match_path(path_arg, test_file) + + +@dataclass +class FindSelection: + """A ``find -name ... [-not -name ...] [-maxdepth N] | xargs pytest`` + pipeline. ``pytest`` runs one file per ``find`` hit, so the ``find`` + expression alone defines the selected set.""" + + root: str + name_globs: list[str] = field(default_factory=list) + exclude_name_globs: list[str] = field(default_factory=list) + max_depth: int | None = None + + def runs(self, test_file: str) -> bool: + root = normalize_test_path(self.root) + if not _path_is_within(root, test_file): + return False + + # `find ` puts a direct child at depth 1, whether is a real + # subdir ("compile") or the tests/ root itself ("" / "."). + below = test_file if root in ("", ".") else test_file[len(root) :] + depth_below_root = below.lstrip("/").count("/") + 1 + if self.max_depth is not None and depth_below_root > self.max_depth: + return False + + filename = test_file.rsplit("/", 1)[-1] + if self.name_globs and not _glob_any(filename, self.name_globs): + return False + return not _glob_any(filename, self.exclude_name_globs) + + +Selection = PytestSelection | FindSelection + + +def _glob_any(name: str, globs: list[str]) -> bool: + return any(fnmatch.fnmatch(name, glob) for glob in globs) + + +# --------------------------------------------------------------------------- # +# Parsing CI commands into selections +# --------------------------------------------------------------------------- # + + +def _parse_pytest_command(tokens: list[str]) -> PytestSelection: + """Read a ``pytest ...`` invocation's tokens into a :class:`PytestSelection`. + + Positional path args are the included paths; ``--ignore`` values (and a + bare-path ``--deselect``) are ignored paths, ``--ignore-glob`` values are + ignored globs. Value-taking options and their values, and every other flag, + are skipped. ``-k`` / ``-m`` filtering is intentionally *not* modelled - a + file that a marker expression narrows is still "run". + """ + selection = PytestSelection() + + # Advance past the pytest token itself (and any env-var / wrapper prefix). + tokens = iter(tokens) + for token in tokens: + if token in PYTEST_COMMANDS: + break + + for token in tokens: + opt, eq, inline = token.partition("=") + if opt in PYTEST_IGNORE_OPTIONS: + value = inline if eq else next(tokens, "") + if opt == "--ignore-glob": + selection.ignored_globs.append(value) + else: + selection.ignored_paths.append(value) + elif opt == PYTEST_DESELECT_OPTION: + value = inline if eq else next(tokens, "") + # A `path::node_id` deselect leaves the file collected; only a + # bare-path deselect removes anything at file granularity. + if value and "::" not in value: + selection.ignored_paths.append(value) + elif token in PYTEST_OPTIONS_WITH_VALUE: + next(tokens, None) # consume and discard the option's value + elif token.startswith("-"): + continue + elif _token_is_test_path(token): + selection.included_paths.append(token) + + return selection + + +def _parse_find_command(tokens: list[str]) -> FindSelection | None: + """Read a ``find -name ...`` command's tokens into a + :class:`FindSelection`, or return ``None`` if there is no usable ``find``.""" + if "find" not in tokens: + return None + + root: str | None = None + name_globs: list[str] = [] + exclude_name_globs: list[str] = [] + max_depth: int | None = None + + tokens = iter(tokens[tokens.index("find") + 1 :]) + negated = False + for token in tokens: + if token in ("-not", "!"): + negated = True + continue + if token in ("-name", "-iname"): + glob = next(tokens, "").strip("'\"") + (exclude_name_globs if negated else name_globs).append(glob) + elif token == "-maxdepth": + depth = next(tokens, "") + max_depth = int(depth) if depth.isdigit() else None + elif ( + root is None and not token.startswith("-") and token not in PYTEST_COMMANDS + ): + root = token + negated = False + + if root is None: + return None + return FindSelection(root, name_globs, exclude_name_globs, max_depth) + + +def _dash_c_code_operand(tokens: list[str]) -> str | None: + """For ``bash -c '' [$0] [args]`` / ``sh -c ...`` / a combined short + group with ``c`` anywhere in it (``-ec``, ``-ce``, ``-euxc``), the ```` + string the shell runs. ``None`` when this is not a ``-c`` invocation (plain + ``bash script.sh`` / ``bash -x script.sh``). Everything after the code + operand is a positional parameter (``$0``, ``$1``, ...), not a script or + command that executes.""" + if not tokens or tokens[0].rsplit("/", 1)[-1] not in ("bash", "sh"): + return None + for i, token in enumerate(tokens[1:], start=1): + if not token.startswith("-"): + return None # script operand reached; a later `-c` is *its* flag + # a short-option group (single dash) with `c` among its flags. + if not token.startswith("--") and "c" in token[1:]: + return tokens[i + 1] if i + 1 < len(tokens) else None + return None + + +def _parse_shell_script(tokens: list[str], visited: set[str]) -> list[Selection]: + """Follow a ``bash tests/foo.sh`` command: read the script and parse the + pytest / runner lines inside it. ``visited`` stops a script that (directly or + otherwise) refers back to itself; nesting is one level deep in practice. + """ + if _dash_c_code_operand(tokens) is not None: + return [] # the `-c` code string is parsed by the caller, not a file + script_arg = next((t for t in tokens if t.endswith(".sh")), None) + if script_arg is None: + return [] + + rel = script_arg.strip().strip("'\"").replace("/vllm-workspace/", "") + rel = rel.replace("vllm-workspace/", "").removeprefix("./").removeprefix("/") + candidates = [REPO_ROOT / rel, TESTS_DIR / normalize_test_path(rel)] + script_path = next((p for p in candidates if p.is_file()), None) + if script_path is None or str(script_path) in visited: + return [] + visited.add(str(script_path)) + + # Join `\`-continued lines (a `pytest ... \` with its path on the next line + # is common), then track literal `VAR=value` assignments so a later + # `pytest "$VAR"` - the import-canary pattern - resolves to a real path. + text = script_path.read_text().replace("\\\n", " ") + var_values: dict[str, str] = {} + selections: list[Selection] = [] + for line in text.splitlines(): + line = line.strip() + if not line or line.startswith("#"): + continue + assignment = re.match(r"^(\w+)=(\S+)$", line) + if assignment: + var_values[assignment.group(1)] = assignment.group(2).strip("'\"") + continue + if var_values: + line = _expand_vars(line, var_values) + selections.extend(_parse_command(line, visited)) + return selections + + +def _expand_vars(line: str, var_values: dict[str, str]) -> str: + """Substitute known ``$VAR`` / ``${VAR}`` in a script line - but not inside + single quotes, where the shell does not expand (``pytest '$X'`` passes a + literal ``$X``, collecting nothing).""" + + def sub(segment: str) -> str: + return re.sub( + r"\$\{(\w+)\}|\$(\w+)", + lambda m: var_values.get(m.group(1) or m.group(2), m.group(0)), + segment, + ) + + # Splitting on `'`: even-indexed segments are outside single quotes, odd are + # inside. (A `'` within double quotes splits wrongly, but that only ever + # drops a substitution - the safe direction.) + parts = line.split("'") + return "'".join(sub(p) if i % 2 == 0 else p for i, p in enumerate(parts)) + + +# Shell tokens (or runs of them) that separate one sub-command from the next. +_SHELL_SEP_CHARS = set("|&;()<>") +_RUNNER_KEYWORDS = PYTEST_COMMANDS | FILE_RUNNER_COMMANDS + +# Tokens that can precede the real command word without being it. +_COMMAND_WRAPPERS = {"if", "!", "then", "time", "exec", "nice", "command", "builtin"} + + +def _effective_command_index(tokens: list[str]) -> int: + """Index of the token that names the command actually executed, past leading + ``FOO=bar`` assignments and simple wrappers - ``if ! env FOO=1 bash x.sh`` + resolves to the ``bash``, so ``echo x.sh`` stays ``echo`` and is not + followed. Returns ``len(tokens)`` when nothing executable is left.""" + i = 0 + while i < len(tokens): + token = tokens[i] + if _is_env_assignment(token) or token in _COMMAND_WRAPPERS: + i += 1 + elif token == "env": + i += 1 + while i < len(tokens) and ( + _is_env_assignment(tokens[i]) or "$" in tokens[i] or tokens[i] == "-" + ): + i += 1 + elif token == "timeout" and i + 1 < len(tokens): + i += 1 + while i < len(tokens) and tokens[i].startswith("-"): + # `-k` / `--kill-after` / `-s` / `--signal` take a value. + if tokens[i] in ("-k", "--kill-after", "-s", "--signal"): + i += 1 + i += 1 + i += 1 # the DURATION operand + else: + break + return i + + +def _split_shell_stages(command: str) -> list[tuple[str, list[str]]]: + """Tokenize a shell line - honouring quotes - and split it at the shell + operators, tagging each stage with the operator token that *precedes* it + (``""`` for the first stage, then ``"|"`` / ``"&&"`` / ``"||"`` / ``";"`` / + ``"&"`` / a subshell paren). Knowing which operator joins two stages is what + lets a ``find`` sweep count only when it is genuinely piped into ``pytest``, + not merely on the same line as one. A line that cannot be tokenized + (unbalanced quote, ...) yields nothing: better to miss a selection than to + misparse one. Splitting *after* tokenizing means an operator inside a quoted + argument stays part of that argument.""" + lexer = shlex.shlex(command, posix=True, punctuation_chars=True) + lexer.whitespace_split = True + try: + tokens = list(lexer) + except ValueError: + return [] + + stages: list[tuple[str, list[str]]] = [("", [])] + in_exec = False # inside a `find -exec ... \;` action - its `;` terminates + for token in tokens: + if in_exec and token in (";", "+"): + stages[-1][1].append(token) + in_exec = False + elif token and set(token) <= _SHELL_SEP_CHARS: + stages.append((token, [])) + in_exec = False + else: + stages[-1][1].append(token) + if token in ("-exec", "-execdir"): + in_exec = True + return [(sep, sub) for sep, sub in stages if sub] + + +def _looks_like_command_string(token: str) -> bool: + """True for a single token that is itself a command line - e.g. a quoted + ``"pytest a.py && torchrun b.py"`` passed as an argument to a runner + script.""" + return " " in token and any(word in _RUNNER_KEYWORDS for word in token.split()) + + +def _classify_subcommand(tokens: list[str], visited: set[str]) -> list[Selection]: + """Map one already-tokenized sub-command to the selections it implies: + find / pytest / ``bash