Repository: mitsuhiko/insta
Pull request #639 (2024-10-05 19:30:18)

Fix `--require-full-match` on inline snapshots with leading newlines
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.)
```


Patch:
diff --git a/cargo-insta/src/cli.rs b/cargo-insta/src/cli.rs
index 4d56ed8..f2e5a34 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<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().unwrap());
-                let new_snapshot = snapshot_ref.new.contents_string().unwrap();
+                let old_snapshot = snapshot_ref.old.as_ref().map(|x| match x.contents() {
+                    SnapshotContents::Text(x) => 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 776f8df..c52a492 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 08037b8..8086faa 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()
@@ -542,20 +555,12 @@ impl Snapshot {
         }
     }
 
-    /// The normalized snapshot contents as a String
-    pub fn contents_string(&self) -> Option<String> {
-        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 +641,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 {
@@ -822,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


Test patch:
diff --git a/cargo-insta/tests/main.rs b/cargo-insta/tests/main.rs
index e226056..0b13929 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]