fix(tui): surface incomplete statistics and strengthen review coverage
This commit is contained in:
4 files changed
+232
-15
No files matched your search
@@ -87,12 +87,18 @@ pub fn run_tui(
|
||||
) {
|
||||
drop(handle.stop());
|
||||
}
|
||||
let previous_dropped = dropped_events;
|
||||
let drain_state = drain_tui_events(
|
||||
&mut events,
|
||||
&mut state,
|
||||
&mut terminal_targets,
|
||||
&mut dropped_events,
|
||||
);
|
||||
if dropped_events != previous_dropped {
|
||||
// Data loss changes the meaning of displayed statistics, even while
|
||||
// ordinary measurement refreshes are paused.
|
||||
render_if_due(&mut terminal, &state, &mut next_render, true)?;
|
||||
}
|
||||
match drain_state {
|
||||
ManagedDrainState::Empty => {
|
||||
thread::sleep(managed_tui_wait_duration(&next_render, state.paused));
|
||||
@@ -178,11 +184,16 @@ fn drain_tui_events(
|
||||
terminal_targets: &mut HashSet<TargetInstance>,
|
||||
dropped_events: &mut u64,
|
||||
) -> ManagedDrainState {
|
||||
drain_managed_events(events, dropped_events, |event| {
|
||||
let previous_dropped = *dropped_events;
|
||||
let drained = drain_managed_events(events, dropped_events, |event| {
|
||||
process_tui_event(event, state, terminal_targets);
|
||||
Ok::<(), std::convert::Infallible>(())
|
||||
})
|
||||
.expect("processing TUI managed events is infallible")
|
||||
.expect("processing TUI managed events is infallible");
|
||||
if *dropped_events != previous_dropped {
|
||||
state.mark_dropped_managed_events(*dropped_events);
|
||||
}
|
||||
drained
|
||||
}
|
||||
|
||||
fn drain_final_tui_events(
|
||||
@@ -301,3 +312,45 @@ fn managed_tui_wait_duration(next_render: &Instant, paused: bool) -> Duration {
|
||||
};
|
||||
render_wait.min(TUI_WAIT_SLICE)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use ratatui::{backend::TestBackend, Terminal};
|
||||
|
||||
#[test]
|
||||
fn subscription_lag_keeps_incomplete_statistics_visible_while_paused() {
|
||||
for paused in [false, true] {
|
||||
// The production subscription is a broadcast receiver. Overflow the
|
||||
// real channel rather than synthesizing a lag result or UI state.
|
||||
let (sender, mut events) = tokio::sync::broadcast::channel(1);
|
||||
sender.send(ManagedEvent::Started).unwrap();
|
||||
sender.send(ManagedEvent::Started).unwrap();
|
||||
let mut state = TuiState::default();
|
||||
if paused {
|
||||
state.toggle_pause();
|
||||
}
|
||||
let mut dropped = 0;
|
||||
let mut terminal_targets = HashSet::new();
|
||||
drain_tui_events(&mut events, &mut state, &mut terminal_targets, &mut dropped);
|
||||
assert_eq!(dropped, 1);
|
||||
let mut terminal = Terminal::new(TestBackend::new(200, 40)).unwrap();
|
||||
for _ in 0..2 {
|
||||
// Empty drains must not clear the warning on later redraws.
|
||||
drain_tui_events(&mut events, &mut state, &mut terminal_targets, &mut dropped);
|
||||
terminal
|
||||
.draw(|frame| super::super::ui::draw_dashboard(frame, &state))
|
||||
.unwrap();
|
||||
let text = terminal
|
||||
.backend()
|
||||
.buffer()
|
||||
.content()
|
||||
.iter()
|
||||
.map(|cell| cell.symbol())
|
||||
.collect::<String>();
|
||||
assert!(text.contains("incomplete:dropped=1"));
|
||||
assert_eq!(text.contains("display paused"), paused);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -121,3 +121,138 @@ fn client_arguments_export_a_real_client_run() {
|
||||
.iter()
|
||||
.any(|row| row.contains("\"event\":\"session_closed\"")));
|
||||
}
|
||||
|
||||
#[cfg(any(feature = "client", feature = "tui"))]
|
||||
#[test]
|
||||
fn malformed_target_diagnostics_do_not_expose_authentication_keys() {
|
||||
let secret = "malformed-target-secret-92817";
|
||||
for applet in [
|
||||
#[cfg(feature = "client")]
|
||||
"client",
|
||||
#[cfg(feature = "tui")]
|
||||
"tui",
|
||||
] {
|
||||
let output = Command::new(env!("CARGO_BIN_EXE_irtt-rs"))
|
||||
.args([applet, &format!(r"edge=127.0.0.1:9@hmac={secret}\q")])
|
||||
.output()
|
||||
.unwrap();
|
||||
assert!(!output.status.success());
|
||||
let stderr = String::from_utf8(output.stderr).unwrap();
|
||||
assert!(
|
||||
!stderr.trim().is_empty(),
|
||||
"malformed target must report a diagnostic"
|
||||
);
|
||||
assert!(!stderr.contains(secret), "target authentication key leaked");
|
||||
assert!(!String::from_utf8_lossy(&output.stdout).contains(secret));
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(feature = "server")]
|
||||
#[test]
|
||||
fn server_arguments_resolve_binds_and_map_policy() {
|
||||
use clap::{CommandFactory, FromArgMatches, Parser};
|
||||
use irtt_app::cmd::server::ServerArgs;
|
||||
use irtt_server::TimestampAllowance;
|
||||
use std::time::Duration;
|
||||
// Isolate zero-argument defaults from the test runner's environment without
|
||||
// mutating process-global environment in this parallel test process.
|
||||
let defaults = ServerArgs::from_arg_matches(
|
||||
&ServerArgs::command()
|
||||
.mut_args(|arg| arg.env(None::<&str>))
|
||||
.try_get_matches_from(["irtt-server"])
|
||||
.unwrap(),
|
||||
)
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
defaults.resolve_binds(),
|
||||
vec![
|
||||
"[::]:2112".parse().unwrap(),
|
||||
"0.0.0.0:2112".parse().unwrap()
|
||||
]
|
||||
);
|
||||
assert_eq!(
|
||||
defaults.server_config(),
|
||||
irtt_server::ServerConfig::default()
|
||||
);
|
||||
let args = ServerArgs::try_parse_from([
|
||||
"irtt-server",
|
||||
"--bind",
|
||||
"127.0.0.1:2113",
|
||||
"--bind",
|
||||
"[::1]:2114",
|
||||
"--hmac",
|
||||
"server-cli-test",
|
||||
"--max-sessions",
|
||||
"3",
|
||||
"--max-packet-length",
|
||||
"96",
|
||||
"--min-interval",
|
||||
"25ms",
|
||||
"--burst",
|
||||
"2",
|
||||
"--idle-timeout",
|
||||
"8s",
|
||||
"--max-duration",
|
||||
"1m",
|
||||
"--timestamp-allowance",
|
||||
"single",
|
||||
"--no-dscp",
|
||||
])
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
args.resolve_binds(),
|
||||
vec![
|
||||
"127.0.0.1:2113".parse().unwrap(),
|
||||
"[::1]:2114".parse().unwrap()
|
||||
]
|
||||
);
|
||||
let config = args.server_config();
|
||||
assert_eq!(config.hmac_key(), Some(b"server-cli-test".as_slice()));
|
||||
assert_eq!(config.max_sessions(), 3);
|
||||
assert_eq!(config.max_packet_length(), 96);
|
||||
assert_eq!(config.min_send_interval(), Duration::from_millis(25));
|
||||
assert_eq!(config.burst_allowance(), 2);
|
||||
assert_eq!(config.idle_timeout(), Duration::from_secs(8));
|
||||
assert_eq!(config.max_test_duration(), Some(Duration::from_secs(60)));
|
||||
assert_eq!(config.timestamp_allowance(), TimestampAllowance::Single);
|
||||
assert!(!config.dscp_allowed());
|
||||
}
|
||||
|
||||
#[cfg(feature = "server")]
|
||||
#[test]
|
||||
fn server_command_line_binds_replace_the_environment_bind() {
|
||||
use std::net::UdpSocket;
|
||||
// Occupied ports give bounded, observable startup failures, without starting
|
||||
// a daemon or needing a process/signal harness merely to inspect arguments.
|
||||
let environment = UdpSocket::bind("127.0.0.1:0").unwrap();
|
||||
let explicit = UdpSocket::bind("127.0.0.1:0").unwrap();
|
||||
let environment_addr = environment.local_addr().unwrap().to_string();
|
||||
let explicit_addr = explicit.local_addr().unwrap().to_string();
|
||||
for use_explicit in [false, true] {
|
||||
let mut command = Command::new(env!("CARGO_BIN_EXE_irtt-server"));
|
||||
command
|
||||
.env_clear()
|
||||
.env("IRTT_SERVER_BIND", &environment_addr)
|
||||
.env("IRTT_SERVER_MAX_SESSIONS", "invalid")
|
||||
.args(["--max-sessions", "3"]);
|
||||
if use_explicit {
|
||||
command.args(["--bind", "127.0.0.1:0", "--bind", &explicit_addr]);
|
||||
}
|
||||
let output = command.output().unwrap();
|
||||
assert!(!output.status.success());
|
||||
let stderr = String::from_utf8(output.stderr).unwrap();
|
||||
let expected = if use_explicit {
|
||||
&explicit_addr
|
||||
} else {
|
||||
&environment_addr
|
||||
};
|
||||
assert!(
|
||||
stderr.contains(expected),
|
||||
"startup must attempt the selected bind: {stderr}"
|
||||
);
|
||||
assert!(
|
||||
!stderr.contains("listening on"),
|
||||
"partial startup must not announce listeners"
|
||||
);
|
||||
}
|
||||
}
|
||||
@@ -355,3 +355,12 @@ fn duration_close_waits_for_rate_allowance_after_the_deadline() {
|
||||
assert_eq!(closed.recv_window, Some(3));
|
||||
assert_eq!(server.session_count(), 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn default_server_resource_limits_remain_bounded() {
|
||||
let config = ServerConfig::default();
|
||||
// Upper resource budgets, not an exact default or a comparison against
|
||||
// the same constants used by the implementation. Smaller bounds are fine.
|
||||
assert!((1..=1024).contains(&config.max_sessions()));
|
||||
assert!((1..=65_507).contains(&config.max_packet_length()));
|
||||
}
|
||||
@@ -158,8 +158,8 @@ async fn wildcard_construction_is_refused_without_reply_source_selection() {
|
||||
|
||||
// A current-thread runtime cannot receive while this test holds its thread.
|
||||
// Queue real packets before delaying the server: no synthetic ancillary data or
|
||||
// transport hooks. A short queue delay selects kernel wall time; a delay beyond
|
||||
// the documented one-second sanity bound falls back to userspace. Monotonic and
|
||||
// transport hooks. Observed queue timing determines whether kernel wall time
|
||||
// is plausible under the one-second bound, including scheduler overshoot. Monotonic and
|
||||
// midpoint timestamps must always remain at the later userspace observation.
|
||||
#[cfg(target_os = "linux")]
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
@@ -174,10 +174,10 @@ async fn queued_echoes_select_kernel_receive_wall_only_when_plausible() {
|
||||
)
|
||||
.unwrap()
|
||||
};
|
||||
for (placement, delay, kernel_expected) in [
|
||||
(StampAt::Both, Duration::from_millis(100), true),
|
||||
(StampAt::Both, Duration::from_millis(1100), false),
|
||||
(StampAt::Midpoint, Duration::from_millis(100), false),
|
||||
for (placement, delay) in [
|
||||
(StampAt::Both, Duration::from_millis(100)),
|
||||
(StampAt::Both, Duration::from_millis(1100)),
|
||||
(StampAt::Midpoint, Duration::from_millis(100)),
|
||||
] {
|
||||
let mut server = Server::bind("127.0.0.1:0".parse().unwrap(), unthrottled())
|
||||
.await
|
||||
@@ -238,23 +238,43 @@ async fn queued_echoes_select_kernel_receive_wall_only_when_plausible() {
|
||||
assert_eq!(reply.recv_count, Some(1));
|
||||
assert_eq!(reply.recv_window, Some(1));
|
||||
let timestamps = reply.timestamps;
|
||||
if kernel_expected {
|
||||
if placement == StampAt::Both {
|
||||
let receive = timestamps.recv_wall.unwrap();
|
||||
let send = timestamps.send_wall.unwrap();
|
||||
let kernel_endpoint = (before_send..=queued_by).contains(&receive);
|
||||
let userspace_endpoint = (resumed_at..=send).contains(&receive);
|
||||
// Arrival lies in [before_send, queued_by], and core observation
|
||||
// lies in [resumed_at, send]. Only demand one selection where these
|
||||
// observed bounds put the whole lag interval on one side of 1 s.
|
||||
// Scheduler overshoot can legitimately change the expected path.
|
||||
if send - before_send <= 1_000_000_000 {
|
||||
assert!(
|
||||
receive >= before_send && receive <= queued_by,
|
||||
"receive wall must identify the kernel arrival before the queue delay"
|
||||
kernel_endpoint,
|
||||
"plausible arrival must select kernel wall time"
|
||||
);
|
||||
let wall_elapsed = timestamps.send_wall.unwrap() - receive;
|
||||
} else if resumed_at - queued_by > 1_000_000_000 {
|
||||
assert!(
|
||||
userspace_endpoint,
|
||||
"stale arrival must fall back to userspace"
|
||||
);
|
||||
} else {
|
||||
assert!(
|
||||
kernel_endpoint || userspace_endpoint,
|
||||
"boundary observation must identify a real endpoint"
|
||||
);
|
||||
}
|
||||
if kernel_endpoint {
|
||||
let wall_elapsed = send - receive;
|
||||
let mono_elapsed = timestamps.send_mono.unwrap() - timestamps.recv_mono.unwrap();
|
||||
assert!(
|
||||
wall_elapsed - mono_elapsed >= 50_000_000,
|
||||
"queue delay must not move the monotonic endpoint"
|
||||
);
|
||||
}
|
||||
} else {
|
||||
let receive = timestamps.recv_wall.or(timestamps.midpoint_wall).unwrap();
|
||||
assert!(
|
||||
receive >= resumed_at,
|
||||
"fallback and midpoint must use userspace time"
|
||||
timestamps.midpoint_wall.unwrap() >= resumed_at,
|
||||
"midpoint must use userspace time"
|
||||
);
|
||||
}
|
||||
stop_tx.send(()).unwrap();
|
||||
|
||||
Reference in new issue
Block a user