Repository navigation
Conversation
The MASTER_PORT precedence test took a port from get_open_port() and then required settle_port to accept that same port. The probe does not reserve the port, so when another process takes it settle_port steps ahead by 37 and the equality assertion fails. That step is the intended fallback. Stub is_port_available for every test that asserts which port was selected, including the default-port path that previously re-bound the resolved port. Fixes vllm-project#9208 Signed-off-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Sheng GAO <sheng.gao.dev@gmail.com> (cherry picked from commit c9f15bc) Signed-off-by: sheng <sheng.gao.dev@gmail.com>
The stubbed default-port check only repeated that settle_port returns its first probe. Take the bounded loopback bind from vllm-project#9215, and require the env-precedence probe list to be exactly the env port. Signed-off-by: sheng <sheng.gao.dev@gmail.com> (cherry picked from commit 90f017e) Signed-off-by: sheng <sheng.gao.dev@gmail.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
|
This PR matches CODEOWNERS paths: /tests/. Code owners: @NickCao @yenuo26 Routing: @NickCao via CODEOWNERS; @yenuo26 via CODEOWNERS @ShengDev, please review your own changes and leave a short self-review comment describing what you checked. PRs without author self-review may not be assigned a reviewer. Please take a look when you have a chance. If you would like an automated review, mention @vllm-omni-review-bot in a comment. |
Omni ReviewBot triage noteAutomated triage of commit
These are automated triage suggestions only — the final decision belongs to the maintainers. |
Summary
release/v0.32.0.OmniDiffusionConfig.master_portprecedence tests so a busy port's intended +37 step no longer fails the equality assertion.Test plan
pytest tests/diffusion/test_omni_diffusion_config_master_port.py -q— 6 passed on vLLM 0.32.0 (CPU), against the updated test file42267resolved to42304(+37) with the real probe; the old equality assertion failsMade with Cursor
Need help on this PR? Tag
@codesmith-botwith what you need.