Add binary snapshots

This is the somewhat simplified version of mitsuhiko#489 where we keep the full files in-memory during the assert and when reviewing.
lasernoises · Sep 18, 2024 · e0612ae · e0612ae
1 parent 0894efd
commit e0612ae
Show file tree

Hide file tree

Showing 15 changed files with 657 additions and 202 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/cargo-insta/Cargo.toml b/cargo-insta/Cargo.toml
@@ -30,6 +30,7 @@ tempfile = "3.5.0"
 semver = {version = "1.0.7", features = ["serde"]}
 lazy_static = "1.4.0"
 clap = { workspace=true }
+open = "5.3.0"
 
 [dev-dependencies]
 walkdir = "2.3.1"

diff --git a/cargo-insta/src/cli.rs b/cargo-insta/src/cli.rs
@@ -6,6 +6,7 @@ use std::{env, fs};
 use std::{io, process};
 
 use console::{set_colors_enabled, style, Key, Term};
+use insta::internals::SnapshotContents;
 use insta::Snapshot;
 use insta::_cargo_insta_support::{
     is_ci, SnapshotPrinter, SnapshotUpdate, TestRunner, ToolConfig, UnreferencedSnapshots,
@@ -321,6 +322,24 @@ fn query_snapshot(
             style("toggle snapshot diff").dim()
         );
 
+        let new_is_binary = new.contents().is_binary();
+        let old_is_binary = old.is_some_and(|o| o.contents().is_binary());
+
+        if new_is_binary || old_is_binary {
+            println!(
+                "  {} open       {}",
+                style("o").cyan().bold(),
+                style(if new_is_binary && old_is_binary {
+                    "open snapshot files in external tool"
+                } else if new_is_binary {
+                    "open new snapshot file in external tool"
+                } else {
+                    "open old snapshot file in external tool"
+                })
+                .dim()
+            );
+        }
+
         loop {
             match term.read_key()? {
                 Key::Char('a') | Key::Enter => return Ok(Operation::Accept),
@@ -334,6 +353,19 @@ fn query_snapshot(
                     *show_diff = !*show_diff;
                     break;
                 }
+                Key::Char('o') => {
+                    if let Some(SnapshotContents::Binary(contents)) = old.map(|o| o.contents()) {
+                        open::that_detached(contents.build_path(snapshot_file.unwrap()))?;
+                    }
+
+                    if let SnapshotContents::Binary(contents) = new.contents() {
+                        open::that_detached(
+                            contents.build_path(snapshot_file.unwrap().with_extension("snap.new")),
+                        )?;
+                    }
+
+                    // there's no break here because there's no need to re-output anything
+                }
                 _ => {}
             }
         }
@@ -1081,8 +1113,8 @@ fn pending_snapshots_cmd(cmd: PendingSnapshotsCommand) -> Result<(), Box<dyn Err
                     SnapshotKey::InlineSnapshot {
                         path: &target_file,
                         line: snapshot_ref.line.unwrap(),
-                        old_snapshot: snapshot_ref.old.as_ref().map(|x| x.contents_str()),
-                        new_snapshot: snapshot_ref.new.contents_str(),
+                        old_snapshot: snapshot_ref.old.as_ref().map(|x| x.contents_str().unwrap()),
+                        new_snapshot: snapshot_ref.new.contents_str().unwrap(),
                         expression: snapshot_ref.new.metadata().expression(),
                     }
                 } else {

diff --git a/cargo-insta/src/container.rs b/cargo-insta/src/container.rs
@@ -235,6 +235,12 @@ impl SnapshotContainer {
                     }
                     Operation::Skip => {}
                 }
+
+                if let Operation::Accept | Operation::Reject = snapshot.op {
+                    let snapshot = Snapshot::from_file(&self.target_path).ok();
+
+                    Snapshot::cleanup_extra_files(snapshot.as_ref(), &self.target_path)?;
+                }
             }
         }
         Ok(())

diff --git a/cargo-insta/src/inline.rs b/cargo-insta/src/inline.rs
@@ -104,8 +104,12 @@ impl FilePatcher {
             .collect();
 
         // replace lines
-        let snapshot_line_contents =
-            [prefix, snapshot.to_inline(inline.indentation), suffix].join("");
+        let snapshot_line_contents = [
+            prefix,
+            snapshot.to_inline(inline.indentation).unwrap(),
+            suffix,
+        ]
+        .join("");
 
         self.lines.splice(
             inline.start.0..=inline.end.0,

diff --git a/cargo-insta/tests/main.rs b/cargo-insta/tests/main.rs
@@ -876,12 +876,13 @@ Hello, world!
     assert_snapshot!(test_current_insta.diff("src/snapshots/test_force_update_current__force_update.snap"), @r#"
     --- Original: src/snapshots/test_force_update_current__force_update.snap
     +++ Updated: src/snapshots/test_force_update_current__force_update.snap
-    @@ -1,8 +1,5 @@
+    @@ -1,8 +1,6 @@
     -
      ---
      source: src/lib.rs
     -expression: 
     +expression: "\"Hello, world!\""
+    +snapshot_type: string
      ---
      Hello, world!
     -
@@ -891,12 +892,13 @@ Hello, world!
     assert_snapshot!(test_insta_1_40_0.diff("src/snapshots/test_force_update_1_40_0__force_update.snap"), @r#"
     --- Original: src/snapshots/test_force_update_1_40_0__force_update.snap
     +++ Updated: src/snapshots/test_force_update_1_40_0__force_update.snap
-    @@ -1,8 +1,5 @@
+    @@ -1,8 +1,6 @@
     -
      ---
      source: src/lib.rs
     -expression: 
     +expression: "\"Hello, world!\""
+    +snapshot_type: string
      ---
      Hello, world!
     -

diff --git a/insta/src/content/mod.rs b/insta/src/content/mod.rs
@@ -22,7 +22,6 @@ use std::fmt;
 pub enum Error {
     FailedParsingYaml(std::path::PathBuf),
     UnexpectedDataType,
-    #[cfg(feature = "_cargo_insta_internal")]
     MissingField,
     #[cfg(feature = "_cargo_insta_internal")]
     FileIo(std::io::Error, std::path::PathBuf),
@@ -37,7 +36,6 @@ impl fmt::Display for Error {
             Error::UnexpectedDataType => {
                 f.write_str("The present data type wasn't what was expected")
             }
-            #[cfg(feature = "_cargo_insta_internal")]
             Error::MissingField => f.write_str("A required field was missing"),
             #[cfg(feature = "_cargo_insta_internal")]
             Error::FileIo(e, p) => {

diff --git a/insta/src/lib.rs b/insta/src/lib.rs
@@ -333,7 +333,9 @@ pub use crate::redaction::{dynamic_redaction, rounded_redaction, sorted_redactio
 pub mod _macro_support {
     pub use crate::content::Content;
     pub use crate::env::get_cargo_workspace;
-    pub use crate::runtime::{assert_snapshot, with_allow_duplicates, AutoName, ReferenceValue};
+    pub use crate::runtime::{
+        assert_snapshot, with_allow_duplicates, AutoName, InlineValue, SnapshotValue,
+    };
 
     #[cfg(feature = "serde")]
     pub use crate::serialization::{serialize_value, SerializationFormat, SnapshotLocation};

diff --git a/insta/src/macros.rs b/insta/src/macros.rs
@@ -326,7 +326,7 @@ macro_rules! _assert_snapshot_base {
         $crate::_assert_snapshot_base!(
             transform = $transform,
             #[allow(clippy::needless_raw_string_hashes)]
-            $crate::_macro_support::ReferenceValue::Inline($snapshot),
+            $crate::_macro_support::InlineValue($snapshot),
             $($arg),*
         )
     };
@@ -346,9 +346,39 @@ macro_rules! _assert_snapshot_base {
     // The main macro body — every call to this macro should end up here.
     (transform=$transform:expr, $name:expr, $value:expr, $debug_expr:expr $(,)?) => {
         $crate::_macro_support::assert_snapshot(
-            $name.into(),
-            #[allow(clippy::redundant_closure_call)]
-            &$transform(&$value),
+            (
+                $name,
+                #[allow(clippy::redundant_closure_call)]
+                $transform(&$value).as_str(),
+            ).into(),
+            env!("CARGO_MANIFEST_DIR"),
+            $crate::_function_name!(),
+            module_path!(),
+            file!(),
+            line!(),
+            $debug_expr,
+        )
+        .unwrap()
+    };
+}
+
+#[macro_export]
+macro_rules! assert_binary_snapshot {
+    ($extension:expr, $value:expr $(,)?) => {
+        $crate::assert_binary_snapshot!($extension, $crate::_macro_support::AutoName, $value);
+    };
+
+    ($extension:expr, $name:expr, $value:expr $(,)?) => {
+        $crate::assert_binary_snapshot!($extension, $name, $value, stringify!($value));
+    };
+
+    ($extension:expr, $name:expr, $value:expr, $debug_expr:expr $(,)?) => {
+        $crate::_macro_support::assert_snapshot(
+            $crate::_macro_support::SnapshotValue::Binary {
+                name: $name.into(),
+                content: $value,
+                extension: $extension,
+            },
             env!("CARGO_MANIFEST_DIR"),
             $crate::_function_name!(),
             module_path!(),