fix(cli): preserve target identity in default output #107
No files matched your search
@@ -27,15 +27,14 @@ impl OutputConfig {
|
||||
columns: Option<&str>,
|
||||
header: HeaderMode,
|
||||
verbose: bool,
|
||||
multi_target: bool,
|
||||
) -> Result<Self, String> {
|
||||
let uses_default_columns = columns
|
||||
.map(|columns| columns.trim() == "default")
|
||||
.unwrap_or(true);
|
||||
let default_table_rows = format == OutputFormat::Table && uses_default_columns;
|
||||
let columns = match columns {
|
||||
Some(columns) => parse_columns(columns, format, verbose, multi_target)?,
|
||||
None => default_columns(format, verbose, multi_target),
|
||||
Some(columns) => parse_columns(columns, format, verbose)?,
|
||||
None => default_columns(format, verbose),
|
||||
};
|
||||
|
||||
Ok(Self {
|
||||
@@ -723,12 +722,7 @@ impl CellValue {
|
||||
}
|
||||
}
|
||||
|
||||
fn parse_columns(
|
||||
input: &str,
|
||||
format: OutputFormat,
|
||||
verbose: bool,
|
||||
multi_target: bool,
|
||||
) -> Result<Vec<Column>, String> {
|
||||
fn parse_columns(input: &str, format: OutputFormat, verbose: bool) -> Result<Vec<Column>, String> {
|
||||
let trimmed = input.trim();
|
||||
if trimmed.is_empty() {
|
||||
return Err("--columns requires at least one column".to_owned());
|
||||
@@ -737,7 +731,7 @@ fn parse_columns(
|
||||
return Ok(ALL_COLUMNS.to_vec());
|
||||
}
|
||||
if trimmed == "default" {
|
||||
return Ok(default_columns(format, verbose, multi_target));
|
||||
return Ok(default_columns(format, verbose));
|
||||
}
|
||||
|
||||
let mut columns = Vec::new();
|
||||
@@ -754,10 +748,11 @@ fn parse_columns(
|
||||
Ok(columns)
|
||||
}
|
||||
|
||||
fn default_columns(format: OutputFormat, verbose: bool, multi_target: bool) -> Vec<Column> {
|
||||
fn default_columns(format: OutputFormat, verbose: bool) -> Vec<Column> {
|
||||
match format {
|
||||
OutputFormat::Table => {
|
||||
let mut columns = vec![
|
||||
Column::Target,
|
||||
Column::Event,
|
||||
Column::Seq,
|
||||
Column::Rtt,
|
||||
@@ -767,9 +762,6 @@ fn default_columns(format: OutputFormat, verbose: bool, multi_target: bool) -> V
|
||||
Column::ServerProcessing,
|
||||
Column::Message,
|
||||
];
|
||||
if multi_target {
|
||||
columns.insert(0, Column::Target);
|
||||
}
|
||||
if verbose {
|
||||
columns.extend([
|
||||
Column::Remote,
|
||||
@@ -784,17 +776,7 @@ fn default_columns(format: OutputFormat, verbose: bool, multi_target: bool) -> V
|
||||
}
|
||||
columns
|
||||
}
|
||||
OutputFormat::Csv | OutputFormat::Tsv | OutputFormat::Jsonl => {
|
||||
if multi_target {
|
||||
ALL_COLUMNS.to_vec()
|
||||
} else {
|
||||
ALL_COLUMNS
|
||||
.iter()
|
||||
.copied()
|
||||
.filter(|column| *column != Column::Target)
|
||||
.collect()
|
||||
}
|
||||
}
|
||||
OutputFormat::Csv | OutputFormat::Tsv | OutputFormat::Jsonl => ALL_COLUMNS.to_vec(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1209,12 +1191,8 @@ mod tests {
|
||||
time::Instant,
|
||||
};
|
||||
|
||||
fn output_config(
|
||||
format: OutputFormat,
|
||||
columns: Option<&str>,
|
||||
multi_target: bool,
|
||||
) -> OutputConfig {
|
||||
OutputConfig::new(format, columns, HeaderMode::Auto, false, multi_target).unwrap()
|
||||
fn output_config(format: OutputFormat, columns: Option<&str>) -> OutputConfig {
|
||||
OutputConfig::new(format, columns, HeaderMode::Auto, false).unwrap()
|
||||
}
|
||||
|
||||
fn remote() -> SocketAddr {
|
||||
@@ -1254,38 +1232,48 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn default_table_columns_include_target_only_for_multi_target() {
|
||||
let single = output_config(OutputFormat::Table, None, false)
|
||||
.render_header()
|
||||
.unwrap();
|
||||
let multi = output_config(OutputFormat::Table, None, true)
|
||||
fn default_table_columns_always_include_target_first() {
|
||||
let header = output_config(OutputFormat::Table, None)
|
||||
.render_header()
|
||||
.unwrap();
|
||||
|
||||
assert!(!single.split_whitespace().any(|column| column == "target"));
|
||||
assert_eq!(multi.split_whitespace().next(), Some("target"));
|
||||
assert_eq!(header.split_whitespace().next(), Some("target"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn default_structured_columns_include_target_for_multi_target() {
|
||||
let single = output_config(OutputFormat::Csv, None, false)
|
||||
.render_header()
|
||||
.unwrap();
|
||||
let multi = output_config(OutputFormat::Csv, None, true)
|
||||
.render_header()
|
||||
.unwrap();
|
||||
fn default_structured_columns_always_include_target_first() {
|
||||
for format in [OutputFormat::Csv, OutputFormat::Tsv, OutputFormat::Jsonl] {
|
||||
let columns = default_columns(format, false);
|
||||
assert_eq!(columns.first(), Some(&Column::Target));
|
||||
assert_eq!(columns, ALL_COLUMNS);
|
||||
}
|
||||
}
|
||||
|
||||
assert!(!single.split(',').any(|column| column == "target"));
|
||||
assert_eq!(multi.split(',').next(), Some("target"));
|
||||
#[test]
|
||||
fn columns_default_keyword_matches_omitted_columns() {
|
||||
for format in [
|
||||
OutputFormat::Table,
|
||||
OutputFormat::Csv,
|
||||
OutputFormat::Tsv,
|
||||
OutputFormat::Jsonl,
|
||||
] {
|
||||
let omitted = output_config(format, None).render_header();
|
||||
let explicit = output_config(format, Some("default")).render_header();
|
||||
assert_eq!(omitted, explicit);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn custom_columns_do_not_insert_target() {
|
||||
let config = output_config(OutputFormat::Csv, Some("seq,effective_rtt_us"));
|
||||
let header = config.render_header().unwrap();
|
||||
|
||||
assert!(!header.split(',').any(|column| column == "target"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn custom_target_column_renders_label() {
|
||||
let config = output_config(
|
||||
OutputFormat::Csv,
|
||||
Some("target,seq,effective_rtt_us"),
|
||||
false,
|
||||
);
|
||||
let config = output_config(OutputFormat::Csv, Some("target,seq,effective_rtt_us"));
|
||||
let line = config
|
||||
.render_event(&reply(), Some("ams"), Some(&EventRenderStats::default()))
|
||||
.unwrap();
|
||||
@@ -1296,4 +1284,24 @@ mod tests {
|
||||
);
|
||||
assert_eq!(line, "ams,7,1200");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn default_csv_renders_target_label_in_row() {
|
||||
let config = output_config(OutputFormat::Csv, None);
|
||||
let line = config
|
||||
.render_event(&reply(), Some("ams"), Some(&EventRenderStats::default()))
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(line.split(',').next(), Some("ams"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn default_jsonl_renders_target_key_first() {
|
||||
let config = output_config(OutputFormat::Jsonl, None);
|
||||
let line = config
|
||||
.render_event(&reply(), Some("ams"), Some(&EventRenderStats::default()))
|
||||
.unwrap();
|
||||
|
||||
assert!(line.starts_with("{\"target\":\"ams\""));
|
||||
}
|
||||
}
|
||||
@@ -59,7 +59,6 @@ pub fn run_stream(
|
||||
args.columns.as_deref(),
|
||||
args.header,
|
||||
args.verbose,
|
||||
multi_target,
|
||||
)
|
||||
.map_err(|err| io::Error::new(io::ErrorKind::InvalidInput, err))?;
|
||||
let continuous = args.is_continuous();
|
||||
|
||||
@@ -72,8 +72,9 @@ fn list_columns_succeeds_without_target() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn single_target_default_table_omits_target_and_accepts_pacing() {
|
||||
fn single_target_default_table_includes_target_and_accepts_pacing() {
|
||||
let server = InTreeServer::start();
|
||||
let target = server.addr.to_string();
|
||||
|
||||
let output = Command::new(env!("CARGO_BIN_EXE_irtt-cli"))
|
||||
.args([
|
||||
@@ -85,7 +86,7 @@ fn single_target_default_table_omits_target_and_accepts_pacing() {
|
||||
"burst",
|
||||
"--header",
|
||||
"always",
|
||||
&server.addr.to_string(),
|
||||
&target,
|
||||
])
|
||||
.output()
|
||||
.unwrap();
|
||||
@@ -99,8 +100,173 @@ fn single_target_default_table_omits_target_and_accepts_pacing() {
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
let stdout = String::from_utf8(output.stdout).unwrap();
|
||||
let header = stdout.lines().next().unwrap_or_default();
|
||||
assert!(!header.split_whitespace().any(|column| column == "target"));
|
||||
let mut lines = stdout.lines();
|
||||
let header = lines.next().unwrap_or_default();
|
||||
assert_eq!(header.split_whitespace().next(), Some("target"));
|
||||
assert!(
|
||||
lines.any(|line| line.split_whitespace().next() == Some(target.as_str())),
|
||||
"{stdout}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn single_labeled_target_default_table_includes_target() {
|
||||
let server = InTreeServer::start();
|
||||
|
||||
let output = Command::new(env!("CARGO_BIN_EXE_irtt-cli"))
|
||||
.args([
|
||||
"--duration",
|
||||
"30ms",
|
||||
"--interval",
|
||||
"10ms",
|
||||
"--header",
|
||||
"always",
|
||||
"--target",
|
||||
&format!("eu={}", server.addr),
|
||||
])
|
||||
.output()
|
||||
.unwrap();
|
||||
|
||||
drop(server);
|
||||
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"status={:?}\nstderr={}",
|
||||
output.status.code(),
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
let stdout = String::from_utf8(output.stdout).unwrap();
|
||||
let mut lines = stdout.lines();
|
||||
let header = lines.next().unwrap_or_default();
|
||||
assert_eq!(header.split_whitespace().next(), Some("target"));
|
||||
assert!(
|
||||
lines.any(|line| line.split_whitespace().next() == Some("eu")),
|
||||
"{stdout}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn single_labeled_target_default_csv_includes_target_and_event_wall_ns() {
|
||||
let server = InTreeServer::start();
|
||||
|
||||
let output = Command::new(env!("CARGO_BIN_EXE_irtt-cli"))
|
||||
.args([
|
||||
"--duration",
|
||||
"30ms",
|
||||
"--interval",
|
||||
"10ms",
|
||||
"--format",
|
||||
"csv",
|
||||
"--header",
|
||||
"always",
|
||||
"--target",
|
||||
&format!("eu={}", server.addr),
|
||||
])
|
||||
.output()
|
||||
.unwrap();
|
||||
|
||||
drop(server);
|
||||
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"status={:?}\nstderr={}",
|
||||
output.status.code(),
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
let stdout = String::from_utf8(output.stdout).unwrap();
|
||||
let mut lines = stdout.lines();
|
||||
let header = lines.next().unwrap_or_default();
|
||||
assert!(header.starts_with("target,event,"), "{header}");
|
||||
assert!(
|
||||
header.split(',').any(|column| column == "event_wall_ns"),
|
||||
"{header}"
|
||||
);
|
||||
assert!(lines.any(|line| line.starts_with("eu,")), "{stdout}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn single_positional_target_default_csv_includes_target_as_positional_label() {
|
||||
let server = InTreeServer::start();
|
||||
let target = server.addr.to_string();
|
||||
|
||||
let output = Command::new(env!("CARGO_BIN_EXE_irtt-cli"))
|
||||
.args([
|
||||
"--duration",
|
||||
"30ms",
|
||||
"--interval",
|
||||
"10ms",
|
||||
"--format",
|
||||
"csv",
|
||||
"--header",
|
||||
"always",
|
||||
&target,
|
||||
])
|
||||
.output()
|
||||
.unwrap();
|
||||
|
||||
drop(server);
|
||||
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"status={:?}\nstderr={}",
|
||||
output.status.code(),
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
let stdout = String::from_utf8(output.stdout).unwrap();
|
||||
let mut lines = stdout.lines();
|
||||
let header = lines.next().unwrap_or_default();
|
||||
assert!(header.starts_with("target,event,"), "{header}");
|
||||
assert!(
|
||||
lines.any(|line| line.starts_with(&format!("{target},"))),
|
||||
"{stdout}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn multi_target_default_table_includes_both_labels() {
|
||||
let a = InTreeServer::start();
|
||||
let b = InTreeServer::start();
|
||||
|
||||
let output = Command::new(env!("CARGO_BIN_EXE_irtt-cli"))
|
||||
.args([
|
||||
"--duration",
|
||||
"40ms",
|
||||
"--interval",
|
||||
"10ms",
|
||||
"--header",
|
||||
"always",
|
||||
"--target",
|
||||
&format!("a={}", a.addr),
|
||||
"--target",
|
||||
&format!("b={}", b.addr),
|
||||
])
|
||||
.output()
|
||||
.unwrap();
|
||||
|
||||
drop(a);
|
||||
drop(b);
|
||||
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"status={:?}\nstderr={}",
|
||||
output.status.code(),
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
let stdout = String::from_utf8(output.stdout).unwrap();
|
||||
let mut lines = stdout.lines();
|
||||
let header = lines.next().unwrap_or_default();
|
||||
assert_eq!(header.split_whitespace().next(), Some("target"));
|
||||
let rows: Vec<&str> = lines.collect();
|
||||
assert!(
|
||||
rows.iter()
|
||||
.any(|line| line.split_whitespace().next() == Some("a")),
|
||||
"{stdout}"
|
||||
);
|
||||
assert!(
|
||||
rows.iter()
|
||||
.any(|line| line.split_whitespace().next() == Some("b")),
|
||||
"{stdout}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -142,6 +308,112 @@ fn single_target_custom_target_column_renders_positional_label() {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn custom_columns_remain_authoritative_and_omit_target() {
|
||||
let server = InTreeServer::start();
|
||||
|
||||
let output = Command::new(env!("CARGO_BIN_EXE_irtt-cli"))
|
||||
.args([
|
||||
"--duration",
|
||||
"30ms",
|
||||
"--interval",
|
||||
"10ms",
|
||||
"--format",
|
||||
"csv",
|
||||
"--columns",
|
||||
"seq,rtt",
|
||||
"--header",
|
||||
"always",
|
||||
&server.addr.to_string(),
|
||||
])
|
||||
.output()
|
||||
.unwrap();
|
||||
|
||||
drop(server);
|
||||
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"status={:?}\nstderr={}",
|
||||
output.status.code(),
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
let stdout = String::from_utf8(output.stdout).unwrap();
|
||||
let header = stdout.lines().next().unwrap_or_default();
|
||||
assert_eq!(header, "seq,rtt");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn columns_default_keyword_includes_target_for_single_target() {
|
||||
let server = InTreeServer::start();
|
||||
|
||||
let output = Command::new(env!("CARGO_BIN_EXE_irtt-cli"))
|
||||
.args([
|
||||
"--duration",
|
||||
"30ms",
|
||||
"--interval",
|
||||
"10ms",
|
||||
"--format",
|
||||
"csv",
|
||||
"--columns",
|
||||
"default",
|
||||
"--header",
|
||||
"always",
|
||||
"--target",
|
||||
&format!("eu={}", server.addr),
|
||||
])
|
||||
.output()
|
||||
.unwrap();
|
||||
|
||||
drop(server);
|
||||
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"status={:?}\nstderr={}",
|
||||
output.status.code(),
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
let stdout = String::from_utf8(output.stdout).unwrap();
|
||||
let mut lines = stdout.lines();
|
||||
let header = lines.next().unwrap_or_default();
|
||||
assert!(header.starts_with("target,event,"), "{header}");
|
||||
assert!(lines.any(|line| line.starts_with("eu,")), "{stdout}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn single_labeled_target_default_jsonl_includes_target() {
|
||||
let server = InTreeServer::start();
|
||||
|
||||
let output = Command::new(env!("CARGO_BIN_EXE_irtt-cli"))
|
||||
.args([
|
||||
"--duration",
|
||||
"30ms",
|
||||
"--interval",
|
||||
"10ms",
|
||||
"--format",
|
||||
"jsonl",
|
||||
"--target",
|
||||
&format!("eu={}", server.addr),
|
||||
])
|
||||
.output()
|
||||
.unwrap();
|
||||
|
||||
drop(server);
|
||||
|
||||
assert!(
|
||||
output.status.success(),
|
||||
"status={:?}\nstderr={}",
|
||||
output.status.code(),
|
||||
String::from_utf8_lossy(&output.stderr)
|
||||
);
|
||||
let stdout = String::from_utf8(output.stdout).unwrap();
|
||||
assert!(
|
||||
stdout
|
||||
.lines()
|
||||
.any(|line| line.contains("\"target\":\"eu\"")),
|
||||
"{stdout}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn continuous_single_target_peer_close_exits_nonzero() {
|
||||
let server = start_peer_close_server(test_params(None, Duration::from_millis(10)));
|
||||
|
||||
Reference in new issue
Block a user