Adjust force-update on inline snapshots to only view within the string (

#581)

This solves the issue in #573 for the moment:
- When `--force-update-snapshots` in passed, we only update inline
snapshots when there's some difference _within_ the string, such as an
additional linebreak at the start or the end
- We no longer update the surrounding hashes. In the existing code, this
happened because we were writing too many inline snapshots, not because
we were actually checking the number of hashes
- Checking the number of hashes isn't easy to do — reason outlined at
#573 (comment)
- The main change in the code is that we store the unnormalized snapshot
and then normalize when we need to. The existing code stored the
normalized version, which meant we couldn't evaluate whether the
unnormalized versions were different

It's a decent number of changes, but will integrate nicely with
#563.

~(FYI we currently don't look at the indentation, but we could adjust
this)~ Now indentation works too

CHANGELOG.md

@@ -4,6 +4,11 @@ All notable changes to insta and cargo-insta are documented here.
 
 ## 1.41.0
 
+- `--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
+
 ## 1.40.0
 
 - `cargo-insta` no longer panics when running `cargo insta test --accept --workspace`

cargo-insta/src/cli.rs

@@ -1077,12 +1077,14 @@ fn pending_snapshots_cmd(cmd: PendingSnapshotsCommand) -> Result<(), Box<dyn Err
         let is_inline = snapshot_container.snapshot_file().is_none();
         for snapshot_ref in snapshot_container.iter_snapshots() {
             if cmd.as_json {
+                let old_snapshot = snapshot_ref.old.as_ref().map(|x| x.contents_string());
+                let new_snapshot = snapshot_ref.new.contents_string();
                 let info = if is_inline {
                     SnapshotKey::InlineSnapshot {
                         path: &target_file,
                         line: snapshot_ref.line.unwrap(),
-                        old_snapshot: snapshot_ref.old.as_ref().map(|x| x.contents_str()),
-                        new_snapshot: snapshot_ref.new.contents_str(),
+                        old_snapshot: old_snapshot.as_deref(),
+                        new_snapshot: &new_snapshot,
                         expression: snapshot_ref.new.metadata().expression(),
                     }
                 } else {

cargo-insta/src/container.rs

@@ -3,6 +3,7 @@ use std::fs;
 use std::path::{Path, PathBuf};
 
 use insta::Snapshot;
+pub(crate) use insta::SnapshotKind;
 use insta::_cargo_insta_support::{ContentError, PendingInlineSnapshot};
 
 use crate::inline::FilePatcher;
@@ -14,12 +15,6 @@ pub(crate) enum Operation {
     Skip,
 }
 
-#[derive(Debug)]
-pub(crate) enum SnapshotContainerKind {
-    Inline,
-    External,
-}
-
 #[derive(Debug)]
 pub(crate) struct PendingSnapshot {
     #[allow(dead_code)]
@@ -54,7 +49,7 @@ impl PendingSnapshot {
 pub(crate) struct SnapshotContainer {
     snapshot_path: PathBuf,
     target_path: PathBuf,
-    kind: SnapshotContainerKind,
+    kind: SnapshotKind,
     snapshots: Vec<PendingSnapshot>,
     patcher: Option<FilePatcher>,
 }
@@ -63,11 +58,11 @@ impl SnapshotContainer {
     pub(crate) fn load(
         snapshot_path: PathBuf,
         target_path: PathBuf,
-        kind: SnapshotContainerKind,
+        kind: SnapshotKind,
     ) -> Result<SnapshotContainer, Box<dyn Error>> {
         let mut snapshots = Vec::new();
         let patcher = match kind {
-            SnapshotContainerKind::External => {
+            SnapshotKind::File => {
                 let old = if fs::metadata(&target_path).is_err() {
                     None
                 } else {
@@ -83,7 +78,7 @@ impl SnapshotContainer {
                 });
                 None
             }
-            SnapshotContainerKind::Inline => {
+            SnapshotKind::Inline => {
                 let mut pending_vec = PendingInlineSnapshot::load_batch(&snapshot_path)?;
                 let mut have_new = false;
 
@@ -139,8 +134,8 @@ impl SnapshotContainer {
 
     pub(crate) fn snapshot_file(&self) -> Option<&Path> {
         match self.kind {
-            SnapshotContainerKind::External => Some(&self.target_path),
-            SnapshotContainerKind::Inline => None,
+            SnapshotKind::File => Some(&self.target_path),
+            SnapshotKind::Inline => None,
         }
     }
 

cargo-insta/src/walk.rs

@@ -5,7 +5,7 @@ use std::path::Path;
 use ignore::overrides::OverrideBuilder;
 use ignore::{DirEntry, Walk, WalkBuilder};
 
-use crate::container::{SnapshotContainer, SnapshotContainerKind};
+use crate::container::{SnapshotContainer, SnapshotKind};
 
 #[derive(Debug, Copy, Clone)]
 pub(crate) struct FindFlags {
@@ -37,15 +37,15 @@ pub(crate) fn find_pending_snapshots<'a>(
                 Some(SnapshotContainer::load(
                     new_path,
                     old_path,
-                    SnapshotContainerKind::External,
+                    SnapshotKind::File,
                 ))
             } else if fname.starts_with('.') && fname.ends_with(".pending-snap") {
                 let mut target_path = e.path().to_path_buf();
                 target_path.set_file_name(&fname[1..fname.len() - 13]);
                 Some(SnapshotContainer::load(
                     e.path().to_path_buf(),
                     target_path,
-                    SnapshotContainerKind::Inline,
+                    SnapshotKind::Inline,
                 ))
             } else {
                 None

cargo-insta/tests/main.rs

@@ -905,13 +905,13 @@ Hello, world!
 }
 
 #[test]
-fn test_force_update_inline_snapshot() {
+fn test_force_update_inline_snapshot_linebreaks() {
     let test_project = TestFiles::new()
         .add_file(
             "Cargo.toml",
             r#"
 [package]
-name = "force-update-inline"
+name = "force-update-inline-linebreaks"
 version = "0.1.0"
 edition = "2021"
 
@@ -924,8 +924,11 @@ insta = { path = '$PROJECT_PATH' }
             "src/lib.rs",
             r#####"
 #[test]
-fn test_excessive_hashes() {
-    insta::assert_snapshot!("foo", @r####"foo"####);
+fn test_linebreaks() {
+    insta::assert_snapshot!("foo", @r####"
+    foo
+    
+    "####);
 }
 "#####
                 .to_string(),
@@ -950,16 +953,143 @@ fn test_excessive_hashes() {
     assert_snapshot!(test_project.diff("src/lib.rs"), @r#####"
     --- Original: src/lib.rs
     +++ Updated: src/lib.rs
-    @@ -1,5 +1,5 @@
+    @@ -1,8 +1,5 @@
      
      #[test]
-     fn test_excessive_hashes() {
-    -    insta::assert_snapshot!("foo", @r####"foo"####);
+     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
+        .cmd()
+        .args([
+            "test",
+            "--force-update-snapshots",
+            "--accept",
+            "--",
+            "--nocapture",
+        ])
+        .output()
+        .unwrap();
+
+    assert_success(&output);
+
+    // 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"), @"");
+}
+
+#[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();
+
+    // Confirm the test passes despite the indent
+    let output = test_project
+        .cmd()
+        .args(["test", "--check", "--", "--nocapture"])
+        .output()
+        .unwrap();
+    assert_success(&output);
+
+    // Then run the test with --force-update-snapshots and --accept to confirm
+    // the new snapshot is written
+    let output = test_project
+        .cmd()
+        .args([
+            "test",
+            "--force-update-snapshots",
+            "--accept",
+            "--",
+            "--nocapture",
+        ])
+        .output()
+        .unwrap();
+    assert_success(&output);
+
+    // 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
+    @@ -5,7 +5,7 @@
+         foo
+         foo
+         "#, @r#"
+    -                foo
+    -                foo
+    +    foo
+    +    foo
+         "#);
+     }
+    "##);
+}
+
 #[test]
 fn test_hashtag_escape_in_inline_snapshot() {
     let test_project = TestFiles::new()
@@ -981,7 +1111,7 @@ insta = { path = '$PROJECT_PATH' }
             r#"
 #[test]
 fn test_hashtag_escape() {
-    insta::assert_snapshot!("Value with #### hashtags\n", @"");
+    insta::assert_snapshot!("Value with\n#### hashtags\n", @"");
 }
 "#
             .to_string(),
@@ -999,13 +1129,14 @@ fn test_hashtag_escape() {
     assert_snapshot!(test_project.diff("src/main.rs"), @r######"
     --- Original: src/main.rs
     +++ Updated: src/main.rs
-    @@ -1,5 +1,7 @@
+    @@ -1,5 +1,8 @@
      
      #[test]
      fn test_hashtag_escape() {
-    -    insta::assert_snapshot!("Value with #### hashtags\n", @"");
-    +    insta::assert_snapshot!("Value with #### hashtags\n", @r#####"
-    +    Value with #### hashtags
+    -    insta::assert_snapshot!("Value with\n#### hashtags\n", @"");
+    +    insta::assert_snapshot!("Value with\n#### hashtags\n", @r#####"
+    +    Value with
+    +    #### hashtags
     +    "#####);
      }
     "######);

insta/src/lib.rs

@@ -287,7 +287,7 @@ mod glob;
 mod test;
 
 pub use crate::settings::Settings;
-pub use crate::snapshot::{MetaData, Snapshot};
+pub use crate::snapshot::{MetaData, Snapshot, SnapshotKind};
 
 /// Exposes some library internals.
 ///

insta/src/output.rs

@@ -103,7 +103,7 @@ impl<'a> SnapshotPrinter<'a> {
     fn print_snapshot(&self) {
         print_line(term_width());
 
-        let new_contents = self.new_snapshot.contents_str();
+        let new_contents = self.new_snapshot.contents_string();
 
         let width = term_width();
         if self.show_info {
@@ -118,15 +118,17 @@ impl<'a> SnapshotPrinter<'a> {
     }
 
     fn print_changeset(&self) {
-        let old = self.old_snapshot.as_ref().map_or("", |x| x.contents_str());
-        let new = self.new_snapshot.contents_str();
-        let newlines_matter = newlines_matter(old, new);
+        let old: String = self
+            .old_snapshot
+            .map_or("".to_string(), |x| x.contents_string());
+        let new = self.new_snapshot.contents_string();
+        let newlines_matter = newlines_matter(old.as_str(), new.as_str());
 
         let width = term_width();
         let diff = TextDiff::configure()
             .algorithm(Algorithm::Patience)
             .timeout(Duration::from_millis(500))
-            .diff_lines(old, new);
+            .diff_lines(old.as_str(), new.as_str());
         print_line(width);
 
         if self.show_info {