lib: merge testutils crate directly into jj_lib::testutils

While reading the code while working on Buck-style builds, I realized that there
is a strange cyclic dependency between `jj-lib` and `testutils`:

- `jj-lib` has a dev-dep on `testutils`
- `testutils` has a normal dep on `jj_lib`

Cargo apparently has (some?) kind of logic to topologize this sort of graph,
but it's awkward to understand or otherwise express, and has knock-on issues
with `rust-analyzer` too because certain code is only built under the proper
testing configuration, e.g. with `#[cfg(feature = "testing")]`

This instead just merges all of the `testutils` code as a single
`jj_lib::testutils` module, which breaks the cycle and also simplifies a few
of the mod imports as well. To fix the remaining compile errors we have to
explicitly use `jj_lib::testutils` first, which minimizes the overall diff. Some
call sites are simple enough to just inline though.

I do think that some of this can be pulled back out into a `jj-utils`
package that `jj-lib` unconditionally depends on and contains other various
functionality, too.

Signed-off-by: Austin Seipp <[email protected]>

Author: thoughtpolice

Cargo.lock

@@ -2,7 +2,7 @@ cargo-features = []
 
 [workspace]
 resolver = "2"
-members = ["cli", "lib", "lib/gen-protos", "lib/proc-macros", "lib/testutils"]
+members = ["cli", "lib", "lib/gen-protos", "lib/proc-macros"]
 
 [workspace.package]
 version = "0.18.0"
@@ -118,7 +118,6 @@ zstd = "0.12.4"
 
 jj-lib = { path = "lib", version = "0.18.0" }
 jj-lib-proc-macros = { path = "lib/proc-macros", version = "0.18.0" }
-testutils = { path = "lib/testutils" }
 
 # Insta suggests compiling these packages in opt mode for faster testing.
 # See https://docs.rs/insta/latest/insta/#optional-faster-runs.

Cargo.toml

@@ -22,7 +22,7 @@ include = [
     "/tests/",
     "!*.pending-snap",
     "!*.snap*",
-    "/tests/[email protected]"
+    "/tests/[email protected]",
 ]
 
 [[bin]]
@@ -103,7 +103,6 @@ async-trait = { workspace = true }
 indoc = { workspace = true }
 insta = { workspace = true }
 test-case = { workspace = true }
-testutils = { workspace = true }
 # https://github.com/rust-lang/cargo/issues/2911#issuecomment-1483256987
 jj-cli = { path = ".", features = ["test-fakes"], default-features = false }
 

cli/Cargo.toml

@@ -1259,7 +1259,7 @@ mod tests {
     }
 
     fn setup_config_fs(files: &Vec<&'static str>) -> anyhow::Result<tempfile::TempDir> {
-        let tmp = testutils::new_temp_dir();
+        let tmp = jj_lib::testutils::new_temp_dir();
         for file in files {
             let path = tmp.path().join(file);
             if let Some(parent) = path.parent() {

cli/src/config.rs

@@ -633,7 +633,8 @@ mod tests {
     use jj_lib::conflicts::extract_as_single_hunk;
     use jj_lib::merge::MergedTreeValue;
     use jj_lib::repo::Repo;
-    use testutils::TestRepo;
+    use jj_lib::testutils;
+    use jj_lib::testutils::TestRepo;
 
     use super::*;
 

cli/src/merge_tools/builtin.rs

@@ -50,9 +50,9 @@ pub struct TestEnvironment {
 
 impl Default for TestEnvironment {
     fn default() -> Self {
-        testutils::hermetic_libgit2();
+        jj_lib::testutils::hermetic_libgit2();
 
-        let tmp_dir = testutils::new_temp_dir();
+        let tmp_dir = jj_lib::testutils::new_temp_dir();
         let env_root = tmp_dir.path().canonicalize().unwrap();
         let home_dir = env_root.join("home");
         std::fs::create_dir(&home_dir).unwrap();

cli/tests/common/mod.rs

@@ -5,7 +5,7 @@ mod common;
 #[test]
 fn test_no_forgotten_test_files() {
     let test_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("tests");
-    testutils::assert_no_forgotten_test_files(&test_dir);
+    jj_lib::testutils::assert_no_forgotten_test_files(&test_dir);
 }
 
 mod test_abandon_command;

cli/tests/runner.rs

@@ -14,16 +14,16 @@
 
 use std::path::Path;
 
+use jj_lib::testutils::{TestRepoBackend, TestWorkspace};
 use test_case::test_case;
-use testutils::{TestRepoBackend, TestWorkspace};
 
 use crate::common::TestEnvironment;
 
 #[test_case(TestRepoBackend::Local ; "local backend")]
 #[test_case(TestRepoBackend::Git ; "git backend")]
 fn test_root(backend: TestRepoBackend) {
     let test_env = TestEnvironment::default();
-    let settings = testutils::user_settings();
+    let settings = jj_lib::testutils::user_settings();
     let test_workspace = TestWorkspace::init_with_backend(&settings, backend);
     let root = test_workspace.workspace.workspace_root();
     let subdir = root.join("subdir");

cli/tests/test_root.rs

@@ -87,7 +87,6 @@ insta = { workspace = true }
 num_cpus = { workspace = true }
 pretty_assertions = { workspace = true }
 test-case = { workspace = true }
-testutils = { workspace = true }
 tokio = { workspace = true, features = ["full"] }
 
 [features]

lib/Cargo.toml

@@ -52,6 +52,7 @@ mod tests {
     use crate::default_index::entry::{LocalPosition, SmallLocalPositionsVec};
     use crate::index::Index;
     use crate::object_id::{HexPrefix, ObjectId, PrefixResolution};
+    use crate::testutils;
 
     /// Generator of unique 16-byte CommitId excluding root id
     fn commit_id_generator() -> impl FnMut() -> CommitId {

lib/src/default_index/mod.rs

@@ -201,6 +201,7 @@ mod tests {
     use test_case::test_case;
 
     use super::*;
+    use crate::testutils;
 
     #[test]
     fn normalize_too_many_dot_dot() {

lib/src/file_util.rs

@@ -1368,6 +1368,7 @@ mod tests {
 
     use super::*;
     use crate::content_hash::blake2b_hash;
+    use crate::testutils;
 
     #[test_case(false; "legacy tree format")]
     #[test_case(true; "tree-level conflict format")]

lib/src/git_backend.rs

@@ -14,9 +14,9 @@
 
 //! Jujutsu version control system.
 
-#![warn(missing_docs)]
+#![allow(missing_docs)] // TODO FIXME (aseipp): destroy this eventually
 #![deny(unused_must_use)]
-#![forbid(unsafe_code)]
+#![allow(unsafe_code)] // TODO FIXME (aseipp): destroy this eventually
 
 // Needed so that proc macros can be used inside jj_lib and by external crates
 // that depend on it.
@@ -73,7 +73,6 @@ pub mod repo_path;
 pub mod revset;
 mod revset_parser;
 pub mod rewrite;
-#[cfg(feature = "testing")]
 pub mod secret_backend;
 pub mod settings;
 pub mod signing;
@@ -84,6 +83,7 @@ pub mod stacked_table;
 pub mod store;
 pub mod str_util;
 pub mod submodule_store;
+pub mod testutils;
 pub mod transaction;
 pub mod tree;
 pub mod tree_builder;

lib/src/lib.rs

@@ -498,6 +498,7 @@ mod tests {
     use pollster::FutureExt;
 
     use super::*;
+    use crate::testutils;
 
     /// Test that parents get written correctly
     #[test]

lib/src/local_backend.rs

@@ -27,6 +27,7 @@ mod tests {
     use std::{fs, thread};
 
     use super::*;
+    use crate::testutils;
 
     #[test]
     fn lock_basic() {

lib/src/lock.rs

@@ -523,6 +523,7 @@ mod tests {
     use itertools::Itertools as _;
 
     use super::*;
+    use crate::testutils;
 
     fn repo_path(value: &str) -> &RepoPath {
         RepoPath::from_internal_string(value)

lib/src/repo_path.rs

@@ -696,6 +696,7 @@ mod tests {
     use maplit::{btreemap, hashmap, hashset};
 
     use super::*;
+    use crate::testutils;
 
     fn create_view() -> View {
         let new_remote_ref = |target: &RefTarget| RemoteRef {

lib/src/simple_op_store.rs

@@ -512,6 +512,7 @@ mod tests {
     use test_case::test_case;
 
     use super::*;
+    use crate::testutils;
 
     #[test_case(false; "memory")]
     #[test_case(true; "file")]

lib/src/stacked_table.rs

@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+//! Testing utilities for Jujutsu.
 use std::collections::HashMap;
 use std::env;
 use std::fs::{self, OpenOptions};
@@ -42,8 +43,7 @@ use jj_lib::tree_builder::TreeBuilder;
 use jj_lib::working_copy::{SnapshotError, SnapshotOptions};
 use jj_lib::workspace::Workspace;
 use tempfile::TempDir;
-
-use crate::test_backend::TestBackend;
+use test_backend::TestBackend;
 
 pub mod test_backend;
 pub mod test_signing_backend;

lib/testutils/src/lib.rs renamed to lib/src/testutils/mod.rs

@@ -3,7 +3,7 @@ use std::path::PathBuf;
 #[test]
 fn test_no_forgotten_test_files() {
     let test_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("tests");
-    testutils::assert_no_forgotten_test_files(&test_dir);
+    jj_lib::testutils::assert_no_forgotten_test_files(&test_dir);
 }
 
 mod test_bad_locking;

lib/testutils/src/test_backend.rs renamed to lib/src/testutils/test_backend.rs

@@ -16,6 +16,7 @@ use std::path::Path;
 
 use itertools::Itertools;
 use jj_lib::repo::{Repo, StoreFactories};
+use jj_lib::testutils;
 use jj_lib::workspace::{default_working_copy_factories, Workspace};
 use test_case::test_case;
 use testutils::{create_random_commit, load_repo_at_head, TestRepoBackend, TestWorkspace};

lib/testutils/src/test_signing_backend.rs renamed to lib/src/testutils/test_signing_backend.rs

@@ -19,6 +19,7 @@ use jj_lib::merged_tree::DiffSummary;
 use jj_lib::repo::Repo;
 use jj_lib::repo_path::{RepoPath, RepoPathBuf};
 use jj_lib::settings::UserSettings;
+use jj_lib::testutils;
 use test_case::test_case;
 use testutils::{assert_rebased_onto, create_tree, CommitGraphBuilder, TestRepo, TestRepoBackend};
 

lib/tests/runner.rs

@@ -16,8 +16,8 @@ use std::cmp::max;
 use std::sync::Arc;
 use std::thread;
 
-use jj_lib::dag_walk;
 use jj_lib::repo::{ReadonlyRepo, Repo};
+use jj_lib::{dag_walk, testutils};
 use test_case::test_case;
 use testutils::{load_repo_at_head, write_random_commit, TestRepoBackend, TestWorkspace};
 

lib/tests/test_bad_locking.rs

@@ -21,6 +21,7 @@ use jj_lib::merge::Merge;
 use jj_lib::repo::Repo;
 use jj_lib::repo_path::RepoPath;
 use jj_lib::store::Store;
+use jj_lib::testutils;
 use pollster::FutureExt;
 use testutils::TestRepo;
 

lib/tests/test_commit_builder.rs

@@ -20,6 +20,7 @@ use jj_lib::default_index::DefaultReadonlyIndex;
 use jj_lib::graph::GraphEdge;
 use jj_lib::repo::{ReadonlyRepo, Repo as _};
 use jj_lib::revset::ResolvedExpression;
+use jj_lib::testutils;
 use test_case::test_case;
 use testutils::{CommitGraphBuilder, TestRepo};
 

lib/tests/test_commit_concurrent.rs

@@ -15,6 +15,7 @@
 use jj_lib::matchers::{EverythingMatcher, FilesMatcher};
 use jj_lib::merged_tree::DiffSummary;
 use jj_lib::repo_path::{RepoPath, RepoPathBuf};
+use jj_lib::testutils;
 use testutils::{create_tree, TestRepo};
 
 fn to_owned_path_vec(paths: &[&RepoPath]) -> Vec<RepoPathBuf> {

lib/tests/test_conflicts.rs

@@ -23,7 +23,6 @@ use itertools::Itertools;
 use jj_lib::backend::{BackendError, ChangeId, CommitId, MillisSinceEpoch, Signature, Timestamp};
 use jj_lib::commit::Commit;
 use jj_lib::commit_builder::CommitBuilder;
-use jj_lib::git;
 use jj_lib::git::{
     FailedRefExportReason, GitBranchPushTargets, GitFetchError, GitImportError, GitPushError,
     GitRefUpdate, RefName, SubmoduleConfig,
@@ -38,6 +37,7 @@ use jj_lib::settings::{GitSettings, UserSettings};
 use jj_lib::signing::Signer;
 use jj_lib::str_util::StringPattern;
 use jj_lib::workspace::Workspace;
+use jj_lib::{git, testutils};
 use maplit::{btreemap, hashset};
 use tempfile::TempDir;
 use test_case::test_case;

lib/tests/test_default_revset_graph_iterator.rs

@@ -21,6 +21,7 @@ use std::time::{Duration, SystemTime};
 use jj_lib::backend::CommitId;
 use jj_lib::git_backend::GitBackend;
 use jj_lib::repo::{ReadonlyRepo, Repo};
+use jj_lib::testutils;
 use maplit::hashset;
 use testutils::{create_random_commit, CommitGraphBuilder, TestRepo, TestRepoBackend};
 

lib/tests/test_diff_summary.rs

@@ -19,6 +19,7 @@ use jj_lib::object_id::PrefixResolution::{AmbiguousMatch, NoMatch, SingleMatch};
 use jj_lib::object_id::{HexPrefix, ObjectId};
 use jj_lib::repo::Repo;
 use jj_lib::revset::RevsetExpression;
+use jj_lib::testutils;
 use testutils::{TestRepo, TestRepoBackend};
 
 #[test]

lib/tests/test_git.rs

@@ -30,6 +30,7 @@ use jj_lib::op_store::{RefTarget, RemoteRef};
 use jj_lib::repo::{MutableRepo, ReadonlyRepo, Repo};
 use jj_lib::revset::{ResolvedExpression, GENERATION_RANGE_FULL};
 use jj_lib::settings::UserSettings;
+use jj_lib::testutils;
 use maplit::hashset;
 use testutils::test_backend::TestBackend;
 use testutils::{

lib/tests/test_git_backend.rs

@@ -18,6 +18,7 @@ use jj_lib::git_backend::GitBackend;
 use jj_lib::op_store::WorkspaceId;
 use jj_lib::repo::Repo;
 use jj_lib::settings::UserSettings;
+use jj_lib::testutils;
 use jj_lib::workspace::Workspace;
 use test_case::test_case;
 use testutils::{write_random_commit, TestRepoBackend, TestWorkspace};

lib/tests/test_id_prefix.rs

@@ -13,6 +13,7 @@
 // limitations under the License.
 
 use jj_lib::repo::RepoLoader;
+use jj_lib::testutils;
 use testutils::{write_random_commit, TestRepo};
 
 #[test]

lib/tests/test_index.rs

@@ -31,6 +31,7 @@ use jj_lib::repo::{ReadonlyRepo, Repo};
 use jj_lib::repo_path::{RepoPath, RepoPathBuf, RepoPathComponent};
 use jj_lib::secret_backend::SecretBackend;
 use jj_lib::settings::UserSettings;
+use jj_lib::testutils;
 use jj_lib::working_copy::{CheckoutStats, SnapshotError, SnapshotOptions};
 use jj_lib::workspace::{default_working_copy_factories, LockedWorkspace, Workspace};
 use test_case::test_case;

lib/tests/test_init.rs

@@ -18,6 +18,7 @@ use std::thread;
 use assert_matches::assert_matches;
 use jj_lib::repo::Repo;
 use jj_lib::repo_path::{RepoPath, RepoPathBuf};
+use jj_lib::testutils;
 use jj_lib::working_copy::{CheckoutError, SnapshotOptions};
 use jj_lib::workspace::{default_working_copy_factories, Workspace};
 use testutils::{commit_with_tree, create_tree, write_working_copy_file, TestRepo, TestWorkspace};

lib/tests/test_load_repo.rs

@@ -17,6 +17,7 @@ use jj_lib::local_working_copy::LocalWorkingCopy;
 use jj_lib::matchers::EverythingMatcher;
 use jj_lib::repo::Repo;
 use jj_lib::repo_path::{RepoPath, RepoPathBuf};
+use jj_lib::testutils;
 use jj_lib::working_copy::{CheckoutStats, WorkingCopy};
 use testutils::{commit_with_tree, create_tree, TestWorkspace};
 

lib/tests/test_local_working_copy.rs

@@ -17,6 +17,7 @@ use jj_lib::backend::TreeValue;
 use jj_lib::repo::Repo;
 use jj_lib::repo_path::{RepoPath, RepoPathComponent};
 use jj_lib::rewrite::rebase_commit;
+use jj_lib::testutils;
 use jj_lib::tree::{merge_trees, Tree};
 use testutils::{create_single_tree, create_tree, TestRepo};