Merge pull request #38 from VolumeGraphics/json_reporting

Add Json reporting, decouple reporting html from comparison logic

Cargo.toml

@@ -4,7 +4,7 @@ description = "A flexible rule-based file and folder comparison tool and crate i
 repository = "https://github.com/VolumeGraphics/havocompare"
 homepage = "https://github.com/VolumeGraphics/havocompare"
 documentation = "https://docs.rs/havocompare"
-version = "0.3.2"
+version = "0.4.0"
 edition = "2021"
 license = "MIT"
 authors = ["Volume Graphics GmbH"]
@@ -18,14 +18,14 @@ path = "src/print_args.rs"
 
 
 [dependencies]
-clap = {version= "4.1", features=["derive"]}
+clap = {version= "4.3", features=["derive"]}
 chrono = "0.4"
 serde = "1.0"
 serde_yaml = "0.9"
 schemars = "0.8"
 schemars_derive = "0.8"
 thiserror = "1.0"
-regex = "1.6"
+regex = "1.8"
 image = "0.24"
 image-compare = "0.3.0"
 tracing = "0.1"
@@ -35,15 +35,15 @@ glob = "0.3"
 test-log = {version="0.2", features=["trace"]}
 strsim = "0.10"
 itertools = "0.11"
-tera = "1.17"
+tera = "1.19"
 sha2 = "0.10"
-data-encoding = "2.3"
+data-encoding = "2.4"
 permutation = "0.4"
-pdf-extract = "0.6.4"
+pdf-extract = "0.6"
 vg_errortools = "0.1"
-rayon = "1.6"
+rayon = "1.7.0"
 enable-ansi-support = "0.2"
-tempfile = "3.3"
+tempfile = "3.6"
 fs_extra = "1.3"
 opener = "0.6"
 anyhow = "1.0"

README.md

@@ -223,6 +223,10 @@ rules:
 
 ## Changelog
 
+### 0.4.0
+- Separate reporting logic from comparison logic
+- Implement a machine-readable JSON reporting
+
 ### 0.3.2
 - Allow direct opening of reports after comparison with `--open`
 - Parsing failures when running `compare` are now propagated to terminal

src/csv/mod.rs

@@ -32,9 +32,6 @@ pub enum Error {
     #[error("Failed to compile regex {0}")]
     /// Regex compilation failed
     RegexCompilationFailed(#[from] regex::Error),
-    #[error("Problem creating csv report {0}")]
-    /// Reporting could not be created
-    ReportingFailed(#[from] report::Error),
     #[error("File access failed {0}")]
     /// File access failed
     FileAccessFailed(#[from] FatIOError),
@@ -59,32 +56,52 @@ pub enum Error {
     UnequalRowCount(usize, usize),
 }
 
-#[derive(Clone, Copy, Debug)]
-pub(crate) struct Position {
+/// A position inside a table
+#[derive(Clone, Copy, Debug, Serialize)]
+pub struct Position {
+    /// row number, starting with zero
     pub row: usize,
+    /// column number, starting with zero
     pub col: usize,
 }
 
-#[derive(Debug)]
-pub(crate) enum DiffType {
+#[derive(Debug, Serialize, Clone)]
+/// Difference of a table entry
+pub enum DiffType {
+    /// Both entries were strings, but had different contents
     UnequalStrings {
+        /// nominal string
         nominal: String,
+        /// actual string
         actual: String,
+        /// position
         position: Position,
     },
+    /// Both entries were [`Quantity`]s but exceeded tolerances
     OutOfTolerance {
+        /// nominal
         nominal: Quantity,
+        /// actual
         actual: Quantity,
+        /// compare mode that was exceeded
         mode: Mode,
+        /// position in table
         position: Position,
     },
+    /// both fields had different value types
     DifferentValueTypes {
+        /// nominal
         nominal: Value,
+        /// actual
         actual: Value,
+        /// position
         position: Position,
     },
+    /// Both fields were headers but with different contents
     UnequalHeader {
+        /// nominal
         nominal: String,
+        /// actual
         actual: String,
     },
 }
@@ -209,7 +226,7 @@ impl Mode {
     }
 }
 
-#[derive(JsonSchema, Deserialize, Serialize, Debug, Default)]
+#[derive(JsonSchema, Deserialize, Serialize, Debug, Default, Clone)]
 /// Settings for the CSV comparison module
 pub struct CSVCompareConfig {
     #[serde(flatten)]
@@ -489,23 +506,19 @@ pub(crate) fn compare_paths(
     nominal: impl AsRef<Path>,
     actual: impl AsRef<Path>,
     config: &CSVCompareConfig,
-) -> Result<report::FileCompareResult, Error> {
+) -> Result<report::Difference, Error> {
     let nominal_file = fat_io_wrap_std(nominal.as_ref(), &File::open)?;
     let actual_file = fat_io_wrap_std(actual.as_ref(), &File::open)?;
 
-    let (nominal_table, actual_table, results) =
-        get_diffs_readers(&nominal_file, &actual_file, config)?;
+    let (_, _, results) = get_diffs_readers(&nominal_file, &actual_file, config)?;
     results.iter().for_each(|error| {
         error!("{}", &error);
     });
-
-    Ok(report::write_csv_detail(
-        nominal_table,
-        actual_table,
-        nominal.as_ref(),
-        actual.as_ref(),
-        results.as_slice(),
-    )?)
+    let is_error = !results.is_empty();
+    let mut result = report::Difference::new_for_file(nominal.as_ref(), actual.as_ref());
+    result.is_error = is_error;
+    result.detail = results.into_iter().map(report::DiffDetail::CSV).collect();
+    Ok(result)
 }
 
 #[cfg(test)]

src/csv/preprocessing.rs

@@ -6,7 +6,7 @@ use serde::{Deserialize, Serialize};
 use std::cmp::Ordering::Equal;
 use tracing::{debug, warn};
 
-#[derive(JsonSchema, Deserialize, Serialize, Debug)]
+#[derive(JsonSchema, Deserialize, Serialize, Debug, Clone)]
 /// Preprocessor options
 pub enum Preprocessor {
     /// Try to extract the headers from the first row - fallible if first row contains a number
@@ -237,7 +237,7 @@ mod tests {
             File::open("tests/csv/data/defects_headers.csv").unwrap(),
             &delimiters,
         )
-            .unwrap()
+        .unwrap()
     }
 
     #[test]

src/csv/value.rs

@@ -78,7 +78,7 @@ impl Display for Quantity {
     }
 }
 
-#[derive(Debug, PartialEq, Clone)]
+#[derive(Debug, PartialEq, Clone, Serialize)]
 pub enum Value {
     Quantity(Quantity),
     String(String),

src/external.rs

@@ -1,11 +1,11 @@
-use crate::report::FileCompareResult;
-use crate::{report, Error};
+use crate::report::{DiffDetail, Difference};
+use crate::Error;
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 use std::path::Path;
 use tracing::{error, info};
 
-#[derive(Debug, Deserialize, Serialize, JsonSchema)]
+#[derive(Debug, Deserialize, Serialize, JsonSchema, Clone)]
 pub struct ExternalConfig {
     /// The executable to call - will be started like: `#executable #(#extra_params)* #nominal #actual`
     executable: String,
@@ -17,46 +17,35 @@ pub(crate) fn compare_files<P: AsRef<Path>>(
     nominal: P,
     actual: P,
     config: &ExternalConfig,
-) -> Result<FileCompareResult, Error> {
+) -> Result<Difference, Error> {
+    let mut diff = Difference::new_for_file(&nominal, &actual);
     let compared_file_name = nominal.as_ref().to_string_lossy().into_owned();
-    let mut is_error = false;
     let output = std::process::Command::new(&config.executable)
         .args(&config.extra_params)
         .arg(nominal.as_ref())
         .arg(actual.as_ref())
         .output();
-    let (stdout_string, stderr_string, error_message) = if let Ok(output) = output {
+    if let Ok(output) = output {
         let stdout = String::from_utf8_lossy(&output.stdout).to_string();
         let stderr = String::from_utf8_lossy(&output.stderr).to_string();
         info!("External stdout: {}", stdout.as_str());
         info!("External stderr: {}", stderr.as_str());
-        let error_message = if !output.status.success() {
+        if !output.status.success() {
             let message = format!("External checker denied file {}", &compared_file_name);
             error!("{}", &message);
-            is_error = true;
-            message
-        } else {
-            "".to_owned()
+            diff.push_detail(DiffDetail::External { stdout, stderr });
+            diff.error();
         };
-
-        (stdout, stderr, error_message)
     } else {
         let error_message = format!(
             "External checker execution failed for file {}",
             &compared_file_name
         );
         error!("{}", error_message);
-        is_error = true;
-        ("".to_owned(), "".to_owned(), error_message)
+        diff.push_detail(DiffDetail::Error(error_message));
+        diff.error();
     };
-    Ok(report::write_external_detail(
-        nominal,
-        actual,
-        is_error,
-        &stdout_string,
-        &stderr_string,
-        &error_message,
-    )?)
+    Ok(diff)
 }
 #[cfg(test)]
 mod tests {

src/hash.rs

@@ -1,6 +1,7 @@
 use crate::{report, Deserialize, Serialize};
 use data_encoding::HEXLOWER;
 
+use crate::report::{DiffDetail, Difference};
 use schemars_derive::JsonSchema;
 use std::fs::File;
 use std::io::Read;
@@ -9,7 +10,7 @@ use thiserror::Error;
 use vg_errortools::fat_io_wrap_std;
 use vg_errortools::FatIOError;
 
-#[derive(Debug, Deserialize, Serialize, JsonSchema)]
+#[derive(Debug, Deserialize, Serialize, JsonSchema, Clone, Copy)]
 pub enum HashFunction {
     Sha256,
 }
@@ -43,7 +44,7 @@ impl HashFunction {
     }
 }
 
-#[derive(Debug, Deserialize, Serialize, JsonSchema)]
+#[derive(Debug, Deserialize, Serialize, JsonSchema, Clone)]
 /// Configuration options for the hash comparison module
 pub struct HashConfig {
     /// Which hash function to use
@@ -62,29 +63,23 @@ pub fn compare_files<P: AsRef<Path>>(
     nominal_path: P,
     actual_path: P,
     config: &HashConfig,
-) -> Result<report::FileCompareResult, Error> {
+) -> Result<Difference, Error> {
     let act = config
         .function
         .hash_file(fat_io_wrap_std(actual_path.as_ref(), &File::open)?)?;
     let nom = config
         .function
         .hash_file(fat_io_wrap_std(nominal_path.as_ref(), &File::open)?)?;
 
-    let diff = if act != nom {
-        vec![format!(
-            "Nominal file's hash is '{}' actual is '{}'",
-            HEXLOWER.encode(&nom),
-            HEXLOWER.encode(&act)
-        )]
-    } else {
-        vec![]
-    };
-
-    Ok(report::write_html_detail(
-        nominal_path,
-        actual_path,
-        diff.as_slice(),
-    )?)
+    let mut difference = Difference::new_for_file(nominal_path, actual_path);
+    if act != nom {
+        difference.push_detail(DiffDetail::Hash {
+            actual: HEXLOWER.encode(&act),
+            nominal: HEXLOWER.encode(&nom),
+        });
+        difference.error();
+    }
+    Ok(difference)
 }
 
 #[cfg(test)]

src/html.rs

@@ -1,4 +1,5 @@
 use crate::report;
+use crate::report::{DiffDetail, Difference};
 use regex::Regex;
 use schemars_derive::JsonSchema;
 use serde::{Deserialize, Serialize};
@@ -11,7 +12,7 @@ use tracing::error;
 use vg_errortools::fat_io_wrap_std;
 use vg_errortools::FatIOError;
 
-#[derive(Debug, Deserialize, Serialize, JsonSchema)]
+#[derive(Debug, Deserialize, Serialize, JsonSchema, Clone)]
 /// Plain text comparison config, also used for PDF
 pub struct HTMLCompareConfig {
     /// Normalized Damerau-Levenshtein distance, 0.0 = bad, 1.0 = identity
@@ -58,14 +59,12 @@ pub fn compare_files<P: AsRef<Path>>(
     nominal_path: P,
     actual_path: P,
     config: &HTMLCompareConfig,
-) -> Result<report::FileCompareResult, Error> {
+) -> Result<Difference, Error> {
     let actual = BufReader::new(fat_io_wrap_std(actual_path.as_ref(), &File::open)?);
     let nominal = BufReader::new(fat_io_wrap_std(nominal_path.as_ref(), &File::open)?);
 
-    let mut diffs: Vec<String> = Vec::new();
-
     let exclusion_list = config.get_ignore_list()?;
-
+    let mut difference = Difference::new_for_file(nominal_path, actual_path);
     actual
         .lines()
         .enumerate()
@@ -84,16 +83,12 @@ pub fn compare_files<P: AsRef<Path>>(
                 );
 
                 error!("{}" , &error);
-
-                diffs.push(error);
+                difference.push_detail(DiffDetail::Text {actual: a, nominal: n, score: distance, line: l});
+                difference.error();
             }
         });
 
-    Ok(report::write_html_detail(
-        nominal_path.as_ref(),
-        actual_path.as_ref(),
-        &diffs,
-    )?)
+    Ok(difference)
 }
 
 #[cfg(test)]
@@ -121,8 +116,6 @@ mod test {
         let result = compare_files(actual, nominal, &HTMLCompareConfig::default()).unwrap();
 
         assert!(result.is_error);
-
-        assert!(result.detail_path.is_some());
     }
 
     #[test]