From c0f8ce42c9e21ee8049520122aed9bad9918d27a Mon Sep 17 00:00:00 2001 From: leynos Date: Sat, 1 Aug 2026 21:48:40 +0200 Subject: [PATCH 1/8] Make the locale test stub strict about unexpected variable reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `StubEnv` answered `None` for every key but `NETSUKE_LOCALE`. Had the code under test been changed to read a differently-named variable — through a rename, a typo, or a new precedence rung — the stub would have quietly answered `None` and the test would still have passed, asserting nothing about the new read. That is the failure mode a test double exists to prevent. The stub now declares which variables it answers and panics on anything else, naming the unexpected key. Unset-but-expected is a distinct, declarable state, because an absent variable is a legitimate case to exercise and must be distinguishable from one the test never anticipated. `Default` is removed rather than retained. On a strict stub it would mean "deny every read", so `StubEnv::default()` would compile and then panic at run time for the common "no locale set" case; requiring `without_locale()` moves that to a compile error. One call site in `tests/locale_resolution_tests.rs` relied on it and is updated. This replaces the originally proposed convergence onto `mockable::Env`. That would have required moving `mockable` — and `mockall` with it — from dev-dependencies into the production dependency tree, which is a poor exchange for deleting a two-method trait. `locale_resolution`'s bespoke `EnvProvider` is a narrow seam of exactly the shape AGENTS.md permits and was never in violation. Closes #489. Refs #496. Co-Authored-By: Claude Opus 5 (1M context) --- test_support/src/locale_stubs.rs | 75 ++++++++++++++++++++++++---- tests/bdd/steps/cli.rs | 11 ++-- tests/bdd/steps/locale_resolution.rs | 11 ++-- tests/locale_resolution_tests.rs | 2 +- 4 files changed, 82 insertions(+), 17 deletions(-) diff --git a/test_support/src/locale_stubs.rs b/test_support/src/locale_stubs.rs index 5b2a169f7..f3a7730c8 100644 --- a/test_support/src/locale_stubs.rs +++ b/test_support/src/locale_stubs.rs @@ -4,29 +4,84 @@ //! deterministic environment and system locales. use netsuke::locale_resolution::{self, LocaleEnvProvider, SystemLocale}; +use std::collections::HashMap; /// Stub environment provider for locale resolution. -#[derive(Debug, Default, Clone)] +/// +/// Answers only the variables it was given, and **panics** on any other key. +/// +/// The permissive alternative — returning `None` for anything unrecognised — +/// hides exactly the change a test double should catch. Were the code under +/// test altered to read a differently-named variable, through a rename, a typo, +/// or a new precedence rung, a permissive stub would quietly answer `None` and +/// the test would still pass while asserting nothing about the new read. The +/// panic converts that silent pass into a failure naming the unexpected key. +/// +/// `Default` is deliberately **not** implemented. On a strict stub it would +/// mean "deny every read", so `StubEnv::default()` would compile and then +/// panic at run time for the common "no locale set" case. Requiring +/// [`StubEnv::without_locale`] makes that intent explicit at compile time. +#[derive(Debug, Clone)] pub struct StubEnv { - /// Optional locale value to return for `NETSUKE_LOCALE`. - pub locale: Option, + values: HashMap, + allowed: Vec, } impl StubEnv { - /// Create a stub environment with the provided locale. - pub fn with_locale(locale: impl Into) -> Self { + /// Create a stub declaring nothing; every read panics until one is added. + #[must_use] + pub fn strict() -> Self { Self { - locale: Some(locale.into()), + values: HashMap::new(), + allowed: Vec::new(), } } + + /// Create a stub answering `NETSUKE_LOCALE` with `locale`. + #[must_use] + pub fn with_locale(locale: impl Into) -> Self { + Self::strict().with_var(locale_resolution::NETSUKE_LOCALE_ENV, locale) + } + + /// Create a stub in which `NETSUKE_LOCALE` is unset but may be read. + /// + /// Distinct from a stub that never expected the read at all: an unset + /// variable is a legitimate case to exercise. + #[must_use] + pub fn without_locale() -> Self { + Self::strict().allowing(locale_resolution::NETSUKE_LOCALE_ENV) + } + + /// Answer `key` with `value`. + #[must_use] + pub fn with_var(mut self, key: impl Into, value: impl Into) -> Self { + let key = key.into(); + self.allowed.push(key.clone()); + self.values.insert(key, value.into()); + self + } + + /// Permit `key` to be read, reporting it as unset. + /// + /// Needed because an unset variable is a legitimate case to test, and must + /// be distinguishable from a variable the test never expected to be read. + #[must_use] + pub fn allowing(mut self, key: impl Into) -> Self { + self.allowed.push(key.into()); + self + } } impl LocaleEnvProvider for StubEnv { fn var(&self, key: &str) -> Option { - if key == locale_resolution::NETSUKE_LOCALE_ENV { - return self.locale.clone(); - } - None + assert!( + self.allowed.iter().any(|allowed| allowed == key), + "StubEnv was asked for {key:?}, which the test did not declare. \ + Declare it with `.with_var(..)` or `.allowing(..)` if the read is \ + intended; otherwise the code under test is reading a variable the \ + test does not know about." + ); + self.values.get(key).cloned() } } diff --git a/tests/bdd/steps/cli.rs b/tests/bdd/steps/cli.rs index 450738d9e..39913dec6 100644 --- a/tests/bdd/steps/cli.rs +++ b/tests/bdd/steps/cli.rs @@ -34,9 +34,14 @@ use test_support::locale_stubs::{StubEnv, StubSystemLocale}; /// Tests that do not explicitly set up configuration or environment variables /// may be affected by ambient host configuration. pub(super) fn apply_cli(world: &TestWorld, args: &CliArgs) { - let env = StubEnv { - locale: world.locale_env.get(), - }; + // `NETSUKE_JSON` is read alongside the locale on some startup paths, so it + // is declared as legitimately-readable-but-unset rather than left to trip + // the stub's unexpected-key assertion. + let env = world + .locale_env + .get() + .map_or_else(StubEnv::without_locale, StubEnv::with_locale) + .allowing(netsuke::locale_resolution::NETSUKE_JSON_ENV); let system = StubSystemLocale { locale: world.locale_system.get(), }; diff --git a/tests/bdd/steps/locale_resolution.rs b/tests/bdd/steps/locale_resolution.rs index d8ddee5c8..eded2593f 100644 --- a/tests/bdd/steps/locale_resolution.rs +++ b/tests/bdd/steps/locale_resolution.rs @@ -76,9 +76,14 @@ fn set_cli_override(world: &TestWorld, locale: &str) { #[when("the startup locale is resolved for {args:string}")] fn resolve_startup_locale(world: &TestWorld, args: &str) { - let env = StubEnv { - locale: world.locale_env.get(), - }; + // `NETSUKE_JSON` is read alongside the locale on some startup paths, so it + // is declared as legitimately-readable-but-unset rather than left to trip + // the stub's unexpected-key assertion. + let env = world + .locale_env + .get() + .map_or_else(StubEnv::without_locale, StubEnv::with_locale) + .allowing(netsuke::locale_resolution::NETSUKE_JSON_ENV); let system = StubSystemLocale { locale: world.locale_system.get(), }; diff --git a/tests/locale_resolution_tests.rs b/tests/locale_resolution_tests.rs index 9247f18bc..3d0120b18 100644 --- a/tests/locale_resolution_tests.rs +++ b/tests/locale_resolution_tests.rs @@ -59,7 +59,7 @@ fn resolve_startup_locale_uses_env_then_system() -> Result<()> { "expected env locale to win, got {resolved:?}" ); - let env_fallback = StubEnv::default(); + let env_fallback = StubEnv::without_locale(); let resolved_fallback = resolve_startup_locale(&args, &env_fallback, &system); ensure!( resolved_fallback.as_deref() == Some("es-ES"), From d4b7a59b2100d42763490da1f01251c819bfb999 Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 2 Aug 2026 09:34:19 +0200 Subject: [PATCH 2/8] Fix builder precedence and cover the strictness assertion MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses five review findings on #502. Codex (P2): `allowing` never removed an existing map entry, so `with_var("X", "set").allowing("X")` was documented as declaring X unset yet still answered `Some("set")`. Both builders now let the most recent declaration for a key win, and both orderings are covered. Codex (P1): nothing tested the panic. Relaxing or deleting the assertion would have left every other test passing while restoring the permissive behaviour this change exists to remove. Adds `tests/locale_stub_strictness_tests.rs` covering the panic, that it names the offending key, that declared reads do not panic, both builder orderings, and repeated declaration. Codex (P2): the `NETSUKE_JSON` allowance in the two BDD steps was speculative — neither helper calls `resolve_startup_json`. Keeping it would have let an accidental JSON read added to locale resolution pass silently, defeating the strictness. Removed; the suite still passes. Codex (P1): adds Rustdoc examples to `strict`, `with_locale`, `without_locale`, `with_var`, and `allowing`. CodeRabbit: replaces the escaped string continuations in the assertion message with `concat!()`, per AGENTS.md. Refs #489, #496. Co-Authored-By: Claude Opus 5 (1M context) --- test_support/src/locale_stubs.rs | 79 +++++++++++++++++++++++++-- tests/bdd/steps/cli.rs | 6 +- tests/bdd/steps/locale_resolution.rs | 6 +- tests/locale_stub_strictness_tests.rs | 57 +++++++++++++++++++ 4 files changed, 132 insertions(+), 16 deletions(-) create mode 100644 tests/locale_stub_strictness_tests.rs diff --git a/test_support/src/locale_stubs.rs b/test_support/src/locale_stubs.rs index f3a7730c8..c3df94d55 100644 --- a/test_support/src/locale_stubs.rs +++ b/test_support/src/locale_stubs.rs @@ -29,6 +29,16 @@ pub struct StubEnv { impl StubEnv { /// Create a stub declaring nothing; every read panics until one is added. + /// + /// # Examples + /// + /// ```rust,should_panic + /// use netsuke::locale_resolution::EnvProvider; + /// use test_support::locale_stubs::StubEnv; + /// + /// // Nothing is declared, so any read is a programming error. + /// StubEnv::strict().var("ANYTHING"); + /// ``` #[must_use] pub fn strict() -> Self { Self { @@ -38,6 +48,16 @@ impl StubEnv { } /// Create a stub answering `NETSUKE_LOCALE` with `locale`. + /// + /// # Examples + /// + /// ```rust + /// use netsuke::locale_resolution::EnvProvider; + /// use test_support::locale_stubs::StubEnv; + /// + /// let env = StubEnv::with_locale("es-ES"); + /// assert_eq!(env.var("NETSUKE_LOCALE").as_deref(), Some("es-ES")); + /// ``` #[must_use] pub fn with_locale(locale: impl Into) -> Self { Self::strict().with_var(locale_resolution::NETSUKE_LOCALE_ENV, locale) @@ -47,16 +67,41 @@ impl StubEnv { /// /// Distinct from a stub that never expected the read at all: an unset /// variable is a legitimate case to exercise. + /// + /// # Examples + /// + /// ```rust + /// use netsuke::locale_resolution::EnvProvider; + /// use test_support::locale_stubs::StubEnv; + /// + /// // Declared, so the read is permitted; unset, so it reports `None`. + /// assert_eq!(StubEnv::without_locale().var("NETSUKE_LOCALE"), None); + /// ``` #[must_use] pub fn without_locale() -> Self { Self::strict().allowing(locale_resolution::NETSUKE_LOCALE_ENV) } /// Answer `key` with `value`. + /// + /// The most recent declaration for a key wins, so this overrides an earlier + /// [`StubEnv::allowing`] for the same key. + /// + /// # Examples + /// + /// ```rust + /// use netsuke::locale_resolution::EnvProvider; + /// use test_support::locale_stubs::StubEnv; + /// + /// let env = StubEnv::strict().allowing("X").with_var("X", "set"); + /// assert_eq!(env.var("X").as_deref(), Some("set")); + /// ``` #[must_use] pub fn with_var(mut self, key: impl Into, value: impl Into) -> Self { let key = key.into(); - self.allowed.push(key.clone()); + if !self.allowed.iter().any(|allowed| allowed == &key) { + self.allowed.push(key.clone()); + } self.values.insert(key, value.into()); self } @@ -65,9 +110,28 @@ impl StubEnv { /// /// Needed because an unset variable is a legitimate case to test, and must /// be distinguishable from a variable the test never expected to be read. + /// + /// The most recent declaration for a key wins, so this clears a value set + /// by an earlier [`StubEnv::with_var`]. Were it merely to append to the + /// permitted list, the builder would read as declaring the key unset while + /// still answering with the old value. + /// + /// # Examples + /// + /// ```rust + /// use netsuke::locale_resolution::EnvProvider; + /// use test_support::locale_stubs::StubEnv; + /// + /// let env = StubEnv::strict().with_var("X", "set").allowing("X"); + /// assert_eq!(env.var("X"), None); + /// ``` #[must_use] pub fn allowing(mut self, key: impl Into) -> Self { - self.allowed.push(key.into()); + let key = key.into(); + self.values.remove(&key); + if !self.allowed.iter().any(|allowed| allowed == &key) { + self.allowed.push(key); + } self } } @@ -76,10 +140,13 @@ impl LocaleEnvProvider for StubEnv { fn var(&self, key: &str) -> Option { assert!( self.allowed.iter().any(|allowed| allowed == key), - "StubEnv was asked for {key:?}, which the test did not declare. \ - Declare it with `.with_var(..)` or `.allowing(..)` if the read is \ - intended; otherwise the code under test is reading a variable the \ - test does not know about." + concat!( + "StubEnv was asked for {:?}, which the test did not declare. ", + "Declare it with `.with_var(..)` or `.allowing(..)` if the read ", + "is intended; otherwise the code under test is reading a ", + "variable the test does not know about." + ), + key ); self.values.get(key).cloned() } diff --git a/tests/bdd/steps/cli.rs b/tests/bdd/steps/cli.rs index 39913dec6..344541427 100644 --- a/tests/bdd/steps/cli.rs +++ b/tests/bdd/steps/cli.rs @@ -34,14 +34,10 @@ use test_support::locale_stubs::{StubEnv, StubSystemLocale}; /// Tests that do not explicitly set up configuration or environment variables /// may be affected by ambient host configuration. pub(super) fn apply_cli(world: &TestWorld, args: &CliArgs) { - // `NETSUKE_JSON` is read alongside the locale on some startup paths, so it - // is declared as legitimately-readable-but-unset rather than left to trip - // the stub's unexpected-key assertion. let env = world .locale_env .get() - .map_or_else(StubEnv::without_locale, StubEnv::with_locale) - .allowing(netsuke::locale_resolution::NETSUKE_JSON_ENV); + .map_or_else(StubEnv::without_locale, StubEnv::with_locale); let system = StubSystemLocale { locale: world.locale_system.get(), }; diff --git a/tests/bdd/steps/locale_resolution.rs b/tests/bdd/steps/locale_resolution.rs index eded2593f..822895345 100644 --- a/tests/bdd/steps/locale_resolution.rs +++ b/tests/bdd/steps/locale_resolution.rs @@ -76,14 +76,10 @@ fn set_cli_override(world: &TestWorld, locale: &str) { #[when("the startup locale is resolved for {args:string}")] fn resolve_startup_locale(world: &TestWorld, args: &str) { - // `NETSUKE_JSON` is read alongside the locale on some startup paths, so it - // is declared as legitimately-readable-but-unset rather than left to trip - // the stub's unexpected-key assertion. let env = world .locale_env .get() - .map_or_else(StubEnv::without_locale, StubEnv::with_locale) - .allowing(netsuke::locale_resolution::NETSUKE_JSON_ENV); + .map_or_else(StubEnv::without_locale, StubEnv::with_locale); let system = StubSystemLocale { locale: world.locale_system.get(), }; diff --git a/tests/locale_stub_strictness_tests.rs b/tests/locale_stub_strictness_tests.rs new file mode 100644 index 000000000..d770af0d0 --- /dev/null +++ b/tests/locale_stub_strictness_tests.rs @@ -0,0 +1,57 @@ +//! Tests for `StubEnv`'s strictness about undeclared reads. +//! +//! Without these, relaxing or deleting the assertion would leave every other +//! test passing while restoring exactly the permissive behaviour the stub was +//! made strict to remove. + +use netsuke::locale_resolution::EnvProvider; +use test_support::locale_stubs::StubEnv; + +#[test] +#[should_panic(expected = "which the test did not declare")] +fn undeclared_read_panics() { + StubEnv::strict().var("SOME_UNDECLARED_VARIABLE"); +} + +#[test] +#[should_panic(expected = "SOME_OTHER_VARIABLE")] +fn the_panic_names_the_offending_key() { + StubEnv::with_locale("es-ES").var("SOME_OTHER_VARIABLE"); +} + +#[test] +fn declared_reads_do_not_panic() { + assert_eq!( + StubEnv::with_locale("es-ES") + .var("NETSUKE_LOCALE") + .as_deref(), + Some("es-ES") + ); + assert_eq!(StubEnv::without_locale().var("NETSUKE_LOCALE"), None); +} + +/// The most recent declaration for a key wins, in either order. +/// +/// Were `allowing` merely to append to the permitted list, the second case +/// would read as declaring the key unset while still answering `Some("set")`. +#[test] +fn the_last_declaration_for_a_key_wins() { + let value_then_unset = StubEnv::strict().with_var("X", "set").allowing("X"); + assert_eq!(value_then_unset.var("X"), None, "allowing should clear"); + + let unset_then_value = StubEnv::strict().allowing("X").with_var("X", "set"); + assert_eq!( + unset_then_value.var("X").as_deref(), + Some("set"), + "with_var should override" + ); +} + +/// Declaring a key twice must not make the stub answer differently. +#[test] +fn repeated_declaration_is_idempotent() { + let env = StubEnv::strict() + .with_var("X", "first") + .with_var("X", "second"); + assert_eq!(env.var("X").as_deref(), Some("second")); +} From 469a84d6372c099bc939bf054a46b0181a4d8a78 Mon Sep 17 00:00:00 2001 From: leynos Date: Tue, 4 Aug 2026 15:11:46 +0200 Subject: [PATCH 3/8] Use Oxford -ize spelling in the locale stub doc comment Addresses a CodeRabbit finding on #502. --- test_support/src/locale_stubs.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test_support/src/locale_stubs.rs b/test_support/src/locale_stubs.rs index c3df94d55..c55c28ba7 100644 --- a/test_support/src/locale_stubs.rs +++ b/test_support/src/locale_stubs.rs @@ -10,7 +10,7 @@ use std::collections::HashMap; /// /// Answers only the variables it was given, and **panics** on any other key. /// -/// The permissive alternative — returning `None` for anything unrecognised — +/// The permissive alternative — returning `None` for anything unrecognized — /// hides exactly the change a test double should catch. Were the code under /// test altered to read a differently-named variable, through a rename, a typo, /// or a new precedence rung, a permissive stub would quietly answer `None` and From d47939aebeca9dd0e8e84285874df9e92b62a2d3 Mon Sep 17 00:00:00 2001 From: leynos Date: Wed, 5 Aug 2026 11:50:08 +0200 Subject: [PATCH 4/8] Prove StubEnv's declaration semantics and builder-only construction Two commitments made on the review thread are now tests rather than prose. A property test states the invariant the fixed cases were instances of: over any sequence of with_var and allowing declarations, each key answers per its last declaration and undeclared keys panic, checked against an independent last-write-wins model so a bookkeeping slip between the stub's values and allowed collections cannot agree with itself. The mutation used to validate the property (disabling allowing's value clear) is recorded in the regression seed file. A compile-fail test proves StubEnv::default() does not compile. Trybuild cannot drive it: it removes ambient RUSTFLAGS and overrides workspace build.rustflags, so it would rebuild netsuke without -Zpolonius=next and reject the POLONIUS() sites (dtolnay/trybuild issues #315 and #333, both open). Instead the test_support rlib is built by Cargo, which inherits the ambient flags, and the fixtures are type-checked directly with rustc against that rlib. A control fixture using the sanctioned builders guards the wiring: were the --extern or -L dependency plumbing broken, the rejection would happen for the wrong reason and the compile-fail case would pass vacuously. Co-Authored-By: Claude Fable 5 --- ...stub_strictness_tests.proptest-regressions | 9 + tests/locale_stub_strictness_tests.rs | 66 +++++++ tests/locale_stub_ui_tests.rs | 171 ++++++++++++++++++ tests/ui/stub_env_default_compile_fail.rs | 9 + tests/ui/stub_env_strict_compile_pass.rs | 9 + 5 files changed, 264 insertions(+) create mode 100644 tests/locale_stub_strictness_tests.proptest-regressions create mode 100644 tests/locale_stub_ui_tests.rs create mode 100644 tests/ui/stub_env_default_compile_fail.rs create mode 100644 tests/ui/stub_env_strict_compile_pass.rs diff --git a/tests/locale_stub_strictness_tests.proptest-regressions b/tests/locale_stub_strictness_tests.proptest-regressions new file mode 100644 index 000000000..1a9a7f35c --- /dev/null +++ b/tests/locale_stub_strictness_tests.proptest-regressions @@ -0,0 +1,9 @@ +# Seeds for failure cases proptest has generated in the past. It is +# automatically read and these particular cases re-run before any +# novel cases are generated. +# +# It is recommended to check this file in to source control so that +# everyone who runs the test benefits from these saved cases. +# Recorded while mutation-testing the property (allowing's value clear was +# disabled to prove the test detects it), not from a defect in StubEnv. +cc 04881a9256003f8215b73836a16209c1713861db70e59925b4d1ca3ea1cebec3 # shrinks to declarations = [Set("A", "a"), Allow("A")] diff --git a/tests/locale_stub_strictness_tests.rs b/tests/locale_stub_strictness_tests.rs index d770af0d0..88dfc435f 100644 --- a/tests/locale_stub_strictness_tests.rs +++ b/tests/locale_stub_strictness_tests.rs @@ -55,3 +55,69 @@ fn repeated_declaration_is_idempotent() { .with_var("X", "second"); assert_eq!(env.var("X").as_deref(), Some("second")); } + +mod properties { + //! Property coverage for the builder's declaration semantics. + //! + //! The fixed cases above check single interleavings of `with_var` and + //! `allowing`; this states the invariant they are instances of — the last + //! declaration for a key wins — over arbitrary declaration sequences. + + use super::{EnvProvider, StubEnv}; + use proptest::collection::vec; + use proptest::prelude::*; + use std::collections::HashMap; + use std::panic::{AssertUnwindSafe, catch_unwind}; + + #[derive(Debug, Clone)] + enum Declaration { + Allow(String), + Set(String, String), + } + + /// Three keys only, so generated sequences redeclare the same key often + /// enough for ordering to matter; a wide key space would almost never + /// produce the collisions the invariant is about. + fn declaration() -> impl Strategy { + prop_oneof![ + "[ABC]".prop_map(Declaration::Allow), + ("[ABC]", "[a-z]{1,4}").prop_map(|(key, value)| Declaration::Set(key, value)), + ] + } + + proptest! { + /// Every key answers per its last declaration; undeclared keys panic. + /// + /// The model is a plain last-write-wins map, independent of the + /// stub's split `values`/`allowed` representation, so a bookkeeping + /// slip between the two collections fails here rather than agreeing + /// with itself. + #[test] + fn the_last_declaration_wins_over_any_sequence( + declarations in vec(declaration(), 0..8) + ) { + let mut model: HashMap> = HashMap::new(); + let mut stub = StubEnv::strict(); + for declaration in &declarations { + match declaration { + Declaration::Allow(key) => { + model.insert(key.clone(), None); + stub = stub.allowing(key.clone()); + } + Declaration::Set(key, value) => { + model.insert(key.clone(), Some(value.clone())); + stub = stub.with_var(key.clone(), value.clone()); + } + } + } + for key in ["A", "B", "C"] { + if let Some(expected) = model.get(key) { + prop_assert_eq!(stub.var(key), expected.clone()); + } else { + let read = catch_unwind(AssertUnwindSafe(|| stub.var(key))); + prop_assert!(read.is_err(), "undeclared {} should panic", key); + } + } + } + } +} diff --git a/tests/locale_stub_ui_tests.rs b/tests/locale_stub_ui_tests.rs new file mode 100644 index 000000000..acc803721 --- /dev/null +++ b/tests/locale_stub_ui_tests.rs @@ -0,0 +1,171 @@ +//! Compile-time tests for `StubEnv`'s builder-only construction. +//! +//! `StubEnv` deliberately does not implement `Default`: on a strict stub it +//! would mean "deny every read", so `StubEnv::default()` would compile and +//! then panic at run time for the common "no locale set" case. These tests +//! keep that a compile-time contract rather than a doc-comment promise. +//! +//! Trybuild cannot drive them: it removes ambient `RUSTFLAGS` and overrides +//! workspace `build.rustflags` outright (`env_remove("RUSTFLAGS")` plus +//! `--config=build.rustflags=…` in its cargo invocations), so it would +//! rebuild the `netsuke` dependency without `-Zpolonius=next` and reject the +//! crate's `POLONIUS(...)` sites (see docs/polonius.md). Instead the +//! `test_support` rlib is built by Cargo — which does inherit the ambient +//! flags — and the fixtures are compiled directly with the workspace `rustc` +//! against that rlib. + +use std::{ + io, + path::{Path, PathBuf}, + process::{Command, Output}, +}; + +#[test] +fn stub_env_default_does_not_compile() -> io::Result<()> { + let support = TestSupportRlib::build()?; + let output = support.compile("tests/ui/stub_env_default_compile_fail.rs")?; + + if output.status.success() { + return Err(io::Error::other("StubEnv::default() should not compile")); + } + let stderr = stderr(&output); + if !stderr.contains("E0599") || !stderr.contains("`default`") { + return Err(io::Error::other(format!( + "the rejection should be the missing `default` item, \ + not a harness fault:\n{stderr}", + ))); + } + Ok(()) +} + +/// The builder constructors compile under the same harness. +/// +/// This is the control for the compile-fail case: it fails if the `--extern` +/// or `-L dependency` wiring breaks, which would otherwise make the rejection +/// above pass for the wrong reason. +#[test] +fn stub_env_builders_compile_under_the_same_harness() -> io::Result<()> { + let support = TestSupportRlib::build()?; + let output = support.compile("tests/ui/stub_env_strict_compile_pass.rs")?; + + if !output.status.success() { + return Err(io::Error::other(format!( + "the control fixture should compile; the harness wiring is broken:\n{}", + stderr(&output), + ))); + } + Ok(()) +} + +/// The `test_support` rlib and the deps directory holding its dependencies. +struct TestSupportRlib { + rlib: PathBuf, + deps_dir: PathBuf, +} + +impl TestSupportRlib { + /// Build `test_support` with Cargo and locate the resulting rlib. + /// + /// Cargo inherits the ambient `RUSTFLAGS`, so the rlib is borrow-checked + /// with the same Polonius flags as the rest of the suite — the property + /// trybuild could not preserve. + fn build() -> io::Result { + let output = Command::new(cargo()) + .arg("build") + .arg("--manifest-path") + .arg(manifest_dir().join("test_support/Cargo.toml")) + .arg("--message-format=json") + .output()?; + if !output.status.success() { + return Err(io::Error::other(format!( + "building test_support failed:\n{}", + stderr(&output), + ))); + } + + let stdout = String::from_utf8_lossy(&output.stdout).into_owned(); + let rlib = stdout + .lines() + .filter_map(test_support_rlib_in_message) + .next_back() + .ok_or_else(|| io::Error::other("cargo reported no test_support rlib artefact"))?; + // Cargo uplifts the top-level package's rlib out of `deps/` into the + // profile directory, so the rlib's own parent is not where the + // dependency rlibs live. + let parent = rlib + .parent() + .ok_or_else(|| io::Error::other("the rlib path should have a parent"))?; + let deps_dir = if parent.file_name() == Some(std::ffi::OsStr::new("deps")) { + parent.to_path_buf() + } else { + parent.join("deps") + }; + Ok(Self { rlib, deps_dir }) + } + + /// Type-check `source` against the rlib without linking a binary. + /// + /// `--emit=metadata` is enough to surface the missing-item error while + /// sparing the harness a full link of `test_support`'s dependency tree. + fn compile(&self, source: &str) -> io::Result { + let output_dir = tempfile::tempdir()?; + Command::new(rustc()) + .arg("--edition=2024") + .arg("--crate-type=bin") + .arg("--emit=metadata") + .arg(manifest_dir().join(source)) + .arg("--extern") + .arg(format!("test_support={}", self.rlib.display())) + .arg("-L") + .arg(format!("dependency={}", self.deps_dir.display())) + .arg("-o") + .arg(output_dir.path().join("stub-env-ui.rmeta")) + .output() + } +} + +/// Extract the `test_support` rlib path from one Cargo JSON message, if any. +fn test_support_rlib_in_message(line: &str) -> Option { + let message: serde_json::Value = serde_json::from_str(line).ok()?; + if message.get("reason")? != "compiler-artifact" + || message.get("target")?.get("name")? != "test_support" + { + return None; + } + message + .get("filenames")? + .as_array()? + .iter() + .filter_map(|filename| filename.as_str()) + .filter(|filename| { + Path::new(filename) + .extension() + .is_some_and(|extension| extension.eq_ignore_ascii_case("rlib")) + }) + .map(PathBuf::from) + .next_back() +} + +fn manifest_dir() -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")) +} + +#[expect( + clippy::disallowed_methods, + reason = "locating build tools Cargo reports through the environment; there is no seam to inject and no process state to isolate" +)] +fn cargo() -> PathBuf { + std::env::var_os("CARGO").map_or_else(|| Path::new("cargo").to_path_buf(), PathBuf::from) +} + +#[expect( + clippy::disallowed_methods, + reason = "locating build tools Cargo reports through the environment; there is no seam to inject and no process state to isolate" +)] +fn rustc() -> PathBuf { + std::env::var_os("RUSTC").map_or_else(|| Path::new("rustc").to_path_buf(), PathBuf::from) +} + +fn stderr(output: &Output) -> String { + String::from_utf8_lossy(&output.stderr).into_owned() +} diff --git a/tests/ui/stub_env_default_compile_fail.rs b/tests/ui/stub_env_default_compile_fail.rs new file mode 100644 index 000000000..bb1a99473 --- /dev/null +++ b/tests/ui/stub_env_default_compile_fail.rs @@ -0,0 +1,9 @@ +//! `StubEnv::default()` must not compile. +//! +//! `Default` on a strict stub would mean "deny every read", so the common +//! "no locale set" case would compile and then panic at run time; the builder +//! constructors keep that intent explicit at compile time. + +fn main() { + let _ = test_support::locale_stubs::StubEnv::default(); +} diff --git a/tests/ui/stub_env_strict_compile_pass.rs b/tests/ui/stub_env_strict_compile_pass.rs new file mode 100644 index 000000000..f3aa53332 --- /dev/null +++ b/tests/ui/stub_env_strict_compile_pass.rs @@ -0,0 +1,9 @@ +//! Control fixture proving the harness links `test_support` correctly. +//! +//! Were the `--extern` wiring broken, the compile-fail fixture would be +//! rejected for the wrong reason and its test would pass vacuously; this +//! fixture fails instead, naming the harness as the fault. + +fn main() { + let _ = test_support::locale_stubs::StubEnv::strict(); +} From af63ce5197e6ac4f3163fb17b3dece57ed473e34 Mon Sep 17 00:00:00 2001 From: leynos Date: Wed, 5 Aug 2026 15:28:48 +0200 Subject: [PATCH 5/8] Quieten undeclared-key probes and share one test_support build Round feedback on #502, both points taken. The undeclared-key probes now silence the default panic hook for exactly the catch_unwind call and restore it immediately: 256 cases times three keys of expected panic output was burying genuine failures. The two compile-fixture tests now draw the built rlib from a single #[once] rstest fixture rather than each invoking Cargo, which contended on the target lock and repeated finished work; the expectation on the fixture's expect is scoped to the one statement, since a once fixture cannot return Result. Co-Authored-By: Claude Fable 5 --- tests/locale_stub_strictness_tests.rs | 8 +++++++ tests/locale_stub_ui_tests.rs | 32 ++++++++++++++++++++------- 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/tests/locale_stub_strictness_tests.rs b/tests/locale_stub_strictness_tests.rs index 88dfc435f..d146c0ab2 100644 --- a/tests/locale_stub_strictness_tests.rs +++ b/tests/locale_stub_strictness_tests.rs @@ -114,7 +114,15 @@ mod properties { if let Some(expected) = model.get(key) { prop_assert_eq!(stub.var(key), expected.clone()); } else { + // Silence the default panic hook around the probe: each + // undeclared read otherwise prints its full panic message, + // and 256 cases times three keys of that buries any + // genuine failure output. The hook is process-wide, so it + // is restored immediately rather than left installed. + let prior = std::panic::take_hook(); + std::panic::set_hook(Box::new(|_| {})); let read = catch_unwind(AssertUnwindSafe(|| stub.var(key))); + std::panic::set_hook(prior); prop_assert!(read.is_err(), "undeclared {} should panic", key); } } diff --git a/tests/locale_stub_ui_tests.rs b/tests/locale_stub_ui_tests.rs index acc803721..2692fe9b5 100644 --- a/tests/locale_stub_ui_tests.rs +++ b/tests/locale_stub_ui_tests.rs @@ -14,16 +14,31 @@ //! flags — and the fixtures are compiled directly with the workspace `rustc` //! against that rlib. +use rstest::{fixture, rstest}; use std::{ io, path::{Path, PathBuf}, process::{Command, Output}, }; -#[test] -fn stub_env_default_does_not_compile() -> io::Result<()> { - let support = TestSupportRlib::build()?; - let output = support.compile("tests/ui/stub_env_default_compile_fail.rs")?; +/// One `test_support` build shared by both tests. +/// +/// Built once: the two tests run in parallel, so independent builds would +/// contend on Cargo's target-directory lock and repeat completed work. +#[fixture] +#[once] +fn test_support_rlib() -> TestSupportRlib { + #[expect( + clippy::expect_used, + reason = "a once fixture cannot return Result; a build failure must abort the suite here" + )] + let rlib = TestSupportRlib::build().expect("test_support should build"); + rlib +} + +#[rstest] +fn stub_env_default_does_not_compile(test_support_rlib: &TestSupportRlib) -> io::Result<()> { + let output = test_support_rlib.compile("tests/ui/stub_env_default_compile_fail.rs")?; if output.status.success() { return Err(io::Error::other("StubEnv::default() should not compile")); @@ -43,10 +58,11 @@ fn stub_env_default_does_not_compile() -> io::Result<()> { /// This is the control for the compile-fail case: it fails if the `--extern` /// or `-L dependency` wiring breaks, which would otherwise make the rejection /// above pass for the wrong reason. -#[test] -fn stub_env_builders_compile_under_the_same_harness() -> io::Result<()> { - let support = TestSupportRlib::build()?; - let output = support.compile("tests/ui/stub_env_strict_compile_pass.rs")?; +#[rstest] +fn stub_env_builders_compile_under_the_same_harness( + test_support_rlib: &TestSupportRlib, +) -> io::Result<()> { + let output = test_support_rlib.compile("tests/ui/stub_env_strict_compile_pass.rs")?; if !output.status.success() { return Err(io::Error::other(format!( From efbc241c812ac8ef56c81c09aae68a1a182c770e Mon Sep 17 00:00:00 2001 From: leynos Date: Thu, 6 Aug 2026 03:59:12 +0200 Subject: [PATCH 6/8] Adapt the strict stub to the LocaleEnvProvider rename Post-rebase reconciliation: main renamed the locale trait to LocaleEnvProvider and the hardened lint suite now denies shadowed bindings, so the builder's key parameters rebind as name. Co-Authored-By: Claude Fable 5 --- test_support/src/locale_stubs.rs | 26 +++++++++++++------------- tests/locale_stub_strictness_tests.rs | 4 ++-- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/test_support/src/locale_stubs.rs b/test_support/src/locale_stubs.rs index c55c28ba7..e9c604f40 100644 --- a/test_support/src/locale_stubs.rs +++ b/test_support/src/locale_stubs.rs @@ -33,7 +33,7 @@ impl StubEnv { /// # Examples /// /// ```rust,should_panic - /// use netsuke::locale_resolution::EnvProvider; + /// use netsuke::locale_resolution::LocaleEnvProvider; /// use test_support::locale_stubs::StubEnv; /// /// // Nothing is declared, so any read is a programming error. @@ -52,7 +52,7 @@ impl StubEnv { /// # Examples /// /// ```rust - /// use netsuke::locale_resolution::EnvProvider; + /// use netsuke::locale_resolution::LocaleEnvProvider; /// use test_support::locale_stubs::StubEnv; /// /// let env = StubEnv::with_locale("es-ES"); @@ -71,7 +71,7 @@ impl StubEnv { /// # Examples /// /// ```rust - /// use netsuke::locale_resolution::EnvProvider; + /// use netsuke::locale_resolution::LocaleEnvProvider; /// use test_support::locale_stubs::StubEnv; /// /// // Declared, so the read is permitted; unset, so it reports `None`. @@ -90,7 +90,7 @@ impl StubEnv { /// # Examples /// /// ```rust - /// use netsuke::locale_resolution::EnvProvider; + /// use netsuke::locale_resolution::LocaleEnvProvider; /// use test_support::locale_stubs::StubEnv; /// /// let env = StubEnv::strict().allowing("X").with_var("X", "set"); @@ -98,11 +98,11 @@ impl StubEnv { /// ``` #[must_use] pub fn with_var(mut self, key: impl Into, value: impl Into) -> Self { - let key = key.into(); - if !self.allowed.iter().any(|allowed| allowed == &key) { - self.allowed.push(key.clone()); + let name = key.into(); + if !self.allowed.iter().any(|allowed| allowed == &name) { + self.allowed.push(name.clone()); } - self.values.insert(key, value.into()); + self.values.insert(name, value.into()); self } @@ -119,7 +119,7 @@ impl StubEnv { /// # Examples /// /// ```rust - /// use netsuke::locale_resolution::EnvProvider; + /// use netsuke::locale_resolution::LocaleEnvProvider; /// use test_support::locale_stubs::StubEnv; /// /// let env = StubEnv::strict().with_var("X", "set").allowing("X"); @@ -127,10 +127,10 @@ impl StubEnv { /// ``` #[must_use] pub fn allowing(mut self, key: impl Into) -> Self { - let key = key.into(); - self.values.remove(&key); - if !self.allowed.iter().any(|allowed| allowed == &key) { - self.allowed.push(key); + let name = key.into(); + self.values.remove(&name); + if !self.allowed.iter().any(|allowed| allowed == &name) { + self.allowed.push(name); } self } diff --git a/tests/locale_stub_strictness_tests.rs b/tests/locale_stub_strictness_tests.rs index d146c0ab2..634274c03 100644 --- a/tests/locale_stub_strictness_tests.rs +++ b/tests/locale_stub_strictness_tests.rs @@ -4,7 +4,7 @@ //! test passing while restoring exactly the permissive behaviour the stub was //! made strict to remove. -use netsuke::locale_resolution::EnvProvider; +use netsuke::locale_resolution::LocaleEnvProvider; use test_support::locale_stubs::StubEnv; #[test] @@ -63,7 +63,7 @@ mod properties { //! `allowing`; this states the invariant they are instances of — the last //! declaration for a key wins — over arbitrary declaration sequences. - use super::{EnvProvider, StubEnv}; + use super::{LocaleEnvProvider, StubEnv}; use proptest::collection::vec; use proptest::prelude::*; use std::collections::HashMap; From 2e94a59683a6435c8a1b20c09cbfca3b81702427 Mon Sep 17 00:00:00 2001 From: leynos Date: Thu, 6 Aug 2026 11:47:27 +0200 Subject: [PATCH 7/8] Serialize the panic-hook swap and document StubEnv's contract Round feedback on #502, all three points taken. The undeclared-key probes' hook swap is serialized behind a lock: nextest isolates each test in a process, but the in-process runner used for coverage runs tests as threads, and an unsynchronized take/set pair could strand one thread's no-op hook as another's restored state. The UI harness message uses concat! with an explicit placeholder rather than an escaped continuation. The developers' guide gains a StubEnv strictness section covering the trichotomy, last-declaration-wins, and the compile-time refusal of Default. Co-Authored-By: Claude Fable 5 --- docs/developers-guide.md | 39 +++++++++++++++++++++++++++ tests/locale_stub_strictness_tests.rs | 21 ++++++++++----- tests/locale_stub_ui_tests.rs | 7 +++-- 3 files changed, 59 insertions(+), 8 deletions(-) diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 9ed87131b..319ebcc3b 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -1691,6 +1691,45 @@ lock is still held; `try_lock` from the owning thread returns `WouldBlock`, so "blocked" means the bundle still holds it. Reverting the field order turns that assertion red deterministically. +### `StubEnv` strictness + +`test_support::locale_stubs::StubEnv` is the environment-variable test double +used by locale-resolution tests. It answers only the keys a test declares, and +**panics**, naming the key, on any other read. The permissive alternative — +returning `None` for anything unrecognized — hides exactly the regression a +test double should catch: if the code under test starts reading a differently +named variable, through a rename, a typo, or a new precedence rung, a +permissive stub answers `None` and the test still passes, asserting nothing +about the new read. Recognize the panic message, `"which the test did not +declare"`, when a test starts failing after a rename; it means the test's +declarations need updating, not that the stub is broken. + +Three distinct states are representable for a key: **declared with a value** +(`with_var`), **declared but unset** (`allowing`, which reports `None`), and +**undeclared** (any other key, which panics). The middle case matters because +an unset variable is a legitimate scenario to exercise, and it must be +distinguishable from a variable the test never expected to be read at all. +`StubEnv::with_locale` and `StubEnv::without_locale` are the common +constructors for `NETSUKE_LOCALE`; `strict()` starts from nothing declared. + +Declaring the same key twice is well-defined: the most recent declaration +wins, in either order. `allowing` after `with_var` clears the value; `with_var` +after `allowing` restores one. Were `allowing` merely to append to the +permitted-keys list rather than clearing the stored value, it would read as +declaring the key unset while still answering with the earlier value. + +`Default` is deliberately **not** implemented for `StubEnv`. On a strict stub, +"default" would have to mean "deny every read", so `StubEnv::default()` would +compile and then panic at run time for the common "no locale set" case; +requiring `StubEnv::without_locale()` instead makes that intent explicit at +compile time. This refusal is itself a tested contract: +`tests/locale_stub_ui_tests.rs` compiles a fixture calling +`StubEnv::default()` directly with `rustc` and asserts the compile fails with +`E0599` naming the missing `default` item, guarding against the constraint +regressing to a doc-comment promise. `tests/locale_stub_strictness_tests.rs` +covers the panic, the trichotomy, and the last-declaration-wins rule with +both example-based and property tests. + ### Manifest `env()` reader The `env()` Jinja helper reads through an injected [`EnvReader`], a shared diff --git a/tests/locale_stub_strictness_tests.rs b/tests/locale_stub_strictness_tests.rs index 634274c03..82e979dd3 100644 --- a/tests/locale_stub_strictness_tests.rs +++ b/tests/locale_stub_strictness_tests.rs @@ -117,12 +117,21 @@ mod properties { // Silence the default panic hook around the probe: each // undeclared read otherwise prints its full panic message, // and 256 cases times three keys of that buries any - // genuine failure output. The hook is process-wide, so it - // is restored immediately rather than left installed. - let prior = std::panic::take_hook(); - std::panic::set_hook(Box::new(|_| {})); - let read = catch_unwind(AssertUnwindSafe(|| stub.var(key))); - std::panic::set_hook(prior); + // genuine failure output. The hook is process-wide, so + // the swap is serialized behind a lock — nextest runs + // each test in its own process, but the in-process + // runner used for coverage runs tests as threads, and an + // unsynchronized take/set pair could strand the no-op + // hook installed for another thread's probe. + let read = { + static HOOK_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + let _guard = HOOK_LOCK.lock().unwrap_or_else(std::sync::PoisonError::into_inner); + let prior = std::panic::take_hook(); + std::panic::set_hook(Box::new(|_| {})); + let read = catch_unwind(AssertUnwindSafe(|| stub.var(key))); + std::panic::set_hook(prior); + read + }; prop_assert!(read.is_err(), "undeclared {} should panic", key); } } diff --git a/tests/locale_stub_ui_tests.rs b/tests/locale_stub_ui_tests.rs index 2692fe9b5..fc9a9eb9f 100644 --- a/tests/locale_stub_ui_tests.rs +++ b/tests/locale_stub_ui_tests.rs @@ -46,8 +46,11 @@ fn stub_env_default_does_not_compile(test_support_rlib: &TestSupportRlib) -> io: let stderr = stderr(&output); if !stderr.contains("E0599") || !stderr.contains("`default`") { return Err(io::Error::other(format!( - "the rejection should be the missing `default` item, \ - not a harness fault:\n{stderr}", + concat!( + "the rejection should be the missing `default` item, ", + "not a harness fault:\n{}", + ), + stderr ))); } Ok(()) From 6fa22cea35244a7be93951748a0d11fd54ed1320 Mon Sep 17 00:00:00 2001 From: leynos Date: Thu, 6 Aug 2026 12:02:03 +0200 Subject: [PATCH 8/8] Gate the panic hook per thread instead of swapping it The probe silenced the default hook by taking it, installing a no-op, and restoring the original afterwards. Serializing that swap behind a lock kept the probes from stranding each other's hook, but the swap is still process-wide: under the threaded in-process coverage runner a concurrent test's panic during the window is silenced, and the restore can overwrite a hook installed by someone else in the meantime. Install a wrapper exactly once instead, delegating to the prior hook unless a thread-local flag marks the current thread as inside a probe. Other threads' panics always reach the original hook, and nothing is ever restored, so there is no window and no race. Co-Authored-By: Claude Fable 5 --- tests/locale_stub_strictness_tests.rs | 57 ++++++++++++++++++--------- 1 file changed, 39 insertions(+), 18 deletions(-) diff --git a/tests/locale_stub_strictness_tests.rs b/tests/locale_stub_strictness_tests.rs index 82e979dd3..7b6f0b39b 100644 --- a/tests/locale_stub_strictness_tests.rs +++ b/tests/locale_stub_strictness_tests.rs @@ -66,8 +66,10 @@ mod properties { use super::{LocaleEnvProvider, StubEnv}; use proptest::collection::vec; use proptest::prelude::*; + use std::cell::Cell; use std::collections::HashMap; use std::panic::{AssertUnwindSafe, catch_unwind}; + use std::sync::Once; #[derive(Debug, Clone)] enum Declaration { @@ -85,6 +87,42 @@ mod properties { ] } + thread_local! { + static SILENCED: Cell = const { Cell::new(false) }; + } + + /// Install the gated hook exactly once, wrapping whatever hook was + /// current when the first probe ran. + fn install_gated_hook() { + let prior = std::panic::take_hook(); + let gated = move |info: &std::panic::PanicHookInfo<'_>| { + if SILENCED.with(Cell::get) { + return; + } + prior(info); + }; + std::panic::set_hook(Box::new(gated)); + } + + /// Run `probe` with the default panic hook silenced for this thread only. + /// + /// Each undeclared read otherwise prints its full panic message, and 256 + /// cases times three keys of that buries any genuine failure output. The + /// hook is process-wide, so instead of swapping it around each probe — + /// which under the threaded in-process coverage runner could eat another + /// test's panic or race the restore — a wrapper is installed once and + /// consults a thread-local flag, leaving every other thread's panics on + /// the prior hook. + fn silenced(probe: impl FnOnce() -> T) -> T { + static INSTALL: Once = Once::new(); + INSTALL.call_once(install_gated_hook); + + SILENCED.with(|flag| flag.set(true)); + let result = probe(); + SILENCED.with(|flag| flag.set(false)); + result + } + proptest! { /// Every key answers per its last declaration; undeclared keys panic. /// @@ -114,24 +152,7 @@ mod properties { if let Some(expected) = model.get(key) { prop_assert_eq!(stub.var(key), expected.clone()); } else { - // Silence the default panic hook around the probe: each - // undeclared read otherwise prints its full panic message, - // and 256 cases times three keys of that buries any - // genuine failure output. The hook is process-wide, so - // the swap is serialized behind a lock — nextest runs - // each test in its own process, but the in-process - // runner used for coverage runs tests as threads, and an - // unsynchronized take/set pair could strand the no-op - // hook installed for another thread's probe. - let read = { - static HOOK_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); - let _guard = HOOK_LOCK.lock().unwrap_or_else(std::sync::PoisonError::into_inner); - let prior = std::panic::take_hook(); - std::panic::set_hook(Box::new(|_| {})); - let read = catch_unwind(AssertUnwindSafe(|| stub.var(key))); - std::panic::set_hook(prior); - read - }; + let read = silenced(|| catch_unwind(AssertUnwindSafe(|| stub.var(key)))); prop_assert!(read.is_err(), "undeclared {} should panic", key); } }