Skip to content

Test(rollout): add unit tests for vllm_rollout helpers and mocked generate paths - #25

Merged
aoshen02 merged 1 commit into
mainfrom
refactor/tests
May 23, 2026
Merged

Test(rollout): add unit tests for vllm_rollout helpers and mocked generate paths#25
aoshen02 merged 1 commit into
mainfrom
refactor/tests

Conversation

@CalvinXKY

Copy link
Copy Markdown
Collaborator

Summary

Adds unit tests for slime.rollout.vllm_rollout, covering token preprocessing, vLLM /inference/v1/generate request/response handling, and core async paths (generate, generate_and_rm, abort) via mocks.

  • New: tests/unit/rollout/test_vllm_rollout.py — 55 test cases marked with @pytest.mark.unit
  • New: tests/unit/rollout/conftest.py — stubs for optional deps (vllm_router, PIL, transformers, aiohttp, pylatexenc) so tests collect on minimal dev machines
  • Unchanged: tests/test_vllm_generate_endpoint.py — still used for real vLLM + GPU integration checks

Coverage

Pure helpers

  • _coerce_flat_int_token_ids, _prepare_prompt_ids, _base_dataset_prompt_ids
  • get_model_url
  • _vllm_meta_from_generate_choice, _decode_vllm_routed_experts, _apply_vllm_routed_experts (including disabled / invalid-shape branches)
  • _inference_generate_tokens_and_logprobs, _align_engine_tokens_and_logprobs
  • _build_inference_sampling_params, _mm_render_response_to_generate_body
  • _router_worker_urls, _resume_vllm_workers

Mocked async paths

  • generate(): text rollout, partial-continuation budget, zero-budget truncation, consistent-hashing headers, multimodal render→generate, routed-experts assignment
  • generate_and_rm(): skip completed samples, abort handling, custom generate hook
  • generate_and_rm_group(): per-sample session_id assignment
  • abort(): worker pause + resume

Out of scope (existing integration tests)

  • generate_rollout_async / generate_rollout / eval_rollout*
  • Live vLLM server + GPU E2E (tests/test_vllm_generate_endpoint.py)

Test plan

cd vime
pytest tests/unit/rollout/test_vllm_rollout.py -m unit -q

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request transitions the base Docker image to vllm/vllm-openai, updates the build process with necessary CUDA development headers and dependencies, and introduces a build-time smoke test. It also adds a comprehensive suite of unit tests and shared stubs for the vllm_rollout module. Feedback was provided regarding the inclusion of IPython in the production image, which may be unnecessary and increases the image size.

Comment thread docker/Dockerfile
Comment on lines +98 to +99
RUN pip install --no-deps "sglang==${SGLANG_VERSION}" sglang-router==0.3.2 && \
pip install IPython

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The installation of IPython in the production Docker image seems unnecessary and increases the image size. Unless it is explicitly required for a runtime feature (e.g., interactive debugging within the container, which is usually discouraged for production), it should be removed to keep the image lean.

RUN pip install --no-deps "sglang==${SGLANG_VERSION}" sglang-router==0.3.2

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No changes were made to the Dockerfile.

@CalvinXKY CalvinXKY changed the title test(rollout): add unit tests for vllm_rollout helpers and mocked generate paths Test(rollout): add unit tests for vllm_rollout helpers and mocked generate paths May 23, 2026
@CalvinXKY
CalvinXKY requested a review from aoshen02 May 23, 2026 08:15
@aoshen02
aoshen02 merged commit 2c1b340 into main May 23, 2026
9 of 16 checks passed
momo609 pushed a commit that referenced this pull request May 26, 2026
@aoshen02
aoshen02 deleted the refactor/tests branch June 8, 2026 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants