test(realtime-api): add integration test for websocket - #660
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the test coverage for the Realtime WebSocket proxy by introducing a suite of robust integration tests. Previously, this critical component lacked automated CI testing, relying on manual verification. The new tests simulate various real-world scenarios, ensuring the proxy's reliability in handling WebSocket connections, session states, and error conditions, thereby improving the overall stability and confidence in the Realtime API. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds end-to-end Realtime WebSocket tests and supporting configuration: a new test package under Changes
Sequence Diagram(s)sequenceDiagram
participant Test as Test Client
participant GW as Cloud Gateway
participant WS as WebSocket Proxy
participant API as Realtime API
Test->>GW: Connect to ws_url with headers
activate GW
GW->>WS: Establish WebSocket tunnel
WS->>API: Open session (connect)
API->>WS: Send session.created event
WS->>Test: Deliver session.created
deactivate GW
Test->>WS: Send user message (request)
WS->>API: Forward message
API->>WS: Emit response.started
WS->>Test: Deliver response.started
loop Stream response
API->>WS: Send response.text.delta
WS->>Test: Deliver text delta
end
API->>WS: Send response.done
WS->>Test: Deliver response.done
Test->>Test: Assert collected text
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
5851eca to
400c970
Compare
|
Hi @pallasathena92, the DCO sign-off check has failed. All commits must include a To fix existing commits: # Sign off the last N commits (replace N with the number of unsigned commits)
git rebase HEAD~N --signoff
git push --force-with-leaseTo sign off future commits automatically:
|
github-actions
Bot
added
tests
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 400c97082c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds comprehensive integration tests for the Realtime WebSocket proxy, covering happy path, error handling, and concurrent sessions to ensure stability and correctness. A security audit of model_gateway/tests/routing/realtime_ws_test.rs, model_gateway/src/routers/openai/realtime/ws.rs, model_gateway/src/routers/openai/realtime/proxy.rs, and model_gateway/src/routers/openai/realtime/registry.rs found no vulnerabilities. Feedback focuses on improving test robustness and debuggability by enhancing assertions, replacing fixed-time sleeps with polling, and using more descriptive panic messages, aligning with best practices for error handling and debugging.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@model_gateway/tests/routing/realtime_ws_test.rs`:
- Around line 72-73: Replace fixed tokio::time::sleep calls with condition-based
polling with a timeout: instead of sleeping then asserting session count, loop
(or use tokio::time::timeout) until env.ctx.realtime_registry.session_count()
reaches the expected value (or another concrete readiness condition) sampling at
short intervals (e.g. 10–50ms) and fail if the timeout elapses; apply this
change to the occurrences around tokio::time::sleep(Duration::from_millis(...))
and assertions that call realtime_registry.session_count() (lines referenced:
72, 128, 147, 274, 398) so tests wait deterministically for the registry state
rather than relying on fixed sleeps.
- Around line 116-126: The test is treating a tokio::time::timeout as success by
using unwrap_or(None) and the drain loop can block indefinitely; replace the
unwrap_or(None) pattern with explicit handling of the timeout Result from
tokio::time::timeout(RECV_TIMEOUT, stream.next()).await (i.e., match
Ok(Some(_)), Ok(None), and Err(_elapsed) separately) so a timeout is treated as
a test failure (or handled distinctly) rather than None, and change the draining
logic that calls stream.next().await in a tight loop to use
tokio::time::timeout(RECV_TIMEOUT, stream.next()).await for each iteration (or
break on timeout) to ensure the drain cannot block forever; reference symbols:
tokio::time::timeout, RECV_TIMEOUT, stream.next(), unwrap_or(None).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7a192b8c-895b-491d-bdfb-50d13beebeb7
📒 Files selected for processing (2)
model_gateway/tests/routing/mod.rsmodel_gateway/tests/routing/realtime_ws_test.rs
400c970 to
36226ad
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 36226ad56c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
♻️ Duplicate comments (1)
model_gateway/tests/routing/realtime_ws_test.rs (1)
138-142:⚠️ Potential issue | 🟡 MinorUnbounded drain loop can block indefinitely.
The drain loop at line 141 lacks timeout protection. If the upstream malfunctions and keeps sending messages, the test will hang forever. Consider adding a bounded drain with timeout.
🛡️ Proposed fix to add bounded drain with timeout
Some(Ok(_)) => { // Upstream may have buffered messages before the close frame; - // drain them so we reach the terminal close/None. - while let Some(Ok(_)) = stream.next().await {} + // drain them (bounded) so we reach the terminal close/None. + for _ in 0..100 { + match tokio::time::timeout(Duration::from_millis(100), stream.next()).await { + Ok(Some(Ok(tungstenite::Message::Close(_)))) | Ok(None) | Err(_) => break, + Ok(Some(Ok(_))) => continue, + Ok(Some(Err(_))) => break, + } + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@model_gateway/tests/routing/realtime_ws_test.rs` around lines 138 - 142, The drain loop in the Some(Ok(_)) arm (where the code currently does while let Some(Ok(_)) = stream.next().await {}) is unbounded and can hang; replace it with a bounded drain using a timeout or max-iterations guard (e.g., tokio::time::timeout with a short duration or a counter) so the loop exits if upstream keeps sending messages or doesn't yield a close; update the code that references stream.next().await accordingly to break on timeout or when the stream returns None/Err to avoid indefinite blocking in the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@model_gateway/tests/routing/realtime_ws_test.rs`:
- Around line 138-142: The drain loop in the Some(Ok(_)) arm (where the code
currently does while let Some(Ok(_)) = stream.next().await {}) is unbounded and
can hang; replace it with a bounded drain using a timeout or max-iterations
guard (e.g., tokio::time::timeout with a short duration or a counter) so the
loop exits if upstream keeps sending messages or doesn't yield a close; update
the code that references stream.next().await accordingly to break on timeout or
when the stream returns None/Err to avoid indefinite blocking in the test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4bed8cde-7c25-4360-84a4-676adb8a1eb6
📒 Files selected for processing (2)
model_gateway/tests/routing/mod.rsmodel_gateway/tests/routing/realtime_ws_test.rs
36226ad to
8c7514b
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8c7514b7e7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
I think it is better we add this in |
thanks for suggestion. let me update it. |
8c7514b to
f4d3316
Compare
github-actions
Bot
added
dependencies
f4d3316 to
ed8a017
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@e2e_test/realtime/test_realtime_ws.py`:
- Around line 167-174: The test only logs success and lacks an explicit check;
change the async helper call to return a result and assert it in
test_session_update: have _run return the value produced by _realtime_session
(e.g., the received event or a boolean flag) by writing "result = await
_realtime_session(ws_url, ws_headers)" inside _run, then call "result =
asyncio.run(_run())" and add an assertion like "assert result is True" or
"assert result.get('type') == 'session.updated'". Update _realtime_session to
return the received session.updated payload if it doesn't already, and assert
against that payload in test_session_update.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9531e2da-4396-40b6-9acc-002f381e82d6
📒 Files selected for processing (5)
.github/workflows/pr-test-rust.ymle2e_test/pyproject.tomle2e_test/realtime/__init__.pye2e_test/realtime/test_realtime_ws.pyscripts/ci_install_e2e_deps.sh
84cf435 to
f5c152b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@e2e_test/realtime/test_realtime_ws.py`:
- Line 26: Replace the silent skip via pytest.importorskip: remove the line
"websockets = pytest.importorskip('websockets')" and instead try a normal import
of the websockets module, catching ImportError and calling pytest.fail with a
clear message about the missing dependency so the test suite fails loudly;
locate the usage of the module via the symbol "websockets" in
test_realtime_ws.py and ensure the import replacement preserves that name.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 289e0bb9-033e-4f12-9b4e-2a4db0d7fa22
📒 Files selected for processing (5)
.github/workflows/pr-test-rust.ymle2e_test/pyproject.tomle2e_test/realtime/__init__.pye2e_test/realtime/test_realtime_ws.pyscripts/ci_install_e2e_deps.sh
f5c152b to
3e93849
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e938495ca
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
3e93849 to
39e8af6
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 39e8af6cf7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
39e8af6 to
a9482c1
Compare
a2644e1 to
0ccadb2
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0ccadb25ad
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| @pytest.mark.e2e | ||
| class TestRealtimeWebSocket: |
There was a problem hiding this comment.
Add vendor/GPU markers to this test class, otherwise the new suite is filtered out in CI: e2e-vendor sets E2E_VENDOR and E2E_GPU_TIER=0 in .github/workflows/pr-test-rust.yml, and pytest_collection_modifyitems in e2e_test/fixtures/hooks.py drops any test missing matching @pytest.mark.vendor(...) and @pytest.mark.gpu(...). With only @pytest.mark.e2e here, the openai-realtime matrix entry can run with zero selected tests, so the websocket coverage added in this commit doesn’t actually execute.
Useful? React with 👍 / 👎.
Signed-off-by: yifeliu <yifengliu9@gmail.com>
0ccadb2 to
900e520
Compare
3 participants
Description
Problem
The Realtime WebSocket proxy (/v1/realtime) was added in the previous PR but lacks end-to-end test coverage. Without e2e tests, regressions in session lifecycle, text generation, and function calling over the WebSocket proxy could go undetected.
Solution
Add a comprehensive E2E test suite for the Realtime WebSocket proxy that exercises the full request path through the gateway to OpenAI's cloud Realtime API. The tests use the websockets Python library with sync test methods wrapping asyncio.run(), following the same patterns as the existing e2e_test/responses/ tests.
The test suite covers:
Changes
e2e_test/realtime/__init__.py— New test package with module docstringe2e_test/realtime/test_realtime_ws.py— E2E test suite with 12 tests:test_session_created_on_connect— connect and verify session.created eventtest_session_update— configure session and verify session.updated eventtest_text_response— send user message, collect streamed text deltastest_missing_model_returns_error— verify WebSocket rejection without model paramtest_multi_turn_conversation— Two-turn conversation — model remembers "Alice" from turn 1test_conversation_item_created_event— Server echoes conversation.item.created after client sends an itemtest_response_cancel— Cancel mid-stream → response.done with status: "cancelled"test_invalid_event_returns_error— Unknown event type → server returns error eventtest_missing_auth_returns_error— No Authorization header → connection rejectedtest_session_created_formattest_response_done_formattest_response_text_delta_formate2e_test/pyproject.toml— Add websockets dependencyscripts/ci_install_e2e_deps.sh— Add websockets to CI pip install.github/workflows/pr-test-rust.yml— Add e2e_test/realtime to the agentic-apis test matrixTest Plan
Checklist
cargo +nightly fmtpassescargo clippy --all-targets --all-features -- -D warningspassesSummary by CodeRabbit
Tests
Chores