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/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/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 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..0a79a974 100644 --- a/src/client/pool/slot.rs +++ b/src/client/pool/slot.rs @@ -148,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::from(([127, 0, 0, 1], 1)); + 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_mins(1)); + 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/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/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_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/client/tests/streaming.rs b/src/client/tests/streaming.rs index 7171865a..0c7cfb5d 100644 --- a/src/client/tests/streaming.rs +++ b/src/client/tests/streaming.rs @@ -99,6 +99,13 @@ 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/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/src/connection/multi_packet.rs b/src/connection/multi_packet.rs index c3a5de09..861bdb2e 100644 --- a/src/connection/multi_packet.rs +++ b/src/connection/multi_packet.rs @@ -89,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/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/src/message_assembler/budget.rs b/src/message_assembler/budget.rs index 0d98e6a2..02b8f8c6 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 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)] pub(super) const fn is_enabled(&self) -> bool { self.connection.is_some() || self.in_flight.is_some() } diff --git a/src/message_assembler/budget_enforcement_tests.rs b/src/message_assembler/budget_enforcement_tests.rs index b09c3d86..6deb9fcc 100644 --- a/src/message_assembler/budget_enforcement_tests.rs +++ b/src/message_assembler/budget_enforcement_tests.rs @@ -15,10 +15,12 @@ use super::{ unbounded_state, }; use crate::message_assembler::{ + EnvelopeRouting, + FirstFrameInput, MessageAssemblyError, MessageAssemblyState, MessageKey, - test_helpers::continuation_header, + test_helpers::{continuation_header, first_header_with_total}, }; // ============================================================================= @@ -303,3 +305,68 @@ 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)); + 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"); +} + +// ============================================================================= +// 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/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..d527bcc3 100644 --- a/src/session.rs +++ b/src/session.rs @@ -254,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" + ); + } +} 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; 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/extractor.rs b/tests/extractor.rs index e8a0ff76..b82bf722 100644 --- a/tests/extractor.rs +++ b/tests/extractor.rs @@ -90,3 +90,21 @@ 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/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]); 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..5414baa5 100644 --- a/tests/wireframe_protocol.rs +++ b/tests/wireframe_protocol.rs @@ -220,3 +220,52 @@ 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() { + // 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().expect("builder"); + assert!( + bare.protocol().is_none(), + "bare builder should have no protocol" + ); + + let counter = Arc::new(AtomicUsize::new(0)); + let app = TestApp::new() + .expect("builder") + .with_protocol(TestProtocol { counter }); + assert!( + app.protocol().is_some(), + "installed protocol should be visible" + ); +} + +#[rstest] +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() + .expect("builder") + .with_message_assembler(DemoAssembler); + assert!( + app.message_assembler().is_some(), + "installed assembler should be visible" + ); +} 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", )