-
Notifications
You must be signed in to change notification settings - Fork 779
tests: run insta --force-update-snapshots
#5558
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
tests: run insta --force-update-snapshots
#5558
Conversation
yuja
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the allow_duplicates issue, we can add a patch that inserts allow_duplicates! block per snapshot.
| insta::assert_snapshot!( | ||
| str::from_utf8(recorder.data()).unwrap(), | ||
| @" outer1 inner1 inner2 outer2 "); | ||
| @" outer1 inner1 inner2 outer2"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 |")There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
| // 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"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
affbdb5 to
dc066cd
Compare
dc066cd to
d528719
Compare
|
Do we want to merge this as is? I think it would be fine, but if not I'm gonna close this as the PR bitrots relatively quickly. |
|
I think it's better to fix whitespace issue first (in our tests.) I'll take a look. |
This is a replacement for #5558. Thanks to mitsuhiko/insta#722, this is now easy to generate.
This is a replacement for #5558. Thanks to mitsuhiko/insta#722, this is now easy to generate.
This is a replacement for #5558. Thanks to mitsuhiko/insta#722, this is now easy to generate.
This is a replacement for #5558. Thanks to mitsuhiko/insta#722, this is now easy to generate.
This is a replacement for #5558. Thanks to mitsuhiko/insta#722, this is now easy to generate.
This is a replacement for #5558. Thanks to mitsuhiko/insta#722, this is now easy to generate.
This is a replacement for #5558. Thanks to @yuja 's mitsuhiko/insta#722, this is now easy to generate.
This is a replacement for jj-vcs#5558. Thanks to @yuja 's mitsuhiko/insta#722, this is now easy to generate.
This is a replacement for #5558. Thanks to @yuja 's mitsuhiko/insta#722, this is now easy to generate.
This is a replacement for #5558. Thanks to @yuja 's mitsuhiko/insta#722, this is now easy to generate.
|
superseded by #5888 |
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#712Commit ID: 489c76379e4da3ed28bf69a4a0bdcebe7d99f2f5 Change ID: lmqxpvzykynsnmxwtkpuxvotrrsptomp Author : Jakob Hellermann <jakob.hellermann@protonmail.com> (26m ago) Committer: Jakob Hellermann <jakob.hellermann@protonmail.com> (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 <jakob.hellermann@protonmail.com> (27m ago) Committer: Jakob Hellermann <jakob.hellermann@protonmail.com> (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 test.user@example.com 2001-02-03 08:05:17 bookmark2 my-bookmark c4a3c310 │ (empty) foo │ ○ rlzusymt test.user@example.com 2001-02-03 08:05:10 8476341eIf applicable:
CHANGELOG.md