test: add reusable end-to-end LLM mock server - #133
nachiketb-nvidia wants to merge 2 commits into
Conversation
Signed-off-by: nachiketb <nachiketb@nvidia.com>
Signed-off-by: nachiketb <nachiketb@nvidia.com>
WalkthroughChangesMock LLM Server
Estimated code review effort: 4 (Complex) | ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
crates/switchyard-test-server/tests/server.rs (1)
16-155: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd concise comments for the protocol behaviors under test.
The test names identify broad intent, but the provider-specific response, SSE framing, and canonical-error contracts are important enough to document inline.
crates/switchyard-test-server/tests/server.rs#L16-L155: add short comments describing the buffered-format, stream-terminator, and error-injection contracts.crates/switchyard-server/tests/server.rs#L116-L188: document why classifier routing produces three upstream calls.crates/switchyard-server/tests/server.rs#L316-L387: document the expected OpenAI stream terminator and canonical upstream-error mapping.As per coding guidelines, use concise comments for important behavioral tests.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/switchyard-test-server/tests/server.rs` around lines 16 - 155, Add concise inline comments at crates/switchyard-test-server/tests/server.rs lines 16-155 documenting buffered provider-format responses, SSE stream terminators/framing, and model/header error injection. At crates/switchyard-server/tests/server.rs lines 116-188, explain why classifier routing results in three upstream calls; at lines 316-387, document the expected OpenAI stream terminator and canonical upstream-error mapping. Modify only these behavioral-test comments.Source: Coding guidelines
crates/switchyard-test-server/src/lib.rs (1)
224-264: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the request and streaming behavior contracts.
Add brief comments for capture-before-error handling, model-error precedence, and codec/SSE terminal framing. These are non-obvious test-server contracts.
Suggested clarification
async fn handle_request(...) -> Response { + // Capture before applying injected failures so tests can inspect every request. state.requests.lock().await.push(...); + // Model-specific failures take precedence over the per-request status override. if let Some(status) = state.model_errors.get(model).copied() {fn stream_response(...) -> Response { + // Translate canonical chunks, then append OpenAI's required `[DONE]` terminator. let chunks: LlmResponseStream = Box::pin(...);As per coding guidelines, use concise comments for complex validation, routing, async, and lifecycle logic.
Also applies to: 337-366
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/switchyard-test-server/src/lib.rs` around lines 224 - 264, Add concise comments in handle_request documenting that requests are captured before any error response, configured model errors take precedence over header-requested errors, and streaming responses must preserve codec-specific SSE terminal framing. Add the corresponding terminal-framing comment in stream_response, without changing behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@crates/switchyard-test-server/src/lib.rs`:
- Around line 224-264: Add concise comments in handle_request documenting that
requests are captured before any error response, configured model errors take
precedence over header-requested errors, and streaming responses must preserve
codec-specific SSE terminal framing. Add the corresponding terminal-framing
comment in stream_response, without changing behavior.
In `@crates/switchyard-test-server/tests/server.rs`:
- Around line 16-155: Add concise inline comments at
crates/switchyard-test-server/tests/server.rs lines 16-155 documenting buffered
provider-format responses, SSE stream terminators/framing, and model/header
error injection. At crates/switchyard-server/tests/server.rs lines 116-188,
explain why classifier routing results in three upstream calls; at lines
316-387, document the expected OpenAI stream terminator and canonical
upstream-error mapping. Modify only these behavioral-test comments.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: acfad70e-41db-4cbe-949d-5ad6788bfa4a
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock,!Cargo.lock
📒 Files selected for processing (8)
.agents/skills/switchyard-testing-ci/SKILL.mdCargo.tomlcrates/switchyard-server/Cargo.tomlcrates/switchyard-server/tests/server.rscrates/switchyard-test-server/Cargo.tomlcrates/switchyard-test-server/src/lib.rscrates/switchyard-test-server/src/main.rscrates/switchyard-test-server/tests/server.rs
|
Could we try https://github.com/vidaiUK/VidaiMock or https://github.com/StacklokLabs/mockllm instead, to avoid maintaining it ourselves, and get much higher API coverage? In my experience these tend to turn into a full time job. |
What
switchyard-test-serverworkspace crateswitchyard-serverintegration testsWhy
Switchyard integration tests need one reusable provider mock rather than independent,
partially compatible servers embedded in each test suite. Generating responses through
switchyard-translationkeeps the mock aligned with the same wire formats the productsupports.
Review
publish = falseand dev-only consumption keep this out of production artifactsValidation
cargo test -p switchyard-test-servercargo test -p switchyard-servercargo test --workspacecargo fmt --all --checkcargo clippy --workspace --all-targets -- -D warnings/v1/chat/completionsSummary by CodeRabbit
New Features
Tests
Documentation