From 5b031012b59c7592957fff6d48b945be8b2a1420 Mon Sep 17 00:00:00 2001 From: 21M Date: Sat, 22 Aug 2026 04:13:47 +0200 Subject: [PATCH] test: pin the v2 arm of gate_applies_to_v2_only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `assert_eq!(gate_for(true).is_some(), SpamGate::global().is_some())` compares `gate_for` with itself: it returns exactly that global. Until something installs a gate in the process both sides are `None` and the assert passes without checking anything — making `gate_for(true)` return `None` unconditionally went undetected. The two sides are also read at different instants while tests run in parallel, so an install landing between them yields a spurious `false == true`. Install the gate in the test and assert `is_some()` outright. That forces a second change. `install_global_then_second_install_is_rejected` required being the process's first install, which only held while it was the sole installer. Adopt what `PriceManager`'s equivalent test already does — ignore the install result and pin the contract that survives any ordering: a gate is exposed, and a second install is refused, not a panic. Without it, `--test-threads=1` runs `app::…` first and that test fails. Also refresh a comment naming `is_v2`, a variable the event loops no longer have since #892; they pass the condition to `gate_for`. Closes #913 --- src/app.rs | 20 +++++++++++++++----- src/spam_gate.rs | 9 +++++---- 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/src/app.rs b/src/app.rs index 2170c575..022c88f9 100644 --- a/src/app.rs +++ b/src/app.rs @@ -784,14 +784,17 @@ mod tests { forged } + /// Drive `accept_event` over a v2 (kind-14) event with an explicit + /// gate, the shape both ordering tests below need. async fn accept( ctx: &AppContext, event: &Event, mostro: &Keys, gate: &SpamGate, ) -> Option<(Action, Message, UnwrappedMessage)> { - // Same constant the event loops derive `is_v2` from, so the test - // fails if it ever drifts from `Transport::Nip44Direct`'s kind. + // Same kind constant the event loops use to identify v2 before + // calling `gate_for`, so the test fails if it drifts from + // `Transport::Nip44Direct`'s kind. accept_event( ctx, event, @@ -909,9 +912,16 @@ mod tests { #[test] fn gate_applies_to_v2_only() { assert!(gate_for(false).is_none(), "v1 must fail open"); - // `SpamGate::global()` is `None` unless `install_spam_gate` ran, so - // the v2 arm can only be pinned against the installed state. - assert_eq!(gate_for(true).is_some(), SpamGate::global().is_some()); + // Force the installed state rather than reading it: asserting + // against `SpamGate::global()` compares `gate_for` with itself and + // passes vacuously while nothing has installed yet. Install wins + // here or another test's already did; either way a second one is + // refused, not a panic. + let _ = SpamGate::new(REPLAY_WINDOW_SECS).install_global(); + assert!( + gate_for(true).is_some(), + "v2 must resolve the installed gate" + ); } } diff --git a/src/spam_gate.rs b/src/spam_gate.rs index 37748cc8..b3eaec57 100644 --- a/src/spam_gate.rs +++ b/src/spam_gate.rs @@ -226,12 +226,13 @@ mod tests { assert!(guard.check_and_record(id, 1_000 + 61)); } + /// The `OnceLock` is process-wide and other tests install it too, so this + /// one cannot claim the first install (same reason as + /// `PriceManager::install_global_accepts_once_then_refuses`). The contract + /// asserted below holds either way. #[test] fn install_global_then_second_install_is_rejected() { - // The OnceLock is process-wide, so this single test owns both the - // first (successful) install and the AlreadyInstalled rejection. - let first = SpamGate::new(REPLAY_WINDOW_SECS).install_global(); - assert!(first.is_ok(), "first install must succeed"); + let _ = SpamGate::new(REPLAY_WINDOW_SECS).install_global(); assert!( SpamGate::global().is_some(), "global() must expose the installed gate"