refactor(app): extract reconnect-chime edge triggers + unit tests
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<Sound> 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 <noreply@anthropic.com>
This commit is contained in:
+121
-10
@@ -179,6 +179,38 @@ fn subscription(_state: &AppState) -> Subscription<AppMessage> {
|
||||
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<EndpointId>,
|
||||
ever_connected: &HashSet<EndpointId>,
|
||||
id: EndpointId,
|
||||
) -> Option<Sound> {
|
||||
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<EndpointId>,
|
||||
ever_connected: &mut HashSet<EndpointId>,
|
||||
id: EndpointId,
|
||||
) -> Option<Sound> {
|
||||
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<AppMessage> {
|
||||
match message {
|
||||
AppMessage::NicknameChanged(val) => {
|
||||
@@ -267,19 +299,17 @@ fn update(state: &mut AppState, message: AppMessage) -> Task<AppMessage> {
|
||||
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)
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user