From 21f32d07fa9e1e072bc14cc6156f9ca6802bd0f8 Mon Sep 17 00:00:00 2001 From: Martin Tomka Date: Thu, 9 Jul 2026 14:43:32 +0200 Subject: [PATCH 1/3] ci: run coverage via nextest to fix false tracing-macro coverage gaps The coverage job used the default cargo test/libtest harness (all tests in one process). tracing caches callsite interest process-globally, so a no-subscriber test can register a racing::event! callsite as never-interested before a subscriber-installing test runs, leaving the macro's enabled branch (field-value expressions) recorded as uncovered nondeterministically across platforms. This surfaced as spurious Codecov misses on log macro lines. Switch to cargo llvm-cov nextest (one process per test), matching the just coverage-measure recipe, so each test runs isolated and the enabled branch is measured deterministically. Installs cargo-nextest in the job. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: a49a8937-3f70-47a4-aaad-a24a82a7639b --- .github/workflows/main.yml | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 5a10505bb..1244520ba 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -395,13 +395,21 @@ jobs: enable-sccache: true rust-toolchain: RUST_NIGHTLY rust-components: llvm-tools-preview - cargo-tools: CARGO_LLVM_COV_VERSION + cargo-tools: CARGO_LLVM_COV_VERSION, CARGO_NEXTEST_VERSION # execute + # Coverage is generated with `llvm-cov nextest` (one process per test) instead + # of the default `cargo test` harness (all tests in one process). Process + # isolation prevents cross-test contamination of tracing's process-global + # callsite-interest cache: with the single-process harness, a no-subscriber test + # can register a `tracing::event!` callsite as "never interested" before a + # subscriber-installing test runs, leaving the macro's enabled branch (its + # field-value expressions) recorded as uncovered nondeterministically across + # platforms. This matches the `just coverage-measure` recipe (justfiles/coverage.just). - name: Generate Coverage (all-features) - run: cargo +${{ env.RUST_NIGHTLY }} llvm-cov --all-features --workspace --lcov --output-path lcov-all.info + run: cargo +${{ env.RUST_NIGHTLY }} llvm-cov nextest --all-features --workspace --lcov --output-path lcov-all.info - name: Generate Coverage (no-default-features) - run: cargo +${{ env.RUST_NIGHTLY }} llvm-cov --no-default-features --workspace --lcov --output-path lcov-no-def.info + run: cargo +${{ env.RUST_NIGHTLY }} llvm-cov nextest --no-default-features --workspace --lcov --output-path lcov-no-def.info - name: Upload Coverage to Codecov uses: codecov/codecov-action@v7.0.0 with: From 60473d211294826fe3a9ae001435c2a0821a34dc Mon Sep 17 00:00:00 2001 From: Martin Tomka Date: Thu, 9 Jul 2026 16:15:19 +0200 Subject: [PATCH 2/3] fix(coverage): make coverage_attribute gates nextest-compatible Switching the coverage job to `cargo llvm-cov nextest` makes it compile the non-test library of leaf crates that the previous `cargo test` harness never built. Under the coverage build's `-D warnings` this surfaced pre-existing latent issues: - `automation` and `fetch_hyper` apply `#[coverage(off)]` to non-test items (a crate-level attribute and the no-TLS `no_tls_backend_unreachable` fallback), but gated `feature(coverage_attribute)` on `test` only, so the non-test lib failed to compile (E0658). Enable the feature whenever `coverage_nightly` is set. - `hyper_handler` test module redundantly re-imported `BodyExt`/`Service` (already in scope via `use super::*`); unused under no-default-features. - Allow the `unused_features` lint: leaf crates whose `coverage(off)` is entirely feature/`cfg(test)`-gated leave the coverage-attribute gate unused in some target/feature combinations, which is a benign instrumentation artifact. Verified locally: `llvm-cov nextest` all-features (4752 tests) and no-default-features (3882 tests) both build and pass clean under -D warnings. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: a49a8937-3f70-47a4-aaad-a24a82a7639b --- Cargo.toml | 9 +++++++++ crates/automation/src/lib.rs | 2 +- crates/fetch_hyper/src/connection/hyper_handler.rs | 2 -- crates/fetch_hyper/src/lib.rs | 2 +- 4 files changed, 11 insertions(+), 4 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 4782905b5..f2f07f012 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -211,6 +211,15 @@ unreachable_pub = "warn" unsafe_op_in_unsafe_fn = "warn" unused_lifetimes = "warn" +# Coverage builds (`cargo llvm-cov nextest`) compile the non-test library of +# leaf crates whose `coverage(off)` attributes are all gated behind cargo +# features or `cfg(test)`. In feature/target combinations where none of those +# attributes are compiled, the crate-level `#![feature(coverage_attribute)]` +# gate (enabled by the `coverage_nightly` cfg) is technically unused. That is a +# benign artifact of instrumentation, not a real problem, so allow it rather +# than encoding brittle per-crate feature conditions on every coverage gate. +unused_features = "allow" + # Allow our special-purpose nonstandard cfg attributes. # - coverage / coverage_nightly: set by cargo-llvm-cov to enable nightly coverage attributes. # - loom: set via RUSTFLAGS to swap in the loom model-checker for atomics in select crates. diff --git a/crates/automation/src/lib.rs b/crates/automation/src/lib.rs index 9c8f03ff2..2a611e5a7 100644 --- a/crates/automation/src/lib.rs +++ b/crates/automation/src/lib.rs @@ -4,7 +4,7 @@ //! An unpublished crate for shared code used for writing Rust scripts #![allow(clippy::missing_errors_doc, reason = "this is an internal crate for scripts")] -#![cfg_attr(all(coverage_nightly, test), feature(coverage_attribute))] +#![cfg_attr(coverage_nightly, feature(coverage_attribute))] #![cfg_attr(coverage_nightly, coverage(off))] mod cargo; diff --git a/crates/fetch_hyper/src/connection/hyper_handler.rs b/crates/fetch_hyper/src/connection/hyper_handler.rs index 73d3c7b90..d4d69991c 100644 --- a/crates/fetch_hyper/src/connection/hyper_handler.rs +++ b/crates/fetch_hyper/src/connection/hyper_handler.rs @@ -192,9 +192,7 @@ mod tests { use bytes::Bytes; use fetch_options::{ConnectionLifetime, PoolIndex, RequestFilter}; use http::Version; - use http_body_util::BodyExt as _; use http_extensions::{HttpBodyBuilder, HttpRequestBuilder}; - use layered::Service as _; use super::*; use crate::HyperTransport; diff --git a/crates/fetch_hyper/src/lib.rs b/crates/fetch_hyper/src/lib.rs index 2cf57ceee..9e23c6f40 100644 --- a/crates/fetch_hyper/src/lib.rs +++ b/crates/fetch_hyper/src/lib.rs @@ -1,7 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -#![cfg_attr(all(coverage_nightly, test), feature(coverage_attribute))] +#![cfg_attr(coverage_nightly, feature(coverage_attribute))] #![cfg_attr(docsrs, feature(doc_cfg))] #![doc(html_logo_url = "https://media.githubusercontent.com/media/microsoft/oxidizer/refs/heads/main/crates/fetch_hyper/logo.png")] #![doc(html_favicon_url = "https://media.githubusercontent.com/media/microsoft/oxidizer/refs/heads/main/crates/fetch_hyper/favicon.ico")] From 193fc114c13b6beb6d10faaea6d0104f51927d3b Mon Sep 17 00:00:00 2001 From: Martin Tomka Date: Thu, 9 Jul 2026 16:47:47 +0200 Subject: [PATCH 3/3] fix(cachet): drop uncoverable no-op logging fallbacks The record_{debug,info,error}_with_duration helpers had a `#[cfg(not(any(feature = "logs", test)))] { let _ = (..); }` block whose sole job was to consume the parameters when the logging path is compiled out. That block is only present in non-test, no-`logs` builds, so no test can ever execute it (any test build sets `cfg(test)`). The nextest coverage job compiles cachet's non-test library under `--no-default-features`, which made those lines count as permanently-uncovered and dropped project coverage below 100%. Replace the no-op blocks with a cfg-gated `allow(unused_variables)` so the parameters may go unused without emitting a coverage region. Behaviour is unchanged (both were no-ops); the logging path is still covered via the all-features report. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: a49a8937-3f70-47a4-aaad-a24a82a7639b --- crates/cachet/src/telemetry/cache.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/crates/cachet/src/telemetry/cache.rs b/crates/cachet/src/telemetry/cache.rs index 8fb7037eb..2ed8b14a4 100644 --- a/crates/cachet/src/telemetry/cache.rs +++ b/crates/cachet/src/telemetry/cache.rs @@ -142,6 +142,10 @@ impl CacheTelemetry { not(feature = "logs"), expect(clippy::unused_self, reason = "self.logging_enabled is used when logs is enabled") )] + #[cfg_attr( + not(any(feature = "logs", test)), + allow(unused_variables, reason = "consumed only by the logging path, which is compiled out here") + )] fn record_debug_with_duration(&self, cache_name: CacheName, event: &'static str, duration: Duration) { #[cfg(any(feature = "logs", test))] if self.logging_enabled { @@ -151,16 +155,16 @@ impl CacheTelemetry { cache.duration_ns = duration.as_nanos() ); } - #[cfg(not(any(feature = "logs", test)))] - { - let _ = (cache_name, event, duration); - } } #[cfg_attr( not(feature = "logs"), expect(clippy::unused_self, reason = "self.logging_enabled is used when logs is enabled") )] + #[cfg_attr( + not(any(feature = "logs", test)), + allow(unused_variables, reason = "consumed only by the logging path, which is compiled out here") + )] fn record_info_with_duration(&self, cache_name: CacheName, event: &'static str, duration: Duration) { #[cfg(any(feature = "logs", test))] if self.logging_enabled { @@ -170,16 +174,16 @@ impl CacheTelemetry { cache.duration_ns = duration.as_nanos() ); } - #[cfg(not(any(feature = "logs", test)))] - { - let _ = (cache_name, event, duration); - } } #[cfg_attr( not(feature = "logs"), expect(clippy::unused_self, reason = "self.logging_enabled is used when logs is enabled") )] + #[cfg_attr( + not(any(feature = "logs", test)), + allow(unused_variables, reason = "consumed only by the logging path, which is compiled out here") + )] fn record_error_with_duration(&self, cache_name: CacheName, event: &'static str, duration: Duration) { #[cfg(any(feature = "logs", test))] if self.logging_enabled { @@ -189,10 +193,6 @@ impl CacheTelemetry { cache.duration_ns = duration.as_nanos() ); } - #[cfg(not(any(feature = "logs", test)))] - { - let _ = (cache_name, event, duration); - } } pub(crate) fn record_hit(&self, tier_name: CacheName, duration: Duration, fallback: bool) {