diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e3d9a4d84..58aa99a42 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -33,12 +33,8 @@ jobs: # https://github.com/leynos/wireframe/actions/runs/39038420830). That is # about 29% less per run for 3m40s more wall time. # - # The coverage action supplies a 180-second per-test deadline. The - # aggregate tests/compile_error.rs test builds and executes twelve - # trybuild fixtures under coverage; its cold nested build exceeded that - # deadline even after the ordinary tests completed. .config/nextest.toml - # extends only this test to eight minutes, preserves the ordinary - # deadline, and retains the ten-minute whole-run ceiling. + # Compiler-driving UI fixtures have a separate bounded step. Runtime + # coverage does not start their nested trybuild compilation. # # Disk is the margin to watch: the minimum free space on two vCPU was # 3,998 MiB, against 78,392 MiB on four. @@ -62,14 +58,16 @@ jobs: ${{ (github.event.pull_request.head.repo.fork || github.event.pull_request.user.login == 'dependabot[bot]') && 'ubuntu-latest' || 'ubicloud-standard-2' }} - timeout-minutes: 30 + timeout-minutes: 60 permissions: contents: read env: CARGO_TERM_COLOR: always BUILD_PROFILE: debug steps: - - uses: actions/checkout@v7 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 + with: + persist-credentials: false - name: Setup Rust uses: leynos/shared-actions/.github/actions/setup-rust@8e6a325765ba382691d1b1c65fcccf1fc1d1b00c - name: Install development backend and linker @@ -80,6 +78,10 @@ jobs: sudo apt-get update sudo apt-get install -y mold mold --version + # This compiler-driven suite is bounded separately from runtime coverage. + - name: Validate compile-time APIs + timeout-minutes: 30 + run: make test-ui - name: Install mdtablefix # `make check-fmt` runs `mdtablefix --check --git`; 0.6.0 is the # first release with `--check` and `--git`. diff --git a/.github/workflows/coverage-main.yml b/.github/workflows/coverage-main.yml index 7c92a11d8..0c61d80f8 100644 --- a/.github/workflows/coverage-main.yml +++ b/.github/workflows/coverage-main.yml @@ -32,11 +32,10 @@ jobs: # arm: `push` carries no pull request, so the guard would be constant # here and a constant guard reads as a decision nobody made. # - # Four vCPU, and not measured at two: this lane runs the same - # `generate-coverage` over the same workspace as `ci.yml`, whose - # `build-test` now runs on two vCPU after a warm run passed the trybuild - # case that nextest had killed at 180 s there. This lane's own 240 s - # sample was taken on Namespace's shared profile, which is four vCPU and + # Four vCPU, and not measured at two: this lane runs the same runtime + # coverage as `ci.yml`; compile-time UI checks run in a separate CI step. + # This lane's own 240 s sample was taken on Namespace's shared profile, + # which is four vCPU and # 16 GB, and it has not been sampled on two. It runs on merge pushes # with nobody watching, so moving it needs its own measurement. runs-on: ubicloud-standard-4 diff --git a/Cargo.lock b/Cargo.lock index 0886c4bcc..c879c531e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3621,7 +3621,6 @@ version = "0.1.0" dependencies = [ "rstest", "stateright", - "wireframe", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 658eadcc2..d67e23bd3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,7 +38,7 @@ tokio = { version = "1.47.1", default-features = false, features = [ "time", "io-util", ] } -tokio-util = { version = "0.7.16", features = ["rt", "codec"] } +tokio-util = { version = "0.7.16", features = ["rt", "codec", "io"] } tokio-stream = "0.1.17" futures = "0.3.31" async-trait = "0.1.89" @@ -47,17 +47,17 @@ log = "0.4.28" dashmap = "6.1.0" leaky-bucket = "1.1.2" tracing = { version = "0.1.41", features = ["log", "log-always"] } -tracing-subscriber = "0.3.18" metrics = { version = "0.24.2", optional = true } monotony = "1.0.0" thiserror = "2.0.16" -itoa = "1.0" static_assertions = "1.1.0" derive_more = { version = "2.0.1", features = ["display", "from"] } socket2 = "0.6.0" bb8 = { version = "0.9.1", optional = true } [dev-dependencies] +itoa = "1.0" +tracing-subscriber = { version = "0.3.18", features = ["env-filter"] } rstest = "0.27.0" rstest-bdd = "0.5.0" rstest-bdd-macros = { version = "0.5.0", features = ["strict-compile-time-validation"] } @@ -95,12 +95,12 @@ tokio = { version = "1.47.1", default-features = false, features = [ [target.'cfg(loom)'.dependencies] loom = "0.7.2" -[target.'cfg(not(loom))'.dependencies] -metrics-exporter-prometheus = { version = "0.18.3", optional = true, features = ["http-listener"] } +[target.'cfg(not(loom))'.dev-dependencies] +metrics-exporter-prometheus = { version = "0.18.3", features = ["http-listener"] } [features] default = ["metrics", "serializer-bincode"] -metrics = ["dep:metrics", "dep:metrics-exporter-prometheus"] +metrics = ["dep:metrics"] pool = ["dep:bb8"] serializer-bincode = [] serializer-serde = [] @@ -145,7 +145,7 @@ big_endian_bytes = "deny" unknown_lints = "deny" renamed_and_removed_lints = "deny" missing_docs = "deny" -unexpected_cfgs = { level = "warn", check-cfg = ['cfg(loom)'] } +unexpected_cfgs = { level = "warn", check-cfg = ['cfg(loom)', 'cfg(coverage)'] } [workspace.lints.rustdoc] missing_crate_level_docs = "deny" diff --git a/crates/wireframe-verification/Cargo.toml b/crates/wireframe-verification/Cargo.toml index 1b02d1c06..9ec995183 100644 --- a/crates/wireframe-verification/Cargo.toml +++ b/crates/wireframe-verification/Cargo.toml @@ -9,7 +9,6 @@ workspace = true [dependencies] stateright = "0.31.0" -wireframe = { path = "../..", features = ["test-support"] } [dev-dependencies] rstest = "0.27.0" diff --git a/docs/developers-guide.md b/docs/developers-guide.md index f07ff5f47..65143abaa 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -11,13 +11,19 @@ than all four repository-owned lanes sharing one profile. | Workflow | Job | Trigger | Runner | Ceiling | | ------------------------ | ------------------- | ------------------ | --------------------- | ------- | -| `ci.yml` | `build-test` | pull request, push | `ubicloud-standard-2` | 30 min | +| `ci.yml` | `build-test` | pull request, push | `ubicloud-standard-2` | 60 min | | `coverage-main.yml` | `coverage-upload` | push | `ubicloud-standard-4` | 20 min | | `advanced-tests.yml` | `advanced` | schedule | `ubuntu-latest` | 60 min | | `delayed-pr-comment.yml` | `delay_and_comment` | dispatch | `ubuntu-latest` | none | *Table 1: Where each repository-owned lane runs, and its ceiling.* +The required `build-test` job includes an unconditional, bounded 30-minute +`make test-ui` step after Rust and linker setup and before runtime coverage. +This keeps compile-time API validation within the existing required status, +without a job-level condition that could leave branch protection without a +failure result. + Pull-request, push and tag lanes move to Ubicloud, because those are the ones a developer waits on: `build-test` waited 461 seconds for a GitHub-hosted runner on 2026-09-16 to do 556 seconds of work. Scheduled and delayed-comment lanes @@ -87,15 +93,21 @@ Two vCPU cost about 29% less per run for 3m40s more wall time, and memory is far inside the 7.9 GiB that shape has. The coverage action supplies a 180-second per-test deadline and a ten-minute -whole-run ceiling. `tests/compile_error.rs` runs fourteen trybuild fixtures -inside one test, including nested Cargo builds. A cold coverage run exceeded -that deadline on two vCPU after the other tests had completed -([PR #777's coverage run](https://github.com/leynos/wireframe/actions/runs/39203098537)). -The checked-in `.config/nextest.toml` preserves the action's deadlines for -ordinary tests and extends only `wireframe::compile_error::compile_tests` to -eight minutes. The ten-minute ceiling still bounds the coverage test run. The -four-vCPU `coverage-upload` lane remains on its measured shape; the smaller -runner has not been sampled with this per-test budget. +whole-run ceiling. `tests/compile_error.rs` runs trybuild fixtures inside one +test, including nested Cargo builds, which do not measure runtime behaviour and +can exceed that per-test deadline on a cold two-vCPU runner. An unconditional +`make test-ui` step runs in `build-test` for pull requests and pushes to +`main`, before runtime coverage, with a separate 30-minute step ceiling. This +keeps the compile-time API fixtures covered without making the instrumented +runtime- coverage run compile them. + +The compile-time harnesses are excluded only when `cfg(coverage)` is active; +the fixtures remain in `tests/ui/`, and `make test-ui` runs both the root and +Loom API suites. The checked-in `.config/nextest.toml` retains its eight-minute +override for `wireframe::compile_error::compile_tests` during ordinary, +noncoverage nextest runs. The ten-minute ceiling still bounds the coverage test +run. The four-vCPU `coverage-upload` lane remains on its measured shape; the +smaller runner has not been sampled for coverage generation. Free disk is the margin to watch: 3,998 MiB at its lowest on two vCPU. @@ -112,12 +124,15 @@ default is the expensive failure mode. A ceiling close to the measured work is the other one, because it cancels the run at the moment an overrun becomes interesting and discards the log that would explain it. -Two ceilings are measured: `build-test` at 556 seconds on four vCPU (750 -seconds on two, still well inside 30 minutes) and `coverage-upload` at 240 -seconds on four vCPU. Two are judgements, and the guide says so rather than -implying otherwise. `advanced` has failed every night since at least 2026-09-09 -and its last green run was 2025-10-04 at 32 seconds, so 60 minutes is generous -enough not to mask the repair when it lands. +The historical `build-test` sample was 556 seconds on four vCPU and 750 seconds +on two; the `coverage-upload` sample was 240 seconds on four vCPU. The 60-minute +`build-test` job ceiling is a judgement, not a measured limit: it allows the +existing lane and its new bounded UI step to finish, including cold compiler +work. The UI step's 30-minute ceiling is also a judgement. The four-vCPU +`coverage-upload` lane remains capped at 20 minutes. `advanced` has failed +every night since at least 2026-09-09 and its last green run was 2025-10-04 at +32 seconds, so 60 minutes is generous enough not to mask the repair when it +lands. `delay_and_comment` declares no ceiling, deliberately. Its entire duration is a `sleep` of the caller's `delay_minutes` input, so any fixed ceiling cancels a @@ -1023,10 +1038,10 @@ The Loom, verification, and Whitaker commands keep their own `RUSTFLAGS`, as the development and release recipes to the standard, and `build_standard_ci_test.py` holds the CI install order. The coverage lane (`coverage-main.yml:coverage-upload`) installs `mold` through `setup-rust`'s -`install-mold` input before `generate-coverage`, because the trybuild cases -compile scratch crates under `target/` that link with `mold` through the Cargo -configuration. The same file requires every Linux job that runs -`generate-coverage` to install `mold` first, by that input or by `apt`. +`install-mold` input before `generate-coverage`, because `.cargo/config.toml` +selects `mold` for Linux builds, including the instrumented runtime tests. The +same file requires every Linux job that runs `generate-coverage` to install +`mold` first, by that input or by `apt`. The fragment configures the backend only and declares no `[target.<...>]` table, so a direct `--config tools/dev-fast/config.toml` invocation behaves the @@ -1199,6 +1214,9 @@ tracing and the current-thread Tokio runtime explicitly, then use `WireframeServer::from_app` for server preparation, listener binding, connection spawning, and shutdown-aware accept loops. Keep example-specific address parsing, app construction, handlers, and middleware in the example file. +`tracing-subscriber` and the Prometheus exporter are development dependencies +for examples, tests, and doctests; applications supply their own tracing +subscriber and exporter. Codec benchmark helpers live in `wireframe_testing::codec_benchmarks`. Bench targets, direct unit tests, and BDD fixtures should import the workload matrix, @@ -1459,8 +1477,8 @@ Place new snippets in `tests/ui/`, register them in `tests/compile_error.rs`, and commit the generated `.stderr` file for compile-fail cases after verifying that the diagnostics describe the intended contract. These files are exact `rustc` diagnostic snapshots: generate and refresh them with the repository's -pinned `rust-toolchain.toml`. Normal validation is -`cargo test --test compile_error`; a deliberate developer-only refresh uses +pinned `rust-toolchain.toml`. Normal validation is `make test-ui`; a deliberate +developer-only refresh uses `TRYBUILD=overwrite cargo test --test compile_error`. Review refreshed fixtures as maintenance changes, and never run the overwrite command in CI. diff --git a/docs/multi-layered-testing-strategy.md b/docs/multi-layered-testing-strategy.md index 2e811efbb..d96952538 100644 --- a/docs/multi-layered-testing-strategy.md +++ b/docs/multi-layered-testing-strategy.md @@ -20,6 +20,32 @@ generated `lcov.info` report to Codecov using a pinned version of the Codecov GitHub Action (`18283e04ce6e62d37312384ff67231eb8fd56d24`, corresponding to v5.4.3) to make coverage visible across pull requests. +## 1.1 Layer 0: Compile-Time API Boundary + +**Objective:** To verify that the public API rejects the code it is supposed to +reject, and does so with the diagnostic the documentation promises. Runtime +tests cannot observe this: an API that should fail to compile is, by +construction, absent from any test binary that does compile. + +Two harnesses cover the boundary. `tests/compile_error.rs` drives trybuild +fixtures in `tests/ui/` for the root crate, in both `pass` and `compile_fail` +modes. `wireframe_testing/tests/loom_api_surface.rs` is compiled twice, once +with `--cfg loom` and once without, to prove that Loom-only exports are absent +from ordinary builds and present under instrumentation. + +`make test-ui` runs both harnesses. It runs in the required `build-test` job as +a bounded step *without* coverage instrumentation, because the nested `cargo` +invocations inside trybuild and the compile checks would otherwise report +coverage for code no test executed. Both harnesses therefore carry +`#![cfg(not(coverage))]`, and the coverage lane excludes only those two nested +compiler drivers. The runtime suite around them stays instrumented and +measured, so a green UI gate and a green coverage run answer different +questions rather than duplicating each other. + +**Measurable Objective:** Every compile-fail fixture produces its committed +`.stderr` diagnostic, both Loom polarity checks build, and no runtime test is +excluded from coverage. + ## 2. Layer 1: Foundational Correctness (Unit & Integration) **Objective:** To verify that each component behaves correctly in isolation and diff --git a/docs/users-guide.md b/docs/users-guide.md index d6ab4d955..927f747c2 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -1568,6 +1568,58 @@ worker tasks.[^20][^37][^38] `ServerError` surfaces bind, factory-build, and application-preparation failures as typed errors so callers can react appropriately.[^21] +## Metrics + +Wireframe's default `metrics` feature emits instrumentation through the +`metrics` facade but does not install a global recorder. The Prometheus +exporter is a development dependency of Wireframe, so applications that want +Prometheus metrics must add `metrics-exporter-prometheus` as a direct +dependency and install its recorder once during startup, before Wireframe +begins emitting metrics: + +```rust,no_run +use std::time::Duration; + +use metrics_exporter_prometheus::PrometheusBuilder; + +fn main() -> Result<(), Box> { + let prometheus = PrometheusBuilder::new().install_recorder()?; + + // Keep the handle: the upkeep task holds a clone of it. + let upkeep_handle = prometheus.clone(); + std::thread::spawn(move || { + loop { + std::thread::sleep(Duration::from_secs(5)); + upkeep_handle.run_upkeep(); + } + }); + + // Start the application after installing the recorder. + let _scrape_output = prometheus.render(); + Ok(()) +} +``` + +Keep the returned handle where the application can expose or render the +Prometheus output. Applications choose how to serve or otherwise collect it. + +Wireframe depends on the `tracing` facade, which emits events only when the +application installs a subscriber; it does not ship one. `tracing-subscriber` +was previously a runtime dependency of Wireframe and is now a development +dependency, so an application that wants Wireframe's events must add +`tracing-subscriber` itself and install a subscriber once during start-up, +before the application begins emitting. Omit it and the instrumentation still +compiles, but every event is discarded. + +`install_recorder` returns a `PrometheusHandle` but does **not** start the +exporter's background upkeep task; only `PrometheusBuilder::build` and +`PrometheusBuilder::install` spawn it. The task drains histogram buckets and +other self-maintaining structures, so without it that storage grows for as long +as no scrape triggers a render. Call `PrometheusHandle::run_upkeep` on a timer +(for example, every five seconds, matching the exporter's own default +interval), or serve metrics through `PrometheusBuilder::build`/`install` and +let the exporter own the task. + ## Client runtime `WireframeClient` provides a first-class client runtime that mirrors the diff --git a/docs/v0-3-0-to-v0-4-0-migration-guide.md b/docs/v0-3-0-to-v0-4-0-migration-guide.md index 7228cdbf9..68710c289 100644 --- a/docs/v0-3-0-to-v0-4-0-migration-guide.md +++ b/docs/v0-3-0-to-v0-4-0-migration-guide.md @@ -138,6 +138,35 @@ mutable drivers remain available as deprecated compatibility paths; migrate tests to the prepared helpers when they need to prove one-time middleware transformation or reuse prepared route services. +## Install your own tracing subscriber + +`tracing-subscriber` is no longer a runtime dependency of Wireframe; it is a +development dependency used by the examples, benchmarks and tests. The +`tracing` facade Wireframe emits through stays a runtime dependency, but it has +no subscriber of its own, so a released application that relied on Wireframe +pulling in a subscriber must now declare one and install it during start-up. + +Add the crate to the application's own manifest: + +```toml +[dependencies] +tracing-subscriber = { version = "0.3.18", features = ["env-filter"] } +``` + +Then initialize it before any Wireframe work begins: + +```rust +fn main() { + tracing_subscriber::fmt().init(); + // Start the application here. +} +``` + +Applications that never configured a subscriber see no behaviour change: they +were discarding Wireframe's events before this release too. Applications that +did configure one keep working, because installing an application-owned +subscriber is now the supported arrangement rather than a duplicate. + ## Migrate byte-handling APIs The prepared-application transition is independent of the zero-copy byte diff --git a/src/byte_order.rs b/src/byte_order.rs index d069ce688..5cb519af3 100644 --- a/src/byte_order.rs +++ b/src/byte_order.rs @@ -1,8 +1,9 @@ //! Helpers for explicit network byte-order conversions. //! -//! These helpers keep Clippy expectations scoped to the conversion points so -//! protocol code can remain explicit about wire endianness without repeating -//! lint annotations. +//! The fixed-size buffers match each integer width, so the `bytes` big-endian +//! operations cannot run out of capacity or input. + +use bytes::{Buf, BufMut}; /// Serialize a `u16` in network byte order (big-endian). /// @@ -15,11 +16,10 @@ /// ``` #[must_use] pub fn write_network_u16(value: u16) -> [u8; 2] { - #[expect( - clippy::big_endian_bytes, - reason = "Network byte order requires big-endian bytes." - )] - value.to_be_bytes() + let mut wire_bytes = [0; 2]; + let mut destination = wire_bytes.as_mut_slice(); + destination.put_u16(value); + wire_bytes } /// Parse a network-order `u16` from its on-wire representation. @@ -33,11 +33,8 @@ pub fn write_network_u16(value: u16) -> [u8; 2] { /// ``` #[must_use] pub fn read_network_u16(bytes: [u8; 2]) -> u16 { - #[expect( - clippy::big_endian_bytes, - reason = "Network byte order requires big-endian bytes." - )] - u16::from_be_bytes(bytes) + let mut wire_bytes = bytes.as_slice(); + wire_bytes.get_u16() } /// Serialize a `u32` in network byte order (big-endian). @@ -51,11 +48,10 @@ pub fn read_network_u16(bytes: [u8; 2]) -> u16 { /// ``` #[must_use] pub fn write_network_u32(value: u32) -> [u8; 4] { - #[expect( - clippy::big_endian_bytes, - reason = "Network byte order requires big-endian bytes." - )] - value.to_be_bytes() + let mut wire_bytes = [0; 4]; + let mut destination = wire_bytes.as_mut_slice(); + destination.put_u32(value); + wire_bytes } /// Parse a network-order `u32` from its on-wire representation. @@ -69,11 +65,8 @@ pub fn write_network_u32(value: u32) -> [u8; 4] { /// ``` #[must_use] pub fn read_network_u32(bytes: [u8; 4]) -> u32 { - #[expect( - clippy::big_endian_bytes, - reason = "Network byte order requires big-endian bytes." - )] - u32::from_be_bytes(bytes) + let mut wire_bytes = bytes.as_slice(); + wire_bytes.get_u32() } /// Serialize a `u64` in network byte order (big-endian). @@ -90,11 +83,10 @@ pub fn read_network_u32(bytes: [u8; 4]) -> u32 { /// ``` #[must_use] pub fn write_network_u64(value: u64) -> [u8; 8] { - #[expect( - clippy::big_endian_bytes, - reason = "Network byte order requires big-endian bytes." - )] - value.to_be_bytes() + let mut wire_bytes = [0; 8]; + let mut destination = wire_bytes.as_mut_slice(); + destination.put_u64(value); + wire_bytes } /// Parse a network-order `u64` from its on-wire representation. @@ -111,17 +103,15 @@ pub fn write_network_u64(value: u64) -> [u8; 8] { /// ``` #[must_use] pub fn read_network_u64(bytes: [u8; 8]) -> u64 { - #[expect( - clippy::big_endian_bytes, - reason = "Network byte order requires big-endian bytes." - )] - u64::from_be_bytes(bytes) + let mut wire_bytes = bytes.as_slice(); + wire_bytes.get_u64() } #[cfg(test)] mod tests { //! Round-trip tests for network byte-order conversion helpers. + use proptest::prelude::{any, prop_assert_eq, proptest}; use rstest::rstest; use super::{ @@ -162,4 +152,37 @@ mod tests { assert_eq!(written, expected_bytes); assert_eq!(read_back, value); } + + /// Zero encodes to the same-width zero buffer at every supported width. + #[test] + fn network_byte_order_boundaries_encode_zero() { + assert_eq!(write_network_u16(0), [0; 2]); + assert_eq!(write_network_u32(0), [0; 4]); + assert_eq!(write_network_u64(0), [0; 8]); + } + + /// The maximum value encodes to and decodes from an all-ones buffer. + #[test] + fn network_byte_order_boundaries_round_trip_maximum() { + assert_eq!(write_network_u16(u16::MAX), [0xff; 2]); + assert_eq!(write_network_u32(u32::MAX), [0xff; 4]); + assert_eq!(write_network_u64(u64::MAX), [0xff; 8]); + assert_eq!(read_network_u16([0xff; 2]), u16::MAX); + assert_eq!(read_network_u32([0xff; 4]), u32::MAX); + assert_eq!(read_network_u64([0xff; 8]), u64::MAX); + } + + proptest! { + /// Encoding and decoding preserve every bit at each supported width. + #[test] + fn network_byte_order_round_trips_arbitrary_values( + short in any::(), + medium in any::(), + long in any::(), + ) { + prop_assert_eq!(read_network_u16(write_network_u16(short)), short); + prop_assert_eq!(read_network_u32(write_network_u32(medium)), medium); + prop_assert_eq!(read_network_u64(write_network_u64(long)), long); + } + } } diff --git a/src/codec/error.rs b/src/codec/error.rs index 3321e8362..46f818993 100644 --- a/src/codec/error.rs +++ b/src/codec/error.rs @@ -221,7 +221,7 @@ impl CodecError { /// assert_eq!(err.default_recovery_policy(), RecoveryPolicy::Disconnect); /// ``` #[must_use] - pub fn default_recovery_policy(&self) -> RecoveryPolicy { + pub const fn default_recovery_policy(&self) -> RecoveryPolicy { match self { // Recoverable errors: drop and continue Self::Framing(FramingError::OversizedFrame { .. } | FramingError::EmptyFrame) @@ -251,7 +251,7 @@ impl CodecError { /// assert!(!err.is_clean_close()); /// ``` #[must_use] - pub fn is_clean_close(&self) -> bool { matches!(self, Self::Eof(EofError::CleanClose)) } + pub const fn is_clean_close(&self) -> bool { matches!(self, Self::Eof(EofError::CleanClose)) } /// Returns true if the connection should be terminated. /// @@ -285,7 +285,7 @@ impl CodecError { /// /// One of: `"framing"`, `"protocol"`, `"io"`, or `"eof"`. #[must_use] - pub fn error_type(&self) -> &'static str { + pub const fn error_type(&self) -> &'static str { match self { Self::Framing(_) => "framing", Self::Protocol(_) => "protocol", @@ -293,23 +293,29 @@ impl CodecError { Self::Eof(_) => "eof", } } -} -impl From for io::Error { - fn from(err: CodecError) -> Self { - match err { - CodecError::Io(e) => e, - CodecError::Framing(e) => { - io::Error::new(io::ErrorKind::InvalidData, CodecError::Framing(e)) + /// Map this codec error onto the closest [`io::Error`] category. + /// + /// Framing and protocol failures are invalid data; a clean close is an + /// unexpected end of file. An existing I/O error passes through unchanged. + fn into_io_error(self) -> io::Error { + match self { + Self::Io(error) => error, + Self::Framing(error) => { + io::Error::new(io::ErrorKind::InvalidData, Self::Framing(error)) } - CodecError::Protocol(e) => { - io::Error::new(io::ErrorKind::InvalidData, CodecError::Protocol(e)) + Self::Protocol(error) => { + io::Error::new(io::ErrorKind::InvalidData, Self::Protocol(error)) } - CodecError::Eof(e) => io::Error::new(io::ErrorKind::UnexpectedEof, CodecError::Eof(e)), + Self::Eof(error) => io::Error::new(io::ErrorKind::UnexpectedEof, Self::Eof(error)), } } } +impl From for io::Error { + fn from(error: CodecError) -> Self { error.into_io_error() } +} + #[cfg(test)] #[path = "error_tests.rs"] mod tests; diff --git a/src/codec/examples.rs b/src/codec/examples.rs index c5a2574b3..07dad469b 100644 --- a/src/codec/examples.rs +++ b/src/codec/examples.rs @@ -39,7 +39,7 @@ impl HotlineFrameCodec { /// assert_eq!(codec.max_frame_length(), 1024); /// ``` #[must_use] - pub fn new(max_frame_length: usize) -> Self { Self { max_frame_length } } + pub const fn new(max_frame_length: usize) -> Self { Self { max_frame_length } } } /// Adapter that encodes and decodes Hotline frames. @@ -50,7 +50,7 @@ pub struct HotlineAdapter { } impl HotlineAdapter { - fn new(max_frame_length: usize) -> Self { Self { max_frame_length } } + const fn new(max_frame_length: usize) -> Self { Self { max_frame_length } } } impl Decoder for HotlineAdapter { @@ -99,21 +99,21 @@ impl Encoder for HotlineAdapter { fn encode(&mut self, item: HotlineFrame, dst: &mut BytesMut) -> Result<(), Self::Error> { const HEADER_LEN: usize = 20; - let data_size = item.payload.len(); - if data_size > self.max_frame_length { + let payload_size = item.payload.len(); + if payload_size > self.max_frame_length { return Err(io::Error::new( io::ErrorKind::InvalidInput, "payload too large", )); } - let total_size = data_size + HEADER_LEN; - let data_size = u32::try_from(data_size) + let frame_size = payload_size + HEADER_LEN; + let wire_payload_size = u32::try_from(payload_size) .map_err(|_| io::Error::new(io::ErrorKind::InvalidInput, "payload too large"))?; - let total_size = u32::try_from(total_size) + let wire_frame_size = u32::try_from(frame_size) .map_err(|_| io::Error::new(io::ErrorKind::InvalidInput, "payload too large"))?; - dst.reserve(total_size as usize); - dst.put_u32(data_size); - dst.put_u32(total_size); + dst.reserve(wire_frame_size as usize); + dst.put_u32(wire_payload_size); + dst.put_u32(wire_frame_size); dst.put_u32(item.transaction_id); dst.put_bytes(0, 8); dst.extend_from_slice(&item.payload); @@ -174,7 +174,7 @@ impl MysqlFrameCodec { /// assert_eq!(codec.max_frame_length(), 1024); /// ``` #[must_use] - pub fn new(max_frame_length: usize) -> Self { Self { max_frame_length } } + pub const fn new(max_frame_length: usize) -> Self { Self { max_frame_length } } } /// Adapter that encodes and decodes `MySQL` frames. @@ -185,7 +185,7 @@ pub struct MysqlAdapter { } impl MysqlAdapter { - fn new(max_frame_length: usize) -> Self { Self { max_frame_length } } + const fn new(max_frame_length: usize) -> Self { Self { max_frame_length } } } impl Decoder for MysqlAdapter { @@ -243,11 +243,8 @@ impl Encoder for MysqlAdapter { )); } - #[expect( - clippy::cast_possible_truncation, - reason = "bounded by 0xff_ff_ff check above" - )] - let payload_len_u32 = payload_len as u32; + let payload_len_u32 = u32::try_from(payload_len) + .map_err(|_| io::Error::new(io::ErrorKind::InvalidInput, "payload too long"))?; let len_lo = (payload_len_u32 & 0xff) as u8; let len_mid = ((payload_len_u32 >> 8) & 0xff) as u8; let len_hi = ((payload_len_u32 >> 16) & 0xff) as u8; diff --git a/src/codec/recovery/config.rs b/src/codec/recovery/config.rs index 69e759582..b3c44d371 100644 --- a/src/codec/recovery/config.rs +++ b/src/codec/recovery/config.rs @@ -55,21 +55,21 @@ impl Default for RecoveryConfig { impl RecoveryConfig { /// Set the maximum consecutive dropped frames before disconnect. #[must_use] - pub fn max_consecutive_drops(mut self, count: u32) -> Self { + pub const fn max_consecutive_drops(mut self, count: u32) -> Self { self.max_consecutive_drops = count; self } /// Set the default quarantine duration. #[must_use] - pub fn quarantine_duration(mut self, duration: Duration) -> Self { + pub const fn quarantine_duration(mut self, duration: Duration) -> Self { self.quarantine_duration = duration; self } /// Set whether to log dropped frames. #[must_use] - pub fn log_dropped_frames(mut self, enabled: bool) -> Self { + pub const fn log_dropped_frames(mut self, enabled: bool) -> Self { self.log_dropped_frames = enabled; self } diff --git a/src/codec/recovery/context.rs b/src/codec/recovery/context.rs index 5ab103a21..206e7d8c7 100644 --- a/src/codec/recovery/context.rs +++ b/src/codec/recovery/context.rs @@ -46,21 +46,21 @@ impl CodecErrorContext { /// Set the connection identifier. #[must_use] - pub fn with_connection_id(mut self, id: u64) -> Self { + pub const fn with_connection_id(mut self, id: u64) -> Self { self.connection_id = Some(id); self } /// Set the peer address. #[must_use] - pub fn with_peer_address(mut self, addr: SocketAddr) -> Self { + pub const fn with_peer_address(mut self, addr: SocketAddr) -> Self { self.peer_address = Some(addr); self } /// Set the correlation identifier. #[must_use] - pub fn with_correlation_id(mut self, id: u64) -> Self { + pub const fn with_correlation_id(mut self, id: u64) -> Self { self.correlation_id = Some(id); self } diff --git a/src/frame/conversion.rs b/src/frame/conversion.rs index b58ec81e8..61e1f5936 100644 --- a/src/frame/conversion.rs +++ b/src/frame/conversion.rs @@ -1,13 +1,10 @@ //! Conversion helpers for length prefix encoding. use std::io; +use bytes::Buf; + use super::format::Endianness; -use crate::byte_order::{ - read_network_u64, - write_network_u16, - write_network_u32, - write_network_u64, -}; +use crate::byte_order::{read_network_u64, write_network_u64}; /// Explains that the configured wire prefix cannot represent any supported /// fixed-width integer and therefore cannot be decoded safely. @@ -23,11 +20,8 @@ pub(crate) const ERR_INCOMPLETE_PREFIX: &str = "incomplete length prefix"; /// host architecture selected to run the codec. #[inline] fn u64_from_le_bytes(bytes: [u8; 8]) -> u64 { - #[expect( - clippy::little_endian_bytes, - reason = "Wire endianness is explicit; from_le_bytes keeps decoding host-independent." - )] - u64::from_le_bytes(bytes) + let mut wire_bytes = bytes.as_slice(); + wire_bytes.get_u64_le() } /// Checked conversion from `usize` to a specific prefix integer type. @@ -136,34 +130,91 @@ fn convert_len_to_value(len: usize, size: usize) -> io::Result { /// /// For big-endian, the function delegates to the typed network-order write /// helpers mirroring how the read path delegates to `read_network_u64`. -fn write_bytes_with_endianness(value: u64, endianness: Endianness, prefix: &mut [u8]) { +fn write_bytes_with_endianness( + value: u64, + endianness: Endianness, + prefix: &mut [u8], +) -> io::Result<()> { let size = prefix.len(); match endianness { Endianness::Big => write_big_endian_prefix(value, size, prefix), - Endianness::Little => write_little_endian_prefix(value, prefix), + Endianness::Little => { + write_little_endian_prefix(value, prefix); + Ok(()) + } } } -/// Encode `value` into `prefix` in network (big-endian) byte order. +/// Reject a length that the configured prefix width cannot represent. /// -/// Delegates to the typed `write_network_*` helpers so the encode and -/// decode paths share the same lint-suppressed conversion point. -/// Callers guarantee the value fits in the requested prefix size via -/// `checked_prefix_cast` in `convert_len_to_value`. -fn write_big_endian_prefix(value: u64, size: usize, prefix: &mut [u8]) { - #[expect( - clippy::cast_possible_truncation, - reason = "caller validates value fits in prefix via checked_prefix_cast" - )] +/// A prefix only carries the low `size` bytes of the network-order encoding, so +/// a value wider than the prefix would otherwise be silently truncated. +/// +/// # Examples +/// +/// ```text +/// value_fits_prefix(255, 1) -> true +/// value_fits_prefix(256, 1) -> false +/// ``` +fn value_fits_prefix(value: u64, size: usize) -> bool { match size { - 1 => prefix.copy_from_slice(&[value as u8]), - 2 => prefix.copy_from_slice(&write_network_u16(value as u16)), - 4 => prefix.copy_from_slice(&write_network_u32(value as u32)), - 8 => prefix.copy_from_slice(&write_network_u64(value)), - _ => debug_assert!(false, "size validated upstream to be 1, 2, 4, or 8"), + 1 => u8::try_from(value).is_ok(), + 2 => u16::try_from(value).is_ok(), + 4 => u32::try_from(value).is_ok(), + 8 => true, + _ => false, + } +} + +/// Validates the prefix width and whether `value` fits within that width. +/// +/// # Errors +/// +/// Returns [`io::ErrorKind::InvalidInput`] when the width is unsupported or +/// the value cannot be represented by the selected width. +/// +/// # Examples +/// +/// ```text +/// ensure_fits_prefix(255, 1) -> Ok(()) +/// ensure_fits_prefix(256, 1) -> Err(InvalidInput) +/// ``` +fn ensure_fits_prefix(value: u64, size: usize) -> io::Result<()> { + if !matches!(size, 1 | 2 | 4 | 8) { + Err(io::Error::new( + io::ErrorKind::InvalidInput, + ERR_UNSUPPORTED_PREFIX, + )) + } else if value_fits_prefix(value, size) { + Ok(()) + } else { + Err(io::Error::new( + io::ErrorKind::InvalidInput, + ERR_FRAME_TOO_LARGE, + )) } } +/// Encode `value` into `prefix` in network (big-endian) byte order. +/// +/// Encoding the full width once and keeping its trailing bytes yields the same +/// digits as narrowing to the prefix width first, and it lets the fit check stay +/// separate from the write. +fn write_big_endian_prefix(value: u64, size: usize, prefix: &mut [u8]) -> io::Result<()> { + ensure_fits_prefix(value, size)?; + + let encoded = write_network_u64(value); + let start = 8usize + .checked_sub(size) + .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidInput, ERR_UNSUPPORTED_PREFIX))?; + let trailing = encoded + .get(start..) + .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidInput, ERR_UNSUPPORTED_PREFIX))?; + + prefix.copy_from_slice(trailing); + Ok(()) +} + /// Encode `value` into `prefix` in little-endian byte order. fn write_little_endian_prefix(value: u64, prefix: &mut [u8]) { for (i, byte) in prefix.iter_mut().enumerate() { @@ -195,13 +246,11 @@ pub fn u64_to_bytes( let value = convert_len_to_value(len, size)?; - #[expect( - clippy::indexing_slicing, - reason = "size validated to be within the 8-byte prefix buffer" - )] - let prefix = &mut out[..size]; + let prefix = out + .get_mut(..size) + .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidInput, ERR_UNSUPPORTED_PREFIX))?; - write_bytes_with_endianness(value, endianness, prefix); + write_bytes_with_endianness(value, endianness, prefix)?; if let Some(tail) = out.get_mut(size..) { tail.fill(0); @@ -219,6 +268,15 @@ mod tests { use super::*; use crate::frame::format::Endianness; + /// Fixed-width decoding retains exact little-endian byte order. + #[rstest] + #[case::asymmetric([0x88, 0x77, 0x66, 0x55, 0x44, 0x33, 0x22, 0x11], 0x1122_3344_5566_7788)] + #[case::zero([0; 8], 0)] + #[case::maximum([0xff; 8], u64::MAX)] + fn little_endian_bytes_decode_exactly(#[case] wire_bytes: [u8; 8], #[case] expected: u64) { + assert_eq!(u64_from_le_bytes(wire_bytes), expected); + } + #[rstest] #[case(&[0xab], Endianness::Big, [0, 0, 0, 0, 0, 0, 0, 0xab])] #[case(&[0x01, 0x02], Endianness::Big, [0, 0, 0, 0, 0, 0, 0x01, 0x02])] diff --git a/src/frame/tests.rs b/src/frame/tests.rs index c67b66787..4cd9e9cf4 100644 --- a/src/frame/tests.rs +++ b/src/frame/tests.rs @@ -2,6 +2,7 @@ use std::io; +use proptest::prelude::*; use rstest::rstest; use super::{conversion::*, format::*}; @@ -138,3 +139,67 @@ fn u64_to_bytes_zeroes_remainder( .all(|&b| b == 0) ); } + +fn prefix_endianness() -> impl Strategy { + prop_oneof![Just(Endianness::Big), Just(Endianness::Little)] +} + +fn representable_prefix_cases() -> impl Strategy { + prop_oneof![Just(1usize), Just(2), Just(4), Just(8)].prop_flat_map(|size| { + let maximum = match size { + 1 => usize::from(u8::MAX), + 2 => usize::from(u16::MAX), + 4 => usize::try_from(u32::MAX).unwrap_or(usize::MAX), + _ => usize::try_from(u64::MAX).unwrap_or(usize::MAX), + }; + (Just(size), 0..=maximum, prefix_endianness()) + }) +} + +fn oversized_prefix_cases() -> impl Strategy { + let size = prop_oneof![ + Just(1usize), + Just(2), + Just(4).prop_filter("four-byte overflow requires a wider usize", |_| usize::BITS + > 32), + ]; + size.prop_flat_map(|size| { + let minimum = match size { + 1 => usize::from(u8::MAX) + 1, + 2 => usize::from(u16::MAX) + 1, + _ => { + usize::try_from(u32::MAX) + .expect("a four-byte overflow value requires usize wider than 32 bits") + + 1 + } + }; + (Just(size), minimum..=usize::MAX, prefix_endianness()) + }) +} + +proptest! { + /// Variable-width prefixes preserve representable values in either byte order. + #[test] + fn variable_width_prefixes_round_trip((size, value, endianness) in representable_prefix_cases()) { + let mut encoded = [0u8; 8]; + let written = u64_to_bytes(value, size, endianness, &mut encoded) + .expect("generated value fits its prefix width"); + let encoded_prefix = encoded + .get(..written) + .expect("prefix encoder reports an in-bounds byte count"); + let decoded = bytes_to_u64(encoded_prefix, size, endianness) + .expect("encoded prefix has the requested width"); + + prop_assert_eq!(decoded, u64::try_from(value).expect("generated value fits in u64")); + } + + /// Values above any representable supported prefix are rejected explicitly. + #[test] + fn variable_width_prefixes_reject_oversized_values((size, value, endianness) in oversized_prefix_cases()) { + let mut encoded = [0u8; 8]; + let error = u64_to_bytes(value, size, endianness, &mut encoded) + .expect_err("value exceeds the selected prefix width"); + + prop_assert_eq!(error.kind(), io::ErrorKind::InvalidInput); + } +} diff --git a/src/message_assembler/budget.rs b/src/message_assembler/budget.rs index cf424592c..876845869 100644 --- a/src/message_assembler/budget.rs +++ b/src/message_assembler/budget.rs @@ -45,7 +45,7 @@ impl AggregateBudgets { /// Returns [`MessageAssemblyError::ConnectionBudgetExceeded`] or /// [`MessageAssemblyError::InFlightBudgetExceeded`] when the respective /// limit would be exceeded. -pub(super) fn check_aggregate_budgets( +pub(super) const fn check_aggregate_budgets( key: MessageKey, current_total: usize, additional_bytes: usize, @@ -85,7 +85,7 @@ pub(super) fn check_aggregate_budgets( /// /// Returns [`MessageAssemblyError::MessageTooLarge`] when the new total /// would exceed `max_message_size`. -pub(super) fn check_size_limit( +pub(super) const fn check_size_limit( max_message_size: NonZeroUsize, key: MessageKey, accumulated: usize, diff --git a/src/message_assembler/header.rs b/src/message_assembler/header.rs index 77f414b48..80a89dfdb 100644 --- a/src/message_assembler/header.rs +++ b/src/message_assembler/header.rs @@ -201,5 +201,5 @@ impl ParsedFrameHeader { /// assert!(matches!(parsed.into_header(), FrameHeader::First(_))); /// ``` #[must_use] - pub fn into_header(self) -> FrameHeader { self.header } + pub const fn into_header(self) -> FrameHeader { self.header } } diff --git a/src/message_assembler/state.rs b/src/message_assembler/state.rs index 53411ee03..ff68d8e02 100644 --- a/src/message_assembler/state.rs +++ b/src/message_assembler/state.rs @@ -29,29 +29,29 @@ struct PartialAssembly { /// First-frame metadata kept separate from the body for reconstruction. metadata: Vec, /// Ordered body bytes accumulated from first and continuation frames. - body_buffer: Vec, + body: Vec, /// Start time used to discard incomplete peer-controlled state. started_at: Instant, } impl PartialAssembly { /// Create empty assembly storage anchored at the first frame's timestamp. - fn new(series: MessageSeries, routing: EnvelopeRouting, started_at: Instant) -> Self { + const fn new(series: MessageSeries, routing: EnvelopeRouting, started_at: Instant) -> Self { Self { series, routing, metadata: Vec::new(), - body_buffer: Vec::new(), + body: Vec::new(), started_at, } } /// Append body bytes after the caller has validated size and ordering. - fn push_body(&mut self, data: &[u8]) { self.body_buffer.extend_from_slice(data); } + fn push_body(&mut self, data: &[u8]) { self.body.extend_from_slice(data); } /// Replace the first-frame metadata retained for the completed message. fn set_metadata(&mut self, data: Vec) { self.metadata = data; } /// Return body bytes accumulated so far for per-message budget checks. - fn accumulated_len(&self) -> usize { self.body_buffer.len() } + const fn accumulated_len(&self) -> usize { self.body.len() } /// Logical payload length represented by the body and metadata buffers. - fn buffered_bytes(&self) -> usize { self.body_buffer.len().saturating_add(self.metadata.len()) } + const fn buffered_bytes(&self) -> usize { self.body.len().saturating_add(self.metadata.len()) } } /// Stateful manager for multiple concurrent message assemblies. /// @@ -354,7 +354,7 @@ impl MessageAssemblyState { key, partial.routing, partial.metadata, - partial.body_buffer, + partial.body, ))) } } diff --git a/src/message_assembler/types.rs b/src/message_assembler/types.rs index 91a779ebe..0dee5cf3a 100644 --- a/src/message_assembler/types.rs +++ b/src/message_assembler/types.rs @@ -192,7 +192,7 @@ pub struct AssembledMessage { impl AssembledMessage { /// Create a new assembled message. #[must_use] - pub fn new( + pub const fn new( message_key: MessageKey, routing: EnvelopeRouting, metadata: Vec, diff --git a/src/middleware.rs b/src/middleware.rs index 4dc48f07b..0363b6317 100644 --- a/src/middleware.rs +++ b/src/middleware.rs @@ -20,15 +20,15 @@ pub struct FrameContainer { impl FrameContainer { /// Create a new container holding `frame` bytes. #[must_use] - pub fn new(frame: F) -> Self { Self { frame } } + pub const fn new(frame: F) -> Self { Self { frame } } /// Borrow the inner frame data. #[must_use] - pub fn frame(&self) -> &F { &self.frame } + pub const fn frame(&self) -> &F { &self.frame } /// Mutable access to the frame data. #[must_use] - pub fn frame_mut(&mut self) -> &mut F { &mut self.frame } + pub const fn frame_mut(&mut self) -> &mut F { &mut self.frame } /// Consume the container, returning the frame. #[must_use] @@ -60,11 +60,11 @@ impl ServiceRequest { /// Return the correlation identifier associated with this request, if any. #[must_use] - pub fn correlation_id(&self) -> Option { self.correlation_id } + pub const fn correlation_id(&self) -> Option { self.correlation_id } /// Set or clear the correlation identifier on the request. #[must_use] - pub fn set_correlation_id(&mut self, correlation_id: Option) -> &mut Self { + pub const fn set_correlation_id(&mut self, correlation_id: Option) -> &mut Self { self.correlation_id = correlation_id; self } @@ -107,11 +107,11 @@ impl ServiceResponse { /// Return the correlation identifier associated with this response, if any. #[must_use] - pub fn correlation_id(&self) -> Option { self.correlation_id } + pub const fn correlation_id(&self) -> Option { self.correlation_id } /// Set or clear the correlation identifier. #[must_use] - pub fn set_correlation_id(&mut self, correlation_id: Option) -> &mut Self { + pub const fn set_correlation_id(&mut self, correlation_id: Option) -> &mut Self { self.correlation_id = correlation_id; self } @@ -154,7 +154,7 @@ where /// ``` #[inline] #[must_use] - pub fn new(service: &'a S) -> Self { Self { service } } + pub const fn new(service: &'a S) -> Self { Self { service } } /// Call the next service with the provided request. /// @@ -206,7 +206,7 @@ pub struct FromFn { impl FromFn { /// Construct middleware from the provided asynchronous function. - pub fn new(f: F) -> Self { Self { f } } + pub const fn new(f: F) -> Self { Self { f } } } /// Convenience constructor to build middleware from an async function. diff --git a/src/request/mod.rs b/src/request/mod.rs index d07f539af..a78fd949b 100644 --- a/src/request/mod.rs +++ b/src/request/mod.rs @@ -234,7 +234,7 @@ impl RequestParts { /// assert_eq!(parts.id(), 1); /// ``` #[must_use] - pub fn new(id: u32, correlation_id: Option, metadata: Vec) -> Self { + pub const fn new(id: u32, correlation_id: Option, metadata: Vec) -> Self { Self { id, correlation_id, @@ -298,7 +298,7 @@ impl RequestParts { /// parts.metadata_mut().push(0x02); /// assert_eq!(parts.metadata(), &[0x01, 0x02]); /// ``` - pub fn metadata_mut(&mut self) -> &mut Vec { &mut self.metadata } + pub const fn metadata_mut(&mut self) -> &mut Vec { &mut self.metadata } /// Consume the parts and return the metadata bytes. /// @@ -356,7 +356,7 @@ impl RequestParts { /// must win. #[cfg_attr(test, mutants::skip)] #[inline] - fn select_correlation(current: Option, source: Option) -> CorrelationResult { + const fn select_correlation(current: Option, source: Option) -> CorrelationResult { match (current, source) { (None, cid) => CorrelationResult::Inherited(cid), (Some(found), Some(expected)) if found != expected => { diff --git a/src/response.rs b/src/response.rs index 072aea18f..b8274c325 100644 --- a/src/response.rs +++ b/src/response.rs @@ -99,21 +99,21 @@ pub enum Response { impl std::fmt::Debug for Response { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - Response::Single(frame) => f.debug_tuple("Single").field(frame).finish(), - Response::Vec(v) => f.debug_tuple("Vec").field(v).finish(), - Response::Stream(_) => f.write_str("Stream(..)"), - Response::MultiPacket(_) => f.write_str("MultiPacket(..)"), - Response::Empty => f.write_str("Empty"), + Self::Single(frame) => f.debug_tuple("Single").field(frame).finish(), + Self::Vec(v) => f.debug_tuple("Vec").field(v).finish(), + Self::Stream(_) => f.write_str("Stream(..)"), + Self::MultiPacket(_) => f.write_str("MultiPacket(..)"), + Self::Empty => f.write_str("Empty"), } } } impl From for Response { - fn from(f: F) -> Self { Response::Single(f) } + fn from(f: F) -> Self { Self::Single(f) } } impl From> for Response { - fn from(v: Vec) -> Self { Response::Vec(v) } + fn from(v: Vec) -> Self { Self::Vec(v) } } impl Response { @@ -154,9 +154,9 @@ impl Response { /// } /// ``` #[must_use] - pub fn with_channel(capacity: usize) -> (mpsc::Sender, Response) { + pub fn with_channel(capacity: usize) -> (mpsc::Sender, Self) { let (sender, receiver) = mpsc::channel(capacity); - (sender, Response::MultiPacket(receiver)) + (sender, Self::MultiPacket(receiver)) } /// Convert this response into a stream of frames. @@ -185,13 +185,11 @@ impl Response { /// ``` pub fn into_stream(self) -> FrameStream { match self { - Response::Single(f) => { - stream::once(async move { Ok::>(f) }).boxed() - } - Response::Vec(frames) => stream::iter(frames.into_iter().map(Ok)).boxed(), - Response::Stream(s) => s, - Response::MultiPacket(rx) => ReceiverStream::new(rx).map(Ok).boxed(), - Response::Empty => stream::empty().boxed(), + Self::Single(f) => stream::once(async move { Ok::>(f) }).boxed(), + Self::Vec(frames) => stream::iter(frames.into_iter().map(Ok)).boxed(), + Self::Stream(s) => s, + Self::MultiPacket(rx) => ReceiverStream::new(rx).map(Ok).boxed(), + Self::Empty => stream::empty().boxed(), } } } diff --git a/tests/compile_error.rs b/tests/compile_error.rs index 2063856c8..4a0eafa9b 100644 --- a/tests/compile_error.rs +++ b/tests/compile_error.rs @@ -1,5 +1,7 @@ //! Compile-time trait-bound and public-default tests. +#![cfg(not(coverage))] + /// Check public type contracts and reject a zero fixture budget at compile time. #[test] fn compile_tests() { @@ -8,6 +10,7 @@ fn compile_tests() { t.pass("tests/ui/server_factory_forms.rs"); t.pass("tests/ui/wireframe_result_default_no_protocol.rs"); t.pass("tests/ui/client_const_api.rs"); + t.pass("tests/ui/codec_shared_const_api.rs"); t.pass("tests/ui/fragment_const_api.rs"); t.pass("tests/ui/app_const_api.rs"); t.compile_fail("tests/ui/server_rejects_non_send_factory.rs"); diff --git a/tests/ui/codec_shared_const_api.rs b/tests/ui/codec_shared_const_api.rs new file mode 100644 index 000000000..a9e6a215e --- /dev/null +++ b/tests/ui/codec_shared_const_api.rs @@ -0,0 +1,130 @@ +//! Verifies codec and shared-source APIs remain callable in const contexts. + +use std::{ + net::{Ipv4Addr, SocketAddr, SocketAddrV4}, + time::Duration, +}; + +use wireframe::{ + codec::{ + examples::{HotlineFrameCodec, MysqlFrameCodec}, + CodecError, + CodecErrorContext, + EofError, + RecoveryConfig, + RecoveryPolicy, + }, + message_assembler::{ + AssembledMessage, + CorrelationId, + EnvelopeId, + EnvelopeRouting, + FirstFrameHeader, + FrameHeader, + MessageKey, + ParsedFrameHeader, + }, + middleware::{FrameContainer, FromFn, Next, Service, ServiceRequest, ServiceResponse}, + request::RequestParts, +}; + +const CLEAN_CLOSE: CodecError = CodecError::Eof(EofError::CleanClose); +const RECOVERY_POLICY: RecoveryPolicy = CLEAN_CLOSE.default_recovery_policy(); +const IS_CLEAN_CLOSE: bool = CLEAN_CLOSE.is_clean_close(); +const ERROR_CATEGORY: &str = CLEAN_CLOSE.error_type(); + +const RECOVERY_CONFIG: RecoveryConfig = RecoveryConfig { + max_consecutive_drops: 0, + quarantine_duration: Duration::ZERO, + log_dropped_frames: false, +} +.max_consecutive_drops(3) +.quarantine_duration(Duration::from_secs(5)) +.log_dropped_frames(true); + +const ERROR_CONTEXT: CodecErrorContext = CodecErrorContext { + connection_id: None, + peer_address: None, + correlation_id: None, + codec_state: None, +} +.with_connection_id(7) +.with_peer_address(SocketAddr::V4(SocketAddrV4::new( + Ipv4Addr::LOCALHOST, + 8080, +))) +.with_correlation_id(9); + +const HOTLINE_CODEC: HotlineFrameCodec = HotlineFrameCodec::new(1024); +const MYSQL_CODEC: MysqlFrameCodec = MysqlFrameCodec::new(2048); + +const PARSED_HEADER: ParsedFrameHeader = ParsedFrameHeader::new( + FrameHeader::First(FirstFrameHeader { + message_key: MessageKey(11), + metadata_len: 0, + body_len: 4, + total_body_len: None, + is_last: true, + }), + 12, +); +const FRAME_HEADER: FrameHeader = PARSED_HEADER.into_header(); + +const ASSEMBLED_MESSAGE: AssembledMessage = AssembledMessage::new( + MessageKey(11), + EnvelopeRouting { + envelope_id: EnvelopeId(42), + correlation_id: Some(CorrelationId(9)), + }, + Vec::new(), + Vec::new(), +); + +const REQUEST_PARTS: RequestParts = RequestParts::new(3, Some(9), Vec::new()); + +const FRAME_VALUE: u8 = { + let mut container = FrameContainer::new(1u8); + *container.frame_mut() = 2; + *container.frame() +}; + +fn callback() {} +const MIDDLEWARE: FromFn = FromFn::new(callback); + +const fn continuation( + service: &dyn Service, +) -> Next<'_, dyn Service + '_> { + Next::new(service) +} + +const fn request_correlation(request: &mut ServiceRequest) -> Option { + request.set_correlation_id(Some(9)).correlation_id() +} + +const fn response_correlation(response: &mut ServiceResponse) -> Option { + response.set_correlation_id(Some(9)).correlation_id() +} + +const _: () = assert!(matches!(RECOVERY_POLICY, RecoveryPolicy::Disconnect)); +const _: () = assert!(IS_CLEAN_CLOSE); +const _: () = assert!(FRAME_VALUE == 2); +const _: &str = ERROR_CATEGORY; + +const fn metadata_mut(parts: &mut RequestParts) -> &mut Vec { parts.metadata_mut() } + +fn main() { + let _ = ( + RECOVERY_CONFIG, + ERROR_CONTEXT, + HOTLINE_CODEC, + MYSQL_CODEC, + FRAME_HEADER, + ASSEMBLED_MESSAGE, + REQUEST_PARTS, + metadata_mut, + MIDDLEWARE, + continuation, + request_correlation, + response_correlation, + ); +} diff --git a/tests/workflow_contracts/ci_runner_placement_test.py b/tests/workflow_contracts/ci_runner_placement_test.py index 302ea8f3f..2419c8412 100644 --- a/tests/workflow_contracts/ci_runner_placement_test.py +++ b/tests/workflow_contracts/ci_runner_placement_test.py @@ -58,9 +58,9 @@ of them. Without it, a new GitHub-hosted job added to the placement table with no ceiling entry passes every ceiling test, because the per-coordinate test iterates the ceiling table and the tree walk reads only ``ubicloud-`` -lanes; and a second ``None`` placement value escapes both the label test, -which skips ``None``, and the expression tests, which look only at the one -fork-fallback coordinate. +lanes; and an unreviewed ``None`` placement value escapes both the label test, +which skips ``None``, and the expression tests, which cover only the reviewed +fork-fallback coordinates. Mutation proof; each applied alone and reverted: @@ -85,7 +85,7 @@ - removing the ``advanced`` entry from ``EXPECTED_CEILING_MINUTES`` fails ``test_the_reviewed_tables_are_total`` alone, and nothing else: that is the gap the totality test was added to close; -- giving a second coordinate the ``None`` placement value fails +- giving an unreviewed coordinate the ``None`` placement value fails ``test_the_reviewed_tables_are_total`` alone, for the same reason; - deleting ``timeout-minutes`` from ``coverage-upload`` fails ``test_the_job_declares_its_reviewed_ceiling`` and @@ -108,19 +108,6 @@ lane that has not been sampled on two vCPU cannot follow ``build-test`` unnoticed. -A ninth is not a mutation but a real event, recorded because it is better -evidence than a contrived one. Moving both Ubicloud lanes from -``ubicloud-standard-2`` to ``ubicloud-standard-4`` after the smaller shape -killed a test failed ``test_the_job_carries_its_reviewed_label``, -``test_the_runner_expression_is_exactly_the_reviewed_one`` and -``test_the_actionlint_registry_matches_the_labels_in_use`` the moment the -label changed and before the registry was updated, unprompted. -``test_every_ubicloud_job_has_a_ceiling`` stayed green throughout, because it -tests the ``ubicloud-`` prefix rather than a literal label; written against a -literal it would have silently stopped noticing an unbounded lane at exactly -that moment, which is a dead-guard defect repovec-appliance found in its own -placement contract. - Run via ``make test-workflow-contracts``. """ @@ -134,7 +121,7 @@ EXPECTED_CEILING_MINUTES, EXPECTED_PLACEMENT, FOREIGN_RUNNER_FRAGMENTS, - FORK_FALLBACK_JOB, + FORK_FALLBACK_JOBS, GITHUB_HOSTED_LABEL, GITHUB_HOSTED_LABELS, HOSTED_GUARD, @@ -187,19 +174,17 @@ def test_the_reviewed_tables_are_total() -> None: ``ubicloud-`` lanes, so a new GitHub-hosted lane would inherit the six-hour default unremarked. - A second coordinate carrying the ``None`` placement value escapes the - label test, which skips ``None``, and the expression tests, which read - only ``FORK_FALLBACK_JOB``. Its runner would then be checked by nothing - at all. + A coordinate carrying ``None`` must have an expression assertion, since + the label test skips it. """ expression_jobs = { coordinate for coordinate, label in EXPECTED_PLACEMENT.items() if label is None } - assert expression_jobs == {FORK_FALLBACK_JOB}, ( - "only the fork-fallback lane may be checked by expression rather than " - f"by label; {sorted(expression_jobs - {FORK_FALLBACK_JOB})} would have " + assert expression_jobs == FORK_FALLBACK_JOBS, ( + "only reviewed fallback lanes may be checked by expression rather than " + f"by label; {sorted(expression_jobs - FORK_FALLBACK_JOBS)} would have " "no runner assertion at all" ) needing_ceilings = set(EXPECTED_PLACEMENT) - {CEILING_FREE_JOB} @@ -236,7 +221,8 @@ def test_the_job_carries_its_reviewed_label( ) -def test_the_runner_expression_parses_to_one_line() -> None: +@pytest.mark.parametrize("coordinate", sorted(FORK_FALLBACK_JOBS), ids=case_id) +def test_the_runner_expression_parses_to_one_line(coordinate: tuple[str, str]) -> None: """Scenario: the folded scalar's continuation is indented one level deeper. Invariant: the parsed value is a single line. A more-indented @@ -244,19 +230,20 @@ def test_the_runner_expression_parses_to_one_line() -> None: expression. GitHub evaluates the broken value and the job runs, so a green run is not evidence; only the parsed value shows it. """ - value = runner_value(jobs()[FORK_FALLBACK_JOB]) + value = runner_value(jobs()[coordinate]) assert isinstance(value, str), ( - f"{FORK_FALLBACK_JOB[0]}:{FORK_FALLBACK_JOB[1]} should declare runs-on " + f"{coordinate[0]}:{coordinate[1]} should declare runs-on " f"as one scalar, got {value!r}" ) assert "\n" not in value, ( - f"{FORK_FALLBACK_JOB[0]}:{FORK_FALLBACK_JOB[1]} has a line break inside " + f"{coordinate[0]}:{coordinate[1]} has a line break inside " f"its runs-on expression ({value!r}); the folded scalar's continuation " "line must sit at the same indent as the line above it" ) -def test_the_runner_expression_is_exactly_the_reviewed_one() -> None: +@pytest.mark.parametrize("coordinate", sorted(FORK_FALLBACK_JOBS), ids=case_id) +def test_the_runner_expression_is_exactly_the_reviewed_one(coordinate: tuple[str, str]) -> None: """Scenario: the fork guard is swapped for a plausible sibling field. Invariant: the guard and both arms equal the reviewed strings. Matching @@ -265,10 +252,10 @@ def test_the_runner_expression_is_exactly_the_reviewed_one() -> None: exactly the same form that sends every fork pull request to a runner it cannot obtain, and every other pull request to the wrong place. """ - value = str(runner_value(jobs()[FORK_FALLBACK_JOB])) + value = str(runner_value(jobs()[coordinate])) match = RUNNER_EXPRESSION.match(value) assert match is not None, ( - f"{FORK_FALLBACK_JOB[0]}:{FORK_FALLBACK_JOB[1]} should declare the " + f"{coordinate[0]}:{coordinate[1]} should declare the " f"reviewed fork fallback expression; got {value!r}" ) assert match["guard"] == HOSTED_GUARD, ( diff --git a/tests/workflow_contracts/ci_workflow_test.py b/tests/workflow_contracts/ci_workflow_test.py index b21dbe1e0..c1384a46d 100644 --- a/tests/workflow_contracts/ci_workflow_test.py +++ b/tests/workflow_contracts/ci_workflow_test.py @@ -208,8 +208,16 @@ def test_build_test_checks_out_at_the_default_depth() -> None: assert len(checkouts) == 1, ( f"build-test should check out exactly once; found {len(checkouts)}" ) - options = checkouts[0].get("with") or {} + checkout = checkouts[0] + uses = str(checkout.get("uses", "")) + assert re.fullmatch(r"actions/checkout@[0-9a-f]{40}", uses), ( + f"build-test must pin checkout to a full commit SHA, got {uses!r}" + ) + options = checkout.get("with") or {} assert isinstance(options, dict), "the checkout's with: must be a mapping" + assert options.get("persist-credentials") is False, ( + "build-test checkout must not persist credentials for test builds" + ) assert "fetch-depth" not in options, ( "build-test must check out at the default depth; it sets fetch-depth " f"{options.get('fetch-depth')!r}, and nothing in the lane reads history" diff --git a/tests/workflow_contracts/dependency_scoping_test.py b/tests/workflow_contracts/dependency_scoping_test.py new file mode 100644 index 000000000..a1a410ca4 --- /dev/null +++ b/tests/workflow_contracts/dependency_scoping_test.py @@ -0,0 +1,124 @@ +"""Protect dependency scopes in the root and verification manifests. + +Check the root manifest against example, request-streaming and doctest source +uses, and ensure the verification crate retains its model checker without a +runtime dependency on the main crate. +""" + +from __future__ import annotations + +import tomllib +import typing as typ +from pathlib import Path + +import pytest + +ROOT = Path(__file__).resolve().parents[2] + + +@pytest.fixture +def manifest() -> dict[str, typ.Any]: + """Read Cargo's dependency tables as structured data.""" + with (ROOT / "Cargo.toml").open("rb") as cargo_file: + return tomllib.load(cargo_file) + + +@pytest.mark.parametrize("name", ["tracing-subscriber", "metrics-exporter-prometheus", "itoa"]) +def test_test_dependencies_are_not_runtime_dependencies(manifest: dict[str, typ.Any], name: str) -> None: + """Test and doctest helpers must not enter the published runtime graph.""" + assert name not in manifest["dependencies"], ( + f"{name} must not be a runtime dependency in Cargo.toml" + ) + for target_name, target in manifest.get("target", {}).items(): + assert name not in target.get("dependencies", {}), ( + f"{name} must not be a runtime dependency for {target_name}" + ) + + +def test_tracing_subscriber_remains_available_to_tests(manifest: dict[str, typ.Any]) -> None: + """Examples, benchmarks and tests retain their tracing subscriber.""" + subscriber = manifest["dev-dependencies"]["tracing-subscriber"] + assert "env-filter" in subscriber["features"], ( + "tracing-subscriber must enable env-filter for benchmarks" + ) + + +def test_prometheus_exporter_remains_available_to_non_loom_doctests(manifest: dict[str, typ.Any]) -> None: + """The documented integration uses a development-only HTTP exporter.""" + exporter = manifest["target"]["cfg(not(loom))"]["dev-dependencies"][ + "metrics-exporter-prometheus" + ] + assert "http-listener" in exporter["features"], ( + "metrics-exporter-prometheus must enable http-listener for doctests" + ) + assert not exporter.get("optional", False), ( + "metrics-exporter-prometheus must be available to non-Loom doctests" + ) + assert "use metrics_exporter_prometheus::PrometheusBuilder;" in ( + ROOT / "src" / "metrics.rs" + ).read_text(), "src/metrics.rs must retain the documented Prometheus example" + + +def test_metrics_feature_does_not_enable_exporter(manifest: dict[str, typ.Any]) -> None: + """Runtime metrics emit through the facade without installing an exporter.""" + assert manifest["features"]["metrics"] == ["dep:metrics"], ( + "the metrics feature must enable only the runtime metrics facade" + ) + + +def test_request_stream_reader_enables_tokio_util_io_directly(manifest: dict[str, typ.Any]) -> None: + """Request streaming must not rely on exporter feature unification.""" + assert "io" in manifest["dependencies"]["tokio-util"]["features"], ( + "tokio-util must enable io directly for request streaming" + ) + assert "use tokio_util::io::StreamReader;" in ( + ROOT / "src" / "request" / "mod.rs" + ).read_text(), "src/request/mod.rs must retain its StreamReader import" + + +def test_itoa_remains_available_to_resp_example(manifest: dict[str, typ.Any]) -> None: + """The RESP example can format integers without a runtime dependency.""" + assert "itoa" in manifest["dev-dependencies"], ( + "itoa must remain a dev dependency for the RESP example" + ) + example = ROOT / "examples" / "resp_codec_impl" / "encode.rs" + assert "itoa::Buffer::new()" in example.read_text(), ( + "examples/resp_codec_impl/encode.rs must retain its itoa use" + ) + + +VERIFICATION_DEPENDENCY_TABLES = ("dependencies", "dev-dependencies") + + +def assert_verification_has_no_wireframe_dependency( + verification: dict[str, typ.Any], +) -> None: + """Reject a wireframe edge in either verification dependency table. + + A dev-dependency edge pulls the main crate into the verification build just + as a runtime edge does, so both tables need the same guard. + """ + assert "stateright" in verification["dependencies"], ( + "wireframe-verification must retain its stateright runtime dependency" + ) + for table in VERIFICATION_DEPENDENCY_TABLES: + dependencies = verification.get(table, {}) + assert "wireframe" not in dependencies, ( + f"wireframe-verification must not depend on wireframe via {table}" + ) + + +def test_verification_crate_retains_model_checker_without_wireframe() -> None: + """The model checker needs stateright but does not depend on wireframe.""" + path = ROOT / "crates" / "wireframe-verification" / "Cargo.toml" + with path.open("rb") as cargo_file: + assert_verification_has_no_wireframe_dependency(tomllib.load(cargo_file)) + + +@pytest.mark.parametrize("table", VERIFICATION_DEPENDENCY_TABLES) +def test_verification_guard_rejects_a_wireframe_edge(table: str) -> None: + """Either table admits the main crate only by defeating the guard.""" + verification: dict[str, typ.Any] = {"dependencies": {"stateright": "0.31.0"}} + verification.setdefault(table, {})["wireframe"] = {"path": "../.."} + with pytest.raises(AssertionError, match=table): + assert_verification_has_no_wireframe_dependency(verification) diff --git a/tests/workflow_contracts/nextest_timeout_test.py b/tests/workflow_contracts/nextest_timeout_test.py index 0d2367089..127cf0131 100644 --- a/tests/workflow_contracts/nextest_timeout_test.py +++ b/tests/workflow_contracts/nextest_timeout_test.py @@ -1,4 +1,19 @@ -"""Contracts for the coverage lane's cargo-nextest timeout policy.""" +"""Contract for the bounded, non-instrumented compile-time UI gate. + +Two policies share this module because both describe when compiler-driven work +is allowed to run. The first half pins the coverage lane's `cargo-nextest` +timeouts: the aggregate `compile_error` test gets an eight-minute override +while the global ceiling stays at ten, so a hung nested compiler run cannot +consume the whole job. + +The second half guards the split between compile-time validation and runtime +coverage in `ci.yml`. `build-test` must run `make test-ui` unconditionally, +after Rust and linker setup and with a finite budget, and it must not carry +coverage instrumentation. The gate itself keeps both harnesses, +`tests/compile_error.rs` and `wireframe_testing/tests/loom_api_surface.rs`, +and coverage opts out of exactly those two nested compiler drivers rather than +the runtime suite around them. +""" from __future__ import annotations @@ -138,3 +153,161 @@ def test_only_the_aggregate_compile_test_gets_the_extended_budget() -> None: "grace-period": "5s", } assert 60 * override["slow-timeout"]["terminate-after"] < 10 * 60 + + +def assert_ui_step_is_mandatory(job: dict[str, typ.Any]) -> None: + """Require bounded UI tests after Rust and linker setup in the required job.""" + assert "if" not in job, "build-test must run for every CI event" + assert not job.get("continue-on-error"), "build-test must propagate failures" + assert "needs" not in job, "build-test must run without a skipped prerequisite" + assert job.get("timeout-minutes") == 60, "build-test needs the reviewed budget" + steps = job["steps"] + ui_steps = [step for step in steps if step.get("name") == "Validate compile-time APIs"] + assert len(ui_steps) == 1, "build-test must contain exactly one UI step" + ui = ui_steps[0] + assert ui.get("run") == "make test-ui", "UI step must run the complete target" + assert ui.get("timeout-minutes") == 30, "UI step must have a finite budget" + assert "if" not in ui and not ui.get("continue-on-error"), ( + "UI step must run and propagate failures" + ) + assert_ui_setup_precedes(steps, ui) + assert_no_coverage_flags(job, ui) + + +def assert_ui_setup_precedes(steps: list[dict[str, typ.Any]], ui: dict[str, typ.Any]) -> None: + """Require Rust and linker setup before the non-instrumented UI step.""" + setup = [ + step for step in steps + if str(step.get("uses", "")).startswith( + "leynos/shared-actions/.github/actions/setup-rust@" + ) + ] + assert len(setup) == 1, "build-test must set up Rust" + assert "if" not in setup[0] and not setup[0].get("continue-on-error"), ( + "Rust setup must run and propagate failures" + ) + linker = [ + step for step in steps + if step.get("name") == "Install development backend and linker" + ] + assert len(linker) == 1, "build-test must install the Linux linker" + assert "if" not in linker[0] and not linker[0].get("continue-on-error"), ( + "linker setup must run and propagate failures" + ) + assert steps.index(setup[0]) < steps.index(linker[0]) < steps.index(ui), ( + "Rust and linker setup must precede UI tests" + ) + assert_no_coverage_flags(setup[0], linker[0]) + + +def assert_no_coverage_flags(*scopes: dict[str, typ.Any]) -> None: + """Keep coverage instrumentation out of the UI job and its setup.""" + for scope in scopes: + env = scope.get("env") or {} + flags = " ".join(str(env.get(key, "")) for key in ( + "RUSTFLAGS", "CARGO_ENCODED_RUSTFLAGS" + )) + assert "coverage" not in flags, "UI step must not enable coverage" + + +def assert_ui_permissions(job: dict[str, typ.Any]) -> None: + """Restrict the required UI job to read-only repository contents.""" + assert job.get("permissions") == {"contents": "read"}, ( + "build-test must declare exactly contents: read permissions" + ) + + +def test_ui_step_runs_without_suppression_for_all_ci_events() -> None: + """The required CI job runs a bounded non-instrumented UI gate.""" + workflow = typ.cast(dict[str, typ.Any], workflows()["ci.yml"]) + events = workflow.get(True, workflow.get("on")) + assert "pull_request" in events and "push" in events + workflow_flags = workflow.get("env") or {} + assert "coverage" not in str(workflow_flags.get("RUSTFLAGS", "")) + assert "coverage" not in str(workflow_flags.get("CARGO_ENCODED_RUSTFLAGS", "")) + assert "main" in events["push"]["branches"] + assert_ui_step_is_mandatory(workflow["jobs"]["build-test"]) + assert_ui_permissions(workflow["jobs"]["build-test"]) + + +@pytest.mark.parametrize("permissions", [None, {"contents": "write"}, {"contents": "read", "actions": "read"}]) +def test_ui_permissions_reject_missing_or_broader_access(permissions: object) -> None: + """The required UI gate must not omit or broaden its token permissions.""" + with pytest.raises(AssertionError, match="contents: read"): + assert_ui_permissions({"permissions": permissions}) + + +@pytest.mark.parametrize( + "mutation", ["conditional", "suppressed", "missing", "wrong_order", "no_timeout"] +) +def test_ui_step_contract_rejects_missing_or_ineffective_gate(mutation: str) -> None: + """Reject skipped, suppressed, missing, unbounded or prematurely run UI tests.""" + from copy import deepcopy + + workflow = typ.cast(dict[str, typ.Any], workflows()["ci.yml"]) + job = deepcopy(workflow["jobs"]["build-test"]) + ui = next(step for step in job["steps"] if step.get("name") == "Validate compile-time APIs") + if mutation == "conditional": + ui["if"] = "false" + elif mutation == "suppressed": + ui["continue-on-error"] = True + elif mutation == "missing": + job["steps"].remove(ui) + elif mutation == "no_timeout": + ui.pop("timeout-minutes") + else: + job["steps"].remove(ui) + job["steps"].insert(0, ui) + with pytest.raises(AssertionError): + assert_ui_step_is_mandatory(job) + + +UI_HARNESSES = frozenset({ + REPO_ROOT / "tests" / "compile_error.rs", + REPO_ROOT / "wireframe_testing" / "tests" / "loom_api_surface.rs", +}) + + +def assert_coverage_guard_scope(guarded: set[Path]) -> None: + """Keep coverage exclusion restricted to both compiler-driving tests.""" + assert guarded == UI_HARNESSES, "only the two UI harnesses need coverage guards" + + +def test_coverage_guards_only_compiler_driving_harnesses() -> None: + """Coverage excludes nested compiler runs while ordinary UI tests remain.""" + guarded = { + path + for root in (REPO_ROOT / "tests", REPO_ROOT / "wireframe_testing" / "tests") + for path in root.rglob("*.rs") + if "#![cfg(not(coverage))]" in path.read_text() + } + assert_coverage_guard_scope(guarded) + loom_harness = REPO_ROOT / "wireframe_testing" / "tests" / "loom_api_surface.rs" + assert "#![cfg(loom)]" in loom_harness.read_text() + manifest = tomllib.loads((REPO_ROOT / "Cargo.toml").read_text()) + checks = manifest["workspace"]["lints"]["rust"]["unexpected_cfgs"]["check-cfg"] + assert "cfg(loom)" in checks and "cfg(coverage)" in checks + + +def test_ui_target_keeps_both_harnesses() -> None: + """The non-instrumented gate runs root and Loom compile-time suites.""" + makefile = (REPO_ROOT / "Makefile").read_text() + target = makefile.split("\ntest-ui:", 1)[1].split("\n\n", 1)[0] + assert "test --test compile_error" in target + assert 'CARGO_ENCODED_RUSTFLAGS="--cfg=loom"' in target + assert ( + "test -p wireframe_testing --test loom_api_surface --features runtime-benchmarks" + in target + ) + + +@pytest.mark.parametrize("mutation", ["missing_guard", "unrelated_test"]) +def test_coverage_guard_scope_rejects_missing_or_extra_guards(mutation: str) -> None: + """A missing UI guard or an excluded runtime test violates the scope.""" + guarded = set(UI_HARNESSES) + if mutation == "missing_guard": + guarded.remove(REPO_ROOT / "tests" / "compile_error.rs") + else: + guarded.add(REPO_ROOT / "tests" / "request_handler.rs") + with pytest.raises(AssertionError): + assert_coverage_guard_scope(guarded) diff --git a/tests/workflow_contracts/runner_placement_policy.py b/tests/workflow_contracts/runner_placement_policy.py index af91098ee..054654d7e 100644 --- a/tests/workflow_contracts/runner_placement_policy.py +++ b/tests/workflow_contracts/runner_placement_policy.py @@ -52,14 +52,15 @@ #: The exact guard the fallback keys on: a fork or a Dependabot pull request. HOSTED_GUARD: typ.Final = f"({FORK_GUARD} || {DEPENDABOT_GUARD})" -#: The lane that carries the fork fallback: the only one serving pull -#: requests, and so the only one that can meet a fork. -FORK_FALLBACK_JOB: typ.Final = ("ci.yml", "build-test") +#: The pull-request job needs a fallback for forks and Dependabot. +FORK_FALLBACK_JOBS: typ.Final = frozenset({ + ("ci.yml", "build-test"), +}) #: Every job this repository places, with its reviewed label. A literal label #: is written as itself; the fork-fallback lane is checked by its expression #: instead and appears here as ``None`` so the coordinate set stays total. -#: ``test_the_reviewed_tables_are_total`` holds it to exactly one ``None``. +#: ``test_the_reviewed_tables_are_total`` holds it to the reviewed fallback jobs. EXPECTED_PLACEMENT: typ.Final = { ("ci.yml", "build-test"): None, ("coverage-main.yml", "coverage-upload"): UBICLOUD_LABEL, @@ -70,16 +71,16 @@ #: Ceilings, pinned by value rather than bounded, because a ceiling can drift #: to a number nobody chose while every inequality still holds. #: -#: ``build-test`` at 30 and ``coverage-upload`` at 20 are measured, from 556 s -#: of work on four-vCPU runners (750 s on two vCPU, still inside 30 minutes) -#: and 240 s on four-vCPU runners. ``advanced`` at 60 and +#: ``build-test`` at 60 includes a bounded cold UI suite and coverage; the +#: previous CI run reached 28 minutes. ``coverage-upload`` +#: at 20 was measured at 240 s on four-vCPU runners. ``advanced`` at 60 and #: ``refresh-sha`` at 10 are judgements and are recorded as such in the #: developers' guide: ``advanced`` has failed every scheduled run since at #: least 2026-09-09 and its last green run was 2025-10-04 at 32 s, which a #: Loom suite has long since outgrown, and ``refresh-sha`` has never run at #: all. EXPECTED_CEILING_MINUTES: typ.Final = { - ("ci.yml", "build-test"): 30, + ("ci.yml", "build-test"): 60, ("coverage-main.yml", "coverage-upload"): 20, ("advanced-tests.yml", "advanced"): 60, } diff --git a/wireframe_testing/tests/loom_api_surface.rs b/wireframe_testing/tests/loom_api_surface.rs index 757738400..9a3420eb3 100644 --- a/wireframe_testing/tests/loom_api_surface.rs +++ b/wireframe_testing/tests/loom_api_surface.rs @@ -1,6 +1,7 @@ //! Check that network-backed test helpers stay outside the Loom API surface. #![cfg(loom)] +#![cfg(not(coverage))] #[test] fn loom_build_omits_non_loom_helpers() { diff --git a/wireframe_testing/tests/ui/loom_non_loom_exports.stderr b/wireframe_testing/tests/ui/loom_non_loom_exports.stderr index 48e035edb..41803716d 100644 --- a/wireframe_testing/tests/ui/loom_non_loom_exports.stderr +++ b/wireframe_testing/tests/ui/loom_non_loom_exports.stderr @@ -2,7 +2,9 @@ error[E0432]: unresolved import `wireframe_testing::client_pair` --> tests/ui/loom_non_loom_exports.rs:4:5 | 4 | use wireframe_testing::client_pair as _; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `client_pair` in the root + | ^^^^^^^^^^^^^^^^^^^-----------^^^^^ + | | + | no `client_pair` in the root | note: found an item that was configured out --> src/lib.rs @@ -16,7 +18,9 @@ error[E0432]: unresolved import `wireframe_testing::runtime_ownership_benchmarks --> tests/ui/loom_non_loom_exports.rs:5:5 | 5 | use wireframe_testing::runtime_ownership_benchmarks as _; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `runtime_ownership_benchmarks` in the root + | ^^^^^^^^^^^^^^^^^^^----------------------------^^^^^ + | | + | no `runtime_ownership_benchmarks` in the root | note: found an item that was configured out --> src/lib.rs @@ -30,7 +34,9 @@ error[E0432]: unresolved import `wireframe_testing::wait_for_listener_release` --> tests/ui/loom_non_loom_exports.rs:6:5 | 6 | use wireframe_testing::wait_for_listener_release as _; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `wait_for_listener_release` in the root + | ^^^^^^^^^^^^^^^^^^^-------------------------^^^^^ + | | + | no `wait_for_listener_release` in the root | note: found an item that was configured out --> src/lib.rs @@ -44,11 +50,17 @@ error[E0432]: unresolved imports `wireframe_testing::WireframePair`, `wireframe_ --> tests/ui/loom_non_loom_exports.rs:8:5 | 8 | WireframePair as _, - | ^^^^^^^^^^^^^^^^^^ no `WireframePair` in the root + | -------------^^^^^ + | | + | no `WireframePair` in the root 9 | spawn_wireframe_pair as _, - | ^^^^^^^^^^^^^^^^^^^^^^^^^ no `spawn_wireframe_pair` in the root + | --------------------^^^^^ + | | + | no `spawn_wireframe_pair` in the root 10 | spawn_wireframe_pair_default as _, - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `spawn_wireframe_pair_default` in the root + | ----------------------------^^^^^ + | | + | no `spawn_wireframe_pair_default` in the root | note: found an item that was configured out --> src/lib.rs