Repository: mitsuhiko/insta (language: rust)

Issue:
Multiple `assert_snapshot` within a `allow_duplicates` doesn't work well
### What happened?

I think there might be several problems here

### Reproduction steps

1. Clone https://github.com/jj-vcs/jj/commit/35440ce1bd62c1752a13f1e4e6353eae783ad666
2. Observe that `cargo nextest run` passes
3. Run `cargo insta test  --workspace --test-runner nextest --force-update-snapshots`
4. Try `cargo nextest run` again

Expected results:

tests pass

Actual result:

Compilation fails

```
error: bare CR not allowed in string, use `\r` instead
   --> cli/src/git_util.rs:820:74
    |
820 |         assert_snapshot!(update(Duration::from_millis(1), 0.10), @"␛[?25l␍ 10% [█▊                ]␛[K...
    |                                                                          ^
    |
help: escape the character
    |
820 |         assert_snapshot!(update(Duration::from_millis(1), 0.10), @"␛[?25l\r 10% [█▊                ]␛[K");
    |                                                                          ++

error: could not compile `jj-cli` (lib) due to 1 previous error
```

Furthermore, these `\r`-s are not the only problem. If that one diff is reverted (`\r` replaced with `␍`again) in https://github.com/jj-vcs/jj/pull/5441, there are more compilation errors, see https://github.com/jj-vcs/jj/actions/runs/12934546007/job/36075816189?pr=5441 . For example,

```
error: expected one of `.`, `;`, `?`, `}`, or an operator, found `""`
   --> cli/tests/test_git_fetch.rs:334:10
    |
334 |     ");""""
    |          ^^ expected one of `.`, `;`, `?`, `}`, or an operator
```


### Insta Version

1.42.0, same behavior with 1.41.1

### rustc Version

rustc 1.86.0-nightly (48a426eca 2025-01-12)

### What did you expect?

Tests to pass.


Discussion / hints:
Thanks, this seems important.

~Do you know whether this replicates without `nextest`? Has it been an issue for a while or are there some new test outputs in `jj`?~

OK, I looked at the PR — I think the problem i that we're deciding how many hashes to use depending on whether there are linebreaks, but failing to consider a bare `\r`, which is possibly required. Does that sound correct?
I just started investigating, I think there are at least two problems (as seen by the two error messages).

Running `insta` without `nextest` gives the same result.

I randomly tried running `insta` on jj's commit dddba46 from 6 months ago, and there were no problems.

I tried a commit from earlier today, and got the same error. I might try bisecting to find when the commit that `insta` can't handle happened, it's probably when the relevant tests were last updated.
(I edited my message above a minute after sending)

No need to bisect — I know why it's doing this. We previously always used `###`; now we decide how many to add (which can be "none".) The new approach is better since having `###` in the snapshot used to cause errors, but the implementation likely misses a couple of characters.
#713 goes some way to fixing it; possibly not all of it. If you have an idle moment, give it a try; otherwise I can look later
I don't think #713 is complete fyi, will come back to this later (or welcome any contribution if you have an idea, but no obligation)
No, #713 does not resolve either problem. I didn't look into why yet.
I did narrow down the error to the following files in the `jj` repo.

```
M cli/src/git_util.rs
M cli/tests/test_git_fetch.rs
M cli/tests/test_git_init.rs
M cli/tests/test_git_push.rs
```

So, you can run the following, slightly faster command, to observe a few different errors.

```
cargo insta test  --workspace --test-runner nextest \
  --force-update-snapshots -- git_util test_git_fetch test_git_init test_git_push
```
I think there are at least two problems:

- Something about quoting `\r`.
- Some bad interaction between `allow_duplicates!` and `--force-update-snapshots`. When both are used, insta seems to make up snapshot output sometimes, seemingly copying it from the previous snapshot.
OK, I think the new version of #713 fixes the first; will be offline for a bit but can look more later
Yes, I think that new version of #713 helped. 🎉 

For the second problem, @martinvonz pointed out that it's probably related to https://github.com/jj-vcs/jj/pull/5314 and having more than one `insta_snapshot!` inside `allow_duplicates!`. So, there's probably a workaround.
OK great! I'll change the title if that's OK!

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 57f1e9a7..247509c7 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,7 @@ All notable changes to insta and cargo-insta are documented here.
 ## 1.42.2
 
 - Stop `\t` and `\x1b` (ANSI color escape) from causing snapshots to be escaped.  #715
+- Improved handling of inline snapshots within `allow_duplicates! { .. }`.  #712
 
 ## 1.42.1
 
diff --git a/cargo-insta/src/inline.rs b/cargo-insta/src/inline.rs
index 006f552c..a44ba9db 100644
--- a/cargo-insta/src/inline.rs
+++ b/cargo-insta/src/inline.rs
@@ -76,7 +76,8 @@ impl FilePatcher {
                 if self
                     .inline_snapshots
                     .last()
-                    .map_or(false, |x| x.end.0 > line)
+                    // x.end.0 is 0-origin whereas line is 1-origin
+                    .map_or(false, |x| x.end.0 >= line - 1)
                 {
                     return false;
                 }
@@ -253,6 +254,18 @@ impl FilePatcher {
                     return;
                 }
 
+                // recurse into block-like macro such as allow_duplicates! { .. }
+                if matches!(i.delimiter, syn::MacroDelimiter::Brace(_)) {
+                    if let Ok(stmts) = i.parse_body_with(syn::Block::parse_within) {
+                        for stmt in &stmts {
+                            self.visit_stmt(stmt);
+                        }
+                        return;
+                    }
+                    // TODO: perhaps, we can return here and remove fallback to
+                    // self.scan_nested_macros(&tokens)
+                }
+
                 let indentation = indentation(span_start, self.2);
                 if !self.try_extract_snapshot(&tokens, indentation) {
                     // if we can't extract a snapshot here we want to scan for nested
@@ -357,4 +370,47 @@ fn test_function() {
         // Assert the indentation
         assert_debug_snapshot!(snapshot.indentation, @r#""\t""#);
     }
+
+    #[test]
+    fn test_find_snapshot_macro_within_allow_duplicates() {
+        let content = r######"
+fn test_function() {
+    insta::allow_duplicates! {
+        for x in 0..10 {
+            insta::assert_snapshot!("foo", @"foo"); // 5
+            insta::assert_snapshot!("bar", @"bar"); // 6
+        }
+    }
+}
+"######;
+
+        let file_patcher = FilePatcher {
+            filename: PathBuf::new(),
+            lines: content.lines().map(String::from).collect(),
+            source: syn::parse_file(content).unwrap(),
+            inline_snapshots: vec![],
+        };
+
+        let snapshot5 = file_patcher.find_snapshot_macro(5).unwrap();
+        let snapshot6 = file_patcher.find_snapshot_macro(6).unwrap();
+
+        // Extract the snapshot contents
+        let snapshot_content5 = file_patcher.lines[snapshot5.start.0..=snapshot5.end.0].to_vec();
+        let snapshot_content6 = file_patcher.lines[snapshot6.start.0..=snapshot6.end.0].to_vec();
+
+        assert_debug_snapshot!(snapshot_content5, @r#"
+        [
+            "            insta::assert_snapshot!(\"foo\", @\"foo\"); // 5",
+        ]
+        "#);
+        assert_debug_snapshot!(snapshot_content6, @r#"
+        [
+            "            insta::assert_snapshot!(\"bar\", @\"bar\"); // 6",
+        ]
+        "#);
+
+        // Assert the indentation
+        assert_debug_snapshot!(snapshot5.indentation, @r#""            ""#);
+        assert_debug_snapshot!(snapshot6.indentation, @r#""            ""#);
+    }
 }


Test patch:
diff --git a/cargo-insta/tests/functional/inline.rs b/cargo-insta/tests/functional/inline.rs
index 8a58679a..edb9153e 100644
--- a/cargo-insta/tests/functional/inline.rs
+++ b/cargo-insta/tests/functional/inline.rs
@@ -337,3 +337,146 @@ fn test_hashtag_escape() {
      }
     "####);
 }
+
+#[test]
+fn test_single_line_duplicates() {
+    let test_project = TestFiles::new()
+        .add_cargo_toml("test_single_line_duplicates")
+        .add_file(
+            "src/lib.rs",
+            r#####"
+#[test]
+fn test_single_line_duplicates() {
+    for _ in 0..2 {
+        insta::allow_duplicates! {
+            insta::assert_snapshot!("foo", @"");
+        }
+    }
+}
+"#####
+                .to_string(),
+        )
+        .create_project();
+
+    let output = test_project
+        .insta_cmd()
+        .args(["test", "--accept"])
+        .output()
+        .unwrap();
+
+    assert!(&output.status.success());
+
+    // diff shouldn't be applied twice
+    assert_snapshot!(test_project.diff("src/lib.rs"), @r#"
+    --- Original: src/lib.rs
+    +++ Updated: src/lib.rs
+    @@ -3,7 +3,7 @@
+     fn test_single_line_duplicates() {
+         for _ in 0..2 {
+             insta::allow_duplicates! {
+    -            insta::assert_snapshot!("foo", @"");
+    +            insta::assert_snapshot!("foo", @"foo");
+             }
+         }
+     }
+    "#);
+}
+
+#[test]
+fn test_single_line_assertions() {
+    let test_project = TestFiles::new()
+        .add_cargo_toml("test_single_line_assertions")
+        .add_file(
+            "src/lib.rs",
+            r#####"
+#[test]
+fn test_single_line_assertions() {
+    insta::assert_snapshot!("foo", @"");
+    insta::assert_snapshot!("bar", @"");
+}
+"#####
+                .to_string(),
+        )
+        .create_project();
+
+    let output = test_project
+        .insta_cmd()
+        .args(["test", "--accept"])
+        .output()
+        .unwrap();
+
+    assert!(&output.status.success());
+
+    // all adjacent lines should be updated
+    assert_snapshot!(test_project.diff("src/lib.rs"), @r#"
+    --- Original: src/lib.rs
+    +++ Updated: src/lib.rs
+    @@ -1,6 +1,6 @@
+     
+     #[test]
+     fn test_single_line_assertions() {
+    -    insta::assert_snapshot!("foo", @"");
+    -    insta::assert_snapshot!("bar", @"");
+    +    insta::assert_snapshot!("foo", @"foo");
+    +    insta::assert_snapshot!("bar", @"bar");
+     }
+    "#);
+}
+
+#[test]
+fn test_multiple_assertions_within_allow_duplicates() {
+    let test_project = TestFiles::new()
+        .add_cargo_toml("test_multiple_assertions_within_allow_duplicates")
+        .add_file(
+            "src/lib.rs",
+            r#####"
+#[test]
+fn test_multiple_assertions_within_allow_duplicates() {
+    for _ in 0..2 {
+        insta::allow_duplicates! {
+            insta::assert_snapshot!("1", @"
+            1a
+            1b
+            ");
+            insta::assert_snapshot!("2", @"
+            2a
+            2b
+            ");
+        }
+    }
+}
+"#####
+                .to_string(),
+        )
+        .create_project();
+
+    let output = test_project
+        .insta_cmd()
+        .args(["test", "--accept"])
+        .output()
+        .unwrap();
+
+    assert!(&output.status.success());
+
+    assert_snapshot!(test_project.diff("src/lib.rs"), @r#"
+    --- Original: src/lib.rs
+    +++ Updated: src/lib.rs
+    @@ -3,14 +3,8 @@
+     fn test_multiple_assertions_within_allow_duplicates() {
+         for _ in 0..2 {
+             insta::allow_duplicates! {
+    -            insta::assert_snapshot!("1", @"
+    -            1a
+    -            1b
+    -            ");
+    -            insta::assert_snapshot!("2", @"
+    -            2a
+    -            2b
+    -            ");
+    +            insta::assert_snapshot!("1", @"1");
+    +            insta::assert_snapshot!("2", @"2");
+             }
+         }
+     }
+    "#);
+}