Make the locale test stub strict about unexpected variable reads (#489)

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

test_support/src/locale_stubs.rs

@@ -4,29 +4,151 @@
 //! 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 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
+/// 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<String>,
+    values: HashMap<String, String>,
+    allowed: Vec<String>,
 }
 
 impl StubEnv {
-    /// Create a stub environment with the provided locale.
-    pub fn with_locale(locale: impl Into<String>) -> Self {
+    /// Create a stub declaring nothing; every read panics until one is added.
+    ///
+    /// # Examples
+    ///
+    /// ```rust,should_panic
+    /// use netsuke::locale_resolution::LocaleEnvProvider;
+    /// 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 {
-            locale: Some(locale.into()),
+            values: HashMap::new(),
+            allowed: Vec::new(),
+        }
+    }
+
+    /// Create a stub answering `NETSUKE_LOCALE` with `locale`.
+    ///
+    /// # Examples
+    ///
+    /// ```rust
+    /// use netsuke::locale_resolution::LocaleEnvProvider;
+    /// 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<String>) -> 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.
+    ///
+    /// # Examples
+    ///
+    /// ```rust
+    /// use netsuke::locale_resolution::LocaleEnvProvider;
+    /// 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::LocaleEnvProvider;
+    /// 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<String>, value: impl Into<String>) -> Self {
+        let name = key.into();
+        if !self.allowed.iter().any(|allowed| allowed == &name) {
+            self.allowed.push(name.clone());
         }
+        self.values.insert(name, 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.
+    ///
+    /// 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::LocaleEnvProvider;
+    /// 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<String>) -> Self {
+        let name = key.into();
+        self.values.remove(&name);
+        if !self.allowed.iter().any(|allowed| allowed == &name) {
+            self.allowed.push(name);
+        }
+        self
     }
 }
 
 impl LocaleEnvProvider for StubEnv {
     fn var(&self, key: &str) -> Option<String> {
-        if key == locale_resolution::NETSUKE_LOCALE_ENV {
-            return self.locale.clone();
-        }
-        None
+        assert!(
+            self.allowed.iter().any(|allowed| allowed == key),
+            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()
     }
 }
 

tests/bdd/steps/cli.rs

@@ -34,9 +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) {
-    let env = StubEnv {
-        locale: world.locale_env.get(),
-    };
+    let env = world
+        .locale_env
+        .get()
+        .map_or_else(StubEnv::without_locale, StubEnv::with_locale);
     let system = StubSystemLocale {
         locale: world.locale_system.get(),
     };

tests/bdd/steps/locale_resolution.rs

@@ -76,9 +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) {
-    let env = StubEnv {
-        locale: world.locale_env.get(),
-    };
+    let env = world
+        .locale_env
+        .get()
+        .map_or_else(StubEnv::without_locale, StubEnv::with_locale);
     let system = StubSystemLocale {
         locale: world.locale_system.get(),
     };

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"),

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")]