test(storage): verify appendable object chunking and checksums - #6659
Draft
joshuatants wants to merge 32 commits into
Draft
test(storage): verify appendable object chunking and checksums#6659joshuatants wants to merge 32 commits into
joshuatants wants to merge 32 commits into
Conversation
…#6157) For googleapis#5991 Integration tests covering the happy path cases will be added in an upcoming PR.
…gleapis#6313) For googleapis#5991. Also remove previous test-only helper function `invoker`.
For googleapis#5991 More unit tests to follow in the next PR.
…attempt (googleapis#6365) For googleapis#5991. The new code will be tested via integration tests, similar to how Client in grpc.rs is tested solely via integration tests. These will be added in subsequent PRs.
…gleapis#6385) For googleapis#5991. Also refactor test setup for readability.
…gleapis#6421) For googleapis#5991. Mirrors tests from `src/gax-internal/tests/grpc_simple_request.rs`: - `default_endpoint` - `no_request_params` - `override_endpoint` Omitted: - `multiple_endpoints`: subchannel connection pooling (`grpc_subchannel_count`) is not supported by `GrpcRustClient`. - `non_default_endpoint_ipv6`: currently `#[ignore]`'d in `grpc_simple_request.rs`. - default `user-agent` header checks: grpc-rust does not inject a default "tonic/..." user-agent.
For googleapis#5991. Previously, when `self.send_task.join()` returned an error (meaning sending failed or the pipe broke), the client immediately called `self.terminate()` and returned the send error. This meant we might lose any error trailers that the server had sent over. This PR fixes that by introducing a `SendState` state machine that manages the `SendTask`. In the event of a send error, the `SendState` records the error *without* aborting receiving responses. This allows the client to continue polling inbound responses so that the receive stream is drained until it receives the server trailers. If the send error was due to the server terminating the stream, then the trailers should contain the server error message and we surface that; if the send error was due to an error on the client-side, then the `SendState` preserves the error so that we can surface it even if the server reports `Ok()`.
…c_rust unary (googleapis#6437) For googleapis#5991. Mirrors tests from `src/gax-internal/tests/grpc_simple_request.rs` and `grpc_user_agent.rs`: - `request_error` (mirrors `grpc_simple_request::request_error`) - `test_explicit_user_agent` (mirrors `grpc_user_agent::test_user_agent` with explicit custom header) Omitted: - `credentials_error`, `connection_error`, `endpoint_error`, `uds_and_tls`: pending error classification support in grpc-rust. - default `user-agent` check: grpc-rust does not inject a default user-agent header. - `attempt_interceptor`: `GrpcRustClient` does not support `AttemptInterceptor`.
…pers (googleapis#6438) For googleapis#5991. `grpc-rust` is removing direct `From/Into` type conversions between `grpc::metadata::MetadataMap` and `tonic::metadata::MetadataMap`. This PR introduces explicit conversion helpers to perform the conversion until built-in conversions are provided in a future `grpc-rust` release. This will becoming a breaking change when switching to `0.10`, so fixing now.
…eapis#6456) For googleapis#5991. Mirrors tests from `src/gax-internal/tests/grpc_retry_loop.rs`: - `no_retry_immediate_success` - `no_retry_immediate_error` - `retry_then_success` - `retry_then_error` Omitted: - `retry_policy_exhausted`: added in a subsequent commit. - `interceptor_on_retry`, `interceptor_on_attempt_complete`: `GrpcRustClient` does not support `AttemptInterceptor`.
… RustlsChannelCredentials (googleapis#6459)
…lient (googleapis#6460) For googleapis#5991. Mirrors test from `src/gax-internal/tests/grpc_retry_loop.rs`: - `retry_policy_exhausted`
…is#6480) For googleapis#5991. Mirrors tests from `src/gax-internal/tests/grpc_timeout.rs`: - `no_timeout` - `timeout_does_not_expire` - `timeout_expires` - `client_config_timeout` (adapted from `test_effective_timeout`)
product-auto-label
Bot
added
the
api: storage
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a new integration test file appendable_object_grpc.rs to verify that chunking, offsets, and checksums are correctly preserved during appendable object operations against a mock gRPC storage server. The feedback suggests optimizing the test assertions by slicing the existing payload buffer instead of allocating new vectors when computing CRC32C values, which avoids unnecessary heap allocations.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## grpc_rust #6659 +/- ##
==========================================
Coverage 96.59% 96.59%
==========================================
Files 312 312
Lines 96002 96002
==========================================
+ Hits 92729 92731 +2
+ Misses 3273 3271 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
vsharonlynn
reviewed
Sep 4, 2026
| WriteEvent::Chunk { | ||
| write_offset: (2 * MIB) as i64, | ||
| size: 2 * MIB, | ||
| crc32c: Some(crc32c::crc32c(&payload[0..2 * MIB])), |
Contributor
There was a problem hiding this comment.
Shouldn't this be the second chunk, therefore 2 * MiB..4 * MiB ?
Contributor
There was a problem hiding this comment.
I think it only passes because the payload is 0x42...
vsharonlynn
requested changes
Sep 4, 2026
| WriteEvent::Chunk { | ||
| write_offset: (4 * MIB) as i64, | ||
| size: MIB, | ||
| crc32c: Some(crc32c::crc32c(&payload[0..MIB])), |
Contributor
There was a problem hiding this comment.
Third chunk is 4 to 5 ?
| WriteEvent::Chunk { | ||
| write_offset: (2 * MIB) as i64, | ||
| size: 2 * MIB, | ||
| crc32c: Some(crc32c::crc32c(&payload[0..2 * MIB])), |
Contributor
There was a problem hiding this comment.
I think it only passes because the payload is 0x42...
joshuatants
force-pushed
the
grpc_rust
branch
from
September 8, 2026 23:15
2bb0478 to
ad81a7b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
For #5991