feat(tui): improve multi-target lifecycle management and UI clarity
Refine the TUI event loop to explicitly handle different exit conditions and ensure sessions stop correctly on idle timeouts or interruptions. Update UI labels to clarify when metrics or headers refer specifically to the first target in multi-target sessions.
This commit is contained in:
1 parent
836a8abbeb
commit
a62c53a77e
2 files changed
+188
-15
No files matched your search
@@ -169,7 +169,7 @@ fn run_group_tui(
|
|||||||
let mut saw_target_event = false;
|
let mut saw_target_event = false;
|
||||||
let mut last_event_at = Instant::now();
|
let mut last_event_at = Instant::now();
|
||||||
|
|
||||||
loop {
|
let exit = loop {
|
||||||
if is_shutdown_requested(shutdown_requested) {
|
if is_shutdown_requested(shutdown_requested) {
|
||||||
interrupted = true;
|
interrupted = true;
|
||||||
session.stop();
|
session.stop();
|
||||||
@@ -195,20 +195,26 @@ fn run_group_tui(
|
|||||||
}
|
}
|
||||||
Ok(None) => {
|
Ok(None) => {
|
||||||
if interrupted {
|
if interrupted {
|
||||||
break;
|
break GroupLoopExit::Interrupted;
|
||||||
}
|
}
|
||||||
if terminal_targets.len() >= expected_target_count {
|
if terminal_targets.len() >= expected_target_count {
|
||||||
break;
|
break GroupLoopExit::AllTargetsTerminal;
|
||||||
}
|
}
|
||||||
if should_join_group_after_idle(&args, saw_target_event, last_event_at) {
|
if should_join_group_after_idle(&args, saw_target_event, last_event_at) {
|
||||||
break;
|
break GroupLoopExit::IdleGraceElapsed;
|
||||||
}
|
}
|
||||||
wait_for_tui_activity(None, next_render, state, terminal, shutdown_requested)?;
|
wait_for_tui_activity(None, next_render, state, terminal, shutdown_requested)?;
|
||||||
thread::sleep(IDLE_SLEEP);
|
thread::sleep(IDLE_SLEEP);
|
||||||
}
|
}
|
||||||
Err(EventSubscriptionError::Disconnected) => break,
|
Err(EventSubscriptionError::Disconnected) => {
|
||||||
|
break GroupLoopExit::SubscriptionDisconnected
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
};
|
||||||
|
|
||||||
|
if exit.should_stop_before_join() {
|
||||||
|
session.stop();
|
||||||
|
}
|
||||||
|
|
||||||
if interrupted {
|
if interrupted {
|
||||||
state.set_status(TuiStatus::Interrupted);
|
state.set_status(TuiStatus::Interrupted);
|
||||||
@@ -224,7 +230,17 @@ fn run_group_tui(
|
|||||||
}
|
}
|
||||||
|
|
||||||
if outcome.end_reason == ManagedGroupEndReason::Cancelled && !interrupted {
|
if outcome.end_reason == ManagedGroupEndReason::Cancelled && !interrupted {
|
||||||
return Err("managed client group was cancelled".into());
|
return match exit {
|
||||||
|
GroupLoopExit::IdleGraceElapsed => {
|
||||||
|
Err("managed client group stayed idle before all targets completed".into())
|
||||||
|
}
|
||||||
|
GroupLoopExit::SubscriptionDisconnected => {
|
||||||
|
Err("managed client group event subscription disconnected before completion".into())
|
||||||
|
}
|
||||||
|
GroupLoopExit::Interrupted | GroupLoopExit::AllTargetsTerminal => {
|
||||||
|
Err("managed client group was cancelled".into())
|
||||||
|
}
|
||||||
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
state.set_status(TuiStatus::Complete);
|
state.set_status(TuiStatus::Complete);
|
||||||
@@ -232,6 +248,23 @@ fn run_group_tui(
|
|||||||
Ok(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||||
|
enum GroupLoopExit {
|
||||||
|
Interrupted,
|
||||||
|
AllTargetsTerminal,
|
||||||
|
IdleGraceElapsed,
|
||||||
|
SubscriptionDisconnected,
|
||||||
|
}
|
||||||
|
|
||||||
|
impl GroupLoopExit {
|
||||||
|
fn should_stop_before_join(self) -> bool {
|
||||||
|
matches!(
|
||||||
|
self,
|
||||||
|
Self::Interrupted | Self::IdleGraceElapsed | Self::SubscriptionDisconnected
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
fn is_terminal_target_event(event: &ClientEvent) -> bool {
|
fn is_terminal_target_event(event: &ClientEvent) -> bool {
|
||||||
matches!(
|
matches!(
|
||||||
event,
|
event,
|
||||||
@@ -348,3 +381,52 @@ fn tui_wait_duration(
|
|||||||
};
|
};
|
||||||
send_wait.min(render_wait).min(TUI_WAIT_SLICE)
|
send_wait.min(render_wait).min(TUI_WAIT_SLICE)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[cfg(test)]
|
||||||
|
mod tests {
|
||||||
|
use super::*;
|
||||||
|
use clap::Parser;
|
||||||
|
|
||||||
|
fn parse(args: &[&str]) -> TuiArgs {
|
||||||
|
let mut argv = vec!["irtt-tui"];
|
||||||
|
argv.extend_from_slice(args);
|
||||||
|
TuiArgs::try_parse_from(argv).unwrap()
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn continuous_multi_target_idle_does_not_stop_after_events() {
|
||||||
|
let args = parse(&["127.0.0.1:2112", "127.0.0.2:2112"]);
|
||||||
|
let old_event_at = Instant::now() - estimated_group_completion_grace(&args) - IDLE_SLEEP;
|
||||||
|
|
||||||
|
assert!(!should_join_group_after_idle(&args, true, old_event_at));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn finite_or_unopened_group_can_leave_after_idle_grace() {
|
||||||
|
let finite = parse(&["--duration", "1s", "127.0.0.1:2112", "127.0.0.2:2112"]);
|
||||||
|
let finite_old_event_at =
|
||||||
|
Instant::now() - estimated_group_completion_grace(&finite) - IDLE_SLEEP;
|
||||||
|
assert!(should_join_group_after_idle(
|
||||||
|
&finite,
|
||||||
|
true,
|
||||||
|
finite_old_event_at
|
||||||
|
));
|
||||||
|
|
||||||
|
let continuous = parse(&["127.0.0.1:2112", "127.0.0.2:2112"]);
|
||||||
|
let unopened_old_event_at =
|
||||||
|
Instant::now() - estimated_group_completion_grace(&continuous) - IDLE_SLEEP;
|
||||||
|
assert!(should_join_group_after_idle(
|
||||||
|
&continuous,
|
||||||
|
false,
|
||||||
|
unopened_old_event_at
|
||||||
|
));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn protective_group_exits_stop_before_joining() {
|
||||||
|
assert!(GroupLoopExit::Interrupted.should_stop_before_join());
|
||||||
|
assert!(GroupLoopExit::IdleGraceElapsed.should_stop_before_join());
|
||||||
|
assert!(GroupLoopExit::SubscriptionDisconnected.should_stop_before_join());
|
||||||
|
assert!(!GroupLoopExit::AllTargetsTerminal.should_stop_before_join());
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -642,7 +642,7 @@ fn draw_large(frame: &mut Frame<'_>, area: Rect, state: &TuiState, snapshot: &Sn
|
|||||||
|
|
||||||
frame.render_widget(header(state, HeaderDensity::Large), top[0]);
|
frame.render_widget(header(state, HeaderDensity::Large), top[0]);
|
||||||
frame.render_widget(packet_panel(state, snapshot, top[1].height), top[1]);
|
frame.render_widget(packet_panel(state, snapshot, top[1].height), top[1]);
|
||||||
frame.render_widget(timing_panel(snapshot), middle[0]);
|
frame.render_widget(timing_panel(state, snapshot), middle[0]);
|
||||||
render_graph_area(frame, middle[1], state);
|
render_graph_area(frame, middle[1], state);
|
||||||
frame.render_widget(recent_events_panel(state, bottom[0].height), bottom[0]);
|
frame.render_widget(recent_events_panel(state, bottom[0].height), bottom[0]);
|
||||||
frame.render_widget(sample_panel(state, snapshot), bottom[1]);
|
frame.render_widget(sample_panel(state, snapshot), bottom[1]);
|
||||||
@@ -683,6 +683,9 @@ enum HeaderDensity {
|
|||||||
|
|
||||||
fn header(state: &TuiState, density: HeaderDensity) -> Paragraph<'_> {
|
fn header(state: &TuiState, density: HeaderDensity) -> Paragraph<'_> {
|
||||||
let selected = state.selected_target();
|
let selected = state.selected_target();
|
||||||
|
let selected_label = selected
|
||||||
|
.map(|target| target.label.as_str())
|
||||||
|
.unwrap_or("target");
|
||||||
let remote = selected
|
let remote = selected
|
||||||
.and_then(|target| target.remote.as_deref())
|
.and_then(|target| target.remote.as_deref())
|
||||||
.unwrap_or("-");
|
.unwrap_or("-");
|
||||||
@@ -711,7 +714,11 @@ fn header(state: &TuiState, density: HeaderDensity) -> Paragraph<'_> {
|
|||||||
state.status.label()
|
state.status.label()
|
||||||
)),
|
)),
|
||||||
]),
|
]),
|
||||||
Line::from(format!("remote: {remote}")),
|
Line::from(if state.is_multi_target() {
|
||||||
|
format!("first target: {selected_label} remote: {remote}")
|
||||||
|
} else {
|
||||||
|
format!("remote: {remote}")
|
||||||
|
}),
|
||||||
];
|
];
|
||||||
match density {
|
match density {
|
||||||
HeaderDensity::Large => {
|
HeaderDensity::Large => {
|
||||||
@@ -745,8 +752,11 @@ fn header(state: &TuiState, density: HeaderDensity) -> Paragraph<'_> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
fn full_graph_header(state: &TuiState, snapshot: &Snapshot) -> Paragraph<'static> {
|
fn full_graph_header(state: &TuiState, snapshot: &Snapshot) -> Paragraph<'static> {
|
||||||
let remote = state
|
let selected = state.selected_target();
|
||||||
.selected_target()
|
let selected_label = selected
|
||||||
|
.map(|target| target.label.as_str())
|
||||||
|
.unwrap_or("target");
|
||||||
|
let remote = selected
|
||||||
.and_then(|target| target.remote.as_deref())
|
.and_then(|target| target.remote.as_deref())
|
||||||
.unwrap_or("-");
|
.unwrap_or("-");
|
||||||
let elapsed = format_duration(state.started_at.elapsed());
|
let elapsed = format_duration(state.started_at.elapsed());
|
||||||
@@ -757,13 +767,27 @@ fn full_graph_header(state: &TuiState, snapshot: &Snapshot) -> Paragraph<'static
|
|||||||
.map(|sample| format_ns_i128(Some(sample.effective_ns)))
|
.map(|sample| format_ns_i128(Some(sample.effective_ns)))
|
||||||
.unwrap_or_else(|| "-".to_owned());
|
.unwrap_or_else(|| "-".to_owned());
|
||||||
|
|
||||||
|
let target_context = if state.is_multi_target() {
|
||||||
|
format!("first target {selected_label} remote {remote}")
|
||||||
|
} else {
|
||||||
|
format!("remote {remote}")
|
||||||
|
};
|
||||||
|
|
||||||
Paragraph::new(Line::from(format!(
|
Paragraph::new(Line::from(format!(
|
||||||
"irtt-rs {} remote {remote} elapsed {elapsed} sent {} replies {} last {last}",
|
"irtt-rs {} {target_context} elapsed {elapsed} sent {} replies {} last {last}",
|
||||||
state.status.label(),
|
state.status.label(),
|
||||||
format_count(packets.packets_sent),
|
format_count(packets.packets_sent),
|
||||||
format_count(packets.unique_replies)
|
format_count(packets.unique_replies)
|
||||||
)))
|
)))
|
||||||
.block(Block::default().title("session").borders(Borders::ALL))
|
.block(
|
||||||
|
Block::default()
|
||||||
|
.title(if state.is_multi_target() {
|
||||||
|
"session - first target"
|
||||||
|
} else {
|
||||||
|
"session"
|
||||||
|
})
|
||||||
|
.borders(Borders::ALL),
|
||||||
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
fn packet_panel(state: &TuiState, snapshot: &Snapshot, panel_height: u16) -> Paragraph<'static> {
|
fn packet_panel(state: &TuiState, snapshot: &Snapshot, panel_height: u16) -> Paragraph<'static> {
|
||||||
@@ -852,7 +876,7 @@ fn target_table_panel(state: &TuiState, panel_height: u16) -> Paragraph<'static>
|
|||||||
.wrap(Wrap { trim: false })
|
.wrap(Wrap { trim: false })
|
||||||
}
|
}
|
||||||
|
|
||||||
fn timing_panel(snapshot: &Snapshot) -> Paragraph<'_> {
|
fn timing_panel(state: &TuiState, snapshot: &Snapshot) -> Paragraph<'static> {
|
||||||
let mut lines = vec![Line::from(format!(
|
let mut lines = vec![Line::from(format!(
|
||||||
"{:<18} {:>5} {:>9} {:>9} {:>9} {:>9}",
|
"{:<18} {:>5} {:>9} {:>9} {:>9} {:>9}",
|
||||||
"metric", "n", "min", "mean", "max", "stddev"
|
"metric", "n", "min", "mean", "max", "stddev"
|
||||||
@@ -878,7 +902,15 @@ fn timing_panel(snapshot: &Snapshot) -> Paragraph<'_> {
|
|||||||
push_time_line(&mut lines, "timer error", &snapshot.timer_error);
|
push_time_line(&mut lines, "timer error", &snapshot.timer_error);
|
||||||
|
|
||||||
Paragraph::new(lines)
|
Paragraph::new(lines)
|
||||||
.block(Block::default().title("timing").borders(Borders::ALL))
|
.block(
|
||||||
|
Block::default()
|
||||||
|
.title(if state.is_multi_target() {
|
||||||
|
"timing - first target"
|
||||||
|
} else {
|
||||||
|
"timing"
|
||||||
|
})
|
||||||
|
.borders(Borders::ALL),
|
||||||
|
)
|
||||||
.wrap(Wrap { trim: false })
|
.wrap(Wrap { trim: false })
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1291,7 +1323,15 @@ fn sample_panel(state: &TuiState, snapshot: &Snapshot) -> Paragraph<'static> {
|
|||||||
)),
|
)),
|
||||||
Line::from(format!("last warning: {warning}")),
|
Line::from(format!("last warning: {warning}")),
|
||||||
])
|
])
|
||||||
.block(Block::default().title("sample").borders(Borders::ALL))
|
.block(
|
||||||
|
Block::default()
|
||||||
|
.title(if state.is_multi_target() {
|
||||||
|
"sample - first target"
|
||||||
|
} else {
|
||||||
|
"sample"
|
||||||
|
})
|
||||||
|
.borders(Borders::ALL),
|
||||||
|
)
|
||||||
.wrap(Wrap { trim: true })
|
.wrap(Wrap { trim: true })
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -1771,6 +1811,57 @@ mod tests {
|
|||||||
.is_some_and(|warning| warning.contains("WrongToken")));
|
.is_some_and(|warning| warning.contains("WrongToken")));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn multi_target_global_status_waits_for_all_targets_terminal() {
|
||||||
|
let mut state =
|
||||||
|
TuiState::with_target_labels(TuiConfig::default(), ["a".to_owned(), "b".to_owned()]);
|
||||||
|
|
||||||
|
state.process_target_event(&target_event(
|
||||||
|
"a",
|
||||||
|
ClientEvent::NoTestCompleted {
|
||||||
|
remote: remote(),
|
||||||
|
negotiated: NegotiatedParams {
|
||||||
|
params: irtt_proto::Params::default(),
|
||||||
|
restrictions: Vec::new(),
|
||||||
|
},
|
||||||
|
at: ts(Duration::ZERO),
|
||||||
|
},
|
||||||
|
));
|
||||||
|
|
||||||
|
assert_eq!(state.targets[0].status, TargetStatus::NoTest);
|
||||||
|
assert_eq!(state.targets[1].status, TargetStatus::Opening);
|
||||||
|
assert_eq!(state.status, TuiStatus::Running);
|
||||||
|
|
||||||
|
state.process_target_event(&target_event(
|
||||||
|
"b",
|
||||||
|
ClientEvent::SessionClosed {
|
||||||
|
remote: remote(),
|
||||||
|
token: 0xabc,
|
||||||
|
at: ts(Duration::ZERO),
|
||||||
|
},
|
||||||
|
));
|
||||||
|
|
||||||
|
assert_eq!(state.targets[1].status, TargetStatus::Closed);
|
||||||
|
assert_eq!(state.status, TuiStatus::Complete);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn reset_clears_graph_history_for_all_targets() {
|
||||||
|
let mut state =
|
||||||
|
TuiState::with_target_labels(TuiConfig::default(), ["a".to_owned(), "b".to_owned()]);
|
||||||
|
|
||||||
|
state.process_target_event(&target_event("a", reply(1, 1_000_000)));
|
||||||
|
state.process_target_event(&target_event("b", reply(2, 2_000_000)));
|
||||||
|
state.clear_visible_history();
|
||||||
|
|
||||||
|
assert!(state.targets[0].graph_history.is_empty());
|
||||||
|
assert!(state.targets[1].graph_history.is_empty());
|
||||||
|
assert_eq!(
|
||||||
|
state.recent_events.back().map(String::as_str),
|
||||||
|
Some("visible graph history reset")
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn warning_updates_recent_events_and_last_warning() {
|
fn warning_updates_recent_events_and_last_warning() {
|
||||||
let mut state = TuiState::default();
|
let mut state = TuiState::default();
|
||||||
|
|||||||
Reference in new issue
Block a user