Rust Testing Code Review

Rust Testing Code Review

Review Workflow

1. Check Rust edition — Note edition in Cargo.toml (2021 vs 2024). Edition 2024 changes temporary scoping in if let and tail expressions, and makes #[expect] the preferred lint suppression 2. Check test organization — Unit tests in #[cfg(test)] modules, integration tests in tests/ directory 3. Check async test setup — #[tokio::test] for async tests, proper runtime configuration. Check for async-trait on mocks that could use native async fn in traits 4. Check assertions — Meaningful messages, correct assertion type. Review if let assertions for edition 2024 temporary scope changes 5. Check test isolation — No shared mutable state between tests, proper setup/teardown. Prefer LazyLock over lazy_static!/once_cell for shared fixtures 6. Check coverage patterns — Error paths tested, edge cases covered

Gates (hard)

Do not advance to Output Format until each pass condition is satisfied (yes/no with a concrete artifact).

1. Edition recorded — Open the target crate’s Cargo.toml (or workspace [workspace.package] / inherited edition) and note the edition value. Pass: you can quote edition = "…" (or document “inherited from workspace”) before citing Rust 2024–specific behavior (if let / tail temporary drops, #[expect] vs #[allow] migration, native async fn in traits as default). If edition is not 2024, do not report those items as edition-2024 regressions; at most Informational if still useful. 2. `dyn` vs static async mocks — Before suggesting native async fn in traits instead of async-trait, check whether the mock is used as dyn Trait. Pass: if dyn is required, you either skip that suggestion or align with Valid Patterns (async-trait still needed). 3. Verification protocol — Pass: steps from the review-verification-protocol skill are done before any finding is listed (see Before Submitting Findings).

Output Format

Report findings as:

[FILE:LINE] ISSUE_TITLE
Severity: Critical | Major | Minor | Informational
Description of the issue and why it matters.

Quick Reference

Review Checklist

Test Structure

• [ ] Unit tests in #[cfg(test)] mod tests within source files
• [ ] Integration tests in tests/ directory (one file per module or feature)
• [ ] use super::* in test modules to access parent module items
• [ ] Test function names describe the scenario: test_<function>_<scenario>_<expected>
• [ ] Tests are independent — no reliance on execution order

Async Tests

• [ ] #[tokio::test] used for async test functions
• [ ] #[tokio::test(flavor = "multi_thread")] when testing multi-threaded behavior
• [ ] No block_on inside async tests (use .await directly)
• [ ] Test timeouts set for tests that could hang
• [ ] Mock traits use native async fn instead of async-trait crate (stable since Rust 1.75)

Assertions

• [ ] assert_eq! / assert_ne! used for value comparisons (better error messages than assert!)
• [ ] Custom messages on assertions that aren't self-documenting
• [ ] matches! macro used for enum variant checking
• [ ] Error types checked with matches! or pattern matching, not string comparison
• [ ] One assertion per test where practical (easier to diagnose failures)
• [ ] if let assertions reviewed for edition 2024 temporary scope — temporaries in conditions drop earlier, may invalidate borrows
• [ ] Tail expression returns reviewed for edition 2024 — temporaries in tail expressions drop before local variables

Mocking and Test Doubles

• [ ] Traits used as seams for dependency injection (not concrete types)
• [ ] Mock implementations kept minimal — only what the test needs
• [ ] No mocking of types you don't own (wrap external dependencies behind your own trait)
• [ ] Test fixtures as helper functions, not global state
• [ ] std::sync::LazyLock used for shared test fixtures instead of lazy_static! or once_cell (stable since Rust 1.80)

Error Path Testing

• [ ] Result::Err variants tested, not just happy paths
• [ ] Specific error variants checked (not just "is error")
• [ ] #[should_panic] used sparingly — prefer Result-returning tests

Lint Suppression in Tests

• [ ] #[expect(lint)] used instead of #[allow(lint)] for test-specific suppressions (stable since Rust 1.81)
• [ ] Justification comment on every #[expect] or #[allow] in test code
• [ ] Stale #[allow] attributes migrated to #[expect] for self-cleaning behavior

Test Naming

• [ ] Test names read like sentences describing behavior (not test_happy_path)
• [ ] Related tests grouped in nested mod blocks for organization
• [ ] Test names follow pattern: <function>_should_<behavior>_when_<condition>

Snapshot Testing

• [ ] cargo insta used for complex structural output (JSON, YAML, HTML, CLI output)
• [ ] Snapshots are small and focused (not huge objects)
• [ ] Redactions used for unstable fields (timestamps, UUIDs)
• [ ] Snapshots committed to git in snapshots/ directory
• [ ] Simple values use assert_eq!, not snapshots

Parametrized Testing

• [ ] rstest used to avoid duplicated test functions for similar inputs
• [ ] #[rstest] with #[case::name] attributes for descriptive parametrized tests
• [ ] #[fixture] used for shared test setup when multiple tests need same construction
• [ ] Parametrized tests still have descriptive case names (not just #[case(1)])
• [ ] Combined with async: #[rstest] #[tokio::test] for async parametrized tests

Doc Tests

• [ ] Public API functions have /// # Examples with runnable code
• [ ] Doc tests serve as both documentation and correctness checks
• [ ] Hidden setup lines prefixed with # to keep examples clean
• [ ] cargo test --doc passes (nextest doesn't run doc tests)

Concurrency Testing

Detailed guidance: references/concurrency-testing.md

• [ ] Hand-rolled atomics / unsafe impl Send|Sync / state machines have a #[cfg(loom)] test using loom::sync and loom::thread shims (not std::sync)
• [ ] Crates with unsafe touching atomics, pointers, or UnsafeCell run cargo +nightly miri test --all-features in CI on every PR (not release-only, not module-level cfg_attr(miri, ignore))
• [ ] Lock-free data structures (AtomicPtr stacks/queues/lists) have loom + Miri (Stacked Borrows and Tree Borrows) + proptest-driven operation sequences
• [ ] Nondeterministic inputs (Instant::now, rand, env, HashMap iteration) are kept out of loom::model bodies
• [ ] Loom jobs run in --release with a LOOM_MAX_PREEMPTIONS bound; loom tests live in a separate tests/ file so --cfg loom does not poison normal cargo test
• [ ] nextest run -j1 is not cited as evidence of race-condition coverage; FFI-heavy unsafe extern "C" paths have a ThreadSanitizer job

Test Augmentation (Fakes, Mocks, Stubs, Spies)

Detailed guidance: references/advanced-testing.md

• [ ] Test doubles are typed by purpose: stub (canned data), fake (working but simplified impl), mock (pre-programmed expectations + verification), spy (records calls for after-the-fact inspection) — vocabulary is used precisely
• [ ] mockall-style mocks are not used where a fake would be simpler (in-memory DB beats expectation-heavy mocks for repository-style traits)
• [ ] Fakes are tested against the same contract as the production impl (trait conformance test); no drift
• [ ] Trait-as-seam pattern used to inject test doubles — production code depends on the trait, not the concrete type

Performance Tests (Criterion)

Detailed guidance: references/advanced-testing.md

• [ ] Benchmarks use criterion::black_box(...) to prevent constant-folding; pointer-flavored inputs use black_box(input.as_ptr()) not black_box(&input)
• [ ] Benchmarks isolate I/O — setup (file open, allocation) in iter_batched setup closure, NOT inside the measured closure
• [ ] CI persists a baseline via cargo bench -- --save-baseline main; feature branches compare with --baseline main; regression threshold defined and enforced
• [ ] Criterion benchmarks build with --profile bench (release optimization + debug symbols for flamegraph correlation)
• [ ] No #[bench] (unstable, deprecated) — use criterion exclusively
• [ ] Single-shot timings are not cited as evidence; reports show mean + variance from criterion's statistical engine

Test Generation

Detailed guidance: references/advanced-testing.md

• [ ] Table-driven inputs use rstest with #[case::name(...)] (descriptive case names in test output)
• [ ] Multi-axis input matrices use rstest with #[values(...)] on multiple parameters (Cartesian product), not hand-expanded test functions
• [ ] Per-input-named tests for large corpora use paste! macro or build.rs codegen (one #[test] fn per file/case)
• [ ] Generated tests have stable names that survive CI log diffs

Proc-Macro UI Tests (trybuild)

Detailed guidance: references/advanced-testing.md

• [ ] Proc-macros that emit compile errors have trybuild::TestCases covering each failure path with a .stderr reference
• [ ] .stderr outputs were regenerated after the latest rustc bump (otherwise spurious CI failures); TRYBUILD=overwrite cargo test is documented in CONTRIBUTING
• [ ] CI pins a specific stable rustc for trybuild jobs (the .stderr format shifts between stable releases)
• [ ] trybuild tests skipped on nightly (nightly diagnostics differ from stable)

Clippy Lint Group Strategy

Detailed guidance: references/advanced-testing.md

• [ ] clippy::correctness is deny (always — these are bugs)
• [ ] clippy::suspicious is warn or deny for libraries
• [ ] clippy::perf is warn (real wins on hot paths)
• [ ] clippy::pedantic enabled for libraries; suppressions use #[expect(clippy::lint_name, reason = "...")] with justification, not bare #[allow]
• [ ] clippy::nursery is NOT enabled in CI (experimental; changes between rustc releases)
• [ ] clippy::restriction lints are opt-in individually; the group is never enabled wholesale

Severity Calibration

Critical

• Tests that pass but don't actually verify behavior (assertions on wrong values)
• Shared mutable state between tests causing flaky results
• Missing error path tests for security-critical code

Major

• #[should_panic] without expected message (catches any panic, including wrong ones)
• unwrap() in test setup that hides the real failure location
• Tests that depend on execution order
• if let with inline temporary in assertion that breaks under edition 2024 temporary scoping
• async-trait on mock traits when native async fn in traits is available and project targets edition 2024

Minor

• Missing assertion messages on complex comparisons
• assert!(x == y) instead of assert_eq!(x, y) (worse error messages)
• Test names that don't describe the scenario
• Redundant setup code that could be extracted to a helper
• #[allow] used where #[expect] would provide self-cleaning suppression
• lazy_static! or once_cell used for test fixtures when LazyLock is available

Informational

• Suggestions to add property-based tests via proptest or quickcheck
• Suggestions to add snapshot testing for complex output
• Coverage improvement opportunities

Valid Patterns (Do NOT Flag)

• `unwrap()` / `expect()` in tests — Panicking on unexpected errors is the correct test behavior
• *`use super::` in test modules** — Standard pattern for accessing parent items
• `#[allow(dead_code)]` on test helpers — Helper functions may not be used in every test
• `clone()` in tests — Clarity over performance
• Large test functions — Integration tests can be long; extracting helpers isn't always clearer
• `assert!` for boolean checks — Fine when the expression is clearly boolean (.is_some(), .is_empty())
• Multiple assertions testing one logical behavior — Sometimes one behavior needs multiple checks
• `unwrap()` on `Result`-returning test functions — Propagating with ? is also fine but not required
• `async-trait` on mock traits requiring `dyn` dispatch — Native async fn in traits doesn't support dyn Trait; async-trait is still needed there
• `#[expect]` with justification on test helpers — Self-cleaning lint suppression is correct in test code
• `LazyLock` for expensive shared test fixtures — Thread-safe lazy init is appropriate for test globals

Before Submitting Findings

Load and follow the review-verification-protocol skill before reporting any issue.

Advanced Testing

Fuzzing

Fuzzing generates semi-random inputs to find crashes. Modern fuzzers use code coverage to explore paths efficiently. Use for parsers, deserializers, codec implementations, and anything accepting untrusted input.

cargo-fuzz with libfuzzer

#![no_main]
use libfuzzer_sys::fuzz_target;

fuzz_target!(|data: &[u8]| {
    if let Ok(s) = std::str::from_utf8(data) {
        let _ = url::Url::parse(s); // looking for panics, not checking results
    }
});

For complex types, derive Arbitrary to convert raw bytes into structured inputs:

#[derive(arbitrary::Arbitrary, Debug)]
struct FuzzInput { key: String, value: Vec<u8>, ttl: u32 }

fuzz_target!(|input: FuzzInput| {
    let mut cache = Cache::new();
    cache.insert(&input.key, &input.value, input.ttl);
});

Flag when:

• Fuzz targets exist without a corpus/ directory (no seed inputs)
• Fuzz targets check return values instead of letting panics surface
• Parsers or protocol handlers lack fuzz targets entirely

Property-Based Testing

Verifies invariants hold across generated inputs rather than checking specific cases.

use proptest::prelude::*;

proptest! {
    #[test]
    fn round_trip_serialization(input in any::<MyStruct>()) {
        let bytes = input.serialize();
        let decoded = MyStruct::deserialize(&bytes).unwrap();
        prop_assert_eq!(input, decoded);
    }

    #[test]
    fn sort_is_idempotent(mut v in prop::collection::vec(any::<i32>(), 0..100)) {
        v.sort();
        let sorted = v.clone();
        v.sort();
        prop_assert_eq!(v, sorted);
    }
}

Test stateful types with operation sequences via Vec<Op> where Op is an enum of possible actions. Testers minimize failing sequences automatically.

Flag when:

• proptest strategies are overly narrow (e.g., 1..5 when valid range is 0..u64::MAX)
• Property tests check only success, not invariants (no prop_assert!)
• Data structures lack operation-sequence testing for stateful invariants

Miri

Miri interprets Rust's MIR to detect undefined behavior in unsafe code. Run with cargo +nightly miri test.

Catches: Uninitialized memory reads, use-after-free, out-of-bounds pointer access, invalid exclusive references (Stacked Borrows violations), misaligned accesses. Misses: Data races (use Loom), logic bugs, performance issues, FFI calls to non-Rust code.

Flag when:

• Crate contains unsafe blocks but CI does not run cargo miri test
• Miri is disabled for tests that exercise unsafe code paths
• Raw pointer arithmetic lacks Miri coverage

Loom

Exhaustively tests concurrent code by exploring all thread interleavings at synchronization points.

#[test]
fn concurrent_counter() {
    loom::model(|| {
        let counter = loom::sync::Arc::new(loom::sync::atomic::AtomicUsize::new(0));
        let c1 = counter.clone();
        let t = loom::thread::spawn(move || {
            c1.fetch_add(1, Ordering::SeqCst);
        });
        counter.fetch_add(1, Ordering::SeqCst);
        t.join().unwrap();
        assert_eq!(counter.load(Ordering::SeqCst), 2);
    });
}

When to use Loom: Lock-free data structures, custom synchronization primitives, code using Ordering weaker than SeqCst. Regular #[tokio::test] is sufficient for high-level async workflows.

Flag when:

• Lock-free or atomic-based concurrency code has only regular tests
• Loom tests use std::sync instead of loom::sync (defeats the purpose)

Benchmarking Rigor

criterion with black_box

use criterion::{black_box, criterion_group, criterion_main, Criterion};

fn bench_parse(c: &mut Criterion) {
    let input = "https://example.com/path?query=value";
    c.bench_function("url_parse", |b| {
        b.iter(|| url::Url::parse(black_box(input)))
    });
}

criterion_group!(benches, bench_parse);
criterion_main!(benches);

Without black_box, the compiler may eliminate the entire computation as dead code. Use black_box on mutable pointer (as_ptr()) rather than shared reference -- the compiler can legally assume shared references are not mutated.

Flag when:

• Benchmarks do not use black_box on inputs or outputs
• Benchmark loop body includes I/O (println!, logging) or RNG that dominates measured time
• Benchmarks run once instead of using criterion's statistical sampling
• No harness = false in Cargo.toml for criterion benchmark targets

compile_fail Tests

Verify code correctly fails to compile. Useful for type-level safety guarantees (Send, Sync, lifetimes).

Doctests: compile_fail attribute on doc code blocks. Crude -- passes for any compilation failure including typos. trybuild: Fine-grained compile-fail testing. Each .rs file in tests/ui/ has a matching .stderr with the expected error.

#[test]
fn compile_fail_tests() {
    let t = trybuild::TestCases::new();
    t.compile_fail("tests/ui/*.rs");
}

Flag when:

• compile_fail doctests lack a comment explaining which error is expected
• Crate enforces type-level invariants without compile_fail tests
• trybuild .stderr files are outdated after a rustc version bump

Test Harness Customization

Set harness = false in Cargo.toml for custom test runners (fuzzers, model checkers, criterion benchmarks, WebAssembly targets). Without the harness, #[test] attributes are silently ignored -- you write your own main.

Flag when:

• harness = false set but test file still uses #[test] attributes
• Custom harness does not handle --test-threads or --nocapture when needed

Mocking Strategies

Trait-based (primary pattern): Make code generic over traits, substitute mocks in tests. See integration-tests.md for async trait mock examples. Conditional compilation: Use #[cfg(test)] to swap implementations when generics are inconvenient (e.g., deterministic timestamps, fixed randomness). mockall: Generates mocks via #[automock]. Set times() constraints on expectations to catch unexpected call counts.

#[automock]
trait Storage {
    fn get(&self, key: &str) -> Option<String>;
    fn set(&self, key: &str, value: &str);
}

#[test]
fn cache_miss_fetches_from_source() {
    let mut mock = MockStorage::new();
    mock.expect_get().with(eq("key")).returning(|_| None);
    mock.expect_set().with(eq("key"), eq("value")).times(1).return_const(());
    let svc = Service::new(mock);
    svc.fetch("key");
}

Flag when:

• Mocking external types directly instead of wrapping behind an owned trait
• #[cfg(test)] mocks change behavior that could mask production bugs
• mockall expectations lack times() constraints

Review Rules Summary

Test Augmentation Taxonomy

The earlier "Mocking Strategies" section conflates four distinct patterns. The taxonomy below makes the distinctions Jon Gjengset draws in Rust for Rustaceans Ch 6 explicit. Pick the lightest pattern that proves what you need.

• Stub — returns canned values for a fixed input shape. No state, no recording, no expectations. A function fn now() -> Instant { fixed } is a stub. Use when the test only needs a deterministic input to a downstream computation.
• Fake — a working alternative implementation of the same trait/contract, with simplified internals (in-memory HashMap backing a Repository, in-memory VecDeque backing a Queue, a deterministic monotonic clock). Has state and correct end-to-end behavior; just smaller, faster, and process-local. Use when the production code is generic over the trait.
• Mock — pre-programmed with expectations. The test declares "this method must be called N times with these arguments" up front. Tears down with assertion failures if expectations aren't met. mockall::automock is the standard tool. Use when the interaction pattern itself is the property under test.
• Spy — records calls in a Vec<(method, args)> field without expectations. The test inspects the recording after the fact. Use when calls are unordered or branch-dependent and a mock's strict expectation graph is too rigid.

Trait-as-seam pattern. Production code is generic over a trait the test substitutes. The seam lives at the trait boundary, not at #[cfg(test)] swaps:

pub trait Repository {
    fn get(&self, id: u64) -> Option<Row>;
    fn put(&self, row: Row);
}

pub struct Service<R: Repository> { repo: R }
// production: Service<PgRepository>
// tests:      Service<InMemoryRepository>  // a fake

The InMemoryRepository fake implements Repository with a Mutex<HashMap<u64, Row>>. Tests exercise the real Service logic against a process-local store; no #[cfg(test)] divergence in the production code path.

Review checks:

• [FILE:LINE] MOCKALL_WHERE_FAKE_FITS — Test uses #[automock] to script expect_get().returning(|_| Some(row)) for many calls, when an in-memory fake implementing the same trait would be smaller, more readable, and exercise the production code path identically.
• [FILE:LINE] FAKE_DIVERGES_FROM_PROD — In-memory Repository fake silently sorts entries or returns insertion order, but the production Postgres implementation returns rows by primary key. Tests will pass against bugs the production wiring exposes.
• [FILE:LINE] NO_TRAIT_SEAM — Service directly owns a PgPool. There is no trait boundary to substitute in tests, so every test must spin up Postgres. Introduce a Repository trait and parameterize Service over it.
• [FILE:LINE] SPY_PRESENTED_AS_MOCK — A struct records calls into a Vec but is named MockFoo and reviewed as if it asserts. The test inspects the vec after, so it's a spy. Rename and document, or add explicit expect_* assertions.
• [FILE:LINE] STUB_LEAKS_TEST_API — Stub returns hard-coded data via a pub fn for_tests() constructor that is reachable from production builds. Gate behind #[cfg(test)] or move to tests/common/.

Test Generation Strategies

Beyond hand-written #[test] fn and the rstest examples in integration-tests.md, four generation strategies cover most data shapes.

• `rstest` with `#[case(...)]` — table-driven tests with descriptive case names. Each #[case] becomes a separately-named test in cargo output. Good for a small, hand-curated set of inputs where each row tells a story (each #[case] documents one behavior).
• `rstest` matrix — Cartesian product across parameters via #[values(a, b, c)] on each argument. Three values on one axis and three on another yields nine generated tests. Good when every combination must be exercised and the cases share assertions:

  use rstest::rstest;
  #[rstest]
  fn handles_combinations(
      #[values("ascii", "utf8", "mixed")] encoding: &str,
      #[values(0, 1, 1024)] size: usize,
  ) {
      assert!(roundtrip(encoding, size).is_ok());
  }

• `paste!` macro — generates uniquely-named #[test] fn for a list of inputs by concatenating identifiers. Use when each input deserves a stable, greppable name in CI output (e.g., one test per supported format), and the body is identical except for one substitution.
• `build.rs`-generated tests — for large input corpora (parser fixtures, compiler conformance suites, golden files). The build script writes one #[test] fn case_<name> per file in tests/fixtures/, included via include!(concat!(env!("OUT_DIR"), "/generated_tests.rs")). Good when the test set grows by adding files, not by editing Rust.

Pick by intent: hand-curated stories use #[case]; combinatorial coverage uses matrix; stable per-input names use paste!; large file-driven corpora use build.rs.

Review checks:

• [FILE:LINE] HANDROLLED_PARAMETERIZED_TESTS — Test file contains 12 copy-pasted #[test] functions that vary only in input literals. Replace with #[rstest] #[case(...)] so cargo output names each row.
• [FILE:LINE] MATRIX_EXPANSION_TOO_LARGE — rstest matrix with #[values] over four axes generates 10,000+ tests. CI runtime balloons. Either reduce axes, sample with proptest, or move to a corpus.
• [FILE:LINE] PASTE_INSTEAD_OF_RSTEST — paste! macro generates tests when rstest #[case] would do, losing rstest's per-case failure reporting and IDE integration.
• [FILE:LINE] BUILD_RS_TESTS_NOT_DETERMINISTIC — build.rs walks tests/fixtures/ with read_dir (filesystem order is unspecified). CI test names differ between machines. Sort the directory listing.
• [FILE:LINE] GENERATED_TESTS_NO_RERUN_IF_CHANGED — build.rs emits tests from a fixture directory but does not call cargo:rerun-if-changed=tests/fixtures. Adding a fixture file does not regenerate the test list.

Criterion Specifics

Criterion is the de facto Rust benchmark harness. The basics appeared in "Benchmarking Rigor" above. The points below are what reviewers should actually check on.

• Statistical confidence. Criterion runs each benchmark for many iterations, computes mean and variance, and applies bootstrap resampling to produce a confidence interval. A single timing is meaningless; criterion's number is the mean of the distribution with a stated CI. Reject benchmarks reported as Instant::now(); work(); start.elapsed() one-shot timings.
• Baselines. cargo bench -- --save-baseline main saves a named baseline. On a PR branch, cargo bench -- --baseline main compares the current run against it; criterion reports change: -3.2% (-4.5%, -2.0%) with a confidence interval and classifies the result (Improved / Regressed / No change). CI should save a baseline on merge to trunk and compare against it on every PR.
• `black_box` discipline. criterion::black_box is the prevent-constant-folding marker. For pointer-backed inputs use black_box(input.as_ptr()) — the optimizer can't reason through pointer arithmetic. black_box(&input) is sometimes optimized away because the compiler can legally assume &T is not mutated.
• `iter_batched`. For benchmarks needing fresh setup per iteration (mutating a vec, draining a channel), bencher.iter_batched(setup, routine, BatchSize::SmallInput) runs setup uncounted and the routine closure counted. Without iter_batched, the setup cost pollutes the measurement or worse, the second iteration runs on already-mutated state.

  use criterion::{black_box, BatchSize, Criterion};
  fn bench_drain(c: &mut Criterion) {
      c.bench_function("drain_vec", |b| {
          b.iter_batched(
              || (0..1024).collect::<Vec<u32>>(),
              |mut v| { for x in v.drain(..) { black_box(x); } },
              BatchSize::SmallInput,
          );
      });
  }

• `--profile bench`. Release optimizations with debug symbols retained, so cargo flamegraph --bench foo can correlate samples to source lines. Set [profile.bench] debug = true (or debug = "line-tables-only" for smaller binaries).
• I/O isolation. Any file open, network call, or RNG seed belongs in the setup closure of iter_batched (or before b.iter), not inside the measured closure. Otherwise the benchmark times the OS, not your code.

Review checks:

• [FILE:LINE] BENCH_NO_BLACK_BOX_ON_RESULT — Benchmark closure's return value is dropped without black_box. The compiler can prove the value is unused and eliminate the computation. Wrap with black_box(result).
• [FILE:LINE] BENCH_IO_IN_MEASURED_CLOSURE — File::open or TcpStream::connect inside b.iter. Move to iter_batched setup so I/O cost is excluded.
• [FILE:LINE] BENCH_NO_BASELINE_IN_CI — criterion_group! exists but no CI job runs --save-baseline on trunk merges or --baseline on PRs. Regressions ship undetected. Add a CI step gating PRs on the comparison.
• [FILE:LINE] BENCH_NO_REGRESSION_THRESHOLD — CI runs criterion comparison but does not fail the job on Regressed. Add a script that greps criterion output or use criterion-compare-action.
• [FILE:LINE] ITER_BATCHED_NOT_USED_FOR_MUTATION — Benchmark calls b.iter(|| my_vec.drain(..)) where my_vec is captured by reference. The second iteration runs on an empty vec. Use iter_batched to rebuild input each iteration.

trybuild for Proc-Macro UI Tests

trybuild::TestCases runs .rs files in tests/ui/ through the compiler and compares stderr against a sibling .stderr file. Use it to confirm a proc-macro emits the expected diagnostic for invalid input (a derive macro applied to a union, a function-like macro fed wrong syntax, an attribute macro on the wrong item kind). The mechanism is described in ../../macros-code-review/references/procedural-macros.md.

#[test]
fn ui() {
    let t = trybuild::TestCases::new();
    t.compile_fail("tests/ui/fail/*.rs");
    t.pass("tests/ui/pass/*.rs");
}

The pitfall is the .stderr reference output is rustc-version-sensitive. Span format, lint names, suggestion punctuation, and even ANSI escapes can change between stable releases. A green local run on 1.84 will fail in CI pinned to 1.82. Mitigations:

• Pin a specific stable rustc for the trybuild CI job (rust-toolchain.toml with channel = "1.84"); other jobs can float.
• Regenerate intentionally on rustc upgrades: TRYBUILD=overwrite cargo test --test ui. Review the diff; commit the new .stderr.
• Skip trybuild on nightly: gate the test with #[cfg(not(nightly))] via a build-script-set cfg, or use #[ignore] and run separately.

Review checks:

• [FILE:LINE] TRYBUILD_NO_PINNED_TOOLCHAIN — tests/ui/*.stderr exists but rust-toolchain.toml is missing or floats. Stderr format drifts between stable releases; CI breaks for unrelated rustc updates.
• [FILE:LINE] TRYBUILD_ON_NIGHTLY — Trybuild job runs on nightly. Nightly stderr format changes weekly. Pin to stable or mark the job as allowed-to-fail.
• [FILE:LINE] TRYBUILD_OVERWRITE_COMMITTED — A .stderr file contains placeholders like # overwrite or was generated locally without review. Regenerate cleanly and inspect the diff.

Clippy Lint Group Strategy

Jon's recommended grouping (Ch 6 "Linting"). Library and binary crates should differ: libraries surface lints downstream consumers cannot fix, so be conservative about which groups are denied.

• clippy::correctness — deny always. These are compiler-suggested fixes for near-certain bugs (broken swaps, mem::forget on references, iterating Option::next).
• clippy::suspicious — warn for libraries; consider deny if test coverage is thin. Catches code that looks like a typo.
• clippy::style — warn. Taste; most are obvious wins.
• clippy::complexity — warn. Real refactor signals (nested closures, redundant binding).
• clippy::perf — warn. Real wins on hot paths (String::from vs .to_string(), clone() in iterators).
• clippy::pedantic — warn for libraries; expect false positives. Suppress at the specific site with #[expect(clippy::lint_name, reason = "...")] (the reason field is required on Rust 2024 edition and surfaces in clippy output).
• clippy::nursery — do not enable in CI. Experimental lints whose behavior changes between rustc releases; you will get failures from rustc upgrades alone.
• clippy::restriction — never enable group-wide. Many lints would flag valid code (e.g., clippy::shadow_unrelated flags shadowing across unrelated bindings). Opt in to specific lints only.

Recommended rustc lints for library crates (workspace-wide via [workspace.lints.rust]):

[workspace.lints.rust]
rust_2018_idioms = { level = "warn", priority = -1 }
rust_2024_compatibility = { level = "warn", priority = -1 }
missing_docs = "warn"
missing_debug_implementations = "warn"
unsafe_op_in_unsafe_fn = "deny"

[workspace.lints.clippy]
correctness = { level = "deny", priority = -1 }
suspicious = { level = "warn", priority = -1 }
pedantic = { level = "warn", priority = -1 }
nursery = { level = "allow", priority = -1 }

The priority = -1 lets specific item-level #[expect]/#[allow] attributes override the group setting.

Review checks:

• [FILE:LINE] CLIPPY_NURSERY_IN_CI — [workspace.lints.clippy] nursery = "warn" or cargo clippy -- -W clippy::nursery in CI. Nursery lints change between rustc releases; pin or remove.
• [FILE:LINE] CLIPPY_RESTRICTION_GROUP — Crate enables clippy::restriction group-wide. Many lints flag valid code. Opt in to specific lints individually.
• [FILE:LINE] PEDANTIC_BLANKET_ALLOWED — #![allow(clippy::pedantic)] at crate root masks all pedantic lints including useful ones. Allow specific lints with #[expect(clippy::lint_name, reason = "...")] at the offending site.
• [FILE:LINE] MISSING_REASON_ON_EXPECT — #[expect(clippy::cast_possible_truncation)] without a reason = "..." field. Future readers cannot tell whether the suppression is still justified. Add the reason.
• [FILE:LINE] LIB_MISSING_DOC_LINTS — Public library crate lacks #![warn(missing_docs)] and #![warn(missing_debug_implementations)]. Undocumented and non-Debug public items ship without signal.

Cross-references: concurrency-specific test gates live in concurrency-testing.md; proc-macro test scaffolding details live in ../../macros-code-review/references/procedural-macros.md.

Concurrency Testing

Testing hand-rolled concurrent Rust requires tools that cargo test cannot provide. This reference focuses on loom, cargo miri, and complementary tools used to review tests for concurrent code. For the patterns being tested, see [../../rust-code-review/references/concurrency-primitives.md], [../../rust-code-review/references/memory-ordering.md], and [../../rust-code-review/references/lock-free-patterns.md].

1. Why concurrent code needs special testing

cargo test runs each test in a single OS thread (or up to --test-threads=N worker threads, one test per worker). Within a single test, every thread::spawn produces one interleaving of memory operations. That interleaving is whatever the OS scheduler happened to produce on this run, on this CPU, under this load. The next run may produce the same interleaving a million times in a row.

Atomic ordering bugs are non-deterministic and platform-dependent. The canonical example is Relaxed used on a flag that publishes non-atomic data: x86-64's strong memory model effectively promotes Relaxed to Release, so the bug is invisible there. ARM64 and RISC-V reorder freely, and the same code corrupts ~0.4% of operations on Apple M1 (Mara Bos, Rust Atomics and Locks, Chapter 7). "Passes on CI" is not evidence when CI is x86-64.

The Rust compiler does not catch atomic ordering bugs. The book is explicit: the "fearless concurrency" claim covers data races (the borrow checker prevents two threads from holding &mut T), not memory ordering. Acquire/Release/SeqCst are contracts; the compiler enforces type signatures (load(Release) panics, load(AcqRel) panics), not happens-before relationships.

Two tools change this:

• `loom` — a model checker. Replaces std::sync and std::thread

with versions that explore every legal interleaving of memory operations, then runs each interleaving to find the one that violates your assertions.

• `cargo miri` — a MIR interpreter. Runs your test under a model

that catches undefined behavior (uninit reads, use-after-free, alignment violations, data races, certain weak-memory observations, Stacked/Tree Borrows violations).

They are complementary, not redundant. Loom explores which interleaving could fail; Miri checks whether a given interleaving has UB. Neither subsumes the other.

Review checks

• [FILE:LINE] CONCURRENT_CODE_NO_LOOM_TESTS — module uses raw atomics,

UnsafeCell, hand-rolled Mutex/channel/refcount, or unsafe impl Send/Sync, and has no #[cfg(loom)] test harness.

• [FILE:LINE] UNSAFE_HEAVY_NO_MIRI — crate has non-trivial unsafe

blocks touching atomics or pointer arithmetic with no cargo +nightly miri test step in CI.

• [FILE:LINE] X86_ONLY_TEST_COVERAGE — concurrency tests run only on

target_arch = "x86_64" (no ARM64/aarch64 runner, no loom).

2. loom

loom = "0.7" (current stable line) is a permutation-testing model checker for concurrent Rust. It replaces std::sync::{Arc, Mutex, RwLock, Condvar, atomic::*}, std::thread, and std::cell::UnsafeCell with versions that record every operation and explore alternative interleavings.

loom::model(|| { ... }) runs the closure repeatedly with different schedules until it has explored every legal interleaving (or hits the preemption bound). A single model call may execute the closure thousands to millions of times.

Setup

[dev-dependencies]
loom = "0.7"

Conditional import pattern at the top of the module under test:

#[cfg(loom)]      use loom::sync::Arc;
#[cfg(loom)]      use loom::sync::atomic::AtomicUsize;
#[cfg(loom)]      use loom::thread;
#[cfg(not(loom))] use std::sync::Arc;
#[cfg(not(loom))] use std::sync::atomic::AtomicUsize;
#[cfg(not(loom))] use std::thread;

Same trick for UnsafeCell, Mutex, RwLock, Condvar.

Running

RUSTFLAGS="--cfg loom" cargo test --release --test loom_tests
LOOM_MAX_PREEMPTIONS=3 RUSTFLAGS="--cfg loom" \
    cargo test --release --test loom_tests

Notes:

• --release is mandatory in practice — debug-mode loom is 10–50×

slower, often pushing test runs past CI timeouts.

• LOOM_MAX_PREEMPTIONS=N bounds exploration: most real bugs surface

at N=2 or N=3. Higher N explodes combinatorially.

• Place loom tests in a separate tests/loom_tests.rs (or

--test <name>) so the cfg(loom) flag does not poison the normal test build.

Canonical loom test

#[test]
fn arc_does_not_use_after_free() {
    loom::model(|| {
        let a = Arc::new(AtomicUsize::new(0));
        let b = a.clone();
        let t = thread::spawn(move || { b.store(1, Release); });
        let _ = a.load(Acquire);
        t.join().unwrap();
    });
}

Limitations

• Caps around 64 threads; exponential blow-up means real tests use 2–4

threads and small loops.

• Does not faithfully model every Relaxed reordering (it

under-approximates weak memory — covers DRF-SC, not all of C++20). Pair with Miri for orderings you suspect are too weak.

• Cannot model OS-level effects (signals, real timing, real IO).
• Tests must be deterministic. Random numbers, system clock, hash map

iteration order bias the search and produce phantom "passes."

BAD: loom test using std::sync

// BAD — uses std types, so loom sees zero interleavings.
#[test]
fn channel_send_recv() {
    loom::model(|| {
        let c = std::sync::Arc::new(MyChannel::new());
        let s = c.clone();
        std::thread::spawn(move || s.send(42));
        assert_eq!(c.recv(), 42);
    });
}

GOOD: loom test using loom shims

#[test]
fn channel_send_recv() {
    loom::model(|| {
        let c = Arc::new(MyChannel::new());  // loom::sync::Arc under cfg(loom)
        let s = c.clone();
        thread::spawn(move || s.send(42));   // loom::thread under cfg(loom)
        assert_eq!(c.recv(), 42);
    });
}

BAD: nondeterministic body

// BAD — std::time::Instant is not under loom's control; results vary
// run-to-run, defeating exhaustive exploration.
loom::model(|| {
    let start = std::time::Instant::now();
    let v = Arc::new(AtomicUsize::new(0));
    thread::spawn({ let v = v.clone(); move || v.store(start.elapsed().as_nanos() as usize, Relaxed) });
});

BAD: debug-mode loom

# BAD — no --release, 10-50x slower; CI gives up before exploration completes.
RUSTFLAGS="--cfg loom" cargo test --test loom_tests

Review checks

• [FILE:LINE] LOOM_TEST_USES_STD_SYNC — loom::model body imports

std::sync::Arc, std::sync::Mutex, std::sync::atomic::*, or std::thread::spawn instead of the loom::* shims.

• [FILE:LINE] LOOM_TEST_USES_STD_UNSAFECELL — loom::model over code

that uses std::cell::UnsafeCell; loom cannot track those accesses.

• [FILE:LINE] LOOM_TEST_NONDETERMINISTIC — body uses Instant::now,

SystemTime::now, rand::*, HashMap iteration, or environment reads; loom needs deterministic inputs to enumerate schedules.

• [FILE:LINE] LOOM_TEST_MISSING_RELEASE — Cargo invocation lacks

--release; debug-mode loom is too slow to be useful in CI.

• [FILE:LINE] LOOM_TEST_NO_PREEMPTION_BOUND — no LOOM_MAX_PREEMPTIONS

set on a long-running test (risks CI timeout) or set absurdly high (>=5) without justification.

• [FILE:LINE] LOOM_TEST_LIVES_IN_UNIT_TESTS — loom::model invoked

inside #[cfg(test)] mod tests next to non-loom unit tests, so the --cfg loom build pollutes regular cargo test.

• [FILE:LINE] LOOM_RELAXED_REORDERING_ASSUMED — test relies on loom

to surface a Relaxed-only reorder; loom under-approximates this axis — also run under Miri with -Zmiri-many-seeds.

3. cargo miri

Miri is a Rust interpreter that executes your test against a model of the language semantics rather than against real CPU instructions. It catches undefined behavior that real hardware silently tolerates: out-of-bounds pointer access, use-after-free, uninitialized reads, misaligned accesses, invalid enum/bool/char bit patterns, data races without synchronization, broken intrinsic preconditions, and aliasing violations under Stacked Borrows or Tree Borrows.

Install and run

rustup +nightly component add miri
cargo +nightly miri test
cargo +nightly miri test -- --test-threads=1   # if isolation matters

Miri requires nightly. Pin the nightly version (rust-toolchain.toml) so CI does not break when miri lags rustc.

Useful MIRIFLAGS

# Strict pointer provenance — catches pointer/usize round-trip bugs.
MIRIFLAGS="-Zmiri-strict-provenance" cargo +nightly miri test

# Tree Borrows — newer aliasing model; catches more aliasing bugs than
# Stacked Borrows.
MIRIFLAGS="-Zmiri-tree-borrows" cargo +nightly miri test

# Re-run with many non-determinism seeds (allocator addresses, atomic
# scheduling). Surfaces flaky weak-memory bugs.
MIRIFLAGS="-Zmiri-many-seeds=0..16" cargo +nightly miri test

# Allow filesystem / time / env access. Use sparingly; defeats isolation.
MIRIFLAGS="-Zmiri-disable-isolation" cargo +nightly miri test

Limitations

• Slow: 10–100× test runtime. Budget for it; do not gate every PR on a

full workspace Miri run if the suite is large.

• Does not model real OS scheduling — Miri's concurrency is its own

interpreter scheduler. Pair with loom for interleaving exploration.

• Cannot run inline asm, syscalls, FFI to non-Rust code. Mark such

tests #[cfg_attr(miri, ignore)].

• Under-approximates weak memory in single runs — that is what

-Zmiri-many-seeds=0..16 is for.

Loom + Miri pairing

Standard recipe for any module with unsafe + atomics: loom tests on a nightly/labeled job, Miri test pass on every PR, property tests for shape variation.

BAD: skipping Miri on release-only CI

# BAD — only runs on release branches; unsafe regressions land on main.
on: push:
  branches: [release/*]
jobs:
  miri:
    steps:
      - run: cargo +nightly miri test

BAD: Miri without feature coverage

# BAD — default features only; the `unsafe-fast-path` feature, which is
# the whole reason for the unsafe code, is never exercised.
cargo +nightly miri test
# GOOD
cargo +nightly miri test --all-features

BAD: blanket-ignoring under Miri

// BAD — entire module is excluded, including pure-safe tests Miri could
// run cheaply. Ignore at the test-function granularity instead.
#![cfg_attr(miri, ignore)]

Review checks

• [FILE:LINE] MIRI_NOT_IN_CI — crate has any unsafe { ... } block

touching atomics, pointers, or UnsafeCell and no cargo +nightly miri test step in CI.

• [FILE:LINE] MIRI_CI_RELEASE_ONLY — Miri runs only on release tags

or nightly cron; unsafe regressions slip into main between runs.

• [FILE:LINE] MIRI_EXCLUDES_FEATURE_FLAGS — cargo miri test invoked

without --all-features (or without the specific feature gating the unsafe path).

• [FILE:LINE] MIRI_IGNORE_TOO_BROAD — #![cfg_attr(miri, ignore)] at

module or crate level rather than per-test on the FFI/syscall cases that actually need it.

• [FILE:LINE] MIRI_NO_STRICT_PROVENANCE — code performs usize ↔ ptr

casts and CI does not set MIRIFLAGS=-Zmiri-strict-provenance.

• [FILE:LINE] MIRI_NO_TREE_BORROWS — code uses heavy raw-pointer

aliasing (intrusive lists, lock-free) and CI does not run a -Zmiri-tree-borrows job; Stacked Borrows alone misses some bugs.

4. Other tools

shuttle (AWS)

shuttle is a randomized concurrency tester. Same API surface as loom (replaces std::sync/std::thread), but instead of exhaustively exploring all interleavings, it samples N random schedules. Less rigorous than loom but scales to bigger code under test, so it works on codebases where loom's combinatorics blow up. Treat it as complementary to loom, not a substitute for hand-rolled lock-free correctness work.

cargo nextest run -j1

A common confusion: nextest run -j1 runs tests on a single thread (one test process at a time). It does not force a test's internal thread::spawn calls to run serially. It does nothing for concurrency bugs inside the code under test. Flag any reviewer comment claiming otherwise.

kani and prusti

Formal verification tools for restricted subsets of Rust. Out of scope for most production code, but worth knowing for kernel modules, OS primitives, and cryptographic implementations that justify proof effort.

ThreadSanitizer (-Z sanitizer=thread)

Nightly-only sanitizer that instruments compiled code to detect data races at runtime. Catches races that miri misses when the race spans FFI into a non-Rust dependency (Miri cannot enter FFI). Useful for crates with unsafe extern "C" boundaries.

RUSTFLAGS="-Z sanitizer=thread" cargo +nightly test \
    --target x86_64-unknown-linux-gnu

Review checks

• [FILE:LINE] NEXTEST_J1_AS_CONCURRENCY_FIX — comment or commit

message claims nextest run -j1 "tests for race conditions"; it does not.

• [FILE:LINE] FFI_HEAVY_NO_TSAN — crate has unsafe extern "C"

blocks calling C/C++ libraries and CI has no ThreadSanitizer job; Miri cannot see into the FFI side.

5. CI integration

A workable matrix for an unsafe-heavy concurrency crate:

The general principle: a "tests pass" claim must specify which of these jobs passed. "All green on x86_64-unknown-linux-gnu without loom or miri" is not the same evidence as a full matrix.

BAD: unspecified test pass claim

PR description: "Tests pass." (No CI link, no mention of which jobs.)

The reviewer cannot tell whether the unsafe path was exercised. Demand specifics: cargo test passed, miri passed with --all-features, loom passed for the affected modules.

BAD: loom job runs on debug

# BAD — no --release, slow, may timeout, exploration incomplete.
- run: RUSTFLAGS="--cfg loom" cargo test --test loom_tests

Review checks

• [FILE:LINE] CI_PASS_CLAIM_UNQUALIFIED — PR claims "tests pass"

without naming the toolchains/jobs; loom and miri may have been skipped.

• [FILE:LINE] CI_LOOM_NO_RELEASE — CI loom job omits --release,

making the job too slow to complete real exploration.

6. Patterns that always need a concurrency test

Review checks

• [FILE:LINE] UNSAFE_SEND_SYNC_NO_LOOM_TEST — `unsafe impl (Send|Sync)

for X with no corresponding loom::model` test demonstrating the invariant.

• [FILE:LINE] LOCKFREE_STRUCT_MISSING_BOTH_TOOLS — module defines a

lock-free queue/stack/list and exercises it under loom but not Miri, or under Miri but not loom; lock-free code needs both.

• [FILE:LINE] CUSTOM_DROP_NO_RACE_TEST — type with shared

AtomicUsize/AtomicPtr state and a non-trivial Drop has no test for drop-while-another-thread-still-accessing.

• [FILE:LINE] REFCOUNT_LAST_DECREMENT_UNTESTED — `fetch_sub(1,

Release) + fence(Acquire)` pattern with no loom test that covers both the last-decrement and non-last-decrement branches.

Integration Tests

Test Directory Structure

project/
├── src/
│   └── lib.rs
└── tests/
    ├── common/
    │   └── mod.rs       # shared test utilities
    ├── api_test.rs       # integration test suite
    └── workflow_test.rs  # integration test suite

Each file in tests/ is compiled as a separate crate with access only to the public API.

Shared Test Utilities

// tests/common/mod.rs
use my_crate::Config;

pub fn test_config() -> Config {
    Config {
        database_url: std::env::var("TEST_DATABASE_URL")
            .unwrap_or_else(|_| "postgres://localhost:5433/test".into()),
        ..Config::default()
    }
}

pub async fn setup_test_db(pool: &PgPool) {
    sqlx::query("TRUNCATE users, orders CASCADE")
        .execute(pool)
        .await
        .expect("failed to clean test database");
}

// tests/api_test.rs
mod common;

#[tokio::test]
async fn test_create_user_returns_201() {
    let config = common::test_config();
    // ...
}

Async Integration Tests

#[tokio::test]
async fn test_event_bus_delivers_to_all_subscribers() {
    let (tx, _) = broadcast::channel(100);

    let mut rx1 = tx.subscribe();
    let mut rx2 = tx.subscribe();

    tx.send(Event::new("test")).unwrap();

    let event1 = rx1.recv().await.unwrap();
    let event2 = rx2.recv().await.unwrap();

    assert_eq!(event1.name, "test");
    assert_eq!(event2.name, "test");
}

Multi-Threaded Tests

When testing concurrent behavior, use #[tokio::test(flavor = "multi_thread")]:

#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_concurrent_state_updates() {
    let state = Arc::new(Mutex::new(Vec::new()));
    let mut handles = Vec::new();

    for i in 0..10 {
        let state = Arc::clone(&state);
        handles.push(tokio::spawn(async move {
            let mut guard = state.lock().await;
            guard.push(i);
        }));
    }

    for handle in handles {
        handle.await.unwrap();
    }

    let guard = state.lock().await;
    assert_eq!(guard.len(), 10);
}

Database Integration Tests

Test Isolation

Each test should start with a clean state. Options:

1. Truncate tables — Fast, works for most cases 2. Transaction rollback — Test runs inside a transaction that's rolled back 3. Separate database per test — Most isolated, slowest

// Transaction rollback pattern
#[tokio::test]
async fn test_insert_user() {
    let pool = PgPool::connect(&test_database_url()).await.unwrap();
    let mut tx = pool.begin().await.unwrap();

    let user = sqlx::query_as!(User, "INSERT INTO users (name) VALUES ($1) RETURNING *", "Test")
        .fetch_one(&mut *tx)
        .await
        .unwrap();

    assert_eq!(user.name, "Test");
    // tx dropped here — rolls back automatically
}

sqlx::test Macro

The #[sqlx::test] macro simplifies database test setup by automatically creating a fresh test database, running migrations, and cleaning up after each test. The connection pool is injected as a function argument.

#[sqlx::test]
async fn test_create_user(pool: PgPool) {
    // pool is a fresh database with migrations applied
    let result = sqlx::query!("INSERT INTO users (name) VALUES ($1) RETURNING id", "test")
        .fetch_one(&pool)
        .await
        .unwrap();
    assert!(result.id > 0);
}

Use migrations to specify a custom migrations path, and fixtures to load SQL fixture files from tests/fixtures/:

#[sqlx::test(migrations = "db/migrations")]
async fn test_with_custom_migrations(pool: PgPool) {
    // uses migrations from db/migrations/ instead of the default
}

#[sqlx::test(fixtures("users", "orders"))]
async fn test_with_fixtures(pool: PgPool) {
    // tests/fixtures/users.sql and tests/fixtures/orders.sql are loaded
    let count = sqlx::query_scalar!("SELECT COUNT(*) FROM users")
        .fetch_one(&pool)
        .await
        .unwrap();
    assert!(count.unwrap() > 0);
}

Prefer #[sqlx::test] over manual pool setup with #[tokio::test] for database tests — it eliminates boilerplate and guarantees test isolation without manual truncation or transaction rollback.

Mocking with Traits

Define traits as seams for testing. Implement mock versions for tests.

Since Rust 1.75, async fn works directly in trait definitions without the async-trait crate. Prefer native syntax for new code.

// BAD (edition 2024) - unnecessary async-trait dependency
#[async_trait]
pub trait UserRepository: Send + Sync {
    async fn find(&self, id: Uuid) -> Result<Option<User>>;
    async fn create(&self, input: CreateUser) -> Result<User>;
}

// GOOD (edition 2024) - native async fn in traits
pub trait UserRepository: Send + Sync {
    fn find(&self, id: Uuid) -> impl Future<Output = Result<Option<User>>> + Send;
    fn create(&self, input: CreateUser) -> impl Future<Output = Result<User>> + Send;
}

// Also valid - async fn directly (simpler, but caller can't name the future type)
pub trait UserRepository: Send + Sync {
    async fn find(&self, id: Uuid) -> Result<Option<User>>;
    async fn create(&self, input: CreateUser) -> Result<User>;
}

Production and mock implementations:

// Production implementation
pub struct PgUserRepository { pool: PgPool }

impl UserRepository for PgUserRepository {
    async fn find(&self, id: Uuid) -> Result<Option<User>> {
        sqlx::query_as!(User, "SELECT ... WHERE id = $1", id)
            .fetch_optional(&self.pool)
            .await
            .map_err(Into::into)
    }
    // ...
}

// Test implementation
struct MockUserRepository {
    users: Vec<User>,
}

impl UserRepository for MockUserRepository {
    async fn find(&self, id: Uuid) -> Result<Option<User>> {
        Ok(self.users.iter().find(|u| u.id == id).cloned())
    }
    // ...
}

When `async-trait` is still needed: Native async fn in traits does not support dyn Trait dispatch. If your code requires Box<dyn UserRepository>, keep using async-trait for that trait. See the tokio-async-code-review skill for async trait patterns in detail.

if let Temporary Scope in Test Assertions (Edition 2024)

In edition 2024, temporaries in if let conditions are dropped at the end of the condition, not at the end of the block. This affects test patterns that inline method calls in if let conditions.

// BAD (edition 2024) - temporary lock guard drops after condition evaluates
// val may be a dangling reference inside the block
#[tokio::test]
async fn test_cache_hit() {
    let cache = setup_cache().await;
    if let Some(val) = cache.lock().await.get("key") {
        assert_eq!(val, "expected"); // guard already dropped!
    }
}

// GOOD (edition 2024) - bind the guard to extend its lifetime
#[tokio::test]
async fn test_cache_hit() {
    let cache = setup_cache().await;
    let guard = cache.lock().await;
    if let Some(val) = guard.get("key") {
        assert_eq!(val, "expected"); // guard lives through the block
    }
}

This also affects non-async patterns with RefCell, Mutex, or any method returning a temporary with borrowed data:

// BAD (edition 2024) - RefCell borrow drops after condition
if let Some(item) = state.borrow().items.first() {
    assert_eq!(item.name, "test"); // borrow already dropped
}

// GOOD - bind the borrow
let borrowed = state.borrow();
if let Some(item) = borrowed.items.first() {
    assert_eq!(item.name, "test");
}

See the tokio-async-code-review skill for more if let temporary scope patterns with async lock guards.

Test Configuration

Use environment variables or test-specific config files:

fn test_database_url() -> String {
    std::env::var("TEST_DATABASE_URL")
        .unwrap_or_else(|_| "postgres://postgres:postgres@localhost:5433/test".into())
}

For structured logging in tests:

// Initialize tracing subscriber for test output
use tracing_subscriber::fmt;

#[tokio::test]
async fn test_with_logging() {
    let _ = fmt::try_init(); // ignore error if already initialized
    tracing::info!("test starting");
    // ...
}

Review Questions

1. Are integration tests in the tests/ directory? 2. Is shared test setup extracted to a common module? 3. Are database tests isolated (no cross-test contamination)? 4. Are traits used as seams for dependency injection in tests? 5. Is #[tokio::test] used for async tests? 6. Are multi-threaded tests using flavor = "multi_thread"? 7. Are database tests using #[sqlx::test] instead of manual pool setup? 8. Are mock traits using native async fn instead of async-trait where possible? 9. Do if let assertions with inline temporaries (lock guards, borrows) account for edition 2024 temporary scoping? 10. Is #[expect] used instead of #[allow] for test-specific lint suppressions?

Unit Tests

Standard Structure

// In src/types.rs
pub enum Status {
    Active,
    Inactive,
}

impl Status {
    pub fn is_active(&self) -> bool {
        matches!(self, Self::Active)
    }
}

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn test_status_active_returns_true() {
        assert!(Status::Active.is_active());
    }

    #[test]
    fn test_status_inactive_returns_false() {
        assert!(!Status::Inactive.is_active());
    }
}

Assertion Patterns

Value Comparisons

// BAD - error message is just "assertion failed"
assert!(result == 42);

// GOOD - shows left and right values on failure
assert_eq!(result, 42);
assert_ne!(result, 0);

// With context
assert_eq!(result, 42, "expected 42 for input {input}");

Enum Variant Checking

// BAD - verbose pattern matching
match result {
    Err(Error::NotFound(_)) => (),
    other => panic!("expected NotFound, got {other:?}"),
}

// GOOD - matches! macro
assert!(matches!(result, Err(Error::NotFound(_))));

// With message
assert!(
    matches!(result, Err(Error::NotFound(id)) if id == expected_id),
    "expected NotFound for {expected_id}, got {result:?}"
);

Result Testing

// Return Result from test for cleaner error propagation
#[test]
fn test_parse_valid_input() -> Result<(), Error> {
    let config = parse("valid input")?;
    assert_eq!(config.name, "expected");
    Ok(())
}

// Test error cases
#[test]
fn test_parse_empty_input_returns_error() {
    let result = parse("");
    assert!(matches!(result, Err(Error::Empty)));
}

Should Panic

Use sparingly. Prefer Result-returning tests.

// ACCEPTABLE - when testing an intentional panic
#[test]
#[should_panic(expected = "index out of bounds")]
fn test_invalid_index_panics() {
    let list = FixedList::new(5);
    list.get(10); // should panic
}

Test Helpers

Extract common setup into helper functions. Mark them with #[expect(dead_code)] (edition 2024) or #[allow(dead_code)] if not all tests use them.

#[cfg(test)]
mod tests {
    use super::*;

    fn sample_user() -> User {
        User {
            id: Uuid::nil(),
            name: "Test User".into(),
            email: "test@example.com".into(),
        }
    }

    fn sample_config() -> Config {
        Config {
            port: 8080,
            host: "localhost".into(),
            ..Config::default()
        }
    }
}

Send + Sync Verification

Verify that types satisfy thread-safety bounds at compile time:

#[test]
fn assert_error_is_send_sync() {
    fn assert_send_sync<T: Send + Sync>() {}
    assert_send_sync::<Error>();
    assert_send_sync::<WorkflowError>();
}

Serialization Round-Trip Tests

#[test]
fn test_status_serialization_round_trip() {
    let original = Status::InProgress;
    let json = serde_json::to_string(&original).unwrap();
    let deserialized: Status = serde_json::from_str(&json).unwrap();
    assert_eq!(original, deserialized);
}

#[test]
fn test_status_serializes_to_expected_string() {
    let status = Status::InProgress;
    let s = serde_json::to_string(&status).unwrap();
    assert_eq!(s, r#""in_progress""#);
}

Test Naming Convention

Nested modules make test output readable and allow running groups:

#[cfg(test)]
mod tests {
    use super::*;

    mod parse_config {
        use super::*;

        #[test]
        fn returns_config_when_valid_toml() {
            let config = parse_config(VALID_TOML).unwrap();
            assert_eq!(config.port, 8080);
        }

        #[test]
        fn returns_error_when_empty_input() {
            let err = parse_config("").unwrap_err();
            assert!(matches!(err, ParseError::Empty));
        }

        #[test]
        fn returns_error_when_missing_required_field() {
            let err = parse_config("[server]").unwrap_err();
            assert!(matches!(err, ParseError::MissingField(_)));
        }
    }
}

Output: tests::parse_config::returns_config_when_valid_toml, etc.

One Assertion Per Test

Each test should verify one behavior. This makes failures easier to diagnose:

// BAD - which assertion failed?
#[test]
fn test_valid_inputs() {
    assert!(parse("a").is_ok());
    assert!(parse("ab").is_ok());
    assert!(parse("abc").is_ok());
}

// GOOD - descriptive separate tests, or use rstest
#[rstest]
#[case::single_char("a")]
#[case::two_chars("ab")]
#[case::three_chars("abc")]
fn parse_accepts_valid_strings(#[case] input: &str) {
    assert!(parse(input).is_ok(), "parse failed for: {input}");
}

Snapshot Testing with cargo insta

Snapshot testing compares output against a saved "golden" version. On future runs, the test fails if output changes unless explicitly approved.

Setup

# Cargo.toml
[dev-dependencies]
insta = { version = "1", features = ["yaml"] }

Install the CLI for better review workflow: cargo install cargo-insta

Assert Macros

use insta::{assert_snapshot, assert_yaml_snapshot, assert_json_snapshot};

// Plain text snapshots
#[test]
fn test_error_display() {
    let err = MyError::NotFound("user-123".into());
    assert_snapshot!("error_not_found", err.to_string());
}

// YAML snapshots (best for version control diffs)
#[test]
fn test_config_serialization() {
    let config = Config::default();
    assert_yaml_snapshot!("default_config", config);
}

// JSON snapshots with redactions for unstable fields
#[test]
fn test_user_response() {
    let user = create_test_user();
    assert_json_snapshot!(user, {
        ".created_at" => "[timestamp]",
        ".id" => "[uuid]"
    });
}

Review Workflow

1. Write test with assert_snapshot! / assert_yaml_snapshot! / assert_json_snapshot! 2. Run cargo insta test — creates pending snapshots 3. Run cargo insta review — interactively accept or reject changes 4. Commit the .snap files in snapshots/ alongside your tests

When to Use Snapshots

• Serialized output (JSON, YAML, TOML)
• Error message formatting (Display impls)
• CLI output, rendered HTML, generated code
• Complex nested structures where assert_eq! is unwieldy

When NOT to Use Snapshots

• Simple values — use assert_eq!(x, 42) instead
• Critical path logic — precise unit tests catch regressions faster
• Flaky/random output — use redactions or avoid snapshots entirely
• Huge objects — keep snapshots small and focused for easier review

Parametrized Testing with rstest

rstest eliminates duplicated test functions when testing the same behavior with different inputs.

Setup

# Cargo.toml
[dev-dependencies]
rstest = "0.23"

Basic Parametrized Tests

use rstest::rstest;

#[rstest]
#[case::empty("", true)]
#[case::whitespace("  ", true)]
#[case::content("hello", false)]
fn is_blank_returns_expected(#[case] input: &str, #[case] expected: bool) {
    assert_eq!(is_blank(input), expected);
}

Each #[case] generates a separate test with a descriptive name: is_blank_returns_expected::empty, etc.

Fixtures

Share setup logic across tests with #[fixture]:

use rstest::{fixture, rstest};

#[fixture]
fn test_db() -> TestDb {
    TestDb::new("sqlite::memory:")
}

#[rstest]
fn insert_user_succeeds(test_db: TestDb) {
    let user = User::new("Alice");
    assert!(test_db.insert(&user).is_ok());
}

#[rstest]
fn query_missing_user_returns_none(test_db: TestDb) {
    assert!(test_db.find_user("nonexistent").is_none());
}

Async Parametrized Tests

Combine rstest with tokio::test:

#[rstest]
#[case::valid_url("https://example.com", true)]
#[case::invalid_url("not-a-url", false)]
#[tokio::test]
async fn fetch_url_validates(#[case] url: &str, #[case] should_succeed: bool) {
    let result = fetch(url).await;
    assert_eq!(result.is_ok(), should_succeed);
}

Considerations

• Descriptive case names are important — #[case::empty_input("")] beats #[case("")]
• It is harder for IDEs to run/locate specific parametrized tests
• For complex per-case setup, separate test functions may be clearer

Doc Tests

Public API examples that double as tests:

/// Adds two numbers together.
///
/// # Examples
///
/// ```rust
/// # use my_crate::add;
/// assert_eq!(add(2, 3), 5);
/// ```
pub fn add(a: i32, b: i32) -> i32 {
    a + b
}

Doc test attributes: ignore, should_panic, no_run, compile_fail.

Note: cargo test --doc runs doc tests. cargo nextest does NOT — run separately.

Testing Error Messages

When errors don't implement PartialEq, test via Display:

#[test]
fn divide_by_zero_error_message() {
    let err = divide(10.0, 0.0).unwrap_err();
    assert_eq!(err.to_string(), "division by zero");
}

#[expect] for Test Lint Suppression (Stable Since 1.81)

#[expect(lint)] is a self-cleaning alternative to #[allow(lint)]. The compiler warns when the suppressed lint no longer triggers, preventing stale suppressions from accumulating in test code.

// BAD - stale suppression goes undetected forever
#[allow(unused_variables)]
#[test]
fn test_complex_setup() {
    let db = setup_db();
    let _cache = setup_cache(); // if _cache is later removed, #[allow] stays silently
    assert!(db.is_connected());
}

// GOOD - compiler warns when suppression is no longer needed
#[expect(unused_variables, reason = "cache setup needed for side effects")]
#[test]
fn test_complex_setup() {
    let db = setup_db();
    let _cache = setup_cache();
    assert!(db.is_connected());
}

Common test-specific suppressions to migrate:

LazyLock for Test Fixtures (Stable Since 1.80)

std::sync::LazyLock replaces lazy_static! and once_cell::sync::Lazy for shared test fixtures that are expensive to construct. Thread-safe by default.

// BAD - external dependency for test fixture
use lazy_static::lazy_static;
lazy_static! {
    static ref TEST_CONFIG: Config = Config::load("test.toml").unwrap();
}

// BAD - also external dependency
use once_cell::sync::Lazy;
static TEST_CONFIG: Lazy<Config> = Lazy::new(|| Config::load("test.toml").unwrap());

// GOOD (edition 2024) - std library, no external crate
use std::sync::LazyLock;
static TEST_CONFIG: LazyLock<Config> = LazyLock::new(|| Config::load("test.toml").unwrap());

For test fixtures that don't need to cross thread boundaries, use std::cell::LazyCell instead.

Note: tokio::sync::OnceCell is still preferred when fixture initialization requires .await.

Tail Expression Temporary Scope (Edition 2024)

In edition 2024, temporaries in tail expressions are dropped before local variables. This can affect test functions that return Result and create temporaries in the return expression.

// Edition 2021 - temporaries in tail expression outlive locals
#[test]
fn test_parse_config() -> Result<(), Error> {
    let input = "key=value";
    // temporary String from to_string() lives until end of function
    Ok(parse(input.to_string().as_str())?)
}

// Edition 2024 - temporary String drops BEFORE the function returns
// This may cause "temporary value dropped while borrowed" errors
// Fix: bind the temporary to a local variable
#[test]
fn test_parse_config() -> Result<(), Error> {
    let input = "key=value";
    let owned = input.to_string();
    Ok(parse(owned.as_str())?)
}

This primarily affects tests that chain method calls in the return position. If the compiler reports "temporary value dropped while borrowed" after an edition migration, bind the temporary to a let binding.

Review Questions

1. Are unit tests in #[cfg(test)] modules within source files? 2. Do assertions use assert_eq! for value comparisons? 3. Are error variants checked specifically (not just "is error")? 4. Are test helpers extracted for repeated setup? 5. Do types that cross thread boundaries have Send/Sync tests? 6. Do serialized types have round-trip tests? 7. Are tests named descriptively (not test_happy_path)? 8. Do tests verify one behavior each? 9. Is snapshot testing used for complex structural output? 10. Do public API functions have doc test examples? 11. Is #[expect] used instead of #[allow] for test-specific lint suppressions? 12. Are lazy_static! / once_cell test fixtures replaced with std::sync::LazyLock when MSRV allows? 13. Do tail expression temporaries in Result-returning tests avoid dangling borrows under edition 2024?