From c528a86284efc9490fd962d0017a9d4cb3b5ab4f Mon Sep 17 00:00:00 2001 From: Ignacio Duart Date: Wed, 8 Nov 2023 16:23:42 +0100 Subject: [PATCH 1/2] Don't remove temp dir prematurely in tests --- .gitignore | 3 +- crates/core/src/runtime/delegate.rs | 6 +- crates/core/src/runtime/tests/contract.rs | 75 +++++++++++++++++------ crates/core/src/runtime/tests/mod.rs | 23 +++++-- crates/core/src/runtime/tests/time.rs | 15 +++-- tests/test-contract-1/Cargo.lock | 4 -- tests/test-contract-2/Cargo.lock | 4 -- tests/test-delegate-1/Cargo.lock | 4 -- 8 files changed, 89 insertions(+), 45 deletions(-) diff --git a/.gitignore b/.gitignore index 16868ce09..936bda15a 100644 --- a/.gitignore +++ b/.gitignore @@ -19,7 +19,8 @@ target/ .idea/* **/.vscode/* +# Other config files config.toml +.env .rustc* -expanded_code.rs diff --git a/crates/core/src/runtime/delegate.rs b/crates/core/src/runtime/delegate.rs index 0e619545a..06b3bc6b3 100644 --- a/crates/core/src/runtime/delegate.rs +++ b/crates/core/src/runtime/delegate.rs @@ -416,7 +416,7 @@ mod test { fn setup_runtime( name: &str, - ) -> Result<(DelegateContainer, Runtime), Box> { + ) -> Result<(DelegateContainer, Runtime, TempDir), Box> { // let _ = tracing_subscriber::fmt().with_env_filter("info").try_init(); let temp_dir = TempDir::new().expect("Failed to create a temporary directory"); let contracts_dir = temp_dir.path().join("contracts"); @@ -446,7 +446,7 @@ mod test { .secret_store .register_delegate(delegate.key().clone(), cipher, nonce); - Ok((delegate, runtime)) + Ok((delegate, runtime, temp_dir)) } #[test] @@ -455,7 +455,7 @@ mod test { Arc::new(ContractCode::from(vec![1])), Parameters::from(vec![]), ); - let (delegate, mut runtime) = setup_runtime(TEST_DELEGATE_1)?; + let (delegate, mut runtime, _temp_dir) = setup_runtime(TEST_DELEGATE_1)?; let app = ContractInstanceId::try_from(contract.key.to_string()).unwrap(); // CreateInboxRequest message parts diff --git a/crates/core/src/runtime/tests/contract.rs b/crates/core/src/runtime/tests/contract.rs index 40eb96a98..914a20133 100644 --- a/crates/core/src/runtime/tests/contract.rs +++ b/crates/core/src/runtime/tests/contract.rs @@ -1,17 +1,25 @@ use freenet_stdlib::prelude::*; +use crate::runtime::tests::TestSetup; + use super::super::contract::*; -use super::super::{tests::setup_test_contract, Runtime}; +use super::super::Runtime; const TEST_CONTRACT_1: &str = "test_contract_1"; #[test] fn validate_state() -> Result<(), Box> { - let (contracts, delegates, secrets, key) = setup_test_contract(TEST_CONTRACT_1)?; - let mut runtime = Runtime::build(contracts, delegates, secrets, false).unwrap(); + let TestSetup { + contract_store, + delegate_store, + secrets_store, + contract_key, + temp_dir, + } = super::setup_test_contract(TEST_CONTRACT_1)?; + let mut runtime = Runtime::build(contract_store, delegate_store, secrets_store, false).unwrap(); let is_valid = runtime.validate_state( - &key, + &contract_key, &Parameters::from([].as_ref()), &WrappedState::new(vec![1, 2, 3, 4]), &Default::default(), @@ -19,46 +27,58 @@ fn validate_state() -> Result<(), Box> { assert!(is_valid == ValidateResult::Valid); let not_valid = runtime.validate_state( - &key, + &contract_key, &Parameters::from([].as_ref()), &WrappedState::new(vec![1, 0, 0, 1]), &Default::default(), )?; assert!(matches!(not_valid, ValidateResult::RequestRelated(_))); - + std::mem::drop(temp_dir); Ok(()) } #[test] fn validate_delta() -> Result<(), Box> { - let (contracts, delegates, secrets, key) = setup_test_contract(TEST_CONTRACT_1)?; - let mut runtime = Runtime::build(contracts, delegates, secrets, false).unwrap(); + let TestSetup { + contract_store, + delegate_store, + secrets_store, + contract_key, + temp_dir, + } = super::setup_test_contract(TEST_CONTRACT_1)?; + let mut runtime = Runtime::build(contract_store, delegate_store, secrets_store, false).unwrap(); let is_valid = runtime.validate_delta( - &key, + &contract_key, &Parameters::from([].as_ref()), &StateDelta::from([1, 2, 3, 4].as_ref()), )?; assert!(is_valid); let not_valid = !runtime.validate_delta( - &key, + &contract_key, &Parameters::from([].as_ref()), &StateDelta::from([1, 0, 0, 1].as_ref()), )?; assert!(not_valid); - + std::mem::drop(temp_dir); Ok(()) } #[test] fn update_state() -> Result<(), Box> { - let (contracts, delegates, secrets, key) = setup_test_contract(TEST_CONTRACT_1)?; - let mut runtime = Runtime::build(contracts, delegates, secrets, false).unwrap(); + let TestSetup { + contract_store, + delegate_store, + secrets_store, + contract_key, + temp_dir, + } = super::setup_test_contract(TEST_CONTRACT_1)?; + let mut runtime = Runtime::build(contract_store, delegate_store, secrets_store, false).unwrap(); let new_state = runtime .update_state( - &key, + &contract_key, &Parameters::from([].as_ref()), &WrappedState::new(vec![5, 2, 3]), &[StateDelta::from([4].as_ref()).into()], @@ -66,35 +86,50 @@ fn update_state() -> Result<(), Box> { .unwrap_valid(); assert!(new_state.as_ref().len() == 4); assert!(new_state.as_ref()[3] == 4); + std::mem::drop(temp_dir); Ok(()) } #[test] fn summarize_state() -> Result<(), Box> { - let (contracts, delegates, secrets, key) = setup_test_contract(TEST_CONTRACT_1)?; - let mut runtime = Runtime::build(contracts, delegates, secrets, false).unwrap(); + let TestSetup { + contract_store, + delegate_store, + secrets_store, + contract_key, + temp_dir, + } = super::setup_test_contract(TEST_CONTRACT_1)?; + let mut runtime = Runtime::build(contract_store, delegate_store, secrets_store, false).unwrap(); let summary = runtime.summarize_state( - &key, + &contract_key, &Parameters::from([].as_ref()), &WrappedState::new(vec![5, 2, 3, 4]), )?; assert_eq!(summary.as_ref(), &[5, 2, 3]); + std::mem::drop(temp_dir); Ok(()) } #[test] fn get_state_delta() -> Result<(), Box> { - let (contracts, delegates, secrets, key) = setup_test_contract(TEST_CONTRACT_1)?; - let mut runtime = Runtime::build(contracts, delegates, secrets, false).unwrap(); + let TestSetup { + contract_store, + delegate_store, + secrets_store, + contract_key, + temp_dir, + } = super::setup_test_contract(TEST_CONTRACT_1)?; + let mut runtime = Runtime::build(contract_store, delegate_store, secrets_store, false).unwrap(); let delta = runtime.get_state_delta( - &key, + &contract_key, &Parameters::from([].as_ref()), &WrappedState::new(vec![5, 2, 3, 4]), &StateSummary::from([2, 3].as_ref()), )?; assert!(delta.as_ref().len() == 1); assert!(delta.as_ref()[0] == 4); + std::mem::drop(temp_dir); Ok(()) } diff --git a/crates/core/src/runtime/tests/mod.rs b/crates/core/src/runtime/tests/mod.rs index 5ef2f1807..274f97b90 100644 --- a/crates/core/src/runtime/tests/mod.rs +++ b/crates/core/src/runtime/tests/mod.rs @@ -47,9 +47,16 @@ pub(crate) fn get_test_module(name: &str) -> Result, Box Result<(ContractStore, DelegateStore, SecretsStore, ContractKey), Box> { +pub(crate) struct TestSetup { + #[allow(unused)] + temp_dir: TempDir, + contract_store: ContractStore, + delegate_store: DelegateStore, + secrets_store: SecretsStore, + contract_key: ContractKey, +} + +pub(crate) fn setup_test_contract(name: &str) -> Result> { // let _ = tracing_subscriber::fmt().with_env_filter("info").try_init(); let temp_dir = TempDir::new().expect("Failed to create a temporary directory"); @@ -61,7 +68,13 @@ pub(crate) fn setup_test_contract( vec![].into(), ); let contract = ContractContainer::Wasm(ContractWasmAPIVersion::V1(contract_bytes)); - let key = contract.key(); + let contract_key = contract.key(); contract_store.store_contract(contract)?; - Ok((contract_store, delegate_store, secrets_store, key)) + Ok(TestSetup { + temp_dir, + contract_store, + delegate_store, + secrets_store, + contract_key, + }) } diff --git a/crates/core/src/runtime/tests/time.rs b/crates/core/src/runtime/tests/time.rs index 03e950281..47dc4c6c3 100644 --- a/crates/core/src/runtime/tests/time.rs +++ b/crates/core/src/runtime/tests/time.rs @@ -2,19 +2,26 @@ use wasmer::TypedFunction; -use super::super::Runtime; +use super::{super::Runtime, TestSetup}; #[test] fn now() -> Result<(), Box> { - let (contracts, delegates, secrets, key) = super::setup_test_contract("test_contract_2")?; - let mut runtime = Runtime::build(contracts, delegates, secrets, false).unwrap(); + let TestSetup { + contract_store, + delegate_store, + secrets_store, + contract_key, + temp_dir, + } = super::setup_test_contract("test_contract_2")?; + let mut runtime = Runtime::build(contract_store, delegate_store, secrets_store, false).unwrap(); - let module = runtime.prepare_contract_call(&key, &vec![].into(), 1_000)?; + let module = runtime.prepare_contract_call(&contract_key, &vec![].into(), 1_000)?; let f: TypedFunction<(), ()> = module .instance .exports .get_function("time_func")? .typed(&runtime.wasm_store)?; f.call(&mut runtime.wasm_store)?; + std::mem::drop(temp_dir); Ok(()) } diff --git a/tests/test-contract-1/Cargo.lock b/tests/test-contract-1/Cargo.lock index fe1ff01f1..104bda63c 100644 --- a/tests/test-contract-1/Cargo.lock +++ b/tests/test-contract-1/Cargo.lock @@ -37,9 +37,6 @@ name = "arrayvec" version = "0.7.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "96d30a06541fbafbc7f82ed10c06164cfbd2c401138f6addd8404629c4b16711" -dependencies = [ - "serde", -] [[package]] name = "autocfg" @@ -252,7 +249,6 @@ dependencies = [ name = "freenet-stdlib" version = "0.0.8" dependencies = [ - "arrayvec", "bincode", "blake3", "bs58", diff --git a/tests/test-contract-2/Cargo.lock b/tests/test-contract-2/Cargo.lock index 58690d79b..df4d71fd4 100644 --- a/tests/test-contract-2/Cargo.lock +++ b/tests/test-contract-2/Cargo.lock @@ -37,9 +37,6 @@ name = "arrayvec" version = "0.7.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "96d30a06541fbafbc7f82ed10c06164cfbd2c401138f6addd8404629c4b16711" -dependencies = [ - "serde", -] [[package]] name = "autocfg" @@ -252,7 +249,6 @@ dependencies = [ name = "freenet-stdlib" version = "0.0.8" dependencies = [ - "arrayvec", "bincode", "blake3", "bs58", diff --git a/tests/test-delegate-1/Cargo.lock b/tests/test-delegate-1/Cargo.lock index e4698056d..9f7116812 100644 --- a/tests/test-delegate-1/Cargo.lock +++ b/tests/test-delegate-1/Cargo.lock @@ -37,9 +37,6 @@ name = "arrayvec" version = "0.7.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "96d30a06541fbafbc7f82ed10c06164cfbd2c401138f6addd8404629c4b16711" -dependencies = [ - "serde", -] [[package]] name = "autocfg" @@ -252,7 +249,6 @@ dependencies = [ name = "freenet-stdlib" version = "0.0.8" dependencies = [ - "arrayvec", "bincode", "blake3", "bs58", From 7ea1067decd2f6b164e92d80474c38fa7e47d122 Mon Sep 17 00:00:00 2001 From: Ignacio Duart Date: Thu, 9 Nov 2023 10:44:02 +0100 Subject: [PATCH 2/2] Actually guarantee random temp dirs (avoid fixed seed rng) --- crates/core/src/runtime/contract_store.rs | 4 +--- crates/core/src/runtime/delegate.rs | 10 ++++++---- crates/core/src/runtime/store.rs | 9 +++++---- crates/core/src/runtime/tests/mod.rs | 7 ++++--- crates/core/src/util.rs | 18 ++++++++++++++++++ 5 files changed, 34 insertions(+), 14 deletions(-) diff --git a/crates/core/src/runtime/contract_store.rs b/crates/core/src/runtime/contract_store.rs index c79b1254a..4478e14db 100644 --- a/crates/core/src/runtime/contract_store.rs +++ b/crates/core/src/runtime/contract_store.rs @@ -215,13 +215,11 @@ impl ContractStore { #[cfg(test)] mod test { - use tempfile::TempDir; - use super::*; #[test] fn store_and_load() -> Result<(), Box> { - let contract_dir = TempDir::new()?; + let contract_dir = crate::util::tests_util::get_temp_dir(); std::fs::create_dir_all(contract_dir.path())?; let mut store = ContractStore::new(contract_dir.path().into(), 10_000)?; let contract = WrappedContract::new( diff --git a/crates/core/src/runtime/delegate.rs b/crates/core/src/runtime/delegate.rs index 06b3bc6b3..a61d37691 100644 --- a/crates/core/src/runtime/delegate.rs +++ b/crates/core/src/runtime/delegate.rs @@ -390,7 +390,8 @@ mod test { use freenet_stdlib::prelude::*; use serde::{Deserialize, Serialize}; use std::sync::Arc; - use tempfile::TempDir; + + use crate::util::tests_util::get_temp_dir; use super::super::{delegate_store::DelegateStore, ContractStore, SecretsStore}; use super::*; @@ -416,9 +417,9 @@ mod test { fn setup_runtime( name: &str, - ) -> Result<(DelegateContainer, Runtime, TempDir), Box> { + ) -> Result<(DelegateContainer, Runtime, tempfile::TempDir), Box> { // let _ = tracing_subscriber::fmt().with_env_filter("info").try_init(); - let temp_dir = TempDir::new().expect("Failed to create a temporary directory"); + let temp_dir = get_temp_dir(); let contracts_dir = temp_dir.path().join("contracts"); let delegates_dir = temp_dir.path().join("delegates"); let secrets_dir = temp_dir.path().join("secrets"); @@ -455,7 +456,7 @@ mod test { Arc::new(ContractCode::from(vec![1])), Parameters::from(vec![]), ); - let (delegate, mut runtime, _temp_dir) = setup_runtime(TEST_DELEGATE_1)?; + let (delegate, mut runtime, temp_dir) = setup_runtime(TEST_DELEGATE_1)?; let app = ContractInstanceId::try_from(contract.key.to_string()).unwrap(); // CreateInboxRequest message parts @@ -489,6 +490,7 @@ mod test { outbound.get(0), Some(OutboundDelegateMsg::ApplicationMessage(msg)) if *msg.payload == expected_payload )); + std::mem::drop(temp_dir); Ok(()) } } diff --git a/crates/core/src/runtime/store.rs b/crates/core/src/runtime/store.rs index ca0a7c967..195cf3aaa 100644 --- a/crates/core/src/runtime/store.rs +++ b/crates/core/src/runtime/store.rs @@ -337,9 +337,10 @@ mod tests { sync::{Arc, Barrier}, }; + use crate::util::tests_util::get_temp_dir; + use super::*; use dashmap::DashMap; - use tempfile::TempDir; struct TestStore1; @@ -375,7 +376,7 @@ mod tests { #[test] fn test_store() { - let temp_dir = TempDir::new().expect("Failed to create a temporary directory"); + let temp_dir = crate::util::tests_util::get_temp_dir(); let contract_keys_file_path = temp_dir.path().join("contract_keys"); let delegate_keys_file_path = temp_dir.path().join("delegate_keys"); @@ -453,7 +454,7 @@ mod tests { fn test_concurrent_updates() { const NUM_THREADS: usize = 4; - let temp_dir = TempDir::new().expect("Failed to create a temporary directory"); + let temp_dir = get_temp_dir(); let contract_keys_file_path = temp_dir.path().join("contract_keys"); std::fs::File::create(&contract_keys_file_path).expect("Failed to create file"); @@ -513,7 +514,7 @@ mod tests { #[test] fn test_concurrent_compaction() { - let temp_dir = TempDir::new().expect("Failed to create temp dir"); + let temp_dir = get_temp_dir(); let key_file_path = temp_dir.path().join("data.dat"); std::fs::File::create(&key_file_path).expect("Failed to create file"); diff --git a/crates/core/src/runtime/tests/mod.rs b/crates/core/src/runtime/tests/mod.rs index 274f97b90..3cebbd8cc 100644 --- a/crates/core/src/runtime/tests/mod.rs +++ b/crates/core/src/runtime/tests/mod.rs @@ -7,7 +7,8 @@ use std::{ use freenet_stdlib::prelude::{ ContractCode, ContractContainer, ContractKey, ContractWasmAPIVersion, WrappedContract, }; -use tempfile::TempDir; + +use crate::util::tests_util::get_temp_dir; use super::{ContractStore, DelegateStore, SecretsStore}; @@ -49,7 +50,7 @@ pub(crate) fn get_test_module(name: &str) -> Result, Box Result> { // let _ = tracing_subscriber::fmt().with_env_filter("info").try_init(); - let temp_dir = TempDir::new().expect("Failed to create a temporary directory"); + let temp_dir = get_temp_dir(); let mut contract_store = ContractStore::new(temp_dir.path().join("contract"), 10_000)?; let delegate_store = DelegateStore::new(temp_dir.path().join("delegate"), 10_000)?; diff --git a/crates/core/src/util.rs b/crates/core/src/util.rs index 410d2ea4f..5fe45fd2c 100644 --- a/crates/core/src/util.rs +++ b/crates/core/src/util.rs @@ -236,3 +236,21 @@ impl<'x> Contains for &'x [&PeerKey] { self.contains(&target) } } + +#[cfg(test)] +pub mod tests_util { + use rand::Rng; + use tempfile::TempDir; + + /// Use this to guarantee unique directory names in case you are running multiple tests in parallel. + pub fn get_temp_dir() -> TempDir { + tempfile::Builder::new() + .suffix( + &(0..8) + .map(|_| rand::thread_rng().gen::()) + .collect::(), + ) + .tempdir() + .expect("Failed to create a temporary directory") + } +}