diff --git a/Cargo.lock b/Cargo.lock index 6653e94357d..f75bde16328 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2115,7 +2115,6 @@ dependencies = [ "prost 0.14.1", "query", "rand 0.9.4", - "serde", "serde_json", "snafu 0.8.6", "store-api", diff --git a/src/client/Cargo.toml b/src/client/Cargo.toml index 236776cc59c..e5bd479cc7d 100644 --- a/src/client/Cargo.toml +++ b/src/client/Cargo.toml @@ -46,6 +46,3 @@ tokio.workspace = true tokio-stream = { workspace = true, features = ["net"] } tonic.workspace = true tonic-prost.workspace = true - -[dev-dependencies] -serde.workspace = true diff --git a/src/client/src/region.rs b/src/client/src/region.rs index 19efb60599d..b1df593bb13 100644 --- a/src/client/src/region.rs +++ b/src/client/src/region.rs @@ -666,19 +666,25 @@ mod tests { #[test] fn test_retry_hint_wire_compatibility() { - #[derive(serde::Deserialize)] - struct Fixture { - name: String, - wire_hint: Option, - expected_hint: String, - } - - let fixtures: Vec = serde_json::from_str(include_str!( - "../../../tests/compatibility/wire/write_retry/retry_hints.json" - )) - .unwrap(); - assert_eq!(fixtures.len(), 5); - for fixture in fixtures { + for (name, wire_hint, expected_hint) in [ + ("legacy_missing", None, RetryHint::NonRetryable), + ("legacy_retryable", Some("retryable"), RetryHint::Retryable), + ( + "legacy_non_retryable", + Some("non_retryable"), + RetryHint::NonRetryable, + ), + ( + "route_refresh", + Some("retry_after_route_refresh"), + RetryHint::RetryAfterRouteRefresh, + ), + ( + "unknown_future_hint", + Some("future_retry_hint"), + RetryHint::NonRetryable, + ), + ] { let mut status = Status::failed_precondition("region unavailable"); status.metadata_mut().insert( common_error::GREPTIME_DB_HEADER_ERROR_CODE, @@ -687,30 +693,19 @@ mod tests { .parse() .unwrap(), ); - if let Some(hint) = fixture.wire_hint { + if let Some(hint) = wire_hint { status.metadata_mut().insert( common_error::GREPTIME_DB_HEADER_ERROR_RETRY_HINT, hint.parse().unwrap(), ); } let error: error::Error = status.into(); - assert_eq!( - error.retry_hint().as_str(), - fixture.expected_hint, - "{}", - fixture.name - ); - assert_eq!( - error.status_code(), - StatusCode::RegionNotReady, - "{}", - fixture.name - ); + assert_eq!(error.retry_hint(), expected_hint, "{name}"); + assert_eq!(error.status_code(), StatusCode::RegionNotReady, "{name}"); assert_eq!( error.tonic_code(), Some(tonic::Code::FailedPrecondition), - "{}", - fixture.name + "{name}" ); } } diff --git a/tests/compatibility/README.md b/tests/compatibility/README.md index 51c87698ff2..404b6332b94 100644 --- a/tests/compatibility/README.md +++ b/tests/compatibility/README.md @@ -6,7 +6,7 @@ SQL restart tests are run via `cargo sqlness compat` and reuse the sqlness-runne ## Wire fixtures -Frozen wire fixtures are exercised by the Rust tests that own their context +Frozen protobuf fixtures are exercised by the Rust tests that own their context conversion, not by SQLness case discovery. The [legacy query-context fixture](wire/read_preference/README.md) checks omitted read-preference defaulting and unrelated metadata preservation: @@ -15,13 +15,6 @@ read-preference defaulting and unrelated metadata preservation: cargo nextest run -p session -E 'test(test_legacy_query_context_wire_defaults_to_leader)' ``` -The [write-retry metadata fixture](wire/write_retry/README.md) checks legacy, -route-refresh, missing, and unknown retry-hint decoding in the client: - -```shell -cargo nextest run -p client -E 'test(test_retry_hint_wire_compatibility)' -``` - ## Quick Start ```shell diff --git a/tests/compatibility/wire/write_retry/README.md b/tests/compatibility/wire/write_retry/README.md deleted file mode 100644 index 8b6499ed5a8..00000000000 --- a/tests/compatibility/wire/write_retry/README.md +++ /dev/null @@ -1,25 +0,0 @@ -# Write-retry metadata compatibility - -`retry_hints.json` freezes gRPC `x-greptime-err-retry-hint` metadata values -and the expected current client decoding. These are constructed protocol fixtures, -not captures from released binaries. - -The client test converts real `tonic::Status` metadata through the production -error decoder. Missing and unknown values default to `NonRetryable`; existing -`retryable` and `non_retryable` values retain their meaning; the new -`retry_after_route_refresh` value preserves its distinct hint. The application -status code and gRPC status code remain unchanged. - -```shell -cargo nextest run -p client -E 'test(test_retry_hint_wire_compatibility)' -``` - -`test_transport_error_parity_and_datanode_mapping` separately exercises these -recognized values over an actual gRPC client/server connection. - -An older decoder that recognizes only the two legacy strings treats the new -string as unknown and uses its existing `NonRetryable` fallback. This is a source -compatibility observation, not a claim that an old binary was run. These tests do -not promise that an old client will refresh routing or perform automatic retries. - -The fixture is hand-authored and must not be regenerated from test output. diff --git a/tests/compatibility/wire/write_retry/retry_hints.json b/tests/compatibility/wire/write_retry/retry_hints.json deleted file mode 100644 index d24da878b3e..00000000000 --- a/tests/compatibility/wire/write_retry/retry_hints.json +++ /dev/null @@ -1,7 +0,0 @@ -[ - {"name": "legacy_missing", "wire_hint": null, "expected_hint": "non_retryable"}, - {"name": "legacy_retryable", "wire_hint": "retryable", "expected_hint": "retryable"}, - {"name": "legacy_non_retryable", "wire_hint": "non_retryable", "expected_hint": "non_retryable"}, - {"name": "route_refresh", "wire_hint": "retry_after_route_refresh", "expected_hint": "retry_after_route_refresh"}, - {"name": "unknown_future_hint", "wire_hint": "future_retry_hint", "expected_hint": "non_retryable"} -]