From 9dc87ac3498b7f4035be828a6b2f586833dab193 Mon Sep 17 00:00:00 2001 From: Philipp Paulweber Date: Thu, 24 Mar 2022 07:38:18 +0100 Subject: [PATCH] Refactor MarkdownExporter emitting using a generic MarkupFormatter trait * see: https://github.com/sharkdp/hyperfine/pull/490#issuecomment-1076784074 --- src/export/markdown.rs | 97 ++++++++---- src/export/markup.rs | 344 ++++++++--------------------------------- src/export/mod.rs | 206 ++++++++++++++++++++++++ 3 files changed, 335 insertions(+), 312 deletions(-) diff --git a/src/export/markdown.rs b/src/export/markdown.rs index e7f98c5..f91a5b2 100644 --- a/src/export/markdown.rs +++ b/src/export/markdown.rs @@ -1,19 +1,34 @@ use super::Exporter; use crate::benchmark::benchmark_result::BenchmarkResult; use crate::benchmark::relative_speed; -use crate::export::markup::markup_results; -use crate::export::markup::markup_table; -use crate::export::markup::markup_unit; -use crate::export::markup::MarkupType; +use crate::export::markup::MarkupFormatter; use crate::util::units::Unit; use anyhow::{anyhow, Result}; +#[derive(Default)] +pub struct MarkdownFormatter; + +impl MarkupFormatter for MarkdownFormatter { + fn table_data(&self, data: &[&str]) -> String { + format!("| {} |\n", data.join(" | ")) + } + + fn table_line(&self, size: usize) -> String { + format!("|:---|{}\n", "---:|".repeat(size - 1)) + } + + fn command(&self, cmd: &str) -> String { + format!("`{}`", cmd) + } +} + #[derive(Default)] pub struct MarkdownExporter {} impl Exporter for MarkdownExporter { fn serialize(&self, results: &[BenchmarkResult], unit: Option) -> Result> { + let unit = self.unit(results, unit); let entries = relative_speed::compute(results); if entries.is_none() { return Err(anyhow!( @@ -21,18 +36,46 @@ impl Exporter for MarkdownExporter { )); } - let kind = MarkupType::Markdown; - let unit = markup_unit(results, unit); - let data = markup_results(&kind, &entries.unwrap(), unit); - let table = markup_table(&kind, &data); + let formatter = MarkdownFormatter::default(); + let data_vvs = formatter.results(&entries.unwrap(), unit); + let data_vas: Vec> = data_vvs + .iter() + .map(|list| list.iter().map(AsRef::as_ref).collect::>()) + .collect(); + let data: Vec<&[&str]> = data_vas.iter().map(AsRef::as_ref).collect(); + let table = formatter.table(&data); Ok(table.as_bytes().to_vec()) } } +/// Check Markdown-based data row formatting +#[test] +fn test_markdown_formatter_table_data() { + let formatter = MarkdownFormatter::default(); + let data = vec!["a", "b", "c"]; + + let actual = formatter.table_data(&data); + let expect = "| a | b | c |\n"; + + assert_eq!(expect, actual); +} + +/// Check Markdown-based horizontal line formatting +#[test] +fn test_markdown_formatter_table_line() { + let formatter = MarkdownFormatter::default(); + let size = 5; + + let actual = formatter.table_line(size); + let expect = "|:---|---:|---:|---:|---:|\n"; + + assert_eq!(expect, actual); +} + /// Test helper function to create unit-based header and horizontal line /// independently from the markup functionality for Markdown. #[cfg(test)] -fn test_table_header(unit_short_name: String) -> String { +fn cfg_test_table_header(unit_short_name: String) -> String { format!( "| Command | Mean [{unit}] | Min [{unit}] | Max [{unit}] | Relative |\n|:---|---:|---:|---:|---:|\n", unit = unit_short_name @@ -79,17 +122,16 @@ fn test_markdown_format_ms() { }, ]; - let formatted = String::from_utf8(exporter.serialize(&timing_results, None).unwrap()).unwrap(); - - let formatted_expected = format!( + let actual = String::from_utf8(exporter.serialize(&timing_results, None).unwrap()).unwrap(); + let expect = format!( "{}\ | `sleep 0.1` | 105.7 ± 1.6 | 102.3 | 108.0 | 1.00 | | `sleep 2` | 2005.0 ± 2.0 | 2002.0 | 2008.0 | 18.97 ± 0.29 | ", - test_table_header("ms".to_string()) + cfg_test_table_header("ms".to_string()) ); - assert_eq!(formatted_expected, formatted); + assert_eq!(expect, actual); } /// This (again) demonstrates that the first entry's units (s) are used to set @@ -128,17 +170,16 @@ fn test_markdown_format_s() { }, ]; - let formatted = String::from_utf8(exporter.serialize(&timing_results, None).unwrap()).unwrap(); - - let formatted_expected = format!( + let actual = String::from_utf8(exporter.serialize(&timing_results, None).unwrap()).unwrap(); + let expect = format!( "{}\ | `sleep 2` | 2.005 ± 0.002 | 2.002 | 2.008 | 18.97 ± 0.29 | | `sleep 0.1` | 0.106 ± 0.002 | 0.102 | 0.108 | 1.00 | ", - test_table_header("s".to_string()) + cfg_test_table_header("s".to_string()) ); - assert_eq!(formatted_expected, formatted); + assert_eq!(expect, actual); } /// The given time unit (s) is used to set the units for all entries. @@ -176,22 +217,21 @@ fn test_markdown_format_time_unit_s() { }, ]; - let formatted = String::from_utf8( + let actual = String::from_utf8( exporter .serialize(&timing_results, Some(Unit::Second)) .unwrap(), ) .unwrap(); - - let formatted_expected = format!( + let expect = format!( "{}\ | `sleep 0.1` | 0.106 ± 0.002 | 0.102 | 0.108 | 1.00 | | `sleep 2` | 2.005 ± 0.002 | 2.002 | 2.008 | 18.97 ± 0.29 | ", - test_table_header("s".to_string()) + cfg_test_table_header("s".to_string()) ); - assert_eq!(formatted_expected, formatted); + assert_eq!(expect, actual); } /// This (again) demonstrates that the given time unit (ms) is used to set @@ -230,20 +270,19 @@ fn test_markdown_format_time_unit_ms() { }, ]; - let formatted = String::from_utf8( + let actual = String::from_utf8( exporter .serialize(&timing_results, Some(Unit::MilliSecond)) .unwrap(), ) .unwrap(); - - let formatted_expected = format!( + let expect = format!( "{}\ | `sleep 2` | 2005.0 ± 2.0 | 2002.0 | 2008.0 | 18.97 ± 0.29 | | `sleep 0.1` | 105.7 ± 1.6 | 102.3 | 108.0 | 1.00 | ", - test_table_header("ms".to_string()) + cfg_test_table_header("ms".to_string()) ); - assert_eq!(formatted_expected, formatted); + assert_eq!(expect, actual); } diff --git a/src/export/markup.rs b/src/export/markup.rs index ab9bfce..9005eca 100644 --- a/src/export/markup.rs +++ b/src/export/markup.rs @@ -1,299 +1,77 @@ -use crate::benchmark::benchmark_result::BenchmarkResult; use crate::benchmark::relative_speed::BenchmarkResultWithRelativeSpeed; use crate::output::format::format_duration_value; use crate::util::units::Unit; -#[derive(Clone)] -pub enum MarkupType { - /// Markdown table - Markdown, -} +pub trait MarkupFormatter { + fn table(&self, data: &[&[&str]]) -> String { + let head: &[&str] = data.first().unwrap(); + let tail = &data[1..]; -pub fn markup_table(kind: &MarkupType, data: &[Vec]) -> String { - let head: &Vec = data.first().unwrap(); - let tail: &[Vec] = &data[1..]; + // emit header + let table = &mut self.table_data(head); - // emit header - let mut table = markup_table_data(&kind, head); + // emit horizontal line + table.push_str(&self.table_line(head.len())); - // emit horizontal line - table.push_str(&markup_table_line(&kind, head.len())); + // emit data rows + for row in tail { + table.push_str(&self.table_data(row)) + } - // emit data rows - for row in tail { - table.push_str(&markup_table_data(&kind, row)) + table.to_string() } - return table; -} + fn table_data(&self, data: &[&str]) -> String; -fn markup_table_data(kind: &MarkupType, data: &Vec) -> String { - return match kind { - MarkupType::Markdown => format!("| {} |\n", data.join(" | ")), - }; -} + fn table_line(&self, size: usize) -> String; -fn markup_table_line(kind: &MarkupType, size: usize) -> String { - return match kind { - MarkupType::Markdown => format!("|:---|{}\n", "---:|".repeat(size - 1)), - }; -} + fn command(&self, size: &str) -> String; -pub fn markup_results( - kind: &MarkupType, - entries: &[BenchmarkResultWithRelativeSpeed], - unit: Unit, -) -> Vec> { - // prepare table header strings - let notation = format!("[{}]", unit.short_name()); - let mut data: Vec> = vec![vec![ - format!("Command"), - format!("Mean {}", notation), - format!("Min {}", notation), - format!("Max {}", notation), - format!("Relative"), - ]]; + fn results( + &self, + entries: &[BenchmarkResultWithRelativeSpeed], + unit: Unit, + ) -> Vec> { + // prepare table header strings + let notation = format!("[{}]", unit.short_name()); + let mut data: Vec> = vec![vec![ + "Command".to_string(), + format!("Mean {}", notation), + format!("Min {}", notation), + format!("Max {}", notation), + "Relative".to_string(), + ]]; - for entry in entries { - let measurement = &entry.result; - // prepare data row strings - let cmd_str = measurement.command.replace("|", "\\|"); - let mean_str = format_duration_value(measurement.mean, Some(unit)).0; - let stddev_str = if let Some(stddev) = measurement.stddev { - format!(" ± {}", format_duration_value(stddev, Some(unit)).0) - } else { - "".into() - }; - let min_str = format_duration_value(measurement.min, Some(unit)).0; - let max_str = format_duration_value(measurement.max, Some(unit)).0; - let rel_str = format!("{:.2}", entry.relative_speed); - let rel_stddev_str = if entry.is_fastest { - "".into() - } else if let Some(stddev) = entry.relative_speed_stddev { - format!(" ± {:.2}", stddev) - } else { - "".into() - }; - // prepare table row entries - data.push(vec![ - match kind { - MarkupType::Markdown => format!("`{}`", cmd_str), - }, - format!("{}{}", mean_str, stddev_str), - format!("{}", min_str), - format!("{}", max_str), - format!("{}{}", rel_str, rel_stddev_str), - ]) + for entry in entries { + let measurement = &entry.result; + // prepare data row strings + let cmd_str = measurement.command.replace('|', "\\|"); + let mean_str = format_duration_value(measurement.mean, Some(unit)).0; + let stddev_str = if let Some(stddev) = measurement.stddev { + format!(" ± {}", format_duration_value(stddev, Some(unit)).0) + } else { + "".into() + }; + let min_str = format_duration_value(measurement.min, Some(unit)).0; + let max_str = format_duration_value(measurement.max, Some(unit)).0; + let rel_str = format!("{:.2}", entry.relative_speed); + let rel_stddev_str = if entry.is_fastest { + "".into() + } else if let Some(stddev) = entry.relative_speed_stddev { + format!(" ± {:.2}", stddev) + } else { + "".into() + }; + // prepare table row entries + data.push(vec![ + self.command(&cmd_str), + format!("{}{}", mean_str, stddev_str), + min_str, + max_str, + format!("{}{}", rel_str, rel_stddev_str), + ]) + } + + data } - - return data; -} - -pub fn markup_unit(results: &[BenchmarkResult], unit: Option) -> Unit { - return if let Some(unit) = unit { - // Use the given unit for all entries. - unit - } else if let Some(first_result) = results.first() { - // Use the first BenchmarkResult entry to determine the unit for all entries. - format_duration_value(first_result.mean, None).1 - } else { - // Default to `Second`. - Unit::Second - }; -} - -/// Check Markdown-based data row formatting -#[test] -fn test_markup_table_data_markdown() { - let kind = MarkupType::Markdown; - let data: Vec<_> = vec!["a", "b", "c"].into_iter().map(String::from).collect(); - - let markup_actual = markup_table_data(&kind, &data); - let markup_expected = format!("| a | b | c |\n"); - - assert_eq!(markup_expected, markup_actual); -} - -/// Check Markdown-based horizontal line formatting -#[test] -fn test_markup_table_line_markdown() { - let kind = MarkupType::Markdown; - let size = 5; - - let markup_actual = markup_table_line(&kind, size); - let markup_expected = format!("|:---|---:|---:|---:|---:|\n"); - - assert_eq!(markup_expected, markup_actual); -} - -/// Check unit resolving for timing results and given unit 's' -#[test] -fn test_markup_table_unit_given_s() { - use std::collections::BTreeMap; - let results = vec![ - BenchmarkResult { - command: String::from("sleep 2"), - mean: 2.0050, - stddev: Some(0.0020), - median: 2.0050, - user: 0.0009, - system: 0.0012, - min: 2.0020, - max: 2.0080, - times: Some(vec![2.0, 2.0, 2.0]), - exit_codes: vec![Some(0), Some(0), Some(0)], - parameters: BTreeMap::new(), - }, - BenchmarkResult { - command: String::from("sleep 0.1"), - mean: 0.1057, - stddev: Some(0.0016), - median: 0.1057, - user: 0.0009, - system: 0.0011, - min: 0.1023, - max: 0.1080, - times: Some(vec![0.1, 0.1, 0.1]), - exit_codes: vec![Some(0), Some(0), Some(0)], - parameters: BTreeMap::new(), - }, - ]; - let unit = Some(Unit::Second); - - let markup_actual = markup_unit(&results, unit); - let markup_expected = Unit::Second; - - assert_eq!(markup_expected, markup_actual); -} - -/// Check unit resolving for timing results and given unit 'ms' -#[test] -fn test_markup_table_unit_given_ms() { - use std::collections::BTreeMap; - let results = vec![ - BenchmarkResult { - command: String::from("sleep 2"), - mean: 2.0050, - stddev: Some(0.0020), - median: 2.0050, - user: 0.0009, - system: 0.0012, - min: 2.0020, - max: 2.0080, - times: Some(vec![2.0, 2.0, 2.0]), - exit_codes: vec![Some(0), Some(0), Some(0)], - parameters: BTreeMap::new(), - }, - BenchmarkResult { - command: String::from("sleep 0.1"), - mean: 0.1057, - stddev: Some(0.0016), - median: 0.1057, - user: 0.0009, - system: 0.0011, - min: 0.1023, - max: 0.1080, - times: Some(vec![0.1, 0.1, 0.1]), - exit_codes: vec![Some(0), Some(0), Some(0)], - parameters: BTreeMap::new(), - }, - ]; - let unit = Some(Unit::MilliSecond); - - let markup_actual = markup_unit(&results, unit); - let markup_expected = Unit::MilliSecond; - - assert_eq!(markup_expected, markup_actual); -} - -/// Check unit resolving for timing results using the first result entry as 's' -#[test] -fn test_markup_table_unit_first_s() { - use std::collections::BTreeMap; - let results = vec![ - BenchmarkResult { - command: String::from("sleep 2"), - mean: 2.0050, - stddev: Some(0.0020), - median: 2.0050, - user: 0.0009, - system: 0.0012, - min: 2.0020, - max: 2.0080, - times: Some(vec![2.0, 2.0, 2.0]), - exit_codes: vec![Some(0), Some(0), Some(0)], - parameters: BTreeMap::new(), - }, - BenchmarkResult { - command: String::from("sleep 0.1"), - mean: 0.1057, - stddev: Some(0.0016), - median: 0.1057, - user: 0.0009, - system: 0.0011, - min: 0.1023, - max: 0.1080, - times: Some(vec![0.1, 0.1, 0.1]), - exit_codes: vec![Some(0), Some(0), Some(0)], - parameters: BTreeMap::new(), - }, - ]; - let unit = None; - - let markup_actual = markup_unit(&results, unit); - let markup_expected = Unit::Second; - - assert_eq!(markup_expected, markup_actual); -} - -/// Check unit resolving for timing results using the first result entry as 'ms' -#[test] -fn test_markup_table_unit_first_ms() { - use std::collections::BTreeMap; - let results = vec![ - BenchmarkResult { - command: String::from("sleep 0.1"), - mean: 0.1057, - stddev: Some(0.0016), - median: 0.1057, - user: 0.0009, - system: 0.0011, - min: 0.1023, - max: 0.1080, - times: Some(vec![0.1, 0.1, 0.1]), - exit_codes: vec![Some(0), Some(0), Some(0)], - parameters: BTreeMap::new(), - }, - BenchmarkResult { - command: String::from("sleep 2"), - mean: 2.0050, - stddev: Some(0.0020), - median: 2.0050, - user: 0.0009, - system: 0.0012, - min: 2.0020, - max: 2.0080, - times: Some(vec![2.0, 2.0, 2.0]), - exit_codes: vec![Some(0), Some(0), Some(0)], - parameters: BTreeMap::new(), - }, - ]; - let unit = None; - - let markup_actual = markup_unit(&results, unit); - let markup_expected = Unit::MilliSecond; - - assert_eq!(markup_expected, markup_actual); -} - -/// Check unit resolving for not timing results and no given unit defaulting to 's' -#[test] -fn test_markup_table_unit_default_s() { - let results: Vec = vec![]; - let unit = None; - - let markup_actual = markup_unit(&results, unit); - let markup_expected = Unit::Second; - - assert_eq!(markup_expected, markup_actual); } diff --git a/src/export/mod.rs b/src/export/mod.rs index bdd5d3c..8adb440 100644 --- a/src/export/mod.rs +++ b/src/export/mod.rs @@ -13,6 +13,7 @@ use self::json::JsonExporter; use self::markdown::MarkdownExporter; use crate::benchmark::benchmark_result::BenchmarkResult; +use crate::output::format::format_duration_value; use crate::util::units::Unit; use anyhow::{Context, Result}; @@ -38,6 +39,19 @@ pub enum ExportType { trait Exporter { /// Export the given entries in the serialized form. fn serialize(&self, results: &[BenchmarkResult], unit: Option) -> Result>; + + fn unit(&self, results: &[BenchmarkResult], unit: Option) -> Unit { + return if let Some(unit) = unit { + // Use the given unit for all entries. + unit + } else if let Some(first_result) = results.first() { + // Use the first BenchmarkResult entry to determine the unit for all entries. + format_duration_value(first_result.mean, None).1 + } else { + // Default to `Second`. + Unit::Second + }; + } } struct ExporterWithFilename { @@ -106,3 +120,195 @@ fn write_to_file(filename: &str, content: &[u8]) -> Result<()> { file.write_all(content) .with_context(|| format!("Failed to export results to '{}'", filename)) } + +#[cfg(test)] +#[derive(Default)] +struct TestExporter; + +#[cfg(test)] +impl Exporter for TestExporter { + fn serialize(&self, _results: &[BenchmarkResult], _unit: Option) -> Result> { + assert_eq!( + "", + "This 'Exporter' trait implementation shall only be used to test the 'unit' function!" + ); + Ok(vec![]) + } +} + +/// Check unit resolving for timing results and given unit 's' +#[test] +fn test_markup_table_unit_given_s() { + use std::collections::BTreeMap; + let results = vec![ + BenchmarkResult { + command: String::from("sleep 2"), + mean: 2.0050, + stddev: Some(0.0020), + median: 2.0050, + user: 0.0009, + system: 0.0012, + min: 2.0020, + max: 2.0080, + times: Some(vec![2.0, 2.0, 2.0]), + exit_codes: vec![Some(0), Some(0), Some(0)], + parameters: BTreeMap::new(), + }, + BenchmarkResult { + command: String::from("sleep 0.1"), + mean: 0.1057, + stddev: Some(0.0016), + median: 0.1057, + user: 0.0009, + system: 0.0011, + min: 0.1023, + max: 0.1080, + times: Some(vec![0.1, 0.1, 0.1]), + exit_codes: vec![Some(0), Some(0), Some(0)], + parameters: BTreeMap::new(), + }, + ]; + let unit = Some(Unit::Second); + + let exporter = TestExporter::default(); + let markup_actual = exporter.unit(&results, unit); + let markup_expected = Unit::Second; + + assert_eq!(markup_expected, markup_actual); +} + +/// Check unit resolving for timing results and given unit 'ms' +#[test] +fn test_markup_table_unit_given_ms() { + use std::collections::BTreeMap; + let results = vec![ + BenchmarkResult { + command: String::from("sleep 2"), + mean: 2.0050, + stddev: Some(0.0020), + median: 2.0050, + user: 0.0009, + system: 0.0012, + min: 2.0020, + max: 2.0080, + times: Some(vec![2.0, 2.0, 2.0]), + exit_codes: vec![Some(0), Some(0), Some(0)], + parameters: BTreeMap::new(), + }, + BenchmarkResult { + command: String::from("sleep 0.1"), + mean: 0.1057, + stddev: Some(0.0016), + median: 0.1057, + user: 0.0009, + system: 0.0011, + min: 0.1023, + max: 0.1080, + times: Some(vec![0.1, 0.1, 0.1]), + exit_codes: vec![Some(0), Some(0), Some(0)], + parameters: BTreeMap::new(), + }, + ]; + let unit = Some(Unit::MilliSecond); + + let exporter = TestExporter::default(); + let markup_actual = exporter.unit(&results, unit); + let markup_expected = Unit::MilliSecond; + + assert_eq!(markup_expected, markup_actual); +} + +/// Check unit resolving for timing results using the first result entry as 's' +#[test] +fn test_markup_table_unit_first_s() { + use std::collections::BTreeMap; + let results = vec![ + BenchmarkResult { + command: String::from("sleep 2"), + mean: 2.0050, + stddev: Some(0.0020), + median: 2.0050, + user: 0.0009, + system: 0.0012, + min: 2.0020, + max: 2.0080, + times: Some(vec![2.0, 2.0, 2.0]), + exit_codes: vec![Some(0), Some(0), Some(0)], + parameters: BTreeMap::new(), + }, + BenchmarkResult { + command: String::from("sleep 0.1"), + mean: 0.1057, + stddev: Some(0.0016), + median: 0.1057, + user: 0.0009, + system: 0.0011, + min: 0.1023, + max: 0.1080, + times: Some(vec![0.1, 0.1, 0.1]), + exit_codes: vec![Some(0), Some(0), Some(0)], + parameters: BTreeMap::new(), + }, + ]; + let unit = None; + + let exporter = TestExporter::default(); + let markup_actual = exporter.unit(&results, unit); + let markup_expected = Unit::Second; + + assert_eq!(markup_expected, markup_actual); +} + +/// Check unit resolving for timing results using the first result entry as 'ms' +#[test] +fn test_markup_table_unit_first_ms() { + use std::collections::BTreeMap; + let results = vec![ + BenchmarkResult { + command: String::from("sleep 0.1"), + mean: 0.1057, + stddev: Some(0.0016), + median: 0.1057, + user: 0.0009, + system: 0.0011, + min: 0.1023, + max: 0.1080, + times: Some(vec![0.1, 0.1, 0.1]), + exit_codes: vec![Some(0), Some(0), Some(0)], + parameters: BTreeMap::new(), + }, + BenchmarkResult { + command: String::from("sleep 2"), + mean: 2.0050, + stddev: Some(0.0020), + median: 2.0050, + user: 0.0009, + system: 0.0012, + min: 2.0020, + max: 2.0080, + times: Some(vec![2.0, 2.0, 2.0]), + exit_codes: vec![Some(0), Some(0), Some(0)], + parameters: BTreeMap::new(), + }, + ]; + let unit = None; + + let exporter = TestExporter::default(); + let markup_actual = exporter.unit(&results, unit); + let markup_expected = Unit::MilliSecond; + + assert_eq!(markup_expected, markup_actual); +} + +/// Check unit resolving for not timing results and no given unit defaulting to 's' +#[test] +fn test_markup_table_unit_default_s() { + let results: Vec = vec![]; + let unit = None; + + let exporter = TestExporter::default(); + let markup_actual = exporter.unit(&results, unit); + let markup_expected = Unit::Second; + + assert_eq!(markup_expected, markup_actual); +}