Skip to content

[Test] Add test for /health endpoint on engine failure - #26074

Merged
DarkLight1337 merged 2 commits into
vllm-project:mainfrom
dongbo910220:test/health-check-503
Oct 18, 2025
Merged

DarkLight1337 merged 2 commits into
vllm-project:mainfrom
dongbo910220:test/health-check-503

Conversation

@dongbo910220

@dongbo910220 dongbo910220 commented Oct 2, 2025 •

Copy link
Copy Markdown
Contributor

Purpose

This PR adds an automated test for the behavior fixed in #24897, as requested by @cadedaniel in the original PR's comments.

The new test verifies that the /health endpoint correctly returns 503 Service Unavailable when EngineDeadError is raised, which prevents future regressions of this behavior.

Relates to: #24897

Test Plan

Run the new test case:

pytest tests/entrypoints/openai/test_basic.py::test_health_check_engine_dead_error -v

Test Result

Test passes successfully:

tests/entrypoints/openai/test_basic.py::test_health_check_engine_dead_error PASSED [100%]
======================== 1 passed in 3.55s =========================

The test uses mocks to simulate EngineDeadError being raised during health check, and verifies that the
endpoint correctly returns HTTP 503 status code instead of 500.

Test characteristics:
- Runs quickly (~3-6 seconds) using mocks
- No GPU or model loading required
- Independent of environment-specific configurations
- Focuses specifically on the EngineDeadError → 503 logic

@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 introduces a valuable unit test for the /health endpoint. The test correctly verifies that a 503 Service Unavailable status is returned when an EngineDeadError is raised, which is crucial for ensuring the service's health check behaves as expected under failure conditions. The implementation is clean, using mocks effectively to isolate the component under test. This change improves the overall test coverage and helps prevent future regressions. The code is well-structured and I have no concerns.

Signed-off-by: dongbo910220 <1275604947@qq.com>
@dongbo910220
dongbo910220 force-pushed the test/health-check-503 branch from 1a63686 to f681112 Compare October 2, 2025 04:13
@dongbo910220

Copy link
Copy Markdown
Contributor Author

Hi @DarkLight1337,

Just a friendly follow-up on this PR. Could you please take a look when you have a chance? Any feedback would be greatly appreciated.

Thank you!

@DarkLight1337

Copy link
Copy Markdown
Member

Sorry for the delay, thought @cadedaniel would review it. I'll just stamp this

@DarkLight1337
DarkLight1337 enabled auto-merge (squash) October 18, 2025 09:17
@github-actions github-actions Bot added the ready ONLY add when PR is ready to merge/full CI is needed label Oct 18, 2025
@dongbo910220

dongbo910220 commented Oct 18, 2025 •

Copy link
Copy Markdown
Contributor Author

@DarkLight1337 Great, thank you so much!

@DarkLight1337
DarkLight1337 merged commit 8300402 into vllm-project:main Oct 18, 2025
19 checks passed
ptrdlr14 pushed a commit to ptrdlr14/vllm that referenced this pull request Oct 20, 2025
adabeyta pushed a commit to adabeyta/vllm that referenced this pull request Oct 20, 2025
0xrushi pushed a commit to 0xrushi/vllm that referenced this pull request Oct 26, 2025
…#26074)

Signed-off-by: dongbo910220 <1275604947@qq.com>
Signed-off-by: 0xrushi <6279035+0xrushi@users.noreply.github.com>
0xrushi pushed a commit to 0xrushi/vllm that referenced this pull request Oct 26, 2025
…#26074)

Signed-off-by: dongbo910220 <1275604947@qq.com>
Signed-off-by: 0xrushi <6279035+0xrushi@users.noreply.github.com>
ilmarkov pushed a commit to neuralmagic/vllm that referenced this pull request Nov 7, 2025
rtourgeman pushed a commit to rtourgeman/vllm that referenced this pull request Nov 10, 2025
devpatelio pushed a commit to SumanthRH/vllm that referenced this pull request Nov 29, 2025
mystous pushed a commit to mystous/vllm_hybrid that referenced this pull request May 10, 2026
my-other-github-account pushed a commit to my-other-github-account/vllm that referenced this pull request May 15, 2026
0826joyce pushed a commit to 0826joyce/vllm-serving-optimization that referenced this pull request May 19, 2026
plasticchris pushed a commit to plasticchris/vllm that referenced this pull request Jul 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready ONLY add when PR is ready to merge/full CI is needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants