diff --git a/AGENTS.md b/AGENTS.md index e05a3dbb..f4e38bc0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -307,6 +307,11 @@ project: - Use dashes (`-`) for list bullets. - Use GitHub-flavoured Markdown footnotes (`[^1]`) for references and footnotes. + - Exception: roadmap-shaped Markdown edited with `mapsplice` (for example + `docs/roadmap.md`) must use inline links instead, because the `mapsplice` + roadmap grammar rejects footnote references (`unsupported inline node + footnoteReference`). Cite references inline, either as a parenthetical + `(see [target](path))` or by linking an existing phrase. ## Additional tooling diff --git a/docs/documentation-style-guide.md b/docs/documentation-style-guide.md index d4334c3a..a6fc9a31 100644 --- a/docs/documentation-style-guide.md +++ b/docs/documentation-style-guide.md @@ -62,6 +62,10 @@ consistent, and easy to maintain across projects. - Do not wrap tables. - Use GitHub-flavoured numeric footnotes referenced as `[^1]`. - Footnotes must be numbered in order of appearance in the document. +- Exception: roadmap-shaped Markdown edited with `mapsplice` (for example + `docs/roadmap.md`) must cite references with inline links instead of + footnotes, because the `mapsplice` roadmap grammar rejects footnote + references. - Caption every table, and caption every diagram. ## Standard document types diff --git a/docs/roadmap.md b/docs/roadmap.md index 75c8be66..9055a848 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -207,10 +207,12 @@ stream. - [x] 6.3.1. Provide a helper (for example `Response::with_channel`) that returns a bounded channel sender alongside a `Response::MultiPacket` so - handlers can opt into streaming ergonomically.[^adr-0001] + handlers can opt into streaming ergonomically (see + [ADR 0001](adr-001-multi-packet-streaming-response-api.md)). - [x] 6.3.2. Update the multi-packet design documentation and user guide with tuple return examples that explain initial-frame handling, back-pressure, and - graceful termination.[^adr-0001] + graceful termination (see + [ADR 0001](adr-001-multi-packet-streaming-response-api.md)). - [x] 6.3.3. Add an example handler (or test fixture) demonstrating spawning a background task that pushes frames through the returned sender while the connection actor manages delivery. @@ -258,7 +260,8 @@ into a single frame, making the process transparent to the application logic. ## 8. Streaming requests and shared message assembly -This phase implements the decisions from ADR 0002,[^adr-0002] adding +This phase implements the decisions from +[ADR 0002](adr-002-streaming-requests-and-shared-message-assembly.md), adding first-class streaming request bodies, a generic message assembly abstraction, and standardized per-connection memory budgets. @@ -371,9 +374,9 @@ integration boundaries. ### 9.2. Fragment adaptor alignment - [x] 9.2.1. Introduce a `FragmentAdapter` trait as described in the - fragmentation design.[^fragmentation-design] Fragmentation behaviour must - explicitly define duplicate handling, out-of-order policies, and ownership of - purge scheduling. + [fragmentation](generic-message-fragmentation-and-re-assembly-design.md) + design. Fragmentation behaviour must explicitly define duplicate handling, + out-of-order policies, and ownership of purge scheduling. - [x] Make fragmentation opt-in by requiring explicit configuration on the `WireframeApp` builder. - [x] Expose a public purge API, so callers can drive timeout eviction. @@ -389,7 +392,8 @@ integration boundaries. ### 9.3. Unified codec handling - [x] 9.3.1. Unify codec handling between the app router and the `Connection` - actor.[^outbound-design] + actor (see + [outbound messaging design](asynchronous-outbound-messaging-design.md)). - [x] Route app-level request and response handling through the `FramePipeline` so fragmentation and metrics apply consistently. - [x] Remove duplicate codec construction in `src/app/inbound_handler.rs`; the @@ -398,8 +402,9 @@ integration boundaries. fragmentation, sequential requests, disabled fragmentation). - [x] Add BDD behavioural tests exercising the unified codec path. - [x] Note: protocol hooks (`before_send`) are deferred to a follow-up - stage because `F::Frame` and `Envelope` types may - differ.[^streaming-design] + stage because `F::Frame` and `Envelope` types may differ; see the + [streaming responses](multi-packet-and-streaming-responses-design.md) + design. ### 9.4. Property-based codec tests @@ -412,12 +417,15 @@ integration boundaries. ### 9.5. Serializer boundaries and protocol metadata - [x] 9.5.1. Decouple message encoding from `bincode`-specific traits to - support alternative serializers.[^router-design][^adr-005] + support alternative serializers (see the + [binary router library design](rust-binary-router-library-design.md) and + [ADR 0005](adr-005-serializer-abstraction.md)). - [x] Introduce a serializer-agnostic message trait or adaptor layer for `Message` types. - [x] Provide optional wire-rs or Serde bridges to reduce manual boilerplate. - [x] Define how frame metadata is exposed to the deserialization context to - enable version negotiation.[^message-versioning] + enable version negotiation (see the + [message versioning design](message-versioning.md)). - [x] Add migration guidance covering existing `bincode` users. ### 9.6. Codec performance benchmarks @@ -436,7 +444,7 @@ integration boundaries. invalid frames, including oversized payloads and correlation metadata. - [x] 9.7.3. Introduce a test observability harness in `wireframe_testing` that captures logs and metrics per test run for asserting codec failures and - recovery policies.[^adr-006] + recovery policies (see [ADR 0006](adr-006-test-observability.md)). - [x] 9.7.4. Add regression tests backed by `wireframe_testing` for the `CodecError` taxonomy and recovery policy behaviours defined in 9.1.2. Requires 9.1.2. @@ -614,26 +622,29 @@ Wireframe's protocol, framing, and message assembly layers. ### 15.2. Protocol contract decisions +Each decision below is analysed in the +[formal verification guide](formal-verification-methods-in-wireframe.md); the +cited section names the specific question it answers. + - [ ] 15.2.1. Support a determined set of length-prefix widths (either `1`, `2`, `4`, and `8`, or the full `1..=8` range) and enforce them in constructors, conversions, and tests; record the decision in an ADR. Requires - 15.1.1. See the formal verification guide[^fv-guide] §"What widths does - Wireframe actually support for length prefixes?". Success criteria: an ADR - records the decision, constructors enforce the chosen set, and existing tests - cover rejected widths. + 15.1.1. See §"What widths does Wireframe actually support for length + prefixes?". Success criteria: an ADR records the decision, constructors + enforce the chosen set, and existing tests cover rejected widths. - [ ] 15.2.2. Treat `total_body_len` as either authoritative or advisory and enforce or rename it consistently across the message assembly path; record the decision in an ADR and add tests for both conforming and violating - inputs. Requires 15.1.1. See the formal verification guide[^fv-guide] §"Is - `total_body_len` authoritative or advisory?". Success criteria: an ADR - records the decision, runtime code enforces the chosen semantics, and tests - verify both conforming and violating inputs. + inputs. Requires 15.1.1. See §"Is `total_body_len` authoritative or + advisory?". Success criteria: an ADR records the decision, runtime code + enforces the chosen semantics, and tests verify both conforming and violating + inputs. - [ ] 15.2.3. Publish named fairness and priority guarantees for `ConnectionActor` and encode them as model properties for Stateright checks. - Requires 15.1.1. See the formal verification guide[^fv-guide] §"What fairness - guarantee does `ConnectionActor` actually make?". Success criteria: the - design document enumerates each guarantee as a named property that can be - referenced by Stateright model checks. + Requires 15.1.1. See §"What fairness guarantee does `ConnectionActor` + actually make?". Success criteria: the design document enumerates each + guarantee as a named property that can be referenced by Stateright model + checks. ### 15.3. Kani bounded model checks @@ -801,7 +812,8 @@ ecosystem. - [ ] 18.2.1. Implement a formal message versioning system to allow for protocol evolution. - [ ] 18.2.2. Ensure version negotiation can consume codec metadata without - leaking framing details into handlers.[^message-versioning] + leaking framing details into handlers (see the + [message versioning design](message-versioning.md)). ### 18.3. Security @@ -832,25 +844,3 @@ and usability. - [ ] 19.4.1. Ensure all public items have clear, useful documentation examples. - [ ] 19.4.2. Publish documentation to `docs.rs`. - -[^adr-0001]: Refer to -[ADR 0001](adr-001-multi-packet-streaming-response-api.md). -[^adr-0002]: Refer to -[ADR 0002](adr-002-streaming-requests-and-shared-message-assembly.md). -[^fragmentation-design]: See - [fragmentation doc](generic-message-fragmentation-and-re-assembly-design.md). -[^outbound-design]: See - [outbound messaging design](asynchronous-outbound-messaging-design.md). -[^streaming-design]: See - [streaming responses design](multi-packet-and-streaming-responses-design.md). -[^router-design]: See -[rust-binary-router-library-design.md](rust-binary-router-library-design.md). -[^message-versioning]: See -[message-versioning.md](message-versioning.md). -[^adr-005]: See -[adr-005-serializer-abstraction.md](adr-005-serializer-abstraction.md). -[^adr-006]: See -[adr-006-test-observability.md](adr-006-test-observability.md). - -[^fv-guide]: See -[formal-verification-methods-in-wireframe.md](formal-verification-methods-in-wireframe.md). diff --git a/src/client/tests/error_handling.rs b/src/client/tests/error_handling.rs index 0b64d275..ed985787 100644 --- a/src/client/tests/error_handling.rs +++ b/src/client/tests/error_handling.rs @@ -30,9 +30,15 @@ use crate::{ /// /// This helper is used by tests that need to manipulate the server stream after connection, /// such as sending invalid data or dropping the connection at specific times. +/// +/// # Errors +/// Returns an error if the client fails to connect or the accept task cannot be joined. async fn connect_with_server( configure_builder: F, -) -> (WireframeClient, C>, TcpStream) +) -> Result< + (WireframeClient, C>, TcpStream), + Box, +> where S: Serializer + Send + Sync + 'static, F: FnOnce(WireframeClientBuilder) -> WireframeClientBuilder, @@ -41,10 +47,9 @@ where let (addr, accept) = spawn_listener().await; let client = configure_builder(WireframeClient::builder()) .connect(addr) - .await - .expect("connect client"); - let server_stream = accept.await.expect("join accept task"); - (client, server_stream) + .await?; + let server_stream = accept.await?; + Ok((client, server_stream)) } #[tokio::test] @@ -100,7 +105,8 @@ async fn error_callback_invoked_on_deserialize_error() { } }) }) - .await; + .await + .expect("connect with server"); // Send invalid bincode data via the server stream let mut framed = Framed::new(server_stream, LengthDelimitedCodec::new()); @@ -151,7 +157,8 @@ async fn error_callback_invoked_on_send_io_error() { } }) }) - .await; + .await + .expect("connect with server"); // Drop the server side to cause a broken pipe on send drop(server_stream); @@ -201,7 +208,8 @@ async fn error_callback_invoked_on_serialize_error() { } }) }) - .await; + .await + .expect("connect with server"); // Try to send - should fail with serialization error and invoke error hook let result = client.send(&TestMessage(42)).await; diff --git a/src/client/tests/helpers.rs b/src/client/tests/helpers.rs index 50167c80..2d29aed6 100644 --- a/src/client/tests/helpers.rs +++ b/src/client/tests/helpers.rs @@ -24,20 +24,36 @@ pub type CountingHookClosure = Arc Pin + Send>> + Send + Sync>; /// Spawns a TCP listener and returns the address and a handle to accept a connection. +/// +/// This helper feeds signature-locked helpers (used from `lifecycle.rs`), so it +/// keeps returning plain values and fails loudly via `let ... else { panic! }` +/// rather than the bare `expect`/`unwrap`/`unwrap_or_else` panics the test-only +/// lints forbid. pub async fn spawn_listener() -> (SocketAddr, tokio::task::JoinHandle) { - let listener = TcpListener::bind("127.0.0.1:0") - .await - .expect("bind listener"); - let addr = listener.local_addr().expect("listener addr"); + let Ok(listener) = TcpListener::bind("127.0.0.1:0").await else { + panic!("bind listener"); + }; + let Ok(addr) = listener.local_addr() else { + panic!("listener addr"); + }; let accept = tokio::spawn(async move { - let (stream, _) = listener.accept().await.expect("accept client"); + let Ok((stream, _)) = listener.accept().await else { + panic!("accept client"); + }; stream }); (addr, accept) } /// Helper function to test that a builder option is correctly applied to the TCP socket. -pub async fn assert_builder_option(configure_builder: F, assert_option: A) +/// +/// # Errors +/// Returns an error if the client fails to connect or the accept task cannot be +/// joined. +pub async fn assert_builder_option( + configure_builder: F, + assert_option: A, +) -> Result<(), Box> where F: FnOnce(WireframeClientBuilder) -> WireframeClientBuilder, A: FnOnce(&WireframeClient>), @@ -46,12 +62,12 @@ where let client = configure_builder(WireframeClient::builder()) .connect(addr) - .await - .expect("connect client"); + .await?; assert_option(&client); - let _server_stream = accept.await.expect("join accept task"); + let _server_stream = accept.await?; + Ok(()) } /// Helper function to test lifecycle hooks with a connected client. @@ -66,12 +82,16 @@ where C: Send + 'static, { let (addr, accept) = spawn_listener().await; - let client = configure_builder(WireframeClient::builder()) + let Ok(client) = configure_builder(WireframeClient::builder()) .connect(addr) .await - .expect("connect client"); + else { + panic!("connect client"); + }; // Server stream is dropped here, simulating a disconnected server. - let _server = accept.await.expect("join accept task"); + let Ok(_server) = accept.await else { + panic!("join accept task"); + }; client } @@ -112,13 +132,17 @@ where let error_count = Arc::new(AtomicUsize::new(0)); let (addr, accept) = spawn_listener().await; - let mut client = configure_builder(WireframeClient::builder(), error_count.clone()) + let Ok(mut client) = configure_builder(WireframeClient::builder(), error_count.clone()) .connect(addr) .await - .expect("connect client"); + else { + panic!("connect client"); + }; // Drop the server side to cause a disconnection - let server = accept.await.expect("join accept task"); + let Ok(server) = accept.await else { + panic!("join accept task"); + }; drop(server); // Try to receive - should fail and invoke error hook @@ -161,7 +185,9 @@ macro_rules! socket_option_test { ($name:ident, $configure:expr, $assert:expr $(,)?) => { #[tokio::test] async fn $name() { - $crate::client::tests::helpers::assert_builder_option($configure, $assert).await; + $crate::client::tests::helpers::assert_builder_option($configure, $assert) + .await + .expect("assert builder option"); } }; } diff --git a/src/client/tests/messaging.rs b/src/client/tests/messaging.rs index 5514cbf8..0306163a 100644 --- a/src/client/tests/messaging.rs +++ b/src/client/tests/messaging.rs @@ -19,17 +19,26 @@ use crate::{ correlation::CorrelatableFrame, }; +/// Result alias for fallible test helpers and tests in this module. +type TestResult = Result<(), Box>; + +/// Address and server task produced by the spawn helpers. +type ServerHandle = (std::net::SocketAddr, tokio::task::JoinHandle<()>); + /// Spawn a test server with the specified mode. +/// +/// # Errors +/// Returns an error if binding the listener or reading its address fails. async fn spawn_test_server( mode: ServerMode, -) -> (std::net::SocketAddr, tokio::task::JoinHandle<()>) { - let listener = TcpListener::bind("127.0.0.1:0") - .await - .expect("bind listener"); - let addr = listener.local_addr().expect("listener addr"); +) -> Result> { + let listener = TcpListener::bind("127.0.0.1:0").await?; + let addr = listener.local_addr()?; let handle = tokio::spawn(async move { - let (stream, _) = listener.accept().await.expect("accept client"); + let Ok((stream, _)) = listener.accept().await else { + return; + }; let mut framed = Framed::new(stream, LengthDelimitedCodec::new()); while let Some(Ok(bytes)) = framed.next().await { @@ -42,17 +51,24 @@ async fn spawn_test_server( } }); - (addr, handle) + Ok((addr, handle)) } /// Spawn a TCP listener and return an echo server that preserves correlation IDs. -async fn spawn_envelope_echo_server() -> (std::net::SocketAddr, tokio::task::JoinHandle<()>) { +/// +/// # Errors +/// Returns an error if binding the listener or reading its address fails. +async fn spawn_envelope_echo_server() +-> Result> { spawn_test_server(ServerMode::Echo).await } /// Spawn a server that returns envelopes with a different correlation ID. -async fn spawn_mismatched_correlation_server() -> (std::net::SocketAddr, tokio::task::JoinHandle<()>) -{ +/// +/// # Errors +/// Returns an error if binding the listener or reading its address fails. +async fn spawn_mismatched_correlation_server() +-> Result> { spawn_test_server(ServerMode::Mismatch).await } @@ -87,7 +103,7 @@ async fn next_correlation_id_generates_sequential_unique_ids() { #[tokio::test] async fn send_envelope_auto_generates_correlation_id_when_none() { - let (addr, server) = spawn_envelope_echo_server().await; + let (addr, server) = spawn_envelope_echo_server().await.expect("spawn server"); let mut client = WireframeClient::builder() .connect(addr) @@ -111,7 +127,7 @@ async fn send_envelope_auto_generates_correlation_id_when_none() { #[tokio::test] async fn send_envelope_preserves_existing_correlation_id() { - let (addr, server) = spawn_envelope_echo_server().await; + let (addr, server) = spawn_envelope_echo_server().await.expect("spawn server"); let mut client = WireframeClient::builder() .connect(addr) @@ -133,7 +149,7 @@ async fn send_envelope_preserves_existing_correlation_id() { #[tokio::test] async fn receive_envelope_returns_envelope_with_correlation_id() { - let (addr, server) = spawn_envelope_echo_server().await; + let (addr, server) = spawn_envelope_echo_server().await.expect("spawn server"); let mut client = WireframeClient::builder() .connect(addr) @@ -160,7 +176,7 @@ async fn receive_envelope_returns_envelope_with_correlation_id() { #[tokio::test] async fn call_correlated_validates_matching_correlation_id() { - let (addr, server) = spawn_envelope_echo_server().await; + let (addr, server) = spawn_envelope_echo_server().await.expect("spawn server"); let mut client = WireframeClient::builder() .connect(addr) @@ -183,7 +199,9 @@ async fn call_correlated_validates_matching_correlation_id() { #[tokio::test] async fn call_correlated_returns_error_on_correlation_mismatch() { - let (addr, server) = spawn_mismatched_correlation_server().await; + let (addr, server) = spawn_mismatched_correlation_server() + .await + .expect("spawn server"); let mut client = WireframeClient::builder() .connect(addr) @@ -211,7 +229,9 @@ async fn call_correlated_returns_error_on_correlation_mismatch() { #[tokio::test] async fn call_correlated_invokes_error_hook_on_mismatch() { - let (addr, server) = spawn_mismatched_correlation_server().await; + let (addr, server) = spawn_mismatched_correlation_server() + .await + .expect("spawn server"); let error_count = Arc::new(AtomicUsize::new(0)); let count = error_count.clone(); @@ -241,7 +261,7 @@ async fn call_correlated_invokes_error_hook_on_mismatch() { #[tokio::test] async fn multiple_sequential_calls_get_unique_correlation_ids() { - let (addr, server) = spawn_envelope_echo_server().await; + let (addr, server) = spawn_envelope_echo_server().await.expect("spawn server"); let mut client = WireframeClient::builder() .connect(addr) @@ -281,8 +301,8 @@ async fn multiple_sequential_calls_get_unique_correlation_ids() { #[case(vec![0u8; 100])] #[case(vec![255u8; 500])] #[tokio::test] -async fn round_trip_with_various_payload_sizes(#[case] payload: Vec) { - let (addr, server) = spawn_envelope_echo_server().await; +async fn round_trip_with_various_payload_sizes(#[case] payload: Vec) -> TestResult { + let (addr, server) = spawn_envelope_echo_server().await?; let mut client = WireframeClient::builder() .connect(addr) @@ -298,6 +318,7 @@ async fn round_trip_with_various_payload_sizes(#[case] payload: Vec) { assert_eq!(response.into_parts().into_payload(), payload.as_slice()); server.abort(); + Ok(()) } #[tokio::test] diff --git a/src/client/tests/request_hooks.rs b/src/client/tests/request_hooks.rs index f28315c6..09ba6f21 100644 --- a/src/client/tests/request_hooks.rs +++ b/src/client/tests/request_hooks.rs @@ -14,15 +14,19 @@ use wireframe_testing::ServerMode; use crate::{app::Envelope, client::WireframeClient, correlation::CorrelatableFrame}; +type BoxError = Box; +type TestResult = Result<(), BoxError>; + /// Spawn a simple echo server that reads one frame and echoes it back. -async fn spawn_echo_server() -> (std::net::SocketAddr, tokio::task::JoinHandle<()>) { - let listener = TcpListener::bind("127.0.0.1:0") - .await - .expect("bind listener"); - let addr = listener.local_addr().expect("listener addr"); +async fn spawn_echo_server() -> Result<(std::net::SocketAddr, tokio::task::JoinHandle<()>), BoxError> +{ + let listener = TcpListener::bind("127.0.0.1:0").await?; + let addr = listener.local_addr()?; let handle = tokio::spawn(async move { - let (stream, _) = listener.accept().await.expect("accept client"); + let Ok((stream, _)) = listener.accept().await else { + return; + }; let mut framed = Framed::new(stream, LengthDelimitedCodec::new()); while let Some(Ok(bytes)) = framed.next().await { @@ -36,18 +40,19 @@ async fn spawn_echo_server() -> (std::net::SocketAddr, tokio::task::JoinHandle<( } }); - (addr, handle) + Ok((addr, handle)) } /// Spawn a server that captures each received frame payload for inspection. -async fn spawn_capturing_server() -> (std::net::SocketAddr, tokio::task::JoinHandle>>) { - let listener = TcpListener::bind("127.0.0.1:0") - .await - .expect("bind listener"); - let addr = listener.local_addr().expect("listener addr"); +async fn spawn_capturing_server() +-> Result<(std::net::SocketAddr, tokio::task::JoinHandle>>), BoxError> { + let listener = TcpListener::bind("127.0.0.1:0").await?; + let addr = listener.local_addr()?; let handle = tokio::spawn(async move { - let (stream, _) = listener.accept().await.expect("accept client"); + let Ok((stream, _)) = listener.accept().await else { + return Vec::new(); + }; let mut framed = Framed::new(stream, LengthDelimitedCodec::new()); let mut captured = Vec::new(); @@ -61,7 +66,7 @@ async fn spawn_capturing_server() -> (std::net::SocketAddr, tokio::task::JoinHan captured }); - (addr, handle) + Ok((addr, handle)) } /// Client type returned by [`connect_client_with_hooks`]. @@ -71,51 +76,59 @@ type TestClient = WireframeClient< >; /// Helper to build and connect a client with custom hook configuration. -async fn connect_client_with_hooks(addr: std::net::SocketAddr, configure: F) -> TestClient +async fn connect_client_with_hooks( + addr: std::net::SocketAddr, + configure: F, +) -> Result where F: FnOnce(crate::client::WireframeClientBuilder) -> crate::client::WireframeClientBuilder, { let builder = WireframeClient::builder(); - configure(builder) - .connect(addr) - .await - .expect("connect client") + Ok(configure(builder).connect(addr).await?) } /// Generic test harness for request hook tests. -/// -/// Spawns an echo server, connects a client with custom hooks, -/// runs the test body, and handles cleanup automatically. -async fn run_hook_test(configure_hooks: F, test_body: T) +async fn run_hook_test(configure_hooks: F, test_body: T) -> TestResult where F: FnOnce(crate::client::WireframeClientBuilder) -> crate::client::WireframeClientBuilder, T: for<'a> FnOnce( &'a mut TestClient, ) -> std::pin::Pin + 'a>>, { - let (addr, server) = spawn_echo_server().await; - let mut client = connect_client_with_hooks(addr, configure_hooks).await; + let (addr, server) = spawn_echo_server().await?; + let mut client = connect_client_with_hooks(addr, configure_hooks).await?; test_body(&mut client).await; drop(client); let _ = server.await; + Ok(()) } /// Variant for tests that need the capturing server. -async fn run_hook_test_with_capture(configure_hooks: F, test_body: T) -> Vec> +async fn run_hook_test_with_capture( + configure_hooks: F, + test_body: T, +) -> Result>, BoxError> where F: FnOnce(crate::client::WireframeClientBuilder) -> crate::client::WireframeClientBuilder, T: for<'a> FnOnce( &'a mut TestClient, ) -> std::pin::Pin + 'a>>, { - let (addr, server) = spawn_capturing_server().await; - let mut client = connect_client_with_hooks(addr, configure_hooks).await; + let (addr, server) = spawn_capturing_server().await?; + let mut client = connect_client_with_hooks(addr, configure_hooks).await?; test_body(&mut client).await; drop(client); - server.await.expect("server completes") + Ok(server.await?) +} + +fn bump(counter: &AtomicUsize) { counter.fetch_add(1, Ordering::SeqCst); } + +/// Lock the shared log, recovering the guard if the mutex was poisoned. +fn lock_log(log: &std::sync::Mutex>) -> std::sync::MutexGuard<'_, Vec> { + log.lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) } -/// Test fixture for tracking hook invocations with a counter. struct HookCounter { counter: Arc, } @@ -129,16 +142,12 @@ impl HookCounter { fn before_send_hook(&self) -> impl Fn(&mut Vec) + Send + Sync + 'static { let count = self.counter.clone(); - move |_bytes: &mut Vec| { - count.fetch_add(1, Ordering::SeqCst); - } + move |_bytes: &mut Vec| bump(&count) } fn after_receive_hook(&self) -> impl Fn(&mut bytes::BytesMut) + Send + Sync + 'static { let count = self.counter.clone(); - move |_bytes: &mut bytes::BytesMut| { - count.fetch_add(1, Ordering::SeqCst); - } + move |_bytes: &mut bytes::BytesMut| bump(&count) } fn assert_count(&self, expected: usize, message: &str) { @@ -146,7 +155,6 @@ impl HookCounter { } } -/// Test fixture for tracking hook invocations with a log. struct HookLog { log: Arc>>, } @@ -160,9 +168,7 @@ impl HookLog { fn before_send_hook(&self, marker: u8) -> impl Fn(&mut Vec) + Send + Sync + 'static { let log = self.log.clone(); - move |_bytes: &mut Vec| { - log.lock().expect("lock").push(marker); - } + move |_bytes: &mut Vec| lock_log(&log).push(marker) } fn after_receive_hook( @@ -170,28 +176,20 @@ impl HookLog { marker: u8, ) -> impl Fn(&mut bytes::BytesMut) + Send + Sync + 'static { let log = self.log.clone(); - move |_bytes: &mut bytes::BytesMut| { - log.lock().expect("lock").push(marker); - } + move |_bytes: &mut bytes::BytesMut| lock_log(&log).push(marker) } fn assert_entries(&self, expected: &[u8], message: &str) { - assert_eq!( - self.log.lock().expect("lock").as_slice(), - expected, - "{message}" - ); + assert_eq!(lock_log(&self.log).as_slice(), expected, "{message}"); } } -/// Fixture: create a fresh [`HookCounter`]. #[rustfmt::skip] #[fixture] fn hook_counter() -> HookCounter { HookCounter::new() } -/// Fixture: create a fresh [`HookLog`]. #[rustfmt::skip] #[fixture] fn hook_log() -> HookLog { @@ -199,40 +197,48 @@ fn hook_log() -> HookLog { } /// Helper: test body that sends an envelope. +/// +/// The harness fixes the future output to `()`, so failures panic via a +/// `let ... else` block. async fn send_envelope_test_body(client: &mut TestClient) { let envelope = Envelope::new(1, None, vec![1, 2, 3]); - client.send_envelope(envelope).await.expect("send envelope"); + let Ok(_) = client.send_envelope(envelope).await else { + panic!("send envelope"); + }; } /// Helper: test body that sends an envelope and receives a response. async fn send_and_receive_test_body(client: &mut TestClient) { - let envelope = Envelope::new(1, None, vec![1, 2, 3]); - client.send_envelope(envelope).await.expect("send envelope"); - let _response: Envelope = client.receive_envelope().await.expect("receive envelope"); + send_envelope_test_body(client).await; + let Ok(_response): Result = client.receive_envelope().await else { + panic!("receive envelope"); + }; } #[rstest] #[tokio::test] -async fn before_send_hook_invoked_on_send(hook_counter: HookCounter) { +async fn before_send_hook_invoked_on_send(hook_counter: HookCounter) -> TestResult { run_hook_test( |b| b.before_send(hook_counter.before_send_hook()), |client| Box::pin(send_envelope_test_body(client)), ) - .await; + .await?; hook_counter.assert_count(1, "before_send hook should be invoked once"); + Ok(()) } #[rstest] #[tokio::test] -async fn after_receive_hook_invoked_on_receive(hook_counter: HookCounter) { +async fn after_receive_hook_invoked_on_receive(hook_counter: HookCounter) -> TestResult { run_hook_test( |b| b.after_receive(hook_counter.after_receive_hook()), |client| Box::pin(send_and_receive_test_body(client)), ) - .await; + .await?; hook_counter.assert_count(1, "after_receive hook should be invoked once"); + Ok(()) } #[rstest] @@ -242,7 +248,7 @@ async fn after_receive_hook_invoked_on_receive(hook_counter: HookCounter) { async fn multiple_hooks_execute_in_registration_order( hook_log: HookLog, #[case] is_before_send: bool, -) { +) -> TestResult { run_hook_test( |b| { if is_before_send { @@ -261,9 +267,10 @@ async fn multiple_hooks_execute_in_registration_order( } }, ) - .await; + .await?; hook_log.assert_entries(b"AB", "hooks should execute in registration order"); + Ok(()) } #[tokio::test] @@ -283,7 +290,8 @@ async fn both_hooks_fire_for_call_correlated() { }) }, ) - .await; + .await + .expect("run hook test"); send_counter.assert_count(1, "before_send fires"); recv_counter.assert_count(1, "after_receive fires"); @@ -304,7 +312,8 @@ async fn no_hooks_configured_works_identically() { }) }, ) - .await; + .await + .expect("run hook test"); assert_eq!( *correlation_id.lock().expect("lock"), @@ -318,7 +327,7 @@ struct Ping(u8); #[rstest] #[tokio::test] -async fn before_send_hook_fires_for_plain_send(hook_counter: HookCounter) { +async fn before_send_hook_fires_for_plain_send(hook_counter: HookCounter) -> TestResult { let _captured = run_hook_test_with_capture( |b| b.before_send(hook_counter.before_send_hook()), |client| { @@ -328,9 +337,10 @@ async fn before_send_hook_fires_for_plain_send(hook_counter: HookCounter) { }) }, ) - .await; + .await?; hook_counter.assert_count(1, "before_send should fire for plain send()"); + Ok(()) } #[tokio::test] @@ -345,7 +355,8 @@ async fn before_send_hook_can_mutate_frame_bytes_on_wire() { }, |client| Box::pin(send_envelope_test_body(client)), ) - .await; + .await + .expect("run hook test"); let frame = captured .first() @@ -384,5 +395,6 @@ async fn after_receive_hook_can_mutate_frame_bytes_before_deserialization() { }) }, ) - .await; + .await + .expect("run hook test"); } diff --git a/src/client/tests/streaming.rs b/src/client/tests/streaming.rs index 7171865a..5c42bdc2 100644 --- a/src/client/tests/streaming.rs +++ b/src/client/tests/streaming.rs @@ -34,7 +34,7 @@ async fn verify_single_frame_stream( where S: StreamExt> + Unpin, { - let frame = stream.next().await.expect("data frame").expect("Ok"); + let frame = stream.next().await.ok_or("data frame")??; assert_eq!(frame.payload, expected_payload); let end = stream.next().await; diff --git a/src/client/tests/tracing.rs b/src/client/tests/tracing.rs index a32ffdba..3c8849ca 100644 --- a/src/client/tests/tracing.rs +++ b/src/client/tests/tracing.rs @@ -22,15 +22,24 @@ use crate::{ /// Concrete client type returned by `builder().connect()` in tests. type TestClient = WireframeClient>; +/// Result alias for fallible test helpers and tests in this module. +type TestResult = Result<(), Box>; + /// Spawn a test echo server that deserialises envelopes and echoes them back. -async fn spawn_echo_server() -> (std::net::SocketAddr, tokio::task::JoinHandle<()>) { - let listener = TcpListener::bind("127.0.0.1:0") - .await - .expect("bind listener"); - let addr = listener.local_addr().expect("listener addr"); +/// +/// # Errors +/// Returns an error if binding the listener or reading its address fails. +async fn spawn_echo_server() -> Result< + (std::net::SocketAddr, tokio::task::JoinHandle<()>), + Box, +> { + let listener = TcpListener::bind("127.0.0.1:0").await?; + let addr = listener.local_addr()?; let handle = tokio::spawn(async move { - let (stream, _) = listener.accept().await.expect("accept client"); + let Ok((stream, _)) = listener.accept().await else { + return; + }; let mut framed = Framed::new(stream, LengthDelimitedCodec::new()); while let Some(Ok(bytes)) = framed.next().await { @@ -43,7 +52,7 @@ async fn spawn_echo_server() -> (std::net::SocketAddr, tokio::task::JoinHandle<( } }); - (addr, handle) + Ok((addr, handle)) } /// Spawn an echo server, connect a client with the given tracing config, @@ -51,19 +60,19 @@ async fn spawn_echo_server() -> (std::net::SocketAddr, tokio::task::JoinHandle<( /// /// The closure takes ownership of the client so that operations such as /// `close()` (which consumes `self`) work without lifetime gymnastics. -async fn with_echo_client(config: TracingConfig, f: F) +async fn with_echo_client(config: TracingConfig, f: F) -> TestResult where F: FnOnce(TestClient, std::net::SocketAddr) -> Fut, Fut: std::future::Future, { - let (addr, server) = spawn_echo_server().await; + let (addr, server) = spawn_echo_server().await?; let client = WireframeClient::builder() .tracing_config(config) .connect(addr) - .await - .expect("connect"); + .await?; f(client, addr).await; server.abort(); + Ok(()) } /// Return a closure that asserts at least one log line contains @@ -91,7 +100,9 @@ pub(super) fn span_assertion( /// scope-bound local injected by `#[traced_test]` into each test body. macro_rules! test_span_emission { ($config:expr, $span_name:expr, $required_fields:expr, $operation:expr $(,)?) => { - with_echo_client($config, $operation).await; + with_echo_client($config, $operation) + .await + .expect("with echo client"); logs_assert(span_assertion($span_name, $required_fields)); }; } @@ -99,7 +110,7 @@ macro_rules! test_span_emission { #[rstest] #[traced_test] #[tokio::test] -async fn connect_emits_span_with_peer_address() { +async fn connect_emits_span_with_peer_address() -> TestResult { let captured_addr = std::sync::OnceLock::new(); with_echo_client( TracingConfig::default().with_connect_timing(true), @@ -108,11 +119,12 @@ async fn connect_emits_span_with_peer_address() { async {} }, ) - .await; + .await?; // The peer address is dynamic, so we capture it from the closure. let addr_str = captured_addr.get().expect("addr captured"); logs_assert(span_assertion("client.connect", &[addr_str])); + Ok(()) } #[rstest] @@ -197,16 +209,17 @@ async fn send_envelope_emits_span_with_correlation_id_and_frame_bytes() { #[rstest] #[traced_test] #[tokio::test] -async fn close_emits_span() { +async fn close_emits_span() -> TestResult { with_echo_client( TracingConfig::default().with_close_timing(true), |client, _addr| async move { client.close().await; }, ) - .await; + .await?; logs_assert(span_assertion("client.close", &[])); + Ok(()) } #[rstest] @@ -280,17 +293,18 @@ async fn receive_error_records_result_err() { #[rstest] #[traced_test] #[tokio::test] -async fn timing_disabled_by_default() { +async fn timing_disabled_by_default() -> TestResult { with_echo_client(TracingConfig::default(), |mut client, _addr| async move { let envelope = Envelope::new(1, None, vec![1, 2, 3]); let _response: Envelope = client.call(&envelope).await.expect("call"); }) - .await; + .await?; assert!( !logs_contain("elapsed_us"), "elapsed_us should not appear when timing is disabled" ); + Ok(()) } #[rstest] @@ -323,7 +337,7 @@ async fn timing_enabled_for_connect() { #[rstest] #[traced_test] #[tokio::test] -async fn all_timing_convenience_enables_all_operations() { +async fn all_timing_convenience_enables_all_operations() -> TestResult { with_echo_client( TracingConfig::default().with_all_timing(true), |mut client, _addr| async move { @@ -331,7 +345,7 @@ async fn all_timing_convenience_enables_all_operations() { let _response: Envelope = client.call(&envelope).await.expect("call"); }, ) - .await; + .await?; // At minimum: connect + send + receive + call = 4 timing events. logs_assert(|lines: &[&str]| { @@ -342,15 +356,16 @@ async fn all_timing_convenience_enables_all_operations() { Err(format!("expected >=4 elapsed_us events, found {count}")) } }); + Ok(()) } #[rstest] #[traced_test] #[tokio::test] -async fn default_config_is_backwards_compatible() { +async fn default_config_is_backwards_compatible() -> TestResult { // No tracing_config() call — uses the default. Verifies no panic // occurs and basic operations succeed with default configuration. - let (addr, server) = spawn_echo_server().await; + let (addr, server) = spawn_echo_server().await?; let mut client = WireframeClient::builder() .connect(addr) .await @@ -358,4 +373,5 @@ async fn default_config_is_backwards_compatible() { let envelope = Envelope::new(1, None, vec![1, 2, 3]); let _response: Envelope = client.call(&envelope).await.expect("call"); server.abort(); + Ok(()) } diff --git a/src/fragment/tests/adapter_tests.rs b/src/fragment/tests/adapter_tests.rs index e0d5b23c..e82dbb36 100644 --- a/src/fragment/tests/adapter_tests.rs +++ b/src/fragment/tests/adapter_tests.rs @@ -13,6 +13,17 @@ use crate::fragment::{ encode_fragment_payload, }; +/// Construct a [`NonZeroUsize`] from a compile-time-known non-zero literal. +/// +/// # Panics +/// Panics if `n` is zero; callers pass non-zero literals only. +const fn nz(n: usize) -> NonZeroUsize { + match NonZeroUsize::new(n) { + Some(v) => v, + None => panic!("value must be non-zero"), + } +} + #[derive(Clone, Debug, PartialEq, Eq)] struct TestPacket { id: u32, @@ -36,8 +47,8 @@ impl Fragmentable for TestPacket { fn adapter_config() -> FragmentationConfig { FragmentationConfig { - fragment_payload_cap: NonZeroUsize::new(4).expect("non-zero"), - max_message_size: NonZeroUsize::new(64).expect("non-zero"), + fragment_payload_cap: nz(4), + max_message_size: nz(64), reassembly_timeout: Duration::from_secs(30), } } @@ -64,34 +75,25 @@ fn assert_fragment_metadata(fragments: &[TestPacket], packet: &TestPacket) { fn reassemble_fragment_sequence( adapter: &mut DefaultFragmentAdapter, fragments: &[TestPacket], -) -> Vec { +) -> Result, Box> { let first = fragments .first() .cloned() - .expect("fragment list must contain at least one fragment"); - assert!( - adapter - .reassemble(first.clone()) - .expect("first fragment should be accepted") - .is_none() - ); - assert!( - adapter - .reassemble(first) - .expect("duplicate fragment should be suppressed") - .is_none() - ); + .ok_or("fragment list must contain at least one fragment")?; + if adapter.reassemble(first.clone())?.is_some() { + return Err("first fragment should be accepted without completing".into()); + } + if adapter.reassemble(first)?.is_some() { + return Err("duplicate fragment should be suppressed".into()); + } - fragments - .iter() - .skip(1) - .cloned() - .filter_map(|fragment| { - adapter - .reassemble(fragment) - .expect("reassembly should not fail") - }) - .collect() + let mut reassembled = Vec::new(); + for fragment in fragments.iter().skip(1).cloned() { + if let Some(packet) = adapter.reassemble(fragment)? { + reassembled.push(packet); + } + } + Ok(reassembled) } #[test] @@ -104,7 +106,8 @@ fn default_fragment_adapter_fragments_and_reassembles_test_packets() { .expect("fragmenting packet should succeed"); assert_fragment_metadata(&fragments, &packet); - let reconstructed = reassemble_fragment_sequence(&mut adapter, &fragments); + let reconstructed = + reassemble_fragment_sequence(&mut adapter, &fragments).expect("reassembly should succeed"); assert_eq!(reconstructed.len(), 1); assert_eq!( reconstructed.first(), diff --git a/src/fragment/tests/fragmenter_tests.rs b/src/fragment/tests/fragmenter_tests.rs index de29f893..2b6c521d 100644 --- a/src/fragment/tests/fragmenter_tests.rs +++ b/src/fragment/tests/fragmenter_tests.rs @@ -17,10 +17,9 @@ use crate::fragment::{ struct DummyMessage(Vec); fn assert_fragment(batch: &FragmentBatch, index: usize, payload: &[u8], is_last: bool) { - let fragment = batch - .fragments() - .get(index) - .expect("fragment missing at requested index"); + let Some(fragment) = batch.fragments().get(index) else { + panic!("fragment missing at requested index"); + }; assert_eq!(fragment.payload(), payload); assert_eq!(fragment.header().is_last_fragment(), is_last); } diff --git a/src/fragment/tests/reassembler_tests.rs b/src/fragment/tests/reassembler_tests.rs index ff096283..ef61af8c 100644 --- a/src/fragment/tests/reassembler_tests.rs +++ b/src/fragment/tests/reassembler_tests.rs @@ -19,21 +19,27 @@ use crate::fragment::{ ReassemblyError, }; +/// Construct a [`NonZeroUsize`] from a compile-time-known non-zero literal. +/// +/// # Panics +/// Panics if `n` is zero; callers pass non-zero literals only. +const fn nz(n: usize) -> NonZeroUsize { + match NonZeroUsize::new(n) { + Some(v) => v, + None => panic!("value must be non-zero"), + } +} + #[fixture] fn reassembler_with_first_fragment( #[default(1)] message_id: u64, #[default(&[])] first_payload: &'static [u8], ) -> Reassembler { - let mut reassembler = Reassembler::new( - NonZeroUsize::new(8).expect("non-zero"), - Duration::from_secs(30), - ); + let mut reassembler = Reassembler::new(nz(8), Duration::from_secs(30)); let first = FragmentHeader::new(MessageId::new(message_id), FragmentIndex::zero(), false); assert!( - reassembler - .push(first, first_payload) - .expect("first fragment accepted") - .is_none() + matches!(reassembler.push(first, first_payload), Ok(None)), + "first fragment should be accepted without completing the message", ); reassembler } diff --git a/tests/fixtures/client_lifecycle.rs b/tests/fixtures/client_lifecycle.rs index 12931612..05113ed5 100644 --- a/tests/fixtures/client_lifecycle.rs +++ b/tests/fixtures/client_lifecycle.rs @@ -2,10 +2,6 @@ //! //! Provides server/client coordination for lifecycle hook scenarios. -#![expect( - clippy::expect_used, - reason = "test code uses expect for concise assertions" -)] #![expect( clippy::excessive_nesting, reason = "async closures within builder patterns are inherently nested" @@ -100,7 +96,9 @@ impl ClientLifecycleWorld { let listener = TcpListener::bind("127.0.0.1:0").await?; let addr = listener.local_addr()?; let handle = tokio::spawn(async move { - let (stream, _) = listener.accept().await.expect("accept"); + let Ok((stream, _)) = listener.accept().await else { + return; + }; behaviour(stream).await; }); @@ -137,9 +135,6 @@ impl ClientLifecycleWorld { /// /// # Errors /// Returns an error if binding fails. - /// - /// # Panics - /// The spawned task panics if accept fails. pub async fn start_standard_server(&mut self) -> TestResult { self.spawn_server(|_stream| async { tokio::time::sleep(Duration::from_millis(100)).await; @@ -151,9 +146,6 @@ impl ClientLifecycleWorld { /// /// # Errors /// Returns an error if binding fails. - /// - /// # Panics - /// The spawned task panics if accept fails. pub async fn start_disconnecting_server(&mut self) -> TestResult { self.spawn_server(|stream| async { drop(stream); @@ -163,19 +155,19 @@ impl ClientLifecycleWorld { /// Start a preamble-aware server that sends acknowledgement. /// + /// The spawned task exits early without serving if the preamble read or ack + /// write fails. + /// /// # Errors /// Returns an error if binding fails. - /// - /// # Panics - /// The spawned task panics if preamble read or ack write fails. pub async fn start_ack_server(&mut self) -> TestResult { self.spawn_server(|mut stream| async move { - let (_preamble, _) = read_preamble::<_, TestPreamble>(&mut stream) - .await - .expect("read preamble"); - write_preamble(&mut stream, &ServerAck { accepted: true }) - .await - .expect("write ack"); + let Ok((_preamble, _)) = read_preamble::<_, TestPreamble>(&mut stream).await else { + return; + }; + let Ok(()) = write_preamble(&mut stream, &ServerAck { accepted: true }).await else { + return; + }; tokio::time::sleep(Duration::from_millis(100)).await; }) .await diff --git a/tests/fixtures/client_pair_harness.rs b/tests/fixtures/client_pair_harness.rs index fd59b323..bce58c30 100644 --- a/tests/fixtures/client_pair_harness.rs +++ b/tests/fixtures/client_pair_harness.rs @@ -37,13 +37,16 @@ impl std::fmt::Debug for ClientPairHarnessWorld { } impl Default for ClientPairHarnessWorld { - #[expect( - clippy::expect_used, - reason = "BDD world cannot propagate errors from Default" - )] fn default() -> Self { + // `Default` cannot propagate errors, so fail loudly if the runtime cannot + // be created. `let ... else { panic! }` avoids the bare + // `expect`/`unwrap`/`unwrap_or_else` panics the test-only lints forbid + // while preserving the fail-fast behaviour. + let Ok(runtime) = tokio::runtime::Runtime::new() else { + panic!("failed to create runtime"); + }; Self { - runtime: tokio::runtime::Runtime::new().expect("failed to create runtime"), + runtime, counter: Arc::new(AtomicUsize::new(0)), pair: None, response: None, diff --git a/tests/fixtures/panic.rs b/tests/fixtures/panic.rs index 2b9ccdfa..9f391bf5 100644 --- a/tests/fixtures/panic.rs +++ b/tests/fixtures/panic.rs @@ -20,16 +20,9 @@ struct PanicServer { } impl PanicServer { - #[expect( - clippy::expect_used, - reason = "panic world should fail loudly if the panic app cannot be built" - )] async fn spawn() -> TestResult { - let factory = || { - TestApp::new() - .and_then(|app| app.on_connection_setup(|| async { panic!("boom") })) - .expect("failed to build panic app") - }; + let factory = + || TestApp::new().and_then(|app| app.on_connection_setup(|| async { panic!("boom") })); let listener = unused_listener()?; let server = WireframeServer::new(factory) .workers(1)