Skip to content

Commit

Permalink
testutils: move global TestBackendData mapping to TestEnvironment
Browse files Browse the repository at this point in the history
This unblocks the use of TestBackend in long-running processes such as fuzzer.
It should also be safer because TempDir doesn't guarantee that the path is never
reused.
  • Loading branch information
yuja committed Nov 1, 2024
1 parent 7b5df93 commit 9f1d2ab
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 41 deletions.
24 changes: 14 additions & 10 deletions lib/testutils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ use jj_lib::workspace::Workspace;
use pollster::FutureExt;
use tempfile::TempDir;

use crate::test_backend::TestBackend;
use crate::test_backend::TestBackendFactory;

pub mod test_backend;
pub mod test_signing_backend;
Expand Down Expand Up @@ -122,12 +122,15 @@ pub fn user_settings() -> UserSettings {
#[derive(Debug)]
pub struct TestEnvironment {
temp_dir: TempDir,
test_backend_factory: TestBackendFactory,
}

impl TestEnvironment {
pub fn init() -> Self {
let temp_dir = new_temp_dir();
TestEnvironment { temp_dir }
TestEnvironment {
temp_dir: new_temp_dir(),
test_backend_factory: TestBackendFactory::default(),
}
}

pub fn root(&self) -> &Path {
Expand All @@ -136,10 +139,10 @@ impl TestEnvironment {

pub fn default_store_factories(&self) -> StoreFactories {
let mut factories = StoreFactories::default();
factories.add_backend(
"test",
Box::new(|_settings, store_path| Ok(Box::new(TestBackend::load(store_path)))),
);
factories.add_backend("test", {
let factory = self.test_backend_factory.clone();
Box::new(move |_settings, store_path| Ok(Box::new(factory.load(store_path))))
});
factories.add_backend(
SecretBackend::name(),
Box::new(|settings, store_path| {
Expand Down Expand Up @@ -177,13 +180,14 @@ pub enum TestRepoBackend {
impl TestRepoBackend {
fn init_backend(
&self,
env: &TestEnvironment,
settings: &UserSettings,
store_path: &Path,
) -> Result<Box<dyn Backend>, BackendInitError> {
match self {
TestRepoBackend::Git => Ok(Box::new(GitBackend::init_internal(settings, store_path)?)),
TestRepoBackend::Local => Ok(Box::new(LocalBackend::init(store_path))),
TestRepoBackend::Test => Ok(Box::new(TestBackend::init(store_path))),
TestRepoBackend::Test => Ok(Box::new(env.test_backend_factory.init(store_path))),
}
}
}
Expand Down Expand Up @@ -213,7 +217,7 @@ impl TestRepo {
let repo = ReadonlyRepo::init(
settings,
&repo_dir,
&move |settings, store_path| backend.init_backend(settings, store_path),
&|settings, store_path| backend.init_backend(&env, settings, store_path),
Signer::from_settings(settings).unwrap(),
ReadonlyRepo::default_op_store_initializer(),
ReadonlyRepo::default_op_heads_store_initializer(),
Expand Down Expand Up @@ -266,7 +270,7 @@ impl TestWorkspace {
let (workspace, repo) = Workspace::init_with_backend(
settings,
&workspace_root,
&move |settings, store_path| backend.init_backend(settings, store_path),
&|settings, store_path| backend.init_backend(&env, settings, store_path),
signer,
)
.unwrap();
Expand Down
66 changes: 35 additions & 31 deletions lib/testutils/src/test_backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ use std::path::PathBuf;
use std::sync::Arc;
use std::sync::Mutex;
use std::sync::MutexGuard;
use std::sync::OnceLock;
use std::time::SystemTime;

use async_trait::async_trait;
Expand Down Expand Up @@ -54,16 +53,11 @@ use jj_lib::repo_path::RepoPathBuf;
const HASH_LENGTH: usize = 10;
const CHANGE_ID_LENGTH: usize = 16;

static BACKEND_DATA: OnceLock<Mutex<HashMap<PathBuf, Arc<Mutex<TestBackendData>>>>> =
OnceLock::new();

// Keyed by canonical store path. Since we just use the path as a key, we can't
// rely on on the file system to resolve two different uncanonicalized paths to
// the same real path (as we would if we just used the path with `std::fs`
// functions).
fn backend_data() -> &'static Mutex<HashMap<PathBuf, Arc<Mutex<TestBackendData>>>> {
BACKEND_DATA.get_or_init(|| Mutex::new(HashMap::new()))
}
type TestBackendDataMap = HashMap<PathBuf, Arc<Mutex<TestBackendData>>>;

#[derive(Default)]
pub struct TestBackendData {
Expand All @@ -74,6 +68,39 @@ pub struct TestBackendData {
conflicts: HashMap<RepoPathBuf, HashMap<ConflictId, Conflict>>,
}

#[derive(Clone, Default)]
pub struct TestBackendFactory {
backend_data: Arc<Mutex<TestBackendDataMap>>,
}

impl TestBackendFactory {
pub fn init(&self, store_path: &Path) -> TestBackend {
let data = Arc::new(Mutex::new(TestBackendData::default()));
self.backend_data
.lock()
.unwrap()
.insert(store_path.canonicalize().unwrap(), data.clone());
TestBackend::with_data(data)
}

pub fn load(&self, store_path: &Path) -> TestBackend {
let data = self
.backend_data
.lock()
.unwrap()
.get(&store_path.canonicalize().unwrap())
.unwrap()
.clone();
TestBackend::with_data(data)
}
}

impl Debug for TestBackendFactory {
fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), Error> {
f.debug_struct("TestBackendFactory").finish_non_exhaustive()
}
}

fn get_hash(content: &(impl jj_lib::content_hash::ContentHash + ?Sized)) -> Vec<u8> {
jj_lib::content_hash::blake2b_hash(content).as_slice()[..HASH_LENGTH].to_vec()
}
Expand All @@ -94,30 +121,7 @@ pub struct TestBackend {
}

impl TestBackend {
pub fn init(store_path: &Path) -> Self {
let root_commit_id = CommitId::from_bytes(&[0; HASH_LENGTH]);
let root_change_id = ChangeId::from_bytes(&[0; CHANGE_ID_LENGTH]);
let empty_tree_id = TreeId::new(get_hash(&Tree::default()));
let data = Arc::new(Mutex::new(TestBackendData::default()));
backend_data()
.lock()
.unwrap()
.insert(store_path.canonicalize().unwrap(), data.clone());
TestBackend {
root_commit_id,
root_change_id,
empty_tree_id,
data,
}
}

pub fn load(store_path: &Path) -> Self {
let data = backend_data()
.lock()
.unwrap()
.get(&store_path.canonicalize().unwrap())
.unwrap()
.clone();
pub fn with_data(data: Arc<Mutex<TestBackendData>>) -> Self {
let root_commit_id = CommitId::from_bytes(&[0; HASH_LENGTH]);
let root_change_id = ChangeId::from_bytes(&[0; CHANGE_ID_LENGTH]);
let empty_tree_id = TreeId::new(get_hash(&Tree::default()));
Expand Down

0 comments on commit 9f1d2ab

Please sign in to comment.