From a6a4e6f4854c71b08e4a25403a7bb7b90ef16730 Mon Sep 17 00:00:00 2001 From: Avi Date: Thu, 24 Sep 2026 18:24:15 -0500 Subject: [PATCH] =?UTF-8?q?test(nip46):=20kill=20the=20e2e=20flake=20?= =?UTF-8?q?=E2=80=94=20local=20relay=20for=20strict=20kind-0,=20poison-tol?= =?UTF-8?q?erant=20vault=20lock?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The suite failed intermittently (1 in ~4 runs) with 3-5 simultaneous failures. Two stacked causes: 1. REAL flake: nip46_bunker_connect_params_match_spec_against_strict_amber published its kind-0 through the DEFAULT relay set — the real internet (wss://relay.damus.io + the known-hanging relay.nostr.band) — so 'at least one relay must accept the signed kind-0' was a network lottery against the 6s send timeout. The QR test already pins settings to the in-process relay; the strict test shipped without it. Now pinned too: zero network dependency, deterministic. 2. CASCADE: a panic while holding VAULT_ENV_LOCK poisoned the mutex, so every later test died on PoisonError and one flake reported as many. The lock only serializes process-global XDG_DATA_HOME, so all four sites now unwrap_or_else into_inner — a real failure reports as ONE. 10/10 consecutive green e2e runs (was ~1 in 4 failing); suite time now uniform ~21.5s (was bimodal — the long tail was network waiting). cargo test 216 unit + 5 e2e green; clippy 0 warnings; fmt clean. --- tests/nip46_e2e.rs | 42 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 4 deletions(-) diff --git a/tests/nip46_e2e.rs b/tests/nip46_e2e.rs index 5e20139..c818e12 100644 --- a/tests/nip46_e2e.rs +++ b/tests/nip46_e2e.rs @@ -345,7 +345,13 @@ async fn nip46_client_handshake_and_sign_against_fake_amber() { // XDG_DATA_HOME is process-global and every test in this binary sets it, // so the lock is held for the WHOLE test: data_dir() re-reads the env on // every save, and a setup-only guard lets parallel tests cross-write. - let _vault_guard = VAULT_ENV_LOCK.lock().unwrap(); + let _vault_guard = VAULT_ENV_LOCK + .lock() + // Poison-tolerant on purpose: this lock only serializes the + // process-global XDG_DATA_HOME. A sibling test that panics while + // holding it must not cascade into PoisonError failures of every + // later test — one real failure should report as ONE failure. + .unwrap_or_else(|e| e.into_inner()); let app = { let tmp = std::env::temp_dir().join(format!("keynectr-e2e-{}", std::process::id())); // data_dir() is $XDG_DATA_HOME/keynectr — the isolation vault must @@ -615,7 +621,13 @@ async fn run_strict_amber( #[allow(clippy::await_holding_lock)] async fn nip46_bunker_connect_params_match_spec_against_strict_amber() { // Whole-body env lock: data_dir() re-reads XDG_DATA_HOME on every save. - let _vault_guard = VAULT_ENV_LOCK.lock().unwrap(); + let _vault_guard = VAULT_ENV_LOCK + .lock() + // Poison-tolerant on purpose: this lock only serializes the + // process-global XDG_DATA_HOME. A sibling test that panics while + // holding it must not cascade into PoisonError failures of every + // later test — one real failure should report as ONE failure. + .unwrap_or_else(|e| e.into_inner()); let app = { let tmp = std::env::temp_dir().join(format!("keynectr-e2e-strict-{}", std::process::id())); let app_dir = tmp.join("keynectr"); @@ -714,6 +726,16 @@ async fn nip46_bunker_connect_params_match_spec_against_strict_amber() { // network-visible profile (what other clients display as the name). // Exercises the real GUI "Publish name" path — Signing::External with // identity validation — against the strict signer. + // Point settings at the in-process relay FIRST: the default relay set is + // the real internet (damus + nostr.band, the latter a known-hanger), so + // leaving it made this assertion a network lottery — it failed whenever + // damus dawdled past the 6s send timeout. The QR test already does this; + // the strict test shipped without it. + { + let mut a = app.lock().await; + a.settings.relays = vec![keynectr::settings::RelayConfig::new(relay_url.clone())]; + a.save_settings().expect("save settings"); + } let (settings, signing) = { let guard = app.lock().await; ( @@ -917,7 +939,13 @@ async fn run_qr_pairing(connect_shape: &'static str) { // Isolated vault + pairing relay; the env lock is held for the whole // helper body (see above) so parallel tests cannot cross-write vaults. - let _vault_guard = VAULT_ENV_LOCK.lock().unwrap(); + let _vault_guard = VAULT_ENV_LOCK + .lock() + // Poison-tolerant on purpose: this lock only serializes the + // process-global XDG_DATA_HOME. A sibling test that panics while + // holding it must not cascade into PoisonError failures of every + // later test — one real failure should report as ONE failure. + .unwrap_or_else(|e| e.into_inner()); let app = { let tmp = std::env::temp_dir().join(format!( "keynectr-e2e-pair-{}-{}", @@ -1073,7 +1101,13 @@ async fn run_qr_pairing(connect_shape: &'static str) { #[tokio::test(flavor = "multi_thread", worker_threads = 4)] #[allow(clippy::await_holding_lock)] async fn nip46_session_restore_redials_and_refuses_wrong_identity() { - let _vault_guard = VAULT_ENV_LOCK.lock().unwrap(); + let _vault_guard = VAULT_ENV_LOCK + .lock() + // Poison-tolerant on purpose: this lock only serializes the + // process-global XDG_DATA_HOME. A sibling test that panics while + // holding it must not cascade into PoisonError failures of every + // later test — one real failure should report as ONE failure. + .unwrap_or_else(|e| e.into_inner()); let app = { let tmp = std::env::temp_dir().join(format!( "keynectr-e2e-restore-{}-{}",