Refactor MarkdownExporter emitting using a generic MarkupFormatter trait

* see: https://github.com/sharkdp/hyperfine/pull/490#issuecomment-1076784074

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::MarkupFormatter;
-use crate::export::markup::markup_table;
-use crate::export::markup::markup_unit;
-use crate::export::markup::MarkupType;
 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<Unit>) -> Result<Vec<u8>> {
+        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 formatter = MarkdownFormatter::default();
-        let unit = markup_unit(results, unit);
+        let data_vvs = formatter.results(&entries.unwrap(), unit);
-        let data = markup_results(&kind, &entries.unwrap(), unit);
+        let data_vas: Vec<Vec<&str>> = data_vvs
-        let table = markup_table(&kind, &data);
+            .iter()
+            .map(|list| list.iter().map(AsRef::as_ref).collect::<Vec<&str>>())
+            .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 actual = String::from_utf8(exporter.serialize(&timing_results, None).unwrap()).unwrap();
-
+    let expect = format!(
-    let formatted_expected = 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 actual = String::from_utf8(exporter.serialize(&timing_results, None).unwrap()).unwrap();
-
+    let expect = format!(
-    let formatted_expected = 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 expect = format!(
-    let formatted_expected = 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 expect = format!(
-    let formatted_expected = 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);
 }

src/export/markup.rs

@@ -1,63 +1,51 @@
-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 trait MarkupFormatter {
-pub enum MarkupType {
+    fn table(&self, data: &[&[&str]]) -> String {
-    /// Markdown table
+        let head: &[&str] = data.first().unwrap();
-    Markdown,
+        let tail = &data[1..];
-}
-
-pub fn markup_table(kind: &MarkupType, data: &[Vec<String>]) -> String {
-    let head: &Vec<String> = data.first().unwrap();
-    let tail: &[Vec<String>] = &data[1..];
 
         // emit header
-    let mut table = markup_table_data(&kind, head);
+        let table = &mut self.table_data(head);
 
         // emit horizontal line
-    table.push_str(&markup_table_line(&kind, head.len()));
+        table.push_str(&self.table_line(head.len()));
 
         // emit data rows
         for row in tail {
-        table.push_str(&markup_table_data(&kind, row))
+            table.push_str(&self.table_data(row))
         }
 
-    return table;
+        table.to_string()
-}
+    }
 
-fn markup_table_data(kind: &MarkupType, data: &Vec<String>) -> String {
+    fn table_data(&self, data: &[&str]) -> String;
-    return match kind {
-        MarkupType::Markdown => format!("| {} |\n", data.join(" | ")),
-    };
-}
 
-fn markup_table_line(kind: &MarkupType, size: usize) -> String {
+    fn table_line(&self, size: usize) -> String;
-    return match kind {
-        MarkupType::Markdown => format!("|:---|{}\n", "---:|".repeat(size - 1)),
-    };
-}
 
-pub fn markup_results(
+    fn command(&self, size: &str) -> String;
-    kind: &MarkupType,
+
+    fn results(
+        &self,
         entries: &[BenchmarkResultWithRelativeSpeed],
         unit: Unit,
-) -> Vec<Vec<String>> {
+    ) -> Vec<Vec<String>> {
         // prepare table header strings
         let notation = format!("[{}]", unit.short_name());
         let mut data: Vec<Vec<_>> = vec![vec![
-        format!("Command"),
+            "Command".to_string(),
             format!("Mean {}", notation),
             format!("Min {}", notation),
             format!("Max {}", notation),
-        format!("Relative"),
+            "Relative".to_string(),
         ]];
 
         for entry in entries {
             let measurement = &entry.result;
             // prepare data row strings
-        let cmd_str = measurement.command.replace("|", "\\|");
+            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)
@@ -76,224 +64,14 @@ pub fn markup_results(
             };
             // prepare table row entries
             data.push(vec![
-            match kind {
+                self.command(&cmd_str),
-                MarkupType::Markdown => format!("`{}`", cmd_str),
-            },
                 format!("{}{}", mean_str, stddev_str),
-            format!("{}", min_str),
+                min_str,
-            format!("{}", max_str),
+                max_str,
                 format!("{}{}", rel_str, rel_stddev_str),
             ])
         }
 
-    return data;
+        data
-}
+    }
-
-pub fn markup_unit(results: &[BenchmarkResult], unit: Option<Unit>) -> 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<BenchmarkResult> = vec![];
-    let unit = None;
-
-    let markup_actual = markup_unit(&results, unit);
-    let markup_expected = Unit::Second;
-
-    assert_eq!(markup_expected, markup_actual);
 }

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<Unit>) -> Result<Vec<u8>>;
+
+    fn unit(&self, results: &[BenchmarkResult], unit: Option<Unit>) -> 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<Unit>) -> Result<Vec<u8>> {
+        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<BenchmarkResult> = 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);
+}