[CI] 【Hackathon 9th Sprint No.39】NO.39 功能模块单测补充 #5061
Conversation
This test file implements various unit tests for the ResourceManagerV1 class, including functionality for managing requests, handling multimodal inputs, and testing resource allocation and scheduling behaviors.
|
Thanks for your contribution! |
There was a problem hiding this comment.
Pull Request Overview
This PR adds comprehensive unit tests for the ResourceManagerV1 class in the FastDeploy engine scheduler module, achieving 81% test coverage. The test suite includes various scenarios such as request scheduling, resource allocation, multimodal input handling, cache management, and preemption behaviors.
- Adds extensive test coverage for ResourceManagerV1 functionality
- Implements mock objects and stubs to isolate tests from external dependencies
- Tests core scheduling behaviors including prefill, decode, and preemption flows
| self.calls.append(("inc", value)) | ||
|
|
||
|
|
||
| class _FakePrefixCacheManager: |
There was a problem hiding this comment.
The _FakePrefixCacheManager class lacks documentation. Add a docstring to explain its purpose:
class _FakePrefixCacheManager:
"""
A mock implementation of PrefixCacheManager for testing.
Simulates cache block allocation and management without requiring
actual GPU resources, making tests faster and more isolated.
"""| class _FakePrefixCacheManager: | |
| class _FakePrefixCacheManager: | |
| """ | |
| A mock implementation of PrefixCacheManager for testing. | |
| Simulates cache block allocation and management without requiring | |
| actual GPU resources, making tests faster and more isolated. | |
| """ |
| pass | ||
|
|
||
|
|
||
| class _FakeSignal: |
There was a problem hiding this comment.
The _FakeSignal class lacks documentation. Add a docstring to explain its purpose:
class _FakeSignal:
"""
A mock implementation of IPCSignal for testing.
Provides a numpy array-based signal without actual inter-process communication.
"""| class _FakeSignal: | |
| class _FakeSignal: | |
| """ | |
| A mock implementation of IPCSignal for testing. | |
| Provides a numpy array-based signal without actual inter-process communication. | |
| """ |
|
|
||
| num_tokens = manager._get_num_new_tokens(request, token_budget) | ||
|
|
||
| assert num_tokens == 4 # Modal boundary extended the budget |
There was a problem hiding this comment.
[nitpick] Consider adding a comment to explain why the modal boundary extends the budget from 3 to 4 tokens. This would make the test's intent clearer:
assert num_tokens == 4 # Modal boundary at patch_map[2] requires extending budget from 3 to 4| assert num_tokens == 4 # Modal boundary extended the budget | |
| assert num_tokens == 4 # Modal boundary at patch_map[2] requires extending budget from 3 to 4 |
| def test_schedule_allocates_extend_blocks(resource_manager_factory): | ||
| manager = resource_manager_factory(enable_prefix=False, max_num_seqs=1) | ||
| request = _make_request("extend-flow", list(range(8))) | ||
| request.idx = 0 | ||
| request.block_tables = manager.cache_manager.allocate_gpu_blocks(4) | ||
| request.num_computed_tokens = request.need_prefill_tokens | ||
| request.output_token_ids = [10, 11, 12, 13] | ||
| request.use_extend_tables = True | ||
| manager.running.append(request) | ||
| manager.requests[request.request_id] = request | ||
| manager.req_dict[request.request_id] = 0 | ||
| manager.tasks_list[0] = request | ||
| manager.stop_flags[0] = False | ||
| manager.need_block_num_signal.value[0] = 2 |
There was a problem hiding this comment.
This test manually sets up many internal state variables (lines 380-390). Consider extracting this setup into a helper function to avoid duplication and improve maintainability:
def _setup_running_request(manager, request, idx=0, need_block_num=0):
"""Setup a request in running state with proper internal bookkeeping."""
request.idx = idx
manager.running.append(request)
manager.requests[request.request_id] = request
manager.req_dict[request.request_id] = idx
manager.tasks_list[idx] = request
manager.stop_flags[idx] = False
if need_block_num:
manager.need_block_num_signal.value[idx] = need_block_numThis pattern is repeated in multiple tests (e.g., lines 441-448, 491-496).
| import pytest | ||
|
|
||
|
|
||
| def _install_required_stubs(): |
There was a problem hiding this comment.
The _install_required_stubs function lacks documentation. Consider adding a docstring to explain its purpose:
def _install_required_stubs():
"""
Install stub modules for paddle and paddleformers to enable testing without actual dependencies.
This function creates mock modules that simulate the behavior of paddle and paddleformers
libraries, allowing tests to run in isolation without requiring the full framework installation.
"""| def _install_required_stubs(): | |
| def _install_required_stubs(): | |
| """ | |
| Install stub modules for paddle and paddleformers to enable testing without actual dependencies. | |
| This function creates mock modules that simulate the behavior of paddle and paddleformers | |
| libraries, allowing tests to run in isolation without requiring the full framework installation. | |
| """ |
| ) | ||
|
|
||
|
|
||
| class _MetricRecorder: |
There was a problem hiding this comment.
The _MetricRecorder helper class lacks documentation. Add a docstring to explain its purpose:
class _MetricRecorder:
"""
A test helper class that records metric operations for verification.
Tracks both the current value and a history of all set/inc operations
to facilitate assertions in unit tests.
"""| class _MetricRecorder: | |
| class _MetricRecorder: | |
| """ | |
| A test helper class that records metric operations for verification. | |
| Tracks both the current value and a history of all set/inc operations | |
| to facilitate assertions in unit tests. | |
| """ |
| return _factory | ||
|
|
||
|
|
||
| def _make_request(request_id, prompt_token_ids, **kwargs): |
There was a problem hiding this comment.
The _make_request helper function lacks documentation. Add a docstring to explain its purpose and parameters:
def _make_request(request_id, prompt_token_ids, **kwargs):
"""
Create a test Request object with specified parameters.
Args:
request_id: Unique identifier for the request
prompt_token_ids: List of token IDs for the prompt
**kwargs: Optional attributes like disaggregate_info, cached_block_num, etc.
Returns:
Request: A configured Request object for testing
"""| def _make_request(request_id, prompt_token_ids, **kwargs): | |
| def _make_request(request_id, prompt_token_ids, **kwargs): | |
| """ | |
| Create a test Request object with specified parameters. | |
| Args: | |
| request_id: Unique identifier for the request. | |
| prompt_token_ids: List of token IDs for the prompt. | |
| **kwargs: Optional attributes to set on the Request object, such as: | |
| - disaggregate_info (dict): Additional info for disaggregation. | |
| - cached_block_num (int): Number of cached blocks. | |
| - multimodal_inputs (dict): Multimodal input data. | |
| - output_token_ids (list): Output token IDs. | |
| - reasoning_max_tokens (int): Max tokens for reasoning. | |
| - use_extend_tables (bool): Whether to use extended tables. | |
| - extend_block_tables (list): Extended block tables. | |
| Returns: | |
| Request: A configured Request object for testing. | |
| """ |
| manager.add_request(req1) | ||
| manager.add_request(req2) | ||
|
|
||
| monkeypatch.setattr(rm_v1.paddle, "is_compiled_with_xpu", lambda: True, raising=False) |
There was a problem hiding this comment.
The raising=False parameter is incorrect for monkeypatch.setattr. This parameter doesn't exist in pytest's monkeypatch API. The correct usage is either:
- Just omit this parameter:
monkeypatch.setattr(rm_v1.paddle, "is_compiled_with_xpu", lambda: True) - Or use
raising=True(the default) to raise an error if the attribute doesn't exist
However, since paddle is already stubbed in line 37, and the test is trying to override it, this should work without raising=False.
| monkeypatch.setattr(rm_v1.paddle, "is_compiled_with_xpu", lambda: True, raising=False) | |
| monkeypatch.setattr(rm_v1.paddle, "is_compiled_with_xpu", lambda: True) |
|
|
||
| _install_required_stubs() | ||
|
|
||
| import fastdeploy.engine.sched.resource_manager_v1 as rm_v1 |
There was a problem hiding this comment.
Module 'fastdeploy.engine.sched.resource_manager_v1' is imported with both 'import' and 'import from'.
| import fastdeploy.engine.sched.resource_manager_v1 as rm_v1 |
1 participant
This test file implements various unit tests for the ResourceManagerV1 class, including functionality for managing requests, handling multimodal inputs, and testing resource allocation and scheduling behaviors.
Motivation
NO.39 功能模块 fastdeploy/engine/sched/resource_manager_v1.py 单测补充
Modifications
add tests/engine/test_resource_manager_v1.py
Usage or Command
tests/engine/test_resource_manager_v1.py:Accuracy Tests
tests/engine/test_resource_manager_v1.py:Checklist
[FDConfig],[APIServer],[Engine],[Scheduler],[PD Disaggregation],[Executor],[Graph Optimization],[Speculative Decoding],[RL],[Models],[Quantization],[Loader],[OP],[KVCache],[DataProcessor],[BugFix],[Docs],[CI],[Optimization],[Feature],[Benchmark],[Others],[XPU],[HPU],[GCU],[DCU],[Iluvatar],[Metax]]pre-commitbefore commit.releasebranch, make sure the PR has been submitted to thedevelopbranch, then cherry-pick it to thereleasebranch with the[Cherry-Pick]PR tag.