From 846150659adc25a08373874acb1bc65609aa7380 Mon Sep 17 00:00:00 2001 From: Mollusk Date: Tue, 2 Jun 2026 05:54:13 -0400 Subject: [PATCH] refactor(app): extract reconnect-chime edge triggers + unit tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pull the PeerConnecting/PeerConnected chime logic out of update()'s match arms into two pure functions — reconnect_attempt_chime and reconnected_chime — that own the connecting/ever_connected set transitions and return the Option to play, leaving the notify::play side effect in update. Behavior-preserving; the full suite still passes. Adds 4 #[cfg(test)] tests pinning the edge-trigger contract: a first dial is silent, a reconnect attempt chimes exactly once and stays silent across the supervisor's repeated redials, "reconnected" only fires after a prior link, and a two-outage cycle chimes attempt->reconnected each time (per-outage, not once-ever). Closes the last catalogued Tier B test gap. Co-Authored-By: Claude Opus 4.8 --- src/app/mod.rs | 131 +++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 121 insertions(+), 10 deletions(-) diff --git a/src/app/mod.rs b/src/app/mod.rs index d9e582b..4982321 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -179,6 +179,38 @@ fn subscription(_state: &AppState) -> Subscription { Subscription::batch(vec![core_sub, event_sub]) } +/// Reconnect-chime edge trigger for `UiEvent::PeerConnecting`. Marks the peer as +/// connecting and returns `Some(Sound::ReconnectAttempt)` exactly once per outage: +/// only when the peer had a live link before (a genuine reconnect, not a first +/// dial) AND we weren't already in the connecting state (so the supervisor's +/// repeated redials while still down don't re-chime). Pure so the once-per- +/// disconnect behavior is unit-testable without a GUI or audio. +fn reconnect_attempt_chime( + connecting: &mut HashSet, + ever_connected: &HashSet, + id: EndpointId, +) -> Option { + let is_reconnect_attempt = ever_connected.contains(&id); + let was_already_connecting = connecting.contains(&id); + connecting.insert(id); + (is_reconnect_attempt && !was_already_connecting).then_some(Sound::ReconnectAttempt) +} + +/// Reconnect-chime edge trigger for `UiEvent::PeerConnected`. Clears the connecting +/// state, records that we've linked with this peer at least once, and returns +/// `Some(Sound::Reconnected)` only if it had connected before (a true reconnect, not +/// the first link). Pure so the logic is unit-testable. +fn reconnected_chime( + connecting: &mut HashSet, + ever_connected: &mut HashSet, + id: EndpointId, +) -> Option { + let was_reconnect = ever_connected.contains(&id); + connecting.remove(&id); + ever_connected.insert(id); + was_reconnect.then_some(Sound::Reconnected) +} + fn update(state: &mut AppState, message: AppMessage) -> Task { match message { AppMessage::NicknameChanged(val) => { @@ -267,19 +299,17 @@ fn update(state: &mut AppState, message: AppMessage) -> Task { state.peers.insert(id, peer_state); } UiEvent::PeerConnecting { id } => { - let is_reconnect_attempt = state.ever_connected.contains(&id); - let was_already_connecting = state.connecting.contains(&id); - state.connecting.insert(id); - if is_reconnect_attempt && !was_already_connecting { - notify::play(Sound::ReconnectAttempt, state.config.custom_sound_reconnect_attempt.as_deref()); + if let Some(sound) = + reconnect_attempt_chime(&mut state.connecting, &state.ever_connected, id) + { + notify::play(sound, state.config.custom_sound_reconnect_attempt.as_deref()); } } UiEvent::PeerConnected { id } => { - let was_reconnect = state.ever_connected.contains(&id); - state.connecting.remove(&id); - state.ever_connected.insert(id); - if was_reconnect { - notify::play(Sound::Reconnected, state.config.custom_sound_reconnected.as_deref()); + if let Some(sound) = + reconnected_chime(&mut state.connecting, &mut state.ever_connected, id) + { + notify::play(sound, state.config.custom_sound_reconnected.as_deref()); } } UiEvent::AudioLevels(levels) => { @@ -884,3 +914,84 @@ fn view(state: &AppState) -> Element<'_, AppMessage> { .into() } } + +#[cfg(test)] +mod tests { + use super::{reconnect_attempt_chime, reconnected_chime}; + use crate::notify::Sound; + use iroh::EndpointId; + use std::collections::HashSet; + + /// A distinct, real `EndpointId` (via the same path the network tests use). + fn id() -> EndpointId { + iroh::EndpointAddr::from(iroh::SecretKey::generate().public()).id + } + + #[test] + fn first_dial_does_not_chime() { + let mut connecting = HashSet::new(); + let ever = HashSet::new(); // never connected + let peer = id(); + // A peer we've never linked with is just an initial connect, not a reconnect. + assert_eq!(reconnect_attempt_chime(&mut connecting, &ever, peer), None); + assert!(connecting.contains(&peer)); // but it is now marked connecting + } + + #[test] + fn first_connected_does_not_chime() { + let mut connecting = HashSet::from([id()]); + let mut ever = HashSet::new(); + let peer = id(); + connecting.insert(peer); + // First successful link: record it, but no "reconnected" chime. + assert_eq!(reconnected_chime(&mut connecting, &mut ever, peer), None); + assert!(ever.contains(&peer)); + assert!(!connecting.contains(&peer)); // connecting state cleared + } + + #[test] + fn reconnect_attempt_chimes_once_then_stays_silent_on_redials() { + let peer = id(); + let mut connecting = HashSet::new(); + let ever = HashSet::from([peer]); // previously connected + + // First drop → one ReconnectAttempt chime. + assert_eq!( + reconnect_attempt_chime(&mut connecting, &ever, peer), + Some(Sound::ReconnectAttempt) + ); + // The supervisor redials repeatedly while still down — must NOT re-chime. + assert_eq!(reconnect_attempt_chime(&mut connecting, &ever, peer), None); + assert_eq!(reconnect_attempt_chime(&mut connecting, &ever, peer), None); + } + + #[test] + fn full_outage_cycle_chimes_attempt_then_reconnected_each_time() { + let peer = id(); + let mut connecting = HashSet::new(); + let mut ever = HashSet::new(); + + // Initial connect: silent, records the peer. + assert_eq!(reconnected_chime(&mut connecting, &mut ever, peer), None); + + // Outage 1: attempt chimes once, recovery chimes "reconnected". + assert_eq!( + reconnect_attempt_chime(&mut connecting, &ever, peer), + Some(Sound::ReconnectAttempt) + ); + assert_eq!( + reconnected_chime(&mut connecting, &mut ever, peer), + Some(Sound::Reconnected) + ); + + // Outage 2: a fresh disconnect chimes again (per-outage, not once-ever). + assert_eq!( + reconnect_attempt_chime(&mut connecting, &ever, peer), + Some(Sound::ReconnectAttempt) + ); + assert_eq!( + reconnected_chime(&mut connecting, &mut ever, peer), + Some(Sound::Reconnected) + ); + } +}