tests: run `insta --force-update-snapshots` #5558

jakobhellermann · 2025-02-02T13:13:18Z

Run cargo insta test --test-runner nextest --force-update-snapshots.

There are a few additional changes that insta makes that break the tests, but only inside allow_duplicates, so that's related to mitsuhiko/insta#712

Commit ID: 489c76379e4da3ed28bf69a4a0bdcebe7d99f2f5
Change ID: lmqxpvzykynsnmxwtkpuxvotrrsptomp
Author   : Jakob Hellermann <[email protected]> (26m ago)
Committer: Jakob Hellermann <[email protected]> (49s ago)

    insta failure: allow_multiple

diff --git a/cli/tests/test_git_fetch.rs b/cli/tests/test_git_fetch.rs
index 6951ada78d..5dbe606a5c 100644
--- a/cli/tests/test_git_fetch.rs
+++ b/cli/tests/test_git_fetch.rs
@@ -406,15 +406,11 @@
     insta::allow_duplicates! {
     insta::assert_snapshot!(stderr, @"");
 
     // Explicit import is an error.
     // (This could be warning if we add mechanism to report ignored refs.)
-    insta::assert_snapshot!(test_env.jj_cmd_failure(&repo_path, &["git", "import"]), @r###"
-    Error: Failed to import refs from underlying Git repo
-    Caused by: Git remote named 'git' is reserved for local Git repository
-    Hint: Run `jj git remote rename` to give different name.
-    "###);
+    insta::assert_snapshot!(test_env.jj_cmd_failure(&repo_path, &["git", "import"]), @"");
     }
 
     // The remote can be renamed, and the ref can be imported.
     test_env.jj_cmd_ok(&repo_path, &["git", "remote", "rename", "git", "bar"]);
     let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["bookmark", "list", "--all-remotes"]);
diff --git a/cli/tests/test_git_init.rs b/cli/tests/test_git_init.rs
index 501aefdd7c..8038d46821 100644
--- a/cli/tests/test_git_init.rs
+++ b/cli/tests/test_git_init.rs
@@ -143,17 +143,11 @@
             git_repo_path.to_str().unwrap(),
         ],
     );
     insta::allow_duplicates! {
         insta::assert_snapshot!(stdout, @"");
-        insta::assert_snapshot!(stderr, @r###"
-        Done importing changes from the underlying Git repo.
-        Working copy now at: sqpuoqvx f6950fc1 (empty) (no description set)
-        Parent commit      : mwrttmos 8d698d4a my-bookmark | My commit message
-        Added 1 files, modified 0 files, removed 0 files
-        Initialized repo in "repo"
-        "###);
+        insta::assert_snapshot!(stderr, @"");
     }
 
     let workspace_root = test_env.env_root().join("repo");
     let jj_path = workspace_root.join(".jj");
     let repo_path = jj_path.join("repo");

and

Commit ID: 9ea82f1e65794c0853cd13123aa44f17f63d33df
Change ID: nyrlvnsxwzrqurpyvqmzoxzxlmtxzskl
Author   : Jakob Hellermann <[email protected]> (27m ago)
Committer: Jakob Hellermann <[email protected]> (1m ago)

    insta failure: idk

diff --git a/cli/tests/test_git_fetch.rs b/cli/tests/test_git_fetch.rs
index 5dbe606a5c..ad4b28ed88 100644
--- a/cli/tests/test_git_fetch.rs
+++ b/cli/tests/test_git_fetch.rs
@@ -875,25 +875,15 @@
     insta::allow_duplicates! {
     insta::assert_snapshot!(stdout, @"");
     }
     insta::allow_duplicates! {
     insta::assert_snapshot!(stderr, @r###"
     Nothing changed.
     "###);
-    insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @r#"
-    @  230dd059e1b0
-    │ ○  decaa3966c83 descr_for_a2 a2
-    │ │ ○  359a9a02457d descr_for_a1 a1
-    │ ├─╯
-    │ │ ○  c7d4bdcbc215 descr_for_b b
-    │ ├─╯
-    │ ○  ff36dc55760e descr_for_trunk1
-    ├─╯
-    ◆  000000000000
-    "#);
+    insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @"Nothing changed.");
     }
 
     // ==== Change both repos ====
     // First, change the target repo:
     let source_log = create_trunk2_and_rebase_bookmarks(&test_env, &source_git_repo_path);
     insta::allow_duplicates! {
     insta::assert_snapshot!(source_log, @r"
@@ -1184,55 +1174,41 @@
     insta::assert_snapshot!(stdout, @"");
     }
     insta::allow_duplicates! {
     insta::assert_snapshot!(stderr, @r###"
     bookmark: a1@origin [new] tracked
     bookmark: b@origin  [new] tracked
     "###);
-    insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @r#"
-    @  230dd059e1b0
-    │ ○  c7d4bdcbc215 descr_for_b b
-    │ │ ○  359a9a02457d descr_for_a1 a1
-    │ ├─╯
-    │ ○  ff36dc55760e descr_for_trunk1
-    ├─╯
-    ◆  000000000000
-    "#);
+    insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @r"
+    bookmark: a1@origin [new] tracked
+    bookmark: b@origin  [new] tracked
+    ");
     }
     let (stdout, stderr) = test_env.jj_cmd_ok(&target_jj_repo_path, &["undo"]);
     insta::allow_duplicates! {
     insta::assert_snapshot!(stdout, @"");
     }
     insta::allow_duplicates! {
     insta::assert_snapshot!(stderr, @r#"
     Undid operation: eb2029853b02 (2001-02-03 08:05:18) fetch from git remote(s) origin
     "#);
     // The undo works as expected
-    insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @r###"
-    @  230dd059e1b0
-    ◆  000000000000
-    "###);
+    insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @"Undid operation: eb2029853b02 (2001-02-03 08:05:18) fetch from git remote(s) origin");
     }
     // Now try to fetch just one bookmark
     let (stdout, stderr) =
         test_env.jj_cmd_ok(&target_jj_repo_path, &["git", "fetch", "--branch", "b"]);
     insta::allow_duplicates! {
     insta::assert_snapshot!(stdout, @"");
     }
     insta::allow_duplicates! {
     insta::assert_snapshot!(stderr, @r###"
     bookmark: b@origin [new] tracked
     "###);
-    insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @r#"
-    @  230dd059e1b0
-    │ ○  c7d4bdcbc215 descr_for_b b
-    │ ○  ff36dc55760e descr_for_trunk1
-    ├─╯
-    ◆  000000000000
-    "#);
+    insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @"bookmark: b@origin [new] tracked");
     }
 }
 
 // Compare to `test_git_import_undo` in test_git_import_export
 // TODO: Explain why these behaviors are useful
 #[test_case(false; "use git2 for remote calls")]
 #[test_case(true; "spawn a git subprocess for remote calls")]
@@ -1564,23 +1540,18 @@
     insta::assert_snapshot!(stdout, @"");
     }
     insta::allow_duplicates! {
     insta::assert_snapshot!(stderr, @r###"
     bookmark: a2@origin [deleted] untracked
     Abandoned 1 commits that are no longer reachable.
     "###);
-    insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @r#"
-    @  230dd059e1b0
-    │ ○  c7d4bdcbc215 descr_for_b b
-    │ │ ○  359a9a02457d descr_for_a1 a1
-    │ ├─╯
-    │ ○  ff36dc55760e descr_for_trunk1 trunk1
-    ├─╯
-    ◆  000000000000
-    "#);
+    insta::assert_snapshot!(get_log_output(&test_env, &target_jj_repo_path), @r"
+    bookmark: a2@origin [deleted] untracked
+    Abandoned 1 commits that are no longer reachable.
+    ");
     }
 }
 
 #[test_case(false; "use git2 for remote calls")]
 #[test_case(true; "spawn a git subprocess for remote calls")]
 fn test_git_fetch_removed_parent_bookmark(subprocess: bool) {
     let test_env = TestEnvironment::default();
diff --git a/cli/tests/test_git_push.rs b/cli/tests/test_git_push.rs
index cd2b3b2f96..dbc72ed2b7 100644
--- a/cli/tests/test_git_push.rs
+++ b/cli/tests/test_git_push.rs
@@ -125,22 +125,19 @@
     }
     insta::allow_duplicates! {
     insta::assert_snapshot!(stderr, @r#"
     Changes to push to origin:
       Move forward bookmark bookmark2 from 8476341eb395 to bc7610b65a91
       Add bookmark my-bookmark to bc7610b65a91
     "#);
-    insta::assert_snapshot!(get_bookmark_output(&test_env, &workspace_root), @r###"
-    bookmark1: xtvrqkyv 0f8dc656 (empty) modified bookmark1 commit
-      @origin (ahead by 1 commits, behind by 1 commits): xtvrqkyv hidden d13ecdbd (empty) description 1
-    bookmark2: yostqsxw bc7610b6 (empty) foo
-      @origin: yostqsxw bc7610b6 (empty) foo
-    my-bookmark: yostqsxw bc7610b6 (empty) foo
-      @origin: yostqsxw bc7610b6 (empty) foo
-    "###);
+    insta::assert_snapshot!(get_bookmark_output(&test_env, &workspace_root), @r"
+    Changes to push to origin:
+      Move forward bookmark bookmark2 from 8476341eb395 to bc7610b65a91
+      Add bookmark my-bookmark to bc7610b65a91
+    ");
     }
 
     // Try pushing backwards
     test_env.jj_cmd_ok(
         &workspace_root,
         &[
             "bookmark",
@@ -752,20 +749,20 @@
     insta::allow_duplicates! {
     insta::assert_snapshot!(stderr, @r#"
     Changes to push to origin:
       Delete bookmark bookmark1 from d13ecdbda2a2
       Move sideways bookmark bookmark2 from 8476341eb395 to c4a3c3105d92
       Add bookmark my-bookmark to c4a3c3105d92
     "#);
-    insta::assert_snapshot!(get_bookmark_output(&test_env, &workspace_root), @r###"
-    bookmark2: yqosqzyt c4a3c310 (empty) foo
-      @origin: yqosqzyt c4a3c310 (empty) foo
-    my-bookmark: yqosqzyt c4a3c310 (empty) foo
-      @origin: yqosqzyt c4a3c310 (empty) foo
-    "###);
+    insta::assert_snapshot!(get_bookmark_output(&test_env, &workspace_root), @r"
+    Changes to push to origin:
+      Delete bookmark bookmark1 from d13ecdbda2a2
+      Move sideways bookmark bookmark2 from 8476341eb395 to c4a3c3105d92
+      Add bookmark my-bookmark to c4a3c3105d92
+    ");
     }
     let stdout = test_env.jj_cmd_success(&workspace_root, &["log", "-rall()"]);
     insta::allow_duplicates! {
     insta::assert_snapshot!(stdout, @r"
     @  yqosqzyt [email protected] 2001-02-03 08:05:17 bookmark2 my-bookmark c4a3c310
     │  (empty) foo
     │ ○  rlzusymt [email protected] 2001-02-03 08:05:10 8476341e

If applicable:

I have updated CHANGELOG.md
I have updated the documentation (README.md, docs/, demos/)
I have updated the config schema (cli/src/config-schema.json)
I have added tests to cover my changes

yuja

For the allow_duplicates issue, we can add a patch that inserts allow_duplicates! block per snapshot.

yuja · 2025-02-02T14:15:33Z

cli/src/formatter.rs

@@ -1274,7 +1272,7 @@ mod tests {

        insta::assert_snapshot!(
            str::from_utf8(recorder.data()).unwrap(),
-            @" outer1  inner1  inner2  outer2 ");
+            @" outer1  inner1  inner2  outer2");


nit: It's not important that the trailing space is preserved here, but it looks a bit odd. We might have to add leading/trailing non-space character to the snapshot sample.

You could also file a bug to insta. (Update: Thought I'm not 100% sure they would consider it a bug. See below for another possibility)

Perhaps a better option is to change the test (in a separate commit, like with inserting allow_duplicates!), so that we are in fact testing everything:

insta::assert_snapshot!( format!("|{}|",str::from_utf8(recorder.data()).unwrap()), @"| outer1 inner1 inner2 outer2 |")

Yes. iirc, insta isn't strict about leading/trailing whitespaces, and I assume that's by design. I just pointed that out because this patch contained whitespace changes like this.

Yes. iirc, insta isn't strict about leading/trailing whitespaces

What's weird is that it strips the whitespace at the end, but not at the start. I would have expected either both or neither.

yuja · 2025-02-02T14:18:16Z

cli/src/git_util.rs

@@ -702,7 +702,7 @@ mod tests {
        };
        // First output is after the initial delay
        assert_snapshot!(update(crate::progress::INITIAL_DELAY - Duration::from_millis(1), 0.1), @"");
-        assert_snapshot!(update(Duration::from_millis(1), 0.10), @"[?25l\r 10% [█▊                ][K");
+        assert_snapshot!(update(Duration::from_millis(1), 0.10), @"\u{1b}[?25l\r 10% [█▊                ]\u{1b}[K");


Is this supposed to be changed by mitsuhiko/insta#715 ? If that's the case, it's probably better to merge this PR after new insta is released.

It's also not very difficult to compile cargo-insta from source. I do it for mitsuhiko/insta#438.

cargo build -p cargo-insta --release && cp target/release/cargo-insta ~/.cargo/bin

Is this supposed to be changed by mitsuhiko/insta#715?

Yes, this is with mitsuhiko/insta#716 merged.

The current logic is that if there are any bad control characters, then everything will be escaped.
So, because the string contains \r, the \x1b will be escaped as well.

One could imagine another solution where even if there are unrenderable control characters, only they are escaped, and \n, \t, \x1b are kept as is, but that is not implemented in insta right now.

yuja reviewed Feb 2, 2025

View reviewed changes

tests: run insta --force-update-snapshots

cec83a7

jakobhellermann force-pushed the insta-update-snapshots branch from 485fecf to cec83a7 Compare February 3, 2025 17:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tests: run `insta --force-update-snapshots` #5558

tests: run `insta --force-update-snapshots` #5558

jakobhellermann commented Feb 2, 2025

yuja left a comment

yuja Feb 2, 2025 •

edited

Loading

ilyagr Feb 2, 2025 •

edited

Loading

ilyagr Feb 2, 2025 •

edited

Loading

yuja Feb 3, 2025

jakobhellermann Feb 3, 2025

yuja Feb 2, 2025

ilyagr Feb 2, 2025

jakobhellermann Feb 3, 2025

tests: run insta --force-update-snapshots #5558

Are you sure you want to change the base?

tests: run insta --force-update-snapshots #5558

Conversation

jakobhellermann commented Feb 2, 2025

yuja left a comment

Choose a reason for hiding this comment

yuja Feb 2, 2025 • edited Loading

Choose a reason for hiding this comment

ilyagr Feb 2, 2025 • edited Loading

Choose a reason for hiding this comment

ilyagr Feb 2, 2025 • edited Loading

Choose a reason for hiding this comment

yuja Feb 3, 2025

Choose a reason for hiding this comment

jakobhellermann Feb 3, 2025

Choose a reason for hiding this comment

yuja Feb 2, 2025

Choose a reason for hiding this comment

ilyagr Feb 2, 2025

Choose a reason for hiding this comment

jakobhellermann Feb 3, 2025

Choose a reason for hiding this comment

tests: run `insta --force-update-snapshots` #5558

tests: run `insta --force-update-snapshots` #5558

yuja Feb 2, 2025 •

edited

Loading

ilyagr Feb 2, 2025 •

edited

Loading

ilyagr Feb 2, 2025 •

edited

Loading