test(gax): add tests for bidi_stream and bidi_stream_with_status - #6313
test(gax): add tests for bidi_stream and bidi_stream_with_status#6313joshuatants wants to merge 1 commit into
Conversation
![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 refactors the gRPC Rust client integration tests to use the newly implemented bidi_stream and bidi_stream_with_status APIs instead of the temporary invoker helper, which has been removed. It adds comprehensive test coverage for bidirectional streaming, including happy paths, cancellation, stream dropping, mid-stream server errors, and initial connection errors. There are no review comments to address, and I have no feedback to provide.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## grpc_rust #6313 +/- ##
=============================================
+ Coverage 96.23% 96.27% +0.04%
=============================================
Files 286 286
Lines 74036 74072 +36
=============================================
+ Hits 71250 71315 +65
+ Misses 2786 2757 -29 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
For googleapis#5991. Also remove previous test-only helper function `invoker`.
ace1533 to
6549f01
Compare
| tx, | ||
| mut stream, | ||
| metadata, | ||
| .. |
There was a problem hiding this comment.
In Rust, when we destructure a struct like this, the unmentioned fields (e.g. _server_task, _client) will be dropped, right? So these tasks might continue running in the background, and any panic or failure in the background will be silently ignored.
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn bidi_call_yields_error_on_server_error_status() -> anyhow::Result<()> { |
There was a problem hiding this comment.
Quick check of my understanding:
IIUC in this test case, the error is only observed when stream.message().await is called.
So this doesn't test the immediate error check in invoke_bidi on line 214.
Do you want to test the case where invoke_bidi returns an error immediately?
| }) | ||
| } | ||
|
|
||
| // TODO(#5991): Add integration tests for `GrpcRustClient::bidi_stream` and `GrpcRustClient::bidi_stream_with_status` (covering happy paths and request stream failures). |
There was a problem hiding this comment.
I think the unit test cases do not cover the case of request stream failure? Do we need to test this case?
2 participants
For #5991.
Also remove previous test-only helper function
invoker.