From 88ad304b9d0d08a5694c9e5d3ee6364136f03699 Mon Sep 17 00:00:00 2001 From: leynos Date: Thu, 16 Jul 2026 20:07:52 +0200 Subject: [PATCH 01/15] Exclude test scaffolding submodules from mutation runs The mutation-testing caller's `src/test_helpers.rs` exclude glob matched only the module root, leaving the `src/test_helpers/` submodules (frame_codec.rs, pool_client.rs) to emit noise survivors. Companion `tests.rs` files carrying cfg(test)-only modules were likewise mutated because cargo-mutants cannot detect them as test code. Add `src/test_helpers/**` and a repo-wide `src/**/tests.rs` glob to the caller and pin both in the workflow contract test. --- .github/workflows/mutation-testing.yml | 8 ++++++-- tests/workflow_contracts/mutation_testing_test.py | 6 ++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/.github/workflows/mutation-testing.yml b/.github/workflows/mutation-testing.yml index 3bbd305b..d69b3fb9 100644 --- a/.github/workflows/mutation-testing.yml +++ b/.github/workflows/mutation-testing.yml @@ -33,8 +33,12 @@ jobs: # baseline cannot pass (#578). Restore # `extra-crate-dirs: "wireframe_testing"` when that issue closes. # Illustrative codecs and test-support scaffolding whose - # survivors are noise (#571). - exclude-globs: "src/codec/examples.rs,src/test_helpers.rs,src/connection/test_support.rs" + # survivors are noise (#571). The `src/test_helpers.rs` glob only + # matches the module root; `src/test_helpers/**` also covers the + # helper submodules (frame_codec.rs, pool_client.rs). `src/**/tests.rs` + # covers the cfg(test)-only test modules split into companion files, + # which cargo-mutants cannot detect as test code (#599). + exclude-globs: "src/codec/examples.rs,src/test_helpers.rs,src/test_helpers/**,src/connection/test_support.rs,src/**/tests.rs" # Match the CI baseline (`make test` runs --all-features) so # feature-gated tests run against mutants (#571). extra-args: "--all-features" diff --git a/tests/workflow_contracts/mutation_testing_test.py b/tests/workflow_contracts/mutation_testing_test.py index 07e5299b..c99af0cb 100644 --- a/tests/workflow_contracts/mutation_testing_test.py +++ b/tests/workflow_contracts/mutation_testing_test.py @@ -30,8 +30,14 @@ SCAFFOLDING_EXCLUDES = ( "src/test_helpers.rs", + # The module-root glob above matches only src/test_helpers.rs; the + # directory glob covers the helper submodules (#599). + "src/test_helpers/**", "src/connection/test_support.rs", "src/codec/examples.rs", + # cfg(test)-only test modules split into companion files, which + # cargo-mutants cannot detect as test code (#599). + "src/**/tests.rs", ) From ef07d019dceb663fb12e190868e75953969ecedf Mon Sep 17 00:00:00 2001 From: leynos Date: Thu, 16 Jul 2026 20:15:25 +0200 Subject: [PATCH 02/15] Assert WireframeError clean-close false cases The WireframeError wrapper only had its clean-close true case asserted; every false-case assertion targeted CodecError directly, so a wrapper that unconditionally reported a clean close passed the suite. Add false-case assertions for the Io, mid-frame Codec, and Protocol variants at the WireframeError level. Addresses #567. --- tests/codec_error.rs | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/codec_error.rs b/tests/codec_error.rs index dcb89294..35b87520 100644 --- a/tests/codec_error.rs +++ b/tests/codec_error.rs @@ -174,6 +174,40 @@ fn wireframe_error_from_codec_method() { assert!(wf_err.is_clean_close()); } +#[test] +fn wireframe_error_io_is_not_clean_close() { + use wireframe::WireframeError; + + // The Io wrapper never represents a clean close, so `is_clean_close` + // forced to a constant `true` must fail here. + let wf_err: WireframeError<()> = WireframeError::from_io(io::Error::other("reset")); + + assert!(!wf_err.is_clean_close()); +} + +#[test] +fn wireframe_error_from_mid_frame_codec_is_not_clean_close() { + use wireframe::WireframeError; + + // A non-EOF codec error wrapped at the `WireframeError` level must + // still report `false`, guarding the wrapper's own polarity. + let codec_err = CodecError::Framing(FramingError::InvalidLengthEncoding); + let wf_err: WireframeError<()> = WireframeError::from_codec(codec_err); + + assert!(!wf_err.is_clean_close()); +} + +#[test] +fn wireframe_error_protocol_is_not_clean_close() { + use wireframe::WireframeError; + + // The Protocol variant carries no codec error, so the `matches!` + // guard must reject it. + let wf_err: WireframeError<&str> = WireframeError::from("boom"); + + assert!(!wf_err.is_clean_close()); +} + #[test] fn wireframe_error_codec_variant_displays_correctly() { use wireframe::WireframeError; From b1d02efbb1215cde904bd1daacf2a1b3a27d9339 Mon Sep 17 00:00:00 2001 From: leynos Date: Thu, 16 Jul 2026 20:15:25 +0200 Subject: [PATCH 03/15] Assert untested public accessors A cluster of public accessors and trait methods was never asserted directly, so each could be replaced with a constant. Add direct assertions for WireframeApp::protocol and message_assembler, the FrameCodec trait's max_frame_length via UFCS dispatch (the inherent method shadows it), ServiceRequest::frame, BincodeSerializer's should_deserialize_after_parse, and MessageRequest::take_body_stream's take-once semantics. Addresses #598. --- src/codec/tests.rs | 12 +++++++++++ tests/extractor.rs | 12 +++++++++++ tests/middleware.rs | 8 ++++++++ tests/serializer.rs | 13 ++++++++++++ tests/wireframe_protocol.rs | 41 +++++++++++++++++++++++++++++++++++++ 5 files changed, 86 insertions(+) create mode 100644 tests/serializer.rs diff --git a/src/codec/tests.rs b/src/codec/tests.rs index ba2d0527..93770a6f 100644 --- a/src/codec/tests.rs +++ b/src/codec/tests.rs @@ -315,3 +315,15 @@ fn mysql_decode_produces_zero_copy_payload() { } mod property; + +/// The inherent `max_frame_length` shadows the `FrameCodec` trait method in +/// direct calls, so the trait impl is only observable through trait dispatch. +#[test] +fn frame_codec_trait_max_frame_length_matches_configuration() { + let codec = LengthDelimitedFrameCodec::new(256); + // UFCS resolves to the trait method rather than the inherent one. + assert_eq!( + ::max_frame_length(&codec), + 256, + ); +} diff --git a/tests/extractor.rs b/tests/extractor.rs index e8a0ff76..2b57d5e6 100644 --- a/tests/extractor.rs +++ b/tests/extractor.rs @@ -90,3 +90,15 @@ fn shared_state_missing_error(request: MessageRequest, mut empty_payload: Payloa _ => panic!("unexpected error"), } } + +/// `take_body_stream` yields the stream exactly once; the second call must +/// return `None`, guarding against a mutant that always reports `None`. +#[test] +fn take_body_stream_returns_stream_once() { + let mut req = MessageRequest::default(); + let (_tx, stream) = wireframe::request::body_channel(4); + req.set_body_stream(stream); + + assert!(req.take_body_stream().is_some(), "first take should yield the stream"); + assert!(req.take_body_stream().is_none(), "second take should be empty"); +} diff --git a/tests/middleware.rs b/tests/middleware.rs index 802217e9..9666e0f7 100644 --- a/tests/middleware.rs +++ b/tests/middleware.rs @@ -74,3 +74,11 @@ fn service_request_setter_updates_correlation_id() { let _ = req.set_correlation_id(None); assert_eq!(req.correlation_id(), None); } + +/// The `frame` accessor has no production callers, so a mutant returning a +/// leaked constant vector survives unless asserted directly. +#[test] +fn service_request_frame_borrows_original_bytes() { + let req = ServiceRequest::new(vec![1, 2, 3], None); + assert_eq!(req.frame(), &[1, 2, 3]); +} diff --git a/tests/serializer.rs b/tests/serializer.rs new file mode 100644 index 00000000..8a025942 --- /dev/null +++ b/tests/serializer.rs @@ -0,0 +1,13 @@ +//! Tests for serializer configuration accessors. +//! +//! The `should_deserialize_after_parse` override on [`BincodeSerializer`] is +//! consumed by the inbound handler but never asserted, so a mutant forcing it +//! to the trait default (`true`) survives without a direct check. +#![cfg(not(loom))] + +use wireframe::serializer::{BincodeSerializer, Serializer}; + +#[test] +fn bincode_serializer_does_not_deserialize_after_parse() { + assert!(!BincodeSerializer.should_deserialize_after_parse()); +} diff --git a/tests/wireframe_protocol.rs b/tests/wireframe_protocol.rs index 5532269a..94d99b6d 100644 --- a/tests/wireframe_protocol.rs +++ b/tests/wireframe_protocol.rs @@ -220,3 +220,44 @@ async fn connection_actor_uses_protocol_from_builder(queues: QueueResult) -> Tes ); Ok(()) } + +/// A no-op assembler used purely to exercise the `message_assembler` accessor. +struct DemoAssembler; + +impl wireframe::message_assembler::MessageAssembler for DemoAssembler { + fn parse_frame_header( + &self, + _payload: &[u8], + ) -> Result { + Err(io::Error::new(io::ErrorKind::InvalidData, "unimplemented")) + } +} + +#[rstest] +fn protocol_accessor_reflects_installation() -> TestResult<()> { + // A bare builder installs no protocol; the accessor must report `None` + // rather than a constant, and `Some` once one is installed. + let bare = TestApp::new()?; + assert!(bare.protocol().is_none(), "bare builder should have no protocol"); + + let counter = Arc::new(AtomicUsize::new(0)); + let app = TestApp::new()?.with_protocol(TestProtocol { counter }); + assert!(app.protocol().is_some(), "installed protocol should be visible"); + Ok(()) +} + +#[rstest] +fn message_assembler_accessor_reflects_installation() -> TestResult<()> { + let bare = TestApp::new()?; + assert!( + bare.message_assembler().is_none(), + "bare builder should have no assembler" + ); + + let app = TestApp::new()?.with_message_assembler(DemoAssembler); + assert!( + app.message_assembler().is_some(), + "installed assembler should be visible" + ); + Ok(()) +} From 7c67315c4362d124a33a713fe8840882860f95a2 Mon Sep 17 00:00:00 2001 From: leynos Date: Thu, 16 Jul 2026 20:15:25 +0200 Subject: [PATCH 04/15] Assert state predicates in both polarities Several state predicates were asserted in only one polarity, so forcing them to a constant survived the suite. Assert a plain Envelope reports is_stream_terminator() == false, that a response stream is not terminated mid-stream, and that the actor state reports is_shutting_down() == true during shutdown (via a new ActorStateHarness::start_shutdown helper). Addresses #569. --- src/client/tests/streaming.rs | 4 ++++ src/connection/test_support.rs | 3 +++ tests/connection.rs | 15 +++++++++++++++ tests/packet_parts.rs | 9 +++++++++ 4 files changed, 31 insertions(+) diff --git a/src/client/tests/streaming.rs b/src/client/tests/streaming.rs index 7171865a..4285f5ad 100644 --- a/src/client/tests/streaming.rs +++ b/src/client/tests/streaming.rs @@ -99,6 +99,10 @@ async fn response_stream_terminates_on_terminator( assert!(first.is_some(), "should yield one data frame"); assert!(first.expect("some").is_ok(), "data frame should be Ok"); + // Mid-stream, before the terminator is polled, the stream must report + // that it has not terminated (guards the `is_terminated` false polarity). + assert!(!stream.is_terminated(), "stream should not be terminated mid-stream"); + // Second poll returns None (terminator consumed). let second = stream.next().await; assert!(second.is_none(), "stream should terminate after terminator"); diff --git a/src/connection/test_support.rs b/src/connection/test_support.rs index 82c8f7ea..eae07e33 100644 --- a/src/connection/test_support.rs +++ b/src/connection/test_support.rs @@ -213,6 +213,9 @@ impl ActorStateHarness { /// Mark a source as closed. pub fn mark_closed(&mut self) { self.state.mark_closed(); } + /// Begin shutdown, transitioning an active state to shutting-down. + pub fn start_shutdown(&mut self) { self.state.start_shutdown(); } + /// Observe the current state snapshot. #[must_use] pub fn snapshot(&self) -> ActorStateSnapshot { diff --git a/tests/connection.rs b/tests/connection.rs index 98fe4a42..f8d70013 100644 --- a/tests/connection.rs +++ b/tests/connection.rs @@ -519,6 +519,21 @@ async fn poll_queue_returns_none_after_close() { assert!(value.is_none()); } +#[test] +fn actor_state_reports_shutting_down_after_start_shutdown() { + // `is_shutting_down` is only ever asserted false elsewhere; drive the + // transition and assert the positive polarity so a constant-`false` + // mutant cannot survive. + let mut harness = ActorStateHarness::new(false, false); + assert!(!harness.snapshot().is_shutting_down, "should start active"); + + harness.start_shutdown(); + let snapshot = harness.snapshot(); + assert!(snapshot.is_shutting_down, "should be shutting down"); + assert!(!snapshot.is_active, "shutting down is not active"); + assert!(!snapshot.is_done, "shutting down is not done"); +} + #[rstest( has_multi, expected_marks, diff --git a/tests/packet_parts.rs b/tests/packet_parts.rs index a1179c94..0f2c2d47 100644 --- a/tests/packet_parts.rs +++ b/tests/packet_parts.rs @@ -3,6 +3,15 @@ use wireframe::app::{Envelope, Packet, PacketParts}; +#[test] +fn plain_envelope_is_not_a_stream_terminator() { + // `Envelope` does not override the `Packet::is_stream_terminator` default, + // so a plain data envelope must report `false`. Terminator tests all use + // types that override the method, leaving the default's false case unguarded. + let env = Envelope::new(1, None, vec![42]); + assert!(!env.is_stream_terminator()); +} + #[test] fn envelope_from_parts_round_trip() { let env = Envelope::new(2, Some(5), vec![1, 2]); From 4e1a72e1f36d4ad6513182d0b6c36a96e23f9b17 Mon Sep 17 00:00:00 2001 From: leynos Date: Thu, 16 Jul 2026 20:24:23 +0200 Subject: [PATCH 05/15] Annotate equivalent mutants with mutants::skip Nine surviving mutants are equivalent: they cannot change observable behaviour, so no test can kill them. Add the mutants dev-dependency and mark each with #[cfg_attr(test, mutants::skip)] plus a one-line justification, keeping them out of future survivor reports without a production dependency. Addresses #566, #595, and the worklist equivalent-mutant list. --- Cargo.lock | 7 +++++++ Cargo.toml | 3 +++ src/client/connect_parts.rs | 3 +++ src/client/pool/slot.rs | 6 ++++++ src/client/runtime.rs | 3 +++ src/message_assembler/budget.rs | 4 ++++ src/push/queues/mod.rs | 3 +++ src/request/mod.rs | 4 ++++ src/server/runtime/backoff.rs | 3 +++ src/session.rs | 5 +++++ 10 files changed, 41 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index cd9e640c..4d11906e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1621,6 +1621,12 @@ dependencies = [ "syn 2.0.117", ] +[[package]] +name = "mutants" +version = "0.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "add0ac067452ff1aca8c5002111bd6b1c895baee6e45fcbc44e0193aea17be56" + [[package]] name = "newt-hype" version = "0.2.0" @@ -3641,6 +3647,7 @@ dependencies = [ "metrics-exporter-prometheus", "metrics-util", "mockall", + "mutants", "pretty_assertions", "proptest", "rstest", diff --git a/Cargo.toml b/Cargo.toml index 823c08fe..5212932a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -60,6 +60,9 @@ rstest-bdd-macros = { version = "0.5.0", features = ["strict-compile-time-valida wireframe = { path = ".", features = ["test-support", "pool", "testkit"] } wireframe_testing = { path = "./wireframe_testing" } logtest = "2.0.0" +# No-op decorator attributes recognised by cargo-mutants; used only under +# cfg(test) to annotate equivalent mutants, so no production dependency. +mutants = "0.0.4" proptest = "1.7.0" googletest = "0.14.3" pretty_assertions = "1.4.1" diff --git a/src/client/connect_parts.rs b/src/client/connect_parts.rs index 83bd2eeb..2aaac0a3 100644 --- a/src/client/connect_parts.rs +++ b/src/client/connect_parts.rs @@ -20,6 +20,9 @@ use super::{ }; use crate::{rewind_stream::RewindStream, serializer::Serializer}; +// Equivalent mutant: a pre-allocation hint later min'd against the codec's +// max_frame_length, so `*`→`+`/`/` cannot change observable behaviour. +#[cfg_attr(test, mutants::skip)] const INITIAL_READ_BUFFER_CAPACITY: usize = 64 * 1024; /// Cloneable connection recipe used by pooled reconnect paths. diff --git a/src/client/pool/slot.rs b/src/client/pool/slot.rs index a1ad404f..f08e192e 100644 --- a/src/client/pool/slot.rs +++ b/src/client/pool/slot.rs @@ -47,6 +47,9 @@ where } } + // Equivalent mutant: the synchronous fast path; forcing `None` merely + // defers to the async `acquire_permit`, which yields an identical lease. + #[cfg_attr(test, mutants::skip)] pub(crate) fn try_acquire_permit(self: &Arc) -> Option { self.permits.clone().try_acquire_owned().ok() } @@ -95,6 +98,9 @@ where .is_some_and(|returned_at| returned_at.elapsed() >= self.idle_timeout) } + // Equivalent mutant: the cleared value is overwritten by SlotConnection's + // drop before any subsequent read, so an empty body is indistinguishable. + #[cfg_attr(test, mutants::skip)] fn clear_last_returned_at(&self) { *self.lock_last_returned_at() = None; } fn lock_last_returned_at(&self) -> MutexGuard<'_, Option> { diff --git a/src/client/runtime.rs b/src/client/runtime.rs index 83cd87dc..dde66d39 100644 --- a/src/client/runtime.rs +++ b/src/client/runtime.rs @@ -102,6 +102,9 @@ impl WireframeClient { /// # Ok(()) /// # } /// ``` + // Equivalent mutant: `WireframeClientBuilder::new()` is literally what its + // `Default` impl returns, so `builder`→`Default::default()` is a no-op. + #[cfg_attr(test, mutants::skip)] #[must_use] pub fn builder() -> WireframeClientBuilder { WireframeClientBuilder::new() diff --git a/src/message_assembler/budget.rs b/src/message_assembler/budget.rs index 0d98e6a2..e9a3dcca 100644 --- a/src/message_assembler/budget.rs +++ b/src/message_assembler/budget.rs @@ -21,6 +21,10 @@ pub(super) struct AggregateBudgets { impl AggregateBudgets { /// Returns `true` when at least one aggregate budget limit is configured. + // Equivalent mutant (`is_enabled` → `true`): a hot-path optimisation only; + // `check_aggregate_budgets` returns `Ok(())` when both budgets are `None`, + // so forcing the gate on changes nothing observable. + #[cfg_attr(test, mutants::skip)] pub(super) const fn is_enabled(&self) -> bool { self.connection.is_some() || self.in_flight.is_some() } diff --git a/src/push/queues/mod.rs b/src/push/queues/mod.rs index 36db7c53..4e9feb61 100644 --- a/src/push/queues/mod.rs +++ b/src/push/queues/mod.rs @@ -113,6 +113,9 @@ impl PushQueueConfig { impl PushQueues { /// Start building a new set of push queues. + // Equivalent mutant: `from(Default::default())` reduces to the same value + // via the blanket `From for T` identity, so it is indistinguishable. + #[cfg_attr(test, mutants::skip)] #[must_use] pub fn builder() -> PushQueuesBuilder { PushQueuesBuilder::default() } diff --git a/src/request/mod.rs b/src/request/mod.rs index 331a0ce7..a9fdd73b 100644 --- a/src/request/mod.rs +++ b/src/request/mod.rs @@ -356,6 +356,10 @@ impl RequestParts { self } + // Equivalent mutant (`found != expected` guard → `true`): when the ids are + // equal the data path is identical to the mismatch arm bar a spurious + // `warn!`, so the returned correlation is unchanged. + #[cfg_attr(test, mutants::skip)] #[inline] fn select_correlation(current: Option, source: Option) -> CorrelationResult { match (current, source) { diff --git a/src/server/runtime/backoff.rs b/src/server/runtime/backoff.rs index 2656cce9..1b4e990c 100644 --- a/src/server/runtime/backoff.rs +++ b/src/server/runtime/backoff.rs @@ -53,6 +53,9 @@ impl BackoffConfig { /// assert_eq!(normalized.initial_delay, Duration::from_millis(1)); /// assert_eq!(normalized.max_delay, Duration::from_millis(5)); /// ``` + // Equivalent mutant (`initial_delay > max_delay` → `>=`): when the two + // durations are equal the swap moves identical values, a no-op. + #[cfg_attr(test, mutants::skip)] #[must_use] pub fn normalized(mut self) -> Self { self.initial_delay = self.initial_delay.max(Duration::from_millis(1)); diff --git a/src/session.rs b/src/session.rs index acaf9f0c..160e7efe 100644 --- a/src/session.rs +++ b/src/session.rs @@ -93,6 +93,11 @@ impl SessionRegistry { /// registry.insert(id, &handle); /// assert!(registry.get(&id).is_some()); /// ``` + // Equivalent mutant (`strong_count() == 0` → `!=`): assessed in #566. The + // opportunistic removal only runs once the handle has already failed to + // upgrade, so `get` returns `None` regardless; the sole difference is a + // lingering dead entry that later pruning reclaims. + #[cfg_attr(test, mutants::skip)] pub fn get(&self, id: &ConnectionId) -> Option> { let guard = self.0.get(id); let handle = guard.as_ref().and_then(|weak| weak.upgrade()); From 20f3a40335f58eeafff9c57f937224c20d196db5 Mon Sep 17 00:00:00 2001 From: leynos Date: Thu, 16 Jul 2026 20:24:23 +0200 Subject: [PATCH 06/15] Assert message-assembler boundary and wall-clock purge The budget and size-limit checks were only asserted strictly over their limits, and the wall-clock purge_expired() was never called. Add exact-fit assertions for the connection budget, the per-message size limit, and a first frame declaring exactly the limit, plus a zero-timeout test that drives the no-argument purge_expired(). Addresses #595. --- .../budget_enforcement_tests.rs | 61 +++++++++++++++++++ src/message_assembler/budget_tests.rs | 14 +++++ 2 files changed, 75 insertions(+) diff --git a/src/message_assembler/budget_enforcement_tests.rs b/src/message_assembler/budget_enforcement_tests.rs index b09c3d86..6048c746 100644 --- a/src/message_assembler/budget_enforcement_tests.rs +++ b/src/message_assembler/budget_enforcement_tests.rs @@ -12,6 +12,7 @@ use super::{ nz, submit_first, submit_first_at, + submit_first_with_total, unbounded_state, }; use crate::message_assembler::{ @@ -303,3 +304,63 @@ fn single_frame_message_not_subject_to_aggregate_budgets( // Buffered bytes unchanged (single-frame was never buffered) assert_eq!(state.total_buffered_bytes(), 19); } + +// ============================================================================= +// Boundary-equality cases (`>` must not become `>=`) +// ============================================================================= + +/// A first frame whose body exactly fills the connection budget must be +/// accepted; the guard is `new_total > limit`, so a `>=` mutant would wrongly +/// reject the exact-fit case. +#[rstest] +fn connection_budget_accepts_exact_fit( + #[from(connection_budgeted_state)] mut state: MessageAssemblyState, +) { + submit_first(&mut state, 1, &[0u8; 20], false).expect("exact-fit frame accepted"); + assert_eq!(state.buffered_count(), 1); + assert_eq!(state.total_buffered_bytes(), 20); +} + +/// A message that reassembles to exactly the per-message size limit must +/// complete; the size guard is `new_len > max`, so a `>=` mutant would reject +/// the exact-limit assembly. +#[test] +fn size_limit_accepts_exact_total() { + let mut state = MessageAssemblyState::new(nz(10), Duration::from_secs(30)); + submit_first(&mut state, 1, &[0u8; 5], false).expect("first frame within limit"); + + let cont = continuation_header(1, 1, 5, true); + let msg = state + .accept_continuation_frame(&cont, &[0u8; 5]) + .expect("continuation accepted") + .expect("message completes at exact limit"); + assert_eq!(msg.body().len(), 10); +} + +/// A first frame declaring a total body length equal to the per-message limit +/// must pass early validation; `accept_first_frame`'s guard is +/// `total_message_size > max`, so a `>=` mutant would reject the exact case. +#[test] +fn declared_total_at_size_limit_is_accepted() { + let mut state = MessageAssemblyState::new(nz(10), Duration::from_secs(30)); + submit_first_with_total(&mut state, 1, &[], 10).expect("declared exact total accepted"); +} + +// ============================================================================= +// Wall-clock purge (no-argument variant) +// ============================================================================= + +/// The no-argument `purge_expired()` reads the wall clock; with a zero timeout +/// a buffered assembly is immediately expired, so it must return the evicted +/// key and empty the state (guards the `-> vec![]` mutant). +#[test] +fn wall_clock_purge_evicts_expired_assembly() { + let mut state = MessageAssemblyState::new(nz(1024), Duration::ZERO); + submit_first(&mut state, 7, &[0u8; 5], false).expect("buffered first frame"); + assert_eq!(state.buffered_count(), 1); + + let evicted = state.purge_expired(); + assert_eq!(evicted, vec![MessageKey(7)]); + assert_eq!(state.buffered_count(), 0); + assert_eq!(state.total_buffered_bytes(), 0); +} diff --git a/src/message_assembler/budget_tests.rs b/src/message_assembler/budget_tests.rs index 604f54ad..f34f8ed8 100644 --- a/src/message_assembler/budget_tests.rs +++ b/src/message_assembler/budget_tests.rs @@ -52,6 +52,20 @@ fn submit_first( state.accept_first_frame(input) } +/// Submit a first frame that declares a `total` body length up front, +/// exercising the early size-limit validation on the first frame. +fn submit_first_with_total( + state: &mut MessageAssemblyState, + key: u64, + body: &[u8], + total: usize, +) -> Result, MessageAssemblyError> { + let header = + crate::message_assembler::test_helpers::first_header_with_total(key, body.len(), total); + let input = FirstFrameInput::new(&header, EnvelopeRouting::default(), vec![], body).unwrap(); + state.accept_first_frame(input) +} + /// Submit a first frame at a specific timestamp. fn submit_first_at( state: &mut MessageAssemblyState, From e6682f5ed3996dc7275635047197bb5f21559231 Mon Sep 17 00:00:00 2001 From: leynos Date: Thu, 16 Jul 2026 20:24:23 +0200 Subject: [PATCH 07/15] Assert send-streaming config accessors SendStreamingConfig's chunk_size and timeout getters have no production callers, so their accessor mutants survived. Assert both reflect the builder and default to None. Addresses #593 and #570. --- src/client/tests/send_streaming.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/client/tests/send_streaming.rs b/src/client/tests/send_streaming.rs index b8ff0cb1..19f5ac9f 100644 --- a/src/client/tests/send_streaming.rs +++ b/src/client/tests/send_streaming.rs @@ -385,3 +385,19 @@ async fn reports_frames_sent(protocol_header: Vec) -> TestResult { Ok(()) } + +/// The `chunk_size` getter has no production callers (internal code reads the +/// field directly), so its accessor mutants survive unless asserted directly. +/// The neighbouring `timeout` getter is guarded the same way. +#[test] +fn config_accessors_reflect_builder() { + let default = SendStreamingConfig::default(); + assert_eq!(default.chunk_size(), None, "default chunk size is unset"); + assert_eq!(default.timeout(), None, "default timeout is unset"); + + let configured = SendStreamingConfig::default() + .with_chunk_size(4096) + .with_timeout(Duration::from_secs(3)); + assert_eq!(configured.chunk_size(), Some(4096)); + assert_eq!(configured.timeout(), Some(Duration::from_secs(3))); +} From bdf2dd755feb02ae802e2bd394817188ef40e26c Mon Sep 17 00:00:00 2001 From: leynos Date: Thu, 16 Jul 2026 20:24:23 +0200 Subject: [PATCH 08/15] Apply rustfmt to newly added tests Reflow the assertions added in the preceding survivor-kill commits to satisfy the formatting gate. --- src/client/tests/streaming.rs | 5 ++++- tests/extractor.rs | 10 ++++++++-- tests/wireframe_protocol.rs | 10 ++++++++-- 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/client/tests/streaming.rs b/src/client/tests/streaming.rs index 4285f5ad..0c7cfb5d 100644 --- a/src/client/tests/streaming.rs +++ b/src/client/tests/streaming.rs @@ -101,7 +101,10 @@ async fn response_stream_terminates_on_terminator( // Mid-stream, before the terminator is polled, the stream must report // that it has not terminated (guards the `is_terminated` false polarity). - assert!(!stream.is_terminated(), "stream should not be terminated mid-stream"); + assert!( + !stream.is_terminated(), + "stream should not be terminated mid-stream" + ); // Second poll returns None (terminator consumed). let second = stream.next().await; diff --git a/tests/extractor.rs b/tests/extractor.rs index 2b57d5e6..b82bf722 100644 --- a/tests/extractor.rs +++ b/tests/extractor.rs @@ -99,6 +99,12 @@ fn take_body_stream_returns_stream_once() { let (_tx, stream) = wireframe::request::body_channel(4); req.set_body_stream(stream); - assert!(req.take_body_stream().is_some(), "first take should yield the stream"); - assert!(req.take_body_stream().is_none(), "second take should be empty"); + assert!( + req.take_body_stream().is_some(), + "first take should yield the stream" + ); + assert!( + req.take_body_stream().is_none(), + "second take should be empty" + ); } diff --git a/tests/wireframe_protocol.rs b/tests/wireframe_protocol.rs index 94d99b6d..9f0688de 100644 --- a/tests/wireframe_protocol.rs +++ b/tests/wireframe_protocol.rs @@ -238,11 +238,17 @@ fn protocol_accessor_reflects_installation() -> TestResult<()> { // A bare builder installs no protocol; the accessor must report `None` // rather than a constant, and `Some` once one is installed. let bare = TestApp::new()?; - assert!(bare.protocol().is_none(), "bare builder should have no protocol"); + assert!( + bare.protocol().is_none(), + "bare builder should have no protocol" + ); let counter = Arc::new(AtomicUsize::new(0)); let app = TestApp::new()?.with_protocol(TestProtocol { counter }); - assert!(app.protocol().is_some(), "installed protocol should be visible"); + assert!( + app.protocol().is_some(), + "installed protocol should be visible" + ); Ok(()) } From e6233f28b941ac3b668ceac337bc1e51c8df63b7 Mon Sep 17 00:00:00 2001 From: leynos Date: Thu, 16 Jul 2026 20:25:40 +0200 Subject: [PATCH 09/15] Annotate multi-packet stamping predicate as equivalent is_stamping_enabled's false polarity is unreachable during frame processing: the sole MultiPacketContext construction site always installs the channel with install(Some(rx), _), which yields Enabled, so a Disabled stamp never coexists with an active channel. The survivor is equivalent, not a test gap; #569's proposed disabled-stamping test cannot be constructed through the actor API. Mark it accordingly. Addresses #569. --- src/connection/multi_packet.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/connection/multi_packet.rs b/src/connection/multi_packet.rs index c3a5de09..8abb85dd 100644 --- a/src/connection/multi_packet.rs +++ b/src/connection/multi_packet.rs @@ -78,6 +78,12 @@ impl MultiPacketContext { pub(super) fn channel_mut(&mut self) -> Option<&mut mpsc::Receiver> { self.channel.as_mut() } /// Returns `true` if correlation stamping is enabled. + // Equivalent mutant (`is_stamping_enabled` → `true`): the only construction + // site installs the channel via `install(Some(rx), _)`, which always yields + // `Enabled`, so a `Disabled` stamp never coexists with an active channel. + // Every reachable call during frame processing therefore already returns + // `true`; forcing it changes nothing. See #569. + #[cfg_attr(test, mutants::skip)] pub(super) fn is_stamping_enabled(&self) -> bool { matches!(self.stamp, MultiPacketStamp::Enabled(_)) } From d88af8910ec13e2f06510880491e201ae0477877 Mon Sep 17 00:00:00 2001 From: leynos Date: Thu, 16 Jul 2026 20:49:46 +0200 Subject: [PATCH 10/15] Fix clippy violations in new survivor-kill tests Drop the Result return from the two accessor tests (panic_in_result_fn) and replace an unwrap with expect in the declared-total submission helper (unwrap_used). --- src/message_assembler/budget_tests.rs | 3 ++- tests/wireframe_protocol.rs | 18 ++++++++++-------- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/src/message_assembler/budget_tests.rs b/src/message_assembler/budget_tests.rs index f34f8ed8..5b2682b0 100644 --- a/src/message_assembler/budget_tests.rs +++ b/src/message_assembler/budget_tests.rs @@ -62,7 +62,8 @@ fn submit_first_with_total( ) -> Result, MessageAssemblyError> { let header = crate::message_assembler::test_helpers::first_header_with_total(key, body.len(), total); - let input = FirstFrameInput::new(&header, EnvelopeRouting::default(), vec![], body).unwrap(); + let input = FirstFrameInput::new(&header, EnvelopeRouting::default(), vec![], body) + .expect("valid first-frame input"); state.accept_first_frame(input) } diff --git a/tests/wireframe_protocol.rs b/tests/wireframe_protocol.rs index 9f0688de..5414baa5 100644 --- a/tests/wireframe_protocol.rs +++ b/tests/wireframe_protocol.rs @@ -234,36 +234,38 @@ impl wireframe::message_assembler::MessageAssembler for DemoAssembler { } #[rstest] -fn protocol_accessor_reflects_installation() -> TestResult<()> { +fn protocol_accessor_reflects_installation() { // A bare builder installs no protocol; the accessor must report `None` // rather than a constant, and `Some` once one is installed. - let bare = TestApp::new()?; + let bare = TestApp::new().expect("builder"); assert!( bare.protocol().is_none(), "bare builder should have no protocol" ); let counter = Arc::new(AtomicUsize::new(0)); - let app = TestApp::new()?.with_protocol(TestProtocol { counter }); + let app = TestApp::new() + .expect("builder") + .with_protocol(TestProtocol { counter }); assert!( app.protocol().is_some(), "installed protocol should be visible" ); - Ok(()) } #[rstest] -fn message_assembler_accessor_reflects_installation() -> TestResult<()> { - let bare = TestApp::new()?; +fn message_assembler_accessor_reflects_installation() { + let bare = TestApp::new().expect("builder"); assert!( bare.message_assembler().is_none(), "bare builder should have no assembler" ); - let app = TestApp::new()?.with_message_assembler(DemoAssembler); + let app = TestApp::new() + .expect("builder") + .with_message_assembler(DemoAssembler); assert!( app.message_assembler().is_some(), "installed assembler should be visible" ); - Ok(()) } From 57fce33dca61b4df42f47b29f26ebd010a327499 Mon Sep 17 00:00:00 2001 From: leynos Date: Thu, 16 Jul 2026 20:55:47 +0200 Subject: [PATCH 11/15] Fix whitaker violations in new survivor-kill tests Move the send-streaming config accessor test into its own module so send_streaming.rs stays under the 400-line cap, and inline the declared-total submission into its test function so the expect call lives inside a test rather than a plain helper. --- src/client/tests/mod.rs | 1 + src/client/tests/send_streaming.rs | 16 -------------- src/client/tests/send_streaming_config.rs | 21 +++++++++++++++++++ .../budget_enforcement_tests.rs | 12 ++++++++--- src/message_assembler/budget_tests.rs | 15 ------------- 5 files changed, 31 insertions(+), 34 deletions(-) create mode 100644 src/client/tests/send_streaming_config.rs diff --git a/src/client/tests/mod.rs b/src/client/tests/mod.rs index 7e51c164..bb9379a1 100644 --- a/src/client/tests/mod.rs +++ b/src/client/tests/mod.rs @@ -10,6 +10,7 @@ mod pool; mod pool_handle; mod request_hooks; mod send_streaming; +mod send_streaming_config; mod send_streaming_infra; mod streaming; mod streaming_helpers; diff --git a/src/client/tests/send_streaming.rs b/src/client/tests/send_streaming.rs index 19f5ac9f..b8ff0cb1 100644 --- a/src/client/tests/send_streaming.rs +++ b/src/client/tests/send_streaming.rs @@ -385,19 +385,3 @@ async fn reports_frames_sent(protocol_header: Vec) -> TestResult { Ok(()) } - -/// The `chunk_size` getter has no production callers (internal code reads the -/// field directly), so its accessor mutants survive unless asserted directly. -/// The neighbouring `timeout` getter is guarded the same way. -#[test] -fn config_accessors_reflect_builder() { - let default = SendStreamingConfig::default(); - assert_eq!(default.chunk_size(), None, "default chunk size is unset"); - assert_eq!(default.timeout(), None, "default timeout is unset"); - - let configured = SendStreamingConfig::default() - .with_chunk_size(4096) - .with_timeout(Duration::from_secs(3)); - assert_eq!(configured.chunk_size(), Some(4096)); - assert_eq!(configured.timeout(), Some(Duration::from_secs(3))); -} diff --git a/src/client/tests/send_streaming_config.rs b/src/client/tests/send_streaming_config.rs new file mode 100644 index 00000000..5f9e124d --- /dev/null +++ b/src/client/tests/send_streaming_config.rs @@ -0,0 +1,21 @@ +//! Unit tests for [`SendStreamingConfig`] accessors. + +use std::time::Duration; + +use crate::client::SendStreamingConfig; + +/// The `chunk_size` getter has no production callers (internal code reads the +/// field directly), so its accessor mutants survive unless asserted directly. +/// The neighbouring `timeout` getter is guarded the same way. +#[test] +fn config_accessors_reflect_builder() { + let default = SendStreamingConfig::default(); + assert_eq!(default.chunk_size(), None, "default chunk size is unset"); + assert_eq!(default.timeout(), None, "default timeout is unset"); + + let configured = SendStreamingConfig::default() + .with_chunk_size(4096) + .with_timeout(Duration::from_secs(3)); + assert_eq!(configured.chunk_size(), Some(4096)); + assert_eq!(configured.timeout(), Some(Duration::from_secs(3))); +} diff --git a/src/message_assembler/budget_enforcement_tests.rs b/src/message_assembler/budget_enforcement_tests.rs index 6048c746..6deb9fcc 100644 --- a/src/message_assembler/budget_enforcement_tests.rs +++ b/src/message_assembler/budget_enforcement_tests.rs @@ -12,14 +12,15 @@ use super::{ nz, submit_first, submit_first_at, - submit_first_with_total, unbounded_state, }; use crate::message_assembler::{ + EnvelopeRouting, + FirstFrameInput, MessageAssemblyError, MessageAssemblyState, MessageKey, - test_helpers::continuation_header, + test_helpers::{continuation_header, first_header_with_total}, }; // ============================================================================= @@ -343,7 +344,12 @@ fn size_limit_accepts_exact_total() { #[test] fn declared_total_at_size_limit_is_accepted() { let mut state = MessageAssemblyState::new(nz(10), Duration::from_secs(30)); - submit_first_with_total(&mut state, 1, &[], 10).expect("declared exact total accepted"); + let header = first_header_with_total(1, 0, 10); + let input = FirstFrameInput::new(&header, EnvelopeRouting::default(), vec![], &[]) + .expect("valid first-frame input"); + state + .accept_first_frame(input) + .expect("declared exact total accepted"); } // ============================================================================= diff --git a/src/message_assembler/budget_tests.rs b/src/message_assembler/budget_tests.rs index 5b2682b0..604f54ad 100644 --- a/src/message_assembler/budget_tests.rs +++ b/src/message_assembler/budget_tests.rs @@ -52,21 +52,6 @@ fn submit_first( state.accept_first_frame(input) } -/// Submit a first frame that declares a `total` body length up front, -/// exercising the early size-limit validation on the first frame. -fn submit_first_with_total( - state: &mut MessageAssemblyState, - key: u64, - body: &[u8], - total: usize, -) -> Result, MessageAssemblyError> { - let header = - crate::message_assembler::test_helpers::first_header_with_total(key, body.len(), total); - let input = FirstFrameInput::new(&header, EnvelopeRouting::default(), vec![], body) - .expect("valid first-frame input"); - state.accept_first_frame(input) -} - /// Submit a first frame at a specific timestamp. fn submit_first_at( state: &mut MessageAssemblyState, From 56ca4b2ba11383271c547885a8a0234107ddc0ab Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 17 Jul 2026 18:38:32 +0200 Subject: [PATCH 12/15] Replace four equivalence skips with killing tests Review of the survivor-triage PR challenged three mutants::skip annotations as false equivalents: the pool slot's non-blocking permit fast path, the idle-timestamp clear that guards against recycling a freshly created connection, and the session registry's lazy eviction of dead weak entries. Each mutation does change observable behaviour, so drop the skips and add unit tests that kill the mutants directly. Also drop the is_stamping_enabled skip: the disabled polarity is directly assertable at unit level, making a reachability argument unnecessary. Addresses review feedback on the survivor-triage changes for #566, #569, and the worklist equivalents. --- src/client/pool/slot.rs | 94 +++++++++++++++++++++++++++++++--- src/connection/multi_packet.rs | 44 +++++++++++++--- src/session.rs | 37 +++++++++++-- 3 files changed, 158 insertions(+), 17 deletions(-) diff --git a/src/client/pool/slot.rs b/src/client/pool/slot.rs index f08e192e..1c17a3e7 100644 --- a/src/client/pool/slot.rs +++ b/src/client/pool/slot.rs @@ -47,9 +47,6 @@ where } } - // Equivalent mutant: the synchronous fast path; forcing `None` merely - // defers to the async `acquire_permit`, which yields an identical lease. - #[cfg_attr(test, mutants::skip)] pub(crate) fn try_acquire_permit(self: &Arc) -> Option { self.permits.clone().try_acquire_owned().ok() } @@ -98,9 +95,6 @@ where .is_some_and(|returned_at| returned_at.elapsed() >= self.idle_timeout) } - // Equivalent mutant: the cleared value is overwritten by SlotConnection's - // drop before any subsequent read, so an empty body is indistinguishable. - #[cfg_attr(test, mutants::skip)] fn clear_last_returned_at(&self) { *self.lock_last_returned_at() = None; } fn lock_last_returned_at(&self) -> MutexGuard<'_, Option> { @@ -154,3 +148,91 @@ where } } } + +#[cfg(test)] +mod tests { + //! Unit tests for slot admission permits and idle-recycle bookkeeping. + + use std::{net::SocketAddr, sync::Arc, time::Duration}; + + use super::PoolSlot; + use crate::{ + client::{ + ClientCodecConfig, + SocketOptions, + connect_parts::ClientBuildParts, + hooks::{LifecycleHooks, RequestHooks}, + pool::manager::WireframeConnectionManager, + tracing_config::TracingConfig, + }, + serializer::BincodeSerializer, + }; + + /// Build a slot without connecting: `build_unchecked` creates the bb8 + /// pool lazily, and these tests never touch the socket path. + fn test_slot( + max_in_flight: usize, + idle_timeout: Duration, + ) -> Arc> { + let addr: SocketAddr = "127.0.0.1:1".parse().expect("valid address"); + let parts = ClientBuildParts { + serializer: BincodeSerializer, + codec_config: ClientCodecConfig::default(), + socket_options: SocketOptions::default(), + preamble_config: None, + lifecycle_hooks: LifecycleHooks::default(), + request_hooks: RequestHooks::default(), + tracing_config: TracingConfig::default(), + }; + let pool = bb8::Pool::builder() + .max_size(1) + .build_unchecked(WireframeConnectionManager::new(addr, parts)); + Arc::new(PoolSlot::new(pool, max_in_flight, idle_timeout)) + } + + /// The synchronous fast path must hand out a permit while capacity + /// remains and refuse once it is exhausted; forcing `None` would push + /// every acquire onto the waiting path. + #[tokio::test] + async fn try_acquire_permit_reflects_capacity() { + let slot = test_slot(1, Duration::from_secs(30)); + + let held = slot.try_acquire_permit(); + assert!(held.is_some(), "fresh slot must yield an immediate permit"); + assert!( + slot.try_acquire_permit().is_none(), + "exhausted slot must refuse an immediate permit" + ); + + drop(held); + assert!( + slot.try_acquire_permit().is_some(), + "released capacity must be immediately reusable" + ); + } + + /// Clearing the idle timestamp must reset the recycle decision; leaving + /// a stale timestamp would recycle the freshly created connection on the + /// next checkout after a recycle whose lease never sets a new timestamp + /// (for example when the replacement checkout fails). + #[tokio::test] + async fn clear_last_returned_at_resets_recycle_decision() { + let slot = test_slot(1, Duration::from_millis(1)); + + *slot.lock_last_returned_at() = Some(tokio::time::Instant::now() - Duration::from_secs(60)); + assert!( + slot.should_recycle_idle(), + "stale timestamp must trigger a recycle" + ); + + slot.clear_last_returned_at(); + assert!( + slot.lock_last_returned_at().is_none(), + "clear must remove the timestamp" + ); + assert!( + !slot.should_recycle_idle(), + "cleared timestamp must not trigger a recycle" + ); + } +} diff --git a/src/connection/multi_packet.rs b/src/connection/multi_packet.rs index 8abb85dd..861bdb2e 100644 --- a/src/connection/multi_packet.rs +++ b/src/connection/multi_packet.rs @@ -78,12 +78,6 @@ impl MultiPacketContext { pub(super) fn channel_mut(&mut self) -> Option<&mut mpsc::Receiver> { self.channel.as_mut() } /// Returns `true` if correlation stamping is enabled. - // Equivalent mutant (`is_stamping_enabled` → `true`): the only construction - // site installs the channel via `install(Some(rx), _)`, which always yields - // `Enabled`, so a `Disabled` stamp never coexists with an active channel. - // Every reachable call during frame processing therefore already returns - // `true`; forcing it changes nothing. See #569. - #[cfg_attr(test, mutants::skip)] pub(super) fn is_stamping_enabled(&self) -> bool { matches!(self.stamp, MultiPacketStamp::Enabled(_)) } @@ -95,3 +89,41 @@ impl MultiPacketContext { } } } + +#[cfg(test)] +mod tests { + //! Unit tests for the stamping invariant: stamping is enabled exactly + //! when a channel is installed. + + use tokio::sync::mpsc; + + use super::MultiPacketContext; + + #[test] + fn stamping_tracks_channel_installation() { + let mut ctx = MultiPacketContext::::new(); + assert!( + !ctx.is_stamping_enabled(), + "a fresh context must not stamp frames" + ); + + let (_tx, rx) = mpsc::channel(1); + ctx.install(Some(rx), Some(7)); + assert!( + ctx.is_stamping_enabled(), + "installing a channel must enable stamping" + ); + assert_eq!(ctx.correlation_id(), Some(7)); + + ctx.install(None, Some(9)); + assert!( + !ctx.is_stamping_enabled(), + "removing the channel must disable stamping" + ); + assert_eq!( + ctx.correlation_id(), + None, + "a disabled context must not expose a correlation id" + ); + } +} diff --git a/src/session.rs b/src/session.rs index 160e7efe..d527bcc3 100644 --- a/src/session.rs +++ b/src/session.rs @@ -93,11 +93,6 @@ impl SessionRegistry { /// registry.insert(id, &handle); /// assert!(registry.get(&id).is_some()); /// ``` - // Equivalent mutant (`strong_count() == 0` → `!=`): assessed in #566. The - // opportunistic removal only runs once the handle has already failed to - // upgrade, so `get` returns `None` regardless; the sole difference is a - // lingering dead entry that later pruning reclaims. - #[cfg_attr(test, mutants::skip)] pub fn get(&self, id: &ConnectionId) -> Option> { let guard = self.0.get(id); let handle = guard.as_ref().and_then(|weak| weak.upgrade()); @@ -259,3 +254,35 @@ impl SessionRegistry { #[must_use] pub fn active_ids(&self) -> Vec { self.retain_and_collect(|id, _| id) } } + +#[cfg(test)] +mod tests { + //! Unit tests for lazy dead-entry cleanup in the session registry. + + use super::{ConnectionId, SessionRegistry}; + use crate::push::PushQueues; + + /// A failed lookup must also evict the dead weak entry; retaining it + /// grows the registry until a later `prune`, which is the observable + /// difference the `strong_count() == 0` guard protects. + #[tokio::test] + async fn get_evicts_dead_entry_from_registry() { + let registry = SessionRegistry::::default(); + let id = ConnectionId::new(1); + + let (queues, handle) = PushQueues::::builder() + .high_capacity(1) + .low_capacity(1) + .build() + .expect("failed to build PushQueues"); + registry.insert(id, &handle); + drop(handle); + drop(queues); + + assert!(registry.get(&id).is_none(), "dead handle must not upgrade"); + assert!( + registry.0.is_empty(), + "failed lookup must evict the dead entry rather than leave it to prune" + ); + } +} From b0583f5bf66b095929dfed7e351acec94af9022a Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 17 Jul 2026 18:39:51 +0200 Subject: [PATCH 13/15] Document widened mutation excludes and mutants dev-dependency Record in the developers' guide and ADR-007 the eight-shard dispatch fan-out, the added exclude-globs (src/test_helpers/** and src/**/tests.rs, #599), the workflow contract test that pins the caller configuration, and the convention for annotating proven-equivalent mutants with cfg_attr(test, mutants::skip) via the mutants dev-dependency. --- ...007-mutation-testing-with-cargo-mutants.md | 132 ++++++++++-------- docs/developers-guide.md | 75 +++++----- 2 files changed, 111 insertions(+), 96 deletions(-) diff --git a/docs/adr-007-mutation-testing-with-cargo-mutants.md b/docs/adr-007-mutation-testing-with-cargo-mutants.md index 7d0b0123..75d0800a 100644 --- a/docs/adr-007-mutation-testing-with-cargo-mutants.md +++ b/docs/adr-007-mutation-testing-with-cargo-mutants.md @@ -2,23 +2,34 @@ ## Status -Accepted. Amended 2026-07-05: the decision stands, but the bespoke -workflow implementation described in the sketch below has been -superseded — `.github/workflows/mutation-testing.yml` is now a thin -caller of the shared reusable workflow -`leynos/shared-actions/.github/workflows/mutation-cargo.yml` (pinned by -commit SHA), which generalizes this design and adds shard fan-out for -full dispatch runs, a merged cross-shard summary, tested helper -scripts, and a pinned `cargo-mutants` version. The caller enables -`--all-features` (so feature-gated tests run against mutants) and -excludes example/test-support scaffolding, addressing issue #571. The -sketch below is retained as the historical record of the design the -shared workflow was generalized from; the caller-facing contract now -lives in the shared repository's `docs/mutation-cargo-workflow.md`. +Accepted. Amended 2026-07-05: the decision stands, but the bespoke workflow +implementation described in the sketch below has been superseded — +`.github/workflows/mutation-testing.yml` is now a thin caller of the shared +reusable workflow `leynos/shared-actions/.github/workflows/mutation-cargo.yml` +(pinned by commit SHA), which generalizes this design and adds shard fan-out +for full dispatch runs, a merged cross-shard summary, tested helper scripts, +and a pinned `cargo-mutants` version. The caller enables `--all-features` (so +feature-gated tests run against mutants) and excludes example/test-support +scaffolding, addressing issue #571. The sketch below is retained as the +historical record of the design the shared workflow was generalized from; the +caller-facing contract now lives in the shared repository's +`docs/mutation-cargo-workflow.md`. + +Amended 2026-07-17: full dispatch runs now fan out across eight shards (the +first six-shard run lost a leg to the job ceiling), and the exclude-globs +additionally cover `src/test_helpers/**` and `src/**/tests.rs` — the +module-root glob missed the `src/test_helpers/` submodules, and cfg(test)-only +companion `tests.rs` files are invisible to cargo-mutants' test-code detection +(#599). The caller contract (reusable-workflow reference shape, permissions, +triggers, excludes, extra-args, and shard count) is pinned by +`tests/workflow_contracts/mutation_testing_test.py`. Equivalent mutants are +annotated in source with `#[cfg_attr(test, mutants::skip)]` and a justification +comment, using the `mutants` crate of no-op decorator attributes as a +dev-dependency so production builds are unaffected. ## Date -2026-03-31 (amended 2026-07-05). +2026-03-31 (amended 2026-07-05 and 2026-07-17). ## Context and Problem Statement @@ -109,12 +120,11 @@ files changed on `main` in the preceding 25 hours. relies on reflog state, which is not available in fresh CI clones. The window is 25 hours rather than 24 because GitHub cron start times drift (runs routinely begin 15–60 minutes late); the extra hour means a commit - landing just after one run still falls inside the next run's window, at - the cost of occasionally re-testing a file. Files that no longer exist at - the checked-out tip (deleted or renamed within the window) are filtered - out before being passed to `--file`. The result is stored as a step - output (`has_changes=true|false`) and the heavy mutation step is gated - with + landing just after one run still falls inside the next run's window, at the + cost of occasionally re-testing a file. Files that no longer exist at the + checked-out tip (deleted or renamed within the window) are filtered out + before being passed to `--file`. The result is stored as a step output + (`has_changes=true|false`) and the heavy mutation step is gated with `if: steps.detect.outputs.has_changes == 'true'`. Manual `workflow_dispatch` runs bypass the guard by setting `has_changes=true` unconditionally and running a full (unscoped) mutation against both the root crate and @@ -157,10 +167,10 @@ running up to two targeted invocations: 1. **Execution:** Installs `cargo-mutants` via `cargo binstall` (the shared `setup-rust` action provides `cargo-binstall`), then runs the scoped invocations described above with `--in-place` (the upstream CI - recommendation, which reuses the existing build cache instead of copying - the tree) and a per-mutant timeout multiplier to bound execution time. The - job carries an explicit `timeout-minutes` ceiling as a backstop against - runaway full runs. + recommendation, which reuses the existing build cache instead of copying the + tree) and a per-mutant timeout multiplier to bound execution time. The job + carries an explicit `timeout-minutes` ceiling as a backstop against runaway + full runs. 2. **Output locations:** `cargo-mutants` creates `mutants.out/` inside the source directory it operates on. Note that `--output DIR` places `mutants.out/` _within_ `DIR` (it does not rename the directory), so the @@ -170,9 +180,9 @@ running up to two targeted invocations: 3. **Exit-code handling:** `cargo mutants` exits non-zero for informative outcomes as well as genuine failures: `0` all caught, `1` usage error, `2` mutants missed, `3` tests timed out, `4` baseline tests already failing, - `70` internal error. Because this workflow is informational, exit codes - `2` and `3` are treated as success (the survivors are the deliverable, not - a failure), while `1`, `4`, and `70` still fail the job so genuine faults + `70` internal error. Because this workflow is informational, exit codes `2` + and `3` are treated as success (the survivors are the deliverable, not a + failure), while `1`, `4`, and `70` still fail the job so genuine faults remain visible. 4. **Artefact:** Uploads the `mutants.out/` directory (or directories) as GitHub Actions artefacts, preserving `outcomes.json` (machine-readable) and @@ -413,13 +423,13 @@ jobs: semantically equivalent to the original (e.g. replacing `x + 0` with `x`). These require human triage and cannot be eliminated automatically. - **`wireframe_testing` false survivors:** The companion crate's logic is - exercised chiefly by the root crate's test suite; `wireframe_testing` - itself carries only a small number of in-crate tests. Because - `cargo mutants --dir wireframe_testing` runs only that package's own - tests, most mutants there will appear to survive even when the root - crate's tests would catch the fault. The `wireframe_testing` results - table is therefore advisory until the crate gains meaningful in-crate - coverage; treat its survivors with scepticism during triage. + exercised chiefly by the root crate's test suite; `wireframe_testing` itself + carries only a small number of in-crate tests. Because + `cargo mutants --dir wireframe_testing` runs only that package's own tests, + most mutants there will appear to survive even when the root crate's tests + would catch the fault. The `wireframe_testing` results table is therefore + advisory until the crate gains meaningful in-crate coverage; treat its + survivors with scepticism during triage. - **Runner cost:** Scheduled runs consume GitHub Actions minutes. The daily schedule starts every day, but the change-detection guard ensures the expensive mutation step only runs when `main` received relevant Rust source @@ -429,18 +439,17 @@ jobs: timestamps, not author timestamps. Force-pushed or rebased commits may carry original timestamps outside the 25-hour window, and merge commits whose constituent commits were created earlier are similarly invisible (the - repository's squash-merge convention makes this unlikely in practice). - There is also no catch-up: if a scheduled run fails or is skipped, commits - from that window are never mutation-tested. A manual `workflow_dispatch` - run bypasses the guard entirely, providing the fallback in all these - cases. Diffing against the SHA of the last successful run (via the GitHub - API) would close the gap completely but was rejected as disproportionate - for an informational workflow. + repository's squash-merge convention makes this unlikely in practice). There + is also no catch-up: if a scheduled run fails or is skipped, commits from + that window are never mutation-tested. A manual `workflow_dispatch` run + bypasses the guard entirely, providing the fallback in all these cases. + Diffing against the SHA of the last successful run (via the GitHub API) would + close the gap completely but was rejected as disproportionate for an + informational workflow. - **Scheduled-workflow suspension:** GitHub automatically disables cron - triggers on repositories with no activity for 60 days. On a quiet - repository the workflow silently stops running until re-enabled — an - acceptable failure mode, since no code changes means nothing new to - mutate. + triggers on repositories with no activity for 60 days. On a quiet repository + the workflow silently stops running until re-enabled — an acceptable failure + mode, since no code changes means nothing new to mutate. - **Manifest-only changes:** Changes to `Cargo.toml` or `Cargo.lock` without accompanying Rust source changes are not detected. If a manifest change affects mutation outcomes (e.g. enabling a feature that changes conditional @@ -456,9 +465,9 @@ jobs: - Exact `--timeout-multiplier` value — needs calibration against the current test suite runtime. - Whether to adopt `--in-diff` (line-level scoping from a diff) in place of - file-level `--file` scoping for scheduled runs, once the initial workflow - has proven itself. `--in-diff` tests only mutants on changed lines, which - is cheaper but misses mutants elsewhere in a changed file. + file-level `--file` scoping for scheduled runs, once the initial workflow has + proven itself. `--in-diff` tests only mutants on changed lines, which is + cheaper but misses mutants elsewhere in a changed file. - Whether full `workflow_dispatch` runs need `--shard` support to split the work across parallel jobs, should single-job runs approach the `timeout-minutes` ceiling. @@ -480,26 +489,25 @@ jobs: changes landed on `main`, making quiet days a cheap no-op. - **Change detection mechanism:** Uses `git log --since="25 hours ago"` with commit timestamps rather than `origin/main@{25.hours.ago}` (which relies on - reflog state unavailable in fresh CI clones). The window exceeds the - 24-hour cadence by one hour to absorb GitHub cron start-time drift; - occasional double-testing of a file is preferred over silently dropping a - commit. Files no longer present at the checked-out tip are filtered out - before scoping. + reflog state unavailable in fresh CI clones). The window exceeds the 24-hour + cadence by one hour to absorb GitHub cron start-time drift; occasional + double-testing of a file is preferred over silently dropping a commit. Files + no longer present at the checked-out tip are filtered out before scoping. - **Exit-code policy:** `cargo mutants` exit codes `2` (missed mutants) and - `3` (timeouts) are treated as success because the workflow is - informational — survivors are its output, not a failure. Exit codes `1` - (usage error), `4` (baseline tests failing), and `70` (internal error) - still fail the job so genuine faults surface as red runs. + `3` (timeouts) are treated as success because the workflow is informational — + survivors are its output, not a failure. Exit codes `1` (usage error), `4` + (baseline tests failing), and `70` (internal error) still fail the job so + genuine faults surface as red runs. - **Output directories:** The workflow relies on the default output locations (`./mutants.out/` for the root crate, `wireframe_testing/mutants.out/` for the companion crate) because - `--output DIR` creates `mutants.out/` _within_ `DIR` rather than renaming - it, which would otherwise nest the report one level deeper than the - summary and artefact steps expect. + `--output DIR` creates `mutants.out/` _within_ `DIR` rather than renaming it, + which would otherwise nest the report one level deeper than the summary and + artefact steps expect. - **Build strategy:** Runs use `--in-place`, the upstream recommendation for CI, so mutation builds reuse the runner's existing build cache instead of - copying the source tree. The job also sets `timeout-minutes: 300` as a - hard backstop against unbounded full runs. + copying the source tree. The job also sets `timeout-minutes: 300` as a hard + backstop against unbounded full runs. - **Code change definition:** The detection monitors `src/**/*.rs`, `wireframe_testing/src/**/*.rs`, `examples/**/*.rs`, and `benches/**/*.rs`. Manifest-only changes (`Cargo.toml`, `Cargo.lock`) are excluded because diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 1dc5445f..fda632ff 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -178,52 +178,59 @@ continuous integration (CI). Scheduled mutation testing runs in CI via `.github/workflows/mutation-testing.yml` (see -[ADR-007](adr-007-mutation-testing-with-cargo-mutants.md) for the design -and its rationale). Key points for contributors: +[ADR-007](adr-007-mutation-testing-with-cargo-mutants.md) for the design and +its rationale). Key points for contributors: - The workflow is a thin caller of the shared reusable workflow - `leynos/shared-actions/.github/workflows/mutation-cargo.yml` (pinned - by commit SHA; caller guide in that repository's - `docs/mutation-cargo-workflow.md`). It is informational only: it - never gates pull requests, and surviving mutants do not fail the run. - Scheduled runs execute daily, scoped to Rust files changed in the - preceding 25 hours, and skip cheaply when nothing changed. Manual - dispatch (select the branch in the Actions "Run workflow" control) - runs full mutations, fanned out across six shards with one merged - summary. + `leynos/shared-actions/.github/workflows/mutation-cargo.yml` (pinned by + commit SHA; caller guide in that repository's + `docs/mutation-cargo-workflow.md`). It is informational only: it never gates + pull requests, and surviving mutants do not fail the run. Scheduled runs + execute daily, scoped to Rust files changed in the preceding 25 hours, and + skip cheaply when nothing changed. Manual dispatch (select the branch in the + Actions "Run workflow" control) runs full mutations, fanned out across eight + shards with one merged summary. - The caller passes `--all-features` so feature-gated tests (e.g. the - `serializer-serde` bridge round-trips) run against mutants, and - excludes the example/test-support scaffolding - (`src/codec/examples.rs`, `src/test_helpers.rs`, - `src/connection/test_support.rs`) whose survivors are noise — both - per issue #571. + `serializer-serde` bridge round-trips) run against mutants, and excludes the + example/test-support scaffolding whose survivors are noise — both per issue + #571: `src/codec/examples.rs`, `src/test_helpers.rs`, `src/test_helpers/**` + (the module-root glob does not match the directory's submodules), + `src/connection/test_support.rs`, and `src/**/tests.rs` (cfg(test) companion + files that cargo-mutants cannot detect as test code, #599). The contract test + `tests/workflow_contracts/mutation_testing_test.py` (run via + `make test-workflow-contracts`) pins this exclude list, the `--all-features` + extra-args, and the shard count. +- Mutants proven equivalent (incapable of changing observable + behaviour) are annotated in source with `#[cfg_attr(test, mutants::skip)]` + plus a one-line justification. The attribute comes from the + [`mutants`](https://docs.rs/mutants) crate, a dev-dependency of no-op + decorator attributes; keep skips rare, justified, and reserved for stateless + equivalences — prefer a killing test wherever the mutation is observable. - [`cargo-mutants`](https://mutants.rs/) is a CI-runtime dependency - only, installed by the shared workflow at a pinned version; it is not - a Cargo dependency and is not required locally. To reproduce a run - locally, install it with `cargo install cargo-mutants` and run, for - example, + only, installed by the shared workflow at a pinned version; it is not a Cargo + dependency and is not required locally. To reproduce a run locally, install + it with `cargo install cargo-mutants` and run, for example, `cargo mutants --in-place --all-features --file src/frame/mod.rs`. - Results appear in the run's merged job summary (caught/missed/timeout - counts plus a table of surviving mutants per target) and as - downloadable `mutation-report-*` artefacts containing `mutants.out/` - (one per shard on full runs). + counts plus a table of surviving mutants per target) and as downloadable + `mutation-report-*` artefacts containing `mutants.out/` (one per shard on + full runs). - Surviving mutants are a test-improvement backlog: triage them for equivalent mutations (false survivors) before writing tests. Mutants in - `wireframe_testing` are mostly false survivors because that crate's - logic is exercised chiefly by the root crate's suite; treat its table - as advisory. + `wireframe_testing` are mostly false survivors because that crate's logic is + exercised chiefly by the root crate's suite; treat its table as advisory. ## Workflow pins and Dependabot -Dependabot owns the upgrade of GitHub Actions and reusable workflows, -including calls into `leynos/shared-actions`. Contract tests that assert a -caller's exact commit SHA create a lockstep dependency: every time Dependabot -opens a bump PR, the test fails until a human edits the pinned constant to -match. That defeats the purpose of automated dependency updates and turns a -routine bump into a manual chore. +Dependabot owns the upgrade of GitHub Actions and reusable workflows, including +calls into `leynos/shared-actions`. Contract tests that assert a caller's exact +commit SHA create a lockstep dependency: every time Dependabot opens a bump PR, +the test fails until a human edits the pinned constant to match. That defeats +the purpose of automated dependency updates and turns a routine bump into a +manual chore. -Contract tests may still verify the *shape* of a reusable-workflow caller. -They must not verify the specific SHA value. +Contract tests may still verify the *shape* of a reusable-workflow caller. They +must not verify the specific SHA value. - Do assert the workflow references the correct reusable workflow path. - Do assert the ref is pinned to a full 40-character commit SHA, not a From d1edaab0cbabd32b0d9e2d53f313eaebd072650a Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 17 Jul 2026 18:45:52 +0200 Subject: [PATCH 14/15] Satisfy lint rules in slot test helper Use minute-based Duration construction and an infallible SocketAddr literal so the new slot tests pass clippy's duration-unit lint and whitaker's no-expect-outside-tests rule. --- src/client/pool/slot.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/client/pool/slot.rs b/src/client/pool/slot.rs index 1c17a3e7..0a79a974 100644 --- a/src/client/pool/slot.rs +++ b/src/client/pool/slot.rs @@ -174,7 +174,7 @@ mod tests { max_in_flight: usize, idle_timeout: Duration, ) -> Arc> { - let addr: SocketAddr = "127.0.0.1:1".parse().expect("valid address"); + let addr = SocketAddr::from(([127, 0, 0, 1], 1)); let parts = ClientBuildParts { serializer: BincodeSerializer, codec_config: ClientCodecConfig::default(), @@ -219,7 +219,7 @@ mod tests { async fn clear_last_returned_at_resets_recycle_decision() { let slot = test_slot(1, Duration::from_millis(1)); - *slot.lock_last_returned_at() = Some(tokio::time::Instant::now() - Duration::from_secs(60)); + *slot.lock_last_returned_at() = Some(tokio::time::Instant::now() - Duration::from_mins(1)); assert!( slot.should_recycle_idle(), "stale timestamp must trigger a recycle" From 1df84ef92f91c9dc47caea76e4dd282db3c19761 Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 17 Jul 2026 18:47:15 +0200 Subject: [PATCH 15/15] Use Oxford spelling in budget equivalence comment The new typos gate on main enforces en-GB-oxendict, which requires the -ize form. --- src/message_assembler/budget.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/message_assembler/budget.rs b/src/message_assembler/budget.rs index e9a3dcca..02b8f8c6 100644 --- a/src/message_assembler/budget.rs +++ b/src/message_assembler/budget.rs @@ -21,7 +21,7 @@ pub(super) struct AggregateBudgets { impl AggregateBudgets { /// Returns `true` when at least one aggregate budget limit is configured. - // Equivalent mutant (`is_enabled` → `true`): a hot-path optimisation only; + // Equivalent mutant (`is_enabled` → `true`): a hot-path optimization only; // `check_aggregate_budgets` returns `Ok(())` when both budgets are `None`, // so forcing the gate on changes nothing observable. #[cfg_attr(test, mutants::skip)]