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

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)]
@@ -51,7 +46,7 @@ impl PendingSnapshot {
 pub(crate) struct SnapshotContainer {
     snapshot_path: PathBuf,
     target_path: PathBuf,
-    kind: SnapshotContainerKind,
+    kind: SnapshotKind,
     snapshots: Vec<PendingSnapshot>,
     patcher: Option<FilePatcher>,
 }
@@ -60,11 +55,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 {
@@ -80,7 +75,7 @@ impl SnapshotContainer {
                 });
                 None
             }
-            SnapshotContainerKind::Inline => {
+            SnapshotKind::Inline => {
                 let mut pending_vec = PendingInlineSnapshot::load_batch(&snapshot_path)?;
                 let mut have_new = false;
 
@@ -136,8 +131,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_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

@@ -837,13 +837,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-linkbreaks"
 version = "0.1.0"
 edition = "2021"
 
@@ -856,8 +856,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(),
@@ -882,16 +885,68 @@ 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"
+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_hashtag_escape_in_inline_snapshot() {
     let test_project = TestFiles::new()

insta/src/lib.rs

@@ -288,7 +288,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/runtime.rs

@@ -8,14 +8,17 @@ use std::str;
 use std::sync::{Arc, Mutex};
 use std::{borrow::Cow, env};
 
-use crate::env::{
-    get_cargo_workspace, get_tool_config, memoize_snapshot_file, snapshot_update_behavior,
-    OutputBehavior, SnapshotUpdateBehavior, ToolConfig,
-};
 use crate::output::SnapshotPrinter;
 use crate::settings::Settings;
 use crate::snapshot::{MetaData, PendingInlineSnapshot, Snapshot, SnapshotContents};
 use crate::utils::{path_to_storage, style};
+use crate::{
+    env::{
+        get_cargo_workspace, get_tool_config, memoize_snapshot_file, snapshot_update_behavior,
+        OutputBehavior, SnapshotUpdateBehavior, ToolConfig,
+    },
+    snapshot::SnapshotKind,
+};
 
 lazy_static::lazy_static! {
     static ref TEST_NAME_COUNTERS: Mutex<BTreeMap<String, usize>> =
@@ -77,6 +80,7 @@ impl<'a> From<&'a str> for ReferenceValue<'a> {
     }
 }
 
+#[derive(Debug)]
 /// A reference to a snapshot
 pub enum ReferenceValue<'a> {
     /// A named snapshot, where the inner value is the snapshot name.
@@ -286,6 +290,7 @@ impl<'a> SnapshotAssertionContext<'a> {
                     module_path.replace("::", "__"),
                     None,
                     MetaData::default(),
+                    SnapshotKind::Inline,
                     SnapshotContents::from_inline(contents),
                 ));
             }
@@ -314,7 +319,12 @@ impl<'a> SnapshotAssertionContext<'a> {
     }
 
     /// Creates the new snapshot from input values.
-    pub fn new_snapshot(&self, contents: SnapshotContents, expr: &str) -> Snapshot {
+    pub fn new_snapshot(
+        &self,
+        contents: SnapshotContents,
+        expr: &str,
+        kind: SnapshotKind,
+    ) -> Snapshot {
         Snapshot::from_components(
             self.module_path.replace("::", "__"),
             self.snapshot_name.as_ref().map(|x| x.to_string()),
@@ -333,6 +343,7 @@ impl<'a> SnapshotAssertionContext<'a> {
                     .and_then(|x| self.localize_path(x))
                     .map(|x| path_to_storage(&x)),
             }),
+            kind,
             contents,
         )
     }
@@ -636,7 +647,11 @@ pub fn assert_snapshot(
     let new_snapshot_value =
         Settings::with(|settings| settings.filters().apply_to(new_snapshot_value));
 
-    let new_snapshot = ctx.new_snapshot(new_snapshot_value.into(), expr);
+    let kind = match ctx.snapshot_file {
+        Some(_) => SnapshotKind::File,
+        None => SnapshotKind::Inline,
+    };
+    let new_snapshot = ctx.new_snapshot(new_snapshot_value.into(), expr, kind);
 
     // memoize the snapshot file if requested.
     if let Some(ref snapshot_file) = ctx.snapshot_file {
@@ -668,7 +683,22 @@ pub fn assert_snapshot(
         ctx.cleanup_passing()?;
 
         if tool_config.force_update_snapshots() {
-            ctx.update_snapshot(new_snapshot)?;
+            // Avoid creating new files if contents match exactly. In
+            // particular, this would otherwise create lots of unneeded files
+            // for inline snapshots
+            //
+            // Note that there's a check down the stack on whether the file
+            // contents match exactly for file snapshots; probably we should
+            // combine that check with `matches_fully` and then use a single
+            // check for whether we force update 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)?;
+            }
         }
     // otherwise print information and update snapshots.
     } else {