Skip to content

Commit 9c165d6

Browse files
committed
tests: supplement create_tree() with a builder-style API
The original form of `create_tree()` is limited to creating (valid UTF-8) text files but cannot create binary files, executable files, or symlinks. Dedicated helpers like `write_executable_file()` or `write_symlink()` partially compensated for this, but required manually assembling the tree in the test code. This commit introduces `TestTreeBuilder` which provides an API to successively add entries to a tree which can represent all of the above. `TestTreeBuilder` can then create either a single `Tree`, or a resolved `MergedTree`. In addition to using `TestTreeBuilder` directly, `create_tree_with()` and `create_single_tree_with()` accept a closure which receives a `TestTreeBuilder`. This allows test code to quickly describe the tree without requiring the a named builder at caller scope. Riffing off the familiar function names should help in discovering the new builder facilities. However, it is completely possible to use `TestTreeBuilder` directly, if preferred.
1 parent a92651e commit 9c165d6

File tree

6 files changed

+266
-264
lines changed

6 files changed

+266
-264
lines changed

cli/src/merge_tools/builtin.rs

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -863,17 +863,11 @@ mod tests {
863863

864864
let added_executable_file_path = repo_path("executable_file");
865865
let left_tree = testutils::create_tree(&test_repo.repo, &[]);
866-
let right_tree = {
867-
// let store = test_repo.repo.store();
868-
let mut tree_builder = store.tree_builder(store.empty_tree_id().clone());
869-
testutils::write_executable_file(
870-
&mut tree_builder,
871-
added_executable_file_path,
872-
"executable",
873-
);
874-
let id = tree_builder.write_tree().unwrap();
875-
MergedTree::resolved(store.get_tree(RepoPathBuf::root(), &id).unwrap())
876-
};
866+
let right_tree = testutils::create_tree_with(&test_repo.repo, |builder| {
867+
builder
868+
.file(added_executable_file_path, "executable")
869+
.executable(true);
870+
});
877871

878872
let (changed_files, files) = make_diff(store, &left_tree, &right_tree);
879873
insta::assert_debug_snapshot!(changed_files, @r#"

lib/tests/test_fix.rs

Lines changed: 29 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,7 @@
1414

1515
use std::collections::HashMap;
1616
use std::collections::HashSet;
17-
use std::sync::Arc;
1817

19-
use itertools::Itertools as _;
2018
use jj_lib::backend::CommitId;
2119
use jj_lib::backend::FileId;
2220
use jj_lib::backend::MergedTreeId;
@@ -26,13 +24,12 @@ use jj_lib::fix::FileToFix;
2624
use jj_lib::fix::FixError;
2725
use jj_lib::fix::ParallelFileFixer;
2826
use jj_lib::matchers::EverythingMatcher;
29-
use jj_lib::merged_tree::MergedTree;
30-
use jj_lib::repo::ReadonlyRepo;
3127
use jj_lib::repo::Repo as _;
3228
use jj_lib::store::Store;
3329
use jj_lib::transaction::Transaction;
3430
use pollster::FutureExt as _;
3531
use testutils::create_tree;
32+
use testutils::create_tree_with;
3633
use testutils::read_file;
3734
use testutils::repo_path;
3835
use testutils::TestRepo;
@@ -93,17 +90,6 @@ fn fix_file(store: &Store, file_to_fix: &FileToFix) -> Result<Option<FileId>, Fi
9390
}
9491
}
9592

96-
fn create_tree_helper(
97-
repo: &Arc<ReadonlyRepo>,
98-
path_and_content: &[(String, String)],
99-
) -> MergedTree {
100-
let content_map = path_and_content
101-
.iter()
102-
.map(|p| (repo_path(&p.0), p.1.as_str()))
103-
.collect_vec();
104-
create_tree(repo, &content_map)
105-
}
106-
10793
fn create_commit(tx: &mut Transaction, parents: Vec<CommitId>, tree_id: MergedTreeId) -> CommitId {
10894
tx.repo_mut()
10995
.new_commit(parents, tree_id)
@@ -418,13 +404,11 @@ fn test_parallel_fixer_fixes_files() {
418404
let repo = &test_repo.repo;
419405

420406
let mut tx = repo.start_transaction();
421-
let mut path_contents1 = vec![];
422-
for i in 0..100 {
423-
let path = format!("file{i}");
424-
let content = format!("fixme:content{i}");
425-
path_contents1.push((path, content));
426-
}
427-
let tree1 = create_tree_helper(repo, &path_contents1);
407+
let tree1 = create_tree_with(repo, |builder| {
408+
for i in 0..100 {
409+
builder.file(repo_path(&format!("file{i}")), format!("fixme:content{i}"));
410+
}
411+
});
428412
let commit_a = create_commit(
429413
&mut tx,
430414
vec![repo.store().root_commit_id().clone()],
@@ -444,13 +428,11 @@ fn test_parallel_fixer_fixes_files() {
444428
)
445429
.unwrap();
446430

447-
let mut expected_path_contents = vec![];
448-
for i in 0..100 {
449-
let path = format!("file{i}");
450-
let content = format!("CONTENT{i}");
451-
expected_path_contents.push((path, content));
452-
}
453-
let expected_tree_a = create_tree_helper(repo, &expected_path_contents);
431+
let expected_tree_a = create_tree_with(repo, |builder| {
432+
for i in 0..100 {
433+
builder.file(repo_path(&format!("file{i}")), format!("CONTENT{i}"));
434+
}
435+
});
454436

455437
assert_eq!(summary.rewrites.len(), 1);
456438
assert!(summary.rewrites.contains_key(&commit_a));
@@ -470,13 +452,11 @@ fn test_parallel_fixer_does_not_change_content() {
470452
let repo = &test_repo.repo;
471453

472454
let mut tx = repo.start_transaction();
473-
let mut path_contents1 = vec![];
474-
for i in 0..100 {
475-
let path = format!("file{i}");
476-
let content = format!("content{i}");
477-
path_contents1.push((path, content));
478-
}
479-
let tree1 = create_tree_helper(repo, &path_contents1);
455+
let tree1 = create_tree_with(repo, |builder| {
456+
for i in 0..100 {
457+
builder.file(repo_path(&format!("file{i}")), format!("content{i}"));
458+
}
459+
});
480460
let commit_a = create_commit(
481461
&mut tx,
482462
vec![repo.store().root_commit_id().clone()],
@@ -507,19 +487,19 @@ fn test_parallel_fixer_no_changes_upon_partial_failure() {
507487
let repo = &test_repo.repo;
508488

509489
let mut tx = repo.start_transaction();
510-
let mut path_contents1 = vec![];
511-
for i in 0..100 {
512-
let path = format!("file{i}");
513-
let content = if i == 7 {
514-
format!("error:boo{i}")
515-
} else if i % 3 == 0 {
516-
format!("fixme:content{i}")
517-
} else {
518-
format!("foobar:{i}")
519-
};
520-
path_contents1.push((path, content));
521-
}
522-
let tree1 = create_tree_helper(repo, &path_contents1);
490+
let tree1 = create_tree_with(repo, |builder| {
491+
for i in 0..100 {
492+
let contents = if i == 7 {
493+
format!("error:boo{i}")
494+
} else if i % 3 == 0 {
495+
format!("fixme:content{i}")
496+
} else {
497+
format!("foobar:{i}")
498+
};
499+
500+
builder.file(repo_path(&format!("file{i}")), &contents);
501+
}
502+
});
523503
let commit_a = create_commit(
524504
&mut tx,
525505
vec![repo.store().root_commit_id().clone()],

lib/tests/test_git.rs

Lines changed: 62 additions & 107 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,9 @@ use itertools::Itertools as _;
2929
use jj_lib::backend::BackendError;
3030
use jj_lib::backend::ChangeId;
3131
use jj_lib::backend::CommitId;
32-
use jj_lib::backend::MergedTreeId;
3332
use jj_lib::backend::MillisSinceEpoch;
3433
use jj_lib::backend::Signature;
3534
use jj_lib::backend::Timestamp;
36-
use jj_lib::backend::TreeValue;
3735
use jj_lib::commit::Commit;
3836
use jj_lib::commit_builder::CommitBuilder;
3937
use jj_lib::config::ConfigLayer;
@@ -68,7 +66,6 @@ use jj_lib::settings::GitSettings;
6866
use jj_lib::settings::UserSettings;
6967
use jj_lib::signing::Signer;
7068
use jj_lib::str_util::StringPattern;
71-
use jj_lib::tree_builder::TreeBuilder;
7269
use jj_lib::workspace::Workspace;
7370
use maplit::btreemap;
7471
use maplit::hashset;
@@ -78,7 +75,6 @@ use testutils::base_user_config;
7875
use testutils::commit_transactions;
7976
use testutils::create_random_commit;
8077
use testutils::repo_path;
81-
use testutils::repo_path_buf;
8278
use testutils::write_random_commit;
8379
use testutils::CommitGraphBuilder;
8480
use testutils::TestRepo;
@@ -2559,30 +2555,20 @@ fn test_reset_head_with_index_no_conflict() {
25592555
let mut_repo = tx.repo_mut();
25602556

25612557
// Build tree containing every mode of file
2562-
let tree_id = {
2563-
let mut tree_builder =
2564-
TreeBuilder::new(repo.store().clone(), repo.store().empty_tree_id().clone());
2565-
testutils::write_normal_file(
2566-
&mut tree_builder,
2567-
repo_path("some/dir/normal-file"),
2568-
"file\n",
2558+
let tree_id = testutils::create_tree_with(&repo, |builder| {
2559+
builder
2560+
.file(repo_path("some/dir/normal-file"), "file\n")
2561+
.executable(false);
2562+
builder
2563+
.file(repo_path("some/dir/executable-file"), "file\n")
2564+
.executable(true);
2565+
builder.symlink(repo_path("some/dir/symlink"), "./normal-file");
2566+
builder.submodule(
2567+
repo_path("some/dir/commit"),
2568+
testutils::write_random_commit(mut_repo).id().clone(),
25692569
);
2570-
testutils::write_executable_file(
2571-
&mut tree_builder,
2572-
repo_path("some/dir/executable-file"),
2573-
"file\n",
2574-
);
2575-
testutils::write_symlink(
2576-
&mut tree_builder,
2577-
repo_path("some/dir/symlink"),
2578-
"./normal-file",
2579-
);
2580-
tree_builder.set(
2581-
repo_path_buf("some/dir/commit"),
2582-
TreeValue::GitSubmodule(testutils::write_random_commit(mut_repo).id().clone()),
2583-
);
2584-
MergedTreeId::resolved(tree_builder.write_tree().unwrap())
2585-
};
2570+
})
2571+
.id();
25862572

25872573
let parent_commit = mut_repo
25882574
.new_commit(vec![repo.store().root_commit_id().clone()], tree_id.clone())
@@ -2622,76 +2608,50 @@ fn test_reset_head_with_index_merge_conflict() {
26222608
let mut_repo = tx.repo_mut();
26232609

26242610
// Build conflict trees containing every mode of file
2625-
let base_tree_id = {
2626-
let mut tree_builder =
2627-
TreeBuilder::new(repo.store().clone(), repo.store().empty_tree_id().clone());
2628-
testutils::write_normal_file(
2629-
&mut tree_builder,
2630-
repo_path("some/dir/normal-file"),
2631-
"base\n",
2632-
);
2633-
testutils::write_executable_file(
2634-
&mut tree_builder,
2635-
repo_path("some/dir/executable-file"),
2636-
"base\n",
2637-
);
2638-
testutils::write_symlink(
2639-
&mut tree_builder,
2640-
repo_path("some/dir/symlink"),
2641-
"./normal-file",
2642-
);
2643-
tree_builder.set(
2644-
repo_path_buf("some/dir/commit"),
2645-
TreeValue::GitSubmodule(testutils::write_random_commit(mut_repo).id().clone()),
2646-
);
2647-
MergedTreeId::resolved(tree_builder.write_tree().unwrap())
2648-
};
2649-
2650-
let left_tree_id = {
2651-
let mut tree_builder =
2652-
TreeBuilder::new(repo.store().clone(), repo.store().empty_tree_id().clone());
2653-
testutils::write_normal_file(
2654-
&mut tree_builder,
2655-
repo_path("some/dir/normal-file"),
2656-
"left\n",
2657-
);
2658-
testutils::write_executable_file(
2659-
&mut tree_builder,
2660-
repo_path("some/dir/executable-file"),
2661-
"left\n",
2662-
);
2663-
testutils::write_symlink(
2664-
&mut tree_builder,
2665-
repo_path("some/dir/symlink"),
2666-
"./executable-file",
2611+
let base_tree_id = testutils::create_tree_with(&repo, |builder| {
2612+
builder
2613+
.file(repo_path("some/dir/normal-file"), "base\n")
2614+
.executable(false);
2615+
builder
2616+
.file(repo_path("some/dir/executable-file"), "base\n")
2617+
.executable(true);
2618+
builder.symlink(repo_path("some/dir/symlink"), "./normal-file");
2619+
builder.submodule(
2620+
repo_path("some/dir/commit"),
2621+
testutils::write_random_commit(mut_repo).id().clone(),
26672622
);
2668-
tree_builder.set(
2669-
repo_path_buf("some/dir/commit"),
2670-
TreeValue::GitSubmodule(testutils::write_random_commit(mut_repo).id().clone()),
2671-
);
2672-
MergedTreeId::resolved(tree_builder.write_tree().unwrap())
2673-
};
2674-
2675-
let right_tree_id = {
2676-
let mut tree_builder =
2677-
TreeBuilder::new(repo.store().clone(), repo.store().empty_tree_id().clone());
2678-
testutils::write_normal_file(
2679-
&mut tree_builder,
2680-
repo_path("some/dir/normal-file"),
2681-
"right\n",
2682-
);
2683-
testutils::write_executable_file(
2684-
&mut tree_builder,
2685-
repo_path("some/dir/executable-file"),
2686-
"right\n",
2623+
})
2624+
.id();
2625+
2626+
let left_tree_id = testutils::create_tree_with(&repo, |builder| {
2627+
builder
2628+
.file(repo_path("some/dir/normal-file"), "left\n")
2629+
.executable(false);
2630+
builder
2631+
.file(repo_path("some/dir/executable-file"), "left\n")
2632+
.executable(true);
2633+
builder.symlink(repo_path("some/dir/symlink"), "./executable-file");
2634+
builder.submodule(
2635+
repo_path("some/dir/commit"),
2636+
testutils::write_random_commit(mut_repo).id().clone(),
26872637
);
2688-
testutils::write_symlink(&mut tree_builder, repo_path("some/dir/symlink"), "./commit");
2689-
tree_builder.set(
2690-
repo_path_buf("some/dir/commit"),
2691-
TreeValue::GitSubmodule(testutils::write_random_commit(mut_repo).id().clone()),
2638+
})
2639+
.id();
2640+
2641+
let right_tree_id = testutils::create_tree_with(&repo, |builder| {
2642+
builder
2643+
.file(repo_path("some/dir/normal-file"), "right\n")
2644+
.executable(false);
2645+
builder
2646+
.file(repo_path("some/dir/executable-file"), "right\n")
2647+
.executable(true);
2648+
builder.symlink(repo_path("some/dir/symlink"), "./commit");
2649+
builder.submodule(
2650+
repo_path("some/dir/commit"),
2651+
testutils::write_random_commit(mut_repo).id().clone(),
26922652
);
2693-
MergedTreeId::resolved(tree_builder.write_tree().unwrap())
2694-
};
2653+
})
2654+
.id();
26952655

26962656
let base_commit = mut_repo
26972657
.new_commit(
@@ -2756,19 +2716,14 @@ fn test_reset_head_with_index_file_directory_conflict() {
27562716
let mut_repo = tx.repo_mut();
27572717

27582718
// Build conflict trees containing file-directory conflict
2759-
let left_tree_id = {
2760-
let mut tree_builder =
2761-
TreeBuilder::new(repo.store().clone(), repo.store().empty_tree_id().clone());
2762-
testutils::write_normal_file(&mut tree_builder, repo_path("test/dir/file"), "dir\n");
2763-
MergedTreeId::resolved(tree_builder.write_tree().unwrap())
2764-
};
2765-
2766-
let right_tree_id = {
2767-
let mut tree_builder =
2768-
TreeBuilder::new(repo.store().clone(), repo.store().empty_tree_id().clone());
2769-
testutils::write_normal_file(&mut tree_builder, repo_path("test"), "file\n");
2770-
MergedTreeId::resolved(tree_builder.write_tree().unwrap())
2771-
};
2719+
let left_tree_id = testutils::create_tree_with(&repo, |builder| {
2720+
builder.file(repo_path("test/dir/file"), "dir\n");
2721+
})
2722+
.id();
2723+
let right_tree_id = testutils::create_tree_with(&repo, |builder| {
2724+
builder.file(repo_path("test"), "file\n");
2725+
})
2726+
.id();
27722727

27732728
let left_commit = mut_repo
27742729
.new_commit(

0 commit comments

Comments
 (0)