repo: move rebase_descendants_with_options_return_map() to tests

lib/src/repo.rs

@@ -1275,40 +1275,6 @@ impl MutableRepo {
         Ok(())
     }
 
-    /// Rebase descendants of the rewritten commits.
-    ///
-    /// The descendants of the commits registered in `self.parent_mappings` will
-    /// be recursively rebased onto the new version of their parents.
-    ///
-    /// If `options.empty` is the default (`EmptyBehaviour::Keep`), all
-    /// rebased descendant commits will be preserved even if they were
-    /// emptied following the rebase operation. A map of newly rebased
-    /// commit ID to original commit ID will be returned.
-    ///
-    /// Otherwise, this function may rebase some commits and abandon others,
-    /// based on the given `EmptyBehaviour`. The behavior is such that only
-    /// commits with a single parent will ever be abandoned. In the returned
-    /// map, an abandoned commit will look as a key-value pair where the key
-    /// is the abandoned commit and the value is **its parent**. One can
-    /// tell this case apart since the change ids of the key and the value
-    /// will not match. The parent will inherit the descendants and the
-    /// bookmarks of the abandoned commit.
-    pub fn rebase_descendants_with_options_return_map(
-        &mut self,
-        options: &RebaseOptions,
-    ) -> BackendResult<HashMap<CommitId, CommitId>> {
-        let mut rebased: HashMap<CommitId, CommitId> = HashMap::new();
-        self.rebase_descendants_with_options(options, |old_commit, rebased_commit| {
-            let old_commit_id = old_commit.id().clone();
-            let new_commit_id = match rebased_commit {
-                RebasedCommit::Rewritten(new_commit) => new_commit.id().clone(),
-                RebasedCommit::Abandoned { parent_id } => parent_id,
-            };
-            rebased.insert(old_commit_id, new_commit_id);
-        })?;
-        Ok(rebased)
-    }
-
     /// Rebase descendants of the rewritten commits.
     ///
     /// The descendants of the commits registered in `self.parent_mappings` will

lib/tests/test_commit_builder.rs

@@ -33,6 +33,7 @@ use pollster::FutureExt as _;
 use test_case::test_case;
 use testutils::assert_rebased_onto;
 use testutils::create_tree;
+use testutils::rebase_descendants_with_options_return_map;
 use testutils::CommitGraphBuilder;
 use testutils::TestRepo;
 use testutils::TestRepoBackend;
@@ -372,19 +373,15 @@ fn test_commit_builder_descendants(backend: TestRepoBackend) {
         )
         .write()
         .unwrap();
-    let rebase_map = tx
-        .repo_mut()
-        .rebase_descendants_with_options_return_map(&RebaseOptions::default())
-        .unwrap();
+    let rebase_map =
+        rebase_descendants_with_options_return_map(tx.repo_mut(), &RebaseOptions::default());
     assert_eq!(rebase_map.len(), 0);
 
     // Test with for_rewrite_from()
     let mut tx = repo.start_transaction();
     let commit4 = tx.repo_mut().rewrite_commit(&commit2).write().unwrap();
-    let rebase_map = tx
-        .repo_mut()
-        .rebase_descendants_with_options_return_map(&RebaseOptions::default())
-        .unwrap();
+    let rebase_map =
+        rebase_descendants_with_options_return_map(tx.repo_mut(), &RebaseOptions::default());
     assert_rebased_onto(tx.repo_mut(), &rebase_map, &commit3, &[commit4.id()]);
     assert_eq!(rebase_map.len(), 1);
 
@@ -395,9 +392,7 @@ fn test_commit_builder_descendants(backend: TestRepoBackend) {
         .generate_new_change_id()
         .write()
         .unwrap();
-    let rebase_map = tx
-        .repo_mut()
-        .rebase_descendants_with_options_return_map(&RebaseOptions::default())
-        .unwrap();
+    let rebase_map =
+        rebase_descendants_with_options_return_map(tx.repo_mut(), &RebaseOptions::default());
     assert!(rebase_map.is_empty());
 }

lib/tests/test_mut_repo.rs

@@ -23,6 +23,7 @@ use maplit::hashset;
 use testutils::assert_rebased_onto;
 use testutils::create_random_commit;
 use testutils::create_random_tree;
+use testutils::rebase_descendants_with_options_return_map;
 use testutils::write_random_commit;
 use testutils::CommitGraphBuilder;
 use testutils::TestRepo;
@@ -531,10 +532,8 @@ fn test_rebase_descendants_simple() {
     let commit6 = graph_builder.commit_with_parents(&[&commit1]);
     mut_repo.set_rewritten_commit(commit2.id().clone(), commit6.id().clone());
     mut_repo.record_abandoned_commit(&commit4);
-    let rebase_map = tx
-        .repo_mut()
-        .rebase_descendants_with_options_return_map(&RebaseOptions::default())
-        .unwrap();
+    let rebase_map =
+        rebase_descendants_with_options_return_map(tx.repo_mut(), &RebaseOptions::default());
     // Commit 3 got rebased onto commit 2's replacement, i.e. commit 6
     assert_rebased_onto(tx.repo_mut(), &rebase_map, &commit3, &[commit6.id()]);
     // Commit 5 got rebased onto commit 4's parent, i.e. commit 1
@@ -542,10 +541,8 @@ fn test_rebase_descendants_simple() {
     assert_eq!(rebase_map.len(), 2);
 
     // No more descendants to rebase if we try again.
-    let rebase_map = tx
-        .repo_mut()
-        .rebase_descendants_with_options_return_map(&RebaseOptions::default())
-        .unwrap();
+    let rebase_map =
+        rebase_descendants_with_options_return_map(tx.repo_mut(), &RebaseOptions::default());
     assert_eq!(rebase_map.len(), 0);
 }
 
@@ -575,10 +572,8 @@ fn test_rebase_descendants_divergent_rewrite() {
     );
     // Commit 3 does *not* get rebased because it's unclear if it should go onto
     // commit 4 or commit 5
-    let rebase_map = tx
-        .repo_mut()
-        .rebase_descendants_with_options_return_map(&RebaseOptions::default())
-        .unwrap();
+    let rebase_map =
+        rebase_descendants_with_options_return_map(tx.repo_mut(), &RebaseOptions::default());
     assert!(rebase_map.is_empty());
 }
 

lib/tests/test_rewrite.rs

@@ -35,6 +35,7 @@ use testutils::assert_abandoned_with_parent;
 use testutils::assert_rebased_onto;
 use testutils::create_random_commit;
 use testutils::create_tree;
+use testutils::rebase_descendants_with_options_return_map;
 use testutils::write_random_commit;
 use testutils::CommitGraphBuilder;
 use testutils::TestRepo;
@@ -98,10 +99,8 @@ fn test_rebase_descendants_sideways() {
 
     tx.repo_mut()
         .set_rewritten_commit(commit_b.id().clone(), commit_f.id().clone());
-    let rebase_map = tx
-        .repo_mut()
-        .rebase_descendants_with_options_return_map(&RebaseOptions::default())
-        .unwrap();
+    let rebase_map =
+        rebase_descendants_with_options_return_map(tx.repo_mut(), &RebaseOptions::default());
     assert_eq!(rebase_map.len(), 3);
     let new_commit_c = assert_rebased_onto(tx.repo_mut(), &rebase_map, &commit_c, &[commit_f.id()]);
     let new_commit_d =
@@ -151,10 +150,8 @@ fn test_rebase_descendants_forward() {
 
     tx.repo_mut()
         .set_rewritten_commit(commit_b.id().clone(), commit_f.id().clone());
-    let rebase_map = tx
-        .repo_mut()
-        .rebase_descendants_with_options_return_map(&RebaseOptions::default())
-        .unwrap();
+    let rebase_map =
+        rebase_descendants_with_options_return_map(tx.repo_mut(), &RebaseOptions::default());
     let new_commit_d =
         assert_rebased_onto(tx.repo_mut(), &rebase_map, &commit_d, &[(commit_f.id())]);
     let new_commit_f =
@@ -211,10 +208,8 @@ fn test_rebase_descendants_reorder() {
         .set_rewritten_commit(commit_c.id().clone(), commit_f.id().clone());
     tx.repo_mut()
         .set_rewritten_commit(commit_g.id().clone(), commit_h.id().clone());
-    let rebase_map = tx
-        .repo_mut()
-        .rebase_descendants_with_options_return_map(&RebaseOptions::default())
-        .unwrap();
+    let rebase_map =
+        rebase_descendants_with_options_return_map(tx.repo_mut(), &RebaseOptions::default());
     let new_commit_i = assert_rebased_onto(tx.repo_mut(), &rebase_map, &commit_i, &[commit_h.id()]);
     assert_eq!(rebase_map.len(), 1);
 
@@ -246,10 +241,8 @@ fn test_rebase_descendants_backward() {
 
     tx.repo_mut()
         .set_rewritten_commit(commit_c.id().clone(), commit_b.id().clone());
-    let rebase_map = tx
-        .repo_mut()
-        .rebase_descendants_with_options_return_map(&RebaseOptions::default())
-        .unwrap();
+    let rebase_map =
+        rebase_descendants_with_options_return_map(tx.repo_mut(), &RebaseOptions::default());
     let new_commit_d = assert_rebased_onto(tx.repo_mut(), &rebase_map, &commit_d, &[commit_b.id()]);
     assert_eq!(rebase_map.len(), 1);
 
@@ -287,10 +280,8 @@ fn test_rebase_descendants_chain_becomes_branchy() {
         .set_rewritten_commit(commit_b.id().clone(), commit_e.id().clone());
     tx.repo_mut()
         .set_rewritten_commit(commit_c.id().clone(), commit_f.id().clone());
-    let rebase_map = tx
-        .repo_mut()
-        .rebase_descendants_with_options_return_map(&RebaseOptions::default())
-        .unwrap();
+    let rebase_map =
+        rebase_descendants_with_options_return_map(tx.repo_mut(), &RebaseOptions::default());
     let new_commit_f = assert_rebased_onto(tx.repo_mut(), &rebase_map, &commit_f, &[commit_e.id()]);
     let new_commit_d =
         assert_rebased_onto(tx.repo_mut(), &rebase_map, &commit_d, &[new_commit_f.id()]);
@@ -330,10 +321,8 @@ fn test_rebase_descendants_internal_merge() {
 
     tx.repo_mut()
         .set_rewritten_commit(commit_b.id().clone(), commit_f.id().clone());
-    let rebase_map = tx
-        .repo_mut()
-        .rebase_descendants_with_options_return_map(&RebaseOptions::default())
-        .unwrap();
+    let rebase_map =
+        rebase_descendants_with_options_return_map(tx.repo_mut(), &RebaseOptions::default());
     let new_commit_c = assert_rebased_onto(tx.repo_mut(), &rebase_map, &commit_c, &[commit_f.id()]);
     let new_commit_d = assert_rebased_onto(tx.repo_mut(), &rebase_map, &commit_d, &[commit_f.id()]);
     let new_commit_e = assert_rebased_onto(
@@ -377,10 +366,8 @@ fn test_rebase_descendants_external_merge() {
 
     tx.repo_mut()
         .set_rewritten_commit(commit_c.id().clone(), commit_f.id().clone());
-    let rebase_map = tx
-        .repo_mut()
-        .rebase_descendants_with_options_return_map(&RebaseOptions::default())
-        .unwrap();
+    let rebase_map =
+        rebase_descendants_with_options_return_map(tx.repo_mut(), &RebaseOptions::default());
     let new_commit_e = assert_rebased_onto(
         tx.repo_mut(),
         &rebase_map,
@@ -420,10 +407,8 @@ fn test_rebase_descendants_abandon() {
 
     tx.repo_mut().record_abandoned_commit(&commit_b);
     tx.repo_mut().record_abandoned_commit(&commit_e);
-    let rebase_map = tx
-        .repo_mut()
-        .rebase_descendants_with_options_return_map(&RebaseOptions::default())
-        .unwrap();
+    let rebase_map =
+        rebase_descendants_with_options_return_map(tx.repo_mut(), &RebaseOptions::default());
     let new_commit_c = assert_rebased_onto(tx.repo_mut(), &rebase_map, &commit_c, &[commit_a.id()]);
     let new_commit_d = assert_rebased_onto(tx.repo_mut(), &rebase_map, &commit_d, &[commit_a.id()]);
     let new_commit_f =
@@ -457,10 +442,8 @@ fn test_rebase_descendants_abandon_no_descendants() {
 
     tx.repo_mut().record_abandoned_commit(&commit_b);
     tx.repo_mut().record_abandoned_commit(&commit_c);
-    let rebase_map = tx
-        .repo_mut()
-        .rebase_descendants_with_options_return_map(&RebaseOptions::default())
-        .unwrap();
+    let rebase_map =
+        rebase_descendants_with_options_return_map(tx.repo_mut(), &RebaseOptions::default());
     assert_eq!(rebase_map.len(), 0);
 
     assert_eq!(
@@ -495,10 +478,8 @@ fn test_rebase_descendants_abandon_and_replace() {
     tx.repo_mut()
         .set_rewritten_commit(commit_b.id().clone(), commit_e.id().clone());
     tx.repo_mut().record_abandoned_commit(&commit_c);
-    let rebase_map = tx
-        .repo_mut()
-        .rebase_descendants_with_options_return_map(&RebaseOptions::default())
-        .unwrap();
+    let rebase_map =
+        rebase_descendants_with_options_return_map(tx.repo_mut(), &RebaseOptions::default());
     let new_commit_d = assert_rebased_onto(tx.repo_mut(), &rebase_map, &commit_d, &[commit_e.id()]);
     assert_eq!(rebase_map.len(), 1);
 
@@ -529,13 +510,13 @@ fn test_rebase_descendants_abandon_degenerate_merge_simplify() {
     let commit_d = graph_builder.commit_with_parents(&[&commit_b, &commit_c]);
 
     tx.repo_mut().record_abandoned_commit(&commit_b);
-    let rebase_map = tx
-        .repo_mut()
-        .rebase_descendants_with_options_return_map(&RebaseOptions {
+    let rebase_map = rebase_descendants_with_options_return_map(
+        tx.repo_mut(),
+        &RebaseOptions {
             simplify_ancestor_merge: true,
             ..Default::default()
-        })
-        .unwrap();
+        },
+    );
     let new_commit_d = assert_rebased_onto(tx.repo_mut(), &rebase_map, &commit_d, &[commit_c.id()]);
     assert_eq!(rebase_map.len(), 1);
 
@@ -566,13 +547,13 @@ fn test_rebase_descendants_abandon_degenerate_merge_preserve() {
     let commit_d = graph_builder.commit_with_parents(&[&commit_b, &commit_c]);
 
     tx.repo_mut().record_abandoned_commit(&commit_b);
-    let rebase_map = tx
-        .repo_mut()
-        .rebase_descendants_with_options_return_map(&RebaseOptions {
+    let rebase_map = rebase_descendants_with_options_return_map(
+        tx.repo_mut(),
+        &RebaseOptions {
             simplify_ancestor_merge: false,
             ..Default::default()
-        })
-        .unwrap();
+        },
+    );
     let new_commit_d = assert_rebased_onto(
         tx.repo_mut(),
         &rebase_map,
@@ -612,10 +593,8 @@ fn test_rebase_descendants_abandon_widen_merge() {
     let commit_f = graph_builder.commit_with_parents(&[&commit_e, &commit_d]);
 
     tx.repo_mut().record_abandoned_commit(&commit_e);
-    let rebase_map = tx
-        .repo_mut()
-        .rebase_descendants_with_options_return_map(&RebaseOptions::default())
-        .unwrap();
+    let rebase_map =
+        rebase_descendants_with_options_return_map(tx.repo_mut(), &RebaseOptions::default());
     let new_commit_f = assert_rebased_onto(
         tx.repo_mut(),
         &rebase_map,
@@ -656,10 +635,8 @@ fn test_rebase_descendants_multiple_sideways() {
         .set_rewritten_commit(commit_b.id().clone(), commit_f.id().clone());
     tx.repo_mut()
         .set_rewritten_commit(commit_d.id().clone(), commit_f.id().clone());
-    let rebase_map = tx
-        .repo_mut()
-        .rebase_descendants_with_options_return_map(&RebaseOptions::default())
-        .unwrap();
+    let rebase_map =
+        rebase_descendants_with_options_return_map(tx.repo_mut(), &RebaseOptions::default());
     let new_commit_c = assert_rebased_onto(tx.repo_mut(), &rebase_map, &commit_c, &[commit_f.id()]);
     let new_commit_e = assert_rebased_onto(tx.repo_mut(), &rebase_map, &commit_e, &[commit_f.id()]);
     assert_eq!(rebase_map.len(), 2);
@@ -773,10 +750,8 @@ fn test_rebase_descendants_divergent_rewrite() {
     );
     tx.repo_mut()
         .set_rewritten_commit(commit_f.id().clone(), commit_f2.id().clone());
-    let rebase_map = tx
-        .repo_mut()
-        .rebase_descendants_with_options_return_map(&RebaseOptions::default())
-        .unwrap();
+    let rebase_map =
+        rebase_descendants_with_options_return_map(tx.repo_mut(), &RebaseOptions::default());
     let new_commit_c =
         assert_rebased_onto(tx.repo_mut(), &rebase_map, &commit_c, &[commit_b2.id()]);
     let new_commit_g =
@@ -823,10 +798,8 @@ fn test_rebase_descendants_repeated() {
         .set_description("b2")
         .write()
         .unwrap();
-    let rebase_map = tx
-        .repo_mut()
-        .rebase_descendants_with_options_return_map(&RebaseOptions::default())
-        .unwrap();
+    let rebase_map =
+        rebase_descendants_with_options_return_map(tx.repo_mut(), &RebaseOptions::default());
     let commit_c2 = assert_rebased_onto(tx.repo_mut(), &rebase_map, &commit_c, &[commit_b2.id()]);
     assert_eq!(rebase_map.len(), 1);
 
@@ -838,10 +811,8 @@ fn test_rebase_descendants_repeated() {
     );
 
     // We made no more changes, so nothing should be rebased.
-    let rebase_map = tx
-        .repo_mut()
-        .rebase_descendants_with_options_return_map(&RebaseOptions::default())
-        .unwrap();
+    let rebase_map =
+        rebase_descendants_with_options_return_map(tx.repo_mut(), &RebaseOptions::default());
     assert_eq!(rebase_map.len(), 0);
 
     // Now mark B3 as rewritten from B2 and rebase descendants again.
@@ -851,10 +822,8 @@ fn test_rebase_descendants_repeated() {
         .set_description("b3")
         .write()
         .unwrap();
-    let rebase_map = tx
-        .repo_mut()
-        .rebase_descendants_with_options_return_map(&RebaseOptions::default())
-        .unwrap();
+    let rebase_map =
+        rebase_descendants_with_options_return_map(tx.repo_mut(), &RebaseOptions::default());
     let commit_c3 = assert_rebased_onto(tx.repo_mut(), &rebase_map, &commit_c2, &[commit_b3.id()]);
     assert_eq!(rebase_map.len(), 1);
 
@@ -912,10 +881,8 @@ fn test_rebase_descendants_contents() {
 
     tx.repo_mut()
         .set_rewritten_commit(commit_b.id().clone(), commit_d.id().clone());
-    let rebase_map = tx
-        .repo_mut()
-        .rebase_descendants_with_options_return_map(&RebaseOptions::default())
-        .unwrap();
+    let rebase_map =
+        rebase_descendants_with_options_return_map(tx.repo_mut(), &RebaseOptions::default());
     assert_eq!(rebase_map.len(), 1);
     let new_commit_c = repo
         .store()
@@ -1570,13 +1537,13 @@ fn test_empty_commit_option(empty_behavior: EmptyBehaviour) {
 
     tx.repo_mut()
         .set_rewritten_commit(commit_b.id().clone(), commit_bd.id().clone());
-    let rebase_map = tx
-        .repo_mut()
-        .rebase_descendants_with_options_return_map(&RebaseOptions {
+    let rebase_map = rebase_descendants_with_options_return_map(
+        tx.repo_mut(),
+        &RebaseOptions {
             empty: empty_behavior,
             simplify_ancestor_merge: true,
-        })
-        .unwrap();
+        },
+    );
 
     let new_head = match empty_behavior {
         EmptyBehaviour::Keep => {
@@ -1710,10 +1677,7 @@ fn test_rebase_abandoning_empty() {
     };
     let rewriter = CommitRewriter::new(tx.repo_mut(), commit_b, vec![commit_b2.id().clone()]);
     rebase_commit_with_options(rewriter, &rebase_options).unwrap();
-    let rebase_map = tx
-        .repo_mut()
-        .rebase_descendants_with_options_return_map(&rebase_options)
-        .unwrap();
+    let rebase_map = rebase_descendants_with_options_return_map(tx.repo_mut(), &rebase_options);
     assert_eq!(rebase_map.len(), 5);
     let new_commit_c =
         assert_rebased_onto(tx.repo_mut(), &rebase_map, &commit_c, &[commit_b2.id()]);

lib/testutils/src/lib.rs

@@ -51,6 +51,8 @@ use jj_lib::repo::RepoLoader;
 use jj_lib::repo::StoreFactories;
 use jj_lib::repo_path::RepoPath;
 use jj_lib::repo_path::RepoPathBuf;
+use jj_lib::rewrite::RebaseOptions;
+use jj_lib::rewrite::RebasedCommit;
 use jj_lib::secret_backend::SecretBackend;
 use jj_lib::settings::UserSettings;
 use jj_lib::signing::Signer;
@@ -521,6 +523,25 @@ impl<'repo> CommitGraphBuilder<'repo> {
     }
 }
 
+/// Rebase descendants of the rewritten commits. Returns map of original commit
+/// ID to rebased (or abandoned parent) commit ID.
+pub fn rebase_descendants_with_options_return_map(
+    repo: &mut MutableRepo,
+    options: &RebaseOptions,
+) -> HashMap<CommitId, CommitId> {
+    let mut rebased: HashMap<CommitId, CommitId> = HashMap::new();
+    repo.rebase_descendants_with_options(options, |old_commit, rebased_commit| {
+        let old_commit_id = old_commit.id().clone();
+        let new_commit_id = match rebased_commit {
+            RebasedCommit::Rewritten(new_commit) => new_commit.id().clone(),
+            RebasedCommit::Abandoned { parent_id } => parent_id,
+        };
+        rebased.insert(old_commit_id, new_commit_id);
+    })
+    .unwrap();
+    rebased
+}
+
 fn assert_in_rebased_map(
     repo: &impl Repo,
     rebased: &HashMap<CommitId, CommitId>,