diff --git a/.claude/skills/ci-workflow-guide/SKILL.md b/.claude/skills/ci-workflow-guide/SKILL.md index 00a7991c9..e5eb3d6ef 100644 --- a/.claude/skills/ci-workflow-guide/SKILL.md +++ b/.claude/skills/ci-workflow-guide/SKILL.md @@ -387,7 +387,7 @@ group: pr-test-{event_name}-{branch}-{pr_sha}-{stage} | `/tag-and-rerun-ci` | Adds `run-ci` label + reruns failed | | `/tag-and-rerun-ci extra` | Adds both `run-ci` and `run-ci-extra` labels + reruns failed | | `/rerun-stage ` | Deprecated; posts deprecation notice | -| `/rerun-test ` | Reruns a specific test file via `rerun-test.yml` | +| `/rerun-test [ ...]` | Reruns specific test file(s) via `rerun-test.yml`. A file arg containing a glob metacharacter (`*`, `?`, `[...]`) expands against `test/registered/` and the multimodal test dir to every matching `test_*.py` (e.g. `/rerun-test test_*backend*.py` — wrap in backticks so GitHub doesn't italicize the `*`); matches are deduped, grouped by dispatch shape, and can't carry a `::test` selector. No match → single ⛔ reply, nothing dispatched. Each reply echoes its originating command (`Results for …`) so concurrent commands stay distinguishable | | `/rerun-group [ ...]` | Expands registered test groups, then reuses `/rerun-test` | Handled by `scripts/ci/utils/slash_command_handler.py` → `.github/workflows/slash-command-handler.yml`. diff --git a/scripts/ci/utils/slash_command_handler.py b/scripts/ci/utils/slash_command_handler.py index 44c0dacfe..d64a6cd06 100644 --- a/scripts/ci/utils/slash_command_handler.py +++ b/scripts/ci/utils/slash_command_handler.py @@ -486,6 +486,72 @@ def resolve_test_group_specs(group_name): return [os.path.relpath(path, "test") for path in test_files], None +# A spec is treated as a wildcard pattern (expanding to many files) when its +# file part contains a glob metacharacter. Plain specs keep the existing +# single-file resolution, which requires a unique match. +_GLOB_METACHARS = ("*", "?", "[") + + +def _is_glob_pattern(file_part): + return any(ch in file_part for ch in _GLOB_METACHARS) + + +def expand_glob_spec(file_part): + """ + Expand a wildcard file_part into matching test files (repo-relative paths). + + 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`. + + glob's `*` matches path separators only via `**`, so a bare pattern is + searched recursively under each root; a path-ful pattern is anchored. + + Returns (sorted_repo_relative_paths, error). On success error is None. + """ + pat = file_part + if pat.startswith("test/"): + pat = pat[len("test/") :] + + matches = set() + if "/" in pat: + # Path-ful pattern. Glob from the repo root (handles fully qualified + # multimodal paths like python/sglang/multimodal_gen/test/**/test_*.py) + # and under test/ (handles test/-relative patterns like + # registered/attention/test_*.py). + for base in (".", "test"): + matches.update(glob.glob(os.path.join(base, pat), recursive=True)) + else: + # Bare pattern: search recursively under each known test root. + for root in ("test/registered", MULTIMODAL_TEST_DIR): + matches.update(glob.glob(os.path.join(root, "**", pat), recursive=True)) + + 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 not files: + return [], ( + f"No test files matched wildcard `{file_part}` under " + f"`test/registered/` or `{MULTIMODAL_TEST_DIR}/` " + f"(patterns only match files named `test_*.py`)." + ) + return files, None + + def resolve_test_file(file_part): """ Resolve a user-provided file path to a path relative to test/ or full path for multimodal. @@ -887,7 +953,14 @@ def _check_rerun_test_permissions(gh_repo, pr, comment, user_perms, command_name def handle_rerun_test( - gh_repo, pr, comment, user_perms, test_specs, token, skip_permission_check=False + gh_repo, + pr, + comment, + user_perms, + test_specs, + token, + skip_permission_check=False, + command_label=None, ): """ Handles the /rerun-test command. Resolves all test specs, groups them by @@ -907,7 +980,9 @@ def handle_rerun_test( "- `/rerun-test test/registered/core/test_srt_endpoint.py::TestSRTEndpoint.test_simple_decode`\n" "- `/rerun-test registered/core/test_srt_endpoint.py::TestSRTEndpoint`\n" "- `/rerun-test test_srt_endpoint.py`\n" - "- `/rerun-test test_a.py test_b.py test_c.py` (multiple tests)" + "- `/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)" ) return False @@ -917,15 +992,63 @@ def handle_rerun_test( pr.create_issue_comment(gate_msg) return False - # Phase 1: Resolve all specs - resolved = [] + # 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. resolve_failures = [] + seen_failures = set() + + def _record_failure(spec, error): + # De-dupe failures by canonical path so the same un-dispatchable file + # reported two ways (explicit + glob, or two globs) yields one line. + key = spec.strip().strip("\"'`") + if key.startswith("test/"): + key = key[len("test/") :] + if key not in seen_failures: + seen_failures.add(key) + resolve_failures.append({"spec": spec, "error": error}) + + expanded_specs = [] for spec in test_specs: + # Quotes/backticks are never meaningful here (the command isn't + # shell-parsed), so strip them. Backtick-wrapping is in fact the + # recommended way to write a glob: plain `*backend*` renders as italics + # in a GitHub comment, but `` `test_*backend*.py` `` stays literal — and + # either way the handler reads the raw body, so the `*` survives. + file_part = spec.split("::", 1)[0].strip().strip("\"'`") + if not _is_glob_pattern(file_part): + expanded_specs.append(spec) + continue + if "::" in spec: + _record_failure( + spec, + "Wildcard patterns can't be combined with a `::test` " + "selector — drop the `::...` to rerun whole files.", + ) + continue + matched, err = expand_glob_spec(file_part) + if err: + _record_failure(spec, err) + continue + expanded_specs.extend(matched) + + # Phase 1: Resolve all specs, de-duping by the *resolved* identity. A glob + # expands to `test/`-prefixed paths while an explicit spec keeps the form + # the user typed, so the same file requested both ways resolves to two + # different raw strings — keying de-dup on the resolved (mode, test_command) + # is what collapses them into a single dispatch instead of running twice. + resolved = [] + seen_commands = set() + for spec in expanded_specs: r = _resolve_test_spec(spec) if r.get("error"): - resolve_failures.append(r) - else: - resolved.append(r) + _record_failure(r["spec"], r["error"]) + continue + key = (r["mode"], r["test_command"]) + if key in seen_commands: + continue + seen_commands.add(key) + resolved.append(r) # Phase 2: Group by dispatch shape. groups = {} @@ -942,7 +1065,8 @@ def handle_rerun_test( # Phase 3a: Create placeholder reply comment so we have its ID before # dispatching workflows. This lets each dispatched run write its # success/failure result back to the right line in this comment. - reply_comment = pr.create_issue_comment("🚀 Dispatching rerun-test workflow(s)...") + dispatching = f"`{command_label}`" if command_label else "rerun-test workflow(s)" + reply_comment = pr.create_issue_comment(f"🚀 Dispatching {dispatching}...") # Phase 3b: Dispatch one workflow per group, with a unique per-batch # marker each. The marker is an HTML comment that the writeback step @@ -997,6 +1121,11 @@ def handle_rerun_test( lines.append(f"⛔ `{r['spec']}`: {r['error']}") body = "\n\n".join(lines) + # Echo the originating command so each reply is self-identifying when + # several /rerun-test commands are in flight at once. Backtick-wrapping + # also keeps any `*` in the pattern from rendering as italics. + if command_label: + body = f"Results for `{command_label}`:\n\n{body}" successes = [dr for dr in dispatch_results if dr["success"]] if successes: @@ -1008,7 +1137,9 @@ def handle_rerun_test( return len(successes) > 0 -def handle_rerun_group(gh_repo, pr, comment, user_perms, group_names, token): +def handle_rerun_group( + gh_repo, pr, comment, user_perms, group_names, token, command_label=None +): """ Handles the /rerun-group command. Expands one or more registered test groups into test file specs, then reuses /rerun-test dispatch behavior. @@ -1055,6 +1186,7 @@ def handle_rerun_group(gh_repo, pr, comment, user_perms, group_names, token): test_specs, token, skip_permission_check=True, + command_label=command_label, ) @@ -1165,11 +1297,27 @@ def main(): elif first_line.startswith("/rerun-group"): group_names = first_line.split()[1:] - handle_rerun_group(repo, pr, comment, user_perms, group_names or None, token) + handle_rerun_group( + repo, + pr, + comment, + user_perms, + group_names or None, + token, + command_label=first_line, + ) elif first_line.startswith("/rerun-test"): test_specs = first_line.split()[1:] - handle_rerun_test(repo, pr, comment, user_perms, test_specs or None, token) + handle_rerun_test( + repo, + pr, + comment, + user_perms, + test_specs or None, + token, + command_label=first_line, + ) else: print(f"Unknown or ignored command: {first_line}")