Refactor test code and introduce TestSyncStore

.. which asserts that all `KVStore` implementations operatate
synchronously, i.e., yield identical results given the same inputs.

Author: tnull

src/io/sqlite_store/migrations.rs

@@ -37,6 +37,7 @@ pub(super) fn migrate_schema(
 mod tests {
 	use crate::io::sqlite_store::SqliteStore;
 	use crate::io::test_utils::do_read_write_remove_list_persist;
+	use crate::test::utils::random_storage_path;
 
 	use lightning::util::persist::KVStore;
 
@@ -48,7 +49,7 @@ mod tests {
 	fn rwrl_post_schema_1_migration() {
 		let old_schema_version = 1;
 
-		let mut temp_path = std::env::temp_dir();
+		let mut temp_path = random_storage_path();
 		temp_path.push("rwrl_post_schema_1_migration");
 
 		let db_file_name = "test_db".to_string();

src/io/sqlite_store/mod.rs

@@ -281,6 +281,7 @@ impl KVStore for SqliteStore {
 mod tests {
 	use super::*;
 	use crate::io::test_utils::{do_read_write_remove_list_persist, do_test_store};
+	use crate::test::utils::random_storage_path;
 
 	impl Drop for SqliteStore {
 		fn drop(&mut self) {
@@ -293,7 +294,7 @@ mod tests {
 
 	#[test]
 	fn read_write_remove_list_persist() {
-		let mut temp_path = std::env::temp_dir();
+		let mut temp_path = random_storage_path();
 		temp_path.push("read_write_remove_list_persist");
 		let store = SqliteStore::new(
 			temp_path,
@@ -306,7 +307,7 @@ mod tests {
 
 	#[test]
 	fn test_sqlite_store() {
-		let mut temp_path = std::env::temp_dir();
+		let mut temp_path = random_storage_path();
 		temp_path.push("test_sqlite_store");
 		let store_0 = SqliteStore::new(
 			temp_path.clone(),

src/io/test_utils.rs

@@ -15,6 +15,7 @@ use lightning::{check_added_monitors, check_closed_broadcast, check_closed_event
 
 use std::panic::RefUnwindSafe;
 use std::path::PathBuf;
+use std::sync::RwLock;
 
 pub(crate) fn do_read_write_remove_list_persist<K: KVStore + RefUnwindSafe>(kv_store: &K) {
 	let data = [42u8; 32];
@@ -171,3 +172,135 @@ pub(crate) fn do_test_store<K: KVStore>(store_0: &K, store_1: &K) {
 	// Make sure everything is persisted as expected after close.
 	check_persisted_data!(CLOSED_CHANNEL_UPDATE_ID);
 }
+
+// A `KVStore` impl for testing purposes that wraps all our `KVStore`s and asserts their synchronicity.
+pub(crate) struct TestSyncStore {
+	serializer: RwLock<()>,
+	test_store: TestStore,
+	fs_store: FilesystemStore,
+	sqlite_store: SqliteStore,
+}
+
+impl TestSyncStore {
+	pub(crate) fn new(dest_dir: PathBuf) -> Self {
+		let serializer = RwLock::new(());
+		let mut fs_dir = dest_dir.clone();
+		fs_dir.push("fs_store");
+		let fs_store = FilesystemStore::new(fs_dir);
+		let mut sql_dir = dest_dir.clone();
+		sql_dir.push("sqlite_store");
+		let sqlite_store = SqliteStore::new(
+			sql_dir,
+			Some("test_sync_db".to_string()),
+			Some("test_sync_table".to_string()),
+		)
+		.unwrap();
+		let test_store = TestStore::new();
+		Self { serializer, fs_store, sqlite_store, test_store }
+	}
+
+	fn do_list(&self, namespace: &str, sub_namespace: &str) -> std::io::Result<Vec<String>> {
+		let fs_res = self.fs_store.list(namespace, sub_namespace);
+		let sqlite_res = self.sqlite_store.list(namespace, sub_namespace);
+		let test_res = self.test_store.list(namespace, sub_namespace);
+
+		match fs_res {
+			Ok(mut list) => {
+				list.sort();
+
+				let mut sqlite_list = sqlite_res.unwrap();
+				sqlite_list.sort();
+				assert_eq!(list, sqlite_list);
+
+				let mut test_list = test_res.unwrap();
+				test_list.sort();
+				assert_eq!(list, test_list);
+
+				Ok(list)
+			}
+			Err(e) => {
+				assert!(sqlite_res.is_err());
+				assert!(test_res.is_err());
+				Err(e)
+			}
+		}
+	}
+}
+
+impl KVStore for TestSyncStore {
+	fn read(&self, namespace: &str, sub_namespace: &str, key: &str) -> std::io::Result<Vec<u8>> {
+		let _guard = self.serializer.read().unwrap();
+
+		let fs_res = self.fs_store.read(namespace, sub_namespace, key);
+		let sqlite_res = self.sqlite_store.read(namespace, sub_namespace, key);
+		let test_res = self.test_store.read(namespace, sub_namespace, key);
+
+		match fs_res {
+			Ok(read) => {
+				assert_eq!(read, sqlite_res.unwrap());
+				assert_eq!(read, test_res.unwrap());
+				Ok(read)
+			}
+			Err(e) => {
+				assert!(sqlite_res.is_err());
+				assert_eq!(e.kind(), unsafe { sqlite_res.unwrap_err_unchecked().kind() });
+				assert!(test_res.is_err());
+				assert_eq!(e.kind(), unsafe { test_res.unwrap_err_unchecked().kind() });
+				Err(e)
+			}
+		}
+	}
+
+	fn write(
+		&self, namespace: &str, sub_namespace: &str, key: &str, buf: &[u8],
+	) -> std::io::Result<()> {
+		let _guard = self.serializer.write().unwrap();
+		let fs_res = self.fs_store.write(namespace, sub_namespace, key, buf);
+		let sqlite_res = self.sqlite_store.write(namespace, sub_namespace, key, buf);
+		let test_res = self.test_store.write(namespace, sub_namespace, key, buf);
+
+		assert!(self.do_list(namespace, sub_namespace).unwrap().contains(&key.to_string()));
+
+		match fs_res {
+			Ok(()) => {
+				assert!(sqlite_res.is_ok());
+				assert!(test_res.is_ok());
+				Ok(())
+			}
+			Err(e) => {
+				assert!(sqlite_res.is_err());
+				assert!(test_res.is_err());
+				Err(e)
+			}
+		}
+	}
+
+	fn remove(
+		&self, namespace: &str, sub_namespace: &str, key: &str, lazy: bool,
+	) -> std::io::Result<()> {
+		let _guard = self.serializer.write().unwrap();
+		let fs_res = self.fs_store.remove(namespace, sub_namespace, key, lazy);
+		let sqlite_res = self.sqlite_store.remove(namespace, sub_namespace, key, lazy);
+		let test_res = self.test_store.remove(namespace, sub_namespace, key, lazy);
+
+		assert!(!self.do_list(namespace, sub_namespace).unwrap().contains(&key.to_string()));
+
+		match fs_res {
+			Ok(()) => {
+				assert!(sqlite_res.is_ok());
+				assert!(test_res.is_ok());
+				Ok(())
+			}
+			Err(e) => {
+				assert!(sqlite_res.is_err());
+				assert!(test_res.is_err());
+				Err(e)
+			}
+		}
+	}
+
+	fn list(&self, namespace: &str, sub_namespace: &str) -> std::io::Result<Vec<String>> {
+		let _guard = self.serializer.read().unwrap();
+		self.do_list(namespace, sub_namespace)
+	}
+}

src/peer_store.rs

@@ -163,7 +163,11 @@ mod tests {
 
 		// Check we can read back what we persisted.
 		let persisted_bytes = store
-			.read(PEER_INFO_PERSISTENCE_NAMESPACE, PEER_INFO_PERSISTENCE_SUB_NAMESPACE, PEER_INFO_PERSISTENCE_KEY)
+			.read(
+				PEER_INFO_PERSISTENCE_NAMESPACE,
+				PEER_INFO_PERSISTENCE_SUB_NAMESPACE,
+				PEER_INFO_PERSISTENCE_KEY,
+			)
 			.unwrap();
 		let deser_peer_store =
 			PeerStore::read(&mut &persisted_bytes[..], (Arc::clone(&store), logger)).unwrap();