--force-update-snapshots always writes, and implies --accept (#644)

As discussed in #630

Will wait for a few days before merging.

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
 

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<dyn Error>> {
     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<dyn Error>
             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!(

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]

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 {