diff --git a/Cargo.lock b/Cargo.lock index 6596130f..c8765f28 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -97,7 +97,7 @@ checksum = "c7c24de15d275a1ecfd47a380fb4d5ec9bfe0933f309ed5e705b775596a3574d" dependencies = [ "proc-macro2", "quote", - "syn 2.0.104", + "syn 2.0.117", ] [[package]] @@ -108,7 +108,7 @@ checksum = "9035ad2d096bed7955a320ee7e2230574d28fd3c3a0f186cbea1ff3c7eed5dbb" dependencies = [ "proc-macro2", "quote", - "syn 2.0.104", + "syn 2.0.117", ] [[package]] @@ -227,7 +227,7 @@ dependencies = [ "regex", "rustc-hash 1.1.0", "shlex", - "syn 2.0.104", + "syn 2.0.117", "which", ] @@ -550,7 +550,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32a2785755761f3ddc1492979ce1e48d2c00d09311c39e4466429188f3dd6501" dependencies = [ "quote", - "syn 2.0.104", + "syn 2.0.117", ] [[package]] @@ -577,7 +577,7 @@ dependencies = [ "proc-macro2", "quote", "rustc_version", - "syn 2.0.104", + "syn 2.0.117", ] [[package]] @@ -597,10 +597,16 @@ checksum = "bda628edc44c4bb645fbe0f758797143e4e07926f7ebf4e9bdfbd3d2ce621df3" dependencies = [ "proc-macro2", "quote", - "syn 2.0.104", + "syn 2.0.117", "unicode-xid", ] +[[package]] +name = "diff" +version = "0.1.13" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "56254986775e3233ffa9c4d7d3faaf6d36a2c09d30b20687e9f88bc8bafc16c8" + [[package]] name = "digest" version = "0.10.7" @@ -619,7 +625,7 @@ checksum = "97369cbbc041bc366949bc74d34658d6cda5621039731c6310521892a3a20ae0" dependencies = [ "proc-macro2", "quote", - "syn 2.0.104", + "syn 2.0.117", ] [[package]] @@ -819,7 +825,7 @@ checksum = "162ee34ebcb7c64a8abebc059ce0fee27c2262618d7b60ed8faf72fef13c3650" dependencies = [ "proc-macro2", "quote", - "syn 2.0.104", + "syn 2.0.117", ] [[package]] @@ -916,7 +922,7 @@ dependencies = [ "quote", "serde", "serde_json", - "syn 2.0.104", + "syn 2.0.117", "textwrap", "thiserror 1.0.69", "typed-builder", @@ -934,6 +940,29 @@ version = "0.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a8d1add55171497b4705a648c6b583acafb01d58050a51727785f0b2c8e0a2b2" +[[package]] +name = "googletest" +version = "0.14.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f6b5e2f2b556b7b90297a5a35c8267dd43a537923d2b329beefdba2b4ec19d94" +dependencies = [ + "googletest_macro", + "num-traits", + "regex", + "rustversion", +] + +[[package]] +name = "googletest_macro" +version = "0.14.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2ae6abc96141edd26bf5aeec0f119c129c44de3ced09e5073711a02cb74725d0" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.117", +] + [[package]] name = "h2" version = "0.4.11" @@ -1160,7 +1189,7 @@ dependencies = [ "i18n-config", "proc-macro2", "quote", - "syn 2.0.104", + "syn 2.0.117", ] [[package]] @@ -1509,7 +1538,7 @@ dependencies = [ "cfg-if", "proc-macro2", "quote", - "syn 2.0.104", + "syn 2.0.117", ] [[package]] @@ -1728,6 +1757,16 @@ dependencies = [ "termtree", ] +[[package]] +name = "pretty_assertions" +version = "1.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3ae130e2f271fbc2ac3a40fb1d07180839cdbbe443c7a27e1e3c13c5cac0116d" +dependencies = [ + "diff", + "yansi", +] + [[package]] name = "prettyplease" version = "0.2.35" @@ -1735,7 +1774,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "061c1221631e079b26479d25bbf2275bfe5917ae8419cd7e34f13bfc2aa7539a" dependencies = [ "proc-macro2", - "syn 2.0.104", + "syn 2.0.117", ] [[package]] @@ -1779,9 +1818,9 @@ checksum = "dc375e1527247fe1a97d8b7156678dfe7c1af2fc075c9a4db3690ecd2a148068" [[package]] name = "proc-macro2" -version = "1.0.95" +version = "1.0.106" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "02b3e5e68a3a1a02aad3ec490a98007cbc13c37cbe84a3cd7b8e406d76e7f778" +checksum = "8fd00f0bb2e90d81d1044c2b32617f68fcb9fa3bb7640c23e9c748e53fb30934" dependencies = [ "unicode-ident", ] @@ -1829,9 +1868,9 @@ checksum = "a1d01941d82fa2ab50be1e79e6714289dd7cde78eba4c074bc5a4374f650dfe0" [[package]] name = "quote" -version = "1.0.40" +version = "1.0.45" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1885c039570dc00dcb4ff087a89e185fd56bae234ddc7f056a945bf36467248d" +checksum = "41f2619966050689382d2b44f664f4bc593e129785a36d6ee376ddf37259b924" dependencies = [ "proc-macro2", ] @@ -2052,7 +2091,7 @@ dependencies = [ "regex", "rstest-bdd-patterns", "rstest-bdd-policy", - "syn 2.0.104", + "syn 2.0.117", "thiserror 1.0.69", "walkdir", ] @@ -2087,7 +2126,7 @@ dependencies = [ "regex", "relative-path", "rustc_version", - "syn 2.0.104", + "syn 2.0.117", "unicode-ident", ] @@ -2105,7 +2144,7 @@ dependencies = [ "regex", "relative-path", "rustc_version", - "syn 2.0.104", + "syn 2.0.117", "unicode-ident", ] @@ -2129,7 +2168,7 @@ dependencies = [ "proc-macro2", "quote", "rust-embed-utils", - "syn 2.0.104", + "syn 2.0.117", "walkdir", ] @@ -2255,9 +2294,9 @@ dependencies = [ [[package]] name = "rustversion" -version = "1.0.21" +version = "1.0.22" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8a0d197bd2c9dc6e53b84da9556a69ba4cdfab8619eb41a8bd1cc2027a0f6b1d" +checksum = "b39cdef0fa800fc44525c84ccb54a029961a8215f9619753635a9c0d2538d46d" [[package]] name = "rusty-fork" @@ -2384,7 +2423,7 @@ checksum = "d540f220d3187173da220f885ab66608367b6574e925011a9353e4badda91d79" dependencies = [ "proc-macro2", "quote", - "syn 2.0.104", + "syn 2.0.117", ] [[package]] @@ -2430,7 +2469,7 @@ checksum = "5d69265a08751de7844521fd15003ae0a888e035773ba05695c5c759a6f89eef" dependencies = [ "proc-macro2", "quote", - "syn 2.0.104", + "syn 2.0.117", ] [[package]] @@ -2556,9 +2595,9 @@ dependencies = [ [[package]] name = "syn" -version = "2.0.104" +version = "2.0.117" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "17b6f705963418cdb9927482fa304bc562ece2fdd4f616084c50b7023b435a40" +checksum = "e665b8803e7b1d2a727f4023456bbbbe74da67099c585258af0ad9c5013b9b99" dependencies = [ "proc-macro2", "quote", @@ -2645,7 +2684,7 @@ checksum = "4fee6c4efc90059e10f81e6d42c60a18f76588c3d74cb83a0b242a2b6c7504c1" dependencies = [ "proc-macro2", "quote", - "syn 2.0.104", + "syn 2.0.117", ] [[package]] @@ -2656,7 +2695,7 @@ checksum = "6c5e1be1c48b9172ee610da68fd9cd2770e7a4056cb3fc98710ee6906f0c7960" dependencies = [ "proc-macro2", "quote", - "syn 2.0.104", + "syn 2.0.117", ] [[package]] @@ -2729,7 +2768,7 @@ checksum = "6e06d43f1345a3bcd39f6a56dbb7dcab2ba47e68e8ac134855e7e2bdbaf8cab8" dependencies = [ "proc-macro2", "quote", - "syn 2.0.104", + "syn 2.0.117", ] [[package]] @@ -2858,7 +2897,7 @@ checksum = "81383ab64e72a7a8b8e13130c49e3dab29def6d0c7d76a03087b3cf71c5c6903" dependencies = [ "proc-macro2", "quote", - "syn 2.0.104", + "syn 2.0.117", ] [[package]] @@ -2918,7 +2957,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "04659ddb06c87d233c566112c1c9c5b9e98256d9af50ec3bc9c8327f873a7568" dependencies = [ "quote", - "syn 2.0.104", + "syn 2.0.117", ] [[package]] @@ -2968,7 +3007,7 @@ checksum = "29a3151c41d0b13e3d011f98adc24434560ef06673a155a6c7f66b9879eecce2" dependencies = [ "proc-macro2", "quote", - "syn 2.0.104", + "syn 2.0.117", ] [[package]] @@ -3023,7 +3062,7 @@ checksum = "a1249a628de3ad34b821ecb1001355bca3940bcb2f88558f1a8bd82e977f75b5" dependencies = [ "proc-macro-hack", "quote", - "syn 2.0.104", + "syn 2.0.117", "unic-langid-impl", ] @@ -3158,7 +3197,7 @@ dependencies = [ "log", "proc-macro2", "quote", - "syn 2.0.104", + "syn 2.0.117", "wasm-bindgen-shared", ] @@ -3180,7 +3219,7 @@ checksum = "8ae87ea40c9f689fc23f209965b6fb8a99ad69aeeb0231408be24920604395de" dependencies = [ "proc-macro2", "quote", - "syn 2.0.104", + "syn 2.0.117", "wasm-bindgen-backend", "wasm-bindgen-shared", ] @@ -3301,7 +3340,7 @@ checksum = "a47fddd13af08290e67f4acabf4b459f647552718f683a7b415d290ac744a836" dependencies = [ "proc-macro2", "quote", - "syn 2.0.104", + "syn 2.0.117", ] [[package]] @@ -3312,7 +3351,7 @@ checksum = "bd9211b69f8dcdfa817bfd14bf1c97c9188afa36f4750130fcdf3f400eca9fa8" dependencies = [ "proc-macro2", "quote", - "syn 2.0.104", + "syn 2.0.117", ] [[package]] @@ -3544,6 +3583,7 @@ dependencies = [ "dashmap", "derive_more 2.0.1", "futures", + "googletest", "itoa", "leaky-bucket", "log", @@ -3553,6 +3593,7 @@ dependencies = [ "metrics-exporter-prometheus", "metrics-util", "mockall", + "pretty_assertions", "proptest", "rstest 0.26.1", "rstest-bdd", @@ -3610,6 +3651,12 @@ dependencies = [ "bitflags", ] +[[package]] +name = "yansi" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cfe53a6657fd280eaa890a3bc59152892ffa3e30101319d168b781ed6529b049" + [[package]] name = "zerocopy" version = "0.8.26" @@ -3627,7 +3674,7 @@ checksum = "9ecf5b4cc5364572d7f4c329661bcc82724222973f2cab6f050a4e5c22f75181" dependencies = [ "proc-macro2", "quote", - "syn 2.0.104", + "syn 2.0.117", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 7d2bf71a..90638157 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,7 +11,7 @@ categories = ["network-programming", "asynchronous"] documentation = "https://docs.rs/wireframe" [workspace] -members = [".", "crates/wireframe-verification"] +members = [".", "crates/wireframe-verification", "wireframe_testing"] default-members = ["."] resolver = "3" @@ -61,6 +61,8 @@ wireframe = { path = ".", features = ["test-support", "pool", "testkit"] } wireframe_testing = { path = "./wireframe_testing" } logtest = "2.0.0" proptest = "1.7.0" +googletest = "0.14.3" +pretty_assertions = "1.4.1" loom = "0.7.2" async-stream = "0.3.6" serial_test = "3.2.0" diff --git a/benches/codec_performance.rs b/benches/codec_performance.rs index ed4ac480..76716f84 100644 --- a/benches/codec_performance.rs +++ b/benches/codec_performance.rs @@ -6,24 +6,15 @@ //! - fragmented versus unfragmented payload-wrapping overhead. use criterion::{BenchmarkId, Criterion, Throughput, black_box}; - -#[path = "../tests/common/codec_benchmark_support.rs"] -mod codec_benchmark_support; - -#[path = "../tests/common/codec_fragmentation_benchmark_support.rs"] -mod codec_fragmentation_benchmark_support; - -use codec_benchmark_support::{ +use wireframe_testing::codec_benchmarks::{ + FRAGMENT_PAYLOAD_CAP_BYTES, Measurement, + MeasurementExt as _, PayloadClass, VALIDATION_ITERATIONS, benchmark_workloads, measure_decode, measure_encode, -}; -use codec_fragmentation_benchmark_support::{ - FRAGMENT_PAYLOAD_CAP_BYTES, - MeasurementExt as _, measure_fragmentation_overhead, measure_fragmented_wrap, measure_unfragmented_wrap, diff --git a/benches/codec_performance_alloc.rs b/benches/codec_performance_alloc.rs index b035a66e..b34a4d86 100644 --- a/benches/codec_performance_alloc.rs +++ b/benches/codec_performance_alloc.rs @@ -18,19 +18,13 @@ use wireframe::codec::{ LengthDelimitedFrameCodec, examples::{HotlineAdapter, HotlineFrameCodec}, }; - -#[path = "../tests/common/codec_benchmark_support.rs"] -mod codec_benchmark_support; - -#[path = "../tests/common/codec_alloc_benchmark_support.rs"] -mod codec_alloc_benchmark_support; - -use codec_alloc_benchmark_support::{AllocationBaseline, allocation_label}; -use codec_benchmark_support::{ +use wireframe_testing::codec_benchmarks::{ + AllocationBaseline, BenchmarkWorkload, CodecUnderTest, LARGE_PAYLOAD_BYTES, VALIDATION_ITERATIONS, + allocation_label, benchmark_workloads, measure_decode, measure_encode, diff --git a/docs/adr-005-serializer-abstraction.md b/docs/adr-005-serializer-abstraction.md index 4d3a57b5..13310b42 100644 --- a/docs/adr-005-serializer-abstraction.md +++ b/docs/adr-005-serializer-abstraction.md @@ -137,6 +137,32 @@ Accepted implementation details: - Supporting multiple serializers can increase maintenance overhead. - Metadata-aware decoding requires a clear definition of what metadata is exposed and when it is safe to consume it. +- The current `Serializer::serialize` contract returns `Vec`. App outbound + encoding therefore still converts serialized messages into `Bytes` before + codec wrapping. This keeps the 2026-06-05 audit refactor behaviour-preserving + and source-compatible, but it retains one allocation-backed buffer handoff on + outbound response paths. That cost is acceptable for the audit milestone + because changing the serializer return type would be a public contract change + with broad downstream impact. The zero-copy migration is tracked in + and should be reviewed + before the next public API-breaking release or during the zero-copy frame and + payload roadmap work. + +### Deferred zero-copy serializer output migration + +Issue #538 owns the deliberate migration away from `Vec` serializer output. +The expected implementation steps are: + +- Choose the public byte container for serializer output, aligning with the + zero-copy frame and payload roadmap. +- Change `Serializer::serialize` and serializer adaptor implementations to + return that container. +- Update app outbound helpers such as `encode_message_frame` and + `send_response_framed` to remove `Bytes::from(Vec)` conversion points. +- Validate compatibility and performance with existing bincode, Serde bridge, + app response, and codec-driver tests. +- Add targeted regression tests for the selected zero-copy behaviour where the + chosen container exposes observable ownership or sharing semantics. ## Outstanding Decisions diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 4155a831..5505fbab 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -56,6 +56,45 @@ Use this checklist before merging API naming changes: - Label cross-layer terms (for example, `correlation_id`) explicitly as shared metadata. +## App inbound and outbound helper boundaries + +Application response paths must keep message serialization and codec frame +wrapping in one place. Use `app::outbound_encoding::encode_message_frame` when +an app path needs to turn an `EncodeWith` message into a +`FrameCodec::Frame`. Callers should keep transport-specific work at the edge: + +- Raw stream response methods encode the returned codec frame into a byte + buffer and write that buffer to `AsyncWrite`. +- Framed response methods send the returned codec frame through the supplied + framed sink. +- The length-delimited compatibility path intentionally sends the raw + serialized message to `LengthDelimitedCodec`; do not wrap that payload with + the app codec first, because `framed.send` supplies the length prefix. + +Inbound connection handling should also preserve the phase boundary: +`build_dispatchable_envelope` owns decode, fragment reassembly, message +assembly, and the successful deserialization-counter reset. Individual failure +policy remains in `DeserFailureTracker`, so logging, metrics, and threshold +decisions do not drift across inbound call sites. + +Builder methods that change `WireframeApp` type parameters should route through +the shared rebuild helpers in `app::builder::core`. Serializer and codec +transitions use `rebuild_with_params`; connection-state transitions use +`rebuild_with_connection_state`. A teardown hook is typed to the old connection +state, so `on_connection_setup` clears any teardown hook registered before it. +Register teardown after setup when both hooks are required. + +Client pool internals should use `client::pool::sync::lock_or_recover` for +poison-tolerant `Mutex` access. Keep the policy local to pool synchronization +code so scheduler and slot state recovery cannot drift. Recovery logs a warning +and increments the `wireframe_pool_bookkeeping_poison_recoveries_total` +counter. Client connection construction should flow through +`WireframeClientBuilder::into_parts()` and `ClientBuildParts`, which keeps +single-client and pooled-client socket setup, preamble exchange, lifecycle +hooks, request hooks, and tracing configuration on the same path. Pooled lease +methods should go through `PooledClientLease::dispatch_on_connection` so +checkout and recycle-on-error policy stay in one place. + ## Error surface conventions Library-facing errors should stay typed and inspectable by default. Use @@ -139,11 +178,11 @@ continuous integration (CI). Wireframe now uses a hybrid root manifest: the repository root `Cargo.toml` contains both `[package]` and `[workspace]`. -After roadmap items 15.1.1 and 15.1.2 the workspace explicitly lists the root -package and the internal verification crate, while keeping only the root -package as a default member: +The workspace explicitly lists the root package, the internal verification +crate, and the testing helper crate, while keeping only the root package as a +default member: -- `members = [".", "crates/wireframe-verification"]` +- `members = [".", "crates/wireframe-verification", "wireframe_testing"]` - `default-members = ["."]` That means ordinary root-level commands such as `cargo build`, `cargo check`, @@ -153,19 +192,19 @@ to target the main `wireframe` package by default. Use plain root-level Cargo commands for day-to-day work on the main crate. Reach for `--workspace` when a task is explicitly meant to cover every current workspace member, for example repository-wide validation in CI or when a change -also touches `crates/wireframe-verification`. +also touches `crates/wireframe-verification` or `wireframe_testing`. Use `cargo test -p wireframe-verification` to exercise the Stateright crate in isolation. The existing `Makefile` targets still focus on the root `wireframe` crate because the dedicated formal-verification targets belong to later roadmap items. -One Cargo nuance is worth knowing: `cargo metadata` for this repository still -reports the in-tree helper crate `wireframe_testing` in `workspace_members` -because it lives under the repository root as a path dependency. That sits -alongside the explicit `wireframe-verification` member and does not change -day-to-day command ergonomics because `default-members = ["."]` keeps plain -root-level Cargo commands focused on the main `wireframe` package. +Use `cargo test -p wireframe_testing` when changing shared test fixtures, +observability helpers, codec drivers, or other support APIs exported by the +testing helper crate. Use `cargo test -p wireframe` for the root crate when a +change should stay limited to the published library. Plain root-level commands +keep their day-to-day ergonomics because `default-members = ["."]` leaves the +main `wireframe` package as the only default member. ### Workspace manifest test support @@ -194,6 +233,28 @@ behaviour-driven development (BDD) fixture for these assertions. Extend it by loading more workspace-state inputs in `load()` and adding focused verification methods that the step definitions and scenario can reuse. +## Example and benchmark support + +TCP server examples that share the standard +`WireframeApp` runtime shape should use +`examples/support/runtime_bootstrap.rs` for tracing setup, runtime app +construction, listener binding, connection spawning, shutdown-aware accept +loops, and current-thread Tokio runtime startup. Keep example-specific address +parsing, app construction, handlers, and middleware in the example file. + +Codec benchmark helpers live in `wireframe_testing::codec_benchmarks`. Bench +targets, direct unit tests, and BDD fixtures should import the workload matrix, +measurement helpers, fragmentation helpers, and allocation-label helpers from +that module instead of coupling to files under `tests/common` with `#[path]`. + +Fragment transport integration tests import `tests/common/fragment_helpers.rs` +as a facade. Keep the public re-export surface stable there, and place helper +implementation details in responsibility-focused modules under +`tests/common/fragment_helpers/`: app construction and spawning, assertions, +fragmentation configuration, envelope building, error types, and framed +transport. New fragment helpers should be added to the smallest matching module +and re-exported only when more than one test binary needs the helper. + ## Formal verification tooling Formal-verification tools are pinned in repository metadata and installed diff --git a/docs/execplans/code-base-audit-2026-06-05.md b/docs/execplans/code-base-audit-2026-06-05.md new file mode 100644 index 00000000..338cf3bc --- /dev/null +++ b/docs/execplans/code-base-audit-2026-06-05.md @@ -0,0 +1,699 @@ +# Address code-base audit findings + +This ExecPlan (execution plan) is a living document. The sections `Constraints`, +`Tolerances`, `Risks`, `Progress`, `Surprises & Discoveries`, `Decision Log`, +and `Outcomes & Retrospective` must be kept up to date as work proceeds. + +Status: COMPLETE + +## Purpose / big picture + +This plan addresses the refactoring concerns found in the 2026-06-05 code-base +audit. The work improves maintainability by moving repeated send, encoding, +builder-transition, deserialization-policy, pool-lock, pool-lease, workspace, +example-bootstrap, benchmark-helper, and fragment-helper patterns behind +focused reusable modules. A reviewer can observe success by reading the smaller +call sites, running the project gates, and seeing behavioural tests continue to +exercise externally observable client, app, workspace, example, benchmark, and +fragment workflows. + +The implementation deliberately preserves public behaviour. The goal is to make +existing behaviours easier to maintain, not to redesign Wireframe's external +API. + +## Constraints + +- Keep the branch named `code-base-audit-2026-06-05` and track + `origin/code-base-audit-2026-06-05`. +- Do not start implementation until this draft plan has been reviewed and + explicitly approved. +- Preserve public API signatures unless the plan is updated and the user + explicitly approves the public API change. +- Use existing repository patterns before adding new abstractions. +- Keep every code file at or below the repository's 400-line limit. +- Use `rstest` for new unit-style tests and `rstest-bdd` for behavioural tests + where a change affects externally observable workflow behaviour. +- Use `googletest` assertions and `pretty_assertions` in new or substantially + revised tests where they improve failure diagnostics. +- Use `insta` snapshot tests only where multivariant output format consistency + is relevant. None of the currently planned refactors changes formatted + output, so no snapshot tests are planned unless implementation reveals such a + case. +- Use `proptest`, Kani, or Verus only when the change introduces a new invariant + over input ranges, states, orderings, or transitions. The planned work is + mostly structure-preserving extraction, so no new proof or model-checking + milestone is planned unless the implementation adds new decision logic. +- Validate each major milestone with `make check-fmt`, `make lint`, and + `make test`, using `tee` to write logs under `/tmp`. +- Run `coderabbit review --agent` after deterministic gates pass for each major + milestone. If rate-limited, run `vsleep $(shuf -i 15-30 -n 1)m` before + retrying. +- Commit after each gated milestone. Use a file-based commit message via + `git commit -F`, not `git commit -m`. +- Update `docs/developers-guide.md` whenever a reusable concern or pattern is + introduced or clarified by this work. + +## Tolerances (exception triggers) + +- Scope: if the implementation of a single milestone requires touching more + than 15 files or 500 net lines, stop, document the reason, and ask for + direction. +- Interface: if a public API signature must change, stop and ask for explicit + approval. +- Behaviour: if any behavioural test needs its expected behaviour changed + rather than preserved, stop and ask for explicit approval. +- Validation: if `make check-fmt`, `make lint`, or `make test` fails for a + reason unrelated to the current milestone and cannot be isolated within one + hour of investigation, stop and ask for direction. +- CodeRabbit: if CodeRabbit reports a concern that conflicts with a repository + requirement, document the conflict in `Decision Log` and ask for direction + before ignoring it. +- Dependency: if adding `googletest`, `pretty_assertions`, or `insta` causes + dependency or lint fallout beyond the touched tests, stop and ask whether to + continue broadening the dependency change. + +## Risks + +- Refactoring async send paths can subtly change hook, timing, span, or error + ordering. Mitigation: keep helpers thin, preserve call order exactly, and add + unit and BDD coverage around success and failure paths. +- Refactoring app outbound encoding can change frame wrapping, flush behaviour, + or error mapping. Mitigation: keep transport-specific send operations at the + edge and test raw stream, framed stream, serialization failure, and send + failure paths. +- Reworking `PooledClientLease` can affect recycle-on-error behaviour. + Mitigation: replace the macro with a helper that returns the same error + values, and add tests for successful dispatch and failed checkout/dispatch. +- Adding `wireframe_testing` explicitly to workspace members can broaden + workspace gates. Mitigation: run the full requested gates after the manifest + change and update `docs/developers-guide.md` to reflect the new semantics. +- Splitting test helpers can disrupt many integration tests through module path + churn. Mitigation: introduce a facade module that preserves current import + names, then move implementation into submodules behind that facade. + +## Progress + +- [x] 2026-06-05: Loaded `execplans`, `leta`, and `rust-router`. +- [x] 2026-06-05: Renamed the local branch to + `code-base-audit-2026-06-05`. +- [x] 2026-06-05: Drafted this ExecPlan. +- [x] 2026-06-05: Ran deterministic gates for the draft plan and applied two + trivial CodeRabbit punctuation fixes. +- [x] 2026-06-05: Resolved CodeRabbit's wording feedback by using Oxford + `-ize` spelling and documenting the explicit filename exception. +- [x] Push branch and create draft pull request for plan review. +- [x] Create GitHub follow-up issues for audit findings not implemented in this + plan. +- [x] Receive explicit approval to implement this plan. +- [x] Milestone 1: centralize client send pipeline logic. +- [x] 2026-06-05: Milestone 1 passed `make check-fmt`, `make lint`, + `make test`, targeted ExecPlan Markdown lint, and CodeRabbit review with zero + findings. +- [x] Milestone 2: centralize app outbound encoding and inbound pipeline + failure policy. +- [x] 2026-06-05: Milestone 2 passed `make check-fmt`, `make lint`, and + `make test`; CodeRabbit review findings have been fixed except repeated + `-ise` spelling requests that conflict with repository Oxford `-ize` style. +- [x] 2026-06-05: Milestone 2 rerun passed `make check-fmt`, `make lint`, + `make test`, targeted Markdown lint, and CodeRabbit review with only the + documented Oxford-spelling conflict findings remaining. +- [x] 2026-06-05: Milestone 2 final rerun passed `make check-fmt`, + `make lint`, `make test`, and targeted Markdown lint after the added + response-helper tests. +- [x] Milestone 3: route app builder transitions through one rebuild path. +- [x] 2026-06-05: Milestone 3 passed `make check-fmt`, `make lint`, + `make test`, targeted Markdown lint, and CodeRabbit review with zero findings. +- [x] Milestone 4: refactor pool lock recovery, builder parts construction, and + pooled lease dispatch. +- [x] 2026-06-05: Milestone 4 passed focused pool BDD coverage, + `make check-fmt`, `make lint`, `make test`, and Markdown lint before + CodeRabbit + review. +- [x] 2026-06-05: Fixed CodeRabbit's Milestone 4 request for direct + `lock_or_recover` tests, then reran `make check-fmt`, `make lint`, + `make test`, and Markdown lint successfully. +- [x] 2026-06-05: Fixed CodeRabbit's follow-up Milestone 4 requests for + `lock_or_recover` contract documentation, poison-recovery observability, and + stronger poison assertions, then reran `make check-fmt`, `make lint`, + `make test`, and Markdown lint successfully. +- [x] 2026-06-05: Resolved CodeRabbit's remaining Milestone 4 spelling finding + by changing the helper module comment to avoid the contested `Synchronization` + /`Synchronisation` word entirely, then reran `make check-fmt`, `make lint`, + `make test`, and Markdown lint successfully. +- [x] 2026-06-05: Fixed CodeRabbit's Milestone 4 encapsulation and metric + findings by narrowing `lock_or_recover` to `pub(super)` and adding + `wireframe_pool_bookkeeping_poison_recoveries_total`, then reran + `make check-fmt`, `make lint`, `make test`, and Markdown lint successfully. +- [x] 2026-06-05: Fixed CodeRabbit's Milestone 4 module-policy documentation + request and created issue #539 for broader scheduler/slot poison recovery + integration coverage, then reran `make check-fmt`, `make lint`, `make test`, + and Markdown lint successfully. +- [x] 2026-06-05: Fixed CodeRabbit's Milestone 4 observability assertion + request by extending the poison-recovery unit test to assert the warning text + and `wireframe_pool_bookkeeping_poison_recoveries_total`, then reran focused + pool-sync tests, `make check-fmt`, `make lint`, `make test`, and Markdown + lint successfully. +- [x] 2026-06-05: Milestone 4 CodeRabbit rerun passed with zero findings. +- [x] Milestone 5: update workspace membership and documentation. +- [x] 2026-06-05: Milestone 5 passed focused workspace-manifest integration + and BDD tests, `make check-fmt`, `make lint`, `make test`, and Markdown lint + before CodeRabbit review. +- [x] 2026-06-05: Milestone 5 CodeRabbit review passed with zero findings. +- [x] Milestone 6: extract example bootstrap and benchmark helper wiring. +- [x] 2026-06-05: Milestone 6 passed focused codec benchmark helper tests, + example compile checks, benchmark BDD tests, `make check-fmt`, `make lint`, + `make test`, and Markdown lint before CodeRabbit review. +- [x] 2026-06-05: Milestone 6 CodeRabbit review passed with zero findings. +- [x] Milestone 7: split fragment test helpers behind a compatibility facade. +- [x] 2026-06-05: Milestone 7 passed focused fragment transport, unified + codec, partial-frame BDD, memory-budget BDD, and budget-cleanup BDD tests, + then passed `make check-fmt`, `make lint`, `make test`, and Markdown lint. +- [x] 2026-06-05: Fixed CodeRabbit's Milestone 7 request to avoid the + 256-byte in-memory duplex buffer in `spawn_app`, then reran focused + fragment/unified tests, `make check-fmt`, `make lint`, and `make test`. +- [x] 2026-06-05: Fixed CodeRabbit's Milestone 7 request to name the + fragmentation-config message-limit multiplier, then reran focused + fragment/unified tests, `make check-fmt`, `make lint`, and `make test`. +- [x] 2026-06-05: Fixed CodeRabbit's Milestone 7 documentation spelling + request by avoiding contested terms in transport helper comments, then reran + `make check-fmt`, `make lint`, and `make test`. +- [x] 2026-06-05: Milestone 7 CodeRabbit rerun passed with zero findings. +- [x] 2026-06-05: Final gates, CodeRabbit review, push, and pull request + update completed. + +## Surprises & Discoveries + +- `cargo metadata` reports `wireframe_testing` as a workspace member even + though the root manifest currently lists only `.` and + `crates/wireframe-verification` in `[workspace].members`. The existing + developers' guide documents this as a nuance. This plan will replace that + nuance with explicit membership. +- `googletest`, `pretty_assertions`, and `insta` are not currently declared in + the root manifest. Milestone 2 added `googletest` and `pretty_assertions` as + dev-dependencies for the new focused response and outbound encoding tests. +- Global `make fmt` currently fails on unrelated pre-existing Markdown lint + violations outside this plan file. Targeted Markdown lint for this plan file + passes. +- `cargo fmt --workspace` is not supported by the installed Cargo formatter + wrapper. Use `cargo fmt --all` or the repository's `make check-fmt` target + for Rust formatting in this worktree. +- Moving inbound frame construction into `build_dispatchable_envelope` pushed + `src/app/inbound_handler.rs` over the 400-line module limit. Extracting the + public response-sending methods to `src/app/outbound_response.rs` brought the + inbound handler back to 326 lines and improved its separation of concerns. +- The local `gh issue comment 538` command cannot add the zero-copy migration + note because the GitHub integration reports + `Resource not accessible by integration`. ADR 005 and this ExecPlan now carry + the migration detail. + +## Decision Log + +- 2026-06-05: Use an approval gate before implementation. The user asked for + `execplans`, and the skill requires draft review before execution. The branch + and draft pull request may be prepared before approval, but code changes must + wait. +- 2026-06-05: Keep public APIs stable. The audit concerns are maintainability + issues, and there is no requirement to change user-visible contracts. +- 2026-06-05: Treat CodeRabbit as a milestone reviewer after deterministic + gates, not as a substitute for formatting, linting, or tests. +- 2026-06-05: Accept CodeRabbit's two draft-plan punctuation findings because + they improve readability without changing scope. +- 2026-06-05: Restore `centralize` in milestone labels because the repository + uses en-GB-oxendict spelling, which favours `-ize` forms. +- 2026-06-05: Decline CodeRabbit's filename-pattern finding because the user + explicitly required the plan path + `docs/execplans/code-base-audit-2026-06-05.md`. +- 2026-06-05: Implement the client send-pipeline extraction as a private + `WireframeClient::serialize_and_send` method in + `src/client/send_pipeline.rs`. The helper accepts a span-construction + callback taking `TracingConfig` and the final frame byte length, so callers + keep correlation-specific span data without borrowing the client twice. +- 2026-06-05: Introduce `src/app/outbound_encoding.rs` as the shared + serialization-to-codec-frame helper for app outbound paths. Keep raw-stream + and framed transport writes in `src/app/outbound_response.rs` and + `src/app/codec_driver.rs`, respectively. +- 2026-06-05: Split inbound dispatch preparation into + `WireframeApp::build_dispatchable_envelope`, with decode failures delegated to + `DeserFailureTracker`. Preserve the counter reset point after decode, + reassembly, and message assembly have all succeeded. +- 2026-06-05: Accept CodeRabbit's buffer-capacity concern in + `send_response`. Since `FrameCodec` has no generic encoded-overhead API and + `F::Frame` need not be cloneable for an encoding probe, remove the + payload-length preallocation rather than guessing overhead. +- 2026-06-05: Decline CodeRabbit's request to change `serialize` and + `serialization` comments to `serialise` and `serialisation` in + `src/app/outbound_response.rs`. The repository requires en-GB-oxendict, which + favours `-ize` forms, and the surrounding app comments consistently use + `serialize`/`serialization`. +- 2026-06-05: Accept CodeRabbit's bound-tightening finding for + `send_response_framed_with_codec`; the method only sends through a framed + sink, so `W: AsyncWrite + Unpin` is sufficient. +- 2026-06-05: Accept CodeRabbit's matching bound-tightening finding for + `send_response_framed`; the length-delimited framed response path also only + sends. +- 2026-06-05: Accept CodeRabbit's request to document the future zero-copy + serializer migration. Create tracking issue + and keep the current + `Bytes::from(Vec)` conversion until the public serializer contract can + change deliberately. +- 2026-06-05: Mirror the zero-copy migration TODO on the length-delimited + `send_response_framed` conversion so both app outbound conversion sites + reference issue #538. +- 2026-06-05: Accept CodeRabbit's request to document why + `send_response_framed` bypasses `encode_message_frame`; the + `LengthDelimitedCodec` path intentionally sends raw serialized messages and + lets `framed.send` perform length-prefix encoding. +- 2026-06-05: Accept CodeRabbit's request for a concrete zero-copy serializer + deferral note. Update ADR 005, add an in-code comment at + `encode_message_frame`, and keep issue #538 as the owner for changing the + public serializer output contract. The audit milestone deliberately retains + `Vec` output to preserve source compatibility and avoid turning a + refactor into an API migration. +- 2026-06-05: Decline repeated CodeRabbit requests to change + `serialization` comments to `serialisation` in `src/app/outbound_encoding.rs` + and `src/app/outbound_response.rs` for the same repository-style reason + already recorded above. +- 2026-06-05: Accept CodeRabbit's request for direct tests around + `encode_message_frame` and the public framed response send helpers. Add + lightweight test-only serializers, codecs, and writers to cover success, + serialization failure, codec encode failure, I/O failure, large payloads, and + codec wrapper variation. +- 2026-06-05: Document the app inbound and outbound helper boundaries in + `docs/developers-guide.md`, because milestone 2 introduces reusable internal + patterns that future app refactors should follow. +- 2026-06-05: Accept CodeRabbit's assertion-style cleanup in + `outbound_encoding` tests and replace the boolean matcher with a concise + googletest `err(pat!(...))` matcher. +- 2026-06-05: Partially accept CodeRabbit's broader public response-helper + coverage request. Existing tests already covered success, serialization, + write, framed-send, and large-payload paths; add the missing focused cases + for codec-encoder failure, flush failure, and raw serialized payload output + on the length-delimited framed path. +- 2026-06-05: Decline CodeRabbit's follow-up request to duplicate the public + response-helper integration coverage inside a new + `src/app/outbound_response.rs` `#[cfg(test)]` module. `tests/response.rs` + already exercises `send_response`, `send_response_framed_with_codec`, + `length_codec`, and `send_response_framed` through the public API, including + serialization, codec-encode, write, flush, framed-send, raw-payload, and + large-payload paths. The requested direct + `LengthDelimitedCodec::max_frame_length()` assertion is not available through + Tokio's public API, so the existing large-payload behavioural checks are the + observable contract. +- 2026-06-05: Implement the app connection-state type transition with + `WireframeApp::rebuild_with_connection_state`. The helper keeps field + movement centralised and clears teardown because a teardown hook registered + for old state `C` cannot be type-correct for new state `C2`. Document that + teardown must be registered after setup when both hooks are required. +- 2026-06-05: Start milestone 4 by extracting + `client::pool::sync::lock_or_recover`, adding + `WireframeClientBuilder::into_parts()`, and replacing `PooledClientLease`'s + dispatch macro with a closure-based async helper. Document the pool + synchronization and client-build-parts patterns in `docs/developers-guide.md`. +- 2026-06-05: Implement milestone 4 with a private pool synchronization helper + instead of duplicated poison recovery, route single-client and pooled-client + connection construction through `WireframeClientBuilder::into_parts()`, and + make pooled lease methods call `PooledClientLease::dispatch_on_connection`. + The closure helper uses `AsyncFnOnce` so the borrowed managed connection can + be awaited without boxing or weakening the recycle-on-error flow. Document + the lock, builder-parts, and lease-dispatch patterns in + `docs/developers-guide.md`. +- 2026-06-05: Accept CodeRabbit's Milestone 4 test request and add direct + `lock_or_recover` coverage for normal acquisition and poison recovery. Use + the googletest harness directly for these tests because `expect_that!` + requires a googletest context, while `verify_that!(...)?` inside an `rstest` + returning `Result` conflicts with the repository's strict + `panic_in_result_fn` lint when the test intentionally poisons a lock. +- 2026-06-05: Accept CodeRabbit's follow-up request to make + `lock_or_recover`'s recovery policy explicit and observable. The helper now + documents that it is only for pool bookkeeping state, warns through + `tracing::warn` before recovering, and the poison test proves the spawned + thread panicked and the mutex entered the poisoned state before recovery. +- 2026-06-05: Resolve CodeRabbit's Oxford-spelling conflict on the module-level + `Synchronization` wording by replacing the sentence with "Pool lock helper + shared across client pool internals." This preserves the repository's + documented Oxford `-ize` style without leaving a repeated review finding. +- 2026-06-05: Accept CodeRabbit's follow-up request to keep the pool poison + policy local to the pool module and observable through metrics. Change + `lock_or_recover` from `pub(crate)` to `pub(super)` and add + `metrics::inc_pool_bookkeeping_poison_recoveries()` backed by + `wireframe_pool_bookkeeping_poison_recoveries_total`. +- 2026-06-05: Accept CodeRabbit's request for module-level policy text around + `lock_or_recover`. Defer the deeper scheduler/slot poison recovery + integration test to issue #539 because it needs a deliberate way to simulate + panic-interrupted private scheduler or slot bookkeeping without weakening + production encapsulation. +- 2026-06-05: Accept CodeRabbit's request to assert `lock_or_recover` + observability side effects. The poison-recovery unit test now runs the + recovery call under `metrics::with_local_recorder`, captures warning output + with a scoped `tracing_subscriber` writer, and asserts both the warning text + and the `wireframe_pool_bookkeeping_poison_recoveries_total` increment. +- 2026-06-05: Start milestone 5 by adding `wireframe_testing` to explicit + workspace members. Update direct workspace-manifest tests, BDD fixture/steps, + feature text, scenario wrappers, and `docs/developers-guide.md` so the helper + crate is no longer described as a Cargo metadata nuance. +- 2026-06-05: Implement milestone 5 by adding `wireframe_testing` to + `[workspace].members`, asserting its package id in direct and BDD workspace + manifest tests, and updating command guidance in `docs/developers-guide.md`. + The root package remains the only default workspace member, so plain Cargo + commands retain their root-crate ergonomics. +- 2026-06-05: Start milestone 6 by extracting example TCP server runtime + bootstrap into `examples/support/runtime_bootstrap.rs` and moving codec + benchmark helpers into `wireframe_testing::codec_benchmarks`. This keeps + example-specific app construction local while replacing bench/test `#[path]` + coupling to `tests/common` benchmark helper files with a stable helper crate + module. +- 2026-06-05: Implement milestone 6 by routing `ping_pong` and `packet_enum` + through `examples/support/runtime_bootstrap.rs`, moving codec benchmark + helpers into `wireframe_testing::codec_benchmarks`, and updating benches, + direct tests, and BDD fixtures to import helpers from the helper crate. Add + `PayloadClass::is_empty()` because moving the helper into public + `wireframe_testing` API made `PayloadClass::len()` subject to + `len_without_is_empty`. +- 2026-06-05: Implement milestone 7 by keeping + `tests/common/fragment_helpers.rs` as the import facade and moving helper + bodies into `tests/common/fragment_helpers/` modules for app construction, + assertions, config, envelopes, errors, and transport. Because the facade is + loaded with `#[path]` from integration tests, its child modules need explicit + `#[path = "fragment_helpers/..."]` attributes. The `unified_codec` test has + a small helper that references the facade entries used by other test binaries + so the shared facade remains warning-clean under `-D warnings`. +- 2026-06-05: Accept CodeRabbit's Milestone 7 finding that the shared + `spawn_app` helper's 256-byte duplex buffer was too small for larger + fragmented traffic. Keep the call surface stable and replace the literal + with an 8 KiB named constant so existing tests get a safer default without + call-site churn. +- 2026-06-05: Accept CodeRabbit's follow-up Milestone 7 finding that + `fragmentation_config` should name the 16x message-limit multiplier. Extract + `MESSAGE_LIMIT_MULTIPLIER` in `tests/common/fragment_helpers/config.rs` and + leave the overflow and frame-budget error paths unchanged. +- 2026-06-05: Resolve CodeRabbit's Milestone 7 documentation spelling finding + by replacing `serialization` and `deserialization` in transport-helper + comments with `encoding` and `decoding`. This avoids another + Oxford-spelling conflict while keeping the comments precise. + +## Implementation Plan + +### Milestone 1: centralize client send pipeline logic + +Create an internal helper near the client runtime modules, likely in +`src/client/send_pipeline.rs` or an existing client helper module. The helper +serializes an `EncodeWith` value, invokes before-send hooks, sends the frame +through the framed transport, emits timing events, and invokes the error hook +on serialization or transport failure. It must allow callers to supply the span +or span-construction data so `send`, `send_envelope`, and `call_streaming` +retain their distinct tracing metadata. + +Update `src/client/runtime.rs`, `src/client/messaging.rs`, and +`src/client/streaming.rs` to use the helper. Keep correlation-specific logic in +the caller: `send_envelope` and `call_streaming` still decide whether to reuse +or generate a correlation identifier before sending. + +Add or update `rstest` unit tests for serialization failure, before-send hook +mutation, send failure, and successful send. Add or update `rstest-bdd` +coverage only where existing client messaging or streaming scenarios can +observe the refactor through public behaviour. + +After the milestone, run: + +```sh +make check-fmt 2>&1 | tee /tmp/check-fmt-wireframe-code-base-audit-2026-06-05.out +make lint 2>&1 | tee /tmp/lint-wireframe-code-base-audit-2026-06-05.out +make test 2>&1 | tee /tmp/test-wireframe-code-base-audit-2026-06-05.out +coderabbit review --agent +``` + +Commit the milestone if all gates and CodeRabbit concerns are clear. + +### Milestone 2: centralize app outbound encoding and inbound pipeline policy + +Extract app outbound encoding into a helper that serializes a message or +envelope and wraps it with the configured `FrameCodec`. Keep raw stream writing +and framed stream sending separate, so transport semantics remain explicit. +Update `src/app/inbound_handler.rs` and `src/app/codec_driver.rs` to use the +helper. + +Change `frame_handling::decode_envelope` so it accepts `DeserFailureTracker` +instead of manually incrementing and checking the counter. Then extract the +chained decode, reassemble, and assemble flow from `WireframeApp::handle_frame` +into an internal +`build_dispatchable_envelope(...) -> io::Result>` helper. The +helper must preserve the rule that deserialization failure count resets only +after the full inbound pipeline produces a dispatchable envelope. + +Add or update tests for outbound serialization success and failure, framed send +failure, raw stream send failure, recoverable decode failure, threshold decode +failure, and successful reset after a complete inbound pipeline. Add BDD +coverage only where existing inbound message assembly scenarios expose this +behaviour. + +Run the same three Makefile gates and `coderabbit review --agent`, then commit. + +### Milestone 3: route app builder transitions through one rebuild path + +Add a `WireframeApp` rebuild helper for changing the connection-context generic +parameter `C`, modelled on the existing `rebuild_with_params` helper. Refactor +`on_connection_setup` to use it instead of manually constructing `WireframeApp`. + +Preserve existing `on_disconnect` behaviour unless analysis proves it is +invalid. If teardown cannot be preserved safely with the current type model, +document the ordering consequence in rustdoc and in `docs/developers-guide.md`. + +Add tests that configure setup and teardown in both orders that currently +compile, then assert the resulting lifecycle behaviour. Use `rstest` for the +matrix and BDD only if an existing lifecycle feature already models the +observable setup/teardown workflow. + +Run the three Makefile gates and `coderabbit review --agent`, then commit. + +### Milestone 4: refactor pool internals + +Move duplicated poison-recovery locking from `src/client/pool/scheduler.rs` and +`src/client/pool/slot.rs` into a shared private pool sync module. Replace +`PooledClientLease`'s dispatch macro with a closure-based async helper that +keeps checkout, dispatch, recycle, and error mapping visible in ordinary Rust +control flow. Add `WireframeClientBuilder::into_parts()` and use it from both +connect paths. + +Add `rstest` coverage for lock recovery, builder-parts construction, lease +dispatch success, and lease dispatch error/recycle behaviour. Existing pool BDD +scenarios should continue to pass; add a new scenario only if the helper change +exposes an externally observable pool behaviour that is not already covered. + +Run the three Makefile gates and `coderabbit review --agent`, then commit. + +### Milestone 5: update workspace membership and documentation + +Add `wireframe_testing` explicitly to `Cargo.toml` `[workspace].members`. Update +`docs/developers-guide.md` so it no longer describes `wireframe_testing` as a +metadata nuance. The guide should explain when to use default-member commands, +`--workspace`, `-p wireframe-verification`, and `-p wireframe_testing`. + +Update or add workspace manifest tests and the existing +`workspace_manifest.feature` scenarios so they assert the explicit membership. +Use `pretty_assertions` and `googletest` assertions in any new Rust tests where +they clarify failure output. + +Run `make fmt` for Markdown formatting, then the three Makefile gates and +`coderabbit review --agent`, then commit. + +### Milestone 6: extract example bootstrap and benchmark helper wiring + +Extract the duplicated runtime bootstrap shape shared by +`examples/ping_pong.rs` and `examples/packet_enum.rs` into +`examples/support/runtime_bootstrap.rs`. Keep example-specific handler, +middleware, and app construction in each example. + +Replace bench `#[path]` coupling with a stable shared helper path. Prefer a +normal shared module or existing helper crate over new public production API. +If the cleanest solution is to move helper code into `wireframe_testing`, +document why the helper belongs in test infrastructure rather than production. + +Add tests that compile or exercise the helper paths already covered by +`make test`, and use BDD only if the externally observable example or benchmark +contract is represented in existing feature files. + +Run the three Makefile gates and `coderabbit review --agent`, then commit. + +### Milestone 7: split fragment test helpers + +Split `tests/common/fragment_helpers.rs` into smaller modules grouped by +responsibility, such as errors, config, transports, app/spawn helpers, and +assertions. Preserve the existing import surface through a facade module so +call-site churn stays limited. + +Run fragment transport, partial-frame, memory-budget, and full test gates to +confirm the split is mechanical. Add tests only if the split exposes an +untested helper contract; otherwise rely on existing integration and BDD +coverage because behaviour should not change. + +Run the three Makefile gates and `coderabbit review --agent`, then commit. + +## Follow-up GitHub issues + +Create separate GitHub issues for audit findings that are not implemented by +this plan or that should remain independent of this broad refactor: + +- Simplify pool waiter state transitions in `src/client/pool/scheduler.rs`. +- Avoid allocation in `ClientPoolInner::ordered_slots`. +- Enforce feature, scenario, and fixture naming consistency for BDD tests. + +If implementation reveals additional deferred concerns, add them to this list +and create issues before marking the plan complete. + +## Validation + +Every implementation milestone must run the following commands sequentially: + +```sh +make check-fmt 2>&1 | tee /tmp/check-fmt-wireframe-code-base-audit-2026-06-05.out +make lint 2>&1 | tee /tmp/lint-wireframe-code-base-audit-2026-06-05.out +make test 2>&1 | tee /tmp/test-wireframe-code-base-audit-2026-06-05.out +coderabbit review --agent +``` + +Documentation-only edits must additionally run: + +```sh +make fmt 2>&1 | tee /tmp/fmt-wireframe-code-base-audit-2026-06-05.out +make markdownlint 2>&1 | tee /tmp/markdownlint-wireframe-code-base-audit-2026-06-05.out +``` + +The final branch must have a clean `git status --short`, all gates passing, +CodeRabbit concerns cleared, and a pushed draft pull request. + +## Outcomes & Retrospective + +- 2026-06-05: Milestone 4 validation before CodeRabbit: + `cargo test --test bdd_pool --features "advanced-tests pool"` passed after + the lease dispatch helper was rewritten to use `AsyncFnOnce`; + `cargo fmt --all`, `make check-fmt`, `make lint`, `make test`, and + `make markdownlint` passed. Logs: + `/tmp/test-bdd-pool-wireframe-code-base-audit-2026-06-05-m4-rerun4.out`, + `/tmp/fmt-wireframe-code-base-audit-2026-06-05-m4.out`, + `/tmp/check-fmt-wireframe-code-base-audit-2026-06-05-m4.out`, + `/tmp/lint-wireframe-code-base-audit-2026-06-05-m4.out`, + `/tmp/test-wireframe-code-base-audit-2026-06-05-m4.out`, and + `/tmp/markdownlint-wireframe-code-base-audit-2026-06-05-m4.out`. +- 2026-06-05: CodeRabbit's first Milestone 4 review requested direct + `lock_or_recover` tests. After adding and adjusting them, validation passed: + `/tmp/test-pool-sync-wireframe-code-base-audit-2026-06-05-m4-rerun1.out`, + `/tmp/check-fmt-wireframe-code-base-audit-2026-06-05-m4-rerun4.out`, + `/tmp/lint-wireframe-code-base-audit-2026-06-05-m4-rerun4.out`, + `/tmp/test-wireframe-code-base-audit-2026-06-05-m4-rerun4.out`, and + `/tmp/markdownlint-wireframe-code-base-audit-2026-06-05-m4-rerun4.out`. +- 2026-06-05: CodeRabbit's second Milestone 4 review requested recovery + contract documentation, warning-level observability, singular module wording, + and stronger poison assertions. After applying those changes, validation + passed: `/tmp/check-fmt-wireframe-code-base-audit-2026-06-05-m4-rerun5.out`, + `/tmp/lint-wireframe-code-base-audit-2026-06-05-m4-rerun5.out`, + `/tmp/test-wireframe-code-base-audit-2026-06-05-m4-rerun5.out`, and + `/tmp/markdownlint-wireframe-code-base-audit-2026-06-05-m4-rerun6.out`. +- 2026-06-05: CodeRabbit's third Milestone 4 review requested `-ise` spelling + for `Synchronization`. The module comment now avoids that word. Validation + passed: `/tmp/check-fmt-wireframe-code-base-audit-2026-06-05-m4-rerun6.out`, + `/tmp/lint-wireframe-code-base-audit-2026-06-05-m4-rerun6.out`, + `/tmp/test-wireframe-code-base-audit-2026-06-05-m4-rerun6.out`, and + `/tmp/markdownlint-wireframe-code-base-audit-2026-06-05-m4-rerun8.out`. +- 2026-06-05: CodeRabbit's fourth Milestone 4 review requested tighter helper + visibility and a poison-recovery metric. After applying both changes and + documenting the metric in `docs/developers-guide.md`, validation passed: + `/tmp/check-fmt-wireframe-code-base-audit-2026-06-05-m4-rerun7.out`, + `/tmp/lint-wireframe-code-base-audit-2026-06-05-m4-rerun7.out`, + `/tmp/test-wireframe-code-base-audit-2026-06-05-m4-rerun7.out`, and + `/tmp/markdownlint-wireframe-code-base-audit-2026-06-05-m4-rerun10.out`. +- 2026-06-05: CodeRabbit's fifth Milestone 4 review requested module-level + recovery policy text and either deeper integration coverage or a tracked + issue. Added module policy text, referenced issue #539 from the unit test + rationale, and created . + Validation passed: + `/tmp/check-fmt-wireframe-code-base-audit-2026-06-05-m4-rerun8.out`, + `/tmp/lint-wireframe-code-base-audit-2026-06-05-m4-rerun8.out`, + `/tmp/test-wireframe-code-base-audit-2026-06-05-m4-rerun8.out`, and + `/tmp/markdownlint-wireframe-code-base-audit-2026-06-05-m4-rerun12.out`. +- 2026-06-05: CodeRabbit's sixth Milestone 4 review requested direct + observability assertions for poison recovery. Added log and metric checks to + the unit test. Validation passed: + `/tmp/test-pool-sync-wireframe-code-base-audit-2026-06-05-m4-rerun3.out`, + `/tmp/check-fmt-wireframe-code-base-audit-2026-06-05-m4-rerun9.out`, + `/tmp/lint-wireframe-code-base-audit-2026-06-05-m4-rerun9.out`, + `/tmp/test-wireframe-code-base-audit-2026-06-05-m4-rerun9.out`, and + `/tmp/markdownlint-wireframe-code-base-audit-2026-06-05-m4-rerun14.out`. +- 2026-06-05: Milestone 4 CodeRabbit review passed with zero findings: + `/tmp/coderabbit-wireframe-code-base-audit-2026-06-05-m4-rerun6.out`. +- 2026-06-05: Milestone 5 validation before CodeRabbit: + `cargo test --test workspace_manifest --all-features`, + `cargo test --test bdd --all-features workspace_manifest`, `make check-fmt`, + `make lint`, `make test`, and `make markdownlint` passed. Logs: + `/tmp/test-workspace-manifest-wireframe-code-base-audit-2026-06-05-m5.out`, + `/tmp/test-bdd-workspace-manifest-wireframe-code-base-audit-2026-06-05-m5.out`, + `/tmp/check-fmt-wireframe-code-base-audit-2026-06-05-m5.out`, + `/tmp/lint-wireframe-code-base-audit-2026-06-05-m5.out`, + `/tmp/test-wireframe-code-base-audit-2026-06-05-m5.out`, and + `/tmp/markdownlint-wireframe-code-base-audit-2026-06-05-m5.out`. +- 2026-06-05: Milestone 5 CodeRabbit review passed with zero findings: + `/tmp/coderabbit-wireframe-code-base-audit-2026-06-05-m5.out`. +- 2026-06-05: Milestone 6 validation before CodeRabbit: + `cargo test --test codec_performance_benchmark_helpers --all-features`, + `cargo check --examples --all-features`, + `cargo test --test bdd --all-features codec_performance_benchmarks`, + `make check-fmt`, `make lint`, `make test`, and `make markdownlint` passed. + Logs: + `/tmp/test-codec-benchmark-helpers-wireframe-code-base-audit-2026-06-05-m6.out`, + `/tmp/check-examples-wireframe-code-base-audit-2026-06-05-m6.out`, + `/tmp/test-bdd-codec-benchmarks-wireframe-code-base-audit-2026-06-05-m6.out`, + `/tmp/check-fmt-wireframe-code-base-audit-2026-06-05-m6-rerun1.out`, + `/tmp/lint-wireframe-code-base-audit-2026-06-05-m6-rerun1.out`, + `/tmp/test-wireframe-code-base-audit-2026-06-05-m6-rerun1.out`, and + `/tmp/markdownlint-wireframe-code-base-audit-2026-06-05-m6.out`. +- 2026-06-05: Milestone 6 CodeRabbit review passed with zero findings: + `/tmp/coderabbit-wireframe-code-base-audit-2026-06-05-m6.out`. +- 2026-06-05: Milestone 7 validation before CodeRabbit: + `cargo test --test fragment_transport --all-features`, + `cargo test --test unified_codec --all-features`, + `cargo test --test bdd --all-features partial_frame_feeding`, + `cargo test --test bdd --all-features memory_budget`, + `cargo test --test bdd --all-features budget_cleanup`, `make check-fmt`, + `make lint`, `make test`, and `make markdownlint` passed. Logs: + `/tmp/test-fragment-transport-wireframe-code-base-audit-2026-06-05-m7-rerun2.out`, + `/tmp/test-unified-codec-wireframe-code-base-audit-2026-06-05-m7-rerun2.out`, + `/tmp/test-bdd-partial-frame-wireframe-code-base-audit-2026-06-05-m7.out`, + `/tmp/test-bdd-memory-budget-wireframe-code-base-audit-2026-06-05-m7.out`, + `/tmp/test-bdd-budget-cleanup-wireframe-code-base-audit-2026-06-05-m7.out`, + `/tmp/check-fmt-wireframe-code-base-audit-2026-06-05-m7-rerun1.out`, + `/tmp/lint-wireframe-code-base-audit-2026-06-05-m7-rerun5.out`, + `/tmp/test-wireframe-code-base-audit-2026-06-05-m7.out`, and + `/tmp/markdownlint-wireframe-code-base-audit-2026-06-05-m7.out`. +- 2026-06-05: After fixing CodeRabbit's duplex-buffer finding, validation + passed again: `cargo test --test fragment_transport --all-features`, + `cargo test --test unified_codec --all-features`, `make check-fmt`, + `make lint`, and `make test`. Logs: + `/tmp/test-fragment-transport-wireframe-code-base-audit-2026-06-05-m7-rerun3.out`, + `/tmp/test-unified-codec-wireframe-code-base-audit-2026-06-05-m7-rerun3.out`, + `/tmp/check-fmt-wireframe-code-base-audit-2026-06-05-m7-rerun2.out`, + `/tmp/lint-wireframe-code-base-audit-2026-06-05-m7-rerun6.out`, and + `/tmp/test-wireframe-code-base-audit-2026-06-05-m7-rerun1.out`. +- 2026-06-05: After fixing CodeRabbit's message-limit multiplier finding, + validation passed again: `cargo test --test fragment_transport + --all-features`, `cargo test --test unified_codec --all-features`, + `make check-fmt`, `make lint`, and `make test`. Logs: + `/tmp/test-fragment-transport-wireframe-code-base-audit-2026-06-05-m7-rerun4.out`, + `/tmp/test-unified-codec-wireframe-code-base-audit-2026-06-05-m7-rerun4.out`, + `/tmp/check-fmt-wireframe-code-base-audit-2026-06-05-m7-rerun3.out`, + `/tmp/lint-wireframe-code-base-audit-2026-06-05-m7-rerun7.out`, and + `/tmp/test-wireframe-code-base-audit-2026-06-05-m7-rerun2.out`. +- 2026-06-05: After fixing CodeRabbit's transport-helper spelling finding, + validation passed again: `make check-fmt`, `make lint`, and `make test`. + Logs: + `/tmp/check-fmt-wireframe-code-base-audit-2026-06-05-m7-rerun4.out`, + `/tmp/lint-wireframe-code-base-audit-2026-06-05-m7-rerun8.out`, and + `/tmp/test-wireframe-code-base-audit-2026-06-05-m7-rerun3.out`. +- 2026-06-05: Milestone 7 CodeRabbit rerun passed with zero findings: + `/tmp/coderabbit-wireframe-code-base-audit-2026-06-05-m7-rerun3.out`. +- 2026-06-05: The final branch includes all seven implementation milestones, + all requested follow-up GitHub issues, successful final deterministic gates, + a zero-finding CodeRabbit review, and a pushed update to pull request #534. +- 2026-06-16: Follow-up review observed that `tests/response.rs` exceeded the + 400-line file-size constraint after Milestone 2. The response error-path + fixtures and tests were split into `tests/response/response_errors.rs`, + leaving `tests/response.rs` focused on successful framing, codec, and buffer + capacity behaviour. Both files are now below 400 lines. diff --git a/docs/v0-2-0-to-v0-3-0-migration-guide.md b/docs/v0-2-0-to-v0-3-0-migration-guide.md index 58435fcc..7a23f580 100644 --- a/docs/v0-2-0-to-v0-3-0-migration-guide.md +++ b/docs/v0-2-0-to-v0-3-0-migration-guide.md @@ -65,7 +65,7 @@ classDiagram +new(value: NonZeroUsize) BudgetBytes } - class AppFactory~Ser, Ctx, E, Codec~ { + class AppFactory { <> +call() R } @@ -115,7 +115,7 @@ classDiagram class Serializer~S~ { <> +serialize(message: EncodeWith~S~) Result~Vec_u8~~ - +deserialize(bytes: &[u8]) Result~DecodeWith~S~~ + +deserialize(bytes: ByteSlice) Result~DecodeWith~S~~ } class EncodeWith~S~ { @@ -125,7 +125,7 @@ classDiagram class DecodeWith~S~ { <> - +decode_with(serializer: S, bytes: &[u8]) Result~Self~ + +decode_with(serializer: S, bytes: ByteSlice) Result~Self~ } class SerdeSerializerBridge { @@ -539,9 +539,9 @@ classDiagram } class TestkitHelpers { - +drive_with_partial_frames(app: WireframeApp, frames: Vec~Bytes~) - +drive_with_fragments(app: WireframeApp, fragments: Vec~Bytes~) - +drive_with_fragment_frames(app: WireframeApp, frames: Vec~Bytes~) + +drive_with_partial_frames(app: WireframeApp, frames: BytesVec) + +drive_with_fragments(app: WireframeApp, fragments: BytesVec) + +drive_with_fragment_frames(app: WireframeApp, frames: BytesVec) +drive_with_slow_frames(app: WireframeApp, config: SlowIoConfig) +drive_with_slow_payloads(app: WireframeApp, config: SlowIoConfig) +assert_message_assembly_completed(snapshot: MessageAssemblySnapshot) @@ -562,11 +562,11 @@ classDiagram +clear() void +recorder() MetricsRecorder +snapshot() void - +codec_error_counter(kind: &str, action: &str) u64 + +codec_error_counter(kind: str, action: str) u64 } class Labels { - +with_label(key: &str, value: &str) Labels + +with_label(key: str, value: str) Labels } class MetricsRecorder { @@ -582,8 +582,8 @@ classDiagram +mismatched_total_size_wire(payload: Bytes) Bytes +truncated_hotline_header() Bytes +truncated_hotline_payload(payload_len: usize) Bytes - +correlated_hotline_wire(transaction_id: u32, payloads: Vec~Bytes~) Bytes - +sequential_hotline_wire(base_transaction_id: u32, payloads: Vec~Bytes~) Bytes + +correlated_hotline_wire(transaction_id: u32, payloads: BytesVec) Bytes + +sequential_hotline_wire(base_transaction_id: u32, payloads: BytesVec) Bytes +new_test_codec() HotlineFrameCodec } @@ -784,7 +784,7 @@ classDiagram class ResponseStream { <> - -client: &mut WireframeClient + -client: WireframeClient_ref_mut +try_next() Result~Option~Frame~~ } @@ -793,7 +793,7 @@ classDiagram +typed_with(mapper: Mapper) TypedResponseStream } - class TypedResponseStream~S, Mapper, P, Item~ { + class TypedResponseStream { <> -inner: S -mapper: Mapper diff --git a/examples/packet_enum.rs b/examples/packet_enum.rs index 700e6019..9cc8a3c5 100644 --- a/examples/packet_enum.rs +++ b/examples/packet_enum.rs @@ -6,8 +6,7 @@ use std::{collections::HashMap, future::Future, net::SocketAddr, pin::Pin, sync::Arc}; use async_trait::async_trait; -use tokio::net::{TcpListener, TcpStream}; -use tracing::{error, info, warn}; +use tracing::{info, warn}; use wireframe::{ app::Envelope, message::Message, @@ -15,6 +14,8 @@ use wireframe::{ serializer::BincodeSerializer, }; +#[path = "support/runtime_bootstrap.rs"] +mod runtime_bootstrap; #[path = "support/server_loop.rs"] mod server_loop; @@ -89,14 +90,6 @@ fn build_app() -> wireframe::app::Result { .route(1, Arc::new(handle_packet)) } -fn init_tracing() { let _ = tracing_subscriber::fmt::try_init(); } - -fn build_runtime_app() -> std::io::Result> { - build_app() - .map(Arc::new) - .map_err(|error| std::io::Error::other(error.to_string())) -} - fn parse_server_addr() -> std::io::Result { let addr_str = std::env::var("SERVER_ADDR").unwrap_or_else(|_| DEFAULT_ADDR.to_string()); addr_str.parse().map_err(|error| { @@ -107,37 +100,16 @@ fn parse_server_addr() -> std::io::Result { }) } -async fn bind_listener() -> std::io::Result { - let addr = parse_server_addr()?; - TcpListener::bind(addr).await -} - -fn spawn_connection(app: Arc, stream: TcpStream) { - tokio::spawn(async move { - if let Err(error) = app.handle_connection_result(stream).await { - error!("connection handling failed: {error}"); - } - }); -} - async fn run() -> std::io::Result<()> { - init_tracing(); - let app = build_runtime_app()?; - let listener = bind_listener().await?; - - while let Some(stream) = - server_loop::accept_until_shutdown(&listener, "packet_enum server received shutdown signal") - .await? - { - spawn_connection(Arc::clone(&app), stream); - } - - Ok(()) + runtime_bootstrap::init_tracing(); + let app = runtime_bootstrap::build_runtime_app(build_app)?; + let listener = runtime_bootstrap::bind_listener(parse_server_addr()?).await?; + runtime_bootstrap::serve_until_shutdown( + listener, + app, + "packet_enum server received shutdown signal", + ) + .await } -fn main() -> std::io::Result<()> { - let runtime = tokio::runtime::Builder::new_current_thread() - .enable_all() - .build()?; - runtime.block_on(run()) -} +fn main() -> std::io::Result<()> { runtime_bootstrap::run_current_thread(run()) } diff --git a/examples/ping_pong.rs b/examples/ping_pong.rs index a3ee4f05..f34ef348 100644 --- a/examples/ping_pong.rs +++ b/examples/ping_pong.rs @@ -6,7 +6,6 @@ use std::{net::SocketAddr, sync::Arc}; use async_trait::async_trait; -use tokio::net::{TcpListener, TcpStream}; use tracing::{error, info}; use wireframe::{ app::{Envelope, Packet, Result as AppResult}, @@ -15,6 +14,8 @@ use wireframe::{ serializer::BincodeSerializer, }; +#[path = "support/runtime_bootstrap.rs"] +mod runtime_bootstrap; #[path = "support/server_loop.rs"] mod server_loop; @@ -149,8 +150,6 @@ fn build_app() -> AppResult { const DEFAULT_ADDR: &str = "127.0.0.1:7878"; -fn init_tracing() { let _ = tracing_subscriber::fmt::try_init(); } - fn parse_server_addr() -> std::io::Result { let addr = std::env::args() .nth(1) @@ -158,46 +157,16 @@ fn parse_server_addr() -> std::io::Result { addr.parse().map_err(std::io::Error::other) } -fn build_runtime_app() -> std::io::Result> { - build_app() - .map(Arc::new) - .map_err(|error| std::io::Error::other(error.to_string())) -} - -async fn bind_listener() -> std::io::Result { - let addr = parse_server_addr()?; - TcpListener::bind(addr).await -} - -fn spawn_connection(app: Arc, stream: TcpStream) { - tokio::spawn(async move { - if let Err(error) = app.handle_connection_result(stream).await { - error!("connection handling failed: {error}"); - } - }); -} - -async fn serve_until_shutdown(listener: TcpListener, app: Arc) -> std::io::Result<()> { - while let Some(stream) = - server_loop::accept_until_shutdown(&listener, "ping-pong server received shutdown signal") - .await? - { - spawn_connection(Arc::clone(&app), stream); - } - - Ok(()) -} - async fn run() -> std::io::Result<()> { - init_tracing(); - let app = build_runtime_app()?; - let listener = bind_listener().await?; - serve_until_shutdown(listener, app).await + runtime_bootstrap::init_tracing(); + let app = runtime_bootstrap::build_runtime_app(build_app)?; + let listener = runtime_bootstrap::bind_listener(parse_server_addr()?).await?; + runtime_bootstrap::serve_until_shutdown( + listener, + app, + "ping-pong server received shutdown signal", + ) + .await } -fn main() -> std::io::Result<()> { - let runtime = tokio::runtime::Builder::new_current_thread() - .enable_all() - .build()?; - runtime.block_on(run()) -} +fn main() -> std::io::Result<()> { runtime_bootstrap::run_current_thread(run()) } diff --git a/examples/support/runtime_bootstrap.rs b/examples/support/runtime_bootstrap.rs new file mode 100644 index 00000000..ef5efb57 --- /dev/null +++ b/examples/support/runtime_bootstrap.rs @@ -0,0 +1,61 @@ +//! Runtime bootstrap shared by TCP server examples. + +use std::{future::Future, net::SocketAddr, sync::Arc}; + +use tokio::net::{TcpListener, TcpStream}; +use tracing::error; +use wireframe::{app::Envelope, serializer::BincodeSerializer}; + +use crate::server_loop; + +type ExampleApp = wireframe::app::WireframeApp; + +/// Initialize tracing for examples, ignoring duplicate global subscriber setup. +pub(crate) fn init_tracing() { let _ = tracing_subscriber::fmt::try_init(); } + +/// Convert an example app builder into a shared runtime app handle. +pub(crate) fn build_runtime_app( + build_app: impl FnOnce() -> wireframe::app::Result, +) -> std::io::Result> { + build_app() + .map(Arc::new) + .map_err(|error| std::io::Error::other(error.to_string())) +} + +/// Bind a TCP listener for an already parsed socket address. +pub(crate) async fn bind_listener(addr: SocketAddr) -> std::io::Result { + TcpListener::bind(addr).await +} + +/// Spawn one accepted TCP stream onto the shared app. +pub(crate) fn spawn_connection(app: Arc, stream: TcpStream) { + tokio::spawn(async move { + if let Err(error) = app.handle_connection_result(stream).await { + error!("connection handling failed: {error}"); + } + }); +} + +/// Accept connections until shutdown and dispatch each stream to the app. +pub(crate) async fn serve_until_shutdown( + listener: TcpListener, + app: Arc, + shutdown_message: &'static str, +) -> std::io::Result<()> { + while let Some(stream) = server_loop::accept_until_shutdown(&listener, shutdown_message).await? + { + spawn_connection(Arc::clone(&app), stream); + } + + Ok(()) +} + +/// Run an async example on a current-thread Tokio runtime. +pub(crate) fn run_current_thread( + future: impl Future>, +) -> std::io::Result<()> { + let runtime = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build()?; + runtime.block_on(future) +} diff --git a/src/app/builder/core.rs b/src/app/builder/core.rs index 1e0b5d0f..7ab2671a 100644 --- a/src/app/builder/core.rs +++ b/src/app/builder/core.rs @@ -170,6 +170,36 @@ where memory_budgets: params.memory_budgets, } } + + /// Rebuild the app after changing the connection state type parameter. + /// + /// A teardown hook receives the old `C` state and cannot be carried across + /// a transition to `C2`. Register `on_connection_teardown` after + /// `on_connection_setup` when both hooks are needed for the same state. + pub(super) fn rebuild_with_connection_state( + self, + on_connect: Option>>, + ) -> WireframeApp + where + C2: Send + 'static, + { + WireframeApp { + handlers: self.handlers, + routes: OnceCell::new(), + middleware: self.middleware, + serializer: self.serializer, + app_data: self.app_data, + on_connect, + on_disconnect: None, + protocol: self.protocol, + push_dlq: self.push_dlq, + codec: self.codec, + read_timeout_ms: self.read_timeout_ms, + fragmentation: self.fragmentation, + message_assembler: self.message_assembler, + memory_budgets: self.memory_budgets, + } + } } #[cfg(test)] diff --git a/src/app/builder/lifecycle.rs b/src/app/builder/lifecycle.rs index 0666aa3c..594df350 100644 --- a/src/app/builder/lifecycle.rs +++ b/src/app/builder/lifecycle.rs @@ -28,6 +28,10 @@ where /// This means that any subsequent builder methods will operate on the new connection state type /// `C2`. Be aware of this type transition when chaining builder methods. /// + /// Because teardown callbacks receive the connection state, this transition + /// clears any teardown callback registered for the previous `C` type. + /// Register teardown after setup when both hooks are needed. + /// /// # Errors /// /// This function always succeeds currently but uses [`Result`] for @@ -41,22 +45,7 @@ where Fut: Future + Send + 'static, C2: Send + 'static, { - Ok(WireframeApp { - handlers: self.handlers, - routes: tokio::sync::OnceCell::new(), - middleware: self.middleware, - serializer: self.serializer, - app_data: self.app_data, - on_connect: Some(Arc::new(move || Box::pin(f()))), - on_disconnect: None, - protocol: self.protocol, - push_dlq: self.push_dlq, - codec: self.codec, - read_timeout_ms: self.read_timeout_ms, - fragmentation: self.fragmentation, - message_assembler: self.message_assembler, - memory_budgets: self.memory_budgets, - }) + Ok(self.rebuild_with_connection_state(Some(Arc::new(move || Box::pin(f()))))) } /// Register a callback invoked when a connection is closed. diff --git a/src/app/codec_driver.rs b/src/app/codec_driver.rs index 32ba5bd2..89d35975 100644 --- a/src/app/codec_driver.rs +++ b/src/app/codec_driver.rs @@ -11,7 +11,6 @@ //! messages, streaming responses, and multi-packet channels — pass through //! the same fragmentation and metrics pipeline before reaching the wire. -use bytes::Bytes; use futures::SinkExt; use log::warn; use tokio::io::{self, AsyncRead, AsyncWrite}; @@ -21,6 +20,7 @@ use super::{ combined_codec::ConnectionCodec, envelope::Envelope, fragmentation_state::FragmentationState, + outbound_encoding::encode_message_frame, }; use crate::{ codec::FrameCodec, @@ -129,7 +129,7 @@ where F: FrameCodec, Envelope: EncodeWith, { - let bytes = serializer.serialize(envelope).map_err(|e| { + let encoded = encode_message_frame(serializer, codec, envelope).map_err(|e| { let id = envelope.id; let correlation_id = envelope.correlation_id; warn!( @@ -139,8 +139,7 @@ where crate::metrics::inc_handler_errors(); io::Error::other(e) })?; - let frame = codec.wrap_payload(Bytes::from(bytes)); - framed.send(frame).await.map_err(|e| { + framed.send(encoded.frame).await.map_err(|e| { let id = envelope.id; let correlation_id = envelope.correlation_id; warn!( diff --git a/src/app/frame_handling/core.rs b/src/app/frame_handling/core.rs index 6fb4b277..1086f118 100644 --- a/src/app/frame_handling/core.rs +++ b/src/app/frame_handling/core.rs @@ -13,13 +13,13 @@ use crate::{ }; /// Tracks deserialization failures and enforces a maximum error threshold. -pub(super) struct DeserFailureTracker<'a> { +pub(crate) struct DeserFailureTracker<'a> { count: &'a mut u32, limit: u32, } impl<'a> DeserFailureTracker<'a> { - pub(super) fn new(count: &'a mut u32, limit: u32) -> Self { Self { count, limit } } + pub(crate) fn new(count: &'a mut u32, limit: u32) -> Self { Self { count, limit } } pub(super) fn record( &mut self, @@ -31,6 +31,10 @@ impl<'a> DeserFailureTracker<'a> { warn!("{context}: correlation_id={correlation_id:?}, error={err:?}"); crate::metrics::inc_deser_errors(); if *self.count >= self.limit { + warn!( + "closing connection after {} deserialization failures: {context}", + self.count + ); return Err(io::Error::new( io::ErrorKind::InvalidData, "too many deserialization failures", diff --git a/src/app/frame_handling/decode.rs b/src/app/frame_handling/decode.rs index dfa5b541..c238d82b 100644 --- a/src/app/frame_handling/decode.rs +++ b/src/app/frame_handling/decode.rs @@ -2,14 +2,14 @@ use std::io; +use super::DeserFailureTracker; use crate::{app::Envelope, codec::FrameCodec}; /// Decode an envelope and apply connection-level deserialization failure policy. pub(crate) fn decode_envelope( parse_result: Result<(Envelope, usize), Box>, frame: &F::Frame, - deser_failures: &mut u32, - max_deser_failures: u32, + failures: &mut DeserFailureTracker<'_>, ) -> io::Result> where F: FrameCodec, @@ -22,20 +22,8 @@ where Ok(Some(env)) } Err(err) => { - *deser_failures += 1; let correlation_id = F::correlation_id(frame); - let context = "failed to decode message"; - log::warn!("{context}: correlation_id={correlation_id:?}, error={err:?}"); - crate::metrics::inc_deser_errors(); - if *deser_failures >= max_deser_failures { - log::warn!( - "closing connection after {deser_failures} deserialization failures: {context}" - ); - return Err(io::Error::new( - io::ErrorKind::InvalidData, - "too many deserialization failures", - )); - } + failures.record(correlation_id, "failed to decode message", err)?; Ok(None) } } diff --git a/src/app/frame_handling/mod.rs b/src/app/frame_handling/mod.rs index 40d4092f..bd3923f9 100644 --- a/src/app/frame_handling/mod.rs +++ b/src/app/frame_handling/mod.rs @@ -9,7 +9,7 @@ mod decode; mod reassembly; mod response; -pub(crate) use core::ResponseContext; +pub(crate) use core::{DeserFailureTracker, ResponseContext}; pub(crate) use assembly::{ AssemblyRuntime, diff --git a/src/app/inbound_handler.rs b/src/app/inbound_handler.rs index 87659336..74eb778f 100644 --- a/src/app/inbound_handler.rs +++ b/src/app/inbound_handler.rs @@ -2,25 +2,23 @@ use std::{collections::HashMap, sync::Arc}; -use bytes::{Bytes, BytesMut}; -use futures::{SinkExt, StreamExt}; +use futures::StreamExt; use log::{debug, warn}; use tokio::{ - io::{self, AsyncRead, AsyncWrite, AsyncWriteExt}, + io::{self, AsyncRead, AsyncWrite}, time::{Duration, timeout}, }; -use tokio_util::codec::{Encoder, Framed, LengthDelimitedCodec}; +use tokio_util::codec::Framed; use super::{ builder::WireframeApp, codec_driver::FramePipeline, combined_codec::{CombinedCodec, ConnectionCodec}, envelope::{Envelope, Packet}, - error::SendError, frame_handling, }; use crate::{ - codec::{FrameCodec, LengthDelimitedFrameCodec, MAX_FRAME_LENGTH, clamp_frame_length}, + codec::{FrameCodec, MAX_FRAME_LENGTH, clamp_frame_length}, frame::FrameMetadata, message::{DecodeWith, DeserializeContext, EncodeWith}, message_assembler::MessageAssemblyState, @@ -52,104 +50,15 @@ where message_assembly: &'a mut Option, } -impl WireframeApp +/// State needed to turn a raw frame into a dispatchable envelope. +struct DispatchBuildContext<'a, F> where - S: Serializer + Send + Sync, - C: Send + 'static, - E: Packet, F: FrameCodec, { - /// Serialize `msg` and write it to `stream` using the configured codec. - /// - /// # Errors - /// - /// Returns a [`SendError`] if serialization or writing fails. - pub async fn send_response( - &self, - stream: &mut W, - msg: &M, - ) -> std::result::Result<(), SendError> - where - W: AsyncWrite + Unpin, - M: EncodeWith, - { - let bytes = self - .serializer - .serialize(msg) - .map_err(SendError::Serialize)?; - let payload_len = bytes.len(); - let frame = self.codec.wrap_payload(Bytes::from(bytes)); - let mut encoder = self.codec.encoder(); - let mut encoded_buf = BytesMut::with_capacity(payload_len); - encoder - .encode(frame, &mut encoded_buf) - .map_err(SendError::Io)?; - stream - .write_all(&encoded_buf) - .await - .map_err(SendError::Io)?; - stream.flush().await.map_err(SendError::Io) - } - - /// Serialize `msg` and send it through an existing framed stream. - /// - /// # Errors - /// - /// Returns a [`SendError`] if serialization or sending fails. - pub async fn send_response_framed_with_codec( - &self, - framed: &mut Framed, - msg: &M, - ) -> std::result::Result<(), SendError> - where - W: AsyncRead + AsyncWrite + Unpin, - M: EncodeWith, - Cc: Encoder, - { - let bytes = self - .serializer - .serialize(msg) - .map_err(SendError::Serialize)?; - let frame = self.codec.wrap_payload(Bytes::from(bytes)); - framed.send(frame).await.map_err(SendError::Io) - } -} - -impl WireframeApp -where - S: Serializer + Send + Sync, - C: Send + 'static, - E: Packet, -{ - /// Construct a length-delimited codec capped by the application's buffer - /// capacity. - #[must_use] - pub fn length_codec(&self) -> LengthDelimitedCodec { - LengthDelimitedCodec::builder() - .max_frame_length(self.codec.max_frame_length()) - .new_codec() - } - - /// Serialize `msg` and send it through an existing framed stream. - /// - /// # Errors - /// - /// Returns a [`SendError`] if serialization or sending fails. - pub async fn send_response_framed( - &self, - framed: &mut Framed, - msg: &M, - ) -> std::result::Result<(), SendError> - where - W: AsyncRead + AsyncWrite + Unpin, - M: EncodeWith, - { - let bytes = self - .serializer - .serialize(msg) - .map_err(SendError::Serialize)?; - framed.send(bytes.into()).await.map_err(SendError::Io) - } + frame: &'a F::Frame, + pipeline: &'a mut FramePipeline, + message_assembly: &'a mut Option, + deser_failures: &'a mut u32, } impl WireframeApp @@ -334,39 +243,16 @@ where } = ctx; crate::metrics::inc_frames(crate::metrics::Direction::Inbound); - let Some(env) = frame_handling::decode_envelope::( - self.parse_envelope(F::frame_payload(frame)), + let Some(env) = self.build_dispatchable_envelope(DispatchBuildContext { frame, - deser_failures, - MAX_DESER_FAILURES, - )? - else { - return Ok(()); - }; - let Some(env) = frame_handling::reassemble_if_needed( pipeline, + message_assembly, deser_failures, - env, - MAX_DESER_FAILURES, - )? - else { - return Ok(()); - }; - let Some(env) = frame_handling::assemble_if_needed( - frame_handling::AssemblyRuntime::new(self.message_assembler.as_ref(), message_assembly), - deser_failures, - env, - MAX_DESER_FAILURES, - )? + })? else { return Ok(()); }; - // Reset failure counter only after the entire inbound pipeline - // (decode, reassemble, assemble) succeeds, so that assembly-stage - // failures accumulate towards the threshold. - *deser_failures = 0; - if let Some(service) = routes.get(&env.id) { frame_handling::forward_response( env, @@ -388,6 +274,52 @@ where Ok(()) } + + fn build_dispatchable_envelope( + &self, + ctx: DispatchBuildContext<'_, F>, + ) -> io::Result> { + let DispatchBuildContext { + frame, + pipeline, + message_assembly, + deser_failures, + } = ctx; + let mut failure_tracker = + frame_handling::DeserFailureTracker::new(deser_failures, MAX_DESER_FAILURES); + let Some(env) = frame_handling::decode_envelope::( + self.parse_envelope(F::frame_payload(frame)), + frame, + &mut failure_tracker, + )? + else { + return Ok(None); + }; + let Some(env) = frame_handling::reassemble_if_needed( + pipeline, + deser_failures, + env, + MAX_DESER_FAILURES, + )? + else { + return Ok(None); + }; + let Some(env) = frame_handling::assemble_if_needed( + frame_handling::AssemblyRuntime::new(self.message_assembler.as_ref(), message_assembly), + deser_failures, + env, + MAX_DESER_FAILURES, + )? + else { + return Ok(None); + }; + + // Reset failure counter only after the entire inbound pipeline + // (decode, reassemble, assemble) succeeds, so that assembly-stage + // failures accumulate towards the threshold. + *deser_failures = 0; + Ok(Some(env)) + } } #[cfg(test)] diff --git a/src/app/inbound_handler/tests.rs b/src/app/inbound_handler/tests.rs index 6221e6ce..e1982193 100644 --- a/src/app/inbound_handler/tests.rs +++ b/src/app/inbound_handler/tests.rs @@ -75,21 +75,23 @@ fn decode_envelope_tracks_failures_and_logs_correlation_id() { let mut deser_failures = 0_u32; for _ in 1..MAX_DESER_FAILURES { + let mut failure_tracker = + frame_handling::DeserFailureTracker::new(&mut deser_failures, MAX_DESER_FAILURES); let result = frame_handling::decode_envelope::( app.parse_envelope(BadCodec::frame_payload(&frame)), &frame, - &mut deser_failures, - MAX_DESER_FAILURES, + &mut failure_tracker, ); assert!(result.is_ok(), "expected recoverable decode failure"); assert!(result.expect("decode result").is_none()); } + let mut failure_tracker = + frame_handling::DeserFailureTracker::new(&mut deser_failures, MAX_DESER_FAILURES); let err = frame_handling::decode_envelope::( app.parse_envelope(BadCodec::frame_payload(&frame)), &frame, - &mut deser_failures, - MAX_DESER_FAILURES, + &mut failure_tracker, ) .expect_err("expected decode failure to close connection"); assert_eq!(err.kind(), io::ErrorKind::InvalidData); diff --git a/src/app/mod.rs b/src/app/mod.rs index 52c82009..46e5eb71 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -21,6 +21,8 @@ mod inbound_handler; mod lifecycle; mod memory_budgets; mod middleware_types; +mod outbound_encoding; +mod outbound_response; pub use builder::WireframeApp; pub use envelope::{Envelope, Packet, PacketParts}; diff --git a/src/app/outbound_encoding.rs b/src/app/outbound_encoding.rs new file mode 100644 index 00000000..1b2d707c --- /dev/null +++ b/src/app/outbound_encoding.rs @@ -0,0 +1,181 @@ +//! Shared outbound encoding helpers for application responses. +//! +//! The helpers in this module keep serialization and codec frame wrapping in +//! one place while leaving raw-stream and framed-transport writes at the edge. +//! +//! TODO: Track the zero-copy serializer output migration in +//! . The current serializer +//! contract returns `Vec`, so this module keeps the `Bytes::from` +//! conversion until that public contract can change deliberately. + +use bytes::Bytes; + +use super::SendError; +use crate::{codec::FrameCodec, message::EncodeWith, serializer::Serializer}; + +/// A serialized message wrapped into a transport codec frame. +#[derive(Debug)] +pub(crate) struct EncodedFrame { + pub(crate) frame: T, +} + +/// Serialize `msg` and wrap it with `codec`. +pub(crate) fn encode_message_frame( + serializer: &S, + codec: &F, + msg: &M, +) -> Result, SendError> +where + S: Serializer + Send + Sync, + F: FrameCodec, + M: EncodeWith, +{ + let bytes = serializer.serialize(msg).map_err(SendError::Serialize)?; + // Keep this bridge behaviour-preserving until issue #538 changes the + // public serializer contract to return a Bytes-native container. + let frame = codec.wrap_payload(Bytes::from(bytes)); + Ok(EncodedFrame { frame }) +} + +#[cfg(test)] +mod tests { + //! Tests for app outbound encoding helper behaviour. + + use std::{error::Error, io}; + + use bytes::{Bytes, BytesMut}; + use googletest::prelude::*; + use rstest::rstest; + use tokio_util::codec::{Decoder, Encoder}; + + use super::*; + use crate::{message::DecodeWith, serializer::Serializer}; + + #[derive(Clone, Debug)] + struct VisibleCodec { + tag: &'static str, + } + + #[derive(Debug, PartialEq)] + struct VisibleFrame { + tag: &'static str, + payload: Bytes, + } + + struct NoopDecoder; + + impl Decoder for NoopDecoder { + type Error = io::Error; + type Item = VisibleFrame; + + fn decode(&mut self, _src: &mut BytesMut) -> io::Result> { Ok(None) } + } + + struct NoopEncoder; + + impl Encoder for NoopEncoder { + type Error = io::Error; + + fn encode(&mut self, _item: VisibleFrame, _dst: &mut BytesMut) -> io::Result<()> { Ok(()) } + } + + impl FrameCodec for VisibleCodec { + type Decoder = NoopDecoder; + type Encoder = NoopEncoder; + type Frame = VisibleFrame; + + fn decoder(&self) -> Self::Decoder { NoopDecoder } + + fn encoder(&self) -> Self::Encoder { NoopEncoder } + + fn frame_payload(frame: &Self::Frame) -> &[u8] { frame.payload.as_ref() } + + fn frame_payload_bytes(frame: &Self::Frame) -> Bytes { frame.payload.clone() } + + fn wrap_payload(&self, payload: Bytes) -> Self::Frame { + VisibleFrame { + tag: self.tag, + payload, + } + } + + fn max_frame_length(&self) -> usize { 1024 } + } + + #[derive(Debug)] + struct TestMessage; + + #[derive(Clone, Copy)] + struct TestSerializer { + should_fail: bool, + } + + impl EncodeWith for TestMessage { + fn encode_with( + &self, + serializer: &TestSerializer, + ) -> Result, Box> { + if serializer.should_fail { + Err(Box::new(io::Error::other("serialize failed"))) + } else { + Ok(vec![1, 2, 3, 4]) + } + } + } + + impl DecodeWith for TestMessage { + fn decode_with( + _serializer: &TestSerializer, + _bytes: &[u8], + _context: &crate::message::DeserializeContext<'_>, + ) -> Result<(Self, usize), Box> { + Ok((Self, 0)) + } + } + + impl Serializer for TestSerializer { + fn serialize(&self, value: &M) -> Result, Box> + where + M: EncodeWith, + { + value.encode_with(self) + } + + fn deserialize(&self, _bytes: &[u8]) -> Result<(M, usize), Box> + where + M: DecodeWith, + { + Err(Box::new(io::Error::other("deserialize unused"))) + } + } + + #[rstest] + #[case("binary")] + #[case("text")] + fn encode_message_frame_wraps_serializer_output_with_codec(#[case] tag: &'static str) { + let encoded = encode_message_frame( + &TestSerializer { should_fail: false }, + &VisibleCodec { tag }, + &TestMessage, + ) + .expect("encoding should succeed"); + + assert_that!( + &encoded.frame, + eq(&VisibleFrame { + tag, + payload: Bytes::from_static(&[1, 2, 3, 4]), + }) + ); + } + + #[test] + fn encode_message_frame_propagates_serialization_failure() { + let result = encode_message_frame( + &TestSerializer { should_fail: true }, + &VisibleCodec { tag: "binary" }, + &TestMessage, + ); + assert_that!(result, err(pat!(SendError::Serialize(_)))); + } +} diff --git a/src/app/outbound_response.rs b/src/app/outbound_response.rs new file mode 100644 index 00000000..1ce0486b --- /dev/null +++ b/src/app/outbound_response.rs @@ -0,0 +1,119 @@ +//! Public response-sending helpers for `WireframeApp`. +//! +//! This module keeps outbound response serialization and write helpers out of +//! the inbound frame-dispatch path while preserving the public API. + +use bytes::BytesMut; +use futures::SinkExt; +use tokio::io::{self, AsyncWrite, AsyncWriteExt}; +use tokio_util::codec::{Encoder, Framed, LengthDelimitedCodec}; + +use super::{ + builder::WireframeApp, + envelope::Packet, + error::SendError, + outbound_encoding::encode_message_frame, +}; +use crate::{ + codec::{FrameCodec, LengthDelimitedFrameCodec}, + message::EncodeWith, + serializer::Serializer, +}; + +impl WireframeApp +where + S: Serializer + Send + Sync, + C: Send + 'static, + E: Packet, + F: FrameCodec, +{ + /// Serialize `msg` and write it to `stream` using the configured codec. + /// + /// # Errors + /// + /// Returns a [`SendError`] if serialization or writing fails. + pub async fn send_response( + &self, + stream: &mut W, + msg: &M, + ) -> std::result::Result<(), SendError> + where + W: AsyncWrite + Unpin, + M: EncodeWith, + { + let outbound_frame = encode_message_frame(&self.serializer, &self.codec, msg)?; + let mut encoder = self.codec.encoder(); + let mut encoded_buf = BytesMut::new(); + encoder + .encode(outbound_frame.frame, &mut encoded_buf) + .map_err(SendError::Io)?; + stream + .write_all(&encoded_buf) + .await + .map_err(SendError::Io)?; + stream.flush().await.map_err(SendError::Io) + } + + /// Serialize `msg` and send it through an existing framed stream. + /// + /// # Errors + /// + /// Returns a [`SendError`] if serialization or sending fails. + pub async fn send_response_framed_with_codec( + &self, + framed: &mut Framed, + msg: &M, + ) -> std::result::Result<(), SendError> + where + W: AsyncWrite + Unpin, + M: EncodeWith, + Cc: Encoder, + { + let encoded = encode_message_frame(&self.serializer, &self.codec, msg)?; + framed.send(encoded.frame).await.map_err(SendError::Io) + } +} + +impl WireframeApp +where + S: Serializer + Send + Sync, + C: Send + 'static, + E: Packet, +{ + /// Construct a length-delimited codec capped by the application's buffer + /// capacity. + #[must_use] + pub fn length_codec(&self) -> LengthDelimitedCodec { + LengthDelimitedCodec::builder() + .max_frame_length(self.codec.max_frame_length()) + .new_codec() + } + + /// Serialize `msg` and send it through an existing framed stream. + /// + /// # Errors + /// + /// Returns a [`SendError`] if serialization or sending fails. + pub async fn send_response_framed( + &self, + framed: &mut Framed, + msg: &M, + ) -> std::result::Result<(), SendError> + where + W: AsyncWrite + Unpin, + M: EncodeWith, + { + let bytes = self + .serializer + .serialize(msg) + .map_err(SendError::Serialize)?; + // `LengthDelimitedCodec` performs the length-prefix encoding in + // `framed.send`. Do not use `encode_message_frame` here: wrapping the + // payload with `self.codec` would add a second application frame around + // a path that intentionally sends the raw serialized message. + // TODO: Remove this `Vec` to `Bytes` conversion when the + // zero-copy serializer migration in + // https://github.com/leynos/wireframe/issues/538 is complete. + framed.send(bytes.into()).await.map_err(SendError::Io) + } +} diff --git a/src/client/builder/connect.rs b/src/client/builder/connect.rs index 83b81475..1dec4d8d 100644 --- a/src/client/builder/connect.rs +++ b/src/client/builder/connect.rs @@ -10,7 +10,6 @@ use crate::{ client::{ ClientError, WireframeClient, - connect_parts::ClientBuildParts, tracing_helpers::{connect_span, emit_timing_event}, }, rewind_stream::RewindStream, @@ -71,16 +70,6 @@ where self, addr: SocketAddr, ) -> Result, C>, ClientError> { - ClientBuildParts { - serializer: self.serializer, - codec_config: self.codec_config, - socket_options: self.socket_options, - preamble_config: self.preamble_config, - lifecycle_hooks: self.lifecycle_hooks, - request_hooks: self.request_hooks, - tracing_config: self.tracing_config, - } - .connect(addr) - .await + self.into_parts().connect(addr).await } } diff --git a/src/client/builder/core.rs b/src/client/builder/core.rs index c07bfca9..61dd8cfa 100644 --- a/src/client/builder/core.rs +++ b/src/client/builder/core.rs @@ -4,6 +4,7 @@ use crate::{ client::{ ClientCodecConfig, SocketOptions, + connect_parts::ClientBuildParts, hooks::{LifecycleHooks, RequestHooks}, preamble_exchange::PreambleConfig, tracing_config::TracingConfig, @@ -64,3 +65,18 @@ impl WireframeClientBuilder { impl Default for WireframeClientBuilder { fn default() -> Self { Self::new() } } + +impl WireframeClientBuilder { + /// Consume the builder into the shared physical-connection recipe. + pub(crate) fn into_parts(self) -> ClientBuildParts { + ClientBuildParts { + serializer: self.serializer, + codec_config: self.codec_config, + socket_options: self.socket_options, + preamble_config: self.preamble_config, + lifecycle_hooks: self.lifecycle_hooks, + request_hooks: self.request_hooks, + tracing_config: self.tracing_config, + } + } +} diff --git a/src/client/builder/pool.rs b/src/client/builder/pool.rs index 6369ba98..fad7c627 100644 --- a/src/client/builder/pool.rs +++ b/src/client/builder/pool.rs @@ -8,7 +8,6 @@ use super::WireframeClientBuilder; use crate::{ client::{ ClientError, - connect_parts::ClientBuildParts, pool::{ClientPoolConfig, WireframeClientPool}, }, serializer::Serializer, @@ -33,19 +32,6 @@ where addr: SocketAddr, pool_config: ClientPoolConfig, ) -> Result, ClientError> { - WireframeClientPool::connect( - addr, - pool_config, - ClientBuildParts { - serializer: self.serializer, - codec_config: self.codec_config, - socket_options: self.socket_options, - preamble_config: self.preamble_config, - lifecycle_hooks: self.lifecycle_hooks, - request_hooks: self.request_hooks, - tracing_config: self.tracing_config, - }, - ) - .await + WireframeClientPool::connect(addr, pool_config, self.into_parts()).await } } diff --git a/src/client/messaging.rs b/src/client/messaging.rs index 70da00a7..a572ffd2 100644 --- a/src/client/messaging.rs +++ b/src/client/messaging.rs @@ -5,8 +5,7 @@ use std::{sync::atomic::Ordering, time::Instant}; -use bytes::Bytes; -use futures::{SinkExt, StreamExt}; +use futures::StreamExt; use tracing::Instrument; use super::{ @@ -128,29 +127,10 @@ where } let timing_start = self.tracing_config.send_timing.then(Instant::now); - let mut bytes = match self.serializer.serialize(&envelope) { - Ok(bytes) => bytes, - Err(e) => { - let err = ClientError::Serialize(e); - emit_timing_event(timing_start); - self.invoke_error_hook(&err).await; - return Err(err); - } - }; - self.invoke_before_send_hooks(&mut bytes); - let span = send_envelope_span(&self.tracing_config, correlation_id, bytes.len()); - let send_result = async { - let result = self.framed.send(Bytes::from(bytes)).await; - emit_timing_event(timing_start); - result - } - .instrument(span) - .await; - if let Err(e) = send_result { - let err = ClientError::from(e); - self.invoke_error_hook(&err).await; - return Err(err); - } + self.serialize_and_send(&envelope, timing_start, |config, frame_bytes| { + send_envelope_span(config, correlation_id, frame_bytes) + }) + .await?; Ok(correlation_id) } diff --git a/src/client/mod.rs b/src/client/mod.rs index 1888c314..cdeae7b0 100644 --- a/src/client/mod.rs +++ b/src/client/mod.rs @@ -23,6 +23,7 @@ mod preamble_builder; mod preamble_exchange; mod response_stream; mod runtime; +mod send_pipeline; mod send_streaming; mod socket_option_methods; mod streaming; diff --git a/src/client/pool/lease.rs b/src/client/pool/lease.rs index 780e2a5b..da9a1848 100644 --- a/src/client/pool/lease.rs +++ b/src/client/pool/lease.rs @@ -8,7 +8,7 @@ use std::sync::Arc; use tokio::sync::OwnedSemaphorePermit; -use super::{client_pool::ClientPoolInner, slot::PoolSlot}; +use super::{client_pool::ClientPoolInner, managed::ManagedClientConnection, slot::PoolSlot}; use crate::{ app::Packet, client::ClientError, @@ -28,19 +28,6 @@ where release_inner: Option>>, } -macro_rules! dispatch_on_connection { - ($self:ident, | $conn:ident | $op:expr) => {{ - let mut $conn = $self.slot.checkout().await?; - let result = $op.await; - if let Err(err) = &result - && Self::should_recycle(err) - { - $conn.mark_broken(); - } - result - }}; -} - impl PooledClientLease where S: Serializer + Clone + Send + Sync + 'static, @@ -61,6 +48,20 @@ where fn should_recycle(err: &ClientError) -> bool { err.should_recycle_connection() } + async fn dispatch_on_connection( + &self, + operation: impl AsyncFnOnce(&mut ManagedClientConnection) -> Result, + ) -> Result { + let mut connection = self.slot.checkout().await?; + let result = operation(&mut connection).await; + if let Err(err) = &result + && Self::should_recycle(err) + { + connection.mark_broken(); + } + result + } + /// Send one message over the leased socket. /// /// # Errors @@ -68,7 +69,8 @@ where /// Returns [`ClientError`] when checkout, serialization, or transport I/O /// fails. pub async fn send>(&self, message: &M) -> Result<(), ClientError> { - dispatch_on_connection!(self, |conn| conn.send(message)) + self.dispatch_on_connection(async |conn| conn.send(message).await) + .await } /// Receive one message from the leased socket. @@ -77,7 +79,8 @@ where /// /// Returns [`ClientError`] when checkout, decode, or transport I/O fails. pub async fn receive>(&self) -> Result { - dispatch_on_connection!(self, |conn| conn.receive()) + self.dispatch_on_connection(async |conn| conn.receive().await) + .await } /// Send one request and await one response over the leased socket. @@ -91,7 +94,8 @@ where Req: EncodeWith, Resp: DecodeWith, { - dispatch_on_connection!(self, |conn| conn.call(request)) + self.dispatch_on_connection(async |conn| conn.call(request).await) + .await } /// Send one envelope and return the correlation ID used. @@ -104,7 +108,8 @@ where where M: Packet + EncodeWith, { - dispatch_on_connection!(self, |conn| conn.send_envelope(envelope)) + self.dispatch_on_connection(async |conn| conn.send_envelope(envelope).await) + .await } /// Receive one envelope from the leased socket. @@ -116,7 +121,8 @@ where where M: Packet + DecodeWith, { - dispatch_on_connection!(self, |conn| conn.receive_envelope()) + self.dispatch_on_connection(async |conn| conn.receive_envelope().await) + .await } /// Send one envelope and await its correlated response. @@ -129,7 +135,8 @@ where where M: Packet + EncodeWith + DecodeWith, { - dispatch_on_connection!(self, |conn| conn.call_correlated(request)) + self.dispatch_on_connection(async |conn| conn.call_correlated(request).await) + .await } } diff --git a/src/client/pool/mod.rs b/src/client/pool/mod.rs index c6213098..4ebd8cda 100644 --- a/src/client/pool/mod.rs +++ b/src/client/pool/mod.rs @@ -13,6 +13,7 @@ mod manager; mod policy; mod scheduler; mod slot; +mod sync; pub use client_pool::WireframeClientPool; pub use config::ClientPoolConfig; diff --git a/src/client/pool/scheduler.rs b/src/client/pool/scheduler.rs index 23a33f2d..08fcf1c5 100644 --- a/src/client/pool/scheduler.rs +++ b/src/client/pool/scheduler.rs @@ -5,23 +5,20 @@ use std::{ sync::{ Arc, Mutex, - MutexGuard, atomic::{AtomicBool, AtomicU64, Ordering}, }, }; use tokio::sync::oneshot; -use super::{client_pool::ClientPoolInner, lease::PooledClientLease, policy::PoolFairnessPolicy}; +use super::{ + client_pool::ClientPoolInner, + lease::PooledClientLease, + policy::PoolFairnessPolicy, + sync::lock_or_recover, +}; use crate::{client::ClientError, serializer::Serializer}; -fn recover_mutex(mutex: &Mutex) -> MutexGuard<'_, T> { - match mutex.lock() { - Ok(guard) => guard, - Err(poisoned) => poisoned.into_inner(), - } -} - type WaiterSender = oneshot::Sender, ClientError>>; struct SchedulerState @@ -165,12 +162,12 @@ where pub(crate) fn register_handle(&self) -> u64 { let handle_id = self.next_handle_id.fetch_add(1, Ordering::Relaxed); - recover_mutex(&self.state).register_handle(handle_id); + lock_or_recover(&self.state).register_handle(handle_id); handle_id } pub(crate) fn deregister_handle(&self, handle_id: u64) { - recover_mutex(&self.state).deregister_handle(handle_id); + lock_or_recover(&self.state).deregister_handle(handle_id); } pub(crate) async fn acquire_for_handle( @@ -183,7 +180,7 @@ where } let (sender, receiver) = oneshot::channel(); - if !recover_mutex(&self.state).enqueue_waiter(handle_id, sender, self.fairness_policy) { + if !lock_or_recover(&self.state).enqueue_waiter(handle_id, sender, self.fairness_policy) { tracing::warn!( handle_id, fairness_policy = ?self.fairness_policy, @@ -210,7 +207,7 @@ where } pub(crate) fn notify_shutdown(&self) { - let mut state = recover_mutex(&self.state); + let mut state = lock_or_recover(&self.state); while let Some(waiter) = state.take_next_waiter(self.fairness_policy) { let _ = waiter.send(Err(ClientError::disconnected())); } @@ -237,7 +234,7 @@ where } fn restart_if_waiters(&self) -> bool { - if !recover_mutex(&self.state).has_waiters() { + if !lock_or_recover(&self.state).has_waiters() { return false; } @@ -248,7 +245,8 @@ where fn take_next_waiter_or_stop(&self) -> Option> { loop { - if let Some(sender) = recover_mutex(&self.state).take_next_waiter(self.fairness_policy) + if let Some(sender) = + lock_or_recover(&self.state).take_next_waiter(self.fairness_policy) { return Some(sender); } diff --git a/src/client/pool/slot.rs b/src/client/pool/slot.rs index 39500035..a1ad404f 100644 --- a/src/client/pool/slot.rs +++ b/src/client/pool/slot.rs @@ -12,16 +12,9 @@ use tokio::{ time::Instant, }; -use super::manager::WireframeConnectionManager; +use super::{manager::WireframeConnectionManager, sync::lock_or_recover}; use crate::{client::ClientError, serializer::Serializer}; -fn recover_mutex(mutex: &Mutex) -> MutexGuard<'_, T> { - match mutex.lock() { - Ok(guard) => guard, - Err(poisoned) => poisoned.into_inner(), - } -} - /// One physical socket slot backed by a `bb8` pool of size one. pub(crate) struct PoolSlot where @@ -105,7 +98,7 @@ where fn clear_last_returned_at(&self) { *self.lock_last_returned_at() = None; } fn lock_last_returned_at(&self) -> MutexGuard<'_, Option> { - recover_mutex(&self.last_returned_at) + lock_or_recover(&self.last_returned_at) } } @@ -146,7 +139,7 @@ where C: Send + 'static, { fn drop(&mut self) { - let mut last_returned_at = recover_mutex(self.last_returned_at); + let mut last_returned_at = lock_or_recover(self.last_returned_at); if self.connection.is_broken() { *last_returned_at = None; diff --git a/src/client/pool/sync.rs b/src/client/pool/sync.rs new file mode 100644 index 00000000..3608bd6b --- /dev/null +++ b/src/client/pool/sync.rs @@ -0,0 +1,135 @@ +//! Pool lock helper shared across client pool internals. +//! +//! [`lock_or_recover`] recovers poisoned mutexes for pool bookkeeping instead +//! of propagating poison. Use it only where a subsequent scheduler or slot +//! operation re-establishes consistency; see the function docs for detailed +//! constraints. + +use std::sync::{Mutex, MutexGuard}; + +use tracing::warn; + +/// Lock pool bookkeeping state, recovering the inner value after poison. +/// +/// # When to use +/// +/// Use this helper only for pool-internal bookkeeping whose consistency is +/// re-established by the next scheduler or slot operation. Current callers are +/// the pool scheduler waiter table and slot-return timestamp state. +/// +/// Do not use this helper for connection state, protocol state, serializer +/// state, lifecycle hook state, or user-provided state. Those locks must +/// propagate poison so callers cannot continue after a panic-interrupted +/// mutation. +/// +/// # Observability +/// +/// Poison recovery is logged at warning level before the guarded value is +/// returned, so unexpected panic recovery remains visible to operators. +pub(super) fn lock_or_recover(mutex: &Mutex) -> MutexGuard<'_, T> { + match mutex.lock() { + Ok(guard) => guard, + Err(poisoned) => { + crate::metrics::inc_pool_bookkeeping_poison_recoveries(); + warn!("recovering poisoned client pool bookkeeping lock"); + poisoned.into_inner() + } + } +} + +#[cfg(test)] +mod tests { + //! Tests for pool lock poison recovery. + + use std::{ + io, + sync::{Arc, Mutex}, + thread, + }; + + use googletest::{gtest, prelude::*}; + use tracing::Level; + use tracing_subscriber::fmt::MakeWriter; + use wireframe_testing::ObservabilityHandle; + + use super::lock_or_recover; + + #[gtest] + fn lock_or_recover_reads_unpoisoned_mutex() { + let mutex = Mutex::new(42); + + let guard = lock_or_recover(&mutex); + + expect_that!(*guard, eq(42)); + } + + #[gtest] + fn lock_or_recover_reads_poisoned_mutex() { + // This covers the local recovery primitive. Issue #539 tracks the + // broader scheduler/slot integration case where a later pool operation + // must prove bookkeeping consistency is re-established after poison. + let mutex = Arc::new(Mutex::new(42)); + let poisoned_mutex = Arc::clone(&mutex); + let join_result = thread::spawn(move || { + let _guard = lock_or_recover(&poisoned_mutex); + panic!("poison pool lock for recovery coverage"); + }) + .join(); + + expect_that!(join_result, err(anything())); + expect_that!(mutex.is_poisoned(), eq(true)); + + let captured_logs = Arc::new(Mutex::new(Vec::new())); + let subscriber = tracing_subscriber::fmt() + .with_ansi(false) + .without_time() + .with_max_level(Level::WARN) + .with_writer(CaptureWriter::new(Arc::clone(&captured_logs))) + .finish(); + let mut observability = ObservabilityHandle::new(); + metrics::with_local_recorder(observability.recorder(), || { + tracing::subscriber::with_default(subscriber, || { + let guard = lock_or_recover(&mutex); + + expect_that!(*guard, eq(42)); + }); + }); + observability.snapshot(); + let log_bytes = lock_or_recover(&captured_logs).clone(); + let logs = String::from_utf8_lossy(&log_bytes); + + expect_that!( + logs.as_ref(), + contains_substring("recovering poisoned client pool bookkeeping lock") + ); + expect_that!( + observability + .counter_without_labels(crate::metrics::POOL_BOOKKEEPING_POISON_RECOVERIES), + eq(1) + ); + } + + #[derive(Clone)] + struct CaptureWriter { + captured: Arc>>, + } + + impl CaptureWriter { + fn new(captured: Arc>>) -> Self { Self { captured } } + } + + impl<'a> MakeWriter<'a> for CaptureWriter { + type Writer = Self; + + fn make_writer(&'a self) -> Self::Writer { self.clone() } + } + + impl io::Write for CaptureWriter { + fn write(&mut self, buf: &[u8]) -> io::Result { + lock_or_recover(&self.captured).extend_from_slice(buf); + Ok(buf.len()) + } + + fn flush(&mut self) -> io::Result<()> { Ok(()) } + } +} diff --git a/src/client/runtime.rs b/src/client/runtime.rs index 8509e5a0..83cd87dc 100644 --- a/src/client/runtime.rs +++ b/src/client/runtime.rs @@ -2,7 +2,6 @@ use std::{fmt, sync::atomic::AtomicU64, time::Instant}; -use bytes::Bytes; use futures::SinkExt; use tokio::{ io::{AsyncRead, AsyncWrite}, @@ -145,30 +144,8 @@ where /// ``` pub async fn send>(&mut self, message: &M) -> Result<(), ClientError> { let timing_start = self.tracing_config.send_timing.then(Instant::now); - let mut bytes = match self.serializer.serialize(message) { - Ok(bytes) => bytes, - Err(e) => { - let err = ClientError::Serialize(e); - emit_timing_event(timing_start); - self.invoke_error_hook(&err).await; - return Err(err); - } - }; - self.invoke_before_send_hooks(&mut bytes); - let span = send_span(&self.tracing_config, bytes.len()); - let send_result = async { - let result = self.framed.send(Bytes::from(bytes)).await; - emit_timing_event(timing_start); - result - } - .instrument(span) - .await; - if let Err(e) = send_result { - let err = ClientError::from(e); - self.invoke_error_hook(&err).await; - return Err(err); - } - Ok(()) + self.serialize_and_send(message, timing_start, send_span) + .await } /// Receive the next message from the peer. diff --git a/src/client/send_pipeline.rs b/src/client/send_pipeline.rs new file mode 100644 index 00000000..2cbc43f0 --- /dev/null +++ b/src/client/send_pipeline.rs @@ -0,0 +1,59 @@ +//! Shared client send pipeline helpers. +//! +//! This module centralizes the common serialize, request-hook, framed-send, +//! timing, and error-hook flow used by the client's send APIs. + +use std::time::Instant; + +use bytes::Bytes; +use futures::SinkExt; +use tracing::{Instrument, Span}; + +use super::{ClientError, WireframeClient, runtime::ClientStream, tracing_config::TracingConfig}; +use crate::{message::EncodeWith, serializer::Serializer}; + +impl WireframeClient +where + S: Serializer + Send + Sync, + T: ClientStream, +{ + /// Serialize a value, run outbound hooks, and send the resulting frame. + pub(crate) async fn serialize_and_send( + &mut self, + value: &M, + timing_start: Option, + span_for_frame: F, + ) -> Result<(), ClientError> + where + M: EncodeWith, + F: FnOnce(&TracingConfig, usize) -> Span, + { + let mut bytes = match self.serializer.serialize(value) { + Ok(bytes) => bytes, + Err(e) => { + let err = ClientError::Serialize(e); + super::tracing_helpers::emit_timing_event(timing_start); + self.invoke_error_hook(&err).await; + return Err(err); + } + }; + + self.invoke_before_send_hooks(&mut bytes); + let span = span_for_frame(&self.tracing_config, bytes.len()); + let send_result = async { + let result = self.framed.send(Bytes::from(bytes)).await; + super::tracing_helpers::emit_timing_event(timing_start); + result + } + .instrument(span) + .await; + + if let Err(e) = send_result { + let err = ClientError::from(e); + self.invoke_error_hook(&err).await; + return Err(err); + } + + Ok(()) + } +} diff --git a/src/client/streaming.rs b/src/client/streaming.rs index 66e74db5..c0e57fa2 100644 --- a/src/client/streaming.rs +++ b/src/client/streaming.rs @@ -7,16 +7,7 @@ use std::{sync::atomic::Ordering, time::Instant}; -use bytes::Bytes; -use futures::SinkExt; -use tracing::Instrument; - -use super::{ - ClientError, - ResponseStream, - runtime::ClientStream, - tracing_helpers::{emit_timing_event, streaming_span}, -}; +use super::{ClientError, ResponseStream, runtime::ClientStream, tracing_helpers::streaming_span}; use crate::{ app::Packet, message::{DecodeWith, EncodeWith}, @@ -91,32 +82,14 @@ where request.set_correlation_id(Some(correlation_id)); } - let span = streaming_span(&self.tracing_config, correlation_id); let timing_start = self.tracing_config.streaming_timing.then(Instant::now); - let mut bytes = match self.serializer.serialize(&request) { - Ok(bytes) => bytes, - Err(e) => { - let err = ClientError::Serialize(e); - emit_timing_event(timing_start); - self.invoke_error_hook(&err).await; - return Err(err); - } - }; - self.invoke_before_send_hooks(&mut bytes); - span.record("frame.bytes", bytes.len()); - let send_result = async { - let result = self.framed.send(Bytes::from(bytes)).await; - emit_timing_event(timing_start); - result - } - .instrument(span) - .await; - if let Err(e) = send_result { - let err = ClientError::from(e); - self.invoke_error_hook(&err).await; - return Err(err); - } + self.serialize_and_send(&request, timing_start, |config, frame_bytes| { + let span = streaming_span(config, correlation_id); + span.record("frame.bytes", frame_bytes); + span + }) + .await?; Ok(ResponseStream::new(self, correlation_id)) } diff --git a/src/metrics.rs b/src/metrics.rs index 867c9b76..cd22ca4f 100644 --- a/src/metrics.rs +++ b/src/metrics.rs @@ -32,6 +32,15 @@ pub const ERRORS_TOTAL: &str = "wireframe_errors_total"; /// wireframe_connection_panics_total 1 /// ``` pub const CONNECTION_PANICS: &str = "wireframe_connection_panics_total"; +/// Name of the counter tracking recovered client pool bookkeeping lock poison. +/// +/// ```plaintext +/// # HELP wireframe_pool_bookkeeping_poison_recoveries_total Count of recovered client pool bookkeeping locks. +/// # TYPE wireframe_pool_bookkeeping_poison_recoveries_total counter +/// wireframe_pool_bookkeeping_poison_recoveries_total 1 +/// ``` +pub const POOL_BOOKKEEPING_POISON_RECOVERIES: &str = + "wireframe_pool_bookkeeping_poison_recoveries_total"; /// Name of the counter tracking codec errors by type and recovery policy. /// @@ -120,6 +129,15 @@ pub fn inc_connection_panics() { counter!(CONNECTION_PANICS).increment(1); } #[cfg(not(feature = "metrics"))] pub fn inc_connection_panics() {} +/// Record a recovered client pool bookkeeping lock poison event. +#[cfg(feature = "metrics")] +pub fn inc_pool_bookkeeping_poison_recoveries() { + counter!(POOL_BOOKKEEPING_POISON_RECOVERIES).increment(1); +} + +#[cfg(not(feature = "metrics"))] +pub fn inc_pool_bookkeeping_poison_recoveries() {} + /// Record a codec error with its type and recovery policy. /// /// # Arguments diff --git a/tests/codec_performance_benchmark_helpers.rs b/tests/codec_performance_benchmark_helpers.rs index de29d4b9..06b0e07d 100644 --- a/tests/codec_performance_benchmark_helpers.rs +++ b/tests/codec_performance_benchmark_helpers.rs @@ -4,33 +4,22 @@ #![cfg(not(loom))] use rstest::rstest; - -#[path = "common/codec_benchmark_support.rs"] -mod codec_benchmark_support; - -#[path = "common/codec_fragmentation_benchmark_support.rs"] -mod codec_fragmentation_benchmark_support; - -#[path = "common/codec_alloc_benchmark_support.rs"] -mod codec_alloc_benchmark_support; - -use codec_alloc_benchmark_support::{AllocationBaseline, allocation_label}; -use codec_benchmark_support::{ +use wireframe_testing::codec_benchmarks::{ + AllocationBaseline, BenchmarkWorkload, CodecUnderTest, + FRAGMENT_PAYLOAD_CAP_BYTES, LARGE_PAYLOAD_BYTES, + MeasurementExt as _, PayloadClass, SMALL_PAYLOAD_BYTES, VALIDATION_ITERATIONS, + allocation_label, benchmark_workloads, measure_decode, measure_encode, - payload_for_class, -}; -use codec_fragmentation_benchmark_support::{ - FRAGMENT_PAYLOAD_CAP_BYTES, - MeasurementExt as _, measure_fragmentation_overhead, + payload_for_class, }; #[rstest] diff --git a/tests/common/fragment_helpers.rs b/tests/common/fragment_helpers.rs index ea576a94..d1730ea6 100644 --- a/tests/common/fragment_helpers.rs +++ b/tests/common/fragment_helpers.rs @@ -1,305 +1,30 @@ //! Shared helpers for fragment transport integration tests. //! -//! Provides configuration builders, fragment encoding/decoding utilities, -//! and test application factories used across fragmentation test modules. - -use std::{io, num::NonZeroUsize, time::Duration}; - -use futures::{SinkExt, StreamExt}; -use thiserror::Error; -use tokio::{ - sync::mpsc, - time::{Duration as TokioDuration, timeout}, -}; -use tokio_util::codec::{Framed, LengthDelimitedCodec}; -use wireframe::{ - app::{Envelope, Handler, Packet, WireframeApp}, - fragment::{ - FragmentationConfig, - Fragmenter, - Reassembler, - ReassemblyError, - decode_fragment_payload, - encode_fragment_payload, - }, - serializer::{BincodeSerializer, Serializer}, -}; -pub type TestResult = Result; - -/// Error type for fragment transport tests. -#[derive(Debug, Error)] -pub enum TestError { - /// Test setup failed. - #[error("test setup failed: {0}")] - Setup(&'static str), - /// Fragmentation operation failed. - #[error("fragmentation failed: {0}")] - Fragmentation(#[from] wireframe::fragment::FragmentationError), - /// Encoding operation failed. - #[error("encoding failed: {0}")] - Encode(#[from] bincode::error::EncodeError), - /// Decoding operation failed. - #[error("decoding failed: {0}")] - Decode(#[from] bincode::error::DecodeError), - /// Reassembly operation failed. - #[error("reassembly failed: {0}")] - Reassembly(#[from] ReassemblyError), - /// Send operation failed. - #[error("send failed: {0}")] - Send(String), - /// Application error. - #[error("application error: {0}")] - App(wireframe::WireframeError), - /// Other error. - #[error(transparent)] - Other(#[from] Box), - /// Test assertion failed. - #[error("assertion failed: {0}")] - Assertion(String), - /// IO operation failed. - #[error("io failed: {0}")] - Io(#[from] std::io::Error), - /// Operation timed out. - #[error("timeout: {0}")] - Timeout(#[from] tokio::time::error::Elapsed), - /// Task join failed. - #[error("task join failed: {0}")] - Join(#[from] tokio::task::JoinError), -} - -impl From> for TestError { - fn from(err: mpsc::error::SendError) -> Self { TestError::Send(err.to_string()) } -} - -impl From for TestError { - fn from(err: wireframe::WireframeError) -> Self { TestError::App(err) } -} +//! Provides a stable facade over focused helper modules for configuration, +//! fragment envelope construction, framed transport, test app construction, +//! and assertions. + +#[path = "fragment_helpers/app.rs"] +mod app; +#[path = "fragment_helpers/assertions.rs"] +mod assertions; +#[path = "fragment_helpers/config.rs"] +mod config; +#[path = "fragment_helpers/envelopes.rs"] +mod envelopes; +#[path = "fragment_helpers/errors.rs"] +mod errors; +#[path = "fragment_helpers/transport.rs"] +mod transport; + +pub use app::{make_app, make_handler, spawn_app}; +pub use assertions::assert_handler_observed; +pub use config::{fragmentation_config, fragmentation_config_with_timeout}; +pub use envelopes::{build_envelopes, fragment_envelope}; +pub use errors::{TestError, TestResult}; +pub use transport::{read_reassembled_response, read_response_payload, send_envelopes}; /// Default route ID used in fragmentation tests. pub const ROUTE_ID: u32 = 42; /// Default correlation ID used in fragmentation tests. pub const CORRELATION: Option = Some(7); - -/// Create a fragmentation config for a given buffer capacity. -/// -/// Uses a message limit of 16x capacity and a 30ms reassembly timeout. -/// -/// # Errors -/// -/// Returns an error if the message limit overflows or if the frame budget -/// is too small to accommodate fragment overhead. -pub fn fragmentation_config(capacity: usize) -> TestResult { - let message_limit = capacity - .checked_mul(16) - .and_then(NonZeroUsize::new) - .ok_or(TestError::Setup("message limit overflow or zero"))?; - - let config = - FragmentationConfig::for_frame_budget(capacity, message_limit, Duration::from_millis(30)) - .ok_or(TestError::Setup( - "frame budget must exceed fragment overhead", - ))?; - - Ok(config) -} - -/// Create a fragmentation config with a custom reassembly timeout. -/// -/// # Errors -/// -/// Returns an error if the underlying [`fragmentation_config`] fails. -pub fn fragmentation_config_with_timeout( - capacity: usize, - timeout_ms: u64, -) -> TestResult { - let mut config = fragmentation_config(capacity)?; - config.reassembly_timeout = Duration::from_millis(timeout_ms); - Ok(config) -} - -/// Fragment an envelope into multiple fragment envelopes. -/// -/// Returns the original envelope wrapped in a vec if the payload fits in a -/// single fragment, otherwise returns the fragmented envelopes. -/// -/// # Errors -/// -/// Returns an error if fragmentation or fragment payload encoding fails. -pub fn fragment_envelope(env: &Envelope, fragmenter: &Fragmenter) -> TestResult> { - let parts = env.clone().into_parts(); - let id = parts.id(); - let correlation = parts.correlation_id(); - let payload = parts.into_payload(); - - if payload.len() <= fragmenter.max_fragment_size().get() { - return Ok(vec![Envelope::new(id, correlation, payload)]); - } - - let envelopes = fragmenter - .fragment_bytes(payload)? - .into_iter() - .map(|fragment| { - let (header, payload) = fragment.into_parts(); - encode_fragment_payload(header, &payload) - .map(|encoded| Envelope::new(id, correlation, encoded)) - .map_err(TestError::from) - }) - .collect::, TestError>>()?; - - Ok(envelopes) -} - -/// Send a slice of envelopes over a framed client connection. -/// -/// # Errors -/// -/// Returns an error if serialization or sending fails. -pub async fn send_envelopes( - client: &mut Framed, - envelopes: &[Envelope], -) -> TestResult { - let serializer = BincodeSerializer; - for env in envelopes { - let bytes = serializer.serialize(env)?; - client.send(bytes.into()).await?; - } - Ok(()) -} - -/// Read and reassemble a fragmented response from a client connection. -/// -/// # Errors -/// -/// Returns an error if reading, deserialization, or reassembly fails, -/// or if the stream ends before reassembly completes. -pub async fn read_reassembled_response( - client: &mut Framed, - cfg: &FragmentationConfig, -) -> TestResult> { - let serializer = BincodeSerializer; - let mut reassembler = Reassembler::new(cfg.max_message_size, cfg.reassembly_timeout); - - while let Some(frame) = client.next().await { - let bytes = frame?; - let (env, _) = serializer.deserialize::(&bytes)?; - let payload = env.into_parts().into_payload(); - match decode_fragment_payload(&payload)? { - Some((header, fragment)) => { - if let Some(message) = reassembler.push(header, fragment)? { - return Ok(message.into_payload()); - } - } - None => return Ok(payload), - } - } - - Err(TestError::Setup( - "response stream ended before reassembly completed", - )) -} - -/// Create a handler that forwards received payloads to an unbounded channel. -/// -/// # Panics -/// -/// The returned handler panics if sending to the channel fails. -#[must_use] -pub fn make_handler(sender: &mpsc::UnboundedSender>) -> Handler { - let tx = sender.clone(); - std::sync::Arc::new(move |env: &Envelope| { - let tx = tx.clone(); - let payload = env.clone().into_parts().into_payload(); - Box::pin(async move { - assert!( - tx.send(payload).is_ok(), - "handler channel send must succeed in tests" - ); - }) - }) -} - -/// Create a test [`WireframeApp`] with fragmentation enabled. -/// -/// # Errors -/// -/// Returns an error if app creation or route registration fails. -pub fn make_app( - capacity: usize, - config: FragmentationConfig, - sender: &mpsc::UnboundedSender>, -) -> TestResult { - Ok(WireframeApp::new()? - .buffer_capacity(capacity) - .fragmentation(Some(config)) - .route(ROUTE_ID, make_handler(sender))?) -} - -/// Spawn an app and return the client connection and server task handle. -pub fn spawn_app( - app: WireframeApp, -) -> ( - Framed, - tokio::task::JoinHandle>, -) { - let codec = app.length_codec(); - let (client_stream, server_stream) = tokio::io::duplex(256); - let client = Framed::new(client_stream, codec.clone()); - let server = tokio::spawn(async move { app.handle_connection_result(server_stream).await }); - (client, server) -} - -/// Build envelopes from a request, optionally fragmenting. -/// -/// # Errors -/// -/// Returns an error if fragmentation fails when `should_fragment` is true. -pub fn build_envelopes( - request: Envelope, - config: &FragmentationConfig, - should_fragment: bool, -) -> TestResult> { - if should_fragment { - let fragmenter = Fragmenter::new(config.fragment_payload_cap); - fragment_envelope(&request, &fragmenter) - } else { - Ok(vec![request]) - } -} - -/// Assert that the handler received the expected payload. -/// -/// # Errors -/// -/// Returns an error if the receive times out, the channel is closed, -/// or the observed payload does not match the expected payload. -pub async fn assert_handler_observed( - rx: &mut mpsc::UnboundedReceiver>, - expected: &[u8], -) -> TestResult<()> { - let observed = timeout(TokioDuration::from_secs(1), rx.recv()) - .await? - .ok_or(TestError::Setup("handler payload missing"))?; - if observed != expected { - return Err(TestError::Assertion(format!( - "observed payload mismatch: expected {expected:?}, got {observed:?}" - ))); - } - Ok(()) -} - -/// Read and return the response payload with a 1-second timeout. -/// -/// # Errors -/// -/// Returns an error if the read times out or reassembly fails. -pub async fn read_response_payload( - client: &mut Framed, - config: &FragmentationConfig, -) -> TestResult> { - let response = timeout( - TokioDuration::from_secs(1), - read_reassembled_response(client, config), - ) - .await??; - Ok(response) -} diff --git a/tests/common/fragment_helpers/app.rs b/tests/common/fragment_helpers/app.rs new file mode 100644 index 00000000..4801cf07 --- /dev/null +++ b/tests/common/fragment_helpers/app.rs @@ -0,0 +1,64 @@ +//! Test application builders for fragment integration tests. + +use std::io; + +use tokio::sync::mpsc; +use tokio_util::codec::{Framed, LengthDelimitedCodec}; +use wireframe::{ + app::{Envelope, Handler, Packet, WireframeApp}, + fragment::FragmentationConfig, +}; + +use super::{ROUTE_ID, TestResult}; + +const APP_DUPLEX_BUFFER_SIZE: usize = 8_192; + +/// Create a handler that forwards received payloads to an unbounded channel. +/// +/// # Panics +/// +/// The returned handler panics if sending to the channel fails. +#[must_use] +pub fn make_handler(sender: &mpsc::UnboundedSender>) -> Handler { + let tx = sender.clone(); + std::sync::Arc::new(move |env: &Envelope| { + let tx = tx.clone(); + let payload = env.clone().into_parts().into_payload(); + Box::pin(async move { + assert!( + tx.send(payload).is_ok(), + "handler channel send must succeed in tests" + ); + }) + }) +} + +/// Create a test [`WireframeApp`] with fragmentation enabled. +/// +/// # Errors +/// +/// Returns an error if app creation or route registration fails. +pub fn make_app( + capacity: usize, + config: FragmentationConfig, + sender: &mpsc::UnboundedSender>, +) -> TestResult { + Ok(WireframeApp::new()? + .buffer_capacity(capacity) + .fragmentation(Some(config)) + .route(ROUTE_ID, make_handler(sender))?) +} + +/// Spawn an app and return the client connection and server task handle. +pub fn spawn_app( + app: WireframeApp, +) -> ( + Framed, + tokio::task::JoinHandle>, +) { + let codec = app.length_codec(); + let (client_stream, server_stream) = tokio::io::duplex(APP_DUPLEX_BUFFER_SIZE); + let client = Framed::new(client_stream, codec.clone()); + let server = tokio::spawn(async move { app.handle_connection_result(server_stream).await }); + (client, server) +} diff --git a/tests/common/fragment_helpers/assertions.rs b/tests/common/fragment_helpers/assertions.rs new file mode 100644 index 00000000..c26cafe4 --- /dev/null +++ b/tests/common/fragment_helpers/assertions.rs @@ -0,0 +1,29 @@ +//! Assertion helpers for fragment integration tests. + +use tokio::{ + sync::mpsc, + time::{Duration as TokioDuration, timeout}, +}; + +use super::{TestError, TestResult}; + +/// Assert that the handler received the expected payload. +/// +/// # Errors +/// +/// Returns an error if the receive times out, the channel is closed, +/// or the observed payload does not match the expected payload. +pub async fn assert_handler_observed( + rx: &mut mpsc::UnboundedReceiver>, + expected: &[u8], +) -> TestResult<()> { + let observed = timeout(TokioDuration::from_secs(1), rx.recv()) + .await? + .ok_or(TestError::Setup("handler payload missing"))?; + if observed != expected { + return Err(TestError::Assertion(format!( + "observed payload mismatch: expected {expected:?}, got {observed:?}" + ))); + } + Ok(()) +} diff --git a/tests/common/fragment_helpers/config.rs b/tests/common/fragment_helpers/config.rs new file mode 100644 index 00000000..5129a7ac --- /dev/null +++ b/tests/common/fragment_helpers/config.rs @@ -0,0 +1,47 @@ +//! Fragmentation configuration builders for integration tests. + +use std::{num::NonZeroUsize, time::Duration}; + +use wireframe::fragment::FragmentationConfig; + +use super::{TestError, TestResult}; + +const MESSAGE_LIMIT_MULTIPLIER: usize = 16; + +/// Create a fragmentation config for a given buffer capacity. +/// +/// Uses [`MESSAGE_LIMIT_MULTIPLIER`] times capacity as the message limit and a +/// 30ms reassembly timeout. +/// +/// # Errors +/// +/// Returns an error if the message limit overflows or if the frame budget +/// is too small to accommodate fragment overhead. +pub fn fragmentation_config(capacity: usize) -> TestResult { + let message_limit = capacity + .checked_mul(MESSAGE_LIMIT_MULTIPLIER) + .and_then(NonZeroUsize::new) + .ok_or(TestError::Setup("message limit overflow or zero"))?; + + let config = + FragmentationConfig::for_frame_budget(capacity, message_limit, Duration::from_millis(30)) + .ok_or(TestError::Setup( + "frame budget must exceed fragment overhead", + ))?; + + Ok(config) +} + +/// Create a fragmentation config with a custom reassembly timeout. +/// +/// # Errors +/// +/// Returns an error if the underlying [`fragmentation_config`] fails. +pub fn fragmentation_config_with_timeout( + capacity: usize, + timeout_ms: u64, +) -> TestResult { + let mut config = fragmentation_config(capacity)?; + config.reassembly_timeout = Duration::from_millis(timeout_ms); + Ok(config) +} diff --git a/tests/common/fragment_helpers/envelopes.rs b/tests/common/fragment_helpers/envelopes.rs new file mode 100644 index 00000000..2b6ce134 --- /dev/null +++ b/tests/common/fragment_helpers/envelopes.rs @@ -0,0 +1,58 @@ +//! Fragment envelope construction helpers for integration tests. + +use wireframe::{ + app::{Envelope, Packet}, + fragment::{FragmentationConfig, Fragmenter, encode_fragment_payload}, +}; + +use super::{TestError, TestResult}; + +/// Fragment an envelope into multiple fragment envelopes. +/// +/// Returns the original envelope wrapped in a vec if the payload fits in a +/// single fragment, otherwise returns the fragmented envelopes. +/// +/// # Errors +/// +/// Returns an error if fragmentation or fragment payload encoding fails. +pub fn fragment_envelope(env: &Envelope, fragmenter: &Fragmenter) -> TestResult> { + let parts = env.clone().into_parts(); + let id = parts.id(); + let correlation = parts.correlation_id(); + let payload = parts.into_payload(); + + if payload.len() <= fragmenter.max_fragment_size().get() { + return Ok(vec![Envelope::new(id, correlation, payload)]); + } + + let envelopes = fragmenter + .fragment_bytes(payload)? + .into_iter() + .map(|fragment| { + let (header, payload) = fragment.into_parts(); + encode_fragment_payload(header, &payload) + .map(|encoded| Envelope::new(id, correlation, encoded)) + .map_err(TestError::from) + }) + .collect::, TestError>>()?; + + Ok(envelopes) +} + +/// Build envelopes from a request, optionally fragmenting. +/// +/// # Errors +/// +/// Returns an error if fragmentation fails when `should_fragment` is true. +pub fn build_envelopes( + request: Envelope, + config: &FragmentationConfig, + should_fragment: bool, +) -> TestResult> { + if should_fragment { + let fragmenter = Fragmenter::new(config.fragment_payload_cap); + fragment_envelope(&request, &fragmenter) + } else { + Ok(vec![request]) + } +} diff --git a/tests/common/fragment_helpers/errors.rs b/tests/common/fragment_helpers/errors.rs new file mode 100644 index 00000000..a3867563 --- /dev/null +++ b/tests/common/fragment_helpers/errors.rs @@ -0,0 +1,56 @@ +//! Error types shared by fragment transport integration helpers. + +use thiserror::Error; +use tokio::sync::mpsc; +use wireframe::fragment::ReassemblyError; + +pub type TestResult = Result; + +/// Error type for fragment transport tests. +#[derive(Debug, Error)] +pub enum TestError { + /// Test setup failed. + #[error("test setup failed: {0}")] + Setup(&'static str), + /// Fragmentation operation failed. + #[error("fragmentation failed: {0}")] + Fragmentation(#[from] wireframe::fragment::FragmentationError), + /// Encoding operation failed. + #[error("encoding failed: {0}")] + Encode(#[from] bincode::error::EncodeError), + /// Decoding operation failed. + #[error("decoding failed: {0}")] + Decode(#[from] bincode::error::DecodeError), + /// Reassembly operation failed. + #[error("reassembly failed: {0}")] + Reassembly(#[from] ReassemblyError), + /// Send operation failed. + #[error("send failed: {0}")] + Send(String), + /// Application error. + #[error("application error: {0}")] + App(wireframe::WireframeError), + /// Other error. + #[error(transparent)] + Other(#[from] Box), + /// Test assertion failed. + #[error("assertion failed: {0}")] + Assertion(String), + /// IO operation failed. + #[error("io failed: {0}")] + Io(#[from] std::io::Error), + /// Operation timed out. + #[error("timeout: {0}")] + Timeout(#[from] tokio::time::error::Elapsed), + /// Task join failed. + #[error("task join failed: {0}")] + Join(#[from] tokio::task::JoinError), +} + +impl From> for TestError { + fn from(err: mpsc::error::SendError) -> Self { TestError::Send(err.to_string()) } +} + +impl From for TestError { + fn from(err: wireframe::WireframeError) -> Self { TestError::App(err) } +} diff --git a/tests/common/fragment_helpers/transport.rs b/tests/common/fragment_helpers/transport.rs new file mode 100644 index 00000000..7c5a6e3a --- /dev/null +++ b/tests/common/fragment_helpers/transport.rs @@ -0,0 +1,78 @@ +//! Framed transport helpers for fragment integration tests. + +use futures::{SinkExt, StreamExt}; +use tokio::time::{Duration as TokioDuration, timeout}; +use tokio_util::codec::{Framed, LengthDelimitedCodec}; +use wireframe::{ + app::{Envelope, Packet}, + fragment::{FragmentationConfig, Reassembler, decode_fragment_payload}, + serializer::{BincodeSerializer, Serializer}, +}; + +use super::{TestError, TestResult}; + +/// Send a slice of envelopes over a framed client connection. +/// +/// # Errors +/// +/// Returns an error if encoding the envelope or sending fails. +pub async fn send_envelopes( + client: &mut Framed, + envelopes: &[Envelope], +) -> TestResult { + let serializer = BincodeSerializer; + for env in envelopes { + let bytes = serializer.serialize(env)?; + client.send(bytes.into()).await?; + } + Ok(()) +} + +/// Read and reassemble a fragmented response from a client connection. +/// +/// # Errors +/// +/// Returns an error if reading, decoding the envelope, or reassembly fails, +/// or if the stream ends before reassembly completes. +pub async fn read_reassembled_response( + client: &mut Framed, + cfg: &FragmentationConfig, +) -> TestResult> { + let serializer = BincodeSerializer; + let mut reassembler = Reassembler::new(cfg.max_message_size, cfg.reassembly_timeout); + + while let Some(frame) = client.next().await { + let bytes = frame?; + let (env, _) = serializer.deserialize::(&bytes)?; + let payload = env.into_parts().into_payload(); + match decode_fragment_payload(&payload)? { + Some((header, fragment)) => { + if let Some(message) = reassembler.push(header, fragment)? { + return Ok(message.into_payload()); + } + } + None => return Ok(payload), + } + } + + Err(TestError::Setup( + "response stream ended before reassembly completed", + )) +} + +/// Read and return the response payload with a 1-second timeout. +/// +/// # Errors +/// +/// Returns an error if the read times out or reassembly fails. +pub async fn read_response_payload( + client: &mut Framed, + config: &FragmentationConfig, +) -> TestResult> { + let response = timeout( + TokioDuration::from_secs(1), + read_reassembled_response(client, config), + ) + .await??; + Ok(response) +} diff --git a/tests/common/workspace_manifest_support.rs b/tests/common/workspace_manifest_support.rs index 61ac6742..af7d7a94 100644 --- a/tests/common/workspace_manifest_support.rs +++ b/tests/common/workspace_manifest_support.rs @@ -39,6 +39,9 @@ pub(crate) fn cargo_package_id(package_name: &str) -> WorkspaceManifestResult WorkspaceManifestResult { cargo_package_id("wireframe") } +pub(crate) fn helper_package_id() -> WorkspaceManifestResult { + cargo_package_id("wireframe_testing") +} pub(crate) fn verification_package_id() -> WorkspaceManifestResult { cargo_package_id("wireframe-verification") } diff --git a/tests/features/workspace_manifest.feature b/tests/features/workspace_manifest.feature index 2c9bf8b1..d0fdd32a 100644 --- a/tests/features/workspace_manifest.feature +++ b/tests/features/workspace_manifest.feature @@ -1,11 +1,12 @@ Feature: Formal verification workspace manifest The repository should expose an explicit hybrid Cargo workspace while keeping the root package as the only default member after adding the verification - crate in roadmap item 15.1.2. + crate and the testing helper crate as explicit members. - Scenario: The root manifest adds the verification crate without widening defaults + Scenario: The root manifest lists support crates without widening defaults Given the repository workspace metadata is loaded Then the root Cargo manifest declares the staged hybrid workspace And the workspace metadata reports the root package as a workspace member And the workspace metadata reports the root package as the only default member And the workspace metadata includes the verification crate as a workspace member + And the workspace metadata includes the testing helper crate as a workspace member diff --git a/tests/fixtures/codec_performance_benchmarks.rs b/tests/fixtures/codec_performance_benchmarks.rs index e2012d37..ef2eba3e 100644 --- a/tests/fixtures/codec_performance_benchmarks.rs +++ b/tests/fixtures/codec_performance_benchmarks.rs @@ -1,31 +1,21 @@ //! `CodecPerformanceBenchmarksWorld` fixture for codec benchmark behaviour tests. use rstest::fixture; - -#[path = "../common/codec_benchmark_support.rs"] -mod codec_benchmark_support; - -#[path = "../common/codec_fragmentation_benchmark_support.rs"] -mod codec_fragmentation_benchmark_support; - -#[path = "../common/codec_alloc_benchmark_support.rs"] -mod codec_alloc_benchmark_support; - -use codec_alloc_benchmark_support::{AllocationBaseline, allocation_label}; -use codec_benchmark_support::{ +/// Re-export `TestResult` from `wireframe_testing` for use in steps. +pub use wireframe_testing::TestResult; +use wireframe_testing::codec_benchmarks::{ + AllocationBaseline, + FRAGMENT_PAYLOAD_CAP_BYTES, + FragmentationOverhead, + MeasurementExt as _, + PayloadClass, VALIDATION_ITERATIONS, + allocation_label, benchmark_workloads, measure_decode, measure_encode, -}; -use codec_fragmentation_benchmark_support::{ - FRAGMENT_PAYLOAD_CAP_BYTES, - FragmentationOverhead, - MeasurementExt as _, measure_fragmentation_overhead, }; -/// Re-export `TestResult` from `wireframe_testing` for use in steps. -pub use wireframe_testing::TestResult; /// Holds benchmark configuration and captured state for codec performance tests. /// @@ -123,7 +113,7 @@ impl CodecPerformanceBenchmarksWorld { } self.fragmentation_overhead = Some(measure_fragmentation_overhead( - codec_benchmark_support::PayloadClass::Large, + PayloadClass::Large, self.iterations, FRAGMENT_PAYLOAD_CAP_BYTES, )?); diff --git a/tests/fixtures/workspace_manifest.rs b/tests/fixtures/workspace_manifest.rs index cf808bc1..807dfe4f 100644 --- a/tests/fixtures/workspace_manifest.rs +++ b/tests/fixtures/workspace_manifest.rs @@ -7,6 +7,7 @@ use crate::workspace_manifest_support::{ WorkspaceManifestResult as FixtureResult, cargo_metadata, has_manifest_line, + helper_package_id, root_manifest, root_package_id, verification_package_id, @@ -22,6 +23,7 @@ const VERIFICATION_PACKAGE_NAME: &str = "wireframe-verification"; pub struct WorkspaceManifestWorld { manifest: Option, metadata: Option, + helper_package_id: Option, package_id: Option, verification_package_id: Option, } @@ -43,6 +45,7 @@ impl WorkspaceManifestWorld { pub fn load(&mut self) -> TestResult { self.manifest = Some(root_manifest()?); self.metadata = Some(cargo_metadata()?); + self.helper_package_id = Some(helper_package_id()?); self.package_id = Some(root_package_id()?); self.verification_package_id = Some(verification_package_id()?); Ok(()) @@ -66,6 +69,12 @@ impl WorkspaceManifestWorld { .ok_or_else(|| "workspace package id not loaded".to_owned()) } + fn helper_package_id(&self) -> Result<&str, String> { + self.helper_package_id + .as_deref() + .ok_or_else(|| "helper package id not loaded".to_owned()) + } + fn verification_package_id(&self) -> Result<&str, String> { self.verification_package_id .as_deref() @@ -92,6 +101,24 @@ impl WorkspaceManifestWorld { .any(|package| package.get("name").and_then(Value::as_str) == Some(name))) } + fn verify_crate_is_workspace_member(&self, package_id: &str, package_name: &str) -> TestResult { + let metadata = self.metadata_json()?; + let workspace_members = Self::metadata_array(&metadata, "workspace_members")?; + if !workspace_members + .iter() + .any(|member| member.as_str() == Some(package_id)) + { + return Err(format!( + "workspace metadata should include `{package_name}` in workspace_members" + ) + .into()); + } + if !Self::packages_include_name(&metadata, package_name)? { + return Err(format!("cargo metadata packages should include `{package_name}`").into()); + } + Ok(()) + } + /// Verify the root manifest declares the staged hybrid workspace contract. /// /// # Errors @@ -102,7 +129,7 @@ impl WorkspaceManifestWorld { let manifest = self.manifest()?; for expected in [ "[workspace]", - "members = [\".\", \"crates/wireframe-verification\"]", + "members = [\".\", \"crates/wireframe-verification\", \"wireframe_testing\"]", "default-members = [\".\"]", "resolver = \"3\"", ] { @@ -169,30 +196,21 @@ impl WorkspaceManifestWorld { /// # Errors /// /// Returns an error when the verification crate is missing from - /// `workspace_members` or the helper crate unexpectedly disappears from - /// Cargo metadata. + /// `workspace_members` or Cargo metadata. pub fn verify_verification_crate_is_workspace_member(&self) -> TestResult { - let metadata = self.metadata_json()?; - let verification_package_id = self.verification_package_id()?; - let workspace_members = Self::metadata_array(&metadata, "workspace_members")?; - if !workspace_members - .iter() - .any(|member| member.as_str() == Some(verification_package_id)) - { - return Err( - "workspace metadata should include the verification crate in workspace_members" - .into(), - ); - } - if !Self::packages_include_name(&metadata, VERIFICATION_PACKAGE_NAME)? { - return Err("cargo metadata packages should include the verification crate".into()); - } - if !Self::packages_include_name(&metadata, HELPER_PACKAGE_NAME)? { - return Err( - "workspace metadata should continue to report the in-repository helper crate" - .into(), - ); - } - Ok(()) + self.verify_crate_is_workspace_member( + self.verification_package_id()?, + VERIFICATION_PACKAGE_NAME, + ) + } + + /// Verify the testing helper crate is part of the workspace membership. + /// + /// # Errors + /// + /// Returns an error when the helper crate is missing from + /// `workspace_members` or Cargo metadata. + pub fn verify_helper_crate_is_workspace_member(&self) -> TestResult { + self.verify_crate_is_workspace_member(self.helper_package_id()?, HELPER_PACKAGE_NAME) } } diff --git a/tests/lifecycle.rs b/tests/lifecycle.rs index 8f6d7c7a..e1b9c2a0 100644 --- a/tests/lifecycle.rs +++ b/tests/lifecycle.rs @@ -101,15 +101,15 @@ async fn setup_and_teardown_callbacks_run() -> TestResult<()> { reason = "asserts provide clearer diagnostics in tests" )] async fn setup_without_teardown_runs() -> TestResult<()> { - let setup_count = Arc::new(AtomicUsize::new(0)); - let cb = call_counting_callback(&setup_count, ()); + let counter = Arc::new(AtomicUsize::new(0)); + let cb = call_counting_callback(&counter, ()); let app = BasicApp::new()?.on_connection_setup(move || cb(()))?; run_with_duplex_server(app).await; assert_eq!( - setup_count.load(Ordering::SeqCst), + counter.load(Ordering::SeqCst), 1, "setup callback did not run" ); @@ -123,15 +123,15 @@ async fn setup_without_teardown_runs() -> TestResult<()> { reason = "asserts provide clearer diagnostics in tests" )] async fn teardown_without_setup_does_not_run() -> TestResult<()> { - let teardown_count = Arc::new(AtomicUsize::new(0)); - let cb = call_counting_callback(&teardown_count, ()); + let counter = Arc::new(AtomicUsize::new(0)); + let cb = call_counting_callback(&counter, ()); let app = BasicApp::new()?.on_connection_teardown(cb)?; run_with_duplex_server(app).await; assert_eq!( - teardown_count.load(Ordering::SeqCst), + counter.load(Ordering::SeqCst), 0, "teardown callback should not run" ); @@ -139,6 +139,37 @@ async fn teardown_without_setup_does_not_run() -> TestResult<()> { Ok(()) } +#[tokio::test] +#[expect( + clippy::panic_in_result_fn, + reason = "asserts provide clearer diagnostics in tests" +)] +async fn setup_after_teardown_clears_previous_teardown() -> TestResult<()> { + let setup_count = Arc::new(AtomicUsize::new(0)); + let teardown_count = Arc::new(AtomicUsize::new(0)); + let setup_cb = call_counting_callback(&setup_count, 42_u32); + let teardown_cb = call_counting_callback(&teardown_count, ()); + + let app = BasicApp::new()? + .on_connection_teardown(teardown_cb)? + .on_connection_setup(move || setup_cb(()))?; + + run_with_duplex_server(app).await; + + assert_eq!( + setup_count.load(Ordering::SeqCst), + 1, + "setup callback did not run" + ); + assert_eq!( + teardown_count.load(Ordering::SeqCst), + 0, + "teardown callback for prior state type should be cleared" + ); + + Ok(()) +} + #[tokio::test] #[expect( clippy::panic_in_result_fn, diff --git a/tests/response.rs b/tests/response.rs index ed4860a8..a782b4dc 100644 --- a/tests/response.rs +++ b/tests/response.rs @@ -7,8 +7,10 @@ use std::sync::Arc; use bytes::BytesMut; +use pretty_assertions::assert_eq; use rstest::rstest; -use tokio_util::codec::{Decoder, Encoder, LengthDelimitedCodec}; +use tokio::io::AsyncReadExt; +use tokio_util::codec::{Decoder, Encoder, Framed, LengthDelimitedCodec}; use wireframe::{ app::{Envelope, Packet, WireframeApp}, frame::{Endianness, LengthFormat}, @@ -24,42 +26,21 @@ use wireframe_testing::{ run_app, }; +#[path = "response/response_errors.rs"] +mod response_errors; + // Larger cap used for oversized frame tests. const LARGE_FRAME: usize = 16 * 1024 * 1024; #[derive(bincode::Encode, bincode::BorrowDecode, PartialEq, Debug)] struct TestResp(u32); -#[derive(Debug)] -struct FailingResp; - -impl bincode::Encode for FailingResp { - fn encode( - &self, - _: &mut E, - ) -> Result<(), bincode::error::EncodeError> { - Err(bincode::error::EncodeError::Other("fail")) - } -} - -impl<'de> bincode::BorrowDecode<'de, ()> for FailingResp { - fn borrow_decode>( - _: &mut D, - ) -> Result { - Ok(FailingResp) - } -} - #[derive(bincode::Encode, bincode::BorrowDecode, PartialEq, Debug)] struct Large(Vec); /// Tests that sending a response serializes and frames the data correctly, /// and that the response can be decoded and deserialized back to its original value asynchronously. #[tokio::test] -#[expect( - clippy::panic_in_result_fn, - reason = "asserts provide clearer diagnostics in tests" -)] async fn send_response_encodes_and_frames() -> TestResult { let app = TestApp::new().expect("failed to create app"); @@ -105,32 +86,6 @@ async fn length_prefixed_decode_requires_full_frame() { assert_eq!(buf.len(), 2); } -struct FailingWriter; - -impl tokio::io::AsyncWrite for FailingWriter { - fn poll_write( - self: std::pin::Pin<&mut Self>, - _: &mut std::task::Context<'_>, - _: &[u8], - ) -> std::task::Poll> { - std::task::Poll::Ready(Err(std::io::Error::other("fail"))) - } - - fn poll_flush( - self: std::pin::Pin<&mut Self>, - _: &mut std::task::Context<'_>, - ) -> std::task::Poll> { - std::task::Poll::Ready(Ok(())) - } - - fn poll_shutdown( - self: std::pin::Pin<&mut Self>, - _: &mut std::task::Context<'_>, - ) -> std::task::Poll> { - std::task::Poll::Ready(Ok(())) - } -} - #[rstest] #[case(LengthFormat::u16_be(), vec![1, 2, 3, 4], vec![0x00, 0x04])] #[case(LengthFormat::u32_le(), vec![9, 8, 7], vec![3, 0, 0, 0])] @@ -161,18 +116,6 @@ fn custom_length_roundtrip( assert!(buf.is_empty(), "unexpected trailing bytes after decode"); } -#[tokio::test] -async fn send_response_propagates_write_error() { - let app = TestApp::new().expect("app creation failed"); - - let mut writer = FailingWriter; - let err = app - .send_response(&mut writer, &TestResp(3)) - .await - .expect_err("send_response should propagate write error"); - assert!(matches!(err, wireframe::app::SendError::Io(_))); -} - #[rstest] #[case(0, Endianness::Big)] #[case(9, Endianness::Little)] @@ -201,28 +144,84 @@ fn encode_fails_for_length_too_large(#[case] fmt: LengthFormat, #[case] len: usi assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput); } -/// Tests that `send_response` returns a serialization error when encoding fails. -/// -/// This test sends a `FailingResp` using `send_response` and asserts that the resulting -/// error is of the `Serialize` variant, indicating a failure during response encoding. #[tokio::test] -async fn send_response_returns_encode_error() { - // Use a type that fails during serialization; encode should fail before any framing. +async fn send_response_framed_with_codec_writes_encoded_frame() { + let app = TestApp::new().expect("failed to create app"); + let (client, mut server) = tokio::io::duplex(1024); + let mut framed = Framed::new(client, app.length_codec()); + + app.send_response_framed_with_codec(&mut framed, &TestResp(11)) + .await + .expect("framed send should succeed"); + drop(framed); + + let mut out = Vec::new(); + server + .read_to_end(&mut out) + .await + .expect("read framed bytes"); + let decoded_frames = decode_frames(out).expect("decode response frames"); + assert_eq!(decoded_frames.len(), 1); + let frame = decoded_frames.first().expect("response frame missing"); + let (decoded, _) = TestResp::from_bytes(frame).expect("deserialize response"); + assert_eq!(decoded, TestResp(11)); +} + +#[tokio::test] +async fn send_response_framed_sends_raw_serialized_payload() -> TestResult { let app = WireframeApp::::new().expect("failed to create app"); - let err = app - .send_response(&mut Vec::new(), &FailingResp) + let (client, mut server) = tokio::io::duplex(1024); + let mut framed = Framed::new(client, app.length_codec()); + let message = TestResp(23); + let expected = BincodeSerializer + .serialize(&message) + .map_err(|e| format!("serialize failed: {e}"))?; + + app.send_response_framed(&mut framed, &message) + .await + .map_err(|e| format!("framed send failed: {e}"))?; + drop(framed); + + let mut out = Vec::new(); + server + .read_to_end(&mut out) .await - .expect_err("send_response should fail when encode errors"); - assert!(matches!(err, wireframe::app::SendError::Serialize(_))); + .map_err(|e| format!("read framed output failed: {e}"))?; + let decoded_frames = decode_frames(out)?; + assert_eq!(decoded_frames.len(), 1); + let frame = decoded_frames.first().ok_or("response frame missing")?; + assert_eq!(frame, &expected); + Ok(()) +} + +#[tokio::test] +async fn send_response_framed_honours_buffer_capacity() -> TestResult { + let app = TestApp::new()?.buffer_capacity(LARGE_FRAME); + let (client, mut server) = tokio::io::duplex(10 * 1024 * 1024); + let mut framed = Framed::new(client, app.length_codec()); + let payload = vec![0_u8; 9 * 1024 * 1024]; + + app.send_response_framed(&mut framed, &Large(payload.clone())) + .await + .map_err(|e| format!("framed send failed: {e}"))?; + drop(framed); + + let mut out = Vec::new(); + server + .read_to_end(&mut out) + .await + .map_err(|e| format!("read framed output failed: {e}"))?; + let decoded_frames = decode_frames_with_max(out, LARGE_FRAME)?; + assert_eq!(decoded_frames.len(), 1); + let frame = decoded_frames.first().ok_or("response frame missing")?; + let (decoded, _) = Large::from_bytes(frame).map_err(|e| format!("deserialize failed: {e}"))?; + assert_eq!(decoded.0.len(), payload.len()); + Ok(()) } /// Ensures `send_response` permits frames up to the configured buffer capacity, /// exceeding the codec's default 8 MiB limit. #[tokio::test] -#[expect( - clippy::panic_in_result_fn, - reason = "asserts provide clearer diagnostics in tests" -)] async fn send_response_honours_buffer_capacity() -> TestResult { let app = TestApp::new()?.buffer_capacity(LARGE_FRAME); @@ -245,10 +244,6 @@ async fn send_response_honours_buffer_capacity() -> TestResult { /// Verifies inbound and outbound codecs respect the application's buffer /// capacity by round-tripping a 9 MiB payload. #[tokio::test] -#[expect( - clippy::panic_in_result_fn, - reason = "asserts provide clearer diagnostics in tests" -)] async fn process_stream_honours_buffer_capacity() -> TestResult { let app = TestApp::new()? .buffer_capacity(LARGE_FRAME) diff --git a/tests/response/response_errors.rs b/tests/response/response_errors.rs new file mode 100644 index 00000000..ee7a0835 --- /dev/null +++ b/tests/response/response_errors.rs @@ -0,0 +1,268 @@ +//! Error-path tests and fixtures for response sending. + +use bytes::{Bytes, BytesMut}; +use tokio::io::{AsyncRead, ReadBuf}; +use tokio_util::codec::{Decoder, Encoder, Framed}; +use wireframe::{ + app::{Envelope, WireframeApp}, + codec::FrameCodec, + serializer::BincodeSerializer, +}; +use wireframe_testing::TestApp; + +use super::TestResp; + +#[derive(Debug)] +struct FailingResp; + +impl bincode::Encode for FailingResp { + fn encode( + &self, + _: &mut E, + ) -> Result<(), bincode::error::EncodeError> { + Err(bincode::error::EncodeError::Other("fail")) + } +} + +impl<'de> bincode::BorrowDecode<'de, ()> for FailingResp { + fn borrow_decode>( + _: &mut D, + ) -> Result { + Ok(FailingResp) + } +} + +struct FailingWriter; + +impl tokio::io::AsyncWrite for FailingWriter { + fn poll_write( + self: std::pin::Pin<&mut Self>, + _: &mut std::task::Context<'_>, + _: &[u8], + ) -> std::task::Poll> { + std::task::Poll::Ready(Err(std::io::Error::other("fail"))) + } + + fn poll_flush( + self: std::pin::Pin<&mut Self>, + _: &mut std::task::Context<'_>, + ) -> std::task::Poll> { + std::task::Poll::Ready(Ok(())) + } + + fn poll_shutdown( + self: std::pin::Pin<&mut Self>, + _: &mut std::task::Context<'_>, + ) -> std::task::Poll> { + std::task::Poll::Ready(Ok(())) + } +} + +impl AsyncRead for FailingWriter { + fn poll_read( + self: std::pin::Pin<&mut Self>, + _: &mut std::task::Context<'_>, + _: &mut ReadBuf<'_>, + ) -> std::task::Poll> { + std::task::Poll::Ready(Ok(())) + } +} + +struct FailingFlushWriter { + bytes: Vec, +} + +impl tokio::io::AsyncWrite for FailingFlushWriter { + fn poll_write( + mut self: std::pin::Pin<&mut Self>, + _: &mut std::task::Context<'_>, + buf: &[u8], + ) -> std::task::Poll> { + self.bytes.extend_from_slice(buf); + std::task::Poll::Ready(Ok(buf.len())) + } + + fn poll_flush( + self: std::pin::Pin<&mut Self>, + _: &mut std::task::Context<'_>, + ) -> std::task::Poll> { + std::task::Poll::Ready(Err(std::io::Error::other("flush failed"))) + } + + fn poll_shutdown( + self: std::pin::Pin<&mut Self>, + _: &mut std::task::Context<'_>, + ) -> std::task::Poll> { + std::task::Poll::Ready(Ok(())) + } +} + +#[derive(Clone, Debug)] +struct FailingEncodeFrameCodec; + +#[derive(Debug)] +struct FailingEncodeFrame(Bytes); + +struct FailingFrameDecoder; + +impl Decoder for FailingFrameDecoder { + type Error = std::io::Error; + type Item = FailingEncodeFrame; + + fn decode(&mut self, _src: &mut BytesMut) -> std::io::Result> { Ok(None) } +} + +struct FailingFrameEncoder; + +impl Encoder for FailingFrameEncoder { + type Error = std::io::Error; + + fn encode(&mut self, item: FailingEncodeFrame, _dst: &mut BytesMut) -> std::io::Result<()> { + let _payload = item.0; + Err(std::io::Error::other("frame encode failed")) + } +} + +impl FrameCodec for FailingEncodeFrameCodec { + type Decoder = FailingFrameDecoder; + type Encoder = FailingFrameEncoder; + type Frame = FailingEncodeFrame; + + fn decoder(&self) -> Self::Decoder { FailingFrameDecoder } + + fn encoder(&self) -> Self::Encoder { FailingFrameEncoder } + + fn frame_payload(frame: &Self::Frame) -> &[u8] { frame.0.as_ref() } + + fn frame_payload_bytes(frame: &Self::Frame) -> Bytes { frame.0.clone() } + + fn wrap_payload(&self, payload: Bytes) -> Self::Frame { FailingEncodeFrame(payload) } + + fn max_frame_length(&self) -> usize { 1024 } +} + +fn basic_app() -> wireframe::app::Result> { + WireframeApp::::new() +} + +fn assert_io_error(err: &wireframe::app::SendError) { + assert!( + matches!(err, wireframe::app::SendError::Io(_)), + "expected SendError::Io, got {err:?}" + ); +} + +fn assert_serialize_error(err: &wireframe::app::SendError) { + assert!( + matches!(err, wireframe::app::SendError::Serialize(_)), + "expected SendError::Serialize, got {err:?}" + ); +} + +#[tokio::test] +async fn send_response_propagates_write_error() { + let app = TestApp::new().expect("app creation failed"); + + let mut writer = FailingWriter; + let err = app + .send_response(&mut writer, &TestResp(3)) + .await + .expect_err("send_response should propagate write error"); + assert_io_error(&err); +} + +#[tokio::test] +async fn send_response_propagates_frame_encoding_error() { + let app = basic_app() + .expect("failed to create app") + .with_codec(FailingEncodeFrameCodec); + let mut writer = Vec::new(); + + let err = app + .send_response(&mut writer, &TestResp(5)) + .await + .expect_err("send_response should propagate codec encode failure"); + + assert_io_error(&err); +} + +#[tokio::test] +async fn send_response_propagates_flush_error_after_write() { + let app = TestApp::new().expect("app creation failed"); + let mut writer = FailingFlushWriter { bytes: Vec::new() }; + + let err = app + .send_response(&mut writer, &TestResp(19)) + .await + .expect_err("send_response should propagate flush failure"); + + assert_io_error(&err); + assert!(!writer.bytes.is_empty(), "response bytes should be written"); +} + +#[tokio::test] +async fn send_response_returns_encode_error() { + let app = basic_app().expect("failed to create app"); + let err = app + .send_response(&mut Vec::new(), &FailingResp) + .await + .expect_err("send_response should fail when encode errors"); + assert_serialize_error(&err); +} + +#[tokio::test] +async fn send_response_framed_with_codec_propagates_serialization_error() { + let app = basic_app().expect("failed to create app"); + let (client, _server) = tokio::io::duplex(1024); + let mut framed = Framed::new(client, app.length_codec()); + + let err = app + .send_response_framed_with_codec(&mut framed, &FailingResp) + .await + .expect_err("framed send should fail before transport"); + + assert_serialize_error(&err); +} + +#[tokio::test] +async fn send_response_framed_with_codec_propagates_frame_encoding_error() { + let app = basic_app() + .expect("failed to create app") + .with_codec(FailingEncodeFrameCodec); + let (client, _server) = tokio::io::duplex(1024); + let mut framed = Framed::new(client, FailingFrameEncoder); + + let err = app + .send_response_framed_with_codec(&mut framed, &TestResp(13)) + .await + .expect_err("framed send should propagate codec encode failure"); + + assert_io_error(&err); +} + +#[tokio::test] +async fn send_response_framed_propagates_serialization_error() { + let app = basic_app().expect("failed to create app"); + let (client, _server) = tokio::io::duplex(1024); + let mut framed = Framed::new(client, app.length_codec()); + + let err = app + .send_response_framed(&mut framed, &FailingResp) + .await + .expect_err("length-delimited send should fail before transport"); + + assert_serialize_error(&err); +} + +#[tokio::test] +async fn send_response_framed_propagates_io_error() { + let app = basic_app().expect("failed to create app"); + let mut framed = Framed::new(FailingWriter, app.length_codec()); + + let err = app + .send_response_framed(&mut framed, &TestResp(17)) + .await + .expect_err("length-delimited send should propagate write failure"); + + assert_io_error(&err); +} diff --git a/tests/scenarios/workspace_manifest_scenarios.rs b/tests/scenarios/workspace_manifest_scenarios.rs index dda6692b..f6f314ba 100644 --- a/tests/scenarios/workspace_manifest_scenarios.rs +++ b/tests/scenarios/workspace_manifest_scenarios.rs @@ -8,7 +8,7 @@ use crate::fixtures::workspace_manifest::{ workspace_manifest_world, }; -fn assert_root_manifest_adds_verification_crate( +fn assert_root_manifest_lists_support_crates( workspace_manifest_world: &mut WorkspaceManifestWorld, ) -> TestResult { workspace_manifest_world.load()?; @@ -16,15 +16,16 @@ fn assert_root_manifest_adds_verification_crate( workspace_manifest_world.verify_root_is_workspace_member()?; workspace_manifest_world.verify_root_is_only_default_member()?; workspace_manifest_world.verify_verification_crate_is_workspace_member()?; + workspace_manifest_world.verify_helper_crate_is_workspace_member()?; Ok(()) } #[scenario( path = "tests/features/workspace_manifest.feature", - name = "The root manifest adds the verification crate without widening defaults" + name = "The root manifest lists support crates without widening defaults" )] -fn root_manifest_adds_verification_crate(workspace_manifest_world: WorkspaceManifestWorld) { +fn root_manifest_lists_support_crates(workspace_manifest_world: WorkspaceManifestWorld) { let mut workspace_manifest_world = workspace_manifest_world; - assert_root_manifest_adds_verification_crate(&mut workspace_manifest_world) + assert_root_manifest_lists_support_crates(&mut workspace_manifest_world) .expect("workspace manifest scenario should pass"); } diff --git a/tests/steps/workspace_manifest_steps.rs b/tests/steps/workspace_manifest_steps.rs index e3f4186b..26be51db 100644 --- a/tests/steps/workspace_manifest_steps.rs +++ b/tests/steps/workspace_manifest_steps.rs @@ -38,3 +38,10 @@ fn then_workspace_metadata_includes_verification_crate( ) -> TestResult { workspace_manifest_world.verify_verification_crate_is_workspace_member() } + +#[then("the workspace metadata includes the testing helper crate as a workspace member")] +fn then_workspace_metadata_includes_testing_helper_crate( + workspace_manifest_world: &mut WorkspaceManifestWorld, +) -> TestResult { + workspace_manifest_world.verify_helper_crate_is_workspace_member() +} diff --git a/tests/unified_codec.rs b/tests/unified_codec.rs index 7654d73a..4f8e1183 100644 --- a/tests/unified_codec.rs +++ b/tests/unified_codec.rs @@ -18,10 +18,6 @@ use wireframe::{ }; #[path = "common/fragment_helpers.rs"] -#[expect( - dead_code, - reason = "shared helper module; not all items used by every test binary" -)] mod fragment_helpers; #[path = "common/unified_codec_transport.rs"] mod unified_codec_transport; @@ -67,12 +63,22 @@ fn echo_app( /// Build the unified codec test harness and return client/server test handles. fn setup_harness(config: Option) -> TestResult { + keep_fragment_helper_facade_reexports_linked(); let (tx, rx) = mpsc::unbounded_channel(); let app = echo_app(config, &tx)?; let (client, server) = spawn_app(app); Ok(UnifiedCodecHarness { client, server, rx }) } +fn keep_fragment_helper_facade_reexports_linked() { + let _ = fragment_helpers::make_app; + let _ = fragment_helpers::assert_handler_observed; + let _ = fragment_helpers::fragmentation_config_with_timeout; + let _ = fragment_helpers::fragment_envelope; + let _ = fragment_helpers::read_reassembled_response; + let _ = fragment_helpers::TestError::Assertion(String::new()); +} + // --------------------------------------------------------------------------- // Test: basic request-response passes through the unified pipeline // --------------------------------------------------------------------------- diff --git a/tests/workspace_manifest.rs b/tests/workspace_manifest.rs index c00a752f..32f2956e 100644 --- a/tests/workspace_manifest.rs +++ b/tests/workspace_manifest.rs @@ -1,8 +1,9 @@ //! Regression tests for the formal-verification workspace manifest contract. //! //! These checks verify that the repository advertises an explicit hybrid -//! workspace, includes the internal verification crate as a workspace member, -//! and still keeps the root package as the only default member. +//! workspace, includes the internal verification and testing crates as +//! workspace members, and still keeps the root package as the only default +//! member. #[path = "common/workspace_manifest_support.rs"] mod workspace_manifest_support; @@ -17,6 +18,7 @@ use workspace_manifest_support::{ WorkspaceManifestResult as TestResult, cargo_metadata, has_manifest_line, + helper_package_id, root_manifest, root_package_id, verification_package_id, @@ -43,9 +45,9 @@ fn root_manifest_declares_explicit_workspace_section() -> TestResult { assert!( has_manifest_line( &manifest, - "members = [\".\", \"crates/wireframe-verification\"]" + "members = [\".\", \"crates/wireframe-verification\", \"wireframe_testing\"]" ), - "15.1.2 should add the verification crate to the explicit workspace members" + "the workspace should explicitly list the root, verification, and testing crates" ); assert!( has_manifest_line(&manifest, "default-members = [\".\"]"), @@ -63,10 +65,11 @@ fn root_manifest_declares_explicit_workspace_section() -> TestResult { clippy::panic_in_result_fn, reason = "assertions provide clearer diagnostics in integration tests" )] -fn cargo_metadata_reports_verification_crate_without_widening_default_members() -> TestResult { +fn cargo_metadata_reports_explicit_members_without_widening_default_members() -> TestResult { let repo_root = repo_root()?; let repo_root_str = repo_root.as_str(); let root_package_id = root_package_id()?; + let helper_package_id = helper_package_id()?; let verification_package_id = verification_package_id()?; let manifest_path = repo_root.join("Cargo.toml"); let manifest_path_str = manifest_path.as_str(); @@ -101,10 +104,20 @@ fn cargo_metadata_reports_verification_crate_without_widening_default_members() .any(|member| member.as_str() == Some(verification_package_id.as_str())), "workspace_members should include the verification crate id" ); + assert!( + workspace_members + .iter() + .any(|member| member.as_str() == Some(helper_package_id.as_str())), + "workspace_members should include the wireframe_testing crate id" + ); assert!( metadata.contains("wireframe-verification"), "15.1.2 should add the verification crate to cargo metadata" ); + assert!( + metadata.contains("wireframe_testing"), + "workspace metadata should include the test helper crate" + ); let workspace_default_members = metadata_json .get("workspace_default_members") .and_then(Value::as_array) diff --git a/tests/common/codec_alloc_benchmark_support.rs b/wireframe_testing/src/codec_benchmarks/codec_alloc_benchmark_support.rs similarity index 72% rename from tests/common/codec_alloc_benchmark_support.rs rename to wireframe_testing/src/codec_benchmarks/codec_alloc_benchmark_support.rs index 08034784..6545d6e4 100644 --- a/tests/common/codec_alloc_benchmark_support.rs +++ b/wireframe_testing/src/codec_benchmarks/codec_alloc_benchmark_support.rs @@ -4,14 +4,11 @@ //! by criterion allocation benchmarks, rstest unit tests, and rstest-bdd //! behavioural tests. //! -//! # Layout coupling +//! # Module layout //! //! This module references `codec_benchmark_support` via `super::` and therefore -//! must be declared as a sibling `mod` in the same parent scope. Every current -//! consumer already satisfies this constraint because the `#[path]` inclusion -//! pattern compiles both modules into the same crate root. If the helpers are -//! ever reused outside the current test/bench tree, consider introducing a -//! `tests/common/mod.rs` hierarchy with normal `mod`/`pub` wiring instead. +//! remains a sibling of the core benchmark helpers under +//! `wireframe_testing::codec_benchmarks`. use super::codec_benchmark_support::BenchmarkWorkload; diff --git a/tests/common/codec_benchmark_support.rs b/wireframe_testing/src/codec_benchmarks/codec_benchmark_support.rs similarity index 98% rename from tests/common/codec_benchmark_support.rs rename to wireframe_testing/src/codec_benchmarks/codec_benchmark_support.rs index 48c9093d..32fac3ce 100644 --- a/tests/common/codec_benchmark_support.rs +++ b/wireframe_testing/src/codec_benchmarks/codec_benchmark_support.rs @@ -65,6 +65,10 @@ impl PayloadClass { Self::Large => LARGE_PAYLOAD_BYTES, } } + + /// Whether this payload class represents an empty payload. + #[must_use] + pub const fn is_empty(self) -> bool { self.len() == 0 } } /// A single benchmark workload definition. diff --git a/tests/common/codec_fragmentation_benchmark_support.rs b/wireframe_testing/src/codec_benchmarks/codec_fragmentation_benchmark_support.rs similarity index 92% rename from tests/common/codec_fragmentation_benchmark_support.rs rename to wireframe_testing/src/codec_benchmarks/codec_fragmentation_benchmark_support.rs index a68ff153..83b726db 100644 --- a/tests/common/codec_fragmentation_benchmark_support.rs +++ b/wireframe_testing/src/codec_benchmarks/codec_fragmentation_benchmark_support.rs @@ -3,14 +3,11 @@ //! This module defines the fragmentation overhead measurement helpers used by //! criterion benches, rstest unit tests, and rstest-bdd behavioural tests. //! -//! # Layout coupling +//! # Module layout //! //! This module references `codec_benchmark_support` via `super::` and therefore -//! must be declared as a sibling `mod` in the same parent scope. Every current -//! consumer already satisfies this constraint because the `#[path]` inclusion -//! pattern compiles both modules into the same crate root. If the helpers are -//! ever reused outside the current test/bench tree, consider introducing a -//! `tests/common/mod.rs` hierarchy with normal `mod`/`pub` wiring instead. +//! remains a sibling of the core benchmark helpers under +//! `wireframe_testing::codec_benchmarks`. use std::num::NonZeroUsize; diff --git a/wireframe_testing/src/codec_benchmarks/mod.rs b/wireframe_testing/src/codec_benchmarks/mod.rs new file mode 100644 index 00000000..e5aa9f7b --- /dev/null +++ b/wireframe_testing/src/codec_benchmarks/mod.rs @@ -0,0 +1,28 @@ +//! Codec benchmark helpers shared by benches and behavioural tests. + +pub mod codec_alloc_benchmark_support; +pub mod codec_benchmark_support; +pub mod codec_fragmentation_benchmark_support; + +pub use codec_alloc_benchmark_support::{AllocationBaseline, allocation_label}; +pub use codec_benchmark_support::{ + BenchmarkWorkload, + CodecUnderTest, + LARGE_PAYLOAD_BYTES, + Measurement, + PayloadClass, + SMALL_PAYLOAD_BYTES, + VALIDATION_ITERATIONS, + benchmark_workloads, + measure_decode, + measure_encode, + payload_for_class, +}; +pub use codec_fragmentation_benchmark_support::{ + FRAGMENT_PAYLOAD_CAP_BYTES, + FragmentationOverhead, + MeasurementExt, + measure_fragmentation_overhead, + measure_fragmented_wrap, + measure_unfragmented_wrap, +}; diff --git a/wireframe_testing/src/lib.rs b/wireframe_testing/src/lib.rs index cd365e88..e425f0bc 100644 --- a/wireframe_testing/src/lib.rs +++ b/wireframe_testing/src/lib.rs @@ -19,6 +19,7 @@ //! ``` pub mod client_pair; +pub mod codec_benchmarks; pub mod echo_server; pub mod helpers; pub mod integration_helpers;