ci: add always-on e2e-test-unit job (pytest tests/unit tests/utils) - #110
Merged
Conversation
The pytest-based unit/utils suites were never wired into CI — they only ran locally. tests/unit is vime-original coverage of the vLLM rollout/ engine/arguments + colocate IPC weight-sync surface (the code most exposed to the slime→vime upstream sync); tests/utils is mostly inherited from upstream slime. Both are pure CPU/mock (no cuda, no GPU lock). Add a single dedicated job rather than folding into an existing one: - existing jobs run one *script* per matrix entry, never `pytest`; - the tests/unit/backends/megatron_utils/update_weight tests import `megatron.core` at module load, so they can't run on the GitHub-hosted CPU plugin-contracts runner — they need the CI image. So e2e-test-unit runs `pytest tests/unit tests/utils` inside inferactinc/public:vime-vllm-cu129-latest (ships megatron+torch) on the self-hosted runner, 0 GPUs, always-on (no run-ci-* label) like e2e-test-plugin-contracts. Edit the .j2 template + regenerate; do not hand-edit pr-test.yml. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
Note Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported. |
Collaborator
|
LGTM |
momo609
pushed a commit
that referenced
this pull request
Jun 8, 2026
…110) The pytest-based unit/utils suites were never wired into CI — they only ran locally. tests/unit is vime-original coverage of the vLLM rollout/ engine/arguments + colocate IPC weight-sync surface (the code most exposed to the slime→vime upstream sync); tests/utils is mostly inherited from upstream slime. Both are pure CPU/mock (no cuda, no GPU lock). Add a single dedicated job rather than folding into an existing one: - existing jobs run one *script* per matrix entry, never `pytest`; - the tests/unit/backends/megatron_utils/update_weight tests import `megatron.core` at module load, so they can't run on the GitHub-hosted CPU plugin-contracts runner — they need the CI image. So e2e-test-unit runs `pytest tests/unit tests/utils` inside inferactinc/public:vime-vllm-cu129-latest (ships megatron+torch) on the self-hosted runner, 0 GPUs, always-on (no run-ci-* label) like e2e-test-plugin-contracts. Edit the .j2 template + regenerate; do not hand-edit pr-test.yml. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
CalvinXKY
pushed a commit
that referenced
this pull request
Jun 11, 2026
* sync: complete slime #1920, #1967, #1985 — lint, fully_async, PYTHONUNBUFFERED Traced 3 individual diffs back to their source slime PRs (以点带面) and synced all remaining changes from each: slime #1985 (make tests shorter): - Wrap NamedTemporaryFile across 3 lines in test_vllm_config_mixed_offload_ft.py - Remove extra blank line in test_vllm_config_mixed_offload.py (parameter shortening already synced in vime PR #218) slime #1920 (move fully_async example to main codebase): - Rewrite README.md to match upstream (qwen2.5-0.5B, not qwen3-4b) - Add run-qwen2.5-0.5B-fully_async.sh with proper vLLM translations - Delete run-qwen3-4b-fully_async.sh (upstream removed it) slime #1967 (fix PYTHONBUFFERED typo): - Fix PYTHONBUFFERED=16 → PYTHONUNBUFFERED=1 in 3 scripts: run-glm4.7-30B-A3B.sh, run-glm4.7-355B-A32B.sh, run-minimax-m2.sh (command_utils.py already fixed in prior sync) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen@inferact.ai> * fix: correct §2.4 flag translations in analyze_profile.py --enforce-eager → --vllm-enforce-eager --gpu-memory-utilization → --vllm-gpu-memory-utilization These are diagnostic hint strings, not CLI invocations, but should still use the canonical vime flag names (§2.4 translation rules). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen@inferact.ai> * fix: mirror slime underline + revert group_id comment to rollout_id - analyze_profile.py: match slime's 30-char underline (was 28) - run_qwen36_35b_a3b_swe_8nodes.sh: revert group_id→rollout_id in comment (partial sync of slime #2013, target commit 44d29ee5) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen@inferact.ai> * refactor(arguments): use FlexibleArgumentParser; revert dist.gather_object patch arguments.py (361→346 lines): - Import FlexibleArgumentParser from vllm.utils.argparse_utils; use it in vllm_parse_args() and get_vllm_cli_action_table() so vLLM's deprecated kwarg is handled natively on Python 3.12 without a shim - Remove _ARGPARSE_UNSUPPORTED_KWARGS + _strip_unsupported_argparse_kwargs - Remove import logging / logger (unused) reloadable_process_group.py: - Drop dist.gather_object monkey-patch (added by PR #22, never in slime; callers explicitly use all_gather_object to stay on the patched path) Architecture note: subprocess vllm serve kept intentionally — run_server() is not in vllm.__all__ and changes across minor versions; AReaL uses the same Popen pattern for the same reason. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen@inferact.ai> * ci: revert aoshen02 CI additions; align J2 template with slime Reverts PR #26 (pre-commit gate) + PR #110 (e2e-test-unit). Syncs from slime: opened/reopened trigger types, --pull=always, pip deps (requests ray safetensors), cpu-unittest rename. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen@inferact.ai> * docs(docker): translate Chinese to English in README Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen@inferact.ai> * fix(ci): stub vllm_router in plugin_contracts for cpu-unittest Mirrors slime's with_sglang_router stub pattern: add with_vllm_router kwarg to install_stubs() and pass it from test_plugin_generate_contracts. vllm_rollout.py imports vllm_router at module level; without the stub the cpu-unittest job fails with ModuleNotFoundError. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen@inferact.ai> * fix(ci): add missing with_vllm_router=True stub to remaining plugin contracts test_plugin_rollout_contracts and test_plugin_path_loading_contracts both import vllm_rollout (which has bare `import vllm_router` at module level) but were not passing with_vllm_router=True to install_stubs — mirroring the same gap fixed in test_plugin_generate_contracts. Mirrors slime: all three tests pass with_sglang_router=True. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen@inferact.ai> * docs(coding_agent_rl): align generate.py and README.md wording with slime - generate.py: drop backtick-wrapping around /inference/v1/generate in module docstring; collapse to one line matching slime style - README.md: condense rollout-max-*-len paragraph (remove verbose "sampling-params" / "generation length" verbiage, restore `max_tokens` inline like slime's `max_new_tokens` form) - README.md: trim vLLM response-structure detail (choices[0].token_ids / choices[0].logprobs.content[i].logprob) from token-out bullet — matches slime's abstraction level - README.md: add missing 3-line unit-test sentence after provenance paragraph (slime has it, vime was missing it) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen@inferact.ai> * refactor(arguments): remove dist_ckpt_optim_fully_reshardable warning block Not present in slime; drop to maintain mirror parity. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen@inferact.ai> * refactor(rollout): remove redundant assertions and finalization re-assignments on_policy_distillation.py: drop two assertions not present in slime (len(plp)==len(sample.tokens) and len(t_log_prob)>=response_length). vllm_streaming_rollout.py: drop 9-line finalization block that re-set sample.tokens/response/response_length/rollout_log_probs/loss_mask after the streaming loop had already written the same values. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen@inferact.ai> * style: apply black formatting to fix pre-commit CI Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: aoshen02 <aoshen@inferact.ai> --------- Signed-off-by: aoshen02 <aoshen@inferact.ai> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Wires the pytest-based
tests/unit+tests/utilssuites into CI as a single dedicated, always-on jobe2e-test-unit. Today these only run locally — nothing in any workflow executes them.Why a separate job (vs. folding into an existing one)
python tests/<file>/gpu_lock_exec.py), neverpyteston a directory.tests/unit/backends/megatron_utils/update_weight/*importmegatron.coreat module load, so they cannot run on the GitHub-hosted CPUe2e-test-plugin-contractsrunner (no megatron there). They need the CI image.So
e2e-test-unitrunspytest tests/unit tests/utilsinsideinferactinc/public:vime-vllm-cu129-latest(ships megatron + torch) on the self-hosted runner, 0 GPUs, always-on (norun-ci-*label, mirroringe2e-test-plugin-contracts). It doesneeds: pre-commitlike the others.Coverage / risk
tests/unitis vime-original (PRs Sync to vLLM 0.21.0 #21/[feat] Support Colocated Weight Sync via CUDA IPC for vime #22/Test(rollout): add unit tests for vllm_rollout helpers and mocked generate paths #25) — covers the vLLM rollout/engine/arguments + colocate IPC weight-sync surface, i.e. the code most exposed to the ongoing slime→vime upstream sync.tests/utilsis mostly inherited from upstream slime (#1204/#1742/#1776/#1866/#1874); onlytest_vllm_config.pyis vime-adapted.cuda/.cuda()/device="cuda", no realvllm/megatronimports at test-file scope (megatron is transitive via the module-under-test). Pure CPU/mock → no--gpus, no GPU lock, no GPU contention with the other self-hosted jobs.Verification status (please confirm on the runner)
generate_github_workflows.py);pr-test.ymlis generated, not hand-edited.check yamlpre-commit hook passes.update_weighttests failing withModuleNotFoundError: megatron— exactly what running inside the image fixes.workflow_dispatchor a PR synchronize and confirme2e-test-unitgoes green; if the image lackspytest, the job installs it.How to run
Always-on, so it fires on every PR
synchronize/labeledand on manual dispatch:🤖 Generated with Claude Code