From feb6623d196de0504bf5591388468209bc7845b9 Mon Sep 17 00:00:00 2001 From: Ilya Grigoriev Date: Sun, 19 Nov 2023 18:38:59 -0800 Subject: [PATCH] test_abandon_command: create another test for #2600 See comments inline for details. In particular, I wanted to make sure these behaviors are not affected by #2646. They don't seem to be. --- cli/tests/test_abandon_command.rs | 187 ++++++++++++++++++++++++++++++ 1 file changed, 187 insertions(+) diff --git a/cli/tests/test_abandon_command.rs b/cli/tests/test_abandon_command.rs index 86a6f28b03..51d692f23e 100644 --- a/cli/tests/test_abandon_command.rs +++ b/cli/tests/test_abandon_command.rs @@ -176,6 +176,193 @@ fn test_basics() { "###); } +// TODO(#2600): Make sure the results here become saner as #2600 is fixed. There +// is an simpler demo of #2600 at https://github.com/martinvonz/jj/pull/2655. +// However, fixing #2600 will likely change how `abandon` works. This test +// exists to track how that happens. See also the corresponding test in +// `test_rebase_command` +#[test] +fn test_bug_2600() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo", "--git"]); + let repo_path = test_env.env_root().join("repo"); + + // We will not touch "nottherootcommit". See the + // `test_bug_2600_rootcommit_special_case` for the one case where base being the + // child of the root commit changes the expected behavior. + create_commit(&test_env, &repo_path, "nottherootcommit", &[]); + create_commit(&test_env, &repo_path, "base", &["nottherootcommit"]); + create_commit(&test_env, &repo_path, "a", &["base"]); + create_commit(&test_env, &repo_path, "b", &["base", "a"]); + create_commit(&test_env, &repo_path, "c", &["b"]); + + // Test the setup + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ [znk] c + ◉ [vru] b + ├─╮ + │ ◉ [roy] a + ├─╯ + ◉ [zsu] base + ◉ [rlv] nottherootcommit + ◉ [zzz] + "###); + let setup_opid = test_env.current_operation_id(&repo_path); + + test_env.jj_cmd_ok(&repo_path, &["op", "restore", &setup_opid]); + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["abandon", "base"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Abandoned commit zsuskuln 73c929fc base | base + Rebased 3 descendant commits onto parents of abandoned commits + Working copy now at: znkkpsqq 510f8756 c | c + Parent commit : vruxwmqv 7301d9ab b | b + Added 0 files, modified 0 files, removed 1 files + "###); + // BUG. The user would expect + // @ c + // ├─╮ + // │ ◉ a + // ├─╯ + // ◉ base nottherootcommit + // ◉ + // This is likely caused by DescendantRebaser + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ [znk] c + ◉ [vru] b + ◉ [roy] a + ◉ [rlv] base nottherootcommit + ◉ [zzz] + "###); + + test_env.jj_cmd_ok(&repo_path, &["op", "restore", &setup_opid]); + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["abandon", "a"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Abandoned commit royxmykx 98f3b9ba a | a + Rebased 2 descendant commits onto parents of abandoned commits + Working copy now at: znkkpsqq 683b9435 c | c + Parent commit : vruxwmqv c10cb7b4 b | b + Added 0 files, modified 0 files, removed 1 files + "###); + // This is likely what the user will expect. + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ [znk] c + ◉ [vru] b + ◉ [zsu] a base + ◉ [rlv] nottherootcommit + ◉ [zzz] + "###); + + test_env.jj_cmd_ok(&repo_path, &["op", "restore", &setup_opid]); + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["abandon", "b"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Abandoned commit vruxwmqv 8c0dced0 b | b + Rebased 1 descendant commits onto parents of abandoned commits + Working copy now at: znkkpsqq 924bdd1c c | c + Parent commit : royxmykx 98f3b9ba a b?? | a + Added 0 files, modified 0 files, removed 1 files + "###); + // BUG. The user would expect + // @ c + // ├─╮ + // │ ◉ a + // ├─╯ + // ◉ base + // ◉ nottherootcommit + // ◉ + // This is likely caused by logic in `cmd_abandon`, not DescendantRebaser + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ [znk] c + ◉ [roy] a b?? + ◉ [zsu] b?? base + ◉ [rlv] nottherootcommit + ◉ [zzz] + "###); + + test_env.jj_cmd_ok(&repo_path, &["op", "restore", &setup_opid]); + // ========= Reminder of the setup =========== + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ [znk] c + ◉ [vru] b + ├─╮ + │ ◉ [roy] a + ├─╯ + ◉ [zsu] base + ◉ [rlv] nottherootcommit + ◉ [zzz] + "###); + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["abandon", "a", "b"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Abandoned the following commits: + royxmykx 98f3b9ba a | a + vruxwmqv 8c0dced0 b | b + Rebased 1 descendant commits onto parents of abandoned commits + Working copy now at: znkkpsqq 84fac1f8 c | c + Parent commit : zsuskuln 73c929fc a b base | base + Added 0 files, modified 0 files, removed 2 files + "###); + // This is likely what the user would expect + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ [znk] c + ◉ [zsu] a b base + ◉ [rlv] nottherootcommit + ◉ [zzz] + "###); + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["branch", "list", "b"]); + insta::assert_snapshot!(stdout, @r###" + b: zsuskuln 73c929fc base + "###); + insta::assert_snapshot!(stderr, @""); +} + +#[test] +fn test_bug_2600_rootcommit_special_case() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo", "--git"]); + let repo_path = test_env.env_root().join("repo"); + + // Set up like `test_bug_2600`, but without the `nottherootcommit` commit. + create_commit(&test_env, &repo_path, "base", &[]); + create_commit(&test_env, &repo_path, "a", &["base"]); + create_commit(&test_env, &repo_path, "b", &["base", "a"]); + create_commit(&test_env, &repo_path, "c", &["b"]); + + // Setup + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ [vru] c + ◉ [roy] b + ├─╮ + │ ◉ [zsu] a + ├─╯ + ◉ [rlv] base + ◉ [zzz] + "###); + + // Now, the test + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["abandon", "base"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r###" + Abandoned commit rlvkpnrz 0c61db1b base | base + Rebased 3 descendant commits onto parents of abandoned commits + Working copy now at: vruxwmqv 73e9185c c | c + Parent commit : royxmykx 80dd9cba b | b + Added 0 files, modified 0 files, removed 1 files + "###); + // The current behavior is either correct or should be replaced with an error + // message. Even though the user would expect `b` to still be a descendant of + // `base`, it is impossible in the Git backend. + // See also https://github.com/martinvonz/jj/issues/2600#issuecomment-1835418824 + insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" + @ [vru] c + ◉ [roy] b + ◉ [zsu] a + ◉ [zzz] base + "###); +} + #[test] fn test_double_abandon() { let test_env = TestEnvironment::default();