From 50e4633c7a9ac7d58277f1762495059defb83020 Mon Sep 17 00:00:00 2001 From: Nils Andreas Svee Date: Sat, 15 Aug 2026 01:30:46 +0700 Subject: [PATCH] refactor(cli): centralize managed target preparation Both client applets duplicated the step from parsed arguments to a runnable managed setup: target specs, managed target preparation, a ClientConfig built from an arbitrary primary target, pacing conversion, and an identical ManagedClientConfig skeleton with its own copy of the event-capacity constant. The primary-target step was the worst of it, since the managed driver replaces server_addr per target, so the address the applets went out of their way to find was immediately discarded. shared::client::prepare now performs that step once: it validates the target set first, then returns the validated targets, one target-free ClientConfig template, and the pacing, with managed_config()/managed_targets() for the driver. CommonClientArgs::to_client_config no longer takes a server address. Applet-specific semantics are unchanged: each command keeps its own duration default and help text, the client still accepts no target for --list-columns, and the TUI still rejects an empty target list at parse time. --- crates/irtt-cli/src/cmd/client/args.rs | 68 +++-- crates/irtt-cli/src/cmd/client/run.rs | 33 +-- crates/irtt-cli/src/cmd/tui/args.rs | 70 +++-- crates/irtt-cli/src/cmd/tui/run.rs | 30 +-- crates/irtt-cli/src/cmd/tui/ui.rs | 6 +- crates/irtt-cli/src/shared/client/config.rs | 8 +- crates/irtt-cli/src/shared/client/mod.rs | 2 + crates/irtt-cli/src/shared/client/prepare.rs | 258 +++++++++++++++++++ 8 files changed, 347 insertions(+), 128 deletions(-) create mode 100644 crates/irtt-cli/src/shared/client/prepare.rs diff --git a/crates/irtt-cli/src/cmd/client/args.rs b/crates/irtt-cli/src/cmd/client/args.rs index 48d39e4..2b698f6 100644 --- a/crates/irtt-cli/src/cmd/client/args.rs +++ b/crates/irtt-cli/src/cmd/client/args.rs @@ -1,12 +1,10 @@ use std::time::Duration; -use clap::{Parser, ValueEnum}; -use irtt_client::ClientConfig; - use crate::shared::client::{ - parse_labelled_target, parse_test_duration, prepare_managed_targets, target_specs, - CommonClientArgs, GroupPacingArg, LabelledTargetArg, PreparedTarget, TargetSpec, TimestampArg, + parse_labelled_target, parse_test_duration, prepare_managed_run, CommonClientArgs, + GroupPacingArg, LabelledTargetArg, ManagedRunSetup, TargetSelection, TimestampArg, }; +use clap::{Parser, ValueEnum}; pub const DEFAULT_CLIENT_DURATION: Duration = Duration::from_secs(10); @@ -91,11 +89,19 @@ pub struct ClientArgs { } impl ClientArgs { - pub fn to_client_config(&self) -> ClientConfig { - self.common.to_client_config( - self.primary_target_addr() - .expect("target is required unless --list-columns is set"), + /// Validate the selected targets and prepare the managed run. + /// + /// Targets are only required here. `--list-columns` returns before this is + /// called, so listing columns still needs no target. + pub fn prepare(&self) -> Result { + prepare_managed_run( + &self.common, self.duration, + TargetSelection { + positional: &self.targets, + labelled: &self.labelled_targets, + pacing: self.pacing, + }, ) } @@ -106,22 +112,6 @@ impl ClientArgs { pub fn timestamp_mode(&self) -> TimestampArg { self.common.tstamp } - - pub fn target_specs(&self) -> Result, String> { - target_specs(&self.targets, &self.labelled_targets) - } - - pub fn managed_targets(&self) -> Result, String> { - prepare_managed_targets(self.target_specs()?) - } - - fn primary_target_addr(&self) -> Option<&str> { - self.targets.first().map(String::as_str).or_else(|| { - self.labelled_targets - .first() - .map(|target| target.addr.as_str()) - }) - } } impl std::ops::Deref for ClientArgs { @@ -144,9 +134,6 @@ pub enum OutputFormat { Jsonl, } -pub type CliTargetSpec = TargetSpec; -pub type CliManagedTarget = PreparedTarget; - impl OutputFormat { pub fn prints_summary(self) -> bool { matches!(self, Self::Table) @@ -191,7 +178,7 @@ mod tests { assert!(!args.is_continuous()); assert_eq!(args.timestamp_mode(), TimestampArg::Both); - let config = args.to_client_config(); + let config = args.prepare().unwrap().client; assert_eq!(config.duration, Some(DEFAULT_CLIENT_DURATION)); assert_eq!(config.received_stats, ReceivedStats::Both); assert_eq!(config.stamp_at, StampAt::Both); @@ -212,17 +199,22 @@ mod tests { let args = parse(&["--list-columns"]).unwrap(); assert!(args.targets.is_empty()); assert!(args.list_columns); + // The parser accepts no target; preparation is where a target becomes + // mandatory, and the stream command returns before preparing when + // listing columns. + let err = args.prepare().unwrap_err(); + assert!(err.contains("--list-columns")); } #[test] fn multiple_positional_targets_parse_as_targets() { let args = parse(&["host-a:2112", "host-b:2112"]).unwrap(); - let specs = args.target_specs().unwrap(); + let specs = args.prepare().unwrap().targets; assert_eq!(specs[0].label, "host-a:2112"); - assert_eq!(specs[0].addr, "host-a:2112"); + assert_eq!(specs[0].managed.server_addr, "host-a:2112"); assert_eq!(specs[1].label, "host-b:2112"); - assert_eq!(specs[1].addr, "host-b:2112"); + assert_eq!(specs[1].managed.server_addr, "host-b:2112"); } #[test] @@ -234,18 +226,18 @@ mod tests { "sg=sg.example.com:2112", ]) .unwrap(); - let specs = args.target_specs().unwrap(); + let specs = args.prepare().unwrap().targets; assert_eq!(specs[0].label, "ams"); - assert_eq!(specs[0].addr, "ams.example.com:2112"); + assert_eq!(specs[0].managed.server_addr, "ams.example.com:2112"); assert_eq!(specs[1].label, "sg"); - assert_eq!(specs[1].addr, "sg.example.com:2112"); + assert_eq!(specs[1].managed.server_addr, "sg.example.com:2112"); } #[test] fn duplicate_labels_are_rejected() { let args = parse(&["host-a:2112", "--target", "host-a:2112=host-b:2112"]).unwrap(); - let err = args.target_specs().unwrap_err(); + let err = args.prepare().unwrap_err(); assert!(err.contains("duplicate target label")); } @@ -253,7 +245,7 @@ mod tests { #[test] fn duplicate_positional_target_strings_get_stable_suffixes() { let args = parse(&["host-a:2112", "host-a:2112"]).unwrap(); - let specs = args.target_specs().unwrap(); + let specs = args.prepare().unwrap().targets; assert_eq!(specs[0].label, "host-a:2112"); assert_eq!(specs[1].label, "host-a:2112#2"); @@ -262,7 +254,7 @@ mod tests { #[test] fn duplicate_target_endpoints_are_allowed() { let args = parse(&["127.0.0.1:2112", "127.0.0.1"]).unwrap(); - assert_eq!(args.managed_targets().unwrap().len(), 2); + assert_eq!(args.prepare().unwrap().target_count(), 2); } #[test] diff --git a/crates/irtt-cli/src/cmd/client/run.rs b/crates/irtt-cli/src/cmd/client/run.rs index 3c78a8d..08754cb 100644 --- a/crates/irtt-cli/src/cmd/client/run.rs +++ b/crates/irtt-cli/src/cmd/client/run.rs @@ -9,9 +9,8 @@ use std::{ use irtt_client::{ managed::{ - BlockingManagedClient, ManagedClientConfig, ManagedCompletionPolicy, ManagedEndReason, - ManagedEvent, ManagedEventSubscription, ManagedEventTryRecvError, ManagedTargetEndReason, - ManagedTargetOutcome, TargetInstance, + BlockingManagedClient, ManagedEndReason, ManagedEvent, ManagedEventSubscription, + ManagedEventTryRecvError, ManagedTargetEndReason, ManagedTargetOutcome, TargetInstance, }, ClientEvent, }; @@ -44,7 +43,6 @@ const FINITE_STATS_MEMORY_STRONG_WARNING_BYTES: u64 = 512 * MIB; const FINITE_STATS_MEMORY_VERY_STRONG_WARNING_BYTES: u64 = GIB; const MANAGED_EVENT_WAIT_SLICE: Duration = Duration::from_millis(20); -const MANAGED_EVENT_CAPACITY: usize = 16_384; pub fn run_stream( args: ClientArgs, @@ -54,10 +52,10 @@ pub fn run_stream( print!("{}", OutputConfig::list_columns()); return Ok(()); } - let targets = args - .managed_targets() + let setup = args + .prepare() .map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err))?; - let multi_target = targets.len() > 1; + let multi_target = setup.is_multi_target(); let output_config = OutputConfig::new( args.format, args.columns.as_deref(), @@ -67,7 +65,7 @@ pub fn run_stream( ) .map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err))?; let continuous = args.is_continuous(); - let target_count = targets.len(); + let target_count = setup.target_count(); if let Some(warning) = finite_stats_memory_warning(&args, target_count) { eprintln!("{warning}"); @@ -75,21 +73,9 @@ pub fn run_stream( if is_shutdown_requested(shutdown_requested) { return Ok(()); } - let config = ManagedClientConfig { - client: args.to_client_config(), - pacing: args.pacing.into(), - completion: ManagedCompletionPolicy::FinishWhenQuiescent, - event_capacity: MANAGED_EVENT_CAPACITY, - outcome_history_limit: target_count, - max_live_target_generations: target_count, - ..ManagedClientConfig::default() - }; let (owner, mut events) = BlockingManagedClient::start_with_subscription( - config, - targets - .iter() - .map(|target| target.managed.clone()) - .collect(), + setup.managed_config(), + setup.managed_targets(), )?; let handle = owner.handle(); let mut stdout = io::LineWriter::new(io::stdout().lock()); @@ -101,7 +87,8 @@ pub fn run_stream( out: &mut stdout, }; - let mut stats = targets + let mut stats = setup + .targets .iter() .map(|target| { ( diff --git a/crates/irtt-cli/src/cmd/tui/args.rs b/crates/irtt-cli/src/cmd/tui/args.rs index 9104f7c..26df3e7 100644 --- a/crates/irtt-cli/src/cmd/tui/args.rs +++ b/crates/irtt-cli/src/cmd/tui/args.rs @@ -1,13 +1,12 @@ use std::time::Duration; use clap::Parser; -use irtt_client::ClientConfig; #[cfg(test)] use crate::shared::client::TimestampArg; use crate::shared::client::{ - parse_labelled_target, parse_test_duration, prepare_managed_targets, target_specs, - CommonClientArgs, GroupPacingArg, LabelledTargetArg, PreparedTarget, TargetSpec, + parse_labelled_target, parse_test_duration, prepare_managed_run, CommonClientArgs, + GroupPacingArg, LabelledTargetArg, ManagedRunSetup, TargetSelection, }; pub const DEFAULT_TUI_DURATION: Duration = Duration::ZERO; @@ -56,11 +55,19 @@ pub struct TuiArgs { } impl TuiArgs { - pub fn to_client_config(&self) -> ClientConfig { - self.common.to_client_config( - self.primary_target_addr() - .expect("at least one target is required"), + /// Validate the selected targets and prepare the managed run. + /// + /// The TUI always requires a target: the parser rejects an empty target + /// list, and this rejects a target set that cannot be labelled uniquely. + pub fn prepare(&self) -> Result { + prepare_managed_run( + &self.common, self.duration, + TargetSelection { + positional: &self.targets, + labelled: &self.labelled_targets, + pacing: self.pacing, + }, ) } @@ -68,31 +75,12 @@ impl TuiArgs { self.duration == Duration::ZERO } - pub fn target_specs(&self) -> Result, String> { - target_specs(&self.targets, &self.labelled_targets) - } - - pub fn managed_targets(&self) -> Result, String> { - prepare_managed_targets(self.target_specs()?) - } - #[cfg(test)] pub fn timestamp_mode(&self) -> TimestampArg { self.common.tstamp } - - fn primary_target_addr(&self) -> Option<&str> { - self.targets.first().map(String::as_str).or_else(|| { - self.labelled_targets - .first() - .map(|target| target.addr.as_str()) - }) - } } -pub type TuiTargetSpec = TargetSpec; -pub type TuiManagedTarget = PreparedTarget; - impl std::ops::Deref for TuiArgs { type Target = CommonClientArgs; @@ -132,12 +120,12 @@ mod tests { assert_eq!(args.pacing, GroupPacingArg::Staggered); assert_eq!(args.duration, DEFAULT_TUI_DURATION); assert!(args.is_continuous()); - assert_eq!(args.to_client_config().duration, None); + assert_eq!(args.prepare().unwrap().client.duration, None); let finite = parse(&["--duration", "30s", "127.0.0.1:2112"]).unwrap(); assert_eq!(finite.duration, Duration::from_secs(30)); assert_eq!( - finite.to_client_config().duration, + finite.prepare().unwrap().client.duration, Some(Duration::from_secs(30)) ); @@ -149,12 +137,12 @@ mod tests { #[test] fn multiple_positional_targets_parse() { let args = parse(&["host-a:2112", "host-b:2112"]).unwrap(); - let specs = args.target_specs().unwrap(); + let specs = args.prepare().unwrap().targets; assert_eq!(specs[0].label, "host-a:2112"); - assert_eq!(specs[0].addr, "host-a:2112"); + assert_eq!(specs[0].managed.server_addr, "host-a:2112"); assert_eq!(specs[1].label, "host-b:2112"); - assert_eq!(specs[1].addr, "host-b:2112"); + assert_eq!(specs[1].managed.server_addr, "host-b:2112"); } #[test] @@ -166,23 +154,26 @@ mod tests { "sg=sg.example.com:2112", ]) .unwrap(); - let specs = args.target_specs().unwrap(); + let specs = args.prepare().unwrap().targets; assert_eq!(specs[0].label, "ams"); - assert_eq!(specs[0].addr, "ams.example.com:2112"); + assert_eq!(specs[0].managed.server_addr, "ams.example.com:2112"); assert_eq!(specs[1].label, "sg"); - assert_eq!(specs[1].addr, "sg.example.com:2112"); + assert_eq!(specs[1].managed.server_addr, "sg.example.com:2112"); } #[test] fn at_least_one_target_is_required() { + // The TUI has no target-free mode, so this is rejected at parse time + // rather than during preparation. assert!(parse(&[]).is_err()); + assert!(parse(&["--target", "ams=ams.example.com:2112"]).is_ok()); } #[test] fn duplicate_labels_are_rejected() { let args = parse(&["host-a:2112", "--target", "host-a:2112=host-b:2112"]).unwrap(); - let err = args.target_specs().unwrap_err(); + let err = args.prepare().unwrap_err(); assert!(err.contains("duplicate target label")); } @@ -190,7 +181,7 @@ mod tests { #[test] fn duplicate_positional_target_strings_get_stable_suffixes() { let args = parse(&["host-a:2112", "host-a:2112"]).unwrap(); - let specs = args.target_specs().unwrap(); + let specs = args.prepare().unwrap().targets; assert_eq!(specs[0].label, "host-a:2112"); assert_eq!(specs[1].label, "host-a:2112#2"); @@ -199,7 +190,7 @@ mod tests { #[test] fn duplicate_target_endpoints_are_allowed() { let args = parse(&["127.0.0.1:2112", "127.0.0.1"]).unwrap(); - assert_eq!(args.managed_targets().unwrap().len(), 2); + assert_eq!(args.prepare().unwrap().target_count(), 2); } #[test] @@ -272,11 +263,10 @@ mod tests { "--loose", "127.0.0.1:2112", ]; - let tui = parse(&tui_args).unwrap().to_client_config(); + let tui = parse(&tui_args).unwrap().prepare().unwrap().client; let common = parse_shared(&shared).unwrap().common; - let shared = common.to_client_config("127.0.0.1:2112", Duration::from_secs(30)); + let shared = common.to_client_config(Duration::from_secs(30)); - assert_eq!(shared.server_addr, tui.server_addr); assert_eq!(shared.duration, tui.duration); assert_eq!(shared.interval, tui.interval); assert_eq!(shared.length, tui.length); diff --git a/crates/irtt-cli/src/cmd/tui/run.rs b/crates/irtt-cli/src/cmd/tui/run.rs index c229fd4..afca9d1 100644 --- a/crates/irtt-cli/src/cmd/tui/run.rs +++ b/crates/irtt-cli/src/cmd/tui/run.rs @@ -19,27 +19,26 @@ use crate::{ }; use crossterm::event::{self, Event, KeyCode, KeyEventKind, KeyModifiers}; use irtt_client::managed::{ - BlockingManagedClient, ManagedClientConfig, ManagedCompletionPolicy, ManagedEndReason, - ManagedEvent, ManagedEventSubscription, ManagedEventTryRecvError, TargetInstance, + BlockingManagedClient, ManagedEndReason, ManagedEvent, ManagedEventSubscription, + ManagedEventTryRecvError, TargetInstance, }; const RENDER_INTERVAL: Duration = Duration::from_millis(250); const TUI_WAIT_SLICE: Duration = Duration::from_millis(20); -const MANAGED_EVENT_CAPACITY: usize = 16_384; const INPUT_EVENT_WORK_BUDGET: usize = 128; pub fn run_tui( args: TuiArgs, shutdown_requested: &AtomicBool, ) -> Result<(), Box> { - let targets = args - .managed_targets() + let setup = args + .prepare() .map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err))?; let continuous = args.is_continuous(); let mut terminal = TuiTerminal::enter()?; let mut state = TuiState::with_target_labels( - TuiConfig::from_args(&args), - targets.iter().map(|target| target.label.clone()), + TuiConfig::from_args(&args, &setup), + setup.targets.iter().map(|target| target.label.clone()), ); let mut next_render = Instant::now(); if is_shutdown_requested(shutdown_requested) { @@ -47,22 +46,9 @@ pub fn run_tui( } state.set_status(TuiStatus::Opening); render_if_due(&mut terminal, &state, &mut next_render, true)?; - let target_count = targets.len(); - let config = ManagedClientConfig { - client: args.to_client_config(), - pacing: args.pacing.into(), - completion: ManagedCompletionPolicy::FinishWhenQuiescent, - event_capacity: MANAGED_EVENT_CAPACITY, - outcome_history_limit: target_count, - max_live_target_generations: target_count, - ..ManagedClientConfig::default() - }; let (owner, mut events) = match BlockingManagedClient::start_with_subscription( - config, - targets - .iter() - .map(|target| target.managed.clone()) - .collect(), + setup.managed_config(), + setup.managed_targets(), ) { Ok(value) => value, Err(error) => { diff --git a/crates/irtt-cli/src/cmd/tui/ui.rs b/crates/irtt-cli/src/cmd/tui/ui.rs index b2bee2d..6769e8f 100644 --- a/crates/irtt-cli/src/cmd/tui/ui.rs +++ b/crates/irtt-cli/src/cmd/tui/ui.rs @@ -30,7 +30,7 @@ use crate::{ }, tui::args::TuiArgs, }, - shared::client::{expected_probe_count, GroupPacingArg}, + shared::client::{expected_probe_count, GroupPacingArg, ManagedRunSetup}, }; const HISTORY_LIMIT: usize = 100_000; @@ -656,11 +656,11 @@ pub(super) struct TuiConfig { } impl TuiConfig { - pub(super) fn from_args(args: &TuiArgs) -> Self { + pub(super) fn from_args(args: &TuiArgs, setup: &ManagedRunSetup) -> Self { Self { interval: args.interval, duration: (!args.is_continuous()).then_some(args.duration), - timeout: args.to_client_config().probe_timeout, + timeout: setup.client.probe_timeout, target_probes: (!args.is_continuous()) .then(|| expected_probe_count(args.duration, args.interval)), pacing: args.pacing, diff --git a/crates/irtt-cli/src/shared/client/config.rs b/crates/irtt-cli/src/shared/client/config.rs index e1c5f7c..8246e06 100644 --- a/crates/irtt-cli/src/shared/client/config.rs +++ b/crates/irtt-cli/src/shared/client/config.rs @@ -7,9 +7,13 @@ use super::args::CommonClientArgs; pub const DEFAULT_RECV_TIMEOUT: Duration = Duration::from_millis(20); impl CommonClientArgs { - pub fn to_client_config(&self, server: &str, duration: Duration) -> ClientConfig { + /// Build the shared client configuration template for a managed run. + /// + /// The template carries no server address: the managed driver replaces it + /// per target from that target's configuration, so deriving one here from + /// an arbitrary target would be dead data. + pub fn to_client_config(&self, duration: Duration) -> ClientConfig { ClientConfig { - server_addr: server.to_owned(), duration: (!duration.is_zero()).then_some(duration), interval: self.interval, length: self.length, diff --git a/crates/irtt-cli/src/shared/client/mod.rs b/crates/irtt-cli/src/shared/client/mod.rs index 69f4ca2..b259c1d 100644 --- a/crates/irtt-cli/src/shared/client/mod.rs +++ b/crates/irtt-cli/src/shared/client/mod.rs @@ -1,5 +1,6 @@ pub mod args; pub mod config; +pub mod prepare; pub mod session; pub mod targets; @@ -8,6 +9,7 @@ pub use args::{ ClockArg, CommonClientArgs, ReceivedStatsArg, TimestampArg, }; pub use config::{expected_probe_count, DEFAULT_RECV_TIMEOUT}; +pub use prepare::{prepare_managed_run, ManagedRunSetup, TargetSelection, MANAGED_EVENT_CAPACITY}; pub use session::is_shutdown_requested; pub use targets::{ parse_labelled_target, prepare_managed_targets, target_specs, GroupPacingArg, diff --git a/crates/irtt-cli/src/shared/client/prepare.rs b/crates/irtt-cli/src/shared/client/prepare.rs new file mode 100644 index 0000000..4a5dc98 --- /dev/null +++ b/crates/irtt-cli/src/shared/client/prepare.rs @@ -0,0 +1,258 @@ +//! One preparation step from parsed arguments to a runnable managed setup. +//! +//! Both client applets accept the same target and pacing arguments and then +//! have to turn them into the same three things: a validated target set, one +//! shared [`ClientConfig`] template, and a [`ManagedPacing`]. Doing that +//! separately in each applet is how the CLI ended up deriving its client +//! configuration from an arbitrary "primary" target whose address the managed +//! driver immediately replaced per target. + +use std::time::Duration; + +use irtt_client::{ + managed::{ManagedClientConfig, ManagedCompletionPolicy, ManagedPacing}, + ClientConfig, +}; + +use super::{ + args::CommonClientArgs, + targets::{prepare_managed_targets, target_specs, GroupPacingArg, LabelledTargetArg}, + PreparedTarget, +}; + +/// Capacity of the lossy managed presentation event channel. +pub const MANAGED_EVENT_CAPACITY: usize = 16_384; + +/// Target and pacing arguments common to the client applets. +/// +/// Duration is deliberately absent: the applets disagree about what a duration +/// means and what its default is, so each keeps its own. +#[derive(Debug, Clone, Copy)] +pub struct TargetSelection<'a> { + pub positional: &'a [String], + pub labelled: &'a [LabelledTargetArg], + pub pacing: GroupPacingArg, +} + +/// Everything a managed run needs, prepared and validated together. +#[derive(Debug, Clone)] +pub struct ManagedRunSetup { + /// Validated targets, in the order the user gave them. + pub targets: Vec, + /// Shared client configuration template. + /// + /// The managed driver supplies each target's address and authentication + /// from its [`ManagedTargetConfig`](irtt_client::managed::ManagedTargetConfig), + /// so this template carries no target of its own. + pub client: ClientConfig, + /// Send coordination across active targets. + pub pacing: ManagedPacing, +} + +impl ManagedRunSetup { + /// Number of validated targets. + pub fn target_count(&self) -> usize { + self.targets.len() + } + + /// Whether this run drives more than one target. + pub fn is_multi_target(&self) -> bool { + self.targets.len() > 1 + } + + /// Managed target configurations, in argument order. + pub fn managed_targets(&self) -> Vec { + self.targets + .iter() + .map(|target| target.managed.clone()) + .collect() + } + + /// Managed driver configuration for this run. + pub fn managed_config(&self) -> ManagedClientConfig { + let target_count = self.target_count(); + ManagedClientConfig { + client: self.client.clone(), + pacing: self.pacing, + completion: ManagedCompletionPolicy::FinishWhenQuiescent, + event_capacity: MANAGED_EVENT_CAPACITY, + outcome_history_limit: target_count, + max_live_target_generations: target_count, + ..ManagedClientConfig::default() + } + } +} + +/// Validate the selected targets and build the shared run setup. +/// +/// Targets are validated before any configuration is built, so a bad target set +/// fails before the caller can act on a half-prepared run. +pub fn prepare_managed_run( + common: &CommonClientArgs, + duration: Duration, + selection: TargetSelection<'_>, +) -> Result { + let specs = target_specs(selection.positional, selection.labelled)?; + let targets = prepare_managed_targets(specs)?; + Ok(ManagedRunSetup { + targets, + client: common.to_client_config(duration), + pacing: selection.pacing.into(), + }) +} + +#[cfg(test)] +mod tests { + use clap::Parser; + + use super::*; + + #[derive(Parser)] + struct CommonOnlyArgs { + #[command(flatten)] + common: CommonClientArgs, + } + + fn common(args: &[&str]) -> CommonClientArgs { + let mut argv = vec!["common-only"]; + argv.extend_from_slice(args); + CommonOnlyArgs::try_parse_from(argv).unwrap().common + } + + fn prepare( + positional: &[&str], + labelled: &[LabelledTargetArg], + pacing: GroupPacingArg, + duration: Duration, + ) -> Result { + let positional: Vec = positional.iter().map(|value| (*value).to_owned()).collect(); + prepare_managed_run( + &common(&[]), + duration, + TargetSelection { + positional: &positional, + labelled, + pacing, + }, + ) + } + + #[test] + fn a_single_target_prepares_one_managed_target_and_a_finite_duration() { + let setup = prepare( + &["127.0.0.1:2112"], + &[], + GroupPacingArg::Staggered, + Duration::from_secs(30), + ) + .unwrap(); + + assert_eq!(setup.target_count(), 1); + assert!(!setup.is_multi_target()); + assert_eq!(setup.targets[0].label, "127.0.0.1:2112"); + assert_eq!(setup.targets[0].managed.server_addr, "127.0.0.1:2112"); + assert_eq!(setup.client.duration, Some(Duration::from_secs(30))); + assert_eq!(setup.pacing, ManagedPacing::Staggered); + } + + #[test] + fn a_zero_duration_prepares_a_continuous_client_config() { + let setup = prepare( + &["127.0.0.1:2112"], + &[], + GroupPacingArg::Staggered, + Duration::ZERO, + ) + .unwrap(); + + assert_eq!(setup.client.duration, None); + } + + #[test] + fn multiple_targets_keep_argument_order_and_share_one_client_config() { + let labelled = vec![LabelledTargetArg { + label: "sg".to_owned(), + addr: "sg.example.test:2112".to_owned(), + }]; + let setup = prepare( + &["host-a:2112", "host-b:2112"], + &labelled, + GroupPacingArg::Burst, + Duration::from_secs(10), + ) + .unwrap(); + + assert!(setup.is_multi_target()); + let labels: Vec<_> = setup + .targets + .iter() + .map(|target| target.label.as_str()) + .collect(); + assert_eq!(labels, ["host-a:2112", "host-b:2112", "sg"]); + assert_eq!(setup.targets[2].managed.server_addr, "sg.example.test:2112"); + assert_eq!(setup.pacing, ManagedPacing::Burst); + + let config = setup.managed_config(); + assert_eq!(config.outcome_history_limit, 3); + assert_eq!(config.max_live_target_generations, 3); + assert_eq!(config.event_capacity, MANAGED_EVENT_CAPACITY); + assert_eq!(config.pacing, ManagedPacing::Burst); + assert_eq!(config.client, setup.client); + assert_eq!(setup.managed_targets().len(), 3); + } + + #[test] + fn the_shared_client_config_carries_no_target_of_its_own() { + // The managed driver replaces the address, and the CLI's key, per + // target, so the template must not pin either to one target. + let positional = vec!["host-a:2112".to_owned(), "host-b:2112".to_owned()]; + let setup = prepare_managed_run( + &common(&["--hmac", "secret"]), + Duration::from_secs(10), + TargetSelection { + positional: &positional, + labelled: &[], + pacing: GroupPacingArg::Staggered, + }, + ) + .unwrap(); + + assert_eq!( + setup.client.server_addr, + ClientConfig::default().server_addr + ); + assert_eq!(setup.client.hmac_key, Some(b"secret".to_vec())); + // Per-target authentication is not exposed on the command line, so no + // target carries an override of the shared key. + assert!(setup + .targets + .iter() + .all(|target| target.managed.auth.is_none())); + } + + #[test] + fn an_empty_target_set_is_rejected_with_the_list_columns_hint() { + let err = + prepare(&[], &[], GroupPacingArg::Staggered, Duration::from_secs(10)).unwrap_err(); + + assert!(err.contains("at least one target is required")); + assert!(err.contains("--list-columns")); + } + + #[test] + fn a_duplicate_label_is_rejected_before_any_configuration_is_built() { + let labelled = vec![LabelledTargetArg { + label: "host-a:2112".to_owned(), + addr: "host-b:2112".to_owned(), + }]; + let err = prepare( + &["host-a:2112"], + &labelled, + GroupPacingArg::Staggered, + Duration::from_secs(10), + ) + .unwrap_err(); + + assert!(err.contains("duplicate target label")); + } +}