From b242de31d1199350f772a6cc892a442fb5eaf105 Mon Sep 17 00:00:00 2001 From: allenyuchen Date: Thu, 9 Jan 2025 19:09:37 +0800 Subject: [PATCH 1/2] feat: Support diffing text "binary" files --- insta/src/output.rs | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/insta/src/output.rs b/insta/src/output.rs index d63d30c2..03f56dc0 100644 --- a/insta/src/output.rs +++ b/insta/src/output.rs @@ -184,16 +184,30 @@ impl<'a> SnapshotPrinter<'a> { self.old_snapshot.as_ref().map(|o| o.contents()), self.new_snapshot.contents(), ) { - (Some(SnapshotContents::Binary(_)) | None, SnapshotContents::Text(new)) => { - Some((None, Some(new.to_string()))) + (None, SnapshotContents::Binary(new)) => String::from_utf8(new.to_vec()) + .ok() + .map(|new_str| (None, Some(new_str))), + (None, SnapshotContents::Text(new)) => Some((None, Some(new.to_string()))), + (Some(SnapshotContents::Binary(old)), SnapshotContents::Text(new)) => { + Some((String::from_utf8(old.to_vec()).ok(), Some(new.to_string()))) } - (Some(SnapshotContents::Text(old)), SnapshotContents::Binary { .. }) => { - Some((Some(old.to_string()), None)) + (Some(SnapshotContents::Text(old)), SnapshotContents::Binary(new)) => { + Some((Some(old.to_string()), String::from_utf8(new.to_vec()).ok())) } (Some(SnapshotContents::Text(old)), SnapshotContents::Text(new)) => { Some((Some(old.to_string()), Some(new.to_string()))) } - _ => None, + (Some(SnapshotContents::Binary(old)), SnapshotContents::Binary(new)) => { + match ( + String::from_utf8(old.to_vec()).ok(), + String::from_utf8(new.to_vec()).ok(), + ) { + (None, None) => None, + (Some(old), None) => Some((Some(old), None)), + (None, Some(new)) => Some((None, Some(new))), + (Some(old), Some(new)) => Some((Some(old), Some(new))), + } + } } { let old_text = old.as_deref().unwrap_or(""); let new_text = new.as_deref().unwrap_or(""); @@ -204,14 +218,18 @@ impl<'a> SnapshotPrinter<'a> { .timeout(Duration::from_millis(500)) .diff_lines(old_text, new_text); - if old.is_some() { + if old.is_some() + && !self + .old_snapshot + .map_or(false, |s| s.contents().is_binary()) + { println!( "{}", style(format_args!("-{}", self.old_snapshot_hint)).red() ); } - if new.is_some() { + if new.is_some() && !self.new_snapshot.contents().is_binary() { println!( "{}", style(format_args!("+{}", self.new_snapshot_hint)).green() From 796ca51a6e79bf7c43b91143708d2a32b4253d19 Mon Sep 17 00:00:00 2001 From: allenyuchen Date: Fri, 14 Feb 2025 16:41:02 -0800 Subject: [PATCH 2/2] feat: Add configuration for forcing text diff on binary files --- cargo-insta/src/cli.rs | 18 ++++++++- insta/src/env.rs | 16 ++++++++ insta/src/output.rs | 89 ++++++++++++++++++++++-------------------- insta/src/runtime.rs | 15 +++++-- 4 files changed, 90 insertions(+), 48 deletions(-) diff --git a/cargo-insta/src/cli.rs b/cargo-insta/src/cli.rs index 7d3e18db..df7b88a9 100644 --- a/cargo-insta/src/cli.rs +++ b/cargo-insta/src/cli.rs @@ -117,6 +117,9 @@ struct ProcessCommand { /// Do not print to stdout. #[arg(short = 'q', long)] quiet: bool, + /// Force files to be considered as text files for diff output. + #[arg(long)] + text: bool, } #[derive(Args, Debug)] @@ -216,6 +219,9 @@ struct TestCommand { test_runner: TestRunner, #[arg(long)] test_runner_fallback: Option, + /// Force files to be considered as text files for diff output. + #[arg(long)] + text: bool, /// Delete unreferenced snapshots after a successful test run (deprecated) #[arg(long, hide = true)] delete_unreferenced_snapshots: bool, @@ -248,6 +254,9 @@ struct ShowCommand { target_args: TargetArgs, /// The path to the snapshot file. path: PathBuf, + /// Force files to be considered as text files for diff output. + #[arg(long)] + text: bool, } #[allow(clippy::too_many_arguments)] @@ -261,6 +270,7 @@ fn query_snapshot( i: usize, n: usize, snapshot_file: Option<&Path>, + force_text: bool, show_info: &mut bool, show_diff: &mut bool, ) -> Result> { @@ -276,7 +286,7 @@ fn query_snapshot( &pkg.version, ); - let mut printer = SnapshotPrinter::new(workspace_root, old, new); + let mut printer = SnapshotPrinter::new(workspace_root, old, new, force_text); printer.set_snapshot_file(snapshot_file); printer.set_line(line); printer.set_show_info(*show_info); @@ -521,6 +531,7 @@ fn process_snapshots( snapshot_filter: Option<&[String]>, loc: &LocationInfo<'_>, op: Option, + force_text: bool, ) -> Result<(), Box> { let term = Term::stdout(); @@ -583,6 +594,7 @@ fn process_snapshots( num, snapshot_count, snapshot_file.as_deref(), + force_text, &mut show_info, &mut show_diff, )?, @@ -769,6 +781,7 @@ fn test_run(mut cmd: TestCommand, color: ColorWhen) -> Result<(), Box } else { None }, + cmd.text, )? } else { let (snapshot_containers, roots) = load_snapshot_containers(&loc)?; @@ -1128,7 +1141,7 @@ fn get_cargo_nextest_command() -> std::process::Command { fn show_cmd(cmd: ShowCommand) -> Result<(), Box> { let loc = handle_target_args(&cmd.target_args, &[])?; let snapshot = Snapshot::from_file(&cmd.path)?; - let mut printer = SnapshotPrinter::new(&loc.workspace_root, None, &snapshot); + let mut printer = SnapshotPrinter::new(&loc.workspace_root, None, &snapshot, cmd.text); printer.set_snapshot_file(Some(&cmd.path)); printer.set_show_info(true); printer.set_show_diff(false); @@ -1287,6 +1300,7 @@ pub(crate) fn run() -> Result<(), Box> { Command::Reject(_) => Some(Operation::Reject), _ => unreachable!(), }, + cmd.text, ) } Command::Test(cmd) => test_run(cmd, opts.color.unwrap_or(ColorWhen::Auto)), diff --git a/insta/src/env.rs b/insta/src/env.rs index 49fa7f05..7079c8c7 100644 --- a/insta/src/env.rs +++ b/insta/src/env.rs @@ -130,6 +130,7 @@ pub struct ToolConfig { require_full_match: bool, output: OutputBehavior, snapshot_update: SnapshotUpdate, + force_text: bool, #[cfg(feature = "glob")] glob_fail_fast: bool, #[cfg(feature = "_cargo_insta_internal")] @@ -266,6 +267,16 @@ impl ToolConfig { _ => return Err(Error::Env("INSTA_UPDATE")), } }, + force_text: { + match env::var("INSTA_FORCE_TEXT").as_deref() { + Err(_) | Ok("") => resolve(&cfg, &["behavior", "force_text"]) + .and_then(|x| x.as_bool()) + .unwrap_or(false), + Ok("0") => false, + Ok("1") => true, + _ => return Err(Error::Env("INSTA_FORCE_TEXT")), + } + }, #[cfg(feature = "glob")] glob_fail_fast: match env::var("INSTA_GLOB_FAIL_FAST").as_deref() { Err(_) | Ok("") => resolve(&cfg, &["behavior", "glob_fail_fast"]) @@ -349,6 +360,11 @@ impl ToolConfig { self.snapshot_update } + /// Returns whether snapshots should treat files as text. + pub fn force_text(&self) -> bool { + self.force_text + } + /// Returns whether the glob should fail fast, as snapshot failures within the glob macro will appear only at the end of execution unless `glob_fail_fast` is set. #[cfg(feature = "glob")] pub fn glob_fail_fast(&self) -> bool { diff --git a/insta/src/output.rs b/insta/src/output.rs index 03f56dc0..46ef8bb0 100644 --- a/insta/src/output.rs +++ b/insta/src/output.rs @@ -14,6 +14,7 @@ pub struct SnapshotPrinter<'a> { new_snapshot: &'a Snapshot, old_snapshot_hint: &'a str, new_snapshot_hint: &'a str, + force_text: bool, show_info: bool, show_diff: bool, title: Option<&'a str>, @@ -26,6 +27,7 @@ impl<'a> SnapshotPrinter<'a> { workspace_root: &'a Path, old_snapshot: Option<&'a Snapshot>, new_snapshot: &'a Snapshot, + force_text: bool, ) -> SnapshotPrinter<'a> { SnapshotPrinter { workspace_root, @@ -33,6 +35,7 @@ impl<'a> SnapshotPrinter<'a> { new_snapshot, old_snapshot_hint: "old snapshot", new_snapshot_hint: "new results", + force_text, show_info: false, show_diff: false, title: None, @@ -66,6 +69,19 @@ impl<'a> SnapshotPrinter<'a> { self.snapshot_file = file; } + fn get_snapshot_contents_text(&self, snapshot_contents: &SnapshotContents) -> Option { + match snapshot_contents { + SnapshotContents::Text(contents) => Some(contents.to_string()), + SnapshotContents::Binary(contents) => { + if self.force_text { + Some(String::from_utf8_lossy(contents).to_string()) + } else { + None + } + } + } + } + pub fn print(&self) { if let Some(title) = self.title { let width = term_width(); @@ -109,29 +125,23 @@ impl<'a> SnapshotPrinter<'a> { } println!("Snapshot Contents:"); - match self.new_snapshot.contents() { - SnapshotContents::Text(new_contents) => { - let new_contents = new_contents.to_string(); - - 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)); - } - SnapshotContents::Binary(_) => { - println!( - "{}", - encode_file_link_escape( - &self - .new_snapshot - .build_binary_path( - self.snapshot_file.unwrap().with_extension("snap.new") - ) - .unwrap() - ) - ); + let new_contents = self.get_snapshot_contents_text(self.new_snapshot.contents()); + if let Some(new_contents) = 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)); + } else { + println!( + "{}", + encode_file_link_escape( + &self + .new_snapshot + .build_binary_path(self.snapshot_file.unwrap().with_extension("snap.new")) + .unwrap() + ) + ); } } @@ -184,28 +194,21 @@ impl<'a> SnapshotPrinter<'a> { self.old_snapshot.as_ref().map(|o| o.contents()), self.new_snapshot.contents(), ) { - (None, SnapshotContents::Binary(new)) => String::from_utf8(new.to_vec()) - .ok() - .map(|new_str| (None, Some(new_str))), - (None, SnapshotContents::Text(new)) => Some((None, Some(new.to_string()))), - (Some(SnapshotContents::Binary(old)), SnapshotContents::Text(new)) => { - Some((String::from_utf8(old.to_vec()).ok(), Some(new.to_string()))) - } - (Some(SnapshotContents::Text(old)), SnapshotContents::Binary(new)) => { - Some((Some(old.to_string()), String::from_utf8(new.to_vec()).ok())) - } - (Some(SnapshotContents::Text(old)), SnapshotContents::Text(new)) => { - Some((Some(old.to_string()), Some(new.to_string()))) + (None, new) => { + let new_text = self.get_snapshot_contents_text(new); + if new_text.is_some() { + Some((None, new_text)) + } else { + None + } } - (Some(SnapshotContents::Binary(old)), SnapshotContents::Binary(new)) => { - match ( - String::from_utf8(old.to_vec()).ok(), - String::from_utf8(new.to_vec()).ok(), - ) { - (None, None) => None, - (Some(old), None) => Some((Some(old), None)), - (None, Some(new)) => Some((None, Some(new))), - (Some(old), Some(new)) => Some((Some(old), Some(new))), + (Some(old), new) => { + let old_text = self.get_snapshot_contents_text(old); + let new_text = self.get_snapshot_contents_text(new); + if old_text.is_some() || new_text.is_some() { + Some((old_text, new_text)) + } else { + None } } } { diff --git a/insta/src/runtime.rs b/insta/src/runtime.rs index f2a0ff18..74ae766f 100644 --- a/insta/src/runtime.rs +++ b/insta/src/runtime.rs @@ -593,8 +593,12 @@ impl<'a> SnapshotAssertionContext<'a> { /// This prints the information about the snapshot fn print_snapshot_info(&self, new_snapshot: &Snapshot) { - let mut printer = - SnapshotPrinter::new(self.workspace, self.old_snapshot.as_ref(), new_snapshot); + let mut printer = SnapshotPrinter::new( + self.workspace, + self.old_snapshot.as_ref(), + new_snapshot, + self.tool_config.force_text(), + ); printer.set_line(Some(self.assertion_line)); printer.set_snapshot_file(self.snapshot_file.as_deref()); printer.set_title(Some("Snapshot Summary")); @@ -708,7 +712,12 @@ fn record_snapshot_duplicate( if let Some(prev_snapshot) = results.get(key) { if prev_snapshot.contents() != snapshot.contents() { println!("Snapshots in allow-duplicates block do not match."); - let mut printer = SnapshotPrinter::new(ctx.workspace, Some(prev_snapshot), snapshot); + let mut printer = SnapshotPrinter::new( + ctx.workspace, + Some(prev_snapshot), + snapshot, + ctx.tool_config.force_text(), + ); printer.set_line(Some(ctx.assertion_line)); printer.set_snapshot_file(ctx.snapshot_file.as_deref()); printer.set_title(Some("Differences in Block"));