test: health check test

[biswapanda]

Overview:

Details:

Where should the reviewer start?

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

• closes GitHub issue: #xxx

Summary by CodeRabbit

Release Notes

• Tests
  • Added backend health check validation to test configurations
  • Health checks verify HTTP status, service readiness status, and endpoint availability before test execution
  • Integrated into VLLM test suite to ensure backend reliability prior to running test payloads

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

A new health-check payload helper function is added to validate backend status and endpoints, then integrated into three VLLM test configurations to execute health checks before other payloads.

Changes

Cohort / File(s)	Summary
Health Check Payload Builder tests/utils/payload_builder.py	Adds new public function backend_health_check_payload() that creates a health-check payload with response validation for HTTP status, JSON "status" field, and "endpoints" mapping. Returns configured payload instance with repeat count of 1.
VLLM Test Integration tests/serve/test_vllm.py	Imports new health-check payload helper and inserts it as the first element in request payloads for three test configurations: aggregated, agg-request-plane-tcp, and agg-request-plane-http.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• New validation logic in backend_health_check_payload() response handler requires verification of HTTP status, JSON field assertions, and summary formatting
• Test integration changes are repetitive across three configurations and straightforward payload insertion
• Limited file scope and homogeneous application pattern reduce overall review complexity

Poem

🐰 A health check hops in first, quick and spry,
Before tests leap—let backends verify.
Status? Endpoints? All looking just right,
With validation that runs like a rabbit in flight! 🏃‍♂️✨

Pre-merge checks

❌ Failed checks (2 warnings, 1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The pull request description contains only template placeholders with no substantive content; all required sections (Overview, Details, Where should reviewer start, Related Issues) are empty or incomplete.	Fill in all required sections with actual information: describe the purpose, explain what files were changed and why, indicate which files to review first, and link any related GitHub issues using action keywords.
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.
Title check	❓ Inconclusive	The title 'test: health check test' is vague and generic. While it relates to the changeset (adding health check payload functionality), it lacks specificity about the primary change.	Use a more descriptive title that clarifies the specific change, such as 'test: Add backend health check payload helper function' or 'test: Add health checks to VLLM test suite'.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

tests/utils/payload_builder.py (2)

286-286: Remove redundant import.

BasePayload is already imported at the top of the file (line 7), so this local import is unnecessary.

Apply this diff:

-    from tests.utils.payloads import BasePayload
-
     class BackendHealthPayload(BasePayload):

---

313-319: Consider adding a shorter timeout for health checks.

Health checks typically benefit from shorter timeouts (e.g., 10-15 seconds) rather than the default 60 seconds from BasePayload. This would allow faster failure detection if the backend is unresponsive.

You could add a timeout parameter to the function and pass it to the payload:

def backend_health_check_payload(
    port: int = 8080,
    expected_log: Optional[List[str]] = None,
    timeout: int = 15,
):
    """Create a backend health check payload that validates system status health endpoint."""
    class BackendHealthPayload(BasePayload):
        endpoint: str = "/health"
        method: str = "GET"

        def response_handler(self, response):
            response.raise_for_status()
            data = response.json()

            # Check status is "ready"
            status = data.get("status", "")
            if status != "ready":
                raise AssertionError(
                    f"Backend health status is '{status}', expected 'ready'"
                )

            # Check that endpoints are listed
            endpoints = data.get("endpoints", {})
            if not endpoints:
                raise AssertionError("Backend health check returned no endpoints")

            # Return summary for validation
            return (
                f"Backend healthy: status={status}, endpoints={list(endpoints.keys())}"
            )

    return BackendHealthPayload(
        body={},
        port=port,
        repeat_count=1,
        expected_log=expected_log or [],
        expected_response=["Backend healthy"],
        timeout=timeout,
    )

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5dafe10 and f8554d4.

📒 Files selected for processing (2)

• tests/serve/test_vllm.py (4 hunks)
• tests/utils/payload_builder.py (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (2)

tests/serve/test_vllm.py (1)

tests/utils/payload_builder.py (1)

• backend_health_check_payload (281-319)

tests/utils/payload_builder.py (1)

tests/utils/payloads.py (6)

• BasePayload (29-98)
• response_handler (54-56)
• response_handler (154-155)
• response_handler (176-177)
• response_handler (217-218)
• response_handler (247-249)

🪛 Ruff (0.14.5)

tests/utils/payload_builder.py

299-301: Avoid specifying long messages outside the exception class

(TRY003)

---

306-306: Avoid specifying long messages outside the exception class

(TRY003)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)

• GitHub Check: sglang (amd64)
• GitHub Check: vllm (amd64)
• GitHub Check: trtllm (amd64)
• GitHub Check: operator (amd64)
• GitHub Check: operator (arm64)
• GitHub Check: Build and Test - dynamo

🔇 Additional comments (2)

tests/utils/payload_builder.py (1)

288-311: LGTM! Well-structured health check validation.

The inner class pattern is appropriate here, and the validation logic correctly checks both the status field and the presence of endpoints. The GET method is correct for a health endpoint.

Note: Static analysis flagged the exception messages (lines 299-301, 306) as potentially verbose, but for test code where clarity is important, these descriptive messages are actually helpful for debugging.

tests/serve/test_vllm.py (1)

17-17: LGTM! Clean integration of health checks.

The import is correctly added, and the health check payload is consistently placed as the first request in three VLLM test configurations. This ensures the backend is healthy before running subsequent test requests. The consistent use of port 8081 across all configurations is appropriate.

Also applies to: 47-47, 61-61, 74-74