diff --git a/Cargo.lock b/Cargo.lock index 075e5cef..38a16d20 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -75,6 +75,7 @@ dependencies = [ "insta", "itertools", "lazy_static", + "open", "proc-macro2", "semver", "serde", @@ -391,6 +392,15 @@ dependencies = [ "windows-sys 0.48.0", ] +[[package]] +name = "is-docker" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "928bae27f42bc99b60d9ac7334e3a21d10ad8f1835a4e12ec3ec0464765ed1b3" +dependencies = [ + "once_cell", +] + [[package]] name = "is-terminal" version = "0.4.12" @@ -402,6 +412,16 @@ dependencies = [ "windows-sys 0.52.0", ] +[[package]] +name = "is-wsl" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "173609498df190136aa7dea1a91db051746d339e18476eed5ca40521f02d7aa5" +dependencies = [ + "is-docker", + "once_cell", +] + [[package]] name = "itertools" version = "0.10.5" @@ -464,9 +484,26 @@ checksum = "78ca9ab1a0babb1e7d5695e3530886289c18cf2f87ec19a575a0abdce112e3a3" [[package]] name = "once_cell" -version = "1.14.0" +version = "1.20.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "33ea5043e58958ee56f3e15a90aee535795cd7dfd319846288d93c5b57d85cbe" + +[[package]] +name = "open" +version = "5.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "61a877bf6abd716642a53ef1b89fb498923a4afca5c754f9050b4d081c05c4b3" +dependencies = [ + "is-wsl", + "libc", + "pathdiff", +] + +[[package]] +name = "pathdiff" +version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2f7254b99e31cad77da24b08ebf628882739a608578bb1bcdfc1f9c21260d7c0" +checksum = "8835116a5c179084a830efb3adc117ab007512b535bc1a21c991d3b32a6b44dd" [[package]] name = "pest" diff --git a/cargo-insta/Cargo.toml b/cargo-insta/Cargo.toml index 9e95c6fd..0f0708ae 100644 --- 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.1.2" [dev-dependencies] walkdir = "2.3.1" diff --git a/cargo-insta/src/cli.rs b/cargo-insta/src/cli.rs index 03c775ba..9f891de2 100644 --- 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,17 @@ fn query_snapshot( *show_diff = !*show_diff; break; } + Key::Char('o') => { + if let Some(SnapshotContents::Binary { path, .. }) = old.map(|o| o.contents()) { + open::that_detached(path)?; + } + + if let SnapshotContents::Binary { path, .. } = new.contents() { + open::that_detached(path)?; + } + + // there's no break here because there's no need to re-output anything + } _ => {} } } @@ -1081,8 +1111,8 @@ fn pending_snapshots_cmd(cmd: PendingSnapshotsCommand) -> Result<(), Box { - let snapshot = Snapshot::from_file(&self.snapshot_path).map_err(|e| { - // If it's an IO error, pass a ContentError back so - // we get a slightly clearer error message - match e.downcast::() { - Ok(io_error) => Box::new(ContentError::FileIo( - *io_error, - self.snapshot_path.to_path_buf(), - )), - Err(other_error) => other_error, - } - })?; - snapshot.save(&self.target_path)?; + let mut new_snapshot = + Snapshot::from_file(&self.snapshot_path).map_err(|e| { + // If it's an IO error, pass a ContentError back so + // we get a slightly clearer error message + match e.downcast::() { + Ok(io_error) => Box::new(ContentError::FileIo( + *io_error, + self.snapshot_path.to_path_buf(), + )), + Err(other_error) => other_error, + } + })?; + new_snapshot.save(&self.target_path)?; try_removing_snapshot(&self.snapshot_path); } Operation::Reject => { try_removing_snapshot(&self.snapshot_path); + + if let insta::internals::SnapshotContents::Binary { path, .. } = + snapshot.new.contents() + { + try_removing_snapshot(path); + } } 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.snapshot_path)?; + } } } Ok(()) diff --git a/cargo-insta/src/inline.rs b/cargo-insta/src/inline.rs index b0ee5150..778e4e2f 100644 --- 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/insta/src/content/mod.rs b/insta/src/content/mod.rs index 8fe5e14e..435e922c 100644 --- 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 index 4de7fc24..b4fd751b 100644 --- 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, ReferenceValue, SnapshotValue, + }; #[cfg(feature = "serde")] pub use crate::serialization::{serialize_value, SerializationFormat, SnapshotLocation}; diff --git a/insta/src/macros.rs b/insta/src/macros.rs index ba837e40..29d12bb6 100644 --- a/insta/src/macros.rs +++ b/insta/src/macros.rs @@ -348,7 +348,35 @@ macro_rules! _assert_snapshot_base { $crate::_macro_support::assert_snapshot( $name.into(), #[allow(clippy::redundant_closure_call)] - &$transform(&$value), + $crate::_macro_support::SnapshotValue::String(&$transform(&$value)), + 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( + $name.into(), + $crate::_macro_support::SnapshotValue::Binary { + write: &mut $value, + extension: $extension, + }, env!("CARGO_MANIFEST_DIR"), $crate::_function_name!(), module_path!(), diff --git a/insta/src/output.rs b/insta/src/output.rs index 075e914c..f1ca64b8 100644 --- a/insta/src/output.rs +++ b/insta/src/output.rs @@ -4,7 +4,7 @@ use std::{path::Path, time::Duration}; use similar::{Algorithm, ChangeTag, TextDiff}; use crate::content::yaml; -use crate::snapshot::{MetaData, Snapshot}; +use crate::snapshot::{MetaData, Snapshot, SnapshotContents}; use crate::utils::{format_rust_expression, style, term_width}; /// Snapshot printer utility. @@ -103,119 +103,174 @@ impl<'a> SnapshotPrinter<'a> { fn print_snapshot(&self) { print_line(term_width()); - let new_contents = self.new_snapshot.contents_str(); + let new_contents = self.new_snapshot.contents(); let width = term_width(); if self.show_info { self.print_info(); } println!("Snapshot Contents:"); - println!("──────┬{:─^1$}", "", width.saturating_sub(7)); - for (idx, line) in new_contents.lines().enumerate() { - println!("{:>5} │ {}", style(idx + 1).cyan().dim().bold(), line); + + match new_contents { + crate::snapshot::SnapshotContents::String(new_contents) => { + println!("──────┬{:─^1$}", "", width.saturating_sub(7)); + for (idx, line) in new_contents.lines().enumerate() { + println!("{:>5} │ {}", style(idx + 1).cyan().dim().bold(), line); + } + println!("──────┴{:─^1$}", "", width.saturating_sub(7)); + } + crate::snapshot::SnapshotContents::Binary { path, .. } => { + println!("{}", encode_file_link_escape(path)); + } } - println!("──────┴{:─^1$}", "", width.saturating_sub(7)); } fn print_changeset(&self) { - let old = self.old_snapshot.as_ref().map_or("", |x| x.contents_str()); - let new = self.new_snapshot.contents_str(); - let newlines_matter = newlines_matter(old, new); - let width = term_width(); - let diff = TextDiff::configure() - .algorithm(Algorithm::Patience) - .timeout(Duration::from_millis(500)) - .diff_lines(old, new); print_line(width); if self.show_info { self.print_info(); } - if !old.is_empty() { + let old_contents = self.old_snapshot.as_ref().map(|o| o.contents()); + let new_contents = self.new_snapshot.contents(); + + if let Some(SnapshotContents::Binary { path, extension: _ }) = old_contents { println!( "{}", - style(format_args!("-{}", self.old_snapshot_hint)).red() + style(format_args!( + "-{}: {}", + self.old_snapshot_hint, + encode_file_link_escape(path), + )) + .red() ); } - println!( - "{}", - style(format_args!("+{}", self.new_snapshot_hint)).green() - ); - println!("────────────┬{:─^1$}", "", width.saturating_sub(13)); - let mut has_changes = false; - for (idx, group) in diff.grouped_ops(4).iter().enumerate() { - if idx > 0 { - println!("┈┈┈┈┈┈┈┈┈┈┈┈┼{:┈^1$}", "", width.saturating_sub(13)); + if let SnapshotContents::Binary { path, extension: _ } = new_contents { + println!( + "{}", + style(format_args!( + "+{}: {}", + self.new_snapshot_hint, + encode_file_link_escape(path), + )) + .green() + ); + } + + if let Some((old, new)) = match ( + self.old_snapshot.as_ref().map(|o| o.contents()), + self.new_snapshot.contents(), + ) { + (Some(SnapshotContents::Binary { .. }) | None, SnapshotContents::String(new)) => { + Some((None, Some(new))) + } + (Some(SnapshotContents::String(old)), SnapshotContents::Binary { .. }) => { + Some((Some(old), None)) + } + (Some(SnapshotContents::String(old)), SnapshotContents::String(new)) => { + Some((Some(old), Some(new))) + } + _ => None, + } { + let old_text = old.map(String::as_str).unwrap_or(""); + let new_text = new.map(String::as_str).unwrap_or(""); + + let newlines_matter = newlines_matter(old_text, new_text); + let diff = TextDiff::configure() + .algorithm(Algorithm::Patience) + .timeout(Duration::from_millis(500)) + .diff_lines(old_text, new_text); + + if old.is_some() { + println!( + "{}", + style(format_args!("-{}", self.old_snapshot_hint)).red() + ); + } + + if new.is_some() { + println!( + "{}", + style(format_args!("+{}", self.new_snapshot_hint)).green() + ); } - for op in group { - for change in diff.iter_inline_changes(op) { - match change.tag() { - ChangeTag::Insert => { - has_changes = true; - print!( - "{:>5} {:>5} │{}", - "", - style(change.new_index().unwrap()).cyan().dim().bold(), - style("+").green(), - ); - for &(emphasized, change) in change.values() { - let change = render_invisible(change, newlines_matter); - if emphasized { - print!("{}", style(change).green().underlined()); - } else { - print!("{}", style(change).green()); + + println!("────────────┬{:─^1$}", "", width.saturating_sub(13)); + let mut has_changes = false; + for (idx, group) in diff.grouped_ops(4).iter().enumerate() { + if idx > 0 { + println!("┈┈┈┈┈┈┈┈┈┈┈┈┼{:┈^1$}", "", width.saturating_sub(13)); + } + for op in group { + for change in diff.iter_inline_changes(op) { + match change.tag() { + ChangeTag::Insert => { + has_changes = true; + print!( + "{:>5} {:>5} │{}", + "", + style(change.new_index().unwrap()).cyan().dim().bold(), + style("+").green(), + ); + for &(emphasized, change) in change.values() { + let change = render_invisible(change, newlines_matter); + if emphasized { + print!("{}", style(change).green().underlined()); + } else { + print!("{}", style(change).green()); + } } } - } - ChangeTag::Delete => { - has_changes = true; - print!( - "{:>5} {:>5} │{}", - style(change.old_index().unwrap()).cyan().dim(), - "", - style("-").red(), - ); - for &(emphasized, change) in change.values() { - let change = render_invisible(change, newlines_matter); - if emphasized { - print!("{}", style(change).red().underlined()); - } else { - print!("{}", style(change).red()); + ChangeTag::Delete => { + has_changes = true; + print!( + "{:>5} {:>5} │{}", + style(change.old_index().unwrap()).cyan().dim(), + "", + style("-").red(), + ); + for &(emphasized, change) in change.values() { + let change = render_invisible(change, newlines_matter); + if emphasized { + print!("{}", style(change).red().underlined()); + } else { + print!("{}", style(change).red()); + } } } - } - ChangeTag::Equal => { - print!( - "{:>5} {:>5} │ ", - style(change.old_index().unwrap()).cyan().dim(), - style(change.new_index().unwrap()).cyan().dim().bold(), - ); - for &(_, change) in change.values() { - let change = render_invisible(change, newlines_matter); - print!("{}", style(change).dim()); + ChangeTag::Equal => { + print!( + "{:>5} {:>5} │ ", + style(change.old_index().unwrap()).cyan().dim(), + style(change.new_index().unwrap()).cyan().dim().bold(), + ); + for &(_, change) in change.values() { + let change = render_invisible(change, newlines_matter); + print!("{}", style(change).dim()); + } } } - } - if change.missing_newline() { - println!(); + if change.missing_newline() { + println!(); + } } } } - } - if !has_changes { - println!( - "{:>5} {:>5} │{}", - "", - style("-").dim(), - style(" snapshots are matching").cyan(), - ); - } + if !has_changes { + println!( + "{:>5} {:>5} │{}", + "", + style("-").dim(), + style(" snapshots are matching").cyan(), + ); + } - println!("────────────┴{:─^1$}", "", width.saturating_sub(13)); + println!("────────────┴{:─^1$}", "", width.saturating_sub(13)); + } } } @@ -356,6 +411,17 @@ fn print_info(metadata: &MetaData) { } } +/// Encodes a path as an OSC-8 escape sequence. This makes it a clickable link in supported +/// terminal emulators. +fn encode_file_link_escape(path: &Path) -> String { + assert!(path.is_absolute()); + format!( + "\x1b]8;;file://{}\x1b\\{}\x1b]8;;\x1b\\", + path.display(), + path.display() + ) +} + #[test] fn test_invisible() { assert_eq!( diff --git a/insta/src/runtime.rs b/insta/src/runtime.rs index 22deb330..2c980182 100644 --- a/insta/src/runtime.rs +++ b/insta/src/runtime.rs @@ -1,7 +1,7 @@ use std::cell::RefCell; use std::collections::{BTreeMap, BTreeSet}; use std::error::Error; -use std::fs; +use std::fs::{self, File}; use std::io::Write; use std::path::{Path, PathBuf}; use std::str; @@ -14,7 +14,7 @@ use crate::env::{ }; use crate::output::SnapshotPrinter; use crate::settings::Settings; -use crate::snapshot::{MetaData, PendingInlineSnapshot, Snapshot, SnapshotContents}; +use crate::snapshot::{MetaData, PendingInlineSnapshot, Snapshot, SnapshotContents, SnapshotType}; use crate::utils::{path_to_storage, style}; lazy_static::lazy_static! { @@ -335,6 +335,12 @@ impl<'a> SnapshotAssertionContext<'a> { .input_file() .and_then(|x| self.localize_path(x)) .map(|x| path_to_storage(&x)), + snapshot_type: match contents { + SnapshotContents::String(_) => SnapshotType::String, + SnapshotContents::Binary { ref extension, .. } => SnapshotType::Binary { + extension: extension.clone(), + }, + }, }), contents, ) @@ -359,10 +365,29 @@ impl<'a> SnapshotAssertionContext<'a> { Ok(()) } + pub fn pre_create_binary_snapshot( + &self, + write: &mut dyn FnMut(&mut File), + extension: &str, + ) -> Result> { + let mut path = self.snapshot_file.as_ref().unwrap().clone(); + + path.set_extension("snap._"); + + let mut file = File::create(&path)?; + + write(&mut file); + + Ok(SnapshotContents::Binary { + path, + extension: extension.to_string(), + }) + } + /// Writes the changes of the snapshot back. pub fn update_snapshot( &self, - new_snapshot: Snapshot, + new_snapshot: &mut Snapshot, ) -> Result> { let unseen = self .snapshot_file @@ -425,7 +450,7 @@ impl<'a> SnapshotAssertionContext<'a> { } } else { PendingInlineSnapshot::new( - Some(new_snapshot), + Some(new_snapshot.clone()), self.old_snapshot.clone(), self.assertion_line, ) @@ -610,6 +635,15 @@ where } } +pub enum SnapshotValue<'a> { + String(&'a str), + + Binary { + write: &'a mut dyn FnMut(&mut File), + extension: &'a str, + }, +} + /// This function is invoked from the macros to run the main assertion logic. /// /// This will create the assertion context, run the main logic to assert @@ -619,7 +653,7 @@ where #[allow(clippy::too_many_arguments)] pub fn assert_snapshot( refval: ReferenceValue<'_>, - new_snapshot_value: &str, + new_snapshot_value: SnapshotValue, manifest_dir: &str, function_name: &str, module_path: &str, @@ -638,12 +672,37 @@ pub fn assert_snapshot( let tool_config = get_tool_config(manifest_dir); - // apply filters if they are available - #[cfg(feature = "filters")] - let new_snapshot_value = - Settings::with(|settings| settings.filters().apply_to(new_snapshot_value)); + if let Some(snapshot_file) = &ctx.snapshot_file { + Snapshot::cleanup_extra_files(ctx.old_snapshot.as_ref(), snapshot_file)?; + } - let new_snapshot = ctx.new_snapshot(new_snapshot_value.into(), expr); + let content = match new_snapshot_value { + SnapshotValue::String(new_snapshot_value) => { + // apply filters if they are available + #[cfg(feature = "filters")] + let new_snapshot_value = + Settings::with(|settings| settings.filters().apply_to(new_snapshot_value)); + + new_snapshot_value.into() + } + SnapshotValue::Binary { + write: new_snapshot_value, + extension, + } => { + assert!( + !["new", "_"].contains(&extension), + "this file extension is not allowed", + ); + assert!( + !extension.starts_with("new."), + "file extensions starting with 'new.' are not allowed", + ); + + ctx.pre_create_binary_snapshot(new_snapshot_value, extension)? + } + }; + + let mut new_snapshot = ctx.new_snapshot(content, expr); // memoize the snapshot file if requested. if let Some(ref snapshot_file) = ctx.snapshot_file { @@ -659,17 +718,13 @@ pub fn assert_snapshot( } }); - let pass = ctx - .old_snapshot - .as_ref() - .map(|x| { - if tool_config.require_full_match() { - x.matches_fully(&new_snapshot) - } else { - x.matches(&new_snapshot) - } - }) - .unwrap_or(false); + let pass = ctx.old_snapshot.as_ref().map_or(Ok(false), |x| { + if tool_config.require_full_match() { + x.matches_fully(&new_snapshot) + } else { + x.matches(&new_snapshot) + } + })?; if pass { ctx.cleanup_passing()?; @@ -678,12 +733,20 @@ pub fn assert_snapshot( tool_config.snapshot_update(), crate::env::SnapshotUpdate::Force ) { - ctx.update_snapshot(new_snapshot)?; + ctx.update_snapshot(&mut new_snapshot)?; + } + + if let SnapshotContents::Binary { + path: new_binary_path, + .. + } = new_snapshot.contents() + { + fs::remove_file(new_binary_path)?; } // otherwise print information and update snapshots. } else { + let update_result = ctx.update_snapshot(&mut new_snapshot)?; print_snapshot_info(&ctx, &new_snapshot); - let update_result = ctx.update_snapshot(new_snapshot)?; finalize_assertion(&ctx, update_result); } diff --git a/insta/src/snapshot.rs b/insta/src/snapshot.rs index a1e1e2b7..c76e11e1 100644 --- a/insta/src/snapshot.rs +++ b/insta/src/snapshot.rs @@ -1,12 +1,13 @@ use std::borrow::Cow; use std::env; use std::error::Error; -use std::fs; +use std::fs::{self, File}; use std::io::{BufRead, BufReader, Write}; use std::path::{Path, PathBuf}; use std::time::{SystemTime, UNIX_EPOCH}; use crate::content::{self, json, yaml, Content}; +use crate::utils::file_eq; lazy_static::lazy_static! { static ref RUN_ID: String = { @@ -130,6 +131,18 @@ impl PendingInlineSnapshot { } } +#[derive(Debug, Clone, PartialEq)] +pub enum SnapshotType { + String, + Binary { extension: String }, +} + +impl Default for SnapshotType { + fn default() -> Self { + SnapshotType::String + } +} + /// Snapshot metadata information. #[derive(Debug, Default, Clone, PartialEq)] pub struct MetaData { @@ -146,6 +159,8 @@ pub struct MetaData { pub(crate) info: Option, /// Reference to the input file. pub(crate) input_file: Option, + /// The type of the snapshot. + pub(crate) snapshot_type: SnapshotType, } impl MetaData { @@ -199,6 +214,13 @@ impl MetaData { let mut expression = None; let mut info = None; let mut input_file = None; + let mut snapshot_type = TmpSnapshotType::String; + let mut extension = None; + + enum TmpSnapshotType { + String, + Binary, + } for (key, value) in map.into_iter() { match key.as_str() { @@ -208,6 +230,15 @@ impl MetaData { Some("expression") => expression = value.as_str().map(Into::into), Some("info") if !value.is_nil() => info = Some(value), Some("input_file") => input_file = value.as_str().map(Into::into), + Some("snapshot_type") => { + snapshot_type = match value.as_str() { + Some("binary") => TmpSnapshotType::Binary, + _ => TmpSnapshotType::String, + } + } + Some("extension") => { + extension = value.as_str().map(Into::into); + } _ => {} } } @@ -219,6 +250,13 @@ impl MetaData { expression, info, input_file, + snapshot_type: match snapshot_type { + TmpSnapshotType::String => SnapshotType::String, + TmpSnapshotType::Binary => SnapshotType::Binary { + extension: extension + .map_or(Err(content::Error::MissingField), |e| Ok(e))?, + }, + }, }) } else { Err(content::Error::UnexpectedDataType.into()) @@ -246,6 +284,16 @@ impl MetaData { fields.push(("input_file", Content::from(input_file))); } + let snapshot_type = Content::from(match self.snapshot_type { + SnapshotType::String => "string", + SnapshotType::Binary { ref extension, .. } => { + fields.push(("extension", Content::from(extension.clone()))); + "binary" + } + }); + + fields.push(("snapshot_type", snapshot_type)); + Content::Struct("MetaData", fields) } @@ -325,14 +373,32 @@ impl Snapshot { rv }; - buf.clear(); - for (idx, line) in f.lines().enumerate() { - let line = line?; - if idx > 0 { - buf.push('\n'); + let contents = match metadata.snapshot_type { + SnapshotType::String => { + buf.clear(); + for (idx, line) in f.lines().enumerate() { + let line = line?; + if idx > 0 { + buf.push('\n'); + } + buf.push_str(&line); + } + + SnapshotContents::String(buf) } - buf.push_str(&line); - } + SnapshotType::Binary { ref extension } => { + let mut path = p.to_path_buf(); + let mut ext = path.extension().unwrap().to_os_string(); + ext.push("."); + ext.push(extension); + path.set_extension(ext); + + SnapshotContents::Binary { + path, + extension: extension.clone(), + } + } + }; let (snapshot_name, module_name) = names_of_path(p); @@ -340,7 +406,7 @@ impl Snapshot { module_name, Some(snapshot_name), metadata, - buf.into(), + contents, )) } @@ -359,6 +425,13 @@ impl Snapshot { } } + pub fn extension(&self) -> Option<&str> { + match self.contents() { + SnapshotContents::Binary { extension, .. } => Some(extension), + _ => None, + } + } + #[cfg(feature = "_cargo_insta_internal")] fn from_content(content: Content) -> Result> { if let Content::Map(map) = content { @@ -373,7 +446,7 @@ impl Snapshot { Some("snapshot_name") => snapshot_name = value.as_str().map(|x| x.to_string()), Some("metadata") => metadata = Some(MetaData::from_content(value)?), Some("snapshot") => { - snapshot = Some(SnapshotContents( + snapshot = Some(SnapshotContents::String( value .as_str() .ok_or(content::Error::UnexpectedDataType)? @@ -401,7 +474,10 @@ impl Snapshot { fields.push(("snapshot_name", Content::from(name))); } fields.push(("metadata", self.metadata.as_content())); - fields.push(("snapshot", Content::from(self.snapshot.0.as_str()))); + + if let Some(content) = self.snapshot.as_str() { + fields.push(("snapshot", Content::from(content))); + } Content::Struct("Content", fields) } @@ -427,31 +503,62 @@ impl Snapshot { } /// Snapshot contents match another snapshot's. - pub fn matches(&self, other: &Snapshot) -> bool { - self.contents() == other.contents() + pub fn matches(&self, other: &Snapshot) -> Result> { + Ok(match (self.contents(), other.contents()) { + (SnapshotContents::String(this), SnapshotContents::String(other)) => { + trim_content_str(this) == trim_content_str(other) + } + ( + SnapshotContents::Binary { + path: this, + extension: this_ext, + }, + SnapshotContents::Binary { + path: other, + extension: other_ext, + }, + ) => { + if this_ext != other_ext { + return Ok(false); + } + + let mut this = File::open(this)?; + let mut other = File::open(other)?; + + file_eq(&mut this, &mut other)? + } + _ => false, + }) } /// Snapshot contents _and_ metadata match another snapshot's. - pub fn matches_fully(&self, other: &Snapshot) -> bool { - self.snapshot.matches_fully(&other.snapshot) - && self.metadata.trim_for_persistence() == other.metadata.trim_for_persistence() + pub fn matches_fully(&self, other: &Snapshot) -> Result> { + Ok(self.matches(other)? + // If they're both None in the binary case that is also ok since self.matches already + // takes care of matching those contents exactly. + && self.snapshot.as_str_exact() == other.snapshot.as_str_exact() + && self.metadata.trim_for_persistence() == other.metadata.trim_for_persistence()) } /// The snapshot contents as a &str - pub fn contents_str(&self) -> &str { + pub fn contents_str(&self) -> Option<&str> { self.snapshot.as_str() } fn serialize_snapshot(&self, md: &MetaData) -> String { let mut buf = yaml::to_string(&md.as_content()); buf.push_str("---\n"); - buf.push_str(self.contents_str()); - buf.push('\n'); + + if let Some(contents_str) = self.contents_str() { + buf.push_str(contents_str); + buf.push('\n'); + } + buf } fn save_with_metadata( - &self, + &mut self, path: &Path, ref_file: Option<&Path>, md: &MetaData, @@ -462,11 +569,13 @@ impl Snapshot { let serialized_snapshot = self.serialize_snapshot(md); + let ref_path = ref_file.unwrap_or(path); + // check the reference file for contents. Note that we always want to // compare snapshots that were trimmed to persistence here. This is a // stricter check than even `matches_fully`, since it's comparing the // exact contents of the file. - if let Ok(old) = fs::read_to_string(ref_file.unwrap_or(path)) { + if let Ok(old) = fs::read_to_string(ref_path) { let persisted = match md.trim_for_persistence() { Cow::Owned(trimmed) => Cow::Owned(self.serialize_snapshot(&trimmed)), Cow::Borrowed(trimmed) => { @@ -477,12 +586,48 @@ impl Snapshot { Cow::Borrowed(&serialized_snapshot) } }; - if old == persisted.as_str() { + + if old == persisted.as_str() + // Either both the metadata and the binary get written or none of the two. + && if let SnapshotContents::Binary { .. } = &self.snapshot { + let old_snapshot = Snapshot::from_file(ref_path)?; + + self.matches(&old_snapshot)? + } else { + true + } + { return Ok(false); } } + // make sure to remove any previous binary files with different extensions. + if let Ok(current) = Snapshot::from_file(path) { + if let SnapshotContents::Binary { path, .. } = current.contents() { + fs::remove_file(path)?; + } + } + fs::write(path, serialized_snapshot)?; + + if let SnapshotContents::Binary { + path: binary_path, + extension, + } = &mut self.snapshot + { + let new_path = path.with_extension({ + // The snapshot file should always have an extension, so this unwrap is fine. + let mut new_extension = path.extension().unwrap().to_os_string(); + new_extension.push("."); + new_extension.push(extension); + new_extension + }); + + fs::rename(&*binary_path, &new_path)?; + + *binary_path = new_path; + } + Ok(true) } @@ -491,8 +636,12 @@ impl Snapshot { /// Returns `true` if the snapshot was saved. This will return `false` if there /// was already a snapshot with matching contents. #[doc(hidden)] - pub fn save(&self, path: &Path) -> Result> { - self.save_with_metadata(path, None, &self.metadata.trim_for_persistence()) + pub fn save(&mut self, path: &Path) -> Result> { + self.save_with_metadata( + path, + None, + &self.metadata.trim_for_persistence().into_owned(), + ) } /// Same as `save` but instead of writing a normal snapshot file this will write @@ -500,97 +649,139 @@ impl Snapshot { /// /// If the existing snapshot matches the new file, then `None` is returned, otherwise /// the name of the new snapshot file. - pub(crate) fn save_new(&self, path: &Path) -> Result, Box> { + pub(crate) fn save_new(&mut self, path: &Path) -> Result, Box> { let new_path = path.to_path_buf().with_extension("snap.new"); - if self.save_with_metadata(&new_path, Some(path), &self.metadata)? { + if self.save_with_metadata(&new_path, Some(path), &self.metadata.clone())? { Ok(Some(new_path)) } else { Ok(None) } } + + pub fn cleanup_extra_files(snapshot: Option<&Self>, path: &Path) -> Result<(), Box> { + let binary_file_name = snapshot.and_then(|snapshot| match &snapshot.contents() { + SnapshotContents::String(_) => None, + SnapshotContents::Binary { path, .. } => Some(path.file_name().unwrap()), + }); + + let file_name = path.file_name().unwrap(); + + for entry in path.parent().unwrap().read_dir()? { + let entry = entry?; + let entry_file_name = entry.file_name(); + if entry_file_name + .as_encoded_bytes() + .starts_with(file_name.as_encoded_bytes()) + && entry_file_name != file_name + && binary_file_name.map_or(true, |x| x != entry_file_name) + { + std::fs::remove_file(entry.path())?; + } + } + + Ok(()) + } } /// The contents of a Snapshot -// Could be Cow, but I think limited savings -#[derive(Debug, Clone)] -pub struct SnapshotContents(String); +#[derive(Debug, Clone, PartialEq)] +pub enum SnapshotContents { + // Could be Cow, but I think limited savings + String(String), + Binary { path: PathBuf, extension: String }, +} impl SnapshotContents { pub fn from_inline(value: &str) -> SnapshotContents { - SnapshotContents(get_inline_snapshot_value(value)) + SnapshotContents::String(get_inline_snapshot_value(value)) + } + + pub fn is_str(&self) -> bool { + matches!(self, SnapshotContents::String(_)) + } + + pub fn is_binary(&self) -> bool { + matches!(self, SnapshotContents::Binary { .. }) } /// Returns the snapshot contents as string with surrounding whitespace removed. - pub fn as_str(&self) -> &str { - let out = self.0.trim_start_matches(['\r', '\n']).trim_end(); - // Old inline snapshots have `---` at the start, so this strips that if - // it exists. Soon we can start printing a warning and then eventually - // remove it in the next version. - match out.strip_prefix("---\n") { - Some(s) => s, - None => out, + pub fn as_str(&self) -> Option<&str> { + match self { + SnapshotContents::String(content) => { + let out = trim_content_str(content); + // Old inline snapshots have `---` at the start, so this strips that if + // it exists. Soon we can start printing a warning and then eventually + // remove it in the next version. + Some(match out.strip_prefix("---\n") { + Some(s) => s, + None => out, + }) + } + _ => None, } } /// Returns the snapshot contents as string without any trimming. - pub fn as_str_exact(&self) -> &str { - self.0.as_str() + pub fn as_str_exact(&self) -> Option<&str> { + match self { + SnapshotContents::String(content) => Some(content.as_str()), + _ => None, + } } - pub fn matches_fully(&self, other: &SnapshotContents) -> bool { - self.as_str_exact() == other.as_str_exact() - } + pub fn to_inline(&self, indentation: usize) -> Option { + if let SnapshotContents::String(ref contents) = self { + let mut out = String::new(); + + // We don't technically need to escape on newlines, but it reduces diffs + let is_escape = contents.contains(['\\', '"', '\n']); + // Escape the string if needed, with `r#`, using with 1 more `#` than + // the maximum number of existing contiguous `#`. + let delimiter = if is_escape { + let max_contiguous_hash = contents + .split(|c| c != '#') + .map(|group| group.len()) + .max() + .unwrap_or(0); + out.push('r'); + "#".repeat(max_contiguous_hash + 1) + } else { + "".to_string() + }; - pub fn to_inline(&self, indentation: usize) -> String { - let contents = &self.0; - let mut out = String::new(); - - // We don't technically need to escape on newlines, but it reduces diffs - let is_escape = contents.contains(['\\', '"', '\n']); - // Escape the string if needed, with `r#`, using with 1 more `#` than - // the maximum number of existing contiguous `#`. - let delimiter = if is_escape { - let max_contiguous_hash = contents - .split(|c| c != '#') - .map(|group| group.len()) - .max() - .unwrap_or(0); - out.push('r'); - "#".repeat(max_contiguous_hash + 1) - } else { - "".to_string() - }; + out.push_str(&delimiter); + out.push('"'); + + // if we have more than one line we want to change into the block + // representation mode + if contents.contains('\n') { + out.extend( + contents + .lines() + // newline needs to be at the start, since we don't want the end + // finishing with a newline - the closing suffix should be on the same line + .map(|l| { + format!( + "\n{:width$}{l}", + "", + width = if l.is_empty() { 0 } else { indentation }, + l = l + ) + }) + // `lines` removes the final line ending - add back + .chain(Some(format!("\n{:width$}", "", width = indentation))), + ); + } else { + out.push_str(contents); + } + + out.push('"'); + out.push_str(&delimiter); - out.push_str(&delimiter); - out.push('"'); - - // if we have more than one line we want to change into the block - // representation mode - if contents.contains('\n') { - out.extend( - contents - .lines() - // newline needs to be at the start, since we don't want the end - // finishing with a newline - the closing suffix should be on the same line - .map(|l| { - format!( - "\n{:width$}{l}", - "", - width = if l.is_empty() { 0 } else { indentation }, - l = l - ) - }) - // `lines` removes the final line ending - add back - .chain(Some(format!("\n{:width$}", "", width = indentation))), - ); + Some(out) } else { - out.push_str(contents); + None } - - out.push('"'); - out.push_str(&delimiter); - - out } } @@ -606,27 +797,19 @@ impl<'a> From> for SnapshotContents { impl From<&str> for SnapshotContents { fn from(value: &str) -> SnapshotContents { // make sure we have unix newlines consistently - SnapshotContents(value.replace("\r\n", "\n")) + SnapshotContents::String(value.replace("\r\n", "\n")) } } impl From for SnapshotContents { fn from(value: String) -> SnapshotContents { // make sure we have unix newlines consistently - SnapshotContents(value.replace("\r\n", "\n")) - } -} - -impl From for String { - fn from(value: SnapshotContents) -> String { - value.0 + SnapshotContents::String(value.replace("\r\n", "\n")) } } -impl PartialEq for SnapshotContents { - fn eq(&self, other: &Self) -> bool { - self.as_str() == other.as_str() - } +fn trim_content_str(content: &str) -> &str { + content.trim_start_matches(['\r', '\n']).trim_end() } fn count_leading_spaces(value: &str) -> usize { @@ -765,14 +948,16 @@ fn get_inline_snapshot_value(frozen_value: &str) -> String { #[test] fn test_snapshot_contents() { use similar_asserts::assert_eq; - let snapshot_contents = SnapshotContents("testing".to_string()); - assert_eq!(snapshot_contents.to_inline(0), r#""testing""#); + let snapshot_contents = SnapshotContents::String("testing".to_string()); + assert_eq!(snapshot_contents.to_inline(0).unwrap(), r#""testing""#); let t = &" a b"[1..]; assert_eq!( - SnapshotContents(t.to_string()).to_inline(0), + SnapshotContents::String(t.to_string()) + .to_inline(0) + .unwrap(), r##"r#" a b @@ -780,12 +965,13 @@ b ); assert_eq!( - SnapshotContents( + SnapshotContents::String( "a b" .to_string() ) - .to_inline(4), + .to_inline(4) + .unwrap(), r##"r#" a b @@ -796,7 +982,9 @@ b" a b"[1..]; assert_eq!( - SnapshotContents(t.to_string()).to_inline(0), + SnapshotContents::String(t.to_string()) + .to_inline(0) + .unwrap(), r##"r#" a b @@ -808,7 +996,9 @@ a b"[1..]; assert_eq!( - SnapshotContents(t.to_string()).to_inline(4), + SnapshotContents::String(t.to_string()) + .to_inline(4) + .unwrap(), r##"r#" a @@ -820,24 +1010,38 @@ b"[1..]; ab "[1..]; assert_eq!( - SnapshotContents(t.to_string()).to_inline(0), + SnapshotContents::String(t.to_string()) + .to_inline(0) + .unwrap(), r##"r#" ab "#"## ); let t = "ab"; - assert_eq!(SnapshotContents(t.to_string()).to_inline(0), r#""ab""#); + assert_eq!( + SnapshotContents::String(t.to_string()) + .to_inline(0) + .unwrap(), + r#""ab""# + ); } #[test] fn test_snapshot_contents_hashes() { let t = "a###b"; - assert_eq!(SnapshotContents(t.to_string()).to_inline(0), r#""a###b""#); + assert_eq!( + SnapshotContents::String(t.to_string()) + .to_inline(0) + .unwrap(), + r#""a###b""# + ); let t = "a\n\\###b"; assert_eq!( - SnapshotContents(t.to_string()).to_inline(0), + SnapshotContents::String(t.to_string()) + .to_inline(0) + .unwrap(), r#####"r####" a \###b diff --git a/insta/src/utils.rs b/insta/src/utils.rs index 35a39da6..01d91dbd 100644 --- a/insta/src/utils.rs +++ b/insta/src/utils.rs @@ -1,7 +1,9 @@ use std::{ borrow::Cow, env, - io::Write, + fs::File, + io::{BufReader, Read, Write}, + ops::ControlFlow, path::Path, process::{Command, Stdio}, }; @@ -108,6 +110,32 @@ pub fn format_rust_expression(value: &str) -> Cow<'_, str> { Cow::Borrowed(value) } +pub fn file_eq(a: &mut File, b: &mut File) -> std::io::Result { + if a.metadata()?.len() != b.metadata()?.len() { + return Ok(false); + } + + let a = BufReader::new(a).bytes(); + let b = BufReader::new(b).bytes(); + + match a.zip(b).try_for_each(|pair| match pair { + (Ok(a), Ok(b)) => { + if a == b { + ControlFlow::Continue(()) + } else { + ControlFlow::Break(Ok(())) + } + } + (Ok(_), Err(e)) | (Err(e), _) => ControlFlow::Break(Err(e)), + }) { + ControlFlow::Continue(_) => Ok(true), + ControlFlow::Break(r) => { + r?; + Ok(false) + } + } +} + #[test] fn test_format_rust_expression() { use crate::assert_snapshot; diff --git a/insta/tests/snapshots/test_binary__binary_snapshot.snap b/insta/tests/snapshots/test_binary__binary_snapshot.snap new file mode 100644 index 00000000..56abd2ba --- /dev/null +++ b/insta/tests/snapshots/test_binary__binary_snapshot.snap @@ -0,0 +1,6 @@ +--- +source: insta/tests/test_binary.rs +expression: "|file| { file.write_all(b\"test\").unwrap(); }" +extension: txt +snapshot_type: binary +--- diff --git a/insta/tests/snapshots/test_binary__binary_snapshot.snap.txt b/insta/tests/snapshots/test_binary__binary_snapshot.snap.txt new file mode 100644 index 00000000..30d74d25 --- /dev/null +++ b/insta/tests/snapshots/test_binary__binary_snapshot.snap.txt @@ -0,0 +1 @@ +test \ No newline at end of file diff --git a/insta/tests/snapshots/test_binary__file_empty_extension.snap b/insta/tests/snapshots/test_binary__file_empty_extension.snap new file mode 100644 index 00000000..977d34ed --- /dev/null +++ b/insta/tests/snapshots/test_binary__file_empty_extension.snap @@ -0,0 +1,6 @@ +--- +source: insta/tests/test_binary.rs +expression: "|file| { file.write_all(b\"test\").unwrap(); }" +extension: "" +snapshot_type: binary +--- diff --git a/insta/tests/snapshots/test_binary__file_empty_extension.snap. b/insta/tests/snapshots/test_binary__file_empty_extension.snap. new file mode 100644 index 00000000..30d74d25 --- /dev/null +++ b/insta/tests/snapshots/test_binary__file_empty_extension.snap. @@ -0,0 +1 @@ +test \ No newline at end of file diff --git a/insta/tests/snapshots/test_binary__file_extension_collision.snap b/insta/tests/snapshots/test_binary__file_extension_collision.snap new file mode 100644 index 00000000..2ff4997a --- /dev/null +++ b/insta/tests/snapshots/test_binary__file_extension_collision.snap @@ -0,0 +1,6 @@ +--- +source: insta/tests/test_binary.rs +expression: "|file| { file.write_all(b\"test\").unwrap(); }" +extension: snap +snapshot_type: binary +--- diff --git a/insta/tests/snapshots/test_binary__file_extension_collision.snap.snap b/insta/tests/snapshots/test_binary__file_extension_collision.snap.snap new file mode 100644 index 00000000..30d74d25 --- /dev/null +++ b/insta/tests/snapshots/test_binary__file_extension_collision.snap.snap @@ -0,0 +1 @@ +test \ No newline at end of file diff --git a/insta/tests/snapshots/test_binary__multipart_extension.snap b/insta/tests/snapshots/test_binary__multipart_extension.snap new file mode 100644 index 00000000..fc32d772 --- /dev/null +++ b/insta/tests/snapshots/test_binary__multipart_extension.snap @@ -0,0 +1,6 @@ +--- +source: insta/tests/test_binary.rs +expression: "|file| { file.write_all(b\"test\").unwrap(); }" +extension: tar.gz +snapshot_type: binary +--- diff --git a/insta/tests/snapshots/test_binary__multipart_extension.snap.tar.gz b/insta/tests/snapshots/test_binary__multipart_extension.snap.tar.gz new file mode 100644 index 00000000..30d74d25 --- /dev/null +++ b/insta/tests/snapshots/test_binary__multipart_extension.snap.tar.gz @@ -0,0 +1 @@ +test \ No newline at end of file diff --git a/insta/tests/test_binary.rs b/insta/tests/test_binary.rs new file mode 100644 index 00000000..58566a19 --- /dev/null +++ b/insta/tests/test_binary.rs @@ -0,0 +1,49 @@ +use std::io::Write; + +#[test] +fn test_binary_snapshot() { + insta::assert_binary_snapshot!("txt", |file| { + file.write_all(b"test").unwrap(); + }); +} + +#[test] +fn test_file_extension_collision() { + // ubuntu snap packages also have the .snap extension so let's make sure that doesn't cause + // problems + insta::assert_binary_snapshot!("snap", |file| { + file.write_all(b"test").unwrap(); + }); +} + +#[test] +fn test_file_empty_extension() { + insta::assert_binary_snapshot!("", |file| { + file.write_all(b"test").unwrap(); + }); +} + +#[test] +#[should_panic(expected = "this file extension is not allowed")] +fn test_new_extension() { + insta::assert_binary_snapshot!("new", |_| {}); +} + +#[test] +#[should_panic(expected = "this file extension is not allowed")] +fn test_underscore_extension() { + insta::assert_binary_snapshot!("_", |_| {}); +} + +#[test] +#[should_panic(expected = "file extensions starting with 'new.' are not allowed")] +fn test_extension_starting_with_new() { + insta::assert_binary_snapshot!("new.gz", |_| {}); +} + +#[test] +fn test_multipart_extension() { + insta::assert_binary_snapshot!("tar.gz", |file| { + file.write_all(b"test").unwrap(); + }); +}