From 79527d707ca067bca600114fb71229724aa1d8e6 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Sun, 17 Sep 2023 09:41:14 -0700 Subject: [PATCH] testutils: use `.jj`-internal git repos in most tests I don't think there's much reason to run most tests with a `.git` directory outside of `.jj`. I think it's just that way for historical reasons. It's been that way since I added support for `.jj`-internal repos in a8a9f7dedd34. The reason I want to switch is to make it a little easier to create test repos for different backends. The problem with `.jj`-external git repos is that they depend on an additional path. I had to update `test_bad_locking.rs` to make the code merging directories able handle missing directories on some side, because git's loose objects result in directories getting created on one or both sides. --- lib/tests/test_bad_locking.rs | 68 +++++++++++++++++++---------------- lib/testutils/src/lib.rs | 8 ++--- 2 files changed, 39 insertions(+), 37 deletions(-) diff --git a/lib/tests/test_bad_locking.rs b/lib/tests/test_bad_locking.rs index 460d9bbdf..94b0770b4 100644 --- a/lib/tests/test_bad_locking.rs +++ b/lib/tests/test_bad_locking.rs @@ -38,45 +38,51 @@ fn merge_directories(left: &Path, base: &Path, right: &Path, output: &Path) { std::fs::create_dir(output).unwrap(); let mut sub_dirs = vec![]; // Walk the left side and copy to the output - for entry in std::fs::read_dir(left).unwrap() { - let path = entry.unwrap().path(); - let base_name = path.file_name().unwrap(); - let child_left = left.join(base_name); - let child_output = output.join(base_name); - if child_left.is_dir() { - sub_dirs.push(base_name.to_os_string()); - } else { - std::fs::copy(&child_left, child_output).unwrap(); + if left.exists() { + for entry in std::fs::read_dir(left).unwrap() { + let path = entry.unwrap().path(); + let base_name = path.file_name().unwrap(); + let child_left = left.join(base_name); + let child_output = output.join(base_name); + if child_left.is_dir() { + sub_dirs.push(base_name.to_os_string()); + } else { + std::fs::copy(&child_left, child_output).unwrap(); + } } } // Walk the base and find files removed in the right side, then remove them in // the output - for entry in std::fs::read_dir(base).unwrap() { - let path = entry.unwrap().path(); - let base_name = path.file_name().unwrap(); - let child_base = base.join(base_name); - let child_right = right.join(base_name); - let child_output = output.join(base_name); - if child_base.is_dir() { - sub_dirs.push(base_name.to_os_string()); - } else if !child_right.exists() { - std::fs::remove_file(child_output).ok(); + if base.exists() { + for entry in std::fs::read_dir(base).unwrap() { + let path = entry.unwrap().path(); + let base_name = path.file_name().unwrap(); + let child_base = base.join(base_name); + let child_right = right.join(base_name); + let child_output = output.join(base_name); + if child_base.is_dir() { + sub_dirs.push(base_name.to_os_string()); + } else if !child_right.exists() { + std::fs::remove_file(child_output).ok(); + } } } // Walk the right side and find files added in the right side, then add them in // the output - for entry in std::fs::read_dir(right).unwrap() { - let path = entry.unwrap().path(); - let base_name = path.file_name().unwrap(); - let child_base = base.join(base_name); - let child_right = right.join(base_name); - let child_output = output.join(base_name); - if child_right.is_dir() { - sub_dirs.push(base_name.to_os_string()); - } else if !child_base.exists() { - // This overwrites the left side if that's been written. That's fine, since the - // point of the test is that it should be okay for either side to win. - std::fs::copy(&child_right, child_output).unwrap(); + if right.exists() { + for entry in std::fs::read_dir(right).unwrap() { + let path = entry.unwrap().path(); + let base_name = path.file_name().unwrap(); + let child_base = base.join(base_name); + let child_right = right.join(base_name); + let child_output = output.join(base_name); + if child_right.is_dir() { + sub_dirs.push(base_name.to_os_string()); + } else if !child_base.exists() { + // This overwrites the left side if that's been written. That's fine, since the + // point of the test is that it should be okay for either side to win. + std::fs::copy(&child_right, child_output).unwrap(); + } } } // Do the merge in subdirectories diff --git a/lib/testutils/src/lib.rs b/lib/testutils/src/lib.rs index 9d09d3e1d..205bb0065 100644 --- a/lib/testutils/src/lib.rs +++ b/lib/testutils/src/lib.rs @@ -93,13 +93,11 @@ impl TestRepo { fs::create_dir(&repo_dir).unwrap(); let repo = if use_git { - let git_path = temp_dir.path().join("git-repo"); - git2::Repository::init(&git_path).unwrap(); ReadonlyRepo::init( &settings, &repo_dir, |store_path| -> Result, BackendInitError> { - Ok(Box::new(GitBackend::init_external(store_path, &git_path)?)) + Ok(Box::new(GitBackend::init_internal(store_path)?)) }, ReadonlyRepo::default_op_store_factory(), ReadonlyRepo::default_op_heads_store_factory(), @@ -144,9 +142,7 @@ impl TestWorkspace { fs::create_dir(&workspace_root).unwrap(); let (workspace, repo) = if use_git { - let git_path = temp_dir.path().join("git-repo"); - git2::Repository::init(&git_path).unwrap(); - Workspace::init_external_git(settings, &workspace_root, &git_path).unwrap() + Workspace::init_internal_git(settings, &workspace_root).unwrap() } else { Workspace::init_local(settings, &workspace_root).unwrap() };