Repository: mitsuhiko/insta
Pull request #690 (2024-12-10 08:43:23)

Don't add `snapshot_kind: text` to snapshots
This was extremely simple, sorry to leave it hanging.

Will do a self-review tomorrow to confirm I haven't missed anything

nyurik: horray, thanks!!
max-sixty: One thing I did miss — this will remove existing `snapshot_kind: text`. Ideally we'd keep those, or it's going to be a lot more churn.

(shows the cost of leaving it for a while — I remember thinking this at the time)
max-sixty: OK, I realize it's not tractable to have it retain `snapshot_kind: text`. So now making changes to any snapshots (or running `--force-update-snapshots`)  that have `snapshot_kind: text` will remove that metadata item.

I personally disagree with merging this. From the attached issue explaining the original change:

> I appreciate it's some aesthetic churn. But OTOH it only appears when updating snapshots. And for those who care about the cleanliness of diffs, doing a single run of `--force-update-snapshots` will update everything in a single commit. And in general it's important for insta to be able to evolve the snapshot format, albeit in the specific case we could have traded away consistency in metadata to reduce changes.

Merging this adds some additional churn (though admittedly more so since I took a couple weeks to do this).

But I weigh @mitsuhiko 's opinion highly, and appreciate some of the trade for being able to move quickly is occasionally reverting things. So I'll merge now, and release in a few days unless there are any second thoughts.

Patch:
diff --git a/insta/src/snapshot.rs b/insta/src/snapshot.rs
index ec80602..9275f45 100644
--- a/insta/src/snapshot.rs
+++ b/insta/src/snapshot.rs
@@ -291,15 +291,13 @@ impl MetaData {
             fields.push(("input_file", Content::from(input_file)));
         }
 
-        let snapshot_type = Content::from(match self.snapshot_kind {
-            SnapshotKind::Text => "text",
+        match self.snapshot_kind {
+            SnapshotKind::Text => {}
             SnapshotKind::Binary { ref extension } => {
                 fields.push(("extension", Content::from(extension.clone())));
-                "binary"
+                fields.push(("snapshot_kind", Content::from("binary")));
             }
-        });
-
-        fields.push(("snapshot_kind", snapshot_type));
+        }
 
         Content::Struct("MetaData", fields)
     }


Test patch:
diff --git a/cargo-insta/tests/functional/main.rs b/cargo-insta/tests/functional/main.rs
index 67438f9..59856fc 100644
--- a/cargo-insta/tests/functional/main.rs
+++ b/cargo-insta/tests/functional/main.rs
@@ -359,13 +359,12 @@ Hello, world!
     assert_snapshot!(test_current_insta.diff("src/snapshots/test_force_update_current__force_update.snap"), @r#"
     --- Original: src/snapshots/test_force_update_current__force_update.snap
     +++ Updated: src/snapshots/test_force_update_current__force_update.snap
-    @@ -1,8 +1,6 @@
+    @@ -1,8 +1,5 @@
     -
      ---
      source: src/lib.rs
     -expression: 
     +expression: "\"Hello, world!\""
-    +snapshot_kind: text
      ---
      Hello, world!
     -
@@ -763,6 +762,63 @@ Hidden snapshot
     );
 }
 
+#[test]
+fn test_snapshot_kind_behavior() {
+    let test_project = TestFiles::new()
+        .add_cargo_toml("test_snapshot_kind")
+        .add_file(
+            "src/lib.rs",
+            r#"
+#[test]
+fn test_snapshots() {
+    insta::assert_snapshot!("new snapshot");
+    insta::assert_snapshot!("existing snapshot");
+}
+"#
+            .to_string(),
+        )
+        .add_file(
+            "src/snapshots/test_snapshot_kind__existing.snap",
+            r#"---
+source: src/lib.rs
+expression: "\"existing snapshot\""
+snapshot_kind: text
+---
+existing snapshot
+"#
+            .to_string(),
+        )
+        .create_project();
+
+    // Run the test with --accept to create the new snapshot
+    let output = test_project
+        .insta_cmd()
+        .args(["test", "--accept"])
+        .output()
+        .unwrap();
+
+    assert!(output.status.success());
+
+    // Verify the new snapshot was created without snapshot_kind
+    let new_snapshot = std::fs::read_to_string(
+        test_project
+            .workspace_dir
+            .join("src/snapshots/test_snapshot_kind__snapshots.snap"),
+    )
+    .unwrap();
+
+    assert!(!new_snapshot.contains("snapshot_kind:"));
+
+    // Verify both snapshots work with --require-full-match
+    let output = test_project
+        .insta_cmd()
+        .args(["test", "--require-full-match"])
+        .output()
+        .unwrap();
+
+    assert!(output.status.success());
+}
+
 #[test]
 fn test_ignored_snapshots() {
     let test_project = TestFiles::new()
diff --git a/cargo-insta/tests/functional/workspace.rs b/cargo-insta/tests/functional/workspace.rs
index 0b4841e..b3976fe 100644
--- a/cargo-insta/tests/functional/workspace.rs
+++ b/cargo-insta/tests/functional/workspace.rs
@@ -576,7 +576,6 @@ fn test_hello() {
     ---
     source: "../tests/lib.rs"
     expression: hello()
-    snapshot_kind: text
     ---
     Hello, world!
     "#);