test_abandon_command: create another test for bug 2600

cli/tests/test_abandon_command.rs

@@ -43,15 +43,15 @@ fn test_basics() {
     create_commit(&test_env, &repo_path, "e", &["a", "d"]);
     // Test the setup
     insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
-    @    e
+    @    [znk] e
     ├─╮
-    │ ◉  d
-    │ ◉  c
-    │ │ ◉  b
+    │ ◉  [vru] d
+    │ ◉  [roy] c
+    │ │ ◉  [zsu] b
     ├───╯
-    ◉ │  a
+    ◉ │  [rlv] a
     ├─╯
-    ◉
+    ◉  [zzz]
     "###);
 
     let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["abandon", "d"]);
@@ -65,14 +65,14 @@ fn test_basics() {
     Added 0 files, modified 0 files, removed 1 files
     "###);
     insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
-    @    e
+    @    [znk] e
     ├─╮
-    │ ◉  c d
-    │ │ ◉  b
+    │ ◉  [roy] c d
+    │ │ ◉  [zsu] b
     ├───╯
-    ◉ │  a
+    ◉ │  [rlv] a
     ├─╯
-    ◉
+    ◉  [zzz]
     "###);
 
     test_env.jj_cmd_ok(&repo_path, &["undo"]);
@@ -85,14 +85,14 @@ fn test_basics() {
     Added 0 files, modified 0 files, removed 3 files
     "###);
     insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
-    @
-    │ ◉  b
+    @  [nkm]
+    │ ◉  [zsu] b
     ├─╯
-    ◉  a e??
-    │ ◉  d e??
-    │ ◉  c
+    ◉  [rlv] a e??
+    │ ◉  [vru] d e??
+    │ ◉  [roy] c
     ├─╯
-    ◉
+    ◉  [zzz]
     "###);
 
     // Abandoning `a` would normally result in its descendant merge commit, `e`,
@@ -109,12 +109,12 @@ fn test_basics() {
     Added 0 files, modified 0 files, removed 1 files
     "###);
     insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
-    @  e
-    ◉  d
-    ◉  c
-    │ ◉  b
+    @  [znk] e
+    ◉  [vru] d
+    ◉  [roy] c
+    │ ◉  [zsu] b
     ├─╯
-    ◉  a
+    ◉  [zzz] a
     "###);
 
     test_env.jj_cmd_ok(&repo_path, &["undo"]);
@@ -130,11 +130,11 @@ fn test_basics() {
     Added 0 files, modified 0 files, removed 3 files
     "###);
     insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
-    @
-    │ ◉  b
+    @  [wvu]
+    │ ◉  [zsu] b
     ├─╯
-    ◉  a e??
-    ◉  c d e??
+    ◉  [rlv] a e??
+    ◉  [zzz] c d e??
     "###);
 
     // Test abandoning the same commit twice directly
@@ -145,13 +145,13 @@ fn test_basics() {
     Abandoned commit zsuskuln 1394f625 b | b
     "###);
     insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
-    @    e
+    @    [znk] e
     ├─╮
-    │ ◉  d
-    │ ◉  c
-    ◉ │  a b
+    │ ◉  [vru] d
+    │ ◉  [roy] c
+    ◉ │  [rlv] a b
     ├─╯
-    ◉
+    ◉  [zzz]
     "###);
 
     // Test abandoning the same commit twice indirectly
@@ -169,10 +169,197 @@ fn test_basics() {
     Added 0 files, modified 0 files, removed 4 files
     "###);
     insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
-    @
-    │ ◉  c d e??
+    @  [oup]
+    │ ◉  [roy] c d e??
     ├─╯
-    ◉  a b e??
+    ◉  [zzz] a b e??
+    "###);
+}
+
+// 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
     "###);
 }
 
@@ -213,5 +400,12 @@ fn test_double_abandon() {
 }
 
 fn get_log_output(test_env: &TestEnvironment, repo_path: &Path) -> String {
-    test_env.jj_cmd_success(repo_path, &["log", "-T", "branches"])
+    test_env.jj_cmd_success(
+        repo_path,
+        &[
+            "log",
+            "-T",
+            r#"separate(" ", "[" ++ change_id.short(3) ++ "]", branches)"#,
+        ],
+    )
 }