From 32d5ddfe40979e8214c691b1f946e8d49aa87c8b Mon Sep 17 00:00:00 2001 From: Huber Date: Wed, 4 Feb 2026 00:44:12 +0200 Subject: [PATCH] fix(test): clean up test temp directories on drop (#21772) --- .../storage/db/src/implementation/mdbx/mod.rs | 67 ++++++++++--------- .../provider/src/providers/database/mod.rs | 10 +-- crates/storage/provider/src/test_utils/mod.rs | 26 ++++--- 3 files changed, 56 insertions(+), 47 deletions(-) diff --git a/crates/storage/db/src/implementation/mdbx/mod.rs b/crates/storage/db/src/implementation/mdbx/mod.rs index f0d46b2c9b..5a53fa53e6 100644 --- a/crates/storage/db/src/implementation/mdbx/mod.rs +++ b/crates/storage/db/src/implementation/mdbx/mod.rs @@ -643,9 +643,11 @@ mod tests { use std::str::FromStr; use tempfile::TempDir; - /// Create database for testing - fn create_test_db(kind: DatabaseEnvKind) -> DatabaseEnv { - create_test_db_with_path(kind, &tempfile::TempDir::new().expect(ERROR_TEMPDIR).keep()) + /// Create database for testing. Returns the `TempDir` to prevent cleanup until test ends. + fn create_test_db(kind: DatabaseEnvKind) -> (TempDir, DatabaseEnv) { + let tempdir = tempfile::TempDir::new().expect(ERROR_TEMPDIR); + let env = create_test_db_with_path(kind, tempdir.path()); + (tempdir, env) } /// Create database for testing with specified path @@ -670,13 +672,13 @@ mod tests { #[test] fn db_creation() { - create_test_db(DatabaseEnvKind::RW); + let _tempdir = create_test_db(DatabaseEnvKind::RW); } #[test] fn db_drop_orphan_table() { - let path = tempfile::TempDir::new().expect(ERROR_TEMPDIR).keep(); - let db = create_test_db_with_path(DatabaseEnvKind::RW, &path); + let tempdir = tempfile::TempDir::new().expect(ERROR_TEMPDIR); + let db = create_test_db_with_path(DatabaseEnvKind::RW, tempdir.path()); // Create an orphan table by manually creating it let orphan_table_name = "OrphanTestTable"; @@ -715,7 +717,7 @@ mod tests { #[test] fn db_manual_put_get() { - let env = create_test_db(DatabaseEnvKind::RW); + let (_tempdir, env) = create_test_db(DatabaseEnvKind::RW); let value = Header::default(); let key = 1u64; @@ -734,7 +736,7 @@ mod tests { #[test] fn db_dup_cursor_delete_first() { - let db: DatabaseEnv = create_test_db(DatabaseEnvKind::RW); + let (_tempdir, db) = create_test_db(DatabaseEnvKind::RW); let tx = db.tx_mut().expect(ERROR_INIT_TX); let mut dup_cursor = tx.cursor_dup_write::().unwrap(); @@ -774,7 +776,7 @@ mod tests { #[test] fn db_cursor_walk() { - let env = create_test_db(DatabaseEnvKind::RW); + let (_tempdir, env) = create_test_db(DatabaseEnvKind::RW); let value = Header::default(); let key = 1u64; @@ -799,7 +801,7 @@ mod tests { #[test] fn db_cursor_walk_range() { - let db: DatabaseEnv = create_test_db(DatabaseEnvKind::RW); + let (_tempdir, db) = create_test_db(DatabaseEnvKind::RW); // PUT (0, 0), (1, 0), (2, 0), (3, 0) let tx = db.tx_mut().expect(ERROR_INIT_TX); @@ -863,7 +865,7 @@ mod tests { #[test] fn db_cursor_walk_range_on_dup_table() { - let db: DatabaseEnv = create_test_db(DatabaseEnvKind::RW); + let (_tempdir, db) = create_test_db(DatabaseEnvKind::RW); let address0 = Address::ZERO; let address1 = Address::with_last_byte(1); @@ -923,7 +925,7 @@ mod tests { #[expect(clippy::reversed_empty_ranges)] #[test] fn db_cursor_walk_range_invalid() { - let db: DatabaseEnv = create_test_db(DatabaseEnvKind::RW); + let (_tempdir, db) = create_test_db(DatabaseEnvKind::RW); // PUT (0, 0), (1, 0), (2, 0), (3, 0) let tx = db.tx_mut().expect(ERROR_INIT_TX); @@ -951,7 +953,7 @@ mod tests { #[test] fn db_walker() { - let db: DatabaseEnv = create_test_db(DatabaseEnvKind::RW); + let (_tempdir, db) = create_test_db(DatabaseEnvKind::RW); // PUT (0, 0), (1, 0), (3, 0) let tx = db.tx_mut().expect(ERROR_INIT_TX); @@ -981,7 +983,7 @@ mod tests { #[test] fn db_reverse_walker() { - let db: DatabaseEnv = create_test_db(DatabaseEnvKind::RW); + let (_tempdir, db) = create_test_db(DatabaseEnvKind::RW); // PUT (0, 0), (1, 0), (3, 0) let tx = db.tx_mut().expect(ERROR_INIT_TX); @@ -1011,7 +1013,7 @@ mod tests { #[test] fn db_walk_back() { - let db: DatabaseEnv = create_test_db(DatabaseEnvKind::RW); + let (_tempdir, db) = create_test_db(DatabaseEnvKind::RW); // PUT (0, 0), (1, 0), (3, 0) let tx = db.tx_mut().expect(ERROR_INIT_TX); @@ -1050,7 +1052,7 @@ mod tests { #[test] fn db_cursor_seek_exact_or_previous_key() { - let db: DatabaseEnv = create_test_db(DatabaseEnvKind::RW); + let (_tempdir, db) = create_test_db(DatabaseEnvKind::RW); // PUT let tx = db.tx_mut().expect(ERROR_INIT_TX); @@ -1074,7 +1076,7 @@ mod tests { #[test] fn db_cursor_insert() { - let db: DatabaseEnv = create_test_db(DatabaseEnvKind::RW); + let (_tempdir, db) = create_test_db(DatabaseEnvKind::RW); // PUT let tx = db.tx_mut().expect(ERROR_INIT_TX); @@ -1114,7 +1116,7 @@ mod tests { #[test] fn db_cursor_insert_dup() { - let db: DatabaseEnv = create_test_db(DatabaseEnvKind::RW); + let (_tempdir, db) = create_test_db(DatabaseEnvKind::RW); let tx = db.tx_mut().expect(ERROR_INIT_TX); let mut dup_cursor = tx.cursor_dup_write::().unwrap(); @@ -1132,7 +1134,7 @@ mod tests { #[test] fn db_cursor_delete_current_non_existent() { - let db: DatabaseEnv = create_test_db(DatabaseEnvKind::RW); + let (_tempdir, db) = create_test_db(DatabaseEnvKind::RW); let tx = db.tx_mut().expect(ERROR_INIT_TX); let key1 = Address::with_last_byte(1); @@ -1162,7 +1164,7 @@ mod tests { #[test] fn db_cursor_insert_wherever_cursor_is() { - let db: DatabaseEnv = create_test_db(DatabaseEnvKind::RW); + let (_tempdir, db) = create_test_db(DatabaseEnvKind::RW); let tx = db.tx_mut().expect(ERROR_INIT_TX); // PUT @@ -1195,7 +1197,7 @@ mod tests { #[test] fn db_cursor_append() { - let db: DatabaseEnv = create_test_db(DatabaseEnvKind::RW); + let (_tempdir, db) = create_test_db(DatabaseEnvKind::RW); // PUT let tx = db.tx_mut().expect(ERROR_INIT_TX); @@ -1222,7 +1224,7 @@ mod tests { #[test] fn db_cursor_append_failure() { - let db: DatabaseEnv = create_test_db(DatabaseEnvKind::RW); + let (_tempdir, db) = create_test_db(DatabaseEnvKind::RW); // PUT let tx = db.tx_mut().expect(ERROR_INIT_TX); @@ -1257,7 +1259,7 @@ mod tests { #[test] fn db_cursor_upsert() { - let db: DatabaseEnv = create_test_db(DatabaseEnvKind::RW); + let (_tempdir, db) = create_test_db(DatabaseEnvKind::RW); let tx = db.tx_mut().expect(ERROR_INIT_TX); let mut cursor = tx.cursor_write::().unwrap(); @@ -1292,7 +1294,7 @@ mod tests { #[test] fn db_cursor_dupsort_append() { - let db: DatabaseEnv = create_test_db(DatabaseEnvKind::RW); + let (_tempdir, db) = create_test_db(DatabaseEnvKind::RW); let transition_id = 2; @@ -1356,7 +1358,8 @@ mod tests { #[test] fn db_closure_put_get() { - let path = TempDir::new().expect(ERROR_TEMPDIR).keep(); + let tempdir = TempDir::new().expect(ERROR_TEMPDIR); + let path = tempdir.path(); let value = Account { nonce: 18446744073709551615, @@ -1367,7 +1370,7 @@ mod tests { .expect(ERROR_ETH_ADDRESS); { - let env = create_test_db_with_path(DatabaseEnvKind::RW, &path); + let env = create_test_db_with_path(DatabaseEnvKind::RW, path); // PUT let result = env.update(|tx| { @@ -1378,7 +1381,7 @@ mod tests { } let env = DatabaseEnv::open( - &path, + path, DatabaseEnvKind::RO, DatabaseArguments::new(ClientVersion::default()), ) @@ -1393,7 +1396,7 @@ mod tests { #[test] fn db_dup_sort() { - let env = create_test_db(DatabaseEnvKind::RW); + let (_tempdir, env) = create_test_db(DatabaseEnvKind::RW); let key = Address::from_str("0xa2c122be93b0074270ebee7f6b7292c7deb45047") .expect(ERROR_ETH_ADDRESS); @@ -1437,7 +1440,7 @@ mod tests { #[test] fn db_walk_dup_with_not_existing_key() { - let env = create_test_db(DatabaseEnvKind::RW); + let (_tempdir, env) = create_test_db(DatabaseEnvKind::RW); let key = Address::from_str("0xa2c122be93b0074270ebee7f6b7292c7deb45047") .expect(ERROR_ETH_ADDRESS); @@ -1465,7 +1468,7 @@ mod tests { #[test] fn db_iterate_over_all_dup_values() { - let env = create_test_db(DatabaseEnvKind::RW); + let (_tempdir, env) = create_test_db(DatabaseEnvKind::RW); let key1 = Address::from_str("0x1111111111111111111111111111111111111111") .expect(ERROR_ETH_ADDRESS); let key2 = Address::from_str("0x2222222222222222222222222222222222222222") @@ -1511,7 +1514,7 @@ mod tests { #[test] fn dup_value_with_same_subkey() { - let env = create_test_db(DatabaseEnvKind::RW); + let (_tempdir, env) = create_test_db(DatabaseEnvKind::RW); let key1 = Address::new([0x11; 20]); let key2 = Address::new([0x22; 20]); @@ -1554,7 +1557,7 @@ mod tests { #[test] fn db_sharded_key() { - let db: DatabaseEnv = create_test_db(DatabaseEnvKind::RW); + let (_tempdir, db) = create_test_db(DatabaseEnvKind::RW); let real_key = address!("0xa2c122be93b0074270ebee7f6b7292c7deb45047"); let shards = 5; diff --git a/crates/storage/provider/src/providers/database/mod.rs b/crates/storage/provider/src/providers/database/mod.rs index 455478325c..cf8ee2a286 100644 --- a/crates/storage/provider/src/providers/database/mod.rs +++ b/crates/storage/provider/src/providers/database/mod.rs @@ -745,9 +745,10 @@ mod tests { fn provider_factory_with_database_path() { let chain_spec = ChainSpecBuilder::mainnet().build(); let (_static_dir, static_dir_path) = create_test_static_files_dir(); - let (_, rocksdb_path) = create_test_rocksdb_dir(); + let (_rocksdb_dir, rocksdb_path) = create_test_rocksdb_dir(); + let _db_tempdir = tempfile::TempDir::new().expect(ERROR_TEMPDIR); let factory = ProviderFactory::>::new_with_database_path( - tempfile::TempDir::new().expect(ERROR_TEMPDIR).keep(), + _db_tempdir.path(), Arc::new(chain_spec), DatabaseArguments::new(Default::default()), StaticFileProvider::read_write(static_dir_path).unwrap(), @@ -785,8 +786,9 @@ mod tests { transaction_lookup: Some(PruneMode::Full), ..PruneModes::default() }; - let factory = create_test_provider_factory(); - let provider = factory.with_prune_modes(prune_modes).provider_rw().unwrap(); + // Keep factory alive until provider is dropped to prevent TempDatabase cleanup + let factory = create_test_provider_factory().with_prune_modes(prune_modes); + let provider = factory.provider_rw().unwrap(); assert_matches!(provider.insert_block(&block.clone().try_recover().unwrap()), Ok(_)); assert_matches!(provider.transaction_sender(0), Ok(None)); assert_matches!( diff --git a/crates/storage/provider/src/test_utils/mod.rs b/crates/storage/provider/src/test_utils/mod.rs index b5d16d093b..3774f18412 100644 --- a/crates/storage/provider/src/test_utils/mod.rs +++ b/crates/storage/provider/src/test_utils/mod.rs @@ -4,12 +4,7 @@ use crate::{ }; use alloy_primitives::B256; use reth_chainspec::{ChainSpec, MAINNET}; -use reth_db::{ - test_utils::{ - create_test_rocksdb_dir, create_test_rw_db, create_test_static_files_dir, TempDatabase, - }, - DatabaseEnv, -}; +use reth_db::{test_utils::TempDatabase, DatabaseEnv}; use reth_errors::ProviderResult; use reth_ethereum_engine_primitives::EthEngineTypes; use reth_node_types::NodeTypesWithDBAdapter; @@ -55,14 +50,23 @@ pub fn create_test_provider_factory_with_chain_spec( pub fn create_test_provider_factory_with_node_types( chain_spec: Arc, ) -> ProviderFactory>>> { - let (static_dir, _) = create_test_static_files_dir(); - let (rocksdb_dir, _) = create_test_rocksdb_dir(); - let db = create_test_rw_db(); - let rocksdb_path = rocksdb_dir.keep(); + // Create a single temp directory that contains all data dirs (db, static_files, rocksdb). + // TempDatabase will clean up the entire directory on drop. + let datadir_path = reth_db::test_utils::tempdir_path(); + + let static_files_path = datadir_path.join("static_files"); + let rocksdb_path = datadir_path.join("rocksdb"); + + // Create static_files directory + std::fs::create_dir_all(&static_files_path).expect("failed to create static_files dir"); + + // Create database with the datadir path so TempDatabase cleans up everything on drop + let db = reth_db::test_utils::create_test_rw_db_with_datadir(&datadir_path); + ProviderFactory::new( db, chain_spec, - StaticFileProvider::read_write(static_dir.keep()).expect("static file provider"), + StaticFileProvider::read_write(static_files_path).expect("static file provider"), RocksDBBuilder::new(&rocksdb_path) .with_default_tables() .build()