From 09f542b23a7d52049c5f2b3a9f2d2f17a5979a02 Mon Sep 17 00:00:00 2001 From: Shuwen Wang <47200617+alphabetc1@users.noreply.github.com> Date: Sat, 5 Sep 2026 13:38:11 +0800 Subject: [PATCH] [CI] Add /rerun-test --changed to rerun every test file a PR modifies (#37618) Co-authored-by: Claude Opus 5 (1M context) Co-authored-by: Alison Shao --- .../developer_guide/contribution_guide.mdx | 2 +- scripts/ci/utils/slash_command_handler.py | 112 +++++++++++++++--- .../unit/tools/test_slash_command_handler.py | 104 +++++++++++++++- 3 files changed, 199 insertions(+), 19 deletions(-) diff --git a/docs/docs/developer_guide/contribution_guide.mdx b/docs/docs/developer_guide/contribution_guide.mdx index f320cbf68..3a74e13cf 100644 --- a/docs/docs/developer_guide/contribution_guide.mdx +++ b/docs/docs/developer_guide/contribution_guide.mdx @@ -122,7 +122,7 @@ For CI to run on a pull request, it must have the "run-ci" label. Authorized use - `/tag-run-ci-label`: Adds the "run-ci" label. Only **future** commits trigger CI; the current commit is unaffected. Add the `extra` argument (`/tag-run-ci-label extra`) to additionally apply the "run-ci-extra" label, opting the PR into the extra test workflow (`pr-test-extra.yml`). - `/rerun-failed-ci`: Reruns workflows from the latest commit with conclusion **failed, skipped, cancelled, or timed out**. - `/tag-and-rerun-ci`: Runs both. Use this on a fresh PR to kick off CI on the current commit — `/tag-run-ci-label` alone won't. Accepts the same `extra` argument (`/tag-and-rerun-ci extra`). -- `/rerun-test [ ...]`: Reruns one or more specific tests directly. A spec may select a file, class, or method using `::[.]`. Multiple specs and file globs are supported. Examples: `/rerun-test test_srt_endpoint.py`, `/rerun-test registered/core/test_srt_endpoint.py::TestSRTEndpoint.test_simple_decode`, `/rerun-test test_a.py test_b.py`, or `/rerun-test test_*backend*.py`. +- `/rerun-test [ ...]`: Reruns one or more specific tests directly. A spec may select a file, class, or method using `::[.]`. Multiple specs and file globs are supported. `--changed` (short form `-c`) adds every test file the PR itself adds or modifies (under `test/registered/` or the multimodal test directory), so a PR that touches several tests can rerun them all without listing them. Examples: `/rerun-test test_srt_endpoint.py`, `/rerun-test registered/core/test_srt_endpoint.py::TestSRTEndpoint.test_simple_decode`, `/rerun-test test_a.py test_b.py`, `/rerun-test test_*backend*.py`, `/rerun-test --changed`, or `/rerun-test -c`. - `/rerun-group [ ...]`: Expands one or more registered test groups (for example, `/rerun-group hicache`) and dispatches their tests through the same selective-rerun workflow. The rerun commands have the following permission rules: diff --git a/scripts/ci/utils/slash_command_handler.py b/scripts/ci/utils/slash_command_handler.py index 5e288c367..b2f3f1ab8 100644 --- a/scripts/ci/utils/slash_command_handler.py +++ b/scripts/ci/utils/slash_command_handler.py @@ -30,6 +30,8 @@ PERMISSIONS_FILE_PATH = ".github/CI_PERMISSIONS.json" TEST_GROUPS_FILE_PATH = "scripts/ci/rerun_test_groups.json" PRECISION_BASELINE_TEST = "registered/debug_utils/test_nightly_precision_regression.py" PRECISION_BASELINE_REFRESH_FLAG = "--refresh-precision-baseline" +CHANGED_TESTS_FLAG = "--changed" +CHANGED_TESTS_SHORT_FLAG = "-c" MAINTENANCE_ISSUE_NUMBER = 21065 @@ -588,8 +590,8 @@ def expand_glob_spec(file_part): Globs are matched against the same locations resolve_test_file() searches — test/registered/ and the multimodal_gen test dir — so e.g. `test_*backend*.py` reruns every backend test without hand-enumerating - each file. Two constraints keep a broad pattern from pulling in non-tests: - a match must live under a known test root and be named `test_*.py`. + each file. `_is_rerunnable_test_path` keeps a broad pattern from pulling in + non-tests. glob's `*` matches path separators only via `**`, so a bare pattern is searched recursively under each root; a path-ful pattern is anchored. @@ -628,19 +630,11 @@ def expand_glob_spec(file_part): expanded.add(p) matches = expanded - def _under_test_root(path): - return path.startswith("test/registered/") or path.startswith( - MULTIMODAL_TEST_DIR + "/" - ) - files = sorted( { os.path.normpath(p) for p in matches - if os.path.isfile(p) - and os.path.basename(p).startswith("test_") - and p.endswith(".py") - and _under_test_root(os.path.normpath(p)) + if os.path.isfile(p) and _is_rerunnable_test_path(os.path.normpath(p)) } ) if not files: @@ -652,6 +646,76 @@ def expand_glob_spec(file_part): return files, None +def _collects_pytest_tests(path): + """Whether a test file defines anything pytest would collect.""" + if not os.path.isfile(path): + # Fork-added file, absent from the handler's main checkout; leave it for + # resolve_test_file() to report as `File not found`. + return True + with open(path, "r", encoding="utf-8", errors="replace") as f: + content = f.read() + return ( + re.search(r"^\s*((async )?def test_|class Test)", content, re.MULTILINE) + is not None + ) + + +def _is_rerunnable_test_path(path): + """A repo-relative test file /rerun-test may select on its own (glob or --changed).""" + under_test_root = path.startswith("test/registered/") or path.startswith( + MULTIMODAL_TEST_DIR + "/" + ) + if ( + not under_test_root + or not os.path.basename(path).startswith("test_") + or not path.endswith(".py") + ): + return False + if not path.startswith(MULTIMODAL_TEST_DIR + "/"): + # detect_suite() rejects an unregistered file, and a registered one may + # expose its cases through load_tests() rather than `def test_`. + return True + # Nothing downstream rejects a multimodal path, so a `test_*.py` helper that + # collects nothing reaches `pytest -x` and exits 5. manual/ is hand-run. + return "manual" not in path.split("/") and _collects_pytest_tests(path) + + +def _move_changes_dispatch(previous_filename, filename): + """Whether a content-free move still changes how `filename` dispatches.""" + previous_filename = previous_filename or "" + is_mm = filename.startswith(MULTIMODAL_TEST_DIR + "/") + if is_mm != previous_filename.startswith(MULTIMODAL_TEST_DIR + "/"): + return True + if not _is_rerunnable_test_path(previous_filename): + return True + return is_mm and ( + detect_multimodal_suite(previous_filename)[0] + != detect_multimodal_suite(filename)[0] + ) + + +def changed_test_files(pr): + """Rerunnable test files the PR adds or edits, as repo-relative paths. + + A pure move reports `renamed` with an empty diff and is dropped, unless the + move itself changes dispatch: into a CI root, across the multimodal + boundary, or onto a different multimodal pool. + """ + return sorted( + f.filename + for f in pr.get_files() + if f.status != "removed" + and _is_rerunnable_test_path(f.filename) + and ( + f.changes > 0 + or ( + f.status == "renamed" + and _move_changes_dispatch(f.previous_filename, f.filename) + ) + ) + ) + + def resolve_test_file(file_part): """ Resolve a user-provided file path to a path relative to test/ or full path for multimodal. @@ -1196,6 +1260,7 @@ def handle_rerun_test( skip_permission_check=False, command_label=None, refresh_precision_baseline=False, + include_changed_tests=False, ): """ Handles the /rerun-test command. Resolves all test specs, groups them by @@ -1212,7 +1277,7 @@ def handle_rerun_test( ): return False - if not test_specs: + if not test_specs and not include_changed_tests: comment.create_reaction("confused") pr.create_issue_comment( "⛔ Please specify a test: `/rerun-test ::`\n\n" @@ -1222,7 +1287,9 @@ def handle_rerun_test( "- `/rerun-test test_srt_endpoint.py`\n" "- `/rerun-test test_a.py test_b.py test_c.py` (multiple tests)\n" "- `/rerun-test test_*backend*.py` (wildcard — reruns every matching " - "file; wrap the pattern in backticks so GitHub keeps the `*` literal)" + "file; wrap the pattern in backticks so GitHub keeps the `*` literal)\n" + f"- `/rerun-test {CHANGED_TESTS_FLAG}` (or `{CHANGED_TESTS_SHORT_FLAG}`; " + "every test file this PR adds or modifies)" ) return False @@ -1232,6 +1299,17 @@ def handle_rerun_test( pr.create_issue_comment(gate_msg) return False + if include_changed_tests: + changed = changed_test_files(pr) + if not changed and not test_specs: + comment.create_reaction("confused") + pr.create_issue_comment( + f"⛔ `{CHANGED_TESTS_FLAG}`: this PR adds or modifies no runnable test files " + f"under `test/registered/` or `{MULTIMODAL_TEST_DIR}/`." + ) + return False + test_specs = list(test_specs or []) + changed + # Phase 0: Expand wildcard specs into concrete test files. A spec whose # file part contains a glob metacharacter (* ? [) expands to every # matching file; plain specs pass through to single-file resolution. @@ -1542,9 +1620,10 @@ def main(): elif first_line.startswith("/rerun-test"): rerun_args = first_line.split()[1:] refresh_precision_baseline = PRECISION_BASELINE_REFRESH_FLAG in rerun_args - test_specs = [ - arg for arg in rerun_args if arg != PRECISION_BASELINE_REFRESH_FLAG - ] + changed_flags = {CHANGED_TESTS_FLAG, CHANGED_TESTS_SHORT_FLAG} + include_changed_tests = bool(changed_flags & set(rerun_args)) + flags = changed_flags | {PRECISION_BASELINE_REFRESH_FLAG} + test_specs = [arg for arg in rerun_args if arg not in flags] handle_rerun_test( repo, pr, @@ -1554,6 +1633,7 @@ def main(): token, command_label=first_line, refresh_precision_baseline=refresh_precision_baseline, + include_changed_tests=include_changed_tests, ) else: diff --git a/test/registered/unit/tools/test_slash_command_handler.py b/test/registered/unit/tools/test_slash_command_handler.py index 10dbf320c..f9cdb51a7 100644 --- a/test/registered/unit/tools/test_slash_command_handler.py +++ b/test/registered/unit/tools/test_slash_command_handler.py @@ -1,4 +1,4 @@ -"""Tests for declarative slash-command test groups.""" +"""Tests for slash-command test selection: declarative groups and `--changed`.""" import importlib.util import json @@ -7,7 +7,7 @@ import sys import tempfile import unittest from pathlib import Path -from types import ModuleType +from types import ModuleType, SimpleNamespace from unittest.mock import patch from sglang.test.ci.ci_register import register_cpu_ci @@ -95,5 +95,105 @@ class TestConfiguredTestGroups(CustomTestCase): os.chdir(previous_cwd) +class TestChangedTestFiles(CustomTestCase): + def test_only_dispatchable_changed_test_files_are_selected(self): + """`--changed` feeds the dispatcher directly, so every path it returns must + be runnable: no deleted tests, helpers, source, `manual/` files, or + multimodal `test_*.py` that collect nothing. A pure move is dropped only + when it leaves dispatch alone.""" + handler = _load_handler() + mm = handler.MULTIMODAL_TEST_DIR + pr = SimpleNamespace( + get_files=lambda: [ + SimpleNamespace( + filename=name, + status=status, + changes=changes, + previous_filename=previous, + ) + for name, status, changes, previous in [ + ( + "test/registered/unit/mem_cache/test_radix_cache_unit.py", + "modified", + 4, + None, + ), + ("test/registered/core/test_srt_endpoint.py", "removed", 12, None), + ("test/registered/unit/mem_cache/helpers.py", "modified", 2, None), + ("python/sglang/srt/mem_cache/radix_cache.py", "modified", 7, None), + ("test/manual/test_not_registered.py", "added", 20, None), + ( + "test/registered/spec/test_moved_untouched.py", + "renamed", + 0, + "test/registered/core/test_moved_untouched.py", + ), + ( + "test/registered/spec/test_moved_into_ci.py", + "renamed", + 0, + "test/manual/test_moved_into_ci.py", + ), + ( + f"{mm}/2_gpu/test_moved_pool.py", + "renamed", + 0, + f"{mm}/unit/test_moved_pool.py", + ), + ( + "test/registered/spec/test_moved_and_edited.py", + "renamed", + 9, + "test/registered/core/test_moved_and_edited.py", + ), + (f"{mm}/server/test_server_common.py", "modified", 3, None), + (f"{mm}/server/test_server_utils.py", "modified", 3, None), + (f"{mm}/unit/manual/test_fp4_linear.py", "modified", 3, None), + # Absent from the checkout, as a fork-added file is; kept so + # resolve_test_file() reports `File not found` for it. + ( + f"{mm}/server/test_server_added_by_fork.py", + "added", + 30, + None, + ), + ] + ] + ) + with tempfile.TemporaryDirectory() as tmp: + root = Path(tmp) / mm + (root / "server").mkdir(parents=True) + (root / "unit" / "manual").mkdir(parents=True) + (root / "2_gpu").mkdir(parents=True) + (root / "server" / "test_server_common.py").write_text( + "def test_diffusion_generation():\n pass\n" + ) + (root / "server" / "test_server_utils.py").write_text( + "def build_server():\n return None\n" + ) + (root / "unit" / "manual" / "test_fp4_linear.py").write_text( + "class TestFp4Linear:\n def test_it(self):\n pass\n" + ) + (root / "2_gpu" / "test_moved_pool.py").write_text( + "def test_two_gpu():\n pass\n" + ) + previous_cwd = os.getcwd() + try: + os.chdir(tmp) + self.assertEqual( + handler.changed_test_files(pr), + [ + f"{mm}/2_gpu/test_moved_pool.py", + f"{mm}/server/test_server_added_by_fork.py", + f"{mm}/server/test_server_common.py", + "test/registered/spec/test_moved_and_edited.py", + "test/registered/spec/test_moved_into_ci.py", + "test/registered/unit/mem_cache/test_radix_cache_unit.py", + ], + ) + finally: + os.chdir(previous_cwd) + + if __name__ == "__main__": unittest.main()