From a07a24471e9e0621419e28062abf62f75e8b2aef Mon Sep 17 00:00:00 2001 From: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Date: Sat, 5 Oct 2024 00:43:51 -0700 Subject: [PATCH 01/22] Fix & test the check for hidden / ignored snapshots (#637) Also some usual refactoring --- cargo-insta/src/cli.rs | 84 +++++++++++-------- cargo-insta/src/container.rs | 6 +- cargo-insta/src/inline.rs | 3 +- cargo-insta/tests/main.rs | 157 +++++++++++++++++++++++++++++++++++ 4 files changed, 213 insertions(+), 37 deletions(-) diff --git a/cargo-insta/src/cli.rs b/cargo-insta/src/cli.rs index d9bbe449..8fc9f450 100644 --- a/cargo-insta/src/cli.rs +++ b/cargo-insta/src/cli.rs @@ -10,6 +10,7 @@ use insta::Snapshot; use insta::_cargo_insta_support::{ is_ci, SnapshotPrinter, SnapshotUpdate, TestRunner, ToolConfig, UnreferencedSnapshots, }; +use itertools::Itertools; use semver::Version; use serde::Serialize; use uuid::Uuid; @@ -516,7 +517,15 @@ fn process_snapshots( if !quiet { println!("{}: no snapshots to review", style("done").bold()); if loc.tool_config.review_warn_undiscovered() { - show_undiscovered_hint(loc.find_flags, &snapshot_containers, &roots, &loc.exts); + show_undiscovered_hint( + loc.find_flags, + &snapshot_containers + .iter() + .map(|x| x.0.clone()) + .collect_vec(), + &roots, + &loc.exts, + ); } } return Ok(()); @@ -732,7 +741,8 @@ fn test_run(mut cmd: TestCommand, color: ColorWhen) -> Result<(), Box )? } else { let (snapshot_containers, roots) = load_snapshot_containers(&loc)?; - let snapshot_count = snapshot_containers.iter().map(|x| x.0.len()).sum::(); + let snapshot_containers = snapshot_containers.into_iter().map(|x| x.0).collect_vec(); + let snapshot_count = snapshot_containers.iter().map(|x| x.len()).sum::(); if snapshot_count > 0 { eprintln!( "{}: {} snapshot{} to review", @@ -1160,7 +1170,7 @@ fn pending_snapshots_cmd(cmd: PendingSnapshotsCommand) -> Result<(), Box, extensions: &[&str], ) { @@ -1169,42 +1179,45 @@ fn show_undiscovered_hint( return; } - let mut found_extra = false; let found_snapshots = snapshot_containers .iter() - .filter_map(|x| x.0.snapshot_file()) + .filter_map(|x| x.snapshot_file()) + .map(|x| x.to_path_buf()) .collect::>(); - for root in roots { - for snapshot in make_snapshot_walker( - root, - extensions, - FindFlags { - include_ignored: true, - include_hidden: true, - }, - ) - .filter_map(|e| e.ok()) - .filter(|x| { - let fname = x.file_name().to_string_lossy(); - // TODO: use `extensions` here - fname.ends_with(".snap.new") || fname.ends_with(".pending-snap") - }) { - if !found_snapshots.contains(snapshot.path()) { - found_extra = true; - break; - } - } - } + let all_snapshots: HashSet<_> = roots + .iter() + .flat_map(|root| { + make_snapshot_walker( + root, + extensions, + FindFlags { + include_ignored: true, + include_hidden: true, + }, + ) + .filter_map(|e| e.ok()) + .filter(|x| { + let fname = x.file_name().to_string_lossy(); + extensions + .iter() + .any(|ext| fname.ends_with(&format!(".{}.new", ext))) + || fname.ends_with(".pending-snap") + }) + .map(|x| x.path().to_path_buf()) + }) + .collect(); + + let missed_snapshots = all_snapshots.difference(&found_snapshots).collect_vec(); // we did not find any extra snapshots - if !found_extra { + if missed_snapshots.is_empty() { return; } let (args, paths) = match (find_flags.include_ignored, find_flags.include_hidden) { - (true, false) => ("--include-ignored", "ignored"), - (false, true) => ("--include-hidden", "hidden"), + (false, true) => ("--include-ignored", "ignored"), + (true, false) => ("--include-hidden", "hidden"), (false, false) => ( "--include-ignored and --include-hidden", "ignored or hidden", @@ -1212,13 +1225,18 @@ fn show_undiscovered_hint( (true, true) => unreachable!(), }; - println!( + eprintln!( "{}: {}", style("warning").yellow().bold(), format_args!( - "found undiscovered snapshots in some paths which are not picked up by cargo \ - insta. Use {} if you have snapshots in {} paths.", - args, paths, + "found undiscovered pending snapshots in some paths which are not picked up by cargo \ + insta. Use {} if you have snapshots in {} paths. Files:\n{}", + args, + paths, + missed_snapshots + .iter() + .map(|x| x.display().to_string()) + .join("\n") ) ); } diff --git a/cargo-insta/src/container.rs b/cargo-insta/src/container.rs index 1764cfac..776f8dfd 100644 --- a/cargo-insta/src/container.rs +++ b/cargo-insta/src/container.rs @@ -15,7 +15,7 @@ pub(crate) enum Operation { Skip, } -#[derive(Debug)] +#[derive(Debug, Clone)] pub(crate) struct PendingSnapshot { #[allow(dead_code)] id: usize, @@ -45,9 +45,9 @@ impl PendingSnapshot { /// A snapshot and its immediate context, which loads & saves the snapshot. It /// holds either a single file snapshot, or all the inline snapshots from a /// single rust file. -#[derive(Debug)] +#[derive(Debug, Clone)] pub(crate) struct SnapshotContainer { - // Path of the pending snapshot file (generally a `.new` or `.pending-snap` file) + // Path of the pending snapshot file (generally a `.snap.new` or `.pending-snap` file) pending_path: PathBuf, // Path of the target snapshot file (generally a `.snap` file) target_path: PathBuf, diff --git a/cargo-insta/src/inline.rs b/cargo-insta/src/inline.rs index a08972db..5fdc8097 100644 --- a/cargo-insta/src/inline.rs +++ b/cargo-insta/src/inline.rs @@ -10,13 +10,14 @@ use proc_macro2::TokenTree; use syn::__private::ToTokens; use syn::spanned::Spanned; -#[derive(Debug)] +#[derive(Debug, Clone)] struct InlineSnapshot { start: (usize, usize), end: (usize, usize), indentation: usize, } +#[derive(Clone)] pub(crate) struct FilePatcher { filename: PathBuf, lines: Vec, diff --git a/cargo-insta/tests/main.rs b/cargo-insta/tests/main.rs index 1ea21df7..08c49ec6 100644 --- a/cargo-insta/tests/main.rs +++ b/cargo-insta/tests/main.rs @@ -1977,6 +1977,163 @@ Unused snapshot "); } +#[test] +fn test_hidden_snapshots() { + let test_project = TestFiles::new() + .add_file( + "Cargo.toml", + r#" +[package] +name = "test_hidden_snapshots" +version = "0.1.0" +edition = "2021" + +[dependencies] +insta = { path = '$PROJECT_PATH' } +"# + .to_string(), + ) + .add_file( + "src/lib.rs", + r#" +#[cfg(test)] +mod tests { + #[test] + fn test_snapshot() { + insta::assert_snapshot!("Hello, world!"); + } +} +"# + .to_string(), + ) + .add_file( + "src/snapshots/test_hidden_snapshots__tests__snapshot.snap", + r#"--- +source: src/lib.rs +expression: "\"Hello, world!\"" +--- +Hello, world! +"# + .to_string(), + ) + .add_file( + "src/snapshots/.hidden/hidden_snapshot.snap.new", + r#"--- +source: src/lib.rs +expression: "Hidden snapshot" +--- +Hidden snapshot +"# + .to_string(), + ) + .create_project(); + + // Run test without --include-hidden flag + let output = test_project + .insta_cmd() + .args(["test"]) + .stderr(std::process::Stdio::piped()) + .output() + .unwrap(); + + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("found undiscovered pending snapshots") + && stderr.contains("--include-hidden"), + "{}", + stderr + ); + + // Run test with --include-hidden flag + let output = test_project + .insta_cmd() + .args(["test", "--include-hidden"]) + .stderr(std::process::Stdio::piped()) + .output() + .unwrap(); + + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + !stderr.contains("found undiscovered pending snapshots"), + "{}", + stderr + ); +} + +#[test] +fn test_ignored_snapshots() { + let test_project = TestFiles::new() + .add_file( + "Cargo.toml", + r#" +[package] +name = "test_ignored_snapshots" +version = "0.1.0" +edition = "2021" + +[dependencies] +insta = { path = '$PROJECT_PATH' } +"# + .to_string(), + ) + .add_file( + "src/lib.rs", + r#" +#[test] +fn test_snapshot() { + insta::assert_snapshot!("Hello, world!", @""); +} +"# + .to_string(), + ) + .add_file( + ".gitignore", + r#" +src/ +"# + .to_string(), + ) + .create_project(); + + // We need to init a git repository in the project directory so it will be ignored + let mut git_cmd = Command::new("git"); + git_cmd.current_dir(&test_project.workspace_dir); + git_cmd.args(["init"]); + git_cmd.output().unwrap(); + + // Run test without --include-ignored flag + let output = test_project + .insta_cmd() + // add the `--hidden` to check it's printing the correct warning + .args(["test", "--include-hidden"]) + .stderr(std::process::Stdio::piped()) + .output() + .unwrap(); + + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("found undiscovered pending snapshots") + && stderr.contains("--include-ignored"), + "{}", + stderr + ); + + // Run test with --include-ignored flag + let output = test_project + .insta_cmd() + .args(["test", "--include-ignored"]) + .stderr(std::process::Stdio::piped()) + .output() + .unwrap(); + + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + !stderr.contains("found undiscovered pending snapshots"), + "{}", + stderr + ); +} + #[test] fn test_binary_unreferenced_delete() { let test_project = TestFiles::new() From 915afdc7db6c8b7f13830873e770d62a522c86c2 Mon Sep 17 00:00:00 2001 From: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Date: Sat, 5 Oct 2024 01:38:19 -0700 Subject: [PATCH 02/22] Fix typos (#638) Running `pre-commit` caught a couple things --- cargo-insta/Cargo.toml | 2 +- cargo-insta/src/cli.rs | 2 +- cargo-insta/tests/main.rs | 8 ++++++-- insta/src/runtime.rs | 2 +- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/cargo-insta/Cargo.toml b/cargo-insta/Cargo.toml index f0202d92..044eddf0 100644 --- a/cargo-insta/Cargo.toml +++ b/cargo-insta/Cargo.toml @@ -38,4 +38,4 @@ walkdir = "2.3.1" similar= "2.2.1" itertools = "0.10.0" termcolor = "1.1.2" -os_pipe = "0.9.0" \ No newline at end of file +os_pipe = "0.9.0" diff --git a/cargo-insta/src/cli.rs b/cargo-insta/src/cli.rs index 8fc9f450..4d56ed86 100644 --- a/cargo-insta/src/cli.rs +++ b/cargo-insta/src/cli.rs @@ -469,7 +469,7 @@ fn handle_target_args<'a>( .iter() .map(|x| { if let Some(no_period) = x.strip_prefix(".") { - eprintln!("`{}` supplied as an extenstion. This will use `foo.{}` as file names; likely you want `{}` instead.", x, x, no_period) + eprintln!("`{}` supplied as an extension. This will use `foo.{}` as file names; likely you want `{}` instead.", x, x, no_period) }; x.as_str() }) diff --git a/cargo-insta/tests/main.rs b/cargo-insta/tests/main.rs index 08c49ec6..e2260565 100644 --- a/cargo-insta/tests/main.rs +++ b/cargo-insta/tests/main.rs @@ -11,11 +11,15 @@ /// .args(["test"]) /// .stderr(Stdio::piped()) /// -/// assert!(String::from_utf8_lossy(&output.stderr).contains("info: 2 snapshots to review") +/// assert!( +/// String::from_utf8_lossy(&output.stderr).contains("info: 2 snapshots to review"), +/// "{}", +/// String::from_utf8_lossy(&output.stderr) +/// ); /// ``` /// /// Often we want to see output from the test commands we run here; for example -/// a `dbg!` statement we add while debugging. Cargo by default hides the output +/// a `dbg` statement we add while debugging. Cargo by default hides the output /// of passing tests. /// - Like any test, to forward the output of a passing outer test (i.e. one of /// the `#[test]`s in this file) to the terminal, pass `--nocapture` to the diff --git a/insta/src/runtime.rs b/insta/src/runtime.rs index 5bc0d745..ecbfb9b0 100644 --- a/insta/src/runtime.rs +++ b/insta/src/runtime.rs @@ -476,7 +476,7 @@ impl<'a> SnapshotAssertionContext<'a> { } /// Removes any old .snap.new.* files that belonged to previous pending snapshots. This should - /// only ever remove maxium one file because we do this every time before we create a new + /// only ever remove maximum one file because we do this every time before we create a new /// pending snapshot. pub fn cleanup_previous_pending_binary_snapshots(&self) -> Result<(), Box> { if let Some(ref path) = self.snapshot_file { From d2650c1138bb97edda40ffadce868564545b177f Mon Sep 17 00:00:00 2001 From: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Date: Sat, 5 Oct 2024 12:57:40 -0700 Subject: [PATCH 03/22] Remove some methods from `SnapshotContents` (#640) Since the binary snapshots have merged, there are some methods which return `Some` if the enum is a certain variant. We can just use pattern matching for that. (Note that the code is a bit complicated and has lots of repetition of 'kind'-like fields; but wanted to merge the binary snapshots feature before cleaning up the internals --- cargo-insta/src/cli.rs | 16 ++++++++++------ cargo-insta/src/container.rs | 7 +++++-- insta/src/snapshot.rs | 19 ++----------------- 3 files changed, 17 insertions(+), 25 deletions(-) diff --git a/cargo-insta/src/cli.rs b/cargo-insta/src/cli.rs index 4d56ed86..f2e5a341 100644 --- a/cargo-insta/src/cli.rs +++ b/cargo-insta/src/cli.rs @@ -6,10 +6,10 @@ use std::{env, fs}; use std::{io, process}; use console::{set_colors_enabled, style, Key, Term}; -use insta::Snapshot; use insta::_cargo_insta_support::{ is_ci, SnapshotPrinter, SnapshotUpdate, TestRunner, ToolConfig, UnreferencedSnapshots, }; +use insta::{internals::SnapshotContents, Snapshot}; use itertools::Itertools; use semver::Version; use serde::Serialize; @@ -1140,11 +1140,15 @@ fn pending_snapshots_cmd(cmd: PendingSnapshotsCommand) -> Result<(), Box x.to_string(), + _ => unreachable!(), + }); + let new_snapshot = match snapshot_ref.new.contents() { + SnapshotContents::Text(x) => x.to_string(), + _ => unreachable!(), + }; + let info = if is_inline { SnapshotKey::InlineSnapshot { path: &target_file, diff --git a/cargo-insta/src/container.rs b/cargo-insta/src/container.rs index 776f8dfd..c52a492e 100644 --- a/cargo-insta/src/container.rs +++ b/cargo-insta/src/container.rs @@ -2,9 +2,9 @@ use std::error::Error; use std::fs; use std::path::{Path, PathBuf}; -use insta::Snapshot; pub(crate) use insta::TextSnapshotKind; use insta::_cargo_insta_support::{ContentError, PendingInlineSnapshot}; +use insta::{internals::SnapshotContents, Snapshot}; use crate::inline::FilePatcher; @@ -186,7 +186,10 @@ impl SnapshotContainer { Operation::Accept => { patcher.set_new_content( idx, - snapshot.new.contents().as_string_contents().unwrap(), + match snapshot.new.contents() { + SnapshotContents::Text(c) => c, + _ => unreachable!(), + }, ); did_accept = true; } diff --git a/insta/src/snapshot.rs b/insta/src/snapshot.rs index 08037b84..73f43084 100644 --- a/insta/src/snapshot.rs +++ b/insta/src/snapshot.rs @@ -542,20 +542,12 @@ impl Snapshot { } } - /// The normalized snapshot contents as a String - pub fn contents_string(&self) -> Option { - match self.contents() { - SnapshotContents::Text(contents) => Some(contents.normalize()), - SnapshotContents::Binary(_) => None, - } - } - fn serialize_snapshot(&self, md: &MetaData) -> String { let mut buf = yaml::to_string(&md.as_content()); buf.push_str("---\n"); - if let Some(ref contents_str) = self.contents_string() { - buf.push_str(contents_str); + if let SnapshotContents::Text(ref contents) = self.snapshot { + buf.push_str(&contents.to_string()); buf.push('\n'); } @@ -636,13 +628,6 @@ impl SnapshotContents { pub fn is_binary(&self) -> bool { matches!(self, SnapshotContents::Binary(_)) } - - pub fn as_string_contents(&self) -> Option<&TextSnapshotContents> { - match self { - SnapshotContents::Text(contents) => Some(contents), - SnapshotContents::Binary(_) => None, - } - } } impl TextSnapshotContents { From 5320d5cddaf87fe57c5d0c5d5ffe8d540447c041 Mon Sep 17 00:00:00 2001 From: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Date: Sat, 5 Oct 2024 13:02:38 -0700 Subject: [PATCH 04/22] Fix `--require-full-match` on inline snapshots with leading newlines (#639) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Otherwise `--force-update-snapshots` would never be satisfied. Note comment inline: rather than spend more time on this, let's just try and get one of those two in: ``` // Note that we previously would match the exact values of the // unnormalized text. But that's too strict — it means we can // never match a snapshot that has leading/trailing whitespace. // So instead we check it matches on the latest format. // Generally those should be the same — latest should be doing // the minimum normalization; if they diverge we could update // this to be stricter. // // (I think to do this perfectly, we'd want to match the // _reference_ value unnormalized, but the _generated_ value // normalized. That way, we can get the But at the moment we // don't distinguish between which is which in our data // structures.) ``` --- cargo-insta/tests/main.rs | 122 +++++++++++++++++++++++++------------- insta/src/snapshot.rs | 18 +++++- 2 files changed, 97 insertions(+), 43 deletions(-) diff --git a/cargo-insta/tests/main.rs b/cargo-insta/tests/main.rs index e2260565..0b13929c 100644 --- a/cargo-insta/tests/main.rs +++ b/cargo-insta/tests/main.rs @@ -1024,20 +1024,27 @@ fn test_linebreaks() { assert!(&output.status.success()); - assert_snapshot!(test_project.diff("src/lib.rs"), @r#####" - --- Original: src/lib.rs - +++ Updated: src/lib.rs - @@ -1,8 +1,5 @@ - - #[test] - fn test_linebreaks() { - - insta::assert_snapshot!("foo", @r####" - - foo - - - - "####); - + insta::assert_snapshot!("foo", @"foo"); - } - "#####); + // When #563 merges, or #630 is resolved, this will change the snapshot. I + // also think it's possible to have it work sooner, but have iterated quite + // a few times trying to get this to work, and then finding something else + // without test coverage didn't work; so not sure it's a great investment of + // time. + assert_snapshot!(test_project.diff("src/lib.rs"), @""); + + // assert_snapshot!(test_project.diff("src/lib.rs"), @r#####" + // --- Original: src/lib.rs + // +++ Updated: src/lib.rs + // @@ -1,8 +1,5 @@ + + // #[test] + // fn test_linebreaks() { + // - insta::assert_snapshot!("foo", @r####" + // - foo + // - + // - "####); + // + insta::assert_snapshot!("foo", @"foo"); + // } + // "#####); } #[test] @@ -1123,48 +1130,83 @@ fn test_wrong_indent_force() { ) .create_project(); - // Confirm the test passes despite the indent + // ...and that it passes with `--require-full-match`. Note that ideally this + // would fail, but we can't read the desired indent without serde, which is + // in `cargo-insta` only. So this tests the current state rather than the + // ideal state (and I don't think there's a reasonable way to get the ideal state) + // Now confirm that `--require-full-match` passes let output = test_project .insta_cmd() - .args(["test", "--check", "--", "--nocapture"]) + .args([ + "test", + "--check", + "--require-full-match", + "--", + "--nocapture", + ]) .output() .unwrap(); assert!(&output.status.success()); +} + +#[test] +fn test_matches_fully_linebreaks() { + // Until #563 merges, we should be OK with different leading newlines, even + // in exact / full match mode. + let test_project = TestFiles::new() + .add_file( + "Cargo.toml", + r#" +[package] +name = "exact-match-inline" +version = "0.1.0" +edition = "2021" + +[lib] +doctest = false + +[dependencies] +insta = { path = '$PROJECT_PATH' } +"# + .to_string(), + ) + .add_file( + "src/lib.rs", + r#####" +#[test] +fn test_additional_linebreak() { + // Additional newline here + insta::assert_snapshot!(r#" + + ( + "name_foo", + "insta_tests__tests", + ) + "#, @r#" + ( + "name_foo", + "insta_tests__tests", + ) + "#); +} +"##### + .to_string(), + ) + .create_project(); - // Then run the test with --force-update-snapshots and --accept to confirm - // the new snapshot is written + // Confirm the test passes despite the indent let output = test_project .insta_cmd() .args([ "test", - "--force-update-snapshots", - "--accept", + "--check", + "--require-full-match", "--", "--nocapture", ]) .output() .unwrap(); assert!(&output.status.success()); - - // https://github.com/mitsuhiko/insta/pull/563 will fix the starting & - // ending newlines - assert_snapshot!(test_project.diff("src/lib.rs"), @r##" - --- Original: src/lib.rs - +++ Updated: src/lib.rs - @@ -4,8 +4,8 @@ - insta::assert_snapshot!(r#" - foo - foo - - "#, @r#" - - foo - - foo - - "#); - + "#, @r" - + foo - + foo - + "); - } - "##); } #[test] diff --git a/insta/src/snapshot.rs b/insta/src/snapshot.rs index 73f43084..8086faa7 100644 --- a/insta/src/snapshot.rs +++ b/insta/src/snapshot.rs @@ -528,7 +528,20 @@ impl Snapshot { pub fn matches_fully(&self, other: &Self) -> bool { match (self.contents(), other.contents()) { (SnapshotContents::Text(self_contents), SnapshotContents::Text(other_contents)) => { - let contents_match_exact = self_contents == other_contents; + // Note that we previously would match the exact values of the + // unnormalized text. But that's too strict — it means we can + // never match a snapshot that has leading/trailing whitespace. + // So instead we check it matches on the latest format. + // Generally those should be the same — latest should be doing + // the minimum normalization; if they diverge we could update + // this to be stricter. + // + // (I think to do this perfectly, we'd want to match the + // _reference_ value unnormalized, but the _generated_ value + // normalized. That way, we can get the But at the moment we + // don't distinguish between which is which in our data + // structures.) + let contents_match_exact = self_contents.matches_latest(other_contents); match self_contents.kind { TextSnapshotKind::File => { self.metadata.trim_for_persistence() @@ -807,8 +820,7 @@ fn min_indentation(snapshot: &str) -> usize { .unwrap_or(0) } -// Removes excess indentation, removes excess whitespace at start & end -// and changes newlines to \n. +/// Removes excess indentation, and changes newlines to \n. fn normalize_inline_snapshot(snapshot: &str) -> String { let indentation = min_indentation(snapshot); snapshot From ca70fd02896d8a90eda0753c1a3a91a681fd320b Mon Sep 17 00:00:00 2001 From: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Date: Sun, 6 Oct 2024 10:25:01 -0700 Subject: [PATCH 05/22] Replace complicated name clash test with simple integration test (#609) --- Cargo.lock | 13 ++++++- cargo-insta/tests/main.rs | 57 +++++++++++++++++++++++++++++ insta/tests/test_clash_detection.rs | 53 --------------------------- 3 files changed, 68 insertions(+), 55 deletions(-) delete mode 100644 insta/tests/test_clash_detection.rs diff --git a/Cargo.lock b/Cargo.lock index be882cb4..70897cfe 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -486,9 +486,12 @@ checksum = "78ca9ab1a0babb1e7d5695e3530886289c18cf2f87ec19a575a0abdce112e3a3" [[package]] name = "once_cell" -version = "1.19.0" +version = "1.20.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3fdb12b2476b595f9358c5161aa467c2438859caa136dec86c26fdd2efe17b92" +checksum = "82881c4be219ab5faaf2ad5e5e5ecdff8c66bd7402ca3160975c93b24961afd1" +dependencies = [ + "portable-atomic", +] [[package]] name = "open" @@ -562,6 +565,12 @@ dependencies = [ "sha2", ] +[[package]] +name = "portable-atomic" +version = "1.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cc9c68a3f6da06753e9335d63e27f6b9754dd1920d941135b7ea8224f141adb2" + [[package]] name = "proc-macro2" version = "1.0.86" diff --git a/cargo-insta/tests/main.rs b/cargo-insta/tests/main.rs index 0b13929c..717d9bdb 100644 --- a/cargo-insta/tests/main.rs +++ b/cargo-insta/tests/main.rs @@ -1262,6 +1262,63 @@ fn test_hashtag_escape() { "####); } +#[test] +fn test_snapshot_name_clash() { + let test_project = TestFiles::new() + .add_file( + "Cargo.toml", + r#" +[package] +name = "snapshot_name_clash_test" +version = "0.1.0" +edition = "2021" + +[lib] +doctest = false + +[dependencies] +insta = { path = '$PROJECT_PATH' } +"# + .to_string(), + ) + .add_file( + "src/lib.rs", + r#" +#[cfg(test)] +mod tests { + use insta::assert_debug_snapshot; + + #[test] + fn test_foo_always_missing() { + assert_debug_snapshot!(42); + } + + #[test] + fn foo_always_missing() { + assert_debug_snapshot!(42); + } +} +"# + .to_string(), + ) + .create_project(); + + let output = test_project + .insta_cmd() + .args(["test", "--accept", "--", "--nocapture"]) + .stderr(Stdio::piped()) + .output() + .unwrap(); + + // The test should fail due to the name clash + assert!(!output.status.success()); + + let error_output = String::from_utf8_lossy(&output.stderr); + + // Check for the name clash error message + assert!(error_output.contains("Insta snapshot name clash detected between 'foo_always_missing' and 'test_foo_always_missing' in 'snapshot_name_clash_test::tests'. Rename one function.")); +} + /// A pending binary snapshot should have a binary file with the passed extension alongside it. #[test] fn test_binary_pending() { diff --git a/insta/tests/test_clash_detection.rs b/insta/tests/test_clash_detection.rs deleted file mode 100644 index f65ce18f..00000000 --- a/insta/tests/test_clash_detection.rs +++ /dev/null @@ -1,53 +0,0 @@ -use std::env; -use std::thread; - -fn test_foo_always_missing() { - insta::assert_debug_snapshot!(42); -} - -fn foo_always_missing() { - insta::assert_debug_snapshot!(42); -} - -#[test] -fn test_clash_detection() { - let old_update_value = env::var("INSTA_UPDATE"); - let old_force_pass_value = env::var("INSTA_FORCE_PASS"); - env::set_var("INSTA_UPDATE", "no"); - env::set_var("INSTA_FORCE_PASS", "0"); - - let err1 = thread::Builder::new() - .spawn(|| { - test_foo_always_missing(); - }) - .unwrap() - .join() - .unwrap_err(); - let err2 = thread::Builder::new() - .spawn(|| { - foo_always_missing(); - }) - .unwrap() - .join() - .unwrap_err(); - - if let Ok(value) = old_update_value { - env::set_var("INSTA_UPDATE", value); - } else { - env::remove_var("INSTA_UPDATE"); - } - if let Ok(value) = old_force_pass_value { - env::set_var("INSTA_FORCE_PASS", value); - } else { - env::remove_var("INSTA_FORCE_PASS"); - } - - let s1 = err1.downcast_ref::().unwrap(); - let s2 = err2.downcast_ref::().unwrap(); - let mut values = [s1.as_str(), s2.as_str()]; - values.sort(); - assert_eq!(&values[..], &vec![ - "Insta snapshot name clash detected between \'foo_always_missing\' and \'test_foo_always_missing\' in \'test_clash_detection\'. Rename one function.", - "snapshot assertion for \'foo_always_missing\' failed in line 5", - ][..]); -} From ea8332fb88d124016e000f07f1e88f818f4de026 Mon Sep 17 00:00:00 2001 From: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Date: Sun, 6 Oct 2024 10:49:32 -0700 Subject: [PATCH 06/22] Better deprecation warning for `--no-force-pass` (#643) Not sure the old code was working, this is simpler --- cargo-insta/src/cli.rs | 20 ++++++-------------- insta/src/snapshot.rs | 4 ++-- 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/cargo-insta/src/cli.rs b/cargo-insta/src/cli.rs index f2e5a341..5783ff55 100644 --- a/cargo-insta/src/cli.rs +++ b/cargo-insta/src/cli.rs @@ -644,11 +644,12 @@ fn test_run(mut cmd: TestCommand, color: ColorWhen) -> Result<(), Box cmd.force_update_snapshots = true; } } - - // --check always implies --no-force-pass as otherwise this command does not - // make a lot of sense. - if cmd.check { - cmd.no_force_pass = true + if cmd.no_force_pass { + cmd.check = true; + eprintln!( + "{}: `--no-force-pass` is deprecated. Please use --check to immediately raise an error on any non-matching snapshots.", + style("warning").bold().yellow() + ) } // the tool config can also indicate that --accept-unseen should be picked @@ -998,15 +999,6 @@ fn prepare_test_runner<'snapshot_ref>( } if !cmd.check { proc.env("INSTA_FORCE_PASS", "1"); - } else if !cmd.no_force_pass { - proc.env("INSTA_FORCE_PASS", "1"); - // If we're not running under cargo insta, raise a warning that this option - // is deprecated. (cargo insta still uses it when running under `--check`, - // but this will stop soon too) - eprintln!( - "{}: `--no-force-pass` is deprecated. Please use --check to immediately raise an error on any non-matching snapshots.", - style("warning").bold().yellow() - ); } proc.env( diff --git a/insta/src/snapshot.rs b/insta/src/snapshot.rs index 8086faa7..0130f546 100644 --- a/insta/src/snapshot.rs +++ b/insta/src/snapshot.rs @@ -385,7 +385,7 @@ impl Snapshot { } } } - elog!("A snapshot uses an old snapshot format; please update it to the new format with `cargo insta test --force-update-snapshots --accept`.\n\nSnapshot is at: {}", p.to_string_lossy()); + elog!("A snapshot uses a legacy snapshot format; please update it to the new format with `cargo insta test --force-update-snapshots --accept`.\n\nSnapshot is at: {}", p.to_string_lossy()); rv }; @@ -752,7 +752,7 @@ impl PartialEq for SnapshotContents { if this.matches_latest(other) { true } else if this.matches_legacy(other) { - elog!("{} {}\n{}",style("Snapshot passes but is a legacy format. Please run `cargo insta test --force-update-snapshots --accept` to update to a newer format.").yellow().bold(),"Snapshot contents:", this.to_string()); + elog!("{} {}\n{}",style("Snapshot test passes but the existing value is in a legacy format. Please run `cargo insta test --force-update-snapshots` to update to a newer format.").yellow().bold(),"Snapshot contents:", this.to_string()); true } else { false From 0e7865ff5940ddf31b3fb138ffe6ef475089125b Mon Sep 17 00:00:00 2001 From: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Date: Sun, 6 Oct 2024 11:32:50 -0700 Subject: [PATCH 07/22] Add some changelog entries (#645) --- CHANGELOG.md | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4eedbe66..97579430 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ All notable changes to insta and cargo-insta are documented here. ## 1.41.0 +- Experimental support for binary snapshots. #610 (Florian Plattner) + - `--force-update-snapshots` has more conservative and consistent behavior for inline snapshots. As a side-effect of this, only the content within the inline snapshot delimiters are assessed for changes, not the delimiters (e.g. `###`). @@ -11,7 +13,16 @@ All notable changes to insta and cargo-insta are documented here. - Inline snapshots only use `#` characters as delimiters when required. #603 -- Experimental support for binary snapshots. #610 (Florian Plattner) +- Warnings for undiscovered snapshots are more robust, and include files with + custom snapshot extensions. #637 + +- Insta runs correctly on packages which reference rust files in a parent path. #626 + +- Warnings are printed when any snapshot uses a legacy format. #599 + +- `insta` now internally uses `INSTA_UPDATE=force` rather than + `INSTA_FORCE_UPDATE`. (This doesn't affect users of `cargo-insta`, which + handles this internally.) #482 ## 1.40.0 From e4c59dd61a7e1bbfbde87020cc2afc18d8cdc52d Mon Sep 17 00:00:00 2001 From: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Date: Sun, 6 Oct 2024 11:34:42 -0700 Subject: [PATCH 08/22] Fix a couple docstrings (#641) --- insta/src/snapshot.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/insta/src/snapshot.rs b/insta/src/snapshot.rs index 0130f546..a11ca207 100644 --- a/insta/src/snapshot.rs +++ b/insta/src/snapshot.rs @@ -424,7 +424,6 @@ impl Snapshot { )) } - /// Creates an empty snapshot. pub(crate) fn from_components( module_name: String, snapshot_name: Option, @@ -604,8 +603,10 @@ impl Snapshot { /// Same as [`Self::save`] but instead of writing a normal snapshot file this will write /// a `.snap.new` file with additional information. /// - /// The name of the new snapshot file is returned. + /// The path of the new snapshot file is returned. pub(crate) fn save_new(&self, path: &Path) -> Result> { + // TODO: should we be the actual extension here rather than defaulting + // to the standard `.snap`? let new_path = path.to_path_buf().with_extension("snap.new"); self.save_with_metadata(&new_path, &self.metadata)?; Ok(new_path) From 53dc3de744ab6d8a724071a85c65e282d20fddf9 Mon Sep 17 00:00:00 2001 From: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Date: Mon, 7 Oct 2024 11:11:17 -0700 Subject: [PATCH 09/22] `--force-update-snapshots` always writes, and implies `--accept` (#644) As discussed in #630 Will wait for a few days before merging. --- CHANGELOG.md | 14 +++++--- cargo-insta/src/cli.rs | 7 +++- cargo-insta/tests/main.rs | 68 +++++++++++++++++---------------------- insta/src/runtime.rs | 12 +------ 4 files changed, 46 insertions(+), 55 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 97579430..be1b18ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,10 +6,16 @@ All notable changes to insta and cargo-insta are documented here. - Experimental support for binary snapshots. #610 (Florian Plattner) -- `--force-update-snapshots` has more conservative and consistent behavior for - inline snapshots. As a side-effect of this, only the content within the inline - snapshot delimiters are assessed for changes, not the delimiters (e.g. `###`). - #581 +- `--force-update-snapshots` now writes every snapshot, regardless of whether + `insta` evaluates a write is required, and now implies `--accept`. This + allows for `--force-update-snapshots` to update inline snapshots when + delimiters or indents require updating. + + For the existing behavior of limiting writes which `insta` can evaluate are + required, use `--require-full-match`. The main difference between + `--require-full-match` and the existing behavior of `--force-update-snapshots` + is that the test run will return a non-zero exit code if any snapshots don't + match fully. #644 - Inline snapshots only use `#` characters as delimiters when required. #603 diff --git a/cargo-insta/src/cli.rs b/cargo-insta/src/cli.rs index 5783ff55..9b04eaf7 100644 --- a/cargo-insta/src/cli.rs +++ b/cargo-insta/src/cli.rs @@ -193,7 +193,7 @@ struct TestCommand { /// Do not reject pending snapshots before run. #[arg(long)] keep_pending: bool, - /// Update all snapshots even if they are still matching. + /// Update all snapshots even if they are still matching; implies `--accept`. #[arg(long)] force_update_snapshots: bool, /// Handle unreferenced snapshots after a successful test run. @@ -618,6 +618,7 @@ fn process_snapshots( Ok(()) } +/// Run the tests fn test_run(mut cmd: TestCommand, color: ColorWhen) -> Result<(), Box> { let loc = handle_target_args(&cmd.target_args, &cmd.test_runner_options.package)?; match loc.tool_config.snapshot_update() { @@ -644,6 +645,10 @@ fn test_run(mut cmd: TestCommand, color: ColorWhen) -> Result<(), Box cmd.force_update_snapshots = true; } } + // `--force-update-snapshots` implies `--accept` + if cmd.force_update_snapshots { + cmd.accept = true; + } if cmd.no_force_pass { cmd.check = true; eprintln!( diff --git a/cargo-insta/tests/main.rs b/cargo-insta/tests/main.rs index 717d9bdb..60a4add8 100644 --- a/cargo-insta/tests/main.rs +++ b/cargo-insta/tests/main.rs @@ -1012,39 +1012,27 @@ fn test_linebreaks() { // Run the test with --force-update-snapshots and --accept let output = test_project .insta_cmd() - .args([ - "test", - "--force-update-snapshots", - "--accept", - "--", - "--nocapture", - ]) + .args(["test", "--force-update-snapshots", "--", "--nocapture"]) .output() .unwrap(); assert!(&output.status.success()); - // When #563 merges, or #630 is resolved, this will change the snapshot. I - // also think it's possible to have it work sooner, but have iterated quite - // a few times trying to get this to work, and then finding something else - // without test coverage didn't work; so not sure it's a great investment of - // time. - assert_snapshot!(test_project.diff("src/lib.rs"), @""); - - // assert_snapshot!(test_project.diff("src/lib.rs"), @r#####" - // --- Original: src/lib.rs - // +++ Updated: src/lib.rs - // @@ -1,8 +1,5 @@ - - // #[test] - // fn test_linebreaks() { - // - insta::assert_snapshot!("foo", @r####" - // - foo - // - - // - "####); - // + insta::assert_snapshot!("foo", @"foo"); - // } - // "#####); + // Linebreaks should be reset + assert_snapshot!(test_project.diff("src/lib.rs"), @r#####" + --- Original: src/lib.rs + +++ Updated: src/lib.rs + @@ -1,8 +1,5 @@ + + #[test] + fn test_linebreaks() { + - insta::assert_snapshot!("foo", @r####" + - foo + - + - "####); + + insta::assert_snapshot!("foo", @"foo"); + } + "#####); } #[test] @@ -1078,22 +1066,24 @@ fn test_excessive_hashes() { // Run the test with --force-update-snapshots and --accept let output = test_project .insta_cmd() - .args([ - "test", - "--force-update-snapshots", - "--accept", - "--", - "--nocapture", - ]) + .args(["test", "--force-update-snapshots", "--", "--nocapture"]) .output() .unwrap(); assert!(&output.status.success()); - // TODO: we would like to update the number of hashes, but that's not easy - // given the reasons at https://github.com/mitsuhiko/insta/pull/573. So this - // result asserts the current state rather than the desired state. - assert_snapshot!(test_project.diff("src/lib.rs"), @""); + // `--force-update-snapshots` should remove the hashes + assert_snapshot!(test_project.diff("src/lib.rs"), @r#####" + --- Original: src/lib.rs + +++ Updated: src/lib.rs + @@ -1,5 +1,5 @@ + + #[test] + fn test_excessive_hashes() { + - insta::assert_snapshot!("foo", @r####"foo"####); + + insta::assert_snapshot!("foo", @"foo"); + } + "#####); } #[test] diff --git a/insta/src/runtime.rs b/insta/src/runtime.rs index ecbfb9b0..e39be03e 100644 --- a/insta/src/runtime.rs +++ b/insta/src/runtime.rs @@ -838,17 +838,7 @@ pub fn assert_snapshot( ctx.tool_config.snapshot_update(), crate::env::SnapshotUpdate::Force ) { - // Avoid creating new files if contents match exactly. In - // particular, this would otherwise create lots of unneeded files - // for inline snapshots - let matches_fully = &ctx - .old_snapshot - .as_ref() - .map(|x| x.matches_fully(&new_snapshot)) - .unwrap_or(false); - if !matches_fully { - ctx.update_snapshot(new_snapshot)?; - } + ctx.update_snapshot(new_snapshot)?; } // otherwise print information and update snapshots. } else { From ad7baa69bf5fcd54554c81aa97f90c53d1277a2a Mon Sep 17 00:00:00 2001 From: Florian Plattner <64537872+lasernoises@users.noreply.github.com> Date: Mon, 7 Oct 2024 20:14:03 +0200 Subject: [PATCH 10/22] Add warning about stability to doc comment for internals module (#648) As discussed in #610 it probably makes sense to be explicit about the `internals` module not being guaranteed to be stable. I also noticed that `TextSnapshotKind` gets exposed globally. This was added in #581 when it was still called `SnapshotKind`. I wonder if this is necessary to expose toplevel. --- insta/src/lib.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/insta/src/lib.rs b/insta/src/lib.rs index 410c271d..663900a0 100644 --- a/insta/src/lib.rs +++ b/insta/src/lib.rs @@ -300,6 +300,9 @@ pub use crate::snapshot::{MetaData, Snapshot, TextSnapshotKind}; /// /// You're unlikely to want to work with these objects but they /// are exposed for documentation primarily. +/// +/// This module does not follow the same stability guarantees as the rest of the crate and is not +/// guaranteed to be compatible between minor versions. pub mod internals { pub use crate::content::Content; #[cfg(feature = "filters")] From 9d65b241f5fa1b192bc636ce61f9254c94536a34 Mon Sep 17 00:00:00 2001 From: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Date: Tue, 8 Oct 2024 22:22:28 -0700 Subject: [PATCH 11/22] refine: Don't use modules for tests in integration tests (#650) --- cargo-insta/tests/main.rs | 77 ++++++++++++++++----------------------- 1 file changed, 31 insertions(+), 46 deletions(-) diff --git a/cargo-insta/tests/main.rs b/cargo-insta/tests/main.rs index 60a4add8..aff4f1f9 100644 --- a/cargo-insta/tests/main.rs +++ b/cargo-insta/tests/main.rs @@ -1274,19 +1274,16 @@ insta = { path = '$PROJECT_PATH' } .add_file( "src/lib.rs", r#" -#[cfg(test)] -mod tests { - use insta::assert_debug_snapshot; +use insta::assert_debug_snapshot; - #[test] - fn test_foo_always_missing() { - assert_debug_snapshot!(42); - } +#[test] +fn test_foo_always_missing() { + assert_debug_snapshot!(42); +} - #[test] - fn foo_always_missing() { - assert_debug_snapshot!(42); - } +#[test] +fn foo_always_missing() { + assert_debug_snapshot!(42); } "# .to_string(), @@ -1306,7 +1303,7 @@ mod tests { let error_output = String::from_utf8_lossy(&output.stderr); // Check for the name clash error message - assert!(error_output.contains("Insta snapshot name clash detected between 'foo_always_missing' and 'test_foo_always_missing' in 'snapshot_name_clash_test::tests'. Rename one function.")); + assert!(error_output.contains("Insta snapshot name clash detected between 'foo_always_missing' and 'test_foo_always_missing' in 'snapshot_name_clash_test'. Rename one function.")); } /// A pending binary snapshot should have a binary file with the passed extension alongside it. @@ -1778,15 +1775,12 @@ fn test_insta_workspace_root() { .add_file( "src/lib.rs", r#" - #[cfg(test)] - mod tests { - use insta::assert_snapshot; +use insta::assert_snapshot; - #[test] - fn test_snapshot() { - assert_snapshot!("Hello, world!"); - } - } +#[test] +fn test_snapshot() { + assert_snapshot!("Hello, world!"); +} "# .to_string(), ) @@ -1823,7 +1817,7 @@ fn test_insta_workspace_root() { assert!(test_project.workspace_dir.join("src/snapshots").exists()); assert!(test_project .workspace_dir - .join("src/snapshots/insta_workspace_root_test__tests__snapshot.snap") + .join("src/snapshots/insta_workspace_root_test__snapshot.snap") .exists()); // Move the workspace @@ -1989,12 +1983,9 @@ insta = { path = '$PROJECT_PATH' } .add_file( "src/lib.rs", r#" -#[cfg(test)] -mod tests { - #[test] - fn test_snapshot() { - insta::assert_snapshot!("Hello, world!"); - } +#[test] +fn test_snapshot() { + insta::assert_snapshot!("Hello, world!"); } "# .to_string(), @@ -2013,7 +2004,7 @@ mod tests { // Manually add an unreferenced snapshot let unreferenced_snapshot_path = test_project .workspace_dir - .join("src/snapshots/test_unreferenced_delete__tests__unused_snapshot.snap"); + .join("src/snapshots/test_unreferenced_delete__unused_snapshot.snap"); std::fs::create_dir_all(unreferenced_snapshot_path.parent().unwrap()).unwrap(); std::fs::write( &unreferenced_snapshot_path, @@ -2036,8 +2027,8 @@ Unused snapshot src src/lib.rs + src/snapshots - + src/snapshots/test_unreferenced_delete__tests__snapshot.snap - + src/snapshots/test_unreferenced_delete__tests__unused_snapshot.snap + + src/snapshots/test_unreferenced_delete__snapshot.snap + + src/snapshots/test_unreferenced_delete__unused_snapshot.snap "); // Run cargo insta test with --unreferenced=delete @@ -2066,7 +2057,7 @@ Unused snapshot src src/lib.rs + src/snapshots - + src/snapshots/test_unreferenced_delete__tests__snapshot.snap + + src/snapshots/test_unreferenced_delete__snapshot.snap "); } @@ -2089,18 +2080,15 @@ insta = { path = '$PROJECT_PATH' } .add_file( "src/lib.rs", r#" -#[cfg(test)] -mod tests { - #[test] - fn test_snapshot() { - insta::assert_snapshot!("Hello, world!"); - } +#[test] +fn test_snapshot() { + insta::assert_snapshot!("Hello, world!"); } "# .to_string(), ) .add_file( - "src/snapshots/test_hidden_snapshots__tests__snapshot.snap", + "src/snapshots/test_hidden_snapshots__snapshot.snap", r#"--- source: src/lib.rs expression: "\"Hello, world!\"" @@ -2249,12 +2237,9 @@ insta = { path = '$PROJECT_PATH' } .add_file( "src/lib.rs", r#" -#[cfg(test)] -mod tests { - #[test] - fn test_snapshot() { - insta::assert_binary_snapshot!(".txt", b"abcd".to_vec()); - } +#[test] +fn test_snapshot() { + insta::assert_binary_snapshot!(".txt", b"abcd".to_vec()); } "# .to_string(), @@ -2282,8 +2267,8 @@ mod tests { src src/lib.rs + src/snapshots - + src/snapshots/test_binary_unreferenced_delete__tests__snapshot.snap - + src/snapshots/test_binary_unreferenced_delete__tests__snapshot.snap.txt + + src/snapshots/test_binary_unreferenced_delete__snapshot.snap + + src/snapshots/test_binary_unreferenced_delete__snapshot.snap.txt "); // Run cargo insta test with --unreferenced=delete From ce2b3fd10692ecd5ed6d29e0b7a2b6efea327a88 Mon Sep 17 00:00:00 2001 From: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Date: Thu, 10 Oct 2024 11:57:43 -0700 Subject: [PATCH 12/22] Small refactorings & comments (#654) --- cargo-insta/src/cli.rs | 53 ++++++++++++++++++------------------------ insta/src/runtime.rs | 4 +++- 2 files changed, 26 insertions(+), 31 deletions(-) diff --git a/cargo-insta/src/cli.rs b/cargo-insta/src/cli.rs index 9b04eaf7..9f840bc9 100644 --- a/cargo-insta/src/cli.rs +++ b/cargo-insta/src/cli.rs @@ -100,6 +100,8 @@ struct TargetArgs { #[arg(long)] all: bool, /// Also walk into ignored paths. + // TODO: this alias is confusing, I think we should remove — does "no" mean + // "don't ignore files" or "not ignored files"? #[arg(long, alias = "no-ignore")] include_ignored: bool, /// Also include hidden paths. @@ -668,7 +670,7 @@ fn test_run(mut cmd: TestCommand, color: ColorWhen) -> Result<(), Box // Legacy command if cmd.delete_unreferenced_snapshots { - println!("Warning: `--delete-unreferenced-snapshots` is deprecated. Use `--unreferenced=delete` instead."); + eprintln!("Warning: `--delete-unreferenced-snapshots` is deprecated. Use `--unreferenced=delete` instead."); cmd.unreferenced = UnreferencedSnapshots::Delete; } @@ -697,6 +699,7 @@ fn test_run(mut cmd: TestCommand, color: ColorWhen) -> Result<(), Box process_snapshots(true, None, &loc, Some(Operation::Reject))?; } + // Run the tests let status = proc.status()?; let mut success = status.success(); @@ -888,6 +891,7 @@ fn handle_unreferenced_snapshots( Ok(()) } +/// Create and setup a `Command`, translating our configs into env vars & cli options #[allow(clippy::type_complexity)] fn prepare_test_runner<'snapshot_ref>( test_runner: TestRunner, @@ -902,37 +906,29 @@ fn prepare_test_runner<'snapshot_ref>( let cargo = cargo .as_deref() .unwrap_or_else(|| std::ffi::OsStr::new("cargo")); - let test_runner = match test_runner { - TestRunner::CargoTest | TestRunner::Auto => test_runner, - TestRunner::Nextest => { - // Fall back to `cargo test` if `cargo nextest` isn't installed and - // `test_runner_fallback` is true (but don't run the cargo command - // unless that's an option) - if !test_runner_fallback - || std::process::Command::new("cargo") - .arg("nextest") - .arg("--version") - .output() - .map(|output| output.status.success()) - .unwrap_or(false) - { - TestRunner::Nextest - } else { - TestRunner::Auto - } - } + // Fall back to `cargo test` if `cargo nextest` isn't installed and + // `test_runner_fallback` is true + let test_runner = if test_runner == TestRunner::Nextest + && test_runner_fallback + && std::process::Command::new("cargo") + .arg("nextest") + .arg("--version") + .output() + .map(|output| !output.status.success()) + .unwrap_or(true) + { + TestRunner::Auto + } else { + test_runner }; - let mut proc = match test_runner { + let mut proc = process::Command::new(cargo); + match test_runner { TestRunner::CargoTest | TestRunner::Auto => { - let mut proc = process::Command::new(cargo); proc.arg("test"); - proc } TestRunner::Nextest => { - let mut proc = process::Command::new(cargo); proc.arg("nextest"); proc.arg("run"); - proc } }; @@ -1077,17 +1073,14 @@ fn prepare_test_runner<'snapshot_ref>( proc.arg("--target"); proc.arg(target); } - proc.arg("--color"); - proc.arg(color.to_string()); + proc.args(["--color", color.to_string().as_str()]); proc.args(extra_args); // Items after this are passed to the test runner proc.arg("--"); if !cmd.no_quiet && matches!(test_runner, TestRunner::CargoTest) { proc.arg("-q"); } - if !cmd.cargo_options.is_empty() { - proc.args(&cmd.cargo_options); - } + proc.args(&cmd.cargo_options); // Currently libtest uses a different approach to color, so we need to pass // it again to get output from the test runner as well as cargo. See // https://github.com/rust-lang/cargo/issues/1983 for more diff --git a/insta/src/runtime.rs b/insta/src/runtime.rs index e39be03e..56166579 100644 --- a/insta/src/runtime.rs +++ b/insta/src/runtime.rs @@ -517,6 +517,8 @@ impl<'a> SnapshotAssertionContext<'a> { &self, new_snapshot: Snapshot, ) -> Result> { + // TODO: this seems to be making `unseen` be true when there is an + // existing snapshot file; which seems wrong?? let unseen = self .snapshot_file .as_ref() @@ -527,8 +529,8 @@ impl<'a> SnapshotAssertionContext<'a> { // If snapshot_update is `InPlace` and we have an inline snapshot, then // use `NewFile`, since we can't use `InPlace` for inline. `cargo-insta` // then accepts all snapshots at the end of the test. - let snapshot_update = + // TOOD: could match on the snapshot kind instead of whether snapshot_file is None if snapshot_update == SnapshotUpdateBehavior::InPlace && self.snapshot_file.is_none() { SnapshotUpdateBehavior::NewFile } else { From c3d6f16a364b13b0dd2a3501b28bfd94729061c9 Mon Sep 17 00:00:00 2001 From: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Date: Thu, 10 Oct 2024 11:58:16 -0700 Subject: [PATCH 13/22] Add test for force-updating legacy snapshots (#653) --- cargo-insta/tests/main.rs | 36 ++++++++++++++++++++++++++++++------ 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/cargo-insta/tests/main.rs b/cargo-insta/tests/main.rs index aff4f1f9..013069bd 100644 --- a/cargo-insta/tests/main.rs +++ b/cargo-insta/tests/main.rs @@ -861,16 +861,40 @@ fn test_old_yaml_format() { ) .create_project(); - // Run the test with --force-update-snapshots and --accept - let output = test_project + // Check it passes + assert!(test_project .insta_cmd() - .args(["test", "--accept", "--", "--nocapture"]) + .args(["test", "--", "--nocapture"]) .output() - .unwrap(); + .unwrap() + .status + .success()); + // shouldn't be any changes + assert_snapshot!(test_project.diff("src/lib.rs"), @""); - assert!(&output.status.success()); + // Also check that running with `--force-update-snapshots` updates the snapshot + assert!(test_project + .insta_cmd() + .args(["test", "--force-update-snapshots", "--", "--nocapture"]) + .output() + .unwrap() + .status + .success()); - assert_snapshot!(test_project.diff("src/lib.rs"), @""); + assert_snapshot!(test_project.diff("src/lib.rs"), @r#####" + --- Original: src/lib.rs + +++ Updated: src/lib.rs + @@ -1,8 +1,5 @@ + + #[test] + fn test_old_yaml_format() { + - insta::assert_yaml_snapshot!("foo", @r####" + - --- + - foo + -"####); + + insta::assert_yaml_snapshot!("foo", @"foo"); + } + "#####); } #[test] From 19a2faf4bb85f4173ba2d2b1245ef40b1a8dec66 Mon Sep 17 00:00:00 2001 From: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Date: Thu, 10 Oct 2024 13:11:54 -0700 Subject: [PATCH 14/22] Collect `insta` version in standard `cargo metadata` call (#655) ...otherwise it doesn't respond to `--manifest-path`. If it does raise an error, we just continue after printing a warning, rather than panicking. Also added some docs on running a reproducible example in another path. (I've also found a bug with `--workspace-root`, will handle separately, and will add a test for both) --- CONTRIBUTING.md | 6 ++++++ cargo-insta/src/cli.rs | 25 ++++++++++++++++++++----- cargo-insta/src/utils.rs | 22 +--------------------- insta/src/env.rs | 11 ++++++++++- 4 files changed, 37 insertions(+), 27 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 3e9adc4d..f30be138 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -37,6 +37,12 @@ cargo run -p cargo-insta -- test # (or `review` or whatever command you want to ...in contrast to running `cargo insta`, which invokes the installed version of `cargo-insta`, and so make iterating more difficult. +To run the version of `cargo-insta` in the working directory on another crate, run: + +```sh +cargo run -p cargo-insta -- test --manifest-path=../insta-bug-repro/Cargo.toml +``` + ## Writing tests If making non-trivial changes to `cargo-insta`, please add an integration test to diff --git a/cargo-insta/src/cli.rs b/cargo-insta/src/cli.rs index 9f840bc9..369486b8 100644 --- a/cargo-insta/src/cli.rs +++ b/cargo-insta/src/cli.rs @@ -18,7 +18,6 @@ use uuid::Uuid; use crate::cargo::{find_snapshot_roots, Package}; use crate::container::{Operation, SnapshotContainer}; use crate::utils::cargo_insta_version; -use crate::utils::INSTA_VERSION; use crate::utils::{err_msg, QuietExit}; use crate::walk::{find_pending_snapshots, make_snapshot_walker, FindFlags}; @@ -395,6 +394,9 @@ struct LocationInfo<'a> { packages: Vec, exts: Vec<&'a str>, find_flags: FindFlags, + /// The insta version in the current workspace (i.e. not the `cargo-insta` + /// binary that's running). + insta_version: Version, } fn get_find_flags(tool_config: &ToolConfig, target_args: &TargetArgs) -> FindFlags { @@ -439,7 +441,7 @@ fn handle_target_args<'a>( (None, None) => {} }; - let metadata = cmd.no_deps().exec()?; + let metadata = cmd.exec()?; let workspace_root = metadata.workspace_root.as_std_path().to_path_buf(); let tool_config = ToolConfig::from_workspace(&workspace_root)?; @@ -462,6 +464,13 @@ fn handle_target_args<'a>( } else { vec![metadata.root_package().unwrap().clone()] }; + let insta_version = metadata + .packages + .iter() + .find(|package| package.name == "insta") + .map(|package| package.version.clone()) + .ok_or_else(|| eprintln!("insta not found in cargo metadata; defaulting to 1.0.0")) + .unwrap_or(Version::new(1, 0, 0)); Ok(LocationInfo { workspace_root, @@ -478,6 +487,7 @@ fn handle_target_args<'a>( .collect(), find_flags: get_find_flags(&tool_config, target_args), tool_config, + insta_version }) } @@ -693,6 +703,7 @@ fn test_run(mut cmd: TestCommand, color: ColorWhen) -> Result<(), Box color, &[], None, + &loc, )?; if !cmd.keep_pending { @@ -717,6 +728,7 @@ fn test_run(mut cmd: TestCommand, color: ColorWhen) -> Result<(), Box color, &["--doc"], snapshot_ref_file.as_deref(), + &loc, )?; success = success && proc.status()?.success(); } @@ -892,7 +904,9 @@ fn handle_unreferenced_snapshots( } /// Create and setup a `Command`, translating our configs into env vars & cli options +// TODO: possibly we can clean this function up a bit, reduce the number of args #[allow(clippy::type_complexity)] +#[allow(clippy::too_many_arguments)] fn prepare_test_runner<'snapshot_ref>( test_runner: TestRunner, test_runner_fallback: bool, @@ -901,6 +915,7 @@ fn prepare_test_runner<'snapshot_ref>( color: ColorWhen, extra_args: &[&str], snapshot_ref_file: Option<&'snapshot_ref Path>, + loc: &LocationInfo, ) -> Result<(process::Command, Option>, bool), Box> { let cargo = env::var_os("CARGO"); let cargo = cargo @@ -910,7 +925,7 @@ fn prepare_test_runner<'snapshot_ref>( // `test_runner_fallback` is true let test_runner = if test_runner == TestRunner::Nextest && test_runner_fallback - && std::process::Command::new("cargo") + && std::process::Command::new(cargo) .arg("nextest") .arg("--version") .output() @@ -1006,7 +1021,7 @@ fn prepare_test_runner<'snapshot_ref>( "INSTA_UPDATE", // Don't set `INSTA_UPDATE=force` for `--force-update-snapshots` on // older versions - if *INSTA_VERSION >= Version::new(1,41,0) { + if loc.insta_version >= Version::new(1,41,0) { match (cmd.check, cmd.accept_unseen, cmd.force_update_snapshots) { (true, false, false) => "no", (false, true, false) => "unseen", @@ -1022,7 +1037,7 @@ fn prepare_test_runner<'snapshot_ref>( } } ); - if cmd.force_update_snapshots && *INSTA_VERSION < Version::new(1, 40, 0) { + if cmd.force_update_snapshots && loc.insta_version < Version::new(1, 40, 0) { // Currently compatible with older versions of insta. proc.env("INSTA_FORCE_UPDATE_SNAPSHOTS", "1"); proc.env("INSTA_FORCE_UPDATE", "1"); diff --git a/cargo-insta/src/utils.rs b/cargo-insta/src/utils.rs index 53d5b621..dc252b54 100644 --- a/cargo-insta/src/utils.rs +++ b/cargo-insta/src/utils.rs @@ -1,10 +1,6 @@ use std::error::Error; use std::fmt; -use cargo_metadata::MetadataCommand; -use lazy_static::lazy_static; -use semver::Version; - /// Close without message but exit code. #[derive(Debug)] pub(crate) struct QuietExit(pub(crate) i32); @@ -32,23 +28,7 @@ pub(crate) fn err_msg>(s: S) -> Box { Box::new(ErrMsg(s.into())) } -/// The insta version in the current workspace (i.e. not the `cargo-insta` -/// binary that's running). -fn read_insta_version() -> Result> { - MetadataCommand::new() - .exec()? - .packages - .iter() - .find(|package| package.name == "insta") - .map(|package| package.version.clone()) - .ok_or("insta not found in cargo metadata".into()) -} - -lazy_static! { - pub static ref INSTA_VERSION: Version = read_insta_version().unwrap(); -} - -/// `cargo-insta` version +/// `cargo-insta` version (i.e. the binary that's currently running). // We could put this in a lazy_static pub(crate) fn cargo_insta_version() -> String { env!("CARGO_PKG_VERSION").to_string() diff --git a/insta/src/env.rs b/insta/src/env.rs index eabd0833..327f7b8e 100644 --- a/insta/src/env.rs +++ b/insta/src/env.rs @@ -406,7 +406,9 @@ pub fn snapshot_update_behavior(tool_config: &ToolConfig, unseen: bool) -> Snaps } } -/// Returns the cargo workspace for a manifest +/// Returns the cargo workspace path for a crate manifest, like +/// `/Users/janedoe/projects/insta` when passed +/// `/Users/janedoe/projects/insta/insta/Cargo.toml`. pub fn get_cargo_workspace(manifest_dir: &str) -> Arc { // If INSTA_WORKSPACE_ROOT environment variable is set, use the value as-is. // This is useful where CARGO_MANIFEST_DIR at compilation points to some @@ -465,6 +467,13 @@ pub fn get_cargo_workspace(manifest_dir: &str) -> Arc { .clone() } +#[test] +fn test_get_cargo_workspace() { + let workspace = get_cargo_workspace(env!("CARGO_MANIFEST_DIR")); + // The absolute path of the workspace, like `/Users/janedoe/projects/insta` + assert!(workspace.ends_with("insta")); +} + #[cfg(feature = "_cargo_insta_internal")] impl std::str::FromStr for TestRunner { type Err = (); From 3a3df35d4a6c81f905a0e18f11cdd95dd1ac9f0b Mon Sep 17 00:00:00 2001 From: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Date: Thu, 10 Oct 2024 17:51:16 -0700 Subject: [PATCH 15/22] Split up integration tests into modules (#656) We're at 2400 lines now This PR moves the main file; another commit will split it up, which I think means git will handle merge conflicts better. --- cargo-insta/tests/{ => functional}/main.rs | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename cargo-insta/tests/{ => functional}/main.rs (100%) diff --git a/cargo-insta/tests/main.rs b/cargo-insta/tests/functional/main.rs similarity index 100% rename from cargo-insta/tests/main.rs rename to cargo-insta/tests/functional/main.rs From ab7957dcfc8f09f3c0a125ba55f25e9fcd1072aa Mon Sep 17 00:00:00 2001 From: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Date: Thu, 10 Oct 2024 18:04:41 -0700 Subject: [PATCH 16/22] Split up integration tests (2/2) (#657) As discussed in #656 --- cargo-insta/tests/functional/binary.rs | 500 ++++++ cargo-insta/tests/functional/inline.rs | 361 ++++ cargo-insta/tests/functional/main.rs | 1876 +++------------------ cargo-insta/tests/functional/workspace.rs | 595 +++++++ 4 files changed, 1679 insertions(+), 1653 deletions(-) create mode 100644 cargo-insta/tests/functional/binary.rs create mode 100644 cargo-insta/tests/functional/inline.rs create mode 100644 cargo-insta/tests/functional/workspace.rs diff --git a/cargo-insta/tests/functional/binary.rs b/cargo-insta/tests/functional/binary.rs new file mode 100644 index 00000000..1f17ad33 --- /dev/null +++ b/cargo-insta/tests/functional/binary.rs @@ -0,0 +1,500 @@ +use insta::assert_snapshot; + +use crate::TestFiles; + +/// A pending binary snapshot should have a binary file with the passed extension alongside it. +#[test] +fn test_binary_pending() { + let test_project = TestFiles::new() + .add_file( + "Cargo.toml", + r#" +[package] +name = "test_binary_pending" +version = "0.1.0" +edition = "2021" + +[dependencies] +insta = { path = '$PROJECT_PATH' } +"# + .to_string(), + ) + .add_file( + "src/main.rs", + r#" +#[test] +fn test_binary_snapshot() { + insta::assert_binary_snapshot!(".txt", b"test".to_vec()); +} +"# + .to_string(), + ) + .create_project(); + + let output = test_project.insta_cmd().args(["test"]).output().unwrap(); + + assert!(!&output.status.success()); + + assert_snapshot!(test_project.file_tree_diff(), @r" + --- Original file tree + +++ Updated file tree + @@ -1,4 +1,8 @@ + + + Cargo.lock + Cargo.toml + src + src/main.rs + + src/snapshots + + src/snapshots/test_binary_pending__binary_snapshot.snap.new + + src/snapshots/test_binary_pending__binary_snapshot.snap.new.txt + "); +} + +/// An accepted binary snapshot should have a binary file with the passed extension alongside it. +#[test] +fn test_binary_accept() { + let test_project = TestFiles::new() + .add_file( + "Cargo.toml", + r#" +[package] +name = "test_binary_accept" +version = "0.1.0" +edition = "2021" + +[dependencies] +insta = { path = '$PROJECT_PATH' } +"# + .to_string(), + ) + .add_file( + "src/main.rs", + r#" +#[test] +fn test_binary_snapshot() { + insta::assert_binary_snapshot!(".txt", b"test".to_vec()); +} +"# + .to_string(), + ) + .create_project(); + + let output = test_project + .insta_cmd() + .args(["test", "--accept"]) + .output() + .unwrap(); + + assert!(&output.status.success()); + + assert_snapshot!(test_project.file_tree_diff(), @r" + --- Original file tree + +++ Updated file tree + @@ -1,4 +1,8 @@ + + + Cargo.lock + Cargo.toml + src + src/main.rs + + src/snapshots + + src/snapshots/test_binary_accept__binary_snapshot.snap + + src/snapshots/test_binary_accept__binary_snapshot.snap.txt + "); +} + +/// Changing the extension passed to the `assert_binary_snapshot` macro should create a new pending +/// snapshot with a binary file with the new extension alongside it and once approved the old binary +/// file with the old extension should be deleted. +#[test] +fn test_binary_change_extension() { + let test_project = TestFiles::new() + .add_file( + "Cargo.toml", + r#" +[package] +name = "test_binary_change_extension" +version = "0.1.0" +edition = "2021" + +[dependencies] +insta = { path = '$PROJECT_PATH' } +"# + .to_string(), + ) + .add_file( + "src/main.rs", + r#" +#[test] +fn test_binary_snapshot() { + insta::assert_binary_snapshot!(".txt", b"test".to_vec()); +} +"# + .to_string(), + ) + .create_project(); + + let output = test_project + .insta_cmd() + .args(["test", "--accept"]) + .output() + .unwrap(); + + assert!(&output.status.success()); + + test_project.update_file( + "src/main.rs", + r#" +#[test] +fn test_binary_snapshot() { + insta::assert_binary_snapshot!(".json", b"test".to_vec()); +} +"# + .to_string(), + ); + + let output = test_project.insta_cmd().args(["test"]).output().unwrap(); + + assert!(!&output.status.success()); + + assert_snapshot!(test_project.file_tree_diff(), @r" + --- Original file tree + +++ Updated file tree + @@ -1,4 +1,10 @@ + + + Cargo.lock + Cargo.toml + src + src/main.rs + + src/snapshots + + src/snapshots/test_binary_change_extension__binary_snapshot.snap + + src/snapshots/test_binary_change_extension__binary_snapshot.snap.new + + src/snapshots/test_binary_change_extension__binary_snapshot.snap.new.json + + src/snapshots/test_binary_change_extension__binary_snapshot.snap.txt + "); + + let output = test_project + .insta_cmd() + .args(["test", "--accept"]) + .output() + .unwrap(); + + assert!(&output.status.success()); + + assert_snapshot!(test_project.file_tree_diff(), @r" + --- Original file tree + +++ Updated file tree + @@ -1,4 +1,8 @@ + + + Cargo.lock + Cargo.toml + src + src/main.rs + + src/snapshots + + src/snapshots/test_binary_change_extension__binary_snapshot.snap + + src/snapshots/test_binary_change_extension__binary_snapshot.snap.json + "); +} + +/// An assert with a pending binary snapshot should have both the metadata file and the binary file +/// deleted when the assert is removed and the tests are re-run. +#[test] +fn test_binary_pending_snapshot_removal() { + let test_project = TestFiles::new() + .add_file( + "Cargo.toml", + r#" +[package] +name = "test_binary_pending_snapshot_removal" +version = "0.1.0" +edition = "2021" + +[dependencies] +insta = { path = '$PROJECT_PATH' } +"# + .to_string(), + ) + .add_file( + "src/main.rs", + r#" +#[test] +fn test_binary_snapshot() { + insta::assert_binary_snapshot!(".txt", b"test".to_vec()); +} +"# + .to_string(), + ) + .create_project(); + + let output = test_project.insta_cmd().args(["test"]).output().unwrap(); + + assert!(!&output.status.success()); + + test_project.update_file("src/main.rs", "".to_string()); + + let output = test_project.insta_cmd().args(["test"]).output().unwrap(); + + assert!(&output.status.success()); + + assert_snapshot!(test_project.file_tree_diff(), @r" + --- Original file tree + +++ Updated file tree + @@ -1,4 +1,6 @@ + + + Cargo.lock + Cargo.toml + src + src/main.rs + + src/snapshots + "); +} + +/// Replacing a text snapshot with binary one should work and simply replace the text snapshot file +/// with the new metadata file and a new binary snapshot file alongside it. +#[test] +fn test_change_text_to_binary() { + let test_project = TestFiles::new() + .add_file( + "Cargo.toml", + r#" +[package] +name = "test_change_text_to_binary" +version = "0.1.0" +edition = "2021" + +[dependencies] +insta = { path = '$PROJECT_PATH' } +"# + .to_string(), + ) + .add_file( + "src/main.rs", + r#" +#[test] +fn test() { + insta::assert_snapshot!("test"); +} +"# + .to_string(), + ) + .create_project(); + + let output = test_project + .insta_cmd() + .args(["test", "--accept"]) + .output() + .unwrap(); + + assert!(&output.status.success()); + assert_snapshot!(test_project.file_tree_diff(), @r" + --- Original file tree + +++ Updated file tree + @@ -1,4 +1,7 @@ + + + Cargo.lock + Cargo.toml + src + src/main.rs + + src/snapshots + + src/snapshots/test_change_text_to_binary__test.snap + "); + + test_project.update_file( + "src/main.rs", + r#" +#[test] +fn test() { + insta::assert_binary_snapshot!(".txt", b"test".to_vec()); +} +"# + .to_string(), + ); + + let output = test_project + .insta_cmd() + .args(["test", "--accept"]) + .output() + .unwrap(); + + assert!(&output.status.success()); + assert_snapshot!(test_project.file_tree_diff(), @r" + --- Original file tree + +++ Updated file tree + @@ -1,4 +1,8 @@ + + + Cargo.lock + Cargo.toml + src + src/main.rs + + src/snapshots + + src/snapshots/test_change_text_to_binary__test.snap + + src/snapshots/test_change_text_to_binary__test.snap.txt + "); +} + +/// When changing a snapshot from a binary to a text snapshot the previous binary file should be +/// gone after having approved the the binary snapshot. +#[test] +fn test_change_binary_to_text() { + let test_project = TestFiles::new() + .add_file( + "Cargo.toml", + r#" +[package] +name = "test_change_binary_to_text" +version = "0.1.0" +edition = "2021" + +[dependencies] +insta = { path = '$PROJECT_PATH' } +"# + .to_string(), + ) + .add_file( + "src/main.rs", + r#" +#[test] +fn test() { + insta::assert_binary_snapshot!("some_name.json", b"{}".to_vec()); +} +"# + .to_string(), + ) + .create_project(); + + let output = test_project + .insta_cmd() + .args(["test", "--accept"]) + .output() + .unwrap(); + + assert!(&output.status.success()); + assert_snapshot!(test_project.file_tree_diff(), @r" + --- Original file tree + +++ Updated file tree + @@ -1,4 +1,8 @@ + + + Cargo.lock + Cargo.toml + src + src/main.rs + + src/snapshots + + src/snapshots/test_change_binary_to_text__some_name.snap + + src/snapshots/test_change_binary_to_text__some_name.snap.json + "); + + test_project.update_file( + "src/main.rs", + r#" +#[test] +fn test() { + insta::assert_snapshot!("some_name", "test"); +} +"# + .to_string(), + ); + + let output = test_project + .insta_cmd() + .args(["test", "--accept"]) + .output() + .unwrap(); + + assert!(&output.status.success()); + assert_snapshot!(test_project.file_tree_diff(), @r" + --- Original file tree + +++ Updated file tree + @@ -1,4 +1,7 @@ + + + Cargo.lock + Cargo.toml + src + src/main.rs + + src/snapshots + + src/snapshots/test_change_binary_to_text__some_name.snap + "); +} + +#[test] +fn test_binary_unreferenced_delete() { + let test_project = TestFiles::new() + .add_file( + "Cargo.toml", + r#" +[package] +name = "test_binary_unreferenced_delete" +version = "0.1.0" +edition = "2021" + +[lib] +doctest = false + +[dependencies] +insta = { path = '$PROJECT_PATH' } +"# + .to_string(), + ) + .add_file( + "src/lib.rs", + r#" +#[test] +fn test_snapshot() { + insta::assert_binary_snapshot!(".txt", b"abcd".to_vec()); +} +"# + .to_string(), + ) + .create_project(); + + // Run tests to create snapshots + let output = test_project + .insta_cmd() + .args(["test", "--accept"]) + .output() + .unwrap(); + + assert!(&output.status.success()); + + test_project.update_file("src/lib.rs", "".to_string()); + + assert_snapshot!(test_project.file_tree_diff(), @r" + --- Original file tree + +++ Updated file tree + @@ -1,4 +1,8 @@ + + + Cargo.lock + Cargo.toml + src + src/lib.rs + + src/snapshots + + src/snapshots/test_binary_unreferenced_delete__snapshot.snap + + src/snapshots/test_binary_unreferenced_delete__snapshot.snap.txt + "); + + // Run cargo insta test with --unreferenced=delete + let output = test_project + .insta_cmd() + .args([ + "test", + "--unreferenced=delete", + "--accept", + "--", + "--nocapture", + ]) + .output() + .unwrap(); + + assert!(&output.status.success()); + + // We should now see the unreferenced snapshot deleted + assert_snapshot!(test_project.file_tree_diff(), @r" + --- Original file tree + +++ Updated file tree + @@ -1,4 +1,6 @@ + + + Cargo.lock + Cargo.toml + src + src/lib.rs + + src/snapshots + "); +} diff --git a/cargo-insta/tests/functional/inline.rs b/cargo-insta/tests/functional/inline.rs new file mode 100644 index 00000000..4d5aaaad --- /dev/null +++ b/cargo-insta/tests/functional/inline.rs @@ -0,0 +1,361 @@ +use insta::assert_snapshot; + +use crate::TestFiles; + +#[test] +fn test_json_inline() { + let test_project = TestFiles::new() + .add_file( + "Cargo.toml", + r#" +[package] +name = "test_json_inline" +version = "0.1.0" +edition = "2021" + +[dependencies] +insta = { path = '$PROJECT_PATH', features=["json", "redactions"] } +serde = { version = "1.0", features = ["derive"] } +"# + .to_string(), + ) + .add_file( + "src/main.rs", + r#" +use serde::Serialize; + +#[derive(Serialize)] +struct User { + id: u64, + email: String, +} + +#[test] +fn test_json_snapshot() { + let user = User { + id: 42, + email: "john.doe@example.com".into(), + }; + insta::assert_json_snapshot!(&user, { + ".id" => "[user_id]", + }, @""); +} +"# + .to_string(), + ) + .create_project(); + + let output = test_project + .insta_cmd() + .args(["test", "--accept", "--", "--nocapture"]) + .output() + .unwrap(); + + assert!(&output.status.success()); + + assert_snapshot!(test_project.diff("src/main.rs"), @r##" + --- Original: src/main.rs + +++ Updated: src/main.rs + @@ -15,5 +15,10 @@ + }; + insta::assert_json_snapshot!(&user, { + ".id" => "[user_id]", + - }, @""); + + }, @r#" + + { + + "id": "[user_id]", + + "email": "john.doe@example.com" + + } + + "#); + } + "##); +} + +#[test] +fn test_yaml_inline() { + let test_project = TestFiles::new() + .add_file( + "Cargo.toml", + r#" +[package] +name = "test_yaml_inline" +version = "0.1.0" +edition = "2021" + +[dependencies] +insta = { path = '$PROJECT_PATH', features=["yaml", "redactions"] } +serde = { version = "1.0", features = ["derive"] } +"# + .to_string(), + ) + .add_file( + "src/main.rs", + r#" +use serde::Serialize; + +#[derive(Serialize)] +struct User { + id: u64, + email: String, +} + +#[test] +fn test_yaml_snapshot() { + let user = User { + id: 42, + email: "john.doe@example.com".into(), + }; + insta::assert_yaml_snapshot!(&user, { + ".id" => "[user_id]", + }, @""); +} +"# + .to_string(), + ) + .create_project(); + + let output = test_project + .insta_cmd() + .args(["test", "--accept"]) + .output() + .unwrap(); + + assert!(&output.status.success()); + + assert_snapshot!(test_project.diff("src/main.rs"), @r###" + --- Original: src/main.rs + +++ Updated: src/main.rs + @@ -15,5 +15,8 @@ + }; + insta::assert_yaml_snapshot!(&user, { + ".id" => "[user_id]", + - }, @""); + + }, @r#" + + id: "[user_id]" + + email: john.doe@example.com + + "#); + } + "###); +} + +#[test] +fn test_utf8_inline() { + let test_project = TestFiles::new() + .add_file( + "Cargo.toml", + r#" +[package] +name = "test_utf8_inline" +version = "0.1.0" +edition = "2021" + +[dependencies] +insta = { path = '$PROJECT_PATH' } +"# + .to_string(), + ) + .add_file( + "src/main.rs", + r#" +#[test] +fn test_non_basic_plane() { + /* an offset here ❄️ */ insta::assert_snapshot!("a 😀oeu", @""); +} + +#[test] +fn test_remove_existing_value() { + insta::assert_snapshot!("this is the new value", @"this is the old value"); +} + +#[test] +fn test_remove_existing_value_multiline() { + insta::assert_snapshot!( + "this is the new value", + @"this is\ + this is the old value\ + it really is" + ); +} + +#[test] +fn test_trailing_comma_in_inline_snapshot() { + insta::assert_snapshot!( + "new value", + @"old value", // comma here + ); +} +"# + .to_string(), + ) + .create_project(); + + let output = test_project + .insta_cmd() + .args(["test", "--accept"]) + .output() + .unwrap(); + + assert!(&output.status.success()); + + assert_snapshot!(test_project.diff("src/main.rs"), @r##" + --- Original: src/main.rs + +++ Updated: src/main.rs + @@ -1,21 +1,19 @@ + + #[test] + fn test_non_basic_plane() { + - /* an offset here ❄️ */ insta::assert_snapshot!("a 😀oeu", @""); + + /* an offset here ❄️ */ insta::assert_snapshot!("a 😀oeu", @"a 😀oeu"); + } + + #[test] + fn test_remove_existing_value() { + - insta::assert_snapshot!("this is the new value", @"this is the old value"); + + insta::assert_snapshot!("this is the new value", @"this is the new value"); + } + + #[test] + fn test_remove_existing_value_multiline() { + insta::assert_snapshot!( + "this is the new value", + - @"this is\ + - this is the old value\ + - it really is" + + @"this is the new value" + ); + } + + @@ -23,6 +21,6 @@ + fn test_trailing_comma_in_inline_snapshot() { + insta::assert_snapshot!( + "new value", + - @"old value", // comma here + + @"new value", // comma here + ); + } + "##); +} + +/// Test the old format of inline YAML snapshots with a leading `---` still passes +#[test] +fn test_old_yaml_format() { + let test_project = TestFiles::new() + .add_file( + "Cargo.toml", + r#" +[package] +name = "old-yaml-format" +version = "0.1.0" +edition = "2021" + +[lib] +doctest = false + +[dependencies] +insta = { path = '$PROJECT_PATH', features = ["yaml"] } +"# + .to_string(), + ) + .add_file( + "src/lib.rs", + r#####" +#[test] +fn test_old_yaml_format() { + insta::assert_yaml_snapshot!("foo", @r####" + --- + foo +"####); +} +"##### + .to_string(), + ) + .create_project(); + + // Check it passes + assert!(test_project + .insta_cmd() + .args(["test", "--", "--nocapture"]) + .output() + .unwrap() + .status + .success()); + // shouldn't be any changes + assert_snapshot!(test_project.diff("src/lib.rs"), @""); + + // Also check that running with `--force-update-snapshots` updates the snapshot + assert!(test_project + .insta_cmd() + .args(["test", "--force-update-snapshots", "--", "--nocapture"]) + .output() + .unwrap() + .status + .success()); + + assert_snapshot!(test_project.diff("src/lib.rs"), @r#####" + --- Original: src/lib.rs + +++ Updated: src/lib.rs + @@ -1,8 +1,5 @@ + + #[test] + fn test_old_yaml_format() { + - insta::assert_yaml_snapshot!("foo", @r####" + - --- + - foo + -"####); + + insta::assert_yaml_snapshot!("foo", @"foo"); + } + "#####); +} + +#[test] +fn test_hashtag_escape_in_inline_snapshot() { + let test_project = TestFiles::new() + .add_file( + "Cargo.toml", + r#" +[package] +name = "test_hashtag_escape" +version = "0.1.0" +edition = "2021" + +[dependencies] +insta = { path = '$PROJECT_PATH' } +"# + .to_string(), + ) + .add_file( + "src/main.rs", + r#####" +#[test] +fn test_hashtag_escape() { + insta::assert_snapshot!(r###"Value with + "## hashtags\n"###, @""); +} +"##### + .to_string(), + ) + .create_project(); + + let output = test_project + .insta_cmd() + .args(["test", "--accept"]) + .output() + .unwrap(); + + assert!(&output.status.success()); + + assert_snapshot!(test_project.diff("src/main.rs"), @r####" + --- Original: src/main.rs + +++ Updated: src/main.rs + @@ -2,5 +2,8 @@ + #[test] + fn test_hashtag_escape() { + insta::assert_snapshot!(r###"Value with + - "## hashtags\n"###, @""); + + "## hashtags\n"###, @r###" + + Value with + + "## hashtags\n + + "###); + } + "####); +} diff --git a/cargo-insta/tests/functional/main.rs b/cargo-insta/tests/functional/main.rs index 013069bd..d92cd865 100644 --- a/cargo-insta/tests/functional/main.rs +++ b/cargo-insta/tests/functional/main.rs @@ -44,13 +44,13 @@ /// > crates the same... This also causes issues when running the same tests /// > concurrently. use std::collections::HashMap; +use std::env; use std::fs; use std::io::{BufRead, BufReader}; use std::path::{Path, PathBuf}; use std::process::Command; use std::process::Stdio; use std::thread; -use std::{env, fs::remove_dir_all}; use console::style; use ignore::WalkBuilder; @@ -59,6 +59,10 @@ use itertools::Itertools; use similar::udiff::unified_diff; use tempfile::TempDir; +mod binary; +mod inline; +mod workspace; + /// Wraps a formatting function to be used as a `Stdio` struct OutputFormatter(F) where @@ -257,149 +261,118 @@ impl TestProject { } #[test] -fn test_json_inline() { - let test_project = TestFiles::new() - .add_file( - "Cargo.toml", - r#" +fn test_force_update_snapshots() { + fn create_test_force_update_project(name: &str, insta_dependency: &str) -> TestProject { + TestFiles::new() + .add_file( + "Cargo.toml", + format!( + r#" [package] -name = "test_json_inline" +name = "test_force_update_{}" version = "0.1.0" edition = "2021" [dependencies] -insta = { path = '$PROJECT_PATH', features=["json", "redactions"] } -serde = { version = "1.0", features = ["derive"] } +insta = {} +"#, + name, insta_dependency + ) + .to_string(), + ) + .add_file( + "src/lib.rs", + r#" +#[test] +fn test_snapshot_with_newline() { + insta::assert_snapshot!("force_update", "Hello, world!"); +} "# - .to_string(), - ) - .add_file( - "src/main.rs", - r#" -use serde::Serialize; + .to_string(), + ) + .add_file( + format!( + "src/snapshots/test_force_update_{}__force_update.snap", + name + ), + r#" +--- +source: src/lib.rs +expression: +--- +Hello, world! -#[derive(Serialize)] -struct User { - id: u64, - email: String, -} -#[test] -fn test_json_snapshot() { - let user = User { - id: 42, - email: "john.doe@example.com".into(), - }; - insta::assert_json_snapshot!(&user, { - ".id" => "[user_id]", - }, @""); -} "# - .to_string(), - ) - .create_project(); + .to_string(), + ) + .create_project() + } - let output = test_project + let test_current_insta = + create_test_force_update_project("current", "{ path = '$PROJECT_PATH' }"); + let test_insta_1_40_0 = create_test_force_update_project("1_40_0", "\"1.40.0\""); + + // Test with current insta version + let output_current = test_current_insta .insta_cmd() - .args(["test", "--accept", "--", "--nocapture"]) + .args(["test", "--accept", "--force-update-snapshots"]) .output() .unwrap(); - assert!(&output.status.success()); - - assert_snapshot!(test_project.diff("src/main.rs"), @r##" - --- Original: src/main.rs - +++ Updated: src/main.rs - @@ -15,5 +15,10 @@ - }; - insta::assert_json_snapshot!(&user, { - ".id" => "[user_id]", - - }, @""); - + }, @r#" - + { - + "id": "[user_id]", - + "email": "john.doe@example.com" - + } - + "#); - } - "##); -} - -#[test] -fn test_yaml_inline() { - let test_project = TestFiles::new() - .add_file( - "Cargo.toml", - r#" -[package] -name = "test_yaml_inline" -version = "0.1.0" -edition = "2021" - -[dependencies] -insta = { path = '$PROJECT_PATH', features=["yaml", "redactions"] } -serde = { version = "1.0", features = ["derive"] } -"# - .to_string(), - ) - .add_file( - "src/main.rs", - r#" -use serde::Serialize; - -#[derive(Serialize)] -struct User { - id: u64, - email: String, -} - -#[test] -fn test_yaml_snapshot() { - let user = User { - id: 42, - email: "john.doe@example.com".into(), - }; - insta::assert_yaml_snapshot!(&user, { - ".id" => "[user_id]", - }, @""); -} -"# - .to_string(), - ) - .create_project(); + assert!(&output_current.status.success()); - let output = test_project + // Test with insta 1.40.0 + let output_1_40_0 = test_insta_1_40_0 .insta_cmd() - .args(["test", "--accept"]) + .args(["test", "--accept", "--force-update-snapshots"]) .output() .unwrap(); - assert!(&output.status.success()); + assert!(&output_1_40_0.status.success()); - assert_snapshot!(test_project.diff("src/main.rs"), @r###" - --- Original: src/main.rs - +++ Updated: src/main.rs - @@ -15,5 +15,8 @@ - }; - insta::assert_yaml_snapshot!(&user, { - ".id" => "[user_id]", - - }, @""); - + }, @r#" - + id: "[user_id]" - + email: john.doe@example.com - + "#); - } - "###); + // Check that both versions updated the snapshot correctly + 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,6 @@ + - + --- + source: src/lib.rs + -expression: + +expression: "\"Hello, world!\"" + +snapshot_kind: text + --- + 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,6 @@ + - + --- + source: src/lib.rs + -expression: + +expression: "\"Hello, world!\"" + +snapshot_kind: text + --- + Hello, world! + - + - + "#); } #[test] -fn test_utf8_inline() { +fn test_force_update_inline_snapshot_linebreaks() { let test_project = TestFiles::new() .add_file( "Cargo.toml", r#" [package] -name = "test_utf8_inline" +name = "force-update-inline-linebreaks" version = "0.1.0" edition = "2021" @@ -409,1266 +382,105 @@ insta = { path = '$PROJECT_PATH' } .to_string(), ) .add_file( - "src/main.rs", - r#" -#[test] -fn test_non_basic_plane() { - /* an offset here ❄️ */ insta::assert_snapshot!("a 😀oeu", @""); -} - -#[test] -fn test_remove_existing_value() { - insta::assert_snapshot!("this is the new value", @"this is the old value"); -} - -#[test] -fn test_remove_existing_value_multiline() { - insta::assert_snapshot!( - "this is the new value", - @"this is\ - this is the old value\ - it really is" - ); -} - + "src/lib.rs", + r#####" #[test] -fn test_trailing_comma_in_inline_snapshot() { - insta::assert_snapshot!( - "new value", - @"old value", // comma here - ); +fn test_linebreaks() { + insta::assert_snapshot!("foo", @r####" + foo + + "####); } -"# - .to_string(), +"##### + .to_string(), ) .create_project(); + // Run the test with --force-update-snapshots and --accept let output = test_project .insta_cmd() - .args(["test", "--accept"]) + .args(["test", "--force-update-snapshots", "--", "--nocapture"]) .output() .unwrap(); assert!(&output.status.success()); - assert_snapshot!(test_project.diff("src/main.rs"), @r##" - --- Original: src/main.rs - +++ Updated: src/main.rs - @@ -1,21 +1,19 @@ - - #[test] - fn test_non_basic_plane() { - - /* an offset here ❄️ */ insta::assert_snapshot!("a 😀oeu", @""); - + /* an offset here ❄️ */ insta::assert_snapshot!("a 😀oeu", @"a 😀oeu"); - } - - #[test] - fn test_remove_existing_value() { - - insta::assert_snapshot!("this is the new value", @"this is the old value"); - + insta::assert_snapshot!("this is the new value", @"this is the new value"); - } + // Linebreaks should be reset + assert_snapshot!(test_project.diff("src/lib.rs"), @r#####" + --- Original: src/lib.rs + +++ Updated: src/lib.rs + @@ -1,8 +1,5 @@ #[test] - fn test_remove_existing_value_multiline() { - insta::assert_snapshot!( - "this is the new value", - - @"this is\ - - this is the old value\ - - it really is" - + @"this is the new value" - ); - } - - @@ -23,6 +21,6 @@ - fn test_trailing_comma_in_inline_snapshot() { - insta::assert_snapshot!( - "new value", - - @"old value", // comma here - + @"new value", // comma here - ); + fn test_linebreaks() { + - insta::assert_snapshot!("foo", @r####" + - foo + - + - "####); + + insta::assert_snapshot!("foo", @"foo"); } - "##); + "#####); } -// Note that names need to be different to prevent the cache confusing them. -fn workspace_with_root_crate(name: String) -> TestFiles { - TestFiles::new() +#[test] +fn test_force_update_inline_snapshot_hashes() { + let test_project = TestFiles::new() .add_file( "Cargo.toml", - format!( - r#" + r#" [package] -name = "{name}" +name = "force-update-inline-hashes" version = "0.1.0" edition = "2021" -[workspace] -members = [ - "member", -] - -[workspace.dependencies] -insta = {{path = '$PROJECT_PATH'}} - -[dependencies] -insta = {{ workspace = true }} - -"# - ) - .to_string(), - ) - .add_file( - "member/Cargo.toml", - format!( - r#" -[package] -name = "{name}-member" -version = "0.0.0" -edition = "2021" - [dependencies] -insta = {{ workspace = true }} -"# - ) - .to_string(), - ) - .add_file( - "member/src/lib.rs", - r#" -#[test] -fn test_member() { - insta::assert_debug_snapshot!(vec![1, 2, 3]); -} +insta = { path = '$PROJECT_PATH' } "# .to_string(), ) .add_file( - "src/main.rs", - r#" -fn main() { - println!("Hello, world!"); -} - + "src/lib.rs", + r#####" #[test] -fn test_root() { - insta::assert_debug_snapshot!(vec![1, 2, 3]); +fn test_excessive_hashes() { + insta::assert_snapshot!("foo", @r####"foo"####); } -"# - .to_string(), +"##### + .to_string(), ) -} - -/// Check that in a workspace with a default root crate, running `cargo insta -/// test --workspace --accept` will update snapshots in both the root crate and the -/// member crate. -#[test] -fn test_root_crate_workspace_accept() { - let test_project = - workspace_with_root_crate("root-crate-workspace-accept".to_string()).create_project(); + .create_project(); + // Run the test with --force-update-snapshots and --accept let output = test_project .insta_cmd() - .args(["test", "--accept", "--workspace"]) + .args(["test", "--force-update-snapshots", "--", "--nocapture"]) .output() .unwrap(); assert!(&output.status.success()); - assert_snapshot!(test_project.file_tree_diff(), @r###" - --- Original file tree - +++ Updated file tree - @@ -1,8 +1,13 @@ - - + Cargo.lock - Cargo.toml - member - member/Cargo.toml - member/src - member/src/lib.rs - + member/src/snapshots - + member/src/snapshots/root_crate_workspace_accept_member__member.snap - src - src/main.rs - + src/snapshots - + src/snapshots/root_crate_workspace_accept__root.snap - "### ); -} - -/// Check that in a workspace with a default root crate, running `cargo insta -/// test --workspace` will correctly report the number of pending snapshots -#[test] -fn test_root_crate_workspace() { - let test_project = - workspace_with_root_crate("root-crate-workspace".to_string()).create_project(); - - let output = test_project - .insta_cmd() - // Need to disable colors to assert the output below - .args(["test", "--workspace", "--color=never"]) - .stderr(Stdio::piped()) - .output() - .unwrap(); - - // 1.39 had a bug where it would claim there were 3 snapshots here - assert!( - String::from_utf8_lossy(&output.stderr).contains("info: 2 snapshots to review"), - "{}", - String::from_utf8_lossy(&output.stderr) - ); -} - -/// Check that in a workspace with a default root crate, running `cargo insta -/// test --accept` will only update snapshots in the root crate -#[test] -fn test_root_crate_no_all() { - let test_project = workspace_with_root_crate("root-crate-no-all".to_string()).create_project(); - - let output = test_project - .insta_cmd() - .args(["test", "--accept"]) - .output() - .unwrap(); - - assert!(&output.status.success()); - - assert_snapshot!(test_project.file_tree_diff(), @r###" - --- Original file tree - +++ Updated file tree - @@ -1,4 +1,5 @@ - - + Cargo.lock - Cargo.toml - member - member/Cargo.toml - @@ -6,3 +7,5 @@ - member/src/lib.rs - src - src/main.rs - + src/snapshots - + src/snapshots/root_crate_no_all__root.snap - "### ); -} - -fn workspace_with_virtual_manifest(name: String) -> TestFiles { - TestFiles::new() - .add_file( - "Cargo.toml", - r#" -[workspace] -members = [ - "member-1", - "member-2", -] - -[workspace.dependencies] -insta = {path = '$PROJECT_PATH'} -"# - .to_string() - .to_string(), - ) - .add_file( - "member-1/Cargo.toml", - format!( - r#" -[package] -name = "{name}-member-1" -version = "0.1.0" -edition = "2021" - -[dependencies] -insta = {{ workspace = true }} -"# - ) - .to_string(), - ) - .add_file( - "member-1/src/lib.rs", - r#" -#[test] -fn test_member_1() { - insta::assert_debug_snapshot!(vec![1, 2, 3]); -} -"# - .to_string(), - ) - .add_file( - "member-2/Cargo.toml", - format!( - r#" -[package] -name = "{name}-member-2" -version = "0.1.0" -edition = "2021" - -[dependencies] -insta = {{ workspace = true }} -"# - ) - .to_string(), - ) - .add_file( - "member-2/src/lib.rs", - r#" -#[test] -fn test_member_2() { - insta::assert_debug_snapshot!(vec![4, 5, 6]); -} -"# - .to_string(), - ) -} - -/// Check that in a workspace with a virtual manifest, running `cargo insta test -/// --workspace --accept` updates snapshots in all member crates. -#[test] -fn test_virtual_manifest_all() { - let test_project = - workspace_with_virtual_manifest("virtual-manifest-all".to_string()).create_project(); - - let output = test_project - .insta_cmd() - .args(["test", "--accept", "--workspace"]) - .output() - .unwrap(); - - assert!(&output.status.success()); - - assert_snapshot!(test_project.file_tree_diff(), @r###" - --- Original file tree - +++ Updated file tree - @@ -1,10 +1,15 @@ - - + Cargo.lock - Cargo.toml - member-1 - member-1/Cargo.toml - member-1/src - member-1/src/lib.rs - + member-1/src/snapshots - + member-1/src/snapshots/virtual_manifest_all_member_1__member_1.snap - member-2 - member-2/Cargo.toml - member-2/src - member-2/src/lib.rs - + member-2/src/snapshots - + member-2/src/snapshots/virtual_manifest_all_member_2__member_2.snap - "### ); -} - -/// Check that in a workspace with a virtual manifest, running `cargo insta test -/// --accept` updates snapshots in all member crates. -#[test] -fn test_virtual_manifest_default() { - let test_project = - workspace_with_virtual_manifest("virtual-manifest-default".to_string()).create_project(); - - let output = test_project - .insta_cmd() - .args(["test", "--accept"]) - .output() - .unwrap(); - - assert!(&output.status.success()); - - assert_snapshot!(test_project.file_tree_diff(), @r###" - --- Original file tree - +++ Updated file tree - @@ -1,10 +1,15 @@ - - + Cargo.lock - Cargo.toml - member-1 - member-1/Cargo.toml - member-1/src - member-1/src/lib.rs - + member-1/src/snapshots - + member-1/src/snapshots/virtual_manifest_default_member_1__member_1.snap - member-2 - member-2/Cargo.toml - member-2/src - member-2/src/lib.rs - + member-2/src/snapshots - + member-2/src/snapshots/virtual_manifest_default_member_2__member_2.snap - "### ); -} - -/// Check that in a workspace with a virtual manifest, running `cargo insta test -/// -p ` will only update snapshots in that crate. -#[test] -fn test_virtual_manifest_single_crate() { - let test_project = - workspace_with_virtual_manifest("virtual-manifest-single".to_string()).create_project(); - - let output = test_project - .insta_cmd() - .args(["test", "--accept", "-p", "virtual-manifest-single-member-1"]) - .output() - .unwrap(); - - assert!(&output.status.success()); - - assert_snapshot!(test_project.file_tree_diff(), @r###" - --- Original file tree - +++ Updated file tree - @@ -1,9 +1,12 @@ - - + Cargo.lock - Cargo.toml - member-1 - member-1/Cargo.toml - member-1/src - member-1/src/lib.rs - + member-1/src/snapshots - + member-1/src/snapshots/virtual_manifest_single_member_1__member_1.snap - member-2 - member-2/Cargo.toml - member-2/src - "### ); -} - -/// Test the old format of inline YAML snapshots with a leading `---` still passes -#[test] -fn test_old_yaml_format() { - let test_project = TestFiles::new() - .add_file( - "Cargo.toml", - r#" -[package] -name = "old-yaml-format" -version = "0.1.0" -edition = "2021" - -[lib] -doctest = false - -[dependencies] -insta = { path = '$PROJECT_PATH', features = ["yaml"] } -"# - .to_string(), - ) - .add_file( - "src/lib.rs", - r#####" -#[test] -fn test_old_yaml_format() { - insta::assert_yaml_snapshot!("foo", @r####" - --- - foo -"####); -} -"##### - .to_string(), - ) - .create_project(); - - // Check it passes - assert!(test_project - .insta_cmd() - .args(["test", "--", "--nocapture"]) - .output() - .unwrap() - .status - .success()); - // shouldn't be any changes - assert_snapshot!(test_project.diff("src/lib.rs"), @""); - - // Also check that running with `--force-update-snapshots` updates the snapshot - assert!(test_project - .insta_cmd() - .args(["test", "--force-update-snapshots", "--", "--nocapture"]) - .output() - .unwrap() - .status - .success()); - - assert_snapshot!(test_project.diff("src/lib.rs"), @r#####" - --- Original: src/lib.rs - +++ Updated: src/lib.rs - @@ -1,8 +1,5 @@ - - #[test] - fn test_old_yaml_format() { - - insta::assert_yaml_snapshot!("foo", @r####" - - --- - - foo - -"####); - + insta::assert_yaml_snapshot!("foo", @"foo"); - } - "#####); -} - -#[test] -fn test_force_update_snapshots() { - fn create_test_force_update_project(name: &str, insta_dependency: &str) -> TestProject { - TestFiles::new() - .add_file( - "Cargo.toml", - format!( - r#" -[package] -name = "test_force_update_{}" -version = "0.1.0" -edition = "2021" - -[dependencies] -insta = {} -"#, - name, insta_dependency - ) - .to_string(), - ) - .add_file( - "src/lib.rs", - r#" -#[test] -fn test_snapshot_with_newline() { - insta::assert_snapshot!("force_update", "Hello, world!"); -} -"# - .to_string(), - ) - .add_file( - format!( - "src/snapshots/test_force_update_{}__force_update.snap", - name - ), - r#" ---- -source: src/lib.rs -expression: ---- -Hello, world! - - -"# - .to_string(), - ) - .create_project() - } - - let test_current_insta = - create_test_force_update_project("current", "{ path = '$PROJECT_PATH' }"); - let test_insta_1_40_0 = create_test_force_update_project("1_40_0", "\"1.40.0\""); - - // Test with current insta version - let output_current = test_current_insta - .insta_cmd() - .args(["test", "--accept", "--force-update-snapshots"]) - .output() - .unwrap(); - - assert!(&output_current.status.success()); - - // Test with insta 1.40.0 - let output_1_40_0 = test_insta_1_40_0 - .insta_cmd() - .args(["test", "--accept", "--force-update-snapshots"]) - .output() - .unwrap(); - - assert!(&output_1_40_0.status.success()); - - // Check that both versions updated the snapshot correctly - 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,6 @@ - - - --- - source: src/lib.rs - -expression: - +expression: "\"Hello, world!\"" - +snapshot_kind: text - --- - 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,6 @@ - - - --- - source: src/lib.rs - -expression: - +expression: "\"Hello, world!\"" - +snapshot_kind: text - --- - Hello, world! - - - - - "#); -} - -#[test] -fn test_force_update_inline_snapshot_linebreaks() { - let test_project = TestFiles::new() - .add_file( - "Cargo.toml", - r#" -[package] -name = "force-update-inline-linebreaks" -version = "0.1.0" -edition = "2021" - -[dependencies] -insta = { path = '$PROJECT_PATH' } -"# - .to_string(), - ) - .add_file( - "src/lib.rs", - r#####" -#[test] -fn test_linebreaks() { - insta::assert_snapshot!("foo", @r####" - foo - - "####); -} -"##### - .to_string(), - ) - .create_project(); - - // Run the test with --force-update-snapshots and --accept - let output = test_project - .insta_cmd() - .args(["test", "--force-update-snapshots", "--", "--nocapture"]) - .output() - .unwrap(); - - assert!(&output.status.success()); - - // Linebreaks should be reset - assert_snapshot!(test_project.diff("src/lib.rs"), @r#####" - --- Original: src/lib.rs - +++ Updated: src/lib.rs - @@ -1,8 +1,5 @@ - - #[test] - fn test_linebreaks() { - - insta::assert_snapshot!("foo", @r####" - - foo - - - - "####); - + insta::assert_snapshot!("foo", @"foo"); - } - "#####); -} - -#[test] -fn test_force_update_inline_snapshot_hashes() { - let test_project = TestFiles::new() - .add_file( - "Cargo.toml", - r#" -[package] -name = "force-update-inline-hashes" -version = "0.1.0" -edition = "2021" - -[dependencies] -insta = { path = '$PROJECT_PATH' } -"# - .to_string(), - ) - .add_file( - "src/lib.rs", - r#####" -#[test] -fn test_excessive_hashes() { - insta::assert_snapshot!("foo", @r####"foo"####); -} -"##### - .to_string(), - ) - .create_project(); - - // Run the test with --force-update-snapshots and --accept - let output = test_project - .insta_cmd() - .args(["test", "--force-update-snapshots", "--", "--nocapture"]) - .output() - .unwrap(); - - assert!(&output.status.success()); - - // `--force-update-snapshots` should remove the hashes - assert_snapshot!(test_project.diff("src/lib.rs"), @r#####" - --- Original: src/lib.rs - +++ Updated: src/lib.rs - @@ -1,5 +1,5 @@ + // `--force-update-snapshots` should remove the hashes + assert_snapshot!(test_project.diff("src/lib.rs"), @r#####" + --- Original: src/lib.rs + +++ Updated: src/lib.rs + @@ -1,5 +1,5 @@ #[test] fn test_excessive_hashes() { - insta::assert_snapshot!("foo", @r####"foo"####); - + insta::assert_snapshot!("foo", @"foo"); - } - "#####); -} - -#[test] -fn test_inline_snapshot_indent() { - let test_project = TestFiles::new() - .add_file( - "Cargo.toml", - r#" -[package] -name = "inline-indent" -version = "0.1.0" -edition = "2021" - -[dependencies] -insta = { path = '$PROJECT_PATH' } -"# - .to_string(), - ) - .add_file( - "src/lib.rs", - r#####" -#[test] -fn test_wrong_indent_force() { - insta::assert_snapshot!(r#" - foo - foo - "#, @r#" - foo - foo - "#); -} -"##### - .to_string(), - ) - .create_project(); - - // ...and that it passes with `--require-full-match`. Note that ideally this - // would fail, but we can't read the desired indent without serde, which is - // in `cargo-insta` only. So this tests the current state rather than the - // ideal state (and I don't think there's a reasonable way to get the ideal state) - // Now confirm that `--require-full-match` passes - let output = test_project - .insta_cmd() - .args([ - "test", - "--check", - "--require-full-match", - "--", - "--nocapture", - ]) - .output() - .unwrap(); - assert!(&output.status.success()); -} - -#[test] -fn test_matches_fully_linebreaks() { - // Until #563 merges, we should be OK with different leading newlines, even - // in exact / full match mode. - let test_project = TestFiles::new() - .add_file( - "Cargo.toml", - r#" -[package] -name = "exact-match-inline" -version = "0.1.0" -edition = "2021" - -[lib] -doctest = false - -[dependencies] -insta = { path = '$PROJECT_PATH' } -"# - .to_string(), - ) - .add_file( - "src/lib.rs", - r#####" -#[test] -fn test_additional_linebreak() { - // Additional newline here - insta::assert_snapshot!(r#" - - ( - "name_foo", - "insta_tests__tests", - ) - "#, @r#" - ( - "name_foo", - "insta_tests__tests", - ) - "#); -} -"##### - .to_string(), - ) - .create_project(); - - // Confirm the test passes despite the indent - let output = test_project - .insta_cmd() - .args([ - "test", - "--check", - "--require-full-match", - "--", - "--nocapture", - ]) - .output() - .unwrap(); - assert!(&output.status.success()); -} - -#[test] -fn test_hashtag_escape_in_inline_snapshot() { - let test_project = TestFiles::new() - .add_file( - "Cargo.toml", - r#" -[package] -name = "test_hashtag_escape" -version = "0.1.0" -edition = "2021" - -[dependencies] -insta = { path = '$PROJECT_PATH' } -"# - .to_string(), - ) - .add_file( - "src/main.rs", - r#####" -#[test] -fn test_hashtag_escape() { - insta::assert_snapshot!(r###"Value with - "## hashtags\n"###, @""); -} -"##### - .to_string(), - ) - .create_project(); - - let output = test_project - .insta_cmd() - .args(["test", "--accept"]) - .output() - .unwrap(); - - assert!(&output.status.success()); - - assert_snapshot!(test_project.diff("src/main.rs"), @r####" - --- Original: src/main.rs - +++ Updated: src/main.rs - @@ -2,5 +2,8 @@ - #[test] - fn test_hashtag_escape() { - insta::assert_snapshot!(r###"Value with - - "## hashtags\n"###, @""); - + "## hashtags\n"###, @r###" - + Value with - + "## hashtags\n - + "###); - } - "####); -} - -#[test] -fn test_snapshot_name_clash() { - let test_project = TestFiles::new() - .add_file( - "Cargo.toml", - r#" -[package] -name = "snapshot_name_clash_test" -version = "0.1.0" -edition = "2021" - -[lib] -doctest = false - -[dependencies] -insta = { path = '$PROJECT_PATH' } -"# - .to_string(), - ) - .add_file( - "src/lib.rs", - r#" -use insta::assert_debug_snapshot; - -#[test] -fn test_foo_always_missing() { - assert_debug_snapshot!(42); -} - -#[test] -fn foo_always_missing() { - assert_debug_snapshot!(42); -} -"# - .to_string(), - ) - .create_project(); - - let output = test_project - .insta_cmd() - .args(["test", "--accept", "--", "--nocapture"]) - .stderr(Stdio::piped()) - .output() - .unwrap(); - - // The test should fail due to the name clash - assert!(!output.status.success()); - - let error_output = String::from_utf8_lossy(&output.stderr); - - // Check for the name clash error message - assert!(error_output.contains("Insta snapshot name clash detected between 'foo_always_missing' and 'test_foo_always_missing' in 'snapshot_name_clash_test'. Rename one function.")); -} - -/// A pending binary snapshot should have a binary file with the passed extension alongside it. -#[test] -fn test_binary_pending() { - let test_project = TestFiles::new() - .add_file( - "Cargo.toml", - r#" -[package] -name = "test_binary_pending" -version = "0.1.0" -edition = "2021" - -[dependencies] -insta = { path = '$PROJECT_PATH' } -"# - .to_string(), - ) - .add_file( - "src/main.rs", - r#" -#[test] -fn test_binary_snapshot() { - insta::assert_binary_snapshot!(".txt", b"test".to_vec()); -} -"# - .to_string(), - ) - .create_project(); - - let output = test_project.insta_cmd().args(["test"]).output().unwrap(); - - assert!(!&output.status.success()); - - assert_snapshot!(test_project.file_tree_diff(), @r" - --- Original file tree - +++ Updated file tree - @@ -1,4 +1,8 @@ - - + Cargo.lock - Cargo.toml - src - src/main.rs - + src/snapshots - + src/snapshots/test_binary_pending__binary_snapshot.snap.new - + src/snapshots/test_binary_pending__binary_snapshot.snap.new.txt - "); -} - -/// An accepted binary snapshot should have a binary file with the passed extension alongside it. -#[test] -fn test_binary_accept() { - let test_project = TestFiles::new() - .add_file( - "Cargo.toml", - r#" -[package] -name = "test_binary_accept" -version = "0.1.0" -edition = "2021" - -[dependencies] -insta = { path = '$PROJECT_PATH' } -"# - .to_string(), - ) - .add_file( - "src/main.rs", - r#" -#[test] -fn test_binary_snapshot() { - insta::assert_binary_snapshot!(".txt", b"test".to_vec()); -} -"# - .to_string(), - ) - .create_project(); - - let output = test_project - .insta_cmd() - .args(["test", "--accept"]) - .output() - .unwrap(); - - assert!(&output.status.success()); - - assert_snapshot!(test_project.file_tree_diff(), @r" - --- Original file tree - +++ Updated file tree - @@ -1,4 +1,8 @@ - - + Cargo.lock - Cargo.toml - src - src/main.rs - + src/snapshots - + src/snapshots/test_binary_accept__binary_snapshot.snap - + src/snapshots/test_binary_accept__binary_snapshot.snap.txt - "); -} - -/// Changing the extension passed to the `assert_binary_snapshot` macro should create a new pending -/// snapshot with a binary file with the new extension alongside it and once approved the old binary -/// file with the old extension should be deleted. -#[test] -fn test_binary_change_extension() { - let test_project = TestFiles::new() - .add_file( - "Cargo.toml", - r#" -[package] -name = "test_binary_change_extension" -version = "0.1.0" -edition = "2021" - -[dependencies] -insta = { path = '$PROJECT_PATH' } -"# - .to_string(), - ) - .add_file( - "src/main.rs", - r#" -#[test] -fn test_binary_snapshot() { - insta::assert_binary_snapshot!(".txt", b"test".to_vec()); -} -"# - .to_string(), - ) - .create_project(); - - let output = test_project - .insta_cmd() - .args(["test", "--accept"]) - .output() - .unwrap(); - - assert!(&output.status.success()); - - test_project.update_file( - "src/main.rs", - r#" -#[test] -fn test_binary_snapshot() { - insta::assert_binary_snapshot!(".json", b"test".to_vec()); -} -"# - .to_string(), - ); - - let output = test_project.insta_cmd().args(["test"]).output().unwrap(); - - assert!(!&output.status.success()); - - assert_snapshot!(test_project.file_tree_diff(), @r" - --- Original file tree - +++ Updated file tree - @@ -1,4 +1,10 @@ - - + Cargo.lock - Cargo.toml - src - src/main.rs - + src/snapshots - + src/snapshots/test_binary_change_extension__binary_snapshot.snap - + src/snapshots/test_binary_change_extension__binary_snapshot.snap.new - + src/snapshots/test_binary_change_extension__binary_snapshot.snap.new.json - + src/snapshots/test_binary_change_extension__binary_snapshot.snap.txt - "); - - let output = test_project - .insta_cmd() - .args(["test", "--accept"]) - .output() - .unwrap(); - - assert!(&output.status.success()); - - assert_snapshot!(test_project.file_tree_diff(), @r" - --- Original file tree - +++ Updated file tree - @@ -1,4 +1,8 @@ - - + Cargo.lock - Cargo.toml - src - src/main.rs - + src/snapshots - + src/snapshots/test_binary_change_extension__binary_snapshot.snap - + src/snapshots/test_binary_change_extension__binary_snapshot.snap.json - "); -} - -/// An assert with a pending binary snapshot should have both the metadata file and the binary file -/// deleted when the assert is removed and the tests are re-run. -#[test] -fn test_binary_pending_snapshot_removal() { - let test_project = TestFiles::new() - .add_file( - "Cargo.toml", - r#" -[package] -name = "test_binary_pending_snapshot_removal" -version = "0.1.0" -edition = "2021" - -[dependencies] -insta = { path = '$PROJECT_PATH' } -"# - .to_string(), - ) - .add_file( - "src/main.rs", - r#" -#[test] -fn test_binary_snapshot() { - insta::assert_binary_snapshot!(".txt", b"test".to_vec()); -} -"# - .to_string(), - ) - .create_project(); - - let output = test_project.insta_cmd().args(["test"]).output().unwrap(); - - assert!(!&output.status.success()); - - test_project.update_file("src/main.rs", "".to_string()); - - let output = test_project.insta_cmd().args(["test"]).output().unwrap(); - - assert!(&output.status.success()); - - assert_snapshot!(test_project.file_tree_diff(), @r" - --- Original file tree - +++ Updated file tree - @@ -1,4 +1,6 @@ - - + Cargo.lock - Cargo.toml - src - src/main.rs - + src/snapshots - "); -} - -/// Replacing a text snapshot with binary one should work and simply replace the text snapshot file -/// with the new metadata file and a new binary snapshot file alongside it. -#[test] -fn test_change_text_to_binary() { - let test_project = TestFiles::new() - .add_file( - "Cargo.toml", - r#" -[package] -name = "test_change_text_to_binary" -version = "0.1.0" -edition = "2021" - -[dependencies] -insta = { path = '$PROJECT_PATH' } -"# - .to_string(), - ) - .add_file( - "src/main.rs", - r#" -#[test] -fn test() { - insta::assert_snapshot!("test"); -} -"# - .to_string(), - ) - .create_project(); - - let output = test_project - .insta_cmd() - .args(["test", "--accept"]) - .output() - .unwrap(); - - assert!(&output.status.success()); - assert_snapshot!(test_project.file_tree_diff(), @r" - --- Original file tree - +++ Updated file tree - @@ -1,4 +1,7 @@ - - + Cargo.lock - Cargo.toml - src - src/main.rs - + src/snapshots - + src/snapshots/test_change_text_to_binary__test.snap - "); - - test_project.update_file( - "src/main.rs", - r#" -#[test] -fn test() { - insta::assert_binary_snapshot!(".txt", b"test".to_vec()); -} -"# - .to_string(), - ); - - let output = test_project - .insta_cmd() - .args(["test", "--accept"]) - .output() - .unwrap(); - - assert!(&output.status.success()); - assert_snapshot!(test_project.file_tree_diff(), @r" - --- Original file tree - +++ Updated file tree - @@ -1,4 +1,8 @@ - - + Cargo.lock - Cargo.toml - src - src/main.rs - + src/snapshots - + src/snapshots/test_change_text_to_binary__test.snap - + src/snapshots/test_change_text_to_binary__test.snap.txt - "); + + insta::assert_snapshot!("foo", @"foo"); + } + "#####); } -/// When changing a snapshot from a binary to a text snapshot the previous binary file should be -/// gone after having approved the the binary snapshot. #[test] -fn test_change_binary_to_text() { +fn test_inline_snapshot_indent() { let test_project = TestFiles::new() .add_file( "Cargo.toml", r#" [package] -name = "test_change_binary_to_text" +name = "inline-indent" version = "0.1.0" edition = "2021" @@ -1678,311 +490,154 @@ insta = { path = '$PROJECT_PATH' } .to_string(), ) .add_file( - "src/main.rs", - r#" + "src/lib.rs", + r#####" #[test] -fn test() { - insta::assert_binary_snapshot!("some_name.json", b"{}".to_vec()); +fn test_wrong_indent_force() { + insta::assert_snapshot!(r#" + foo + foo + "#, @r#" + foo + foo + "#); } -"# - .to_string(), +"##### + .to_string(), ) .create_project(); + // ...and that it passes with `--require-full-match`. Note that ideally this + // would fail, but we can't read the desired indent without serde, which is + // in `cargo-insta` only. So this tests the current state rather than the + // ideal state (and I don't think there's a reasonable way to get the ideal state) + // Now confirm that `--require-full-match` passes let output = test_project .insta_cmd() - .args(["test", "--accept"]) - .output() - .unwrap(); - - assert!(&output.status.success()); - assert_snapshot!(test_project.file_tree_diff(), @r" - --- Original file tree - +++ Updated file tree - @@ -1,4 +1,8 @@ - - + Cargo.lock - Cargo.toml - src - src/main.rs - + src/snapshots - + src/snapshots/test_change_binary_to_text__some_name.snap - + src/snapshots/test_change_binary_to_text__some_name.snap.json - "); - - test_project.update_file( - "src/main.rs", - r#" -#[test] -fn test() { - insta::assert_snapshot!("some_name", "test"); -} -"# - .to_string(), - ); - - let output = test_project - .insta_cmd() - .args(["test", "--accept"]) + .args([ + "test", + "--check", + "--require-full-match", + "--", + "--nocapture", + ]) .output() .unwrap(); - assert!(&output.status.success()); - assert_snapshot!(test_project.file_tree_diff(), @r" - --- Original file tree - +++ Updated file tree - @@ -1,4 +1,7 @@ - - + Cargo.lock - Cargo.toml - src - src/main.rs - + src/snapshots - + src/snapshots/test_change_binary_to_text__some_name.snap - "); } -// Can't get the test binary discovery to work, don't have a windows machine to -// hand, others are welcome to fix it. (No specific reason to think that insta -// doesn't work on windows, just that the test doesn't work.) -#[cfg(not(target_os = "windows"))] #[test] -fn test_insta_workspace_root() { - // This function locates the compiled test binary in the target directory. - // It's necessary because the exact filename of the test binary includes a hash - // that we can't predict, so we need to search for it. - fn find_test_binary(dir: &Path) -> PathBuf { - dir.join("target/debug/deps") - .read_dir() - .unwrap() - .filter_map(Result::ok) - .find(|entry| { - let file_name = entry.file_name(); - let file_name_str = file_name.to_str().unwrap_or(""); - // We're looking for a file that: - file_name_str.starts_with("insta_workspace_root_test-") // Matches our test name - && !file_name_str.contains('.') // Doesn't have an extension (it's the executable, not a metadata file) - && entry.metadata().map(|m| m.is_file()).unwrap_or(false) // Is a file, not a directory - }) - .map(|entry| entry.path()) - .expect("Failed to find test binary") - } - - fn run_test_binary( - binary_path: &Path, - current_dir: &Path, - env: Option<(&str, &str)>, - ) -> std::process::Output { - let mut cmd = Command::new(binary_path); - TestProject::clean_env(&mut cmd); - cmd.current_dir(current_dir); - if let Some((key, value)) = env { - cmd.env(key, value); - } - cmd.output().unwrap() - } - +fn test_matches_fully_linebreaks() { + // Until #563 merges, we should be OK with different leading newlines, even + // in exact / full match mode. let test_project = TestFiles::new() .add_file( "Cargo.toml", r#" - [package] - name = "insta_workspace_root_test" - version = "0.1.0" - edition = "2021" - - [dependencies] - insta = { path = '$PROJECT_PATH' } - "# +[package] +name = "exact-match-inline" +version = "0.1.0" +edition = "2021" + +[lib] +doctest = false + +[dependencies] +insta = { path = '$PROJECT_PATH' } +"# .to_string(), ) .add_file( "src/lib.rs", - r#" -use insta::assert_snapshot; - + r#####" #[test] -fn test_snapshot() { - assert_snapshot!("Hello, world!"); +fn test_additional_linebreak() { + // Additional newline here + insta::assert_snapshot!(r#" + + ( + "name_foo", + "insta_tests__tests", + ) + "#, @r#" + ( + "name_foo", + "insta_tests__tests", + ) + "#); } - "# - .to_string(), +"##### + .to_string(), ) .create_project(); - let mut cargo_cmd = Command::new("cargo"); - TestProject::clean_env(&mut cargo_cmd); - let output = cargo_cmd - .args(["test", "--no-run"]) - .current_dir(&test_project.workspace_dir) + // Confirm the test passes despite the indent + let output = test_project + .insta_cmd() + .args([ + "test", + "--check", + "--require-full-match", + "--", + "--nocapture", + ]) .output() .unwrap(); assert!(&output.status.success()); - - let test_binary_path = find_test_binary(&test_project.workspace_dir); - - // Run the test without snapshot (should fail) - assert!( - !&run_test_binary(&test_binary_path, &test_project.workspace_dir, None,) - .status - .success() - ); - - // Create the snapshot - assert!(&run_test_binary( - &test_binary_path, - &test_project.workspace_dir, - Some(("INSTA_UPDATE", "always")), - ) - .status - .success()); - - // Verify snapshot creation - assert!(test_project.workspace_dir.join("src/snapshots").exists()); - assert!(test_project - .workspace_dir - .join("src/snapshots/insta_workspace_root_test__snapshot.snap") - .exists()); - - // Move the workspace - let moved_workspace = { - let moved_workspace = PathBuf::from("/tmp/cargo-insta-test-moved"); - remove_dir_all(&moved_workspace).ok(); - fs::create_dir(&moved_workspace).unwrap(); - fs::rename(&test_project.workspace_dir, &moved_workspace).unwrap(); - moved_workspace - }; - let moved_binary_path = find_test_binary(&moved_workspace); - - // Run test in moved workspace without INSTA_WORKSPACE_ROOT (should fail) - assert!( - !&run_test_binary(&moved_binary_path, &moved_workspace, None) - .status - .success() - ); - - // Run test in moved workspace with INSTA_WORKSPACE_ROOT (should pass) - assert!(&run_test_binary( - &moved_binary_path, - &moved_workspace, - Some(("INSTA_WORKSPACE_ROOT", moved_workspace.to_str().unwrap())), - ) - .status - .success()); } #[test] -fn test_external_test_path() { +fn test_snapshot_name_clash() { let test_project = TestFiles::new() .add_file( - "proj/Cargo.toml", + "Cargo.toml", r#" [package] -name = "external_test_path" +name = "snapshot_name_clash_test" version = "0.1.0" edition = "2021" +[lib] +doctest = false + [dependencies] insta = { path = '$PROJECT_PATH' } - -[[test]] -name = "tlib" -path = "../tests/lib.rs" "# .to_string(), ) .add_file( - "proj/src/lib.rs", + "src/lib.rs", r#" -pub fn hello() -> String { - "Hello, world!".to_string() +use insta::assert_debug_snapshot; + +#[test] +fn test_foo_always_missing() { + assert_debug_snapshot!(42); } -"# - .to_string(), - ) - .add_file( - "tests/lib.rs", - r#" -use external_test_path::hello; #[test] -fn test_hello() { - insta::assert_snapshot!(hello()); +fn foo_always_missing() { + assert_debug_snapshot!(42); } "# .to_string(), ) .create_project(); - // Change to the proj directory for running cargo commands - let proj_dir = test_project.workspace_dir.join("proj"); - - // Initially, the test should fail - let output = test_project - .insta_cmd() - .current_dir(&proj_dir) - .args(["test", "--"]) - .output() - .unwrap(); - - assert!(!&output.status.success()); - - // Verify that the snapshot was created in the correct location - assert_snapshot!(TestProject::current_file_tree(&test_project.workspace_dir), @r" - proj - proj/Cargo.lock - proj/Cargo.toml - proj/src - proj/src/lib.rs - tests - tests/lib.rs - tests/snapshots - tests/snapshots/tlib__hello.snap.new - "); - - // Run cargo insta accept let output = test_project .insta_cmd() - .current_dir(&proj_dir) - .args(["test", "--accept"]) + .args(["test", "--accept", "--", "--nocapture"]) + .stderr(Stdio::piped()) .output() .unwrap(); - assert!(&output.status.success()); - - // Verify that the snapshot was created in the correct location - assert_snapshot!(TestProject::current_file_tree(&test_project.workspace_dir), @r" - proj - proj/Cargo.lock - proj/Cargo.toml - proj/src - proj/src/lib.rs - tests - tests/lib.rs - tests/snapshots - tests/snapshots/tlib__hello.snap - "); - - // Run the test again, it should pass now - let output = Command::new(env!("CARGO_BIN_EXE_cargo-insta")) - .current_dir(&proj_dir) - .args(["test"]) - .output() - .unwrap(); + // The test should fail due to the name clash + assert!(!output.status.success()); - assert!(&output.status.success()); + let error_output = String::from_utf8_lossy(&output.stderr); - let snapshot_path = test_project - .workspace_dir - .join("tests/snapshots/tlib__hello.snap"); - assert_snapshot!(fs::read_to_string(snapshot_path).unwrap(), @r#" - --- - source: "../tests/lib.rs" - expression: hello() - snapshot_kind: text - --- - Hello, world! - "#); + // Check for the name clash error message + assert!(error_output.contains("Insta snapshot name clash detected between 'foo_always_missing' and 'test_foo_always_missing' in 'snapshot_name_clash_test'. Rename one function.")); } #[test] @@ -2238,88 +893,3 @@ src/ stderr ); } - -#[test] -fn test_binary_unreferenced_delete() { - let test_project = TestFiles::new() - .add_file( - "Cargo.toml", - r#" -[package] -name = "test_binary_unreferenced_delete" -version = "0.1.0" -edition = "2021" - -[lib] -doctest = false - -[dependencies] -insta = { path = '$PROJECT_PATH' } -"# - .to_string(), - ) - .add_file( - "src/lib.rs", - r#" -#[test] -fn test_snapshot() { - insta::assert_binary_snapshot!(".txt", b"abcd".to_vec()); -} -"# - .to_string(), - ) - .create_project(); - - // Run tests to create snapshots - let output = test_project - .insta_cmd() - .args(["test", "--accept"]) - .output() - .unwrap(); - - assert!(&output.status.success()); - - test_project.update_file("src/lib.rs", "".to_string()); - - assert_snapshot!(test_project.file_tree_diff(), @r" - --- Original file tree - +++ Updated file tree - @@ -1,4 +1,8 @@ - - + Cargo.lock - Cargo.toml - src - src/lib.rs - + src/snapshots - + src/snapshots/test_binary_unreferenced_delete__snapshot.snap - + src/snapshots/test_binary_unreferenced_delete__snapshot.snap.txt - "); - - // Run cargo insta test with --unreferenced=delete - let output = test_project - .insta_cmd() - .args([ - "test", - "--unreferenced=delete", - "--accept", - "--", - "--nocapture", - ]) - .output() - .unwrap(); - - assert!(&output.status.success()); - - // We should now see the unreferenced snapshot deleted - assert_snapshot!(test_project.file_tree_diff(), @r" - --- Original file tree - +++ Updated file tree - @@ -1,4 +1,6 @@ - - + Cargo.lock - Cargo.toml - src - src/lib.rs - + src/snapshots - "); -} diff --git a/cargo-insta/tests/functional/workspace.rs b/cargo-insta/tests/functional/workspace.rs new file mode 100644 index 00000000..d0c842dd --- /dev/null +++ b/cargo-insta/tests/functional/workspace.rs @@ -0,0 +1,595 @@ +use std::{ + fs, + process::{Command, Stdio}, +}; + +use insta::assert_snapshot; + +use crate::{TestFiles, TestProject}; + +// Note that names need to be different to prevent the cache confusing them. +fn workspace_with_root_crate(name: String) -> TestFiles { + TestFiles::new() + .add_file( + "Cargo.toml", + format!( + r#" +[package] +name = "{name}" +version = "0.1.0" +edition = "2021" + +[workspace] +members = [ + "member", +] + +[workspace.dependencies] +insta = {{path = '$PROJECT_PATH'}} + +[dependencies] +insta = {{ workspace = true }} + +"# + ) + .to_string(), + ) + .add_file( + "member/Cargo.toml", + format!( + r#" +[package] +name = "{name}-member" +version = "0.0.0" +edition = "2021" + +[dependencies] +insta = {{ workspace = true }} +"# + ) + .to_string(), + ) + .add_file( + "member/src/lib.rs", + r#" +#[test] +fn test_member() { + insta::assert_debug_snapshot!(vec![1, 2, 3]); +} +"# + .to_string(), + ) + .add_file( + "src/main.rs", + r#" +fn main() { + println!("Hello, world!"); +} + +#[test] +fn test_root() { + insta::assert_debug_snapshot!(vec![1, 2, 3]); +} +"# + .to_string(), + ) +} + +/// Check that in a workspace with a default root crate, running `cargo insta +/// test --workspace --accept` will update snapshots in both the root crate and the +/// member crate. +#[test] +fn test_root_crate_workspace_accept() { + let test_project = + workspace_with_root_crate("root-crate-workspace-accept".to_string()).create_project(); + + let output = test_project + .insta_cmd() + .args(["test", "--accept", "--workspace"]) + .output() + .unwrap(); + + assert!(&output.status.success()); + + assert_snapshot!(test_project.file_tree_diff(), @r###" + --- Original file tree + +++ Updated file tree + @@ -1,8 +1,13 @@ + + + Cargo.lock + Cargo.toml + member + member/Cargo.toml + member/src + member/src/lib.rs + + member/src/snapshots + + member/src/snapshots/root_crate_workspace_accept_member__member.snap + src + src/main.rs + + src/snapshots + + src/snapshots/root_crate_workspace_accept__root.snap + "### ); +} + +/// Check that in a workspace with a default root crate, running `cargo insta +/// test --workspace` will correctly report the number of pending snapshots +#[test] +fn test_root_crate_workspace() { + let test_project = + workspace_with_root_crate("root-crate-workspace".to_string()).create_project(); + + let output = test_project + .insta_cmd() + // Need to disable colors to assert the output below + .args(["test", "--workspace", "--color=never"]) + .stderr(Stdio::piped()) + .output() + .unwrap(); + + // 1.39 had a bug where it would claim there were 3 snapshots here + assert!( + String::from_utf8_lossy(&output.stderr).contains("info: 2 snapshots to review"), + "{}", + String::from_utf8_lossy(&output.stderr) + ); +} + +/// Check that in a workspace with a default root crate, running `cargo insta +/// test --accept` will only update snapshots in the root crate +#[test] +fn test_root_crate_no_all() { + let test_project = workspace_with_root_crate("root-crate-no-all".to_string()).create_project(); + + let output = test_project + .insta_cmd() + .args(["test", "--accept"]) + .output() + .unwrap(); + + assert!(&output.status.success()); + + assert_snapshot!(test_project.file_tree_diff(), @r###" + --- Original file tree + +++ Updated file tree + @@ -1,4 +1,5 @@ + + + Cargo.lock + Cargo.toml + member + member/Cargo.toml + @@ -6,3 +7,5 @@ + member/src/lib.rs + src + src/main.rs + + src/snapshots + + src/snapshots/root_crate_no_all__root.snap + "### ); +} + +fn workspace_with_virtual_manifest(name: String) -> TestFiles { + TestFiles::new() + .add_file( + "Cargo.toml", + r#" +[workspace] +members = [ + "member-1", + "member-2", +] + +[workspace.dependencies] +insta = {path = '$PROJECT_PATH'} +"# + .to_string() + .to_string(), + ) + .add_file( + "member-1/Cargo.toml", + format!( + r#" +[package] +name = "{name}-member-1" +version = "0.1.0" +edition = "2021" + +[dependencies] +insta = {{ workspace = true }} +"# + ) + .to_string(), + ) + .add_file( + "member-1/src/lib.rs", + r#" +#[test] +fn test_member_1() { + insta::assert_debug_snapshot!(vec![1, 2, 3]); +} +"# + .to_string(), + ) + .add_file( + "member-2/Cargo.toml", + format!( + r#" +[package] +name = "{name}-member-2" +version = "0.1.0" +edition = "2021" + +[dependencies] +insta = {{ workspace = true }} +"# + ) + .to_string(), + ) + .add_file( + "member-2/src/lib.rs", + r#" +#[test] +fn test_member_2() { + insta::assert_debug_snapshot!(vec![4, 5, 6]); +} +"# + .to_string(), + ) +} + +/// Check that in a workspace with a virtual manifest, running `cargo insta test +/// --workspace --accept` updates snapshots in all member crates. +#[test] +fn test_virtual_manifest_all() { + let test_project = + workspace_with_virtual_manifest("virtual-manifest-all".to_string()).create_project(); + + let output = test_project + .insta_cmd() + .args(["test", "--accept", "--workspace"]) + .output() + .unwrap(); + + assert!(&output.status.success()); + + assert_snapshot!(test_project.file_tree_diff(), @r###" + --- Original file tree + +++ Updated file tree + @@ -1,10 +1,15 @@ + + + Cargo.lock + Cargo.toml + member-1 + member-1/Cargo.toml + member-1/src + member-1/src/lib.rs + + member-1/src/snapshots + + member-1/src/snapshots/virtual_manifest_all_member_1__member_1.snap + member-2 + member-2/Cargo.toml + member-2/src + member-2/src/lib.rs + + member-2/src/snapshots + + member-2/src/snapshots/virtual_manifest_all_member_2__member_2.snap + "### ); +} + +/// Check that in a workspace with a virtual manifest, running `cargo insta test +/// --accept` updates snapshots in all member crates. +#[test] +fn test_virtual_manifest_default() { + let test_project = + workspace_with_virtual_manifest("virtual-manifest-default".to_string()).create_project(); + + let output = test_project + .insta_cmd() + .args(["test", "--accept"]) + .output() + .unwrap(); + + assert!(&output.status.success()); + + assert_snapshot!(test_project.file_tree_diff(), @r###" + --- Original file tree + +++ Updated file tree + @@ -1,10 +1,15 @@ + + + Cargo.lock + Cargo.toml + member-1 + member-1/Cargo.toml + member-1/src + member-1/src/lib.rs + + member-1/src/snapshots + + member-1/src/snapshots/virtual_manifest_default_member_1__member_1.snap + member-2 + member-2/Cargo.toml + member-2/src + member-2/src/lib.rs + + member-2/src/snapshots + + member-2/src/snapshots/virtual_manifest_default_member_2__member_2.snap + "### ); +} + +/// Check that in a workspace with a virtual manifest, running `cargo insta test +/// -p ` will only update snapshots in that crate. +#[test] +fn test_virtual_manifest_single_crate() { + let test_project = + workspace_with_virtual_manifest("virtual-manifest-single".to_string()).create_project(); + + let output = test_project + .insta_cmd() + .args(["test", "--accept", "-p", "virtual-manifest-single-member-1"]) + .output() + .unwrap(); + + assert!(&output.status.success()); + + assert_snapshot!(test_project.file_tree_diff(), @r###" + --- Original file tree + +++ Updated file tree + @@ -1,9 +1,12 @@ + + + Cargo.lock + Cargo.toml + member-1 + member-1/Cargo.toml + member-1/src + member-1/src/lib.rs + + member-1/src/snapshots + + member-1/src/snapshots/virtual_manifest_single_member_1__member_1.snap + member-2 + member-2/Cargo.toml + member-2/src + "### ); +} + +// Can't get the test binary discovery to work on Windows, don't have a windows +// machine to hand, others are welcome to fix it. (No specific reason to think +// that insta doesn't work on windows, just that the test doesn't work.) +#[cfg(not(target_os = "windows"))] +#[test] +fn test_insta_workspace_root() { + use std::{ + fs::{self, remove_dir_all}, + path::{Path, PathBuf}, + process::Command, + }; + + use crate::TestProject; + + // This function locates the compiled test binary in the target directory. + // It's necessary because the exact filename of the test binary includes a hash + // that we can't predict, so we need to search for it. + fn find_test_binary(dir: &Path) -> PathBuf { + dir.join("target/debug/deps") + .read_dir() + .unwrap() + .filter_map(Result::ok) + .find(|entry| { + let file_name = entry.file_name(); + let file_name_str = file_name.to_str().unwrap_or(""); + // We're looking for a file that: + file_name_str.starts_with("insta_workspace_root_test-") // Matches our test name + && !file_name_str.contains('.') // Doesn't have an extension (it's the executable, not a metadata file) + && entry.metadata().map(|m| m.is_file()).unwrap_or(false) // Is a file, not a directory + }) + .map(|entry| entry.path()) + .expect("Failed to find test binary") + } + + fn run_test_binary( + binary_path: &Path, + current_dir: &Path, + env: Option<(&str, &str)>, + ) -> std::process::Output { + let mut cmd = Command::new(binary_path); + TestProject::clean_env(&mut cmd); + cmd.current_dir(current_dir); + if let Some((key, value)) = env { + cmd.env(key, value); + } + cmd.output().unwrap() + } + + let test_project = TestFiles::new() + .add_file( + "Cargo.toml", + r#" + [package] + name = "insta_workspace_root_test" + version = "0.1.0" + edition = "2021" + + [dependencies] + insta = { path = '$PROJECT_PATH' } + "# + .to_string(), + ) + .add_file( + "src/lib.rs", + r#" +use insta::assert_snapshot; + +#[test] +fn test_snapshot() { + assert_snapshot!("Hello, world!"); +} + "# + .to_string(), + ) + .create_project(); + + let mut cargo_cmd = Command::new("cargo"); + TestProject::clean_env(&mut cargo_cmd); + let output = cargo_cmd + .args(["test", "--no-run"]) + .current_dir(&test_project.workspace_dir) + .output() + .unwrap(); + assert!(&output.status.success()); + + let test_binary_path = find_test_binary(&test_project.workspace_dir); + + // Run the test without snapshot (should fail) + assert!( + !&run_test_binary(&test_binary_path, &test_project.workspace_dir, None,) + .status + .success() + ); + + // Create the snapshot + assert!(&run_test_binary( + &test_binary_path, + &test_project.workspace_dir, + Some(("INSTA_UPDATE", "always")), + ) + .status + .success()); + + // Verify snapshot creation + assert!(test_project.workspace_dir.join("src/snapshots").exists()); + assert!(test_project + .workspace_dir + .join("src/snapshots/insta_workspace_root_test__snapshot.snap") + .exists()); + + // Move the workspace + let moved_workspace = { + let moved_workspace = PathBuf::from("/tmp/cargo-insta-test-moved"); + remove_dir_all(&moved_workspace).ok(); + fs::create_dir(&moved_workspace).unwrap(); + fs::rename(&test_project.workspace_dir, &moved_workspace).unwrap(); + moved_workspace + }; + let moved_binary_path = find_test_binary(&moved_workspace); + + // Run test in moved workspace without INSTA_WORKSPACE_ROOT (should fail) + assert!( + !&run_test_binary(&moved_binary_path, &moved_workspace, None) + .status + .success() + ); + + // Run test in moved workspace with INSTA_WORKSPACE_ROOT (should pass) + assert!(&run_test_binary( + &moved_binary_path, + &moved_workspace, + Some(("INSTA_WORKSPACE_ROOT", moved_workspace.to_str().unwrap())), + ) + .status + .success()); +} + +#[test] +fn test_external_test_path() { + let test_project = TestFiles::new() + .add_file( + "proj/Cargo.toml", + r#" +[package] +name = "external_test_path" +version = "0.1.0" +edition = "2021" + +[dependencies] +insta = { path = '$PROJECT_PATH' } + +[[test]] +name = "tlib" +path = "../tests/lib.rs" +"# + .to_string(), + ) + .add_file( + "proj/src/lib.rs", + r#" +pub fn hello() -> String { + "Hello, world!".to_string() +} +"# + .to_string(), + ) + .add_file( + "tests/lib.rs", + r#" +use external_test_path::hello; + +#[test] +fn test_hello() { + insta::assert_snapshot!(hello()); +} +"# + .to_string(), + ) + .create_project(); + + // Change to the proj directory for running cargo commands + let proj_dir = test_project.workspace_dir.join("proj"); + + // Initially, the test should fail + let output = test_project + .insta_cmd() + .current_dir(&proj_dir) + .args(["test", "--"]) + .output() + .unwrap(); + + assert!(!&output.status.success()); + + // Verify that the snapshot was created in the correct location + assert_snapshot!(TestProject::current_file_tree(&test_project.workspace_dir), @r" + proj + proj/Cargo.lock + proj/Cargo.toml + proj/src + proj/src/lib.rs + tests + tests/lib.rs + tests/snapshots + tests/snapshots/tlib__hello.snap.new + "); + + // Run cargo insta accept + let output = test_project + .insta_cmd() + .current_dir(&proj_dir) + .args(["test", "--accept"]) + .output() + .unwrap(); + + assert!(&output.status.success()); + + // Verify that the snapshot was created in the correct location + assert_snapshot!(TestProject::current_file_tree(&test_project.workspace_dir), @r" + proj + proj/Cargo.lock + proj/Cargo.toml + proj/src + proj/src/lib.rs + tests + tests/lib.rs + tests/snapshots + tests/snapshots/tlib__hello.snap + "); + + // Run the test again, it should pass now + let output = Command::new(env!("CARGO_BIN_EXE_cargo-insta")) + .current_dir(&proj_dir) + .args(["test"]) + .output() + .unwrap(); + + assert!(&output.status.success()); + + let snapshot_path = test_project + .workspace_dir + .join("tests/snapshots/tlib__hello.snap"); + assert_snapshot!(fs::read_to_string(snapshot_path).unwrap(), @r#" + --- + source: "../tests/lib.rs" + expression: hello() + snapshot_kind: text + --- + Hello, world! + "#); +} From a982c4c545b9b05e10dd9d646be3d583171e9ee2 Mon Sep 17 00:00:00 2001 From: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Date: Thu, 10 Oct 2024 20:25:38 -0700 Subject: [PATCH 17/22] Fix passing `--workspace-root` to `cargo-insta` (#658) This wasn't working, I think because of my recent changes -- now fixed and tested --- cargo-insta/src/cli.rs | 29 +++- cargo-insta/tests/functional/workspace.rs | 179 +++++++++++++++++++++- 2 files changed, 195 insertions(+), 13 deletions(-) diff --git a/cargo-insta/src/cli.rs b/cargo-insta/src/cli.rs index 369486b8..dd42ae00 100644 --- a/cargo-insta/src/cli.rs +++ b/cargo-insta/src/cli.rs @@ -387,6 +387,7 @@ fn handle_color(color: Option) { } } +#[derive(Debug)] struct LocationInfo<'a> { tool_config: ToolConfig, workspace_root: PathBuf, @@ -445,6 +446,14 @@ fn handle_target_args<'a>( let workspace_root = metadata.workspace_root.as_std_path().to_path_buf(); let tool_config = ToolConfig::from_workspace(&workspace_root)?; + let insta_version = metadata + .packages + .iter() + .find(|package| package.name == "insta") + .map(|package| package.version.clone()) + .ok_or_else(|| eprintln!("insta not found in cargo metadata; defaulting to 1.0.0")) + .unwrap_or(Version::new(1, 0, 0)); + // If `--workspace` is passed, or there's no root package, we include all // packages. If packages are specified, we filter from all packages. // Otherwise we use just the root package. @@ -460,17 +469,17 @@ fn handle_target_args<'a>( .into_iter() .filter(|p| packages.is_empty() || packages.contains(&p.name)) .cloned() + .map(|mut p| { + // Dependencies aren't needed and bloat the object (but we can't pass + // `--no-deps` to the original command as we collect the insta + // version above...) + p.dependencies = vec![]; + p + }) .collect() } else { vec![metadata.root_package().unwrap().clone()] }; - let insta_version = metadata - .packages - .iter() - .find(|package| package.name == "insta") - .map(|package| package.version.clone()) - .ok_or_else(|| eprintln!("insta not found in cargo metadata; defaulting to 1.0.0")) - .unwrap_or(Version::new(1, 0, 0)); Ok(LocationInfo { workspace_root, @@ -710,6 +719,10 @@ fn test_run(mut cmd: TestCommand, color: ColorWhen) -> Result<(), Box process_snapshots(true, None, &loc, Some(Operation::Reject))?; } + if let Some(workspace_root) = &cmd.target_args.workspace_root { + proc.current_dir(workspace_root); + } + // Run the tests let status = proc.status()?; let mut success = status.success(); @@ -1006,7 +1019,7 @@ fn prepare_test_runner<'snapshot_ref>( proc.arg("--exclude"); proc.arg(spec); } - if let Some(ref manifest_path) = cmd.target_args.manifest_path { + if let Some(ref manifest_path) = &cmd.target_args.manifest_path { proc.arg("--manifest-path"); proc.arg(manifest_path); } diff --git a/cargo-insta/tests/functional/workspace.rs b/cargo-insta/tests/functional/workspace.rs index d0c842dd..f70c8d50 100644 --- a/cargo-insta/tests/functional/workspace.rs +++ b/cargo-insta/tests/functional/workspace.rs @@ -1,7 +1,4 @@ -use std::{ - fs, - process::{Command, Stdio}, -}; +use std::{fs, process::Stdio}; use insta::assert_snapshot; @@ -480,6 +477,8 @@ fn test_snapshot() { .success()); } +/// A cargo target that references a file outside of the project's directory +/// should still work #[test] fn test_external_test_path() { let test_project = TestFiles::new() @@ -573,7 +572,8 @@ fn test_hello() { "); // Run the test again, it should pass now - let output = Command::new(env!("CARGO_BIN_EXE_cargo-insta")) + let output = test_project + .insta_cmd() .current_dir(&proj_dir) .args(["test"]) .output() @@ -593,3 +593,172 @@ fn test_hello() { Hello, world! "#); } + +/// Check that `--workspace-root` points `cargo-insta` at another path +#[test] +fn test_workspace_root_option() { + let test_project = TestFiles::new() + .add_file( + "Cargo.toml", + r#" +[package] +name = "workspace_root_test" +version = "0.1.0" +edition = "2021" + +[lib] +doctest = false + +[dependencies] +insta = { path = '$PROJECT_PATH' } +"# + .to_string(), + ) + .add_file( + "src/lib.rs", + r#" +pub fn hello() -> String { + "Hello from workspace root!".to_string() +} + +#[test] +fn test_hello() { + insta::assert_snapshot!(hello()); +} + +#[test] +fn test_inline() { + insta::assert_snapshot!("This is an inline snapshot", @""); +} +"# + .to_string(), + ) + .create_project(); + + // Run the test with --workspace-root option + let output = test_project + .insta_cmd() + .current_dir(std::env::current_dir().unwrap()) // Run from the current directory + .args([ + "test", + "--accept", + "--workspace-root", + test_project.workspace_dir.to_str().unwrap(), + ]) + .output() + .unwrap(); + + assert!(output.status.success()); + + // Verify inline snapshot + assert_snapshot!(test_project.diff("src/lib.rs"), @r#" + --- Original: src/lib.rs + +++ Updated: src/lib.rs + @@ -10,5 +10,5 @@ + + #[test] + fn test_inline() { + - insta::assert_snapshot!("This is an inline snapshot", @""); + + insta::assert_snapshot!("This is an inline snapshot", @"This is an inline snapshot"); + } + "#); + + assert_snapshot!(test_project.file_tree_diff(), @r" + --- Original file tree + +++ Updated file tree + @@ -1,4 +1,7 @@ + + + Cargo.lock + Cargo.toml + src + src/lib.rs + + src/snapshots + + src/snapshots/workspace_root_test__hello.snap + "); +} + +/// Check that `--manifest` points `cargo-insta` at another path +#[test] +fn test_manifest_option() { + let test_project = TestFiles::new() + .add_file( + "Cargo.toml", + r#" +[package] +name = "manifest_path_test" +version = "0.1.0" +edition = "2021" + +[lib] +doctest = false + +[dependencies] +insta = { path = '$PROJECT_PATH' } +"# + .to_string(), + ) + .add_file( + "src/lib.rs", + r#" +pub fn greeting() -> String { + "Greetings from manifest path!".to_string() +} + +#[test] +fn test_greeting() { + insta::assert_snapshot!(greeting()); +} + +#[test] +fn test_inline() { + insta::assert_snapshot!("This is an inline snapshot for manifest path test", @""); +} +"# + .to_string(), + ) + .create_project(); + + // Run the test with --manifest-path option + let output = test_project + .insta_cmd() + .current_dir(std::env::current_dir().unwrap()) // Run from the current directory + .args([ + "test", + "--accept", + "--manifest-path", + test_project + .workspace_dir + .join("Cargo.toml") + .to_str() + .unwrap(), + ]) + .output() + .unwrap(); + + assert!(output.status.success()); + + // Verify inline snapshot + assert_snapshot!(test_project.diff("src/lib.rs"), @r##" + --- Original: src/lib.rs + +++ Updated: src/lib.rs + @@ -10,5 +10,5 @@ + + #[test] + fn test_inline() { + - insta::assert_snapshot!("This is an inline snapshot for manifest path test", @""); + + insta::assert_snapshot!("This is an inline snapshot for manifest path test", @"This is an inline snapshot for manifest path test"); + } + "##); + assert_snapshot!(test_project.file_tree_diff(), @r" + --- Original file tree + +++ Updated file tree + @@ -1,4 +1,7 @@ + + + Cargo.lock + Cargo.toml + src + src/lib.rs + + src/snapshots + + src/snapshots/manifest_path_test__greeting.snap + "); +} From 6a37645bf68dfc02568f917dd1bea1539d19b529 Mon Sep 17 00:00:00 2001 From: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Date: Sat, 12 Oct 2024 08:42:13 -0700 Subject: [PATCH 18/22] Better error message if `cargo metadata` fails (#661) --- cargo-insta/src/cli.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/cargo-insta/src/cli.rs b/cargo-insta/src/cli.rs index dd42ae00..e377c2ef 100644 --- a/cargo-insta/src/cli.rs +++ b/cargo-insta/src/cli.rs @@ -442,7 +442,12 @@ fn handle_target_args<'a>( (None, None) => {} }; - let metadata = cmd.exec()?; + let metadata = cmd.exec().map_err(|e| { + format!( + "failed to load cargo metadata: {}. Command details: {:?}", + e, cmd + ) + })?; let workspace_root = metadata.workspace_root.as_std_path().to_path_buf(); let tool_config = ToolConfig::from_workspace(&workspace_root)?; From 10db5b042ef92dfb6b1c328042f4efa726f27ded Mon Sep 17 00:00:00 2001 From: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Date: Sat, 12 Oct 2024 08:44:52 -0700 Subject: [PATCH 19/22] Changelog refinements (#649) --- CHANGELOG.md | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index be1b18ba..86f78e15 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,16 +6,17 @@ All notable changes to insta and cargo-insta are documented here. - Experimental support for binary snapshots. #610 (Florian Plattner) -- `--force-update-snapshots` now writes every snapshot, regardless of whether - `insta` evaluates a write is required, and now implies `--accept`. This +- `--force-update-snapshots` now causes `cargo-insta` to write every snapshot, regardless of whether + it evaluates snapshots fully match, and now implies `--accept`. This allows for `--force-update-snapshots` to update inline snapshots when - delimiters or indents require updating. + delimiters or indentation can be updated. - For the existing behavior of limiting writes which `insta` can evaluate are - required, use `--require-full-match`. The main difference between + For the existing behavior of limiting writes to when `insta` evaluates writes + are required, use `--require-full-match`. The main difference between `--require-full-match` and the existing behavior of `--force-update-snapshots` - is that the test run will return a non-zero exit code if any snapshots don't - match fully. #644 + is that `cargo-insta` will return a non-zero exit code if any snapshots don't + match fully. `--require-full-match` doesn't track inline snapshots' delimiters or + indentation. #644 - Inline snapshots only use `#` characters as delimiters when required. #603 @@ -27,9 +28,12 @@ All notable changes to insta and cargo-insta are documented here. - Warnings are printed when any snapshot uses a legacy format. #599 - `insta` now internally uses `INSTA_UPDATE=force` rather than - `INSTA_FORCE_UPDATE`. (This doesn't affect users of `cargo-insta`, which + `INSTA_FORCE_UPDATE=1`. (This doesn't affect users of `cargo-insta`, which handles this internally.) #482 +- `cargo-insta`'s integration tests continue to grow over the past couple of versions, + and now offer coverage of most of `cargo-insta`'s interface. + ## 1.40.0 - `cargo-insta` no longer panics when running `cargo insta test --accept --workspace` From 2b8fce7be80305d5a9441efb5211a8c3041e5ef6 Mon Sep 17 00:00:00 2001 From: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Date: Sat, 12 Oct 2024 09:37:20 -0700 Subject: [PATCH 20/22] Consolidate `Cargo.toml` creation in functional tests (#662) --- cargo-insta/tests/functional/binary.rs | 139 ++++---------------- cargo-insta/tests/functional/inline.rs | 60 +++------ cargo-insta/tests/functional/main.rs | 146 ++++++---------------- cargo-insta/tests/functional/workspace.rs | 14 +-- 4 files changed, 83 insertions(+), 276 deletions(-) diff --git a/cargo-insta/tests/functional/binary.rs b/cargo-insta/tests/functional/binary.rs index 1f17ad33..5440c79b 100644 --- a/cargo-insta/tests/functional/binary.rs +++ b/cargo-insta/tests/functional/binary.rs @@ -6,21 +6,9 @@ use crate::TestFiles; #[test] fn test_binary_pending() { let test_project = TestFiles::new() + .add_cargo_toml("test_binary_pending") .add_file( - "Cargo.toml", - r#" -[package] -name = "test_binary_pending" -version = "0.1.0" -edition = "2021" - -[dependencies] -insta = { path = '$PROJECT_PATH' } -"# - .to_string(), - ) - .add_file( - "src/main.rs", + "src/lib.rs", r#" #[test] fn test_binary_snapshot() { @@ -43,7 +31,7 @@ fn test_binary_snapshot() { + Cargo.lock Cargo.toml src - src/main.rs + src/lib.rs + src/snapshots + src/snapshots/test_binary_pending__binary_snapshot.snap.new + src/snapshots/test_binary_pending__binary_snapshot.snap.new.txt @@ -54,21 +42,9 @@ fn test_binary_snapshot() { #[test] fn test_binary_accept() { let test_project = TestFiles::new() + .add_cargo_toml("test_binary_accept") .add_file( - "Cargo.toml", - r#" -[package] -name = "test_binary_accept" -version = "0.1.0" -edition = "2021" - -[dependencies] -insta = { path = '$PROJECT_PATH' } -"# - .to_string(), - ) - .add_file( - "src/main.rs", + "src/lib.rs", r#" #[test] fn test_binary_snapshot() { @@ -95,7 +71,7 @@ fn test_binary_snapshot() { + Cargo.lock Cargo.toml src - src/main.rs + src/lib.rs + src/snapshots + src/snapshots/test_binary_accept__binary_snapshot.snap + src/snapshots/test_binary_accept__binary_snapshot.snap.txt @@ -108,21 +84,9 @@ fn test_binary_snapshot() { #[test] fn test_binary_change_extension() { let test_project = TestFiles::new() + .add_cargo_toml("test_binary_change_extension") .add_file( - "Cargo.toml", - r#" -[package] -name = "test_binary_change_extension" -version = "0.1.0" -edition = "2021" - -[dependencies] -insta = { path = '$PROJECT_PATH' } -"# - .to_string(), - ) - .add_file( - "src/main.rs", + "src/lib.rs", r#" #[test] fn test_binary_snapshot() { @@ -142,7 +106,7 @@ fn test_binary_snapshot() { assert!(&output.status.success()); test_project.update_file( - "src/main.rs", + "src/lib.rs", r#" #[test] fn test_binary_snapshot() { @@ -164,7 +128,7 @@ fn test_binary_snapshot() { + Cargo.lock Cargo.toml src - src/main.rs + src/lib.rs + src/snapshots + src/snapshots/test_binary_change_extension__binary_snapshot.snap + src/snapshots/test_binary_change_extension__binary_snapshot.snap.new @@ -188,7 +152,7 @@ fn test_binary_snapshot() { + Cargo.lock Cargo.toml src - src/main.rs + src/lib.rs + src/snapshots + src/snapshots/test_binary_change_extension__binary_snapshot.snap + src/snapshots/test_binary_change_extension__binary_snapshot.snap.json @@ -200,21 +164,9 @@ fn test_binary_snapshot() { #[test] fn test_binary_pending_snapshot_removal() { let test_project = TestFiles::new() + .add_cargo_toml("test_binary_pending_snapshot_removal") .add_file( - "Cargo.toml", - r#" -[package] -name = "test_binary_pending_snapshot_removal" -version = "0.1.0" -edition = "2021" - -[dependencies] -insta = { path = '$PROJECT_PATH' } -"# - .to_string(), - ) - .add_file( - "src/main.rs", + "src/lib.rs", r#" #[test] fn test_binary_snapshot() { @@ -229,7 +181,7 @@ fn test_binary_snapshot() { assert!(!&output.status.success()); - test_project.update_file("src/main.rs", "".to_string()); + test_project.update_file("src/lib.rs", "".to_string()); let output = test_project.insta_cmd().args(["test"]).output().unwrap(); @@ -243,7 +195,7 @@ fn test_binary_snapshot() { + Cargo.lock Cargo.toml src - src/main.rs + src/lib.rs + src/snapshots "); } @@ -253,21 +205,9 @@ fn test_binary_snapshot() { #[test] fn test_change_text_to_binary() { let test_project = TestFiles::new() + .add_cargo_toml("test_change_text_to_binary") .add_file( - "Cargo.toml", - r#" -[package] -name = "test_change_text_to_binary" -version = "0.1.0" -edition = "2021" - -[dependencies] -insta = { path = '$PROJECT_PATH' } -"# - .to_string(), - ) - .add_file( - "src/main.rs", + "src/lib.rs", r#" #[test] fn test() { @@ -293,13 +233,13 @@ fn test() { + Cargo.lock Cargo.toml src - src/main.rs + src/lib.rs + src/snapshots + src/snapshots/test_change_text_to_binary__test.snap "); test_project.update_file( - "src/main.rs", + "src/lib.rs", r#" #[test] fn test() { @@ -324,7 +264,7 @@ fn test() { + Cargo.lock Cargo.toml src - src/main.rs + src/lib.rs + src/snapshots + src/snapshots/test_change_text_to_binary__test.snap + src/snapshots/test_change_text_to_binary__test.snap.txt @@ -336,21 +276,9 @@ fn test() { #[test] fn test_change_binary_to_text() { let test_project = TestFiles::new() + .add_cargo_toml("test_change_binary_to_text") .add_file( - "Cargo.toml", - r#" -[package] -name = "test_change_binary_to_text" -version = "0.1.0" -edition = "2021" - -[dependencies] -insta = { path = '$PROJECT_PATH' } -"# - .to_string(), - ) - .add_file( - "src/main.rs", + "src/lib.rs", r#" #[test] fn test() { @@ -376,14 +304,14 @@ fn test() { + Cargo.lock Cargo.toml src - src/main.rs + src/lib.rs + src/snapshots + src/snapshots/test_change_binary_to_text__some_name.snap + src/snapshots/test_change_binary_to_text__some_name.snap.json "); test_project.update_file( - "src/main.rs", + "src/lib.rs", r#" #[test] fn test() { @@ -408,7 +336,7 @@ fn test() { + Cargo.lock Cargo.toml src - src/main.rs + src/lib.rs + src/snapshots + src/snapshots/test_change_binary_to_text__some_name.snap "); @@ -417,22 +345,7 @@ fn test() { #[test] fn test_binary_unreferenced_delete() { let test_project = TestFiles::new() - .add_file( - "Cargo.toml", - r#" -[package] -name = "test_binary_unreferenced_delete" -version = "0.1.0" -edition = "2021" - -[lib] -doctest = false - -[dependencies] -insta = { path = '$PROJECT_PATH' } -"# - .to_string(), - ) + .add_cargo_toml("test_binary_unreferenced_delete") .add_file( "src/lib.rs", r#" diff --git a/cargo-insta/tests/functional/inline.rs b/cargo-insta/tests/functional/inline.rs index 4d5aaaad..26579121 100644 --- a/cargo-insta/tests/functional/inline.rs +++ b/cargo-insta/tests/functional/inline.rs @@ -20,7 +20,7 @@ serde = { version = "1.0", features = ["derive"] } .to_string(), ) .add_file( - "src/main.rs", + "src/lib.rs", r#" use serde::Serialize; @@ -53,9 +53,9 @@ fn test_json_snapshot() { assert!(&output.status.success()); - assert_snapshot!(test_project.diff("src/main.rs"), @r##" - --- Original: src/main.rs - +++ Updated: src/main.rs + assert_snapshot!(test_project.diff("src/lib.rs"), @r##" + --- Original: src/lib.rs + +++ Updated: src/lib.rs @@ -15,5 +15,10 @@ }; insta::assert_json_snapshot!(&user, { @@ -89,7 +89,7 @@ serde = { version = "1.0", features = ["derive"] } .to_string(), ) .add_file( - "src/main.rs", + "src/lib.rs", r#" use serde::Serialize; @@ -122,9 +122,9 @@ fn test_yaml_snapshot() { assert!(&output.status.success()); - assert_snapshot!(test_project.diff("src/main.rs"), @r###" - --- Original: src/main.rs - +++ Updated: src/main.rs + assert_snapshot!(test_project.diff("src/lib.rs"), @r###" + --- Original: src/lib.rs + +++ Updated: src/lib.rs @@ -15,5 +15,8 @@ }; insta::assert_yaml_snapshot!(&user, { @@ -141,21 +141,9 @@ fn test_yaml_snapshot() { #[test] fn test_utf8_inline() { let test_project = TestFiles::new() + .add_cargo_toml("test_utf8_inline") .add_file( - "Cargo.toml", - r#" -[package] -name = "test_utf8_inline" -version = "0.1.0" -edition = "2021" - -[dependencies] -insta = { path = '$PROJECT_PATH' } -"# - .to_string(), - ) - .add_file( - "src/main.rs", + "src/lib.rs", r#" #[test] fn test_non_basic_plane() { @@ -197,9 +185,9 @@ fn test_trailing_comma_in_inline_snapshot() { assert!(&output.status.success()); - assert_snapshot!(test_project.diff("src/main.rs"), @r##" - --- Original: src/main.rs - +++ Updated: src/main.rs + assert_snapshot!(test_project.diff("src/lib.rs"), @r##" + --- Original: src/lib.rs + +++ Updated: src/lib.rs @@ -1,21 +1,19 @@ #[test] @@ -310,21 +298,9 @@ fn test_old_yaml_format() { #[test] fn test_hashtag_escape_in_inline_snapshot() { let test_project = TestFiles::new() + .add_cargo_toml("test_hashtag_escape") .add_file( - "Cargo.toml", - r#" -[package] -name = "test_hashtag_escape" -version = "0.1.0" -edition = "2021" - -[dependencies] -insta = { path = '$PROJECT_PATH' } -"# - .to_string(), - ) - .add_file( - "src/main.rs", + "src/lib.rs", r#####" #[test] fn test_hashtag_escape() { @@ -344,9 +320,9 @@ fn test_hashtag_escape() { assert!(&output.status.success()); - assert_snapshot!(test_project.diff("src/main.rs"), @r####" - --- Original: src/main.rs - +++ Updated: src/main.rs + assert_snapshot!(test_project.diff("src/lib.rs"), @r####" + --- Original: src/lib.rs + +++ Updated: src/lib.rs @@ -2,5 +2,8 @@ #[test] fn test_hashtag_escape() { diff --git a/cargo-insta/tests/functional/main.rs b/cargo-insta/tests/functional/main.rs index d92cd865..5eb02f34 100644 --- a/cargo-insta/tests/functional/main.rs +++ b/cargo-insta/tests/functional/main.rs @@ -106,6 +106,29 @@ impl TestFiles { self } + /// Adds a standard `Cargo.toml` (some tests may need to add_file themselves + /// with a different format) + fn add_cargo_toml(self, name: &str) -> Self { + self.add_file( + "Cargo.toml", + format!( + r#" +[package] +name = "{}" +version = "0.1.0" +edition = "2021" + +[lib] +doctest = false + +[dependencies] +insta = {{ path = '$PROJECT_PATH' }} +"#, + name + ), + ) + } + fn create_project(self) -> TestProject { TestProject::new(self.files) } @@ -314,22 +337,22 @@ Hello, world! let test_insta_1_40_0 = create_test_force_update_project("1_40_0", "\"1.40.0\""); // Test with current insta version - let output_current = test_current_insta + assert!(&test_current_insta .insta_cmd() .args(["test", "--accept", "--force-update-snapshots"]) .output() - .unwrap(); - - assert!(&output_current.status.success()); + .unwrap() + .status + .success()); // Test with insta 1.40.0 - let output_1_40_0 = test_insta_1_40_0 + assert!(&test_insta_1_40_0 .insta_cmd() .args(["test", "--accept", "--force-update-snapshots"]) .output() - .unwrap(); - - assert!(&output_1_40_0.status.success()); + .unwrap() + .status + .success()); // Check that both versions updated the snapshot correctly assert_snapshot!(test_current_insta.diff("src/snapshots/test_force_update_current__force_update.snap"), @r#" @@ -368,19 +391,7 @@ Hello, world! #[test] fn test_force_update_inline_snapshot_linebreaks() { let test_project = TestFiles::new() - .add_file( - "Cargo.toml", - r#" -[package] -name = "force-update-inline-linebreaks" -version = "0.1.0" -edition = "2021" - -[dependencies] -insta = { path = '$PROJECT_PATH' } -"# - .to_string(), - ) + .add_cargo_toml("force-update-inline-linebreaks") .add_file( "src/lib.rs", r#####" @@ -425,19 +436,7 @@ fn test_linebreaks() { #[test] fn test_force_update_inline_snapshot_hashes() { let test_project = TestFiles::new() - .add_file( - "Cargo.toml", - r#" -[package] -name = "force-update-inline-hashes" -version = "0.1.0" -edition = "2021" - -[dependencies] -insta = { path = '$PROJECT_PATH' } -"# - .to_string(), - ) + .add_cargo_toml("force-update-inline-hashes") .add_file( "src/lib.rs", r#####" @@ -531,22 +530,7 @@ fn test_matches_fully_linebreaks() { // Until #563 merges, we should be OK with different leading newlines, even // in exact / full match mode. let test_project = TestFiles::new() - .add_file( - "Cargo.toml", - r#" -[package] -name = "exact-match-inline" -version = "0.1.0" -edition = "2021" - -[lib] -doctest = false - -[dependencies] -insta = { path = '$PROJECT_PATH' } -"# - .to_string(), - ) + .add_cargo_toml("exact-match-inline") .add_file( "src/lib.rs", r#####" @@ -589,22 +573,7 @@ fn test_additional_linebreak() { #[test] fn test_snapshot_name_clash() { let test_project = TestFiles::new() - .add_file( - "Cargo.toml", - r#" -[package] -name = "snapshot_name_clash_test" -version = "0.1.0" -edition = "2021" - -[lib] -doctest = false - -[dependencies] -insta = { path = '$PROJECT_PATH' } -"# - .to_string(), - ) + .add_cargo_toml("snapshot_name_clash_test") .add_file( "src/lib.rs", r#" @@ -643,22 +612,7 @@ fn foo_always_missing() { #[test] fn test_unreferenced_delete() { let test_project = TestFiles::new() - .add_file( - "Cargo.toml", - r#" -[package] -name = "test_unreferenced_delete" -version = "0.1.0" -edition = "2021" - -[lib] -doctest = false - -[dependencies] -insta = { path = '$PROJECT_PATH' } -"# - .to_string(), - ) + .add_cargo_toml("test_unreferenced_delete") .add_file( "src/lib.rs", r#" @@ -743,19 +697,7 @@ Unused snapshot #[test] fn test_hidden_snapshots() { let test_project = TestFiles::new() - .add_file( - "Cargo.toml", - r#" -[package] -name = "test_hidden_snapshots" -version = "0.1.0" -edition = "2021" - -[dependencies] -insta = { path = '$PROJECT_PATH' } -"# - .to_string(), - ) + .add_cargo_toml("test_hidden_snapshots") .add_file( "src/lib.rs", r#" @@ -823,19 +765,7 @@ Hidden snapshot #[test] fn test_ignored_snapshots() { let test_project = TestFiles::new() - .add_file( - "Cargo.toml", - r#" -[package] -name = "test_ignored_snapshots" -version = "0.1.0" -edition = "2021" - -[dependencies] -insta = { path = '$PROJECT_PATH' } -"# - .to_string(), - ) + .add_cargo_toml("test_ignored_snapshots") .add_file( "src/lib.rs", r#" diff --git a/cargo-insta/tests/functional/workspace.rs b/cargo-insta/tests/functional/workspace.rs index f70c8d50..0b4841e7 100644 --- a/cargo-insta/tests/functional/workspace.rs +++ b/cargo-insta/tests/functional/workspace.rs @@ -389,19 +389,7 @@ fn test_insta_workspace_root() { } let test_project = TestFiles::new() - .add_file( - "Cargo.toml", - r#" - [package] - name = "insta_workspace_root_test" - version = "0.1.0" - edition = "2021" - - [dependencies] - insta = { path = '$PROJECT_PATH' } - "# - .to_string(), - ) + .add_cargo_toml("insta_workspace_root_test") .add_file( "src/lib.rs", r#" From 4f16d70a377bbb5948df7d16900584d7c47abe4a Mon Sep 17 00:00:00 2001 From: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Date: Sat, 12 Oct 2024 11:04:42 -0700 Subject: [PATCH 21/22] Show hidden files in functional tests (#663) Needed for a different branch, committing this change to `main` --- cargo-insta/tests/functional/main.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/cargo-insta/tests/functional/main.rs b/cargo-insta/tests/functional/main.rs index 5eb02f34..ade947bc 100644 --- a/cargo-insta/tests/functional/main.rs +++ b/cargo-insta/tests/functional/main.rs @@ -251,6 +251,7 @@ impl TestProject { fn current_file_tree(workspace_dir: &Path) -> String { WalkBuilder::new(workspace_dir) + .hidden(false) .filter_entry(|e| e.path().file_name() != Some(std::ffi::OsStr::new("target"))) .build() .filter_map(|e| e.ok()) From a8b6cc221c07df25fca3bc1578fedfc0670027b6 Mon Sep 17 00:00:00 2001 From: Maximilian Roos <5635139+max-sixty@users.noreply.github.com> Date: Sat, 12 Oct 2024 11:06:06 -0700 Subject: [PATCH 22/22] Small refactors (#664) --- cargo-insta/src/cli.rs | 31 ++++++++++++++-------------- cargo-insta/tests/functional/main.rs | 2 +- insta/src/snapshot.rs | 2 +- 3 files changed, 17 insertions(+), 18 deletions(-) diff --git a/cargo-insta/src/cli.rs b/cargo-insta/src/cli.rs index e377c2ef..661af6a4 100644 --- a/cargo-insta/src/cli.rs +++ b/cargo-insta/src/cli.rs @@ -832,23 +832,22 @@ fn handle_unreferenced_snapshots( UnreferencedSnapshots::Ignore => return Ok(()), }; - let mut files = HashSet::new(); - match fs::read_to_string(snapshot_ref_path) { - Ok(s) => { - for line in s.lines() { - if let Ok(path) = fs::canonicalize(line) { - files.insert(path); - } - } - } - Err(err) => { - // if the file was not created, no test referenced - // snapshots. - if err.kind() != io::ErrorKind::NotFound { - return Err(err.into()); + let files = fs::read_to_string(snapshot_ref_path) + .map(|s| { + s.lines() + .filter_map(|line| fs::canonicalize(line).ok()) + .collect() + }) + .or_else(|err| { + if err.kind() == io::ErrorKind::NotFound { + // if the file was not created, no test referenced + // snapshots (though we also check for this in the calling + // function, so maybe duplicative...) + Ok(HashSet::new()) + } else { + Err(err) } - } - } + })?; let mut encountered_any = false; diff --git a/cargo-insta/tests/functional/main.rs b/cargo-insta/tests/functional/main.rs index ade947bc..67438f93 100644 --- a/cargo-insta/tests/functional/main.rs +++ b/cargo-insta/tests/functional/main.rs @@ -226,7 +226,7 @@ impl TestProject { format!("{} {}", style(&stdout_name).green(), line) })) .stderr(OutputFormatter(move |line| { - format!("{} {}", style(&stderr_name).red(), line) + format!("{} {}", style(&stderr_name).yellow(), line) })); command diff --git a/insta/src/snapshot.rs b/insta/src/snapshot.rs index a11ca207..d815a559 100644 --- a/insta/src/snapshot.rs +++ b/insta/src/snapshot.rs @@ -385,7 +385,7 @@ impl Snapshot { } } } - elog!("A snapshot uses a legacy snapshot format; please update it to the new format with `cargo insta test --force-update-snapshots --accept`.\n\nSnapshot is at: {}", p.to_string_lossy()); + elog!("A snapshot uses a legacy snapshot format; please update it to the new format with `cargo insta test --force-update-snapshots --accept`.\nSnapshot is at: {}", p.to_string_lossy()); rv };