Merge pull request #312 from epage/refactor

refactor(harness): Reuse more Assert logic

crates/snapbox/src/assert/error.rs

@@ -17,6 +17,11 @@ impl Error {
             backtrace: Backtrace::new(),
         }
     }
+
+    #[track_caller]
+    pub(crate) fn panic(self) -> ! {
+        panic!("{self}")
+    }
 }
 
 impl PartialEq for Error {

crates/snapbox/src/assert/mod.rs

@@ -29,7 +29,7 @@ pub use error::Result;
 /// ```
 #[derive(Clone, Debug)]
 pub struct Assert {
-    action: Action,
+    pub(crate) action: Action,
     action_var: Option<String>,
     normalize_paths: bool,
     substitutions: crate::Redactions,
@@ -64,24 +64,30 @@ impl Assert {
     pub fn eq(&self, expected: impl Into<crate::Data>, actual: impl Into<crate::Data>) {
         let expected = expected.into();
         let actual = actual.into();
-        self.eq_inner(expected, actual);
+        if let Err(err) = self.try_eq(expected, actual, Some(&"In-memory")) {
+            err.panic();
+        }
     }
 
-    #[track_caller]
-    fn eq_inner(&self, expected: crate::Data, actual: crate::Data) {
+    pub(crate) fn try_eq(
+        &self,
+        expected: crate::Data,
+        actual: crate::Data,
+        actual_name: Option<&dyn std::fmt::Display>,
+    ) -> Result<()> {
         if expected.source().is_none() && actual.source().is_some() {
             panic!("received `(actual, expected)`, expected `(expected, actual)`");
         }
         match self.action {
             Action::Skip => {
-                return;
+                return Ok(());
             }
             Action::Ignore | Action::Verify | Action::Overwrite => {}
         }
 
         let (expected, actual) = self.normalize_eq(expected, actual);
 
-        self.do_action(expected, actual, Some(&"In-memory"));
+        self.do_action(expected, actual, actual_name)
     }
 
     /// Check if a value matches a pattern
@@ -113,24 +119,30 @@ impl Assert {
     pub fn matches(&self, pattern: impl Into<crate::Data>, actual: impl Into<crate::Data>) {
         let pattern = pattern.into();
         let actual = actual.into();
-        self.matches_inner(pattern, actual);
+        if let Err(err) = self.try_matches(pattern, actual, Some(&"In-memory")) {
+            err.panic();
+        }
     }
 
-    #[track_caller]
-    fn matches_inner(&self, pattern: crate::Data, actual: crate::Data) {
+    pub(crate) fn try_matches(
+        &self,
+        pattern: crate::Data,
+        actual: crate::Data,
+        actual_name: Option<&dyn std::fmt::Display>,
+    ) -> Result<()> {
         if pattern.source().is_none() && actual.source().is_some() {
             panic!("received `(actual, expected)`, expected `(expected, actual)`");
         }
         match self.action {
             Action::Skip => {
-                return;
+                return Ok(());
             }
             Action::Ignore | Action::Verify | Action::Overwrite => {}
         }
 
         let (expected, actual) = self.normalize_match(pattern, actual);
 
-        self.do_action(expected, actual, Some(&"In-memory"));
+        self.do_action(expected, actual, actual_name)
     }
 
     pub(crate) fn normalize_eq(
@@ -169,47 +181,49 @@ impl Assert {
         (expected, actual)
     }
 
-    #[track_caller]
     pub(crate) fn do_action(
         &self,
         expected: crate::Data,
         actual: crate::Data,
         actual_name: Option<&dyn std::fmt::Display>,
-    ) {
+    ) -> Result<()> {
         let result = self.try_verify(&expected, &actual, actual_name);
-        if let Err(err) = result {
-            match self.action {
-                Action::Skip => unreachable!("Bailed out earlier"),
-                Action::Ignore => {
-                    use std::io::Write;
-
-                    let _ = writeln!(
-                        stderr(),
-                        "{}: {}",
-                        self.palette.warn("Ignoring failure"),
-                        err
-                    );
-                }
-                Action::Verify => {
-                    let message = if expected.source().is_none() {
-                        crate::report::Styled::new(String::new(), Default::default())
-                    } else if let Some(action_var) = self.action_var.as_deref() {
-                        self.palette
-                            .hint(format!("Update with {}=overwrite", action_var))
-                    } else {
-                        crate::report::Styled::new(String::new(), Default::default())
-                    };
-                    panic!("{err}{message}");
-                }
-                Action::Overwrite => {
-                    use std::io::Write;
-
-                    if let Some(source) = expected.source() {
-                        let _ = writeln!(stderr(), "{}: {}", self.palette.warn("Fixing"), err);
-                        actual.write_to(source).unwrap();
-                    } else {
-                        panic!("{err}");
-                    }
+        let Err(err) = result else {
+            return Ok(());
+        };
+        match self.action {
+            Action::Skip => unreachable!("Bailed out earlier"),
+            Action::Ignore => {
+                use std::io::Write;
+
+                let _ = writeln!(
+                    stderr(),
+                    "{}: {}",
+                    self.palette.warn("Ignoring failure"),
+                    err
+                );
+                Ok(())
+            }
+            Action::Verify => {
+                let message = if expected.source().is_none() {
+                    crate::report::Styled::new(String::new(), Default::default())
+                } else if let Some(action_var) = self.action_var.as_deref() {
+                    self.palette
+                        .hint(format!("Update with {}=overwrite", action_var))
+                } else {
+                    crate::report::Styled::new(String::new(), Default::default())
+                };
+                Err(Error::new(format_args!("{err}{message}")))
+            }
+            Action::Overwrite => {
+                use std::io::Write;
+
+                if let Some(source) = expected.source() {
+                    let _ = writeln!(stderr(), "{}: {}", self.palette.warn("Fixing"), err);
+                    actual.write_to(source).unwrap();
+                    Ok(())
+                } else {
+                    Err(Error::new(format_args!("{err}")))
                 }
             }
         }

crates/snapbox/src/cmd.rs

@@ -620,8 +620,9 @@ impl OutputAssert {
     #[track_caller]
     fn stdout_eq_inner(self, expected: crate::Data) -> Self {
         let actual = crate::Data::from(self.output.stdout.as_slice());
-        let (pattern, actual) = self.config.normalize_eq(expected, actual);
-        self.config.do_action(pattern, actual, Some(&"stdout"));
+        if let Err(err) = self.config.try_eq(expected, actual, Some(&"stdout")) {
+            err.panic();
+        }
 
         self
     }
@@ -660,8 +661,9 @@ impl OutputAssert {
     #[track_caller]
     fn stdout_matches_inner(self, expected: crate::Data) -> Self {
         let actual = crate::Data::from(self.output.stdout.as_slice());
-        let (pattern, actual) = self.config.normalize_match(expected, actual);
-        self.config.do_action(pattern, actual, Some(&"stdout"));
+        if let Err(err) = self.config.try_matches(expected, actual, Some(&"stdout")) {
+            err.panic();
+        }
 
         self
     }
@@ -700,8 +702,9 @@ impl OutputAssert {
     #[track_caller]
     fn stderr_eq_inner(self, expected: crate::Data) -> Self {
         let actual = crate::Data::from(self.output.stderr.as_slice());
-        let (pattern, actual) = self.config.normalize_eq(expected, actual);
-        self.config.do_action(pattern, actual, Some(&"stderr"));
+        if let Err(err) = self.config.try_eq(expected, actual, Some(&"stderr")) {
+            err.panic();
+        }
 
         self
     }
@@ -740,8 +743,9 @@ impl OutputAssert {
     #[track_caller]
     fn stderr_matches_inner(self, expected: crate::Data) -> Self {
         let actual = crate::Data::from(self.output.stderr.as_slice());
-        let (pattern, actual) = self.config.normalize_match(expected, actual);
-        self.config.do_action(pattern, actual, Some(&"stderr"));
+        if let Err(err) = self.config.try_matches(expected, actual, Some(&"stderr")) {
+            err.panic();
+        }
 
         self
     }

crates/snapbox/src/harness.rs

@@ -36,7 +36,6 @@
 //! ```
 
 use crate::data::DataFormat;
-use crate::filter::{Filter as _, FilterNewlines};
 use crate::Action;
 
 use libtest_mimic::Trial;
@@ -49,7 +48,7 @@ pub struct Harness<S, T> {
     overrides: Option<ignore::overrides::Override>,
     setup: S,
     test: T,
-    action: Action,
+    config: crate::Assert,
 }
 
 impl<S, T, I, E> Harness<S, T>
@@ -71,7 +70,7 @@ where
             overrides: None,
             setup,
             test,
-            action: Action::Verify,
+            config: crate::Assert::new().action_env(crate::assert::DEFAULT_ACTION_ENV),
         }
     }
 
@@ -89,14 +88,19 @@ where
 
     /// Read the failure action from an environment variable
     pub fn action_env(mut self, var_name: &str) -> Self {
-        let action = Action::with_env_var(var_name);
-        self.action = action.unwrap_or(self.action);
+        self.config = self.config.action_env(var_name);
         self
     }
 
     /// Override the failure action
     pub fn action(mut self, action: Action) -> Self {
-        self.action = action;
+        self.config = self.config.action(action);
+        self
+    }
+
+    /// Customize the assertion behavior
+    pub fn with_assert(mut self, config: crate::Assert) -> Self {
+        self.config = config;
         self
     }
 
@@ -118,23 +122,22 @@ where
             }
         });
 
+        let shared_config = std::sync::Arc::new(self.config);
         let tests: Vec<_> = tests
             .into_iter()
             .map(|path| {
                 let case = (self.setup)(path);
                 let test = self.test.clone();
+                let config = shared_config.clone();
                 Trial::test(case.name.clone(), move || {
+                    let expected = crate::Data::read_from(&case.expected, Some(DataFormat::Text));
                     let actual = (test)(&case.fixture)?;
                     let actual = actual.to_string();
-                    let actual = FilterNewlines.filter(crate::Data::text(actual));
-                    #[allow(deprecated)]
-                    let verify = Verifier::new()
-                        .palette(crate::report::Palette::auto())
-                        .action(self.action);
-                    verify.verify(&case.expected, actual)?;
+                    let actual = crate::Data::text(actual);
+                    config.try_eq(expected, actual, Some(&case.name))?;
                     Ok(())
                 })
-                .with_ignored_flag(self.action == Action::Ignore)
+                .with_ignored_flag(shared_config.action == Action::Ignore)
             })
             .collect();
 
@@ -143,89 +146,6 @@ where
     }
 }
 
-struct Verifier {
-    palette: crate::report::Palette,
-    action: Action,
-}
-
-impl Verifier {
-    fn new() -> Self {
-        Default::default()
-    }
-
-    fn palette(mut self, palette: crate::report::Palette) -> Self {
-        self.palette = palette;
-        self
-    }
-
-    fn action(mut self, action: Action) -> Self {
-        self.action = action;
-        self
-    }
-
-    fn verify(
-        &self,
-        expected_path: &std::path::Path,
-        actual: crate::Data,
-    ) -> crate::assert::Result<()> {
-        match self.action {
-            Action::Skip => Ok(()),
-            Action::Ignore => {
-                let _ = self.try_verify(expected_path, actual);
-                Ok(())
-            }
-            Action::Verify => self.try_verify(expected_path, actual),
-            Action::Overwrite => self.try_overwrite(expected_path, actual),
-        }
-    }
-
-    fn try_overwrite(
-        &self,
-        expected_path: &std::path::Path,
-        actual: crate::Data,
-    ) -> crate::assert::Result<()> {
-        actual.write_to_path(expected_path)?;
-        Ok(())
-    }
-
-    fn try_verify(
-        &self,
-        expected_path: &std::path::Path,
-        actual: crate::Data,
-    ) -> crate::assert::Result<()> {
-        let expected = FilterNewlines.filter(crate::Data::read_from(
-            expected_path,
-            Some(DataFormat::Text),
-        ));
-
-        if expected != actual {
-            let mut buf = String::new();
-            crate::report::write_diff(
-                &mut buf,
-                &expected,
-                &actual,
-                Some(&expected_path.display()),
-                None,
-                self.palette,
-            )
-            .map_err(|e| e.to_string())?;
-            Err(buf.into())
-        } else {
-            Ok(())
-        }
-    }
-}
-
-impl Default for Verifier {
-    fn default() -> Self {
-        Self {
-            #[allow(deprecated)]
-            palette: crate::report::Palette::auto(),
-            action: Action::Verify,
-        }
-    }
-}
-
 /// A test case enumerated by the [`Harness`] with data from the `setup` function
 ///
 /// See [`harness`][crate::harness] for more details