[CI] 【Hackathon 9th Sprint No.38】NO.38 功能模块单测补充 #5060
Conversation
This file contains tests for the global scheduler, including request handling, load accounting, and response management.
|
Thanks for your contribution! |
There was a problem hiding this comment.
Pull Request Overview
This PR adds comprehensive unit tests for the global scheduler module (fastdeploy/scheduler/global_scheduler.py) as part of Hackathon 9th Sprint No.38. The tests use pytest with custom fixtures and mocks to achieve 83% code coverage.
Key Changes
- Implements fake Redis and worker implementations for isolated testing
- Tests request handling, load balancing, request stealing, and response routing
- Adds tests for scheduler configuration and reset functionality
- Includes helper function tests for block calculation and request/response marking
|
|
||
| def test_get_requests_requeues_when_chunked_limits_hit(scheduler_fixture): | ||
| scheduler, fake_redis = scheduler_fixture | ||
| envs.FD_ENABLE_MAX_PREFILL = 0 |
There was a problem hiding this comment.
Directly modifying envs.FD_ENABLE_MAX_PREFILL without restoring it can lead to test isolation issues. Consider using monkeypatch.setattr(envs, "FD_ENABLE_MAX_PREFILL", 0) to ensure the value is automatically restored after the test completes.
|
|
||
| def test_get_requests_blocking_pop_returns_when_idle(scheduler_fixture): | ||
| scheduler, fake_redis = scheduler_fixture | ||
| envs.FD_ENABLE_MAX_PREFILL = 0 |
There was a problem hiding this comment.
Directly modifying envs.FD_ENABLE_MAX_PREFILL without restoring it can lead to test isolation issues. Consider using monkeypatch.setattr(envs, "FD_ENABLE_MAX_PREFILL", 0) to ensure the value is automatically restored after the test completes.
| bucket = self.lists.get(key) | ||
| if not bucket: | ||
| return None | ||
| if count is None or count <= 1: |
There was a problem hiding this comment.
Inconsistent return behavior in lpop: when count is None or count <= 1, it returns a list with one element [bucket.pop(0)], but the check if count is None or count <= 1 treats count=0 the same as count=1. When count=0, the method should return an empty list [] or None, not a list with one element. This could cause the fake Redis to behave differently than the real Redis implementation.
| if count is None or count <= 1: | |
| if count == 0: | |
| return [] | |
| if count is None or count == 1: |
| removed += int(key in self.kv or key in self.lists) | ||
| self.kv.pop(key, None) | ||
| self.lists.pop(key, None) |
There was a problem hiding this comment.
The delete method doesn't handle keys in sorted_sets. If a key exists in sorted_sets but not in kv or lists, it won't be counted as removed or actually deleted. This could cause the fake Redis to behave incorrectly when testing deletion of sorted set keys.
| removed += int(key in self.kv or key in self.lists) | |
| self.kv.pop(key, None) | |
| self.lists.pop(key, None) | |
| removed += int(key in self.kv or key in self.lists or key in self.sorted_sets) | |
| self.kv.pop(key, None) | |
| self.lists.pop(key, None) | |
| self.sorted_sets.pop(key, None) |
| return removed | ||
|
|
||
| def exists(self, key: str) -> int: | ||
| if key in self.kv or key in self.lists: |
There was a problem hiding this comment.
The exists method doesn't check sorted_sets, only kv and lists. If a key exists in sorted_sets, this method will incorrectly return 0, indicating the key doesn't exist.
| if key in self.kv or key in self.lists: | |
| if key in self.kv or key in self.lists or key in self.sorted_sets: |
| python -m coverage run -m pytest tests/scheduler/test_global_scheduler.py \ | ||
| && python -m coverage report -m --include='fastdeploy/scheduler/global_scheduler.py' |
There was a problem hiding this comment.
The command in the docstring uses pytest (python -m coverage run -m pytest), but the PR description specifies unittest (python -m unittest tests.scheduler.test_global_scheduler). These should be consistent. Since the tests use pytest fixtures, the docstring appears correct, but this inconsistency with the PR description may confuse users.
| @@ -0,0 +1,431 @@ | |||
| """Tests for the global scheduler. | |||
There was a problem hiding this comment.
Missing the Apache 2.0 license header that is present in all other test files in this repository. The copyright header should be added at the top of the file following the project's convention:
"""
# Copyright (c) 2025 PaddlePaddle Authors. All Rights Reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License"
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
"""|
|
||
| def test_get_requests_can_steal_remote_request(monkeypatch, scheduler_fixture): | ||
| scheduler, fake_redis = scheduler_fixture | ||
| envs.FD_ENABLE_MAX_PREFILL = 0 |
There was a problem hiding this comment.
Directly modifying envs.FD_ENABLE_MAX_PREFILL without restoring it can lead to test isolation issues. Consider using monkeypatch.setattr(envs, "FD_ENABLE_MAX_PREFILL", 0) to ensure the value is automatically restored after the test completes.
| envs.FD_ENABLE_MAX_PREFILL = 0 | |
| monkeypatch.setattr(envs, "FD_ENABLE_MAX_PREFILL", 0) |
3 participants
This file contains tests for the global scheduler, including request handling, load accounting, and response management.
Motivation
NO.38 功能模块 fastdeploy/scheduler/global_scheduler.py 单测补充
Modifications
new dir and add tests/scheduler/test_global_scheduler.py
Usage or Command
tests/scheduler/test_global_scheduler.py:Accuracy Tests
tests/scheduler/test_global_scheduler.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.