/rust-testing-code-review

name: rust-testing-code-review
description: Reviews Rust test code for unit test patterns, integration test structure, async testing, mocking approaches, and property-based testing. Covers Rust 2024 edition changes including async fn in traits for mocks, #[expect] lint suppression, LazyLock test fixtures, and temporary scope changes affecting test assertions. Use when reviewing _test.rs files, #[cfg(test)] modules, or test infrastructure in Rust projects. Covers tokio::test, test fixtures, and assertion patterns.

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](../review-verification-protocol/SKILL.md) 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

| Issue Type | Reference | |------------|-----------| | Unit tests, assertions, naming, snapshots, rstest, doc tests, `#[expect]`, `LazyLock` fixtures, tail expression scope | [references/unit-tests.md](references/unit-tests.md) | | Integration tests, async testing, fixtures, test databases, native `async fn` mocks, `if let` temporary scope | [references/integration-tests.md](references/integration-tests.md) | | Fuzzing, proptest, Miri, Loom basics, mocking strategies, **stub/fake/mock/spy taxonomy, rstest matrix, `paste!`, build.rs test gen, criterion baselines + `black_box` discipline, trybuild UI tests, clippy lint groups** | [references/advanced-testing.md](references/advanced-testing.md) | | Loom interleaving tests, Miri UB checks, shuttle, ThreadSanitizer, CI matrix for concurrent code | [references/concurrency-testing.md](references/concurrency-testing.md) |

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-clean