Unify handling of file & inline snapshots (compat) (#528)

This is a version of #468 that doesn't break compatibility — the change in snapshot (removing the leading `---` in inline snapshots) is optional, and snapshots still pass if they have it. Otherwise it has the same benefits of #468 (There is a tiny corner case that if a snapshot is intended to test whether there's a leading `---`, we could get a false match, but this is a very small possibility — much much smaller than removing the line endings we've done) As part of that, this adds `as_str_exact` & `matches_fully` methods to `SnapshotContents`, which allows us to unify our normalization of snapshots contents, and allow detecting snapshots that pass but don't completely match; for example if we want to restore checking for line endings.
mitsuhiko · Aug 31, 2024 · 4bea0fb · 4bea0fb
1 parent c29db01
commit 4bea0fb
Show file tree

Hide file tree

Showing 7 changed files with 55 additions and 74 deletions.
diff --git a/cargo-insta/tests/main.rs b/cargo-insta/tests/main.rs
@@ -290,21 +290,20 @@ fn test_yaml_snapshot() {
 
     assert_success(&output);
 
-    assert_snapshot!(test_project.diff("src/main.rs"), @r##"
+    assert_snapshot!(test_project.diff("src/main.rs"), @r###"
     --- Original: src/main.rs
     +++ Updated: src/main.rs
-    @@ -15,5 +15,9 @@
+    @@ -15,5 +15,8 @@
          };
          insta::assert_yaml_snapshot!(&user, {
              ".id" => "[user_id]",
     -    }, @"");
     +    }, @r#"
-    +    ---
     +    id: "[user_id]"
     +    email: john.doe@example.com
     +    "#);
      }
-    "##);
+    "###);
 }
 
 #[test]

diff --git a/insta/src/macros.rs b/insta/src/macros.rs
@@ -223,57 +223,39 @@ macro_rules! assert_compact_json_snapshot {
 #[doc(hidden)]
 #[macro_export]
 macro_rules! _assert_serialized_snapshot {
-    // If there are redaction expressions and an inline snapshot, capture
-    // the redactions expressions and pass to `_assert_snapshot_base`
-    //
-    // Note that if we could unify the Inline & File representations of snapshots
-    // redactions we could unify some of these branches.
-    (format=$format:ident, $value:expr, $(match ..)? {$($k:expr => $v:expr),* $(,)?}, @$snapshot:literal $(,)?) => {{
+    // If there are redaction expressions, capture the redactions expressions
+    // and pass to `_assert_snapshot_base`
+    (format=$format:ident, $value:expr, $(match ..)? {$($k:expr => $v:expr),* $(,)?} $($arg:tt)*) => {{
         let transform = |value| {
-            let (_, value) = $crate::_prepare_snapshot_for_redaction!(value, {$($k => $v),*}, $format, Inline);
+            let (_, value) = $crate::_prepare_snapshot_for_redaction!(value, {$($k => $v),*}, $format);
             value
         };
-        $crate::_assert_snapshot_base!(transform=transform, $value, @$snapshot);
-    }};
-    // If there are redaction expressions and no name, add a auto-generated name, call self
-    (format=$format:ident, $value:expr, $(match ..)? {$($k:expr => $v:expr),* $(,)?} $(,)?) => {{
-        $crate::_assert_serialized_snapshot!(format=$format, $crate::_macro_support::AutoName, $value, {$($k => $v),*});
+        $crate::_assert_snapshot_base!(transform=transform, $value $($arg)*);
     }};
-    // If there are redaction expressions, capture and pass to `_assert_snapshot_base`
+    // If there's a name and redaction expressions, capture and pass to `_assert_snapshot_base`
     (format=$format:ident, $name:expr, $value:expr, $(match ..)? {$($k:expr => $v:expr),* $(,)?} $(,)?) => {{
         let transform = |value| {
-            let (_, value) = $crate::_prepare_snapshot_for_redaction!(value, {$($k => $v),*}, $format, File);
+            let (_, value) = $crate::_prepare_snapshot_for_redaction!(value, {$($k => $v),*}, $format);
             value
         };
         $crate::_assert_snapshot_base!(transform=transform, $name, $value);
     }};
-    // If there's an inline snapshot, capture serialization function and pass to
-    // `_assert_snapshot_base`, specifying `Inline`
-    (format=$format:ident, $($arg:expr),*, @$snapshot:literal $(,)?) => {{
-        let transform = |value| {$crate::_macro_support::serialize_value(
-            &value,
-            $crate::_macro_support::SerializationFormat::$format,
-            $crate::_macro_support::SnapshotLocation::Inline
-        )};
-        $crate::_assert_snapshot_base!(transform = transform, $($arg),*, @$snapshot);
-    }};
-    // Capture serialization function and pass to `_assert_snapshot_base`,
-    // specifying `File`
-    (format=$format:ident, $($arg:expr),* $(,)?) => {{
+    // Capture serialization function and pass to `_assert_snapshot_base`
+    //
+    (format=$format:ident, $($arg:tt)*) => {{
         let transform = |value| {$crate::_macro_support::serialize_value(
             &value,
             $crate::_macro_support::SerializationFormat::$format,
-            $crate::_macro_support::SnapshotLocation::File
         )};
-        $crate::_assert_snapshot_base!(transform = transform, $($arg),*);
+        $crate::_assert_snapshot_base!(transform = transform, $($arg)*);
     }};
 }
 
 #[cfg(feature = "redactions")]
 #[doc(hidden)]
 #[macro_export]
 macro_rules! _prepare_snapshot_for_redaction {
-    ($value:expr, {$($k:expr => $v:expr),*}, $format:ident, $location:ident) => {
+    ($value:expr, {$($k:expr => $v:expr),*}, $format:ident) => {
         {
             let vec = std::vec![
                 $((
@@ -285,7 +267,6 @@ macro_rules! _prepare_snapshot_for_redaction {
                 &$value,
                 &vec,
                 $crate::_macro_support::SerializationFormat::$format,
-                $crate::_macro_support::SnapshotLocation::$location
             );
             (vec, value)
         }
@@ -296,10 +277,10 @@ macro_rules! _prepare_snapshot_for_redaction {
 #[doc(hidden)]
 #[macro_export]
 macro_rules! _prepare_snapshot_for_redaction {
-    ($value:expr, {$($k:expr => $v:expr),*}, $format:ident, $location:ident) => {
+    ($value:expr, {$($k:expr => $v:expr),*}, $format:ident) => {
         compile_error!(
             "insta was compiled without redactions support. Enable the `redactions` feature."
-        );
+        )
     };
 }
 

diff --git a/insta/src/serialization.rs b/insta/src/serialization.rs
@@ -22,11 +22,7 @@ pub enum SnapshotLocation {
     File,
 }
 
-pub fn serialize_content(
-    mut content: Content,
-    format: SerializationFormat,
-    location: SnapshotLocation,
-) -> String {
+pub fn serialize_content(mut content: Content, format: SerializationFormat) -> String {
     content = Settings::with(|settings| {
         if settings.sort_maps() {
             content.sort_maps();
@@ -41,13 +37,7 @@ pub fn serialize_content(
     });
 
     match format {
-        SerializationFormat::Yaml => {
-            let serialized = yaml::to_string(&content);
-            match location {
-                SnapshotLocation::Inline => serialized,
-                SnapshotLocation::File => serialized[4..].to_string(),
-            }
-        }
+        SerializationFormat::Yaml => yaml::to_string(&content)[4..].to_string(),
         SerializationFormat::Json => json::to_string_pretty(&content),
         SerializationFormat::JsonCompact => json::to_string_compact(&content),
         #[cfg(feature = "csv")]
@@ -98,29 +88,24 @@ pub fn serialize_content(
     }
 }
 
-pub fn serialize_value<S: Serialize>(
-    s: &S,
-    format: SerializationFormat,
-    location: SnapshotLocation,
-) -> String {
+pub fn serialize_value<S: Serialize>(s: &S, format: SerializationFormat) -> String {
     let serializer = ContentSerializer::<ValueError>::new();
     let content = Serialize::serialize(s, serializer).unwrap();
-    serialize_content(content, format, location)
+    serialize_content(content, format)
 }
 
 #[cfg(feature = "redactions")]
 pub fn serialize_value_redacted<S: Serialize>(
     s: &S,
     redactions: &[(crate::redaction::Selector, crate::redaction::Redaction)],
     format: SerializationFormat,
-    location: SnapshotLocation,
 ) -> String {
     let serializer = ContentSerializer::<ValueError>::new();
     let mut content = Serialize::serialize(s, serializer).unwrap();
     for (selector, redaction) in redactions {
         content = selector.redact(content, redaction);
     }
-    serialize_content(content, format, location)
+    serialize_content(content, format)
 }
 
 #[test]
@@ -140,7 +125,6 @@ fn test_yaml_serialization() {
             ),
         ]),
         SerializationFormat::Yaml,
-        SnapshotLocation::File,
     );
     crate::assert_snapshot!(&yaml, @r###"
     env:
@@ -166,9 +150,18 @@ fn test_yaml_serialization() {
             ),
         ]),
         SerializationFormat::Yaml,
-        SnapshotLocation::Inline,
     );
     crate::assert_snapshot!(&inline_yaml, @r###"
+    env:
+      - ENVIRONMENT
+      - production
+    cmdline:
+      - my-tool
+      - run
+    "###);
+
+    // Old approach with leading `---`:
+    crate::assert_snapshot!(&inline_yaml, @r###"
     ---
     env:
       - ENVIRONMENT

diff --git a/insta/src/snapshot.rs b/insta/src/snapshot.rs
@@ -431,7 +431,7 @@ impl Snapshot {
 
     /// Snapshot contents _and_ metadata match another snapshot's.
     pub fn matches_fully(&self, other: &Snapshot) -> bool {
-        self.matches(other)
+        self.snapshot.matches_fully(&other.snapshot)
             && self.metadata.trim_for_persistence() == other.metadata.trim_for_persistence()
     }
 
@@ -520,7 +520,23 @@ impl SnapshotContents {
 
     /// Returns the snapshot contents as string with surrounding whitespace removed.
     pub fn as_str(&self) -> &str {
-        self.0.trim_start_matches(['\r', '\n']).trim_end()
+        let out = self.0.trim_start_matches(['\r', '\n']).trim_end();
+        // Old inline snapshots have `---` at the start, so this strips that if
+        // it exists. Soon we can start printing a warning and then eventually
+        // remove it in the next version.
+        match out.strip_prefix("---\n") {
+            Some(s) => s,
+            None => out,
+        }
+    }
+
+    /// Returns the snapshot contents as string without any trimming.
+    pub fn as_str_exact(&self) -> &str {
+        self.0.as_str()
+    }
+
+    pub fn matches_fully(&self, other: &SnapshotContents) -> bool {
+        self.as_str_exact() == other.as_str_exact()
     }
 
     pub fn to_inline(&self, indentation: usize) -> String {

diff --git a/insta/tests/test_inline.rs b/insta/tests/test_inline.rs
@@ -191,7 +191,6 @@ fn test_yaml_inline() {
         username: "peter-pan".into(),
         email: "peterpan@wonderland.invalid".into()
     }, @r###"
-    ---
     id: 42
     username: peter-pan
     email: peterpan@wonderland.invalid
@@ -215,7 +214,6 @@ fn test_yaml_inline_redacted() {
     }, {
         ".id" => "[user-id]"
     }, @r###"
-    ---
     id: "[user-id]"
     username: peter-pan
     email: peterpan@wonderland.invalid

diff --git a/insta/tests/test_redaction.rs b/insta/tests/test_redaction.rs
@@ -122,12 +122,11 @@ fn test_with_random_value_and_match_comma() {
             ".id" => "[id]",
         },
         @r###"
-        ---
-        id: "[id]"
-        username: john_doe
-        email: john@example.com
-        extra: ""
-        "###, // comma here
+    id: "[id]"
+    username: john_doe
+    email: john@example.com
+    extra: ""
+    "###, // comma here
     );
 }
 
@@ -349,7 +348,6 @@ fn test_redact_newtype_enum() {
     assert_yaml_snapshot!(visitor, {
         r#".id"# => "[id]",
     }, @r###"
-    ---
     Visitor:
       id: "[id]"
       name: my-name
@@ -364,7 +362,6 @@ fn test_redact_newtype_enum() {
     assert_yaml_snapshot!(admin, {
         r#".id"# => "[id]",
     }, @r###"
-    ---
     Admin:
       id: "[id]"
       username: john_doe

diff --git a/insta/tests/test_settings.rs b/insta/tests/test_settings.rs
@@ -17,7 +17,6 @@ fn test_simple() {
     settings.set_sort_maps(true);
     settings.bind(|| {
         assert_yaml_snapshot!(&map, @r###"
-        ---
         a: first value
         b: second value
         c: third value
@@ -40,7 +39,6 @@ fn test_bound_to_scope() {
         settings.set_sort_maps(true);
         let _guard = settings.bind_to_scope();
         assert_yaml_snapshot!(&map, @r###"
-        ---
         a: first value
         b: second value
         c: third value
@@ -62,7 +60,6 @@ fn test_settings_macro() {
 
     with_settings!({sort_maps => true}, {
         insta::assert_yaml_snapshot!(&map, @r###"
-        ---
         a: first value
         b: second value
         c: third value