Codify catch_unwind(AssertUnwindSafe(...)) testing pattern as testing::assert_panics
# Codify the "assert a closure panics" pattern as `testing::assert_panics`
## Summary
Several test sites across the workspace use the same multi-line incantation to
assert that a particular closure call panics — without using `#[should_panic]`,
because the test needs to continue running afterwards (e.g. to verify that the
data structure is still in a valid state, that subsequent calls still work, or
to make additional assertions). The recurring shape is:
```rust
let panic_result = catch_unwind(AssertUnwindSafe(|| {
// ... call expected to panic ...
}));
assert!(panic_result.is_err());
```
This pattern is verbose, repeats `AssertUnwindSafe` boilerplate at every site,
and forces every consumer to remember to import `std::panic::catch_unwind`,
`std::panic::AssertUnwindSafe`, and to combine them correctly. A small helper
in the `testing` package would centralize the idiom and shave 5 lines down to
1 at every call site.
## Proposed API
```rust
// In packages/testing/src/lib.rs
/// Asserts that the closure panics.
///
/// Captures the panic payload (and silences the default panic hook for the
/// duration) so the surrounding test can keep running and make further
/// assertions about state after the panic.
///
/// Prefer `#[should_panic]` when the panic is the only thing the test
/// verifies — this helper is for the case where the test must also assert on
/// post-panic state.
#[track_caller]
pub fn assert_panics<F, R>(f: F)
where
F: FnOnce() -> R,
{
let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(f));
assert!(result.is_err(), "closure did not panic as expected");
}
```
A few open design questions worth resolving before implementation:
1. **Silence the panic hook?** By default the panic message and backtrace
still print to stderr even when caught. Should `assert_panics` install a
no-op hook for the duration of the call to keep test output clean? This
makes the helper non-`Send` / non-thread-safe (the panic hook is global),
so it would need a documented caveat or a separate variant.
2. **Return the payload?** Some sites might want to inspect the payload (e.g.
`await_rethrows_panic` in `vicinal::join_handle` captures it to forward
along a channel). A `pub fn catch_panic<F, R>(f: F) -> Box<dyn Any + Send>`
variant could cover that. Strawman: ship `assert_panics` first, and add
`catch_panic` later only if real demand materializes.
3. **`#[track_caller]`** so a failing assertion points at the test, not at
the helper.
## Existing usage to migrate
Direct "expect this to panic, then keep going" sites (≈7) that would migrate
to `assert_panics`:
- `packages/nm_otel/src/state.rs:432, 462` — `EventState::histogram_deltas`
bucket-count-mismatch and fewer-buckets panic tests
- `packages/region_local/src/region_local.rs:691, 715` —
`callback_panic_during_*` tests
- `packages/region_cached/src/region_cached.rs:789, 813` —
`callback_panic_during_*` tests
- `packages/infinity_pool/src/opaque/slab.rs:859` — drop-panic isolation test
Adjacent sites that use `catch_unwind(AssertUnwindSafe(...))` for *other*
legitimate reasons (capture a payload to forward, do bookkeeping then
`resume_unwind`, etc.) — these are **not** candidates for `assert_panics`
and should remain on raw `catch_unwind`:
- `packages/vicinal/src/join_handle.rs:111` — captures a payload to feed
into a channel
- `packages/region_local/src/region_local.rs:768`,
`packages/region_cached/src/region_cached.rs:872` — propagate the
`Result<R, _>` so the orchestration thread can decide what to do with it
- All `packages/events_once/**`, `packages/vicinal/src/{task,pool}.rs`,
`packages/infinity_pool/src/**/pool_managed.rs`,
`packages/infinity_pool/src/opaque/slab.rs:{459,1757}` — production code
catching panics across abstraction boundaries
## Non-goals
- Do **not** check the panic message. AGENTS.md ("Do not check for specific
panic or error messages") is the existing rule and this helper should not
encourage relaxing it. Canary substrings and pass-through verification
are the documented exceptions; `assert_panics` is for neither.
- Do **not** replace `#[should_panic]` at sites where it is the right tool.
The `#[should_panic]` attribute is more idiomatic and has zero runtime
cost; `assert_panics` is specifically for tests that need to continue
asserting after the panic.
## Acceptance criteria
- [ ] `testing::assert_panics` exists with documentation and an inline example.
- [ ] All ≈7 candidate sites listed above are migrated.
- [ ] AGENTS.md gets a one-line nudge under "Testing for panics and errors"
pointing at the helper for the "keep going after the panic" use case.
- [ ] Mutation testing on the migrated tests still catches the mutations
that the existing assertions catch (the helper should not weaken
coverage).
0 条评论