From 8a910541393e2907ba9dd73275c3fd24285d09bf Mon Sep 17 00:00:00 2001 From: JianMinTang Date: Mon, 4 Nov 2024 11:13:55 +0800 Subject: [PATCH 1/4] chore: replace hardcoding /tmp/xxx path Signed-off-by: JianMinTang --- Cargo.lock | 2 + crates/engine/Cargo.toml | 1 + crates/engine/src/proxy.rs | 57 +++++++++++++------------ crates/engine/src/rocksdb_engine/mod.rs | 18 ++++---- crates/xline/Cargo.toml | 1 + crates/xline/src/storage/db.rs | 35 +++++++-------- 6 files changed, 61 insertions(+), 53 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index edbd09207..dcc2fc051 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -922,6 +922,7 @@ dependencies = [ "parking_lot", "rocksdb", "serde", + "tempfile", "test-macros", "thiserror", "tokio-util", @@ -3741,6 +3742,7 @@ dependencies = [ "sha2", "strum", "strum_macros", + "tempfile", "test-macros", "tokio", "tokio-stream 0.1.12", diff --git a/crates/engine/Cargo.toml b/crates/engine/Cargo.toml index dee74b692..9721112f3 100644 --- a/crates/engine/Cargo.toml +++ b/crates/engine/Cargo.toml @@ -35,3 +35,4 @@ workspace-hack = { version = "0.1", path = "../../workspace-hack" } [dev-dependencies] test-macros = { path = "../test-macros" } +tempfile = "3" diff --git a/crates/engine/src/proxy.rs b/crates/engine/src/proxy.rs index 6952d1667..c52ade8a1 100644 --- a/crates/engine/src/proxy.rs +++ b/crates/engine/src/proxy.rs @@ -335,6 +335,8 @@ mod test { use clippy_utilities::NumericCast; use test_macros::abort_on_panic; + use tempfile::TempDir; + use super::*; use crate::api::snapshot_api::SnapshotApi; @@ -342,8 +344,9 @@ mod test { #[test] fn write_batch_into_a_non_existing_table_should_fail() { - let dir = PathBuf::from("/tmp/write_batch_into_a_non_existing_table_should_fail"); - let rocks_engine_path = dir.join("rocks_engine"); + let dir = + TempDir::with_prefix("/tmp/write_batch_into_a_non_existing_table_should_fail").unwrap(); + let rocks_engine_path = dir.path().join("rocks_engine"); let engines = vec![ Engine::new(EngineType::Memory, &TESTTABLES).unwrap(), Engine::new(EngineType::Rocks(rocks_engine_path), &TESTTABLES).unwrap(), @@ -363,13 +366,13 @@ mod test { let delete_range = WriteOperation::new_delete_range("hello", b"hello", b"world"); assert!(engine.write_multi(vec![delete_range], false).is_err()); } - std::fs::remove_dir_all(dir).unwrap(); + dir.close().unwrap(); } #[test] fn write_batch_should_success() { - let dir = PathBuf::from("/tmp/write_batch_should_success"); - let rocks_engine_path = dir.join("rocks_engine"); + let dir = TempDir::with_prefix("/tmp/write_batch_should_success").unwrap(); + let rocks_engine_path = dir.path().join("rocks_engine"); let engines = vec![ Engine::new(EngineType::Memory, &TESTTABLES).unwrap(), Engine::new(EngineType::Rocks(rocks_engine_path), &TESTTABLES).unwrap(), @@ -409,13 +412,13 @@ mod test { assert!(engine.get("kv", &get_key_1).unwrap().is_some()); assert!(engine.get("kv", &get_key_2).unwrap().is_none()); } - std::fs::remove_dir_all(dir).unwrap(); + dir.close().unwrap(); } #[test] fn get_operation_should_success() { - let dir = PathBuf::from("/tmp/get_operation_should_success"); - let rocks_engine_path = dir.join("rocks_engine"); + let dir = TempDir::with_prefix("/tmp/get_operation_should_success").unwrap(); + let rocks_engine_path = dir.path().join("rocks_engine"); let engines = vec![ Engine::new(EngineType::Memory, &TESTTABLES).unwrap(), Engine::new(EngineType::Rocks(rocks_engine_path), &TESTTABLES).unwrap(), @@ -447,17 +450,17 @@ mod test { .collect::, Vec)>>(); assert_eq!(res_3.sort(), expected_all_values.sort()); } - std::fs::remove_dir_all(dir).unwrap(); + dir.close().unwrap(); } #[tokio::test] #[abort_on_panic] async fn snapshot_should_work() { - let dir = PathBuf::from("/tmp/snapshot_should_work"); - let origin_data_dir = dir.join("origin"); - let recover_data_dir = dir.join("recover"); - let snapshot_dir = dir.join("snapshot"); - let snapshot_bak_dir = dir.join("snapshot_bak"); + let dir = TempDir::with_prefix("/tmp/snapshot_should_work").unwrap(); + let origin_data_dir = dir.path().join("origin"); + let recover_data_dir = dir.path().join("recover"); + let snapshot_dir = dir.path().join("snapshot"); + let snapshot_bak_dir = dir.path().join("snapshot_bak"); let engines = vec![ Engine::new(EngineType::Memory, &TESTTABLES).unwrap(), Engine::new(EngineType::Rocks(origin_data_dir), &TESTTABLES).unwrap(), @@ -508,14 +511,14 @@ mod test { assert!(value2.is_none()); } - std::fs::remove_dir_all(&dir).unwrap(); + dir.close().unwrap(); } #[tokio::test] #[abort_on_panic] async fn txn_write_multi_should_success() { - let dir = PathBuf::from("/tmp/txn_write_multi_should_success"); - let rocks_engine_path = dir.join("rocks_engine"); + let dir = TempDir::with_prefix("/tmp/txn_write_multi_should_success").unwrap(); + let rocks_engine_path = dir.path().join("rocks_engine"); let engines = vec![ Engine::new(EngineType::Memory, &TESTTABLES).unwrap(), Engine::new(EngineType::Rocks(rocks_engine_path), &TESTTABLES).unwrap(), @@ -558,14 +561,14 @@ mod test { txn.commit().unwrap(); } - std::fs::remove_dir_all(dir).unwrap(); + dir.close().unwrap(); } #[tokio::test] #[abort_on_panic] async fn txn_get_operation_should_success() { - let dir = PathBuf::from("/tmp/txn_get_operation_should_success"); - let rocks_engine_path = dir.join("rocks_engine"); + let dir = TempDir::with_prefix("/tmp/txn_get_operation_should_success").unwrap(); + let rocks_engine_path = dir.path().join("rocks_engine"); let engines = vec![ Engine::new(EngineType::Memory, &TESTTABLES).unwrap(), Engine::new(EngineType::Rocks(rocks_engine_path), &TESTTABLES).unwrap(), @@ -592,14 +595,14 @@ mod test { assert_eq!(res_2, expected_multi_values); txn.commit().unwrap(); } - std::fs::remove_dir_all(dir).unwrap(); + dir.close().unwrap(); } #[tokio::test] #[abort_on_panic] async fn txn_operation_is_atomic() { - let dir = PathBuf::from("/tmp/txn_operation_should_be_atomic"); - let rocks_engine_path = dir.join("rocks_engine"); + let dir = TempDir::with_prefix("/tmp/txn_operation_should_be_atomic").unwrap(); + let rocks_engine_path = dir.path().join("rocks_engine"); let engines = vec![ Engine::new(EngineType::Memory, &TESTTABLES).unwrap(), Engine::new(EngineType::Rocks(rocks_engine_path), &TESTTABLES).unwrap(), @@ -625,14 +628,14 @@ mod test { assert_eq!(engine.get("kv", key).unwrap().unwrap(), val); } } - std::fs::remove_dir_all(dir).unwrap(); + dir.close().unwrap(); } #[tokio::test] #[abort_on_panic] async fn txn_revert_should_success() { - let dir = PathBuf::from("/tmp/txn_revert_should_success"); - let rocks_engine_path = dir.join("rocks_engine"); + let dir = TempDir::with_prefix("/tmp/txn_revert_should_success").unwrap(); + let rocks_engine_path = dir.path().join("rocks_engine"); let engines = vec![ Engine::new(EngineType::Memory, &TESTTABLES).unwrap(), Engine::new(EngineType::Rocks(rocks_engine_path), &TESTTABLES).unwrap(), @@ -659,6 +662,6 @@ mod test { assert!(engine.get("kv", key).unwrap().is_none()); } } - std::fs::remove_dir_all(dir).unwrap(); + dir.close().unwrap(); } } diff --git a/crates/engine/src/rocksdb_engine/mod.rs b/crates/engine/src/rocksdb_engine/mod.rs index 28270bdb3..4aee64c73 100644 --- a/crates/engine/src/rocksdb_engine/mod.rs +++ b/crates/engine/src/rocksdb_engine/mod.rs @@ -725,8 +725,7 @@ impl SnapshotApi for RocksSnapshot { #[cfg(test)] mod test { - use std::env::temp_dir; - + use tempfile::TempDir; use test_macros::abort_on_panic; use super::*; @@ -735,9 +734,9 @@ mod test { #[tokio::test] #[abort_on_panic] async fn test_rocks_errors() { - let dir = PathBuf::from("/tmp/test_rocks_errors"); - let engine_path = dir.join("engine"); - let snapshot_path = dir.join("snapshot"); + let dir = TempDir::with_prefix("/tmp/test_rocks_errors").unwrap(); + let engine_path = dir.path().join("engine"); + let snapshot_path = dir.path().join("snapshot"); let engine = RocksEngine::new(engine_path, &TEST_TABLES).unwrap(); let res = engine.get("not_exist", "key"); assert!(res.is_err()); @@ -756,13 +755,14 @@ mod test { }; let res = engine.apply_snapshot(fake_snapshot, &["not_exist"]).await; assert!(res.is_err()); - fs::remove_dir_all(dir).unwrap(); + dir.close().unwrap(); } #[test] fn test_engine_size() { - let path = temp_dir().join("test_engine_size"); - let engine = RocksEngine::new(path.clone(), &TEST_TABLES).unwrap(); + let dir = TempDir::with_prefix("/tmp/test_engine_size").unwrap(); + let engine_path = dir.path().join("engine"); + let engine = RocksEngine::new(engine_path, &TEST_TABLES).unwrap(); let size1 = engine.file_size().unwrap(); engine .write_multi( @@ -776,6 +776,6 @@ mod test { .unwrap(); let size2 = engine.file_size().unwrap(); assert!(size2 > size1); - fs::remove_dir_all(path).unwrap(); + dir.close().unwrap(); } } diff --git a/crates/xline/Cargo.toml b/crates/xline/Cargo.toml index 4d783912a..0d2706f71 100644 --- a/crates/xline/Cargo.toml +++ b/crates/xline/Cargo.toml @@ -89,3 +89,4 @@ strum_macros = "0.26.4" test-macros = { path = "../test-macros" } xline-client = { path = "../xline-client" } xline-test-utils = { path = "../xline-test-utils" } +tempfile = "3" diff --git a/crates/xline/src/storage/db.rs b/crates/xline/src/storage/db.rs index d6389d389..4870b9211 100644 --- a/crates/xline/src/storage/db.rs +++ b/crates/xline/src/storage/db.rs @@ -351,7 +351,7 @@ pub enum WriteOp<'a> { #[cfg(test)] mod test { - use std::path::PathBuf; + use tempfile::TempDir; use engine::SnapshotApi; use test_macros::abort_on_panic; @@ -361,8 +361,9 @@ mod test { #[tokio::test] #[abort_on_panic] async fn test_reset() -> Result<(), ExecuteError> { - let data_dir = PathBuf::from("/tmp/test_reset"); - let db = DB::open(&EngineConfig::RocksDB(data_dir.clone()))?; + let dir = TempDir::with_prefix("/tmp/test_reset").unwrap(); + let engine_path = dir.path().join("engine"); + let db = DB::open(&EngineConfig::RocksDB(engine_path))?; let revision = Revision::new(1, 1); let key = revision.encode_to_vec(); @@ -382,17 +383,17 @@ mod test { let res = db.get_all(KV_TABLE)?; assert!(res.is_empty()); - std::fs::remove_dir_all(data_dir).unwrap(); + dir.close().unwrap(); Ok(()) } #[tokio::test] #[abort_on_panic] async fn test_db_snapshot() -> Result<(), ExecuteError> { - let dir = PathBuf::from("/tmp/test_db_snapshot"); - let origin_db_path = dir.join("origin_db"); - let new_db_path = dir.join("new_db"); - let snapshot_path = dir.join("snapshot"); + let dir = TempDir::with_prefix("/tmp/test_db_snapshot").unwrap(); + let origin_db_path = dir.path().join("origin_db"); + let new_db_path = dir.path().join("new_db"); + let snapshot_path = dir.path().join("snapshot"); let origin_db = DB::open(&EngineConfig::RocksDB(origin_db_path))?; let revision = Revision::new(1, 1); @@ -412,16 +413,16 @@ mod test { let res = new_db.get_values(KV_TABLE, &[&key])?; assert_eq!(res, vec![Some(kv.encode_to_vec())]); - std::fs::remove_dir_all(dir).unwrap(); + dir.close().unwrap(); Ok(()) } #[tokio::test] #[abort_on_panic] async fn test_db_snapshot_wrong_type() -> Result<(), ExecuteError> { - let dir = PathBuf::from("/tmp/test_db_snapshot_wrong_type"); - let db_path = dir.join("db"); - let snapshot_path = dir.join("snapshot"); + let dir = TempDir::with_prefix("/tmp/test_db_snapshot_wrong_type").unwrap(); + let db_path = dir.path().join("db"); + let snapshot_path = dir.path().join("snapshot"); let rocks_db = DB::open(&EngineConfig::RocksDB(db_path))?; let mem_db = DB::open(&EngineConfig::Memory)?; @@ -429,22 +430,22 @@ mod test { let res = mem_db.reset(Some(rocks_snap)).await; assert!(res.is_err()); - std::fs::remove_dir_all(dir).unwrap(); + dir.close().unwrap(); Ok(()) } #[tokio::test] #[abort_on_panic] async fn test_get_snapshot() -> Result<(), ExecuteError> { - let dir = PathBuf::from("/tmp/test_get_snapshot"); - let data_path = dir.join("data"); - let snapshot_path = dir.join("snapshot"); + let dir = TempDir::with_prefix("/tmp/test_get_snapshot").unwrap(); + let data_path = dir.path().join("data"); + let snapshot_path = dir.path().join("snapshot"); let db = DB::open(&EngineConfig::RocksDB(data_path))?; let mut res = db.get_snapshot(snapshot_path)?; assert_ne!(res.size(), 0); res.clean().await.unwrap(); - std::fs::remove_dir_all(dir).unwrap(); + dir.close().unwrap(); Ok(()) } From e34eded343e213c43744b61ae63a2fefa877e92f Mon Sep 17 00:00:00 2001 From: JianMinTang Date: Mon, 4 Nov 2024 17:20:17 +0800 Subject: [PATCH 2/4] chore: miss a hardcoding /tmp/xxx path Signed-off-by: JianMinTang --- crates/xline/src/server/maintenance.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/crates/xline/src/server/maintenance.rs b/crates/xline/src/server/maintenance.rs index 9ecf80209..7ce9834cc 100644 --- a/crates/xline/src/server/maintenance.rs +++ b/crates/xline/src/server/maintenance.rs @@ -273,8 +273,10 @@ fn snapshot_stream( #[cfg(test)] mod test { - use std::{error::Error, path::PathBuf}; + use std::error::Error; + use tempfile::TempDir; + use test_macros::abort_on_panic; use tokio_stream::StreamExt; use utils::config::EngineConfig; @@ -285,9 +287,9 @@ mod test { #[tokio::test] #[abort_on_panic] async fn test_snapshot_stream() -> Result<(), Box> { - let dir = PathBuf::from("/tmp/test_snapshot_rpc"); - let db_path = dir.join("db"); - let snapshot_path = dir.join("snapshot"); + let dir = TempDir::with_prefix("/tmp/test_snapshot_rpc").unwrap(); + let db_path = dir.path().join("db"); + let snapshot_path = dir.path().join("snapshot"); let db = DB::open(&EngineConfig::RocksDB(db_path.clone()))?; let header_gen = HeaderGenerator::new(0, 0); @@ -310,7 +312,7 @@ mod test { assert_eq!(snap1_data, snap2_data); snap2.clean().await.unwrap(); - std::fs::remove_dir_all(dir).unwrap(); + dir.close().unwrap(); Ok(()) } } From 996a9592e40a2a3e8d575f8196a131282e0565c4 Mon Sep 17 00:00:00 2001 From: JianMinTang Date: Mon, 4 Nov 2024 17:27:38 +0800 Subject: [PATCH 3/4] chore: check the code format Signed-off-by: JianMinTang --- crates/xline/src/server/maintenance.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/xline/src/server/maintenance.rs b/crates/xline/src/server/maintenance.rs index 7ce9834cc..528625aa1 100644 --- a/crates/xline/src/server/maintenance.rs +++ b/crates/xline/src/server/maintenance.rs @@ -276,7 +276,6 @@ mod test { use std::error::Error; use tempfile::TempDir; - use test_macros::abort_on_panic; use tokio_stream::StreamExt; use utils::config::EngineConfig; From 87efb60a0f7574282f11956a486c65057b6e4592 Mon Sep 17 00:00:00 2001 From: JianMinTang Date: Wed, 6 Nov 2024 16:41:19 +0800 Subject: [PATCH 4/4] chore: fixed the dependencies sort Signed-off-by: JianMinTang --- crates/engine/Cargo.toml | 2 +- crates/xline/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/engine/Cargo.toml b/crates/engine/Cargo.toml index 9721112f3..2576e8b90 100644 --- a/crates/engine/Cargo.toml +++ b/crates/engine/Cargo.toml @@ -34,5 +34,5 @@ uuid = { version = "1", features = ["v4"] } workspace-hack = { version = "0.1", path = "../../workspace-hack" } [dev-dependencies] -test-macros = { path = "../test-macros" } tempfile = "3" +test-macros = { path = "../test-macros" } diff --git a/crates/xline/Cargo.toml b/crates/xline/Cargo.toml index 0d2706f71..7621dfb55 100644 --- a/crates/xline/Cargo.toml +++ b/crates/xline/Cargo.toml @@ -86,7 +86,7 @@ mockall = "0.12.1" rand = "0.8.5" strum = "0.26" strum_macros = "0.26.4" +tempfile = "3" test-macros = { path = "../test-macros" } xline-client = { path = "../xline-client" } xline-test-utils = { path = "../xline-test-utils" } -tempfile = "3"