Skip to content

Commit

Permalink
merged_tree: use child path when merging child
Browse files Browse the repository at this point in the history
This fixes a bug where we used the parent directory's path when trying
read trees and files for a child entry. Many tests in
`test_merged_tree` fail after switching to the test backend there
without this fix/
  • Loading branch information
martinvonz committed Sep 18, 2023
1 parent 63ba2a6 commit 7ecd64f
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 16 deletions.
3 changes: 2 additions & 1 deletion lib/src/merged_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,8 @@ fn merge_trees(merge: &Merge<Tree>) -> Result<Merge<Tree>, TreeMergeError> {
let mut conflicts = vec![];
for basename in all_tree_conflict_names(merge) {
let path_merge = merge.map(|tree| tree.value(basename).cloned());
let path_merge = merge_tree_values(store, dir, path_merge)?;
let path = dir.join(basename);
let path_merge = merge_tree_values(store, &path, path_merge)?;
match path_merge.into_resolved() {
Ok(value) => {
new_tree.set_or_remove(basename, value);
Expand Down
30 changes: 15 additions & 15 deletions lib/tests/test_merged_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ fn file_value(file_id: &FileId) -> TreeValue {

#[test]
fn test_from_legacy_tree() {
let test_repo = TestRepo::init_with_backend(TestRepoBackend::Git);
let test_repo = TestRepo::init_with_backend(TestRepoBackend::Test);
let repo = &test_repo.repo;
let store = repo.store();

Expand Down Expand Up @@ -223,7 +223,7 @@ fn test_from_legacy_tree() {

#[test]
fn test_path_value_and_entries() {
let test_repo = TestRepo::init_with_backend(TestRepoBackend::Git);
let test_repo = TestRepo::init_with_backend(TestRepoBackend::Test);
let repo = &test_repo.repo;

// Create a MergedTree
Expand Down Expand Up @@ -348,7 +348,7 @@ fn test_path_value_and_entries() {

#[test]
fn test_resolve_success() {
let test_repo = TestRepo::init_with_backend(TestRepoBackend::Git);
let test_repo = TestRepo::init_with_backend(TestRepoBackend::Test);
let repo = &test_repo.repo;

let unchanged_path = RepoPath::from_internal_string("unchanged");
Expand Down Expand Up @@ -415,7 +415,7 @@ fn test_resolve_success() {

#[test]
fn test_resolve_root_becomes_empty() {
let test_repo = TestRepo::init_with_backend(TestRepoBackend::Git);
let test_repo = TestRepo::init_with_backend(TestRepoBackend::Test);
let repo = &test_repo.repo;
let store = repo.store();

Expand All @@ -432,7 +432,7 @@ fn test_resolve_root_becomes_empty() {

#[test]
fn test_resolve_with_conflict() {
let test_repo = TestRepo::init_with_backend(TestRepoBackend::Git);
let test_repo = TestRepo::init_with_backend(TestRepoBackend::Test);
let repo = &test_repo.repo;

// The trivial conflict should be resolved but the non-trivial should not (and
Expand All @@ -459,7 +459,7 @@ fn test_resolve_with_conflict() {

#[test]
fn test_conflict_iterator() {
let test_repo = TestRepo::init_with_backend(TestRepoBackend::Git);
let test_repo = TestRepo::init_with_backend(TestRepoBackend::Test);
let repo = &test_repo.repo;

let unchanged_path = RepoPath::from_internal_string("dir/subdir/unchanged");
Expand Down Expand Up @@ -575,7 +575,7 @@ fn test_conflict_iterator() {
}
#[test]
fn test_conflict_iterator_higher_arity() {
let test_repo = TestRepo::init_with_backend(TestRepoBackend::Git);
let test_repo = TestRepo::init_with_backend(TestRepoBackend::Test);
let repo = &test_repo.repo;

let two_sided_path = RepoPath::from_internal_string("dir/2-sided");
Expand Down Expand Up @@ -652,7 +652,7 @@ fn test_conflict_iterator_higher_arity() {
/// Diff two resolved trees
#[test]
fn test_diff_resolved() {
let test_repo = TestRepo::init_with_backend(TestRepoBackend::Git);
let test_repo = TestRepo::init_with_backend(TestRepoBackend::Test);
let repo = &test_repo.repo;

let clean_path = RepoPath::from_internal_string("dir1/file");
Expand Down Expand Up @@ -711,7 +711,7 @@ fn test_diff_resolved() {
/// Diff two conflicted trees
#[test]
fn test_diff_conflicted() {
let test_repo = TestRepo::init_with_backend(TestRepoBackend::Git);
let test_repo = TestRepo::init_with_backend(TestRepoBackend::Test);
let repo = &test_repo.repo;

// path1 is a clean (unchanged) conflict
Expand Down Expand Up @@ -816,7 +816,7 @@ fn test_diff_conflicted() {

#[test]
fn test_diff_dir_file() {
let test_repo = TestRepo::init_with_backend(TestRepoBackend::Git);
let test_repo = TestRepo::init_with_backend(TestRepoBackend::Test);
let repo = &test_repo.repo;

// path1: file1 -> directory1
Expand Down Expand Up @@ -1048,7 +1048,7 @@ fn test_diff_dir_file() {
/// Merge 3 resolved trees that can be resolved
#[test]
fn test_merge_simple() {
let test_repo = TestRepo::init_with_backend(TestRepoBackend::Git);
let test_repo = TestRepo::init_with_backend(TestRepoBackend::Test);
let repo = &test_repo.repo;

let path1 = RepoPath::from_internal_string("dir1/file");
Expand All @@ -1069,7 +1069,7 @@ fn test_merge_simple() {
/// Merge 3 resolved trees that can be partially resolved
#[test]
fn test_merge_partial_resolution() {
let test_repo = TestRepo::init_with_backend(TestRepoBackend::Git);
let test_repo = TestRepo::init_with_backend(TestRepoBackend::Test);
let repo = &test_repo.repo;

// path1 can be resolved, path2 cannot
Expand All @@ -1096,7 +1096,7 @@ fn test_merge_partial_resolution() {
/// Merge 3 resolved trees, including one empty legacy tree
#[test]
fn test_merge_with_empty_legacy_tree() {
let test_repo = TestRepo::init_with_backend(TestRepoBackend::Git);
let test_repo = TestRepo::init_with_backend(TestRepoBackend::Test);
let repo = &test_repo.repo;

let path1 = RepoPath::from_internal_string("dir1/file");
Expand All @@ -1121,7 +1121,7 @@ fn test_merge_with_empty_legacy_tree() {
/// at by only simplifying the conflict (no need to recurse)
#[test]
fn test_merge_simplify_only() {
let test_repo = TestRepo::init_with_backend(TestRepoBackend::Git);
let test_repo = TestRepo::init_with_backend(TestRepoBackend::Test);
let repo = &test_repo.repo;

let path = RepoPath::from_internal_string("dir1/file");
Expand Down Expand Up @@ -1154,7 +1154,7 @@ fn test_merge_simplify_only() {
/// result is a 3-way conflict.
#[test]
fn test_merge_simplify_result() {
let test_repo = TestRepo::init_with_backend(TestRepoBackend::Git);
let test_repo = TestRepo::init_with_backend(TestRepoBackend::Test);
let repo = &test_repo.repo;

// The conflict in path1 cannot be resolved, but the conflict in path2 can.
Expand Down

0 comments on commit 7ecd64f

Please sign in to comment.