tests: use test backend in working copy tests, fix MergedTree bug

Only tests dealing with Git submodules care about the backend type.

Switching the tests to use the test backend also uncovered another bug
in `MergedTree`, so I fixed that too. The bug only happens with legacy
trees (path-level conflicts) and backends that care about the conflict
path, so it wouldn't happen with Git backends, and it wouldn't happen
at Google either (because we use tree-level conflicts).

lib/src/merged_tree.rs

@@ -180,7 +180,8 @@ impl MergedTree {
         match self {
             MergedTree::Legacy(tree) => match tree.value(basename) {
                 Some(TreeValue::Conflict(conflict_id)) => {
-                    let conflict = tree.store().read_conflict(tree.dir(), conflict_id).unwrap();
+                    let path = tree.dir().join(basename);
+                    let conflict = tree.store().read_conflict(&path, conflict_id).unwrap();
                     MergedTreeValue::Conflict(conflict)
                 }
                 other => MergedTreeValue::Resolved(other),

lib/tests/test_working_copy.rs

@@ -37,12 +37,11 @@ use jj_lib::working_copy::{LockedWorkingCopy, SnapshotError, SnapshotOptions, Wo
 use test_case::test_case;
 use testutils::{create_tree, write_random_commit, TestRepoBackend, TestWorkspace};
 
-#[test_case(TestRepoBackend::Local ; "local backend")]
-#[test_case(TestRepoBackend::Git ; "git backend")]
-fn test_root(backend: TestRepoBackend) {
+#[test]
+fn test_root() {
     // Test that the working copy is clean and empty after init.
     let settings = testutils::user_settings();
-    let mut test_workspace = TestWorkspace::init_with_backend(&settings, backend);
+    let mut test_workspace = TestWorkspace::init(&settings);
 
     let wc = test_workspace.workspace.working_copy();
     assert_eq!(wc.sparse_patterns().unwrap(), vec![RepoPath::root()]);
@@ -264,7 +263,7 @@ fn test_checkout_file_transitions(backend: TestRepoBackend) {
 #[test]
 fn test_conflict_subdirectory() {
     let settings = testutils::user_settings();
-    let mut test_workspace = TestWorkspace::init_with_backend(&settings, TestRepoBackend::Git);
+    let mut test_workspace = TestWorkspace::init(&settings);
     let repo = &test_workspace.repo;
 
     let path = RepoPath::from_internal_string("sub/file");
@@ -279,11 +278,10 @@ fn test_conflict_subdirectory() {
         .unwrap();
 }
 
-#[test_case(TestRepoBackend::Local ; "local backend")]
-#[test_case(TestRepoBackend::Git ; "git backend")]
-fn test_tree_builder_file_directory_transition(backend: TestRepoBackend) {
+#[test]
+fn test_tree_builder_file_directory_transition() {
     let settings = testutils::user_settings();
-    let test_workspace = TestWorkspace::init_with_backend(&settings, backend);
+    let test_workspace = TestWorkspace::init(&settings);
     let repo = &test_workspace.repo;
     let store = repo.store();
     let mut workspace = test_workspace.workspace;
@@ -419,13 +417,12 @@ fn test_checkout_discard() {
     assert!(!reloaded_wc.file_states().unwrap().contains_key(&file2_path));
 }
 
-#[test_case(TestRepoBackend::Local ; "local backend")]
-#[test_case(TestRepoBackend::Git ; "git backend")]
-fn test_snapshot_racy_timestamps(backend: TestRepoBackend) {
+#[test]
+fn test_snapshot_racy_timestamps() {
     // Tests that file modifications are detected even if they happen the same
     // millisecond as the updated working copy state.
     let settings = testutils::user_settings();
-    let mut test_workspace = TestWorkspace::init_with_backend(&settings, backend);
+    let mut test_workspace = TestWorkspace::init(&settings);
     let repo = &test_workspace.repo;
     let workspace_root = test_workspace.workspace.workspace_root().clone();
 
@@ -508,13 +505,12 @@ fn test_snapshot_special_file() {
     );
 }
 
-#[test_case(TestRepoBackend::Local ; "local backend")]
-#[test_case(TestRepoBackend::Git ; "git backend")]
-fn test_gitignores(backend: TestRepoBackend) {
+#[test]
+fn test_gitignores() {
     // Tests that .gitignore files are respected.
 
     let settings = testutils::user_settings();
-    let mut test_workspace = TestWorkspace::init_with_backend(&settings, backend);
+    let mut test_workspace = TestWorkspace::init(&settings);
     let workspace_root = test_workspace.workspace.workspace_root().clone();
 
     let gitignore_path = RepoPath::from_internal_string(".gitignore");
@@ -568,14 +564,13 @@ fn test_gitignores(backend: TestRepoBackend) {
     );
 }
 
-#[test_case(TestRepoBackend::Local ; "local backend")]
-#[test_case(TestRepoBackend::Git ; "git backend")]
-fn test_gitignores_in_ignored_dir(backend: TestRepoBackend) {
+#[test]
+fn test_gitignores_in_ignored_dir() {
     // Tests that .gitignore files in an ignored directory are ignored, i.e. that
     // they cannot override the ignores from the parent
 
     let settings = testutils::user_settings();
-    let mut test_workspace = TestWorkspace::init_with_backend(&settings, backend);
+    let mut test_workspace = TestWorkspace::init(&settings);
     let op_id = test_workspace.repo.op_id().clone();
     let workspace_root = test_workspace.workspace.workspace_root().clone();
 
@@ -616,14 +611,13 @@ fn test_gitignores_in_ignored_dir(backend: TestRepoBackend) {
     );
 }
 
-#[test_case(TestRepoBackend::Local ; "local backend")]
-#[test_case(TestRepoBackend::Git ; "git backend")]
-fn test_gitignores_checkout_never_overwrites_ignored(backend: TestRepoBackend) {
+#[test]
+fn test_gitignores_checkout_never_overwrites_ignored() {
     // Tests that a .gitignore'd file doesn't get overwritten if check out a commit
     // where the file is tracked.
 
     let settings = testutils::user_settings();
-    let mut test_workspace = TestWorkspace::init_with_backend(&settings, backend);
+    let mut test_workspace = TestWorkspace::init(&settings);
     let repo = &test_workspace.repo;
     let workspace_root = test_workspace.workspace.workspace_root().clone();
 
@@ -648,14 +642,13 @@ fn test_gitignores_checkout_never_overwrites_ignored(backend: TestRepoBackend) {
     assert_eq!(std::fs::read(&path).unwrap(), b"garbage");
 }
 
-#[test_case(TestRepoBackend::Local ; "local backend")]
-#[test_case(TestRepoBackend::Git ; "git backend")]
-fn test_gitignores_ignored_directory_already_tracked(backend: TestRepoBackend) {
+#[test]
+fn test_gitignores_ignored_directory_already_tracked() {
     // Tests that a .gitignore'd directory that already has a tracked file in it
     // does not get removed when snapshotting the working directory.
 
     let settings = testutils::user_settings();
-    let mut test_workspace = TestWorkspace::init_with_backend(&settings, backend);
+    let mut test_workspace = TestWorkspace::init(&settings);
     let workspace_root = test_workspace.workspace.workspace_root().clone();
     let repo = test_workspace.repo.clone();
 
@@ -697,14 +690,13 @@ fn test_gitignores_ignored_directory_already_tracked(backend: TestRepoBackend) {
     );
 }
 
-#[test_case(TestRepoBackend::Local ; "local backend")]
-#[test_case(TestRepoBackend::Git ; "git backend")]
-fn test_dotgit_ignored(backend: TestRepoBackend) {
+#[test]
+fn test_dotgit_ignored() {
     // Tests that .git directories and files are always ignored (we could accept
     // them if the backend is not git).
 
     let settings = testutils::user_settings();
-    let mut test_workspace = TestWorkspace::init_with_backend(&settings, backend);
+    let mut test_workspace = TestWorkspace::init(&settings);
     let store = test_workspace.repo.store().clone();
     let workspace_root = test_workspace.workspace.workspace_root().clone();
 
@@ -792,11 +784,10 @@ fn test_gitsubmodule() {
 }
 
 #[cfg(unix)]
-#[test_case(TestRepoBackend::Local ; "local backend")]
-#[test_case(TestRepoBackend::Git ; "git backend")]
-fn test_existing_directory_symlink(backend: TestRepoBackend) {
+#[test]
+fn test_existing_directory_symlink() {
     let settings = testutils::user_settings();
-    let mut test_workspace = TestWorkspace::init_with_backend(&settings, backend);
+    let mut test_workspace = TestWorkspace::init(&settings);
     let repo = &test_workspace.repo;
     let workspace_root = test_workspace.workspace.workspace_root().clone();
 
@@ -817,7 +808,7 @@ fn test_existing_directory_symlink(backend: TestRepoBackend) {
 #[test]
 fn test_fsmonitor() {
     let settings = testutils::user_settings();
-    let mut test_workspace = TestWorkspace::init_with_backend(&settings, TestRepoBackend::Git);
+    let mut test_workspace = TestWorkspace::init(&settings);
     let repo = &test_workspace.repo;
     let workspace_root = test_workspace.workspace.workspace_root().clone();
 
@@ -861,8 +852,8 @@ fn test_fsmonitor() {
         let mut locked_wc = wc.start_mutation().unwrap();
         let tree_id = snapshot(&mut locked_wc, &[&foo_path]);
         insta::assert_snapshot!(testutils::dump_tree(repo.store(), &tree_id), @r###"
-        tree 205f6b799e7d5c2524468ca006a0131aa57ecce7
-          file "foo" (257cc5642cb1a054f08cc83f2d943e56fd3ebe99): "foo\n"
+        tree d5e38c0a1b0ee5de47c5
+          file "foo" (e99c2057c15160add351): "foo\n"
         "###);
     }
 
@@ -873,10 +864,10 @@ fn test_fsmonitor() {
             &[&foo_path, &bar_path, &nested_path, &ignored_path],
         );
         insta::assert_snapshot!(testutils::dump_tree(repo.store(), &tree_id), @r###"
-        tree ab5a0465cc71725a723f28b685844a5bc0f5b599
-          file "bar" (5716ca5987cbf97d6bb54920bea6adde242d87e6): "bar\n"
-          file "foo" (257cc5642cb1a054f08cc83f2d943e56fd3ebe99): "foo\n"
-          file "path/to/nested" (79c53955ef856f16f2107446bc721c8879a1bd2e): "nested\n"
+        tree f408c8d080414f8e90e1
+          file "bar" (94cc973e7e1aefb7eff6): "bar\n"
+          file "foo" (e99c2057c15160add351): "foo\n"
+          file "path/to/nested" (6209060941cd770c8d46): "nested\n"
         "###);
         locked_wc.finish(repo.op_id().clone()).unwrap();
     }
@@ -887,10 +878,10 @@ fn test_fsmonitor() {
         let mut locked_wc = wc.start_mutation().unwrap();
         let tree_id = snapshot(&mut locked_wc, &[&foo_path]);
         insta::assert_snapshot!(testutils::dump_tree(repo.store(), &tree_id), @r###"
-        tree 2f57ab8f48ae62e3137079f2add9878dfa1d1bcc
-          file "bar" (5716ca5987cbf97d6bb54920bea6adde242d87e6): "bar\n"
-          file "foo" (9d053d7c8a18a286dce9b99a59bb058be173b463): "updated foo\n"
-          file "path/to/nested" (79c53955ef856f16f2107446bc721c8879a1bd2e): "nested\n"
+        tree e994a93c46f41dc91704
+          file "bar" (94cc973e7e1aefb7eff6): "bar\n"
+          file "foo" (e0fbd106147cc04ccd05): "updated foo\n"
+          file "path/to/nested" (6209060941cd770c8d46): "nested\n"
         "###);
     }
 
@@ -899,9 +890,9 @@ fn test_fsmonitor() {
         let mut locked_wc = wc.start_mutation().unwrap();
         let tree_id = snapshot(&mut locked_wc, &[&foo_path]);
         insta::assert_snapshot!(testutils::dump_tree(repo.store(), &tree_id), @r###"
-        tree 34b83765131477e1a7d72160079daec12c6144e3
-          file "bar" (5716ca5987cbf97d6bb54920bea6adde242d87e6): "bar\n"
-          file "path/to/nested" (79c53955ef856f16f2107446bc721c8879a1bd2e): "nested\n"
+        tree 1df764981d4d74a4ecfa
+          file "bar" (94cc973e7e1aefb7eff6): "bar\n"
+          file "path/to/nested" (6209060941cd770c8d46): "nested\n"
         "###);
         locked_wc.finish(repo.op_id().clone()).unwrap();
     }
@@ -918,7 +909,7 @@ fn test_snapshot_max_new_file_size() {
             .build()
             .unwrap(),
     );
-    let mut test_workspace = TestWorkspace::init_with_backend(&settings, TestRepoBackend::Git);
+    let mut test_workspace = TestWorkspace::init(&settings);
     let workspace_root = test_workspace.workspace.workspace_root().clone();
     let small_path = RepoPath::from_internal_string("small");
     let large_path = RepoPath::from_internal_string("large");

lib/tests/test_working_copy_concurrent.rs

@@ -16,18 +16,18 @@ use std::cmp::max;
 use std::thread;
 
 use assert_matches::assert_matches;
-use jj_lib::repo::{Repo, StoreFactories};
+use jj_lib::repo::Repo;
 use jj_lib::repo_path::RepoPath;
 use jj_lib::working_copy::{CheckoutError, SnapshotOptions};
 use jj_lib::workspace::Workspace;
-use testutils::{create_tree, write_working_copy_file, TestRepoBackend, TestWorkspace};
+use testutils::{create_tree, write_working_copy_file, TestRepo, TestWorkspace};
 
 #[test]
 fn test_concurrent_checkout() {
     // Test that we error out if a concurrent checkout is detected (i.e. if the
     // working-copy commit changed on disk after we read it).
     let settings = testutils::user_settings();
-    let mut test_workspace1 = TestWorkspace::init_with_backend(&settings, TestRepoBackend::Git);
+    let mut test_workspace1 = TestWorkspace::init(&settings);
     let repo1 = test_workspace1.repo.clone();
     let workspace1_root = test_workspace1.workspace.workspace_root().clone();
 
@@ -45,8 +45,12 @@ fn test_concurrent_checkout() {
 
     // Check out tree2 from another process (simulated by another workspace
     // instance)
-    let mut workspace2 =
-        Workspace::load(&settings, &workspace1_root, &StoreFactories::default()).unwrap();
+    let mut workspace2 = Workspace::load(
+        &settings,
+        &workspace1_root,
+        &TestRepo::default_store_factories(),
+    )
+    .unwrap();
     workspace2
         .working_copy_mut()
         .check_out(repo1.op_id().clone(), Some(&tree_id1), &tree2)
@@ -59,8 +63,12 @@ fn test_concurrent_checkout() {
     );
 
     // Check that the tree2 is still checked out on disk.
-    let workspace3 =
-        Workspace::load(&settings, &workspace1_root, &StoreFactories::default()).unwrap();
+    let workspace3 = Workspace::load(
+        &settings,
+        &workspace1_root,
+        &TestRepo::default_store_factories(),
+    )
+    .unwrap();
     assert_eq!(
         *workspace3.working_copy().current_tree_id().unwrap(),
         tree_id2
@@ -72,7 +80,7 @@ fn test_checkout_parallel() {
     // Test that concurrent checkouts by different processes (simulated by using
     // different repo instances) is safe.
     let settings = testutils::user_settings();
-    let mut test_workspace = TestWorkspace::init_with_backend(&settings, TestRepoBackend::Git);
+    let mut test_workspace = TestWorkspace::init(&settings);
     let repo = &test_workspace.repo;
     let workspace_root = test_workspace.workspace.workspace_root().clone();
 
@@ -104,9 +112,12 @@ fn test_checkout_parallel() {
             let settings = settings.clone();
             let workspace_root = workspace_root.clone();
             s.spawn(move || {
-                let mut workspace =
-                    Workspace::load(&settings, &workspace_root, &StoreFactories::default())
-                        .unwrap();
+                let mut workspace = Workspace::load(
+                    &settings,
+                    &workspace_root,
+                    &TestRepo::default_store_factories(),
+                )
+                .unwrap();
                 let tree = workspace
                     .repo_loader()
                     .store()
@@ -137,7 +148,7 @@ fn test_checkout_parallel() {
 #[test]
 fn test_racy_checkout() {
     let settings = testutils::user_settings();
-    let mut test_workspace = TestWorkspace::init_with_backend(&settings, TestRepoBackend::Git);
+    let mut test_workspace = TestWorkspace::init(&settings);
     let repo = &test_workspace.repo;
     let op_id = repo.op_id().clone();
     let workspace_root = test_workspace.workspace.workspace_root().clone();

lib/testutils/src/lib.rs

@@ -135,6 +135,15 @@ impl TestRepo {
             repo,
         }
     }
+
+    pub fn default_store_factories() -> StoreFactories {
+        let mut factories = StoreFactories::default();
+        factories.add_backend(
+            "test",
+            Box::new(|store_path| Ok(Box::new(TestBackend::load(store_path)))),
+        );
+        factories
+    }
 }
 
 pub struct TestWorkspace {