From 8264a3e3a8352fb32f6788eb56e615c5daf5bee8 Mon Sep 17 00:00:00 2001 From: Jeff Gardner <202880+erskingardner@users.noreply.github.com> Date: Sun, 5 Jul 2026 15:17:18 +0100 Subject: [PATCH 1/3] test: deflake two timing-dependent tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both tests intermittently failed under full-suite concurrency / coverage instrumentation and passed in isolation. shutdown::wait_for_signal_or_trigger_returns_on_internal_trigger relied on two real wall-clock timeouts: a 10ms window to prove the wait was still pending, and a 1s window for it to resolve after the trigger. Under load the 1s bound could be exceeded by a starved task. Replace the 10ms check with a single deterministic poll (no-op waker) — the first poll subscribes the receiver and registers the signal handlers, so with neither fired the future is provably Pending — and await the triggered future directly instead of racing a timeout. events::test_process_rejects_future_notification_rumor dated the event only +1s beyond the future-skew tolerance. process() re-reads Timestamp::now() (1-second granularity) when validating freshness, so a whole-second tick between building the event and that read collapsed the margin onto the exact threshold (> became ==) and spuriously accepted the event. Widen the margin to +1h; there is no injectable clock at the check site, so a drift-proof margin is the deterministic fix without a production API change. Verified 20/20 each under cargo llvm-cov; full suite 647 pass. Co-Authored-By: Claude Fable 5 --- src/nostr/events.rs | 8 +++++++- src/shutdown.rs | 22 ++++++++++++++++------ 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/src/nostr/events.rs b/src/nostr/events.rs index 9558b38..7548dac 100644 --- a/src/nostr/events.rs +++ b/src/nostr/events.rs @@ -1764,10 +1764,16 @@ mod tests { let content = NotificationContentBuilder::new(&server_keys) .with_apns_token("deadbeef12345678deadbeef12345678deadbeef12345678deadbeef12345678") .build(); + // One hour beyond the skew tolerance, not +1s: `process()` re-reads + // `Timestamp::now()` (1-second granularity) when it validates freshness, + // so a whole-second wall-clock tick between here and that read would + // collapse a +1s margin onto the exact threshold (`>` becomes `==`) and + // spuriously accept the event under load. A large margin exercises the + // same "created_at exceeds now + skew" rejection path, drift-proof. let future_created_at = Timestamp::from_secs( Timestamp::now() .as_secs() - .saturating_add(DEFAULT_MAX_NOTIFICATION_FUTURE_SKEW_SECS + 1), + .saturating_add(DEFAULT_MAX_NOTIFICATION_FUTURE_SKEW_SECS + 3600), ); let event = GiftWrapBuilder::new(server_keys.clone(), sender_keys) .build_with_created_at(&content, future_created_at) diff --git a/src/shutdown.rs b/src/shutdown.rs index 56ddb95..f750714 100644 --- a/src/shutdown.rs +++ b/src/shutdown.rs @@ -315,21 +315,31 @@ mod tests { #[tokio::test] async fn wait_for_signal_or_trigger_returns_on_internal_trigger() { + use std::future::Future; + use std::task::{Context, Poll, Waker}; + let handler = ShutdownHandler::new(); let trigger = handler.trigger_handle(); let wait = handler.wait_for_signal_or_trigger(); tokio::pin!(wait); - // No signal and no trigger yet: the wait must still be pending. - let pending = timeout(Duration::from_millis(10), &mut wait).await; - assert!(pending.is_err()); + // No signal and no trigger yet: the wait must still be pending. Poll + // once with a no-op waker instead of racing a wall-clock timeout — the + // first poll subscribes the receiver and registers the signal handlers, + // and with neither fired the future is deterministically `Pending`. + let mut cx = Context::from_waker(Waker::noop()); + assert!( + matches!(wait.as_mut().poll(&mut cx), Poll::Pending), + "wait must be pending before any signal or trigger" + ); trigger.trigger(); - let reason = timeout(Duration::from_secs(1), &mut wait) - .await - .expect("internal trigger must complete the shutdown wait"); + // The trigger set the watch to `true`; awaiting the same future now + // resolves deterministically (a hang here would be a real bug the outer + // test harness surfaces, not a timing flake). + let reason = wait.await; assert_eq!(reason, ShutdownReason::InternalTrigger); } From 814eb03b475c649f3f8339f1cee9a4693f454869 Mon Sep 17 00:00:00 2001 From: Jeff Gardner <202880+erskingardner@users.noreply.github.com> Date: Sun, 5 Jul 2026 15:24:02 +0100 Subject: [PATCH 2/3] test: assert Pending via is_pending() to avoid an uncovered match arm The matches!(poll, Poll::Pending) form introduced an implicit non-matching wildcard arm that the always-Pending poll never exercises, a new uncovered region that tripped the fail-on-any-decrease coverage gate by ~0.0002pp. Use Poll::is_pending() (same assert-on-bool shape as the wall-clock assertion it replaced), which has no uncovered arm. Co-Authored-By: Claude Fable 5 --- src/shutdown.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/shutdown.rs b/src/shutdown.rs index f750714..53f553e 100644 --- a/src/shutdown.rs +++ b/src/shutdown.rs @@ -316,7 +316,7 @@ mod tests { #[tokio::test] async fn wait_for_signal_or_trigger_returns_on_internal_trigger() { use std::future::Future; - use std::task::{Context, Poll, Waker}; + use std::task::{Context, Waker}; let handler = ShutdownHandler::new(); let trigger = handler.trigger_handle(); @@ -330,7 +330,7 @@ mod tests { // and with neither fired the future is deterministically `Pending`. let mut cx = Context::from_waker(Waker::noop()); assert!( - matches!(wait.as_mut().poll(&mut cx), Poll::Pending), + wait.as_mut().poll(&mut cx).is_pending(), "wait must be pending before any signal or trigger" ); From d252e27d98b3c4cd0e48e6fd7abb9126209951a6 Mon Sep 17 00:00:00 2001 From: Jeff Gardner <202880+erskingardner@users.noreply.github.com> Date: Sun, 5 Jul 2026 15:31:02 +0100 Subject: [PATCH 3/3] test: cover ShutdownSignal::name for both variants MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit name() renders the operator-facing signal label in shutdown logs but was entirely uncovered — it is only invoked inside warn!/info! field expressions, which tracing skips when tests run without a subscriber. A direct unit test covers both match arms, and offsets the coverage the deterministic-poll rewrite removed with the wall-clock assertions (the fail-on-any-decrease gate tripped on the net removal of covered lines, not on the logic change). Co-Authored-By: Claude Fable 5 --- src/shutdown.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/shutdown.rs b/src/shutdown.rs index 53f553e..328c190 100644 --- a/src/shutdown.rs +++ b/src/shutdown.rs @@ -392,6 +392,13 @@ mod tests { assert_eq!(signal, ShutdownSignal::Sigterm); } + #[test] + fn shutdown_signal_name_labels_each_variant() { + // These strings appear in the operator-facing shutdown logs. + assert_eq!(ShutdownSignal::CtrlC.name(), "Ctrl+C"); + assert_eq!(ShutdownSignal::Sigterm.name(), "SIGTERM"); + } + #[cfg(unix)] #[tokio::test] async fn wait_for_shutdown_signal_receives_sigterm() {