Change: StoreBuilder does not need to run a test, it only needs to bu…

…ild a store

`StoreBuilder::run_test()` is removed, and a new method `build()` is
added. To test a `RaftStorage` implementation, just implementing
`StoreBuilder::build()` is enough now. It returns a store instance and a
**guard**, for disk backed store to clean up the data when the guard is
dropped.

memstore/src/test.rs

@@ -1,4 +1,3 @@
-use std::future::Future;
 use std::sync::Arc;
 
 use async_trait::async_trait;
@@ -13,13 +12,9 @@ use crate::MemStore;
 struct MemBuilder {}
 #[async_trait]
 impl StoreBuilder<Config, Arc<MemStore>> for MemBuilder {
-    async fn run_test<Fun, Ret, Res>(&self, t: Fun) -> Result<Ret, StorageError<MemNodeId>>
-    where
-        Res: Future<Output = Result<Ret, StorageError<MemNodeId>>> + Send,
-        Fun: Fn(Arc<MemStore>) -> Res + Sync + Send,
-    {
+    async fn build(&self) -> Result<((), Arc<MemStore>), StorageError<MemNodeId>> {
         let store = MemStore::new_async().await;
-        t(store).await
+        Ok(((), store))
     }
 }
 

openraft/src/testing/store_builder.rs

@@ -1,66 +1,58 @@
-use std::fmt::Debug;
-use std::future::Future;
 use std::marker::PhantomData;
 
 use async_trait::async_trait;
 
-use crate::AppData;
-use crate::AppDataResponse;
 use crate::DefensiveCheckBase;
 use crate::RaftStorage;
 use crate::RaftTypeConfig;
 use crate::StorageError;
 use crate::StoreExt;
 
 /// The trait to build a [`RaftStorage`] implementation.
+///
+/// The generic parameter `C` is type config for a `RaftStorage` implementation,
+/// `S` is the type that implements `RaftStorage`,
+/// and `G` is a guard type that cleanup resource when being dropped.
+///
+/// By default `G` is a trivial guard `()`. To test a store that is backed by a folder on disk, `G`
+/// could be the dropper of the temp-dir that stores data.
 #[async_trait]
-pub trait StoreBuilder<C, S>: Send + Sync
+pub trait StoreBuilder<C, S, G = ()>: Send + Sync
 where
     C: RaftTypeConfig,
     S: RaftStorage<C>,
 {
-    async fn run_test<Fun, Ret, Res>(&self, t: Fun) -> Result<Ret, StorageError<C::NodeId>>
-    where
-        Res: Future<Output = Result<Ret, StorageError<C::NodeId>>> + Send,
-        Fun: Fn(S) -> Res + Sync + Send;
+    /// Build a [`RaftStorage`] implementation
+    async fn build(&self) -> Result<(G, S), StorageError<C::NodeId>>;
 }
 
 /// A builder for testing [`StoreExt`].
-pub struct DefensiveStoreBuilder<C, BaseStore, BaseBuilder>
+pub struct DefensiveStoreBuilder<C, BaseStore, BaseBuilder, G>
 where
     C: RaftTypeConfig,
-    C::D: AppData + Debug,
-    C::R: AppDataResponse + Debug,
     BaseStore: RaftStorage<C>,
-    BaseBuilder: StoreBuilder<C, BaseStore>,
+    BaseBuilder: StoreBuilder<C, BaseStore, G>,
 {
     pub base_builder: BaseBuilder,
 
-    pub s: PhantomData<(C, BaseStore)>,
+    pub s: PhantomData<(C, BaseStore, G)>,
 }
 
 #[async_trait]
-impl<C, BaseStore, BaseBuilder> StoreBuilder<C, StoreExt<C, BaseStore>>
-    for DefensiveStoreBuilder<C, BaseStore, BaseBuilder>
+impl<C, G, BaseStore, BaseBuilder> StoreBuilder<C, StoreExt<C, BaseStore>, G>
+    for DefensiveStoreBuilder<C, BaseStore, BaseBuilder, G>
 where
     C: RaftTypeConfig,
-    C::D: AppData + Debug,
-    C::R: AppDataResponse + Debug,
     BaseStore: RaftStorage<C>,
-    BaseBuilder: StoreBuilder<C, BaseStore>,
+    BaseBuilder: StoreBuilder<C, BaseStore, G>,
+    G: Send + Sync,
 {
-    async fn run_test<Fun, Ret, Res>(&self, t: Fun) -> Result<Ret, StorageError<C::NodeId>>
-    where
-        Res: Future<Output = Result<Ret, StorageError<C::NodeId>>> + Send,
-        Fun: Fn(StoreExt<C, BaseStore>) -> Res + Sync + Send,
-    {
-        self.base_builder
-            .run_test(|base_store| async {
-                let sto_ext = StoreExt::new(base_store);
-                sto_ext.set_defensive(true);
-                assert!(sto_ext.is_defensive(), "must impl defensive check");
-                t(sto_ext).await
-            })
-            .await
+    async fn build(&self) -> Result<(G, StoreExt<C, BaseStore>), StorageError<C::NodeId>> {
+        let (g, store) = self.base_builder.build().await?;
+        let sto_ext = StoreExt::new(store);
+        sto_ext.set_defensive(true);
+        assert!(sto_ext.is_defensive(), "must impl defensive check");
+
+        Ok((g, sto_ext))
     }
 }

openraft/src/testing/suite.rs

@@ -46,32 +46,35 @@ macro_rules! btreeset {
 /// Test suite to ensure a `RaftStore` impl works as expected.
 ///
 /// Usage:
-pub struct Suite<C, S, B>
+pub struct Suite<C, S, B, G>
 where
     C: RaftTypeConfig,
     C::D: AppData + Debug,
     C::R: AppDataResponse + Debug,
     S: RaftStorage<C>,
-    B: StoreBuilder<C, S>,
+    B: StoreBuilder<C, S, G>,
+    G: Send + Sync,
 {
     c: PhantomData<C>,
     p: PhantomData<S>,
     f: PhantomData<B>,
+    g: PhantomData<G>,
 }
 
-impl<C, S, B> Suite<C, S, B>
+impl<C, S, B, G> Suite<C, S, B, G>
 where
     C: RaftTypeConfig,
     C::D: AppData + Debug,
     C::R: AppDataResponse + Debug,
     C::NodeId: From<u64>,
     S: RaftStorage<C>,
-    B: StoreBuilder<C, S>,
+    B: StoreBuilder<C, S, G>,
+    G: Send + Sync,
 {
     pub fn test_all(builder: B) -> Result<(), StorageError<C::NodeId>> {
         Suite::test_store(&builder)?;
 
-        let df_builder = DefensiveStoreBuilder::<C, S, B> {
+        let df_builder = DefensiveStoreBuilder::<C, S, B, _> {
             base_builder: builder,
 
             s: Default::default(),
@@ -83,33 +86,33 @@ where
     }
 
     pub fn test_store(builder: &B) -> Result<(), StorageError<C::NodeId>> {
-        run_fut(builder.run_test(Self::last_membership_in_log_initial))?;
-        run_fut(builder.run_test(Self::last_membership_in_log))?;
-        run_fut(builder.run_test(Self::last_membership_in_log_multi_step))?;
-        run_fut(builder.run_test(Self::get_membership_initial))?;
-        run_fut(builder.run_test(Self::get_membership_from_log_and_empty_sm))?;
-        run_fut(builder.run_test(Self::get_membership_from_log_and_sm))?;
-        run_fut(builder.run_test(Self::get_initial_state_without_init))?;
-        run_fut(builder.run_test(Self::get_initial_state_membership_from_log_and_sm))?;
-        run_fut(builder.run_test(Self::get_initial_state_with_state))?;
-        run_fut(builder.run_test(Self::get_initial_state_last_log_gt_sm))?;
-        run_fut(builder.run_test(Self::get_initial_state_last_log_lt_sm))?;
-        run_fut(builder.run_test(Self::get_initial_state_log_ids))?;
-        run_fut(builder.run_test(Self::save_vote))?;
-        run_fut(builder.run_test(Self::get_log_entries))?;
-        run_fut(builder.run_test(Self::try_get_log_entry))?;
-        run_fut(builder.run_test(Self::initial_logs))?;
-        run_fut(builder.run_test(Self::get_log_state))?;
-        run_fut(builder.run_test(Self::get_log_id))?;
-        run_fut(builder.run_test(Self::last_id_in_log))?;
-        run_fut(builder.run_test(Self::last_applied_state))?;
-        run_fut(builder.run_test(Self::purge_logs_upto_0))?;
-        run_fut(builder.run_test(Self::purge_logs_upto_5))?;
-        run_fut(builder.run_test(Self::purge_logs_upto_20))?;
-        run_fut(builder.run_test(Self::delete_logs_since_11))?;
-        run_fut(builder.run_test(Self::delete_logs_since_0))?;
-        run_fut(builder.run_test(Self::append_to_log))?;
-        run_fut(builder.run_test(Self::snapshot_meta))?;
+        run_fut(run_test(builder, Self::last_membership_in_log_initial))?;
+        run_fut(run_test(builder, Self::last_membership_in_log))?;
+        run_fut(run_test(builder, Self::last_membership_in_log_multi_step))?;
+        run_fut(run_test(builder, Self::get_membership_initial))?;
+        run_fut(run_test(builder, Self::get_membership_from_log_and_empty_sm))?;
+        run_fut(run_test(builder, Self::get_membership_from_log_and_sm))?;
+        run_fut(run_test(builder, Self::get_initial_state_without_init))?;
+        run_fut(run_test(builder, Self::get_initial_state_membership_from_log_and_sm))?;
+        run_fut(run_test(builder, Self::get_initial_state_with_state))?;
+        run_fut(run_test(builder, Self::get_initial_state_last_log_gt_sm))?;
+        run_fut(run_test(builder, Self::get_initial_state_last_log_lt_sm))?;
+        run_fut(run_test(builder, Self::get_initial_state_log_ids))?;
+        run_fut(run_test(builder, Self::save_vote))?;
+        run_fut(run_test(builder, Self::get_log_entries))?;
+        run_fut(run_test(builder, Self::try_get_log_entry))?;
+        run_fut(run_test(builder, Self::initial_logs))?;
+        run_fut(run_test(builder, Self::get_log_state))?;
+        run_fut(run_test(builder, Self::get_log_id))?;
+        run_fut(run_test(builder, Self::last_id_in_log))?;
+        run_fut(run_test(builder, Self::last_applied_state))?;
+        run_fut(run_test(builder, Self::purge_logs_upto_0))?;
+        run_fut(run_test(builder, Self::purge_logs_upto_5))?;
+        run_fut(run_test(builder, Self::purge_logs_upto_20))?;
+        run_fut(run_test(builder, Self::delete_logs_since_11))?;
+        run_fut(run_test(builder, Self::delete_logs_since_0))?;
+        run_fut(run_test(builder, Self::append_to_log))?;
+        run_fut(run_test(builder, Self::snapshot_meta))?;
 
         // run_fut(Suite::apply_single(builder))?;
         // run_fut(Suite::apply_multi(builder))?;
@@ -1035,31 +1038,32 @@ where
 // Defensive test:
 // If a RaftStore impl support defensive check, enable it and check if it returns errors when
 // abnormal input is seen. A RaftStore with defensive check is able to expose bugs in raft core.
-impl<C, S, B> Suite<C, S, B>
+impl<C, S, B, G> Suite<C, S, B, G>
 where
     C: RaftTypeConfig,
     C::D: AppData + Debug,
     C::R: AppDataResponse + Debug,
     C::NodeId: From<u64>,
     S: RaftStorage<C>,
-    B: StoreBuilder<C, S>,
+    B: StoreBuilder<C, S, G>,
+    G: Send + Sync,
 {
     pub fn test_store_defensive(builder: &B) -> Result<(), StorageError<C::NodeId>> {
-        run_fut(builder.run_test(Self::df_get_membership_config_dirty_log))?;
-        run_fut(builder.run_test(Self::df_get_initial_state_dirty_log))?;
-        run_fut(builder.run_test(Self::df_save_vote_ascending))?;
-        run_fut(builder.run_test(Self::df_get_log_entries))?;
-        run_fut(builder.run_test(Self::df_append_to_log_nonempty_input))?;
-        run_fut(builder.run_test(Self::df_append_to_log_nonconsecutive_input))?;
-        run_fut(builder.run_test(Self::df_append_to_log_eq_last_plus_one))?;
-        run_fut(builder.run_test(Self::df_append_to_log_eq_last_applied_plus_one))?;
-        run_fut(builder.run_test(Self::df_append_to_log_gt_last_log_id))?;
-        run_fut(builder.run_test(Self::df_append_to_log_gt_last_applied_id))?;
-        run_fut(builder.run_test(Self::df_apply_nonempty_input))?;
-        run_fut(builder.run_test(Self::df_apply_index_eq_last_applied_plus_one))?;
-        run_fut(builder.run_test(Self::df_apply_gt_last_applied_id))?;
-        run_fut(builder.run_test(Self::df_purge_applied_le_last_applied))?;
-        run_fut(builder.run_test(Self::df_delete_conflict_gt_last_applied))?;
+        run_fut(run_test(builder, Self::df_get_membership_config_dirty_log))?;
+        run_fut(run_test(builder, Self::df_get_initial_state_dirty_log))?;
+        run_fut(run_test(builder, Self::df_save_vote_ascending))?;
+        run_fut(run_test(builder, Self::df_get_log_entries))?;
+        run_fut(run_test(builder, Self::df_append_to_log_nonempty_input))?;
+        run_fut(run_test(builder, Self::df_append_to_log_nonconsecutive_input))?;
+        run_fut(run_test(builder, Self::df_append_to_log_eq_last_plus_one))?;
+        run_fut(run_test(builder, Self::df_append_to_log_eq_last_applied_plus_one))?;
+        run_fut(run_test(builder, Self::df_append_to_log_gt_last_log_id))?;
+        run_fut(run_test(builder, Self::df_append_to_log_gt_last_applied_id))?;
+        run_fut(run_test(builder, Self::df_apply_nonempty_input))?;
+        run_fut(run_test(builder, Self::df_apply_index_eq_last_applied_plus_one))?;
+        run_fut(run_test(builder, Self::df_apply_gt_last_applied_id))?;
+        run_fut(run_test(builder, Self::df_purge_applied_le_last_applied))?;
+        run_fut(run_test(builder, Self::df_delete_conflict_gt_last_applied))?;
 
         Ok(())
     }
@@ -1534,3 +1538,16 @@ where
     rt.block_on(f)?;
     Ok(())
 }
+
+/// Build a `RaftStorage` implementation and run a test on it.
+async fn run_test<C, S, G, B, TestFn, Ret, Fu>(builder: &B, test_fn: TestFn) -> Result<Ret, StorageError<C::NodeId>>
+where
+    C: RaftTypeConfig,
+    S: RaftStorage<C>,
+    B: StoreBuilder<C, S, G>,
+    Fu: Future<Output = Result<Ret, StorageError<C::NodeId>>> + Send,
+    TestFn: Fn(S) -> Fu + Sync + Send,
+{
+    let (_g, store) = builder.build().await?;
+    test_fn(store).await
+}

rocksstore-compat07/src/test.rs

@@ -1,30 +1,22 @@
-use std::future::Future;
 use std::sync::Arc;
 
 use async_trait::async_trait;
 use openraft::testing::StoreBuilder;
 use openraft::testing::Suite;
 use openraft::StorageError;
+use tempfile::TempDir;
 
 use crate::Config;
 use crate::RocksNodeId;
 use crate::RocksStore;
 
 struct RocksBuilder {}
 #[async_trait]
-impl StoreBuilder<Config, Arc<RocksStore>> for RocksBuilder {
-    async fn run_test<Fun, Ret, Res>(&self, t: Fun) -> Result<Ret, StorageError<RocksNodeId>>
-    where
-        Res: Future<Output = Result<Ret, StorageError<RocksNodeId>>> + Send,
-        Fun: Fn(Arc<RocksStore>) -> Res + Sync + Send,
-    {
+impl StoreBuilder<Config, Arc<RocksStore>, TempDir> for RocksBuilder {
+    async fn build(&self) -> Result<(TempDir, Arc<RocksStore>), StorageError<RocksNodeId>> {
         let td = tempfile::TempDir::new().expect("couldn't create temp dir");
-        let r = {
-            let store = RocksStore::new(td.path()).await;
-            t(store).await
-        };
-        td.close().expect("could not close temp directory");
-        r
+        let store = RocksStore::new(td.path()).await;
+        Ok((td, store))
     }
 }
 

rocksstore/src/test.rs

@@ -1,30 +1,22 @@
-use std::future::Future;
 use std::sync::Arc;
 
 use async_trait::async_trait;
 use openraft::testing::StoreBuilder;
 use openraft::testing::Suite;
 use openraft::StorageError;
+use tempfile::TempDir;
 
 use crate::Config;
 use crate::RocksNodeId;
 use crate::RocksStore;
 
 struct RocksBuilder {}
 #[async_trait]
-impl StoreBuilder<Config, Arc<RocksStore>> for RocksBuilder {
-    async fn run_test<Fun, Ret, Res>(&self, t: Fun) -> Result<Ret, StorageError<RocksNodeId>>
-    where
-        Res: Future<Output = Result<Ret, StorageError<RocksNodeId>>> + Send,
-        Fun: Fn(Arc<RocksStore>) -> Res + Sync + Send,
-    {
+impl StoreBuilder<Config, Arc<RocksStore>, TempDir> for RocksBuilder {
+    async fn build(&self) -> Result<(TempDir, Arc<RocksStore>), StorageError<RocksNodeId>> {
         let td = tempfile::TempDir::new().expect("couldn't create temp dir");
-        let r = {
-            let store = RocksStore::new(td.path()).await;
-            t(store).await
-        };
-        td.close().expect("could not close temp directory");
-        r
+        let store = RocksStore::new(td.path()).await;
+        Ok((td, store))
     }
 }
 /// To customize a builder: