From 666f33ae6a2b814ef7e03679d4c2d8f224c85c74 Mon Sep 17 00:00:00 2001 From: nemo Date: Thu, 29 Aug 2024 14:52:04 -0400 Subject: [PATCH] feat: properly remove temporary files leftover after running tests --- fil-proofs-param/tests/paramfetch/mod.rs | 43 ++++++++++++++++--- fil-proofs-param/tests/paramfetch/session.rs | 2 +- .../tests/parampublish/prompts_to_publish.rs | 6 +++ .../tests/parampublish/read_metadata_files.rs | 4 ++ .../tests/parampublish/support/session.rs | 2 +- .../tests/parampublish/write_json_manifest.rs | 7 +++ storage-proofs-porep/tests/common.rs | 36 +++++++++++++++- storage-proofs-porep/tests/stacked_circuit.rs | 17 ++++++-- .../tests/stacked_compound.rs | 8 +++- storage-proofs-porep/tests/stacked_vanilla.rs | 23 ++++++++-- 10 files changed, 129 insertions(+), 19 deletions(-) diff --git a/fil-proofs-param/tests/paramfetch/mod.rs b/fil-proofs-param/tests/paramfetch/mod.rs index 326121af76..0efd8506f0 100644 --- a/fil-proofs-param/tests/paramfetch/mod.rs +++ b/fil-proofs-param/tests/paramfetch/mod.rs @@ -29,6 +29,14 @@ fn rand_bytes_with_blake2b() -> Result<(Vec, String), FailureError> { Ok((bytes.to_vec(), hasher.finalize().to_hex()[..32].into())) } +fn clean_up_manifest_and_parent(manifest_pbuf: &PathBuf) { + let parent_dir = std::path::Path::new(manifest_pbuf) + .parent() + .expect("failed to get parent dir"); + std::fs::remove_file(manifest_pbuf).expect("failed to remove manifest file"); + std::fs::remove_dir_all(parent_dir).expect("failed to remove parent dir"); +} + #[test] fn nothing_to_fetch_if_cache_fully_hydrated() -> Result<(), FailureError> { let mut manifest: BTreeMap = BTreeMap::new(); @@ -48,7 +56,7 @@ fn nothing_to_fetch_if_cache_fully_hydrated() -> Result<(), FailureError> { let manifest_pbuf = tmp_manifest(Some(manifest))?; - let mut session = ParamFetchSessionBuilder::new(Some(manifest_pbuf)) + let mut session = ParamFetchSessionBuilder::new(Some(manifest_pbuf.clone())) .with_session_timeout_ms(1000) .with_file_and_bytes("aaa.vk", &mut aaa_bytes) .build(); @@ -57,6 +65,9 @@ fn nothing_to_fetch_if_cache_fully_hydrated() -> Result<(), FailureError> { session.exp_string("file is up to date")?; session.exp_string("no outdated files, exiting")?; + clean_up_manifest_and_parent(&manifest_pbuf); + std::fs::remove_dir_all(session._cache_dir.path())?; + Ok(()) } @@ -75,7 +86,7 @@ fn prompts_to_download_if_file_in_manifest_is_missing() -> Result<(), FailureErr let manifest_pbuf = tmp_manifest(Some(manifest))?; - let mut session = ParamFetchSessionBuilder::new(Some(manifest_pbuf)) + let mut session = ParamFetchSessionBuilder::new(Some(manifest_pbuf.clone())) .with_session_timeout_ms(1000) .build(); @@ -84,6 +95,9 @@ fn prompts_to_download_if_file_in_manifest_is_missing() -> Result<(), FailureErr session.exp_string("Select files to be downloaded")?; session.exp_string("aaa.vk (1 KiB)")?; + clean_up_manifest_and_parent(&manifest_pbuf); + std::fs::remove_dir_all(session._cache_dir.path())?; + Ok(()) } @@ -105,7 +119,7 @@ fn prompts_to_download_if_file_checksum_does_not_match_manifest() -> Result<(), let manifest_pbuf = tmp_manifest(Some(manifest))?; - let mut session = ParamFetchSessionBuilder::new(Some(manifest_pbuf)) + let mut session = ParamFetchSessionBuilder::new(Some(manifest_pbuf.clone())) .with_session_timeout_ms(1000) .with_file_and_bytes("aaa.vk", &mut aaa_bytes) .build(); @@ -116,6 +130,9 @@ fn prompts_to_download_if_file_checksum_does_not_match_manifest() -> Result<(), session.exp_string("Select files to be downloaded")?; session.exp_string("aaa.vk (1 KiB)")?; + clean_up_manifest_and_parent(&manifest_pbuf); + std::fs::remove_dir_all(session._cache_dir.path())?; + Ok(()) } @@ -143,7 +160,7 @@ fn fetches_vk_even_if_sector_size_does_not_match() -> Result<(), FailureError> { let manifest_pbuf = tmp_manifest(Some(manifest))?; - let mut session = ParamFetchSessionBuilder::new(Some(manifest_pbuf)) + let mut session = ParamFetchSessionBuilder::new(Some(manifest_pbuf.clone())) .with_session_timeout_ms(1000) .whitelisted_sector_sizes(vec!["6666".to_string(), "4444".to_string()]) .build(); @@ -153,6 +170,9 @@ fn fetches_vk_even_if_sector_size_does_not_match() -> Result<(), FailureError> { session.exp_string("determining if file is out of date: aaa.vk")?; session.exp_string("file not found, marking for download")?; + clean_up_manifest_and_parent(&manifest_pbuf); + std::fs::remove_dir_all(session._cache_dir.path())?; + Ok(()) } @@ -165,6 +185,8 @@ fn invalid_json_path_produces_error() -> Result<(), FailureError> { session.exp_string("using json file: /invalid/path")?; session.exp_string("failed to open json file, exiting")?; + std::fs::remove_dir_all(session._cache_dir.path())?; + Ok(()) } @@ -172,15 +194,20 @@ fn invalid_json_path_produces_error() -> Result<(), FailureError> { fn invalid_json_produces_error() -> Result<(), FailureError> { let manifest_pbuf = tmp_manifest(None)?; - let mut file = File::create(&manifest_pbuf)?; - file.write_all(b"invalid json")?; + { + let mut file = File::create(&manifest_pbuf)?; + file.write_all(b"invalid json")?; + } - let mut session = ParamFetchSessionBuilder::new(Some(manifest_pbuf)) + let mut session = ParamFetchSessionBuilder::new(Some(manifest_pbuf.clone())) .with_session_timeout_ms(1000) .build(); session.exp_string("failed to parse json file, exiting")?; + clean_up_manifest_and_parent(&manifest_pbuf); + std::fs::remove_dir_all(session._cache_dir.path())?; + Ok(()) } @@ -203,5 +230,7 @@ fn no_json_path_uses_default_manifest() -> Result<(), FailureError> { ))?; } + std::fs::remove_dir_all(session._cache_dir.path())?; + Ok(()) } diff --git a/fil-proofs-param/tests/paramfetch/session.rs b/fil-proofs-param/tests/paramfetch/session.rs index 0fdb4f9a4b..9397a9cd91 100644 --- a/fil-proofs-param/tests/paramfetch/session.rs +++ b/fil-proofs-param/tests/paramfetch/session.rs @@ -112,7 +112,7 @@ impl ParamFetchSessionBuilder { /// An active pseudoterminal (pty) used to interact with paramfetch. pub struct ParamFetchSession { pty_session: PtyReplSession, - _cache_dir: TempDir, + pub _cache_dir: TempDir, } impl ParamFetchSession { diff --git a/fil-proofs-param/tests/parampublish/prompts_to_publish.rs b/fil-proofs-param/tests/parampublish/prompts_to_publish.rs index b290f020a6..938076fb44 100644 --- a/fil-proofs-param/tests/parampublish/prompts_to_publish.rs +++ b/fil-proofs-param/tests/parampublish/prompts_to_publish.rs @@ -22,6 +22,8 @@ fn ignores_files_unrecognized_extensions() -> Result<(), FailureError> { session.send_line("")?; session.exp_string("no params selected, exiting")?; + std::fs::remove_dir_all(session._cache_dir.path())?; + Ok(()) } @@ -47,6 +49,8 @@ fn displays_sector_size_in_prompt() -> Result<(), FailureError> { session.send_line("")?; session.exp_string("no params selected, exiting")?; + std::fs::remove_dir_all(session._cache_dir.path())?; + Ok(()) } @@ -59,5 +63,7 @@ fn no_assets_no_prompt() -> Result<(), FailureError> { session.exp_string("found 0 param files in cache dir")?; session.exp_string("no file triples found, exiting")?; + std::fs::remove_dir_all(session._cache_dir.path())?; + Ok(()) } diff --git a/fil-proofs-param/tests/parampublish/read_metadata_files.rs b/fil-proofs-param/tests/parampublish/read_metadata_files.rs index 2689cb6ae1..ddcc2db573 100644 --- a/fil-proofs-param/tests/parampublish/read_metadata_files.rs +++ b/fil-proofs-param/tests/parampublish/read_metadata_files.rs @@ -15,6 +15,8 @@ fn fails_if_missing_metadata_file() -> Result<(), FailureError> { session.exp_string("found 2 param files in cache dir")?; session.exp_string("no file triples found, exiting")?; + std::fs::remove_dir_all(session._cache_dir.path())?; + Ok(()) } @@ -34,5 +36,7 @@ fn fails_if_malformed_metadata_file() -> Result<(), FailureError> { session.exp_string("failed to parse .meta file")?; session.exp_string("exiting")?; + std::fs::remove_dir_all(session._cache_dir.path())?; + Ok(()) } diff --git a/fil-proofs-param/tests/parampublish/support/session.rs b/fil-proofs-param/tests/parampublish/support/session.rs index 5b050172e4..a31a23d6fa 100644 --- a/fil-proofs-param/tests/parampublish/support/session.rs +++ b/fil-proofs-param/tests/parampublish/support/session.rs @@ -159,7 +159,7 @@ impl ParamPublishSessionBuilder { /// An active pseudoterminal (pty) used to interact with parampublish. pub struct ParamPublishSession { pty_session: PtyReplSession, - _cache_dir: TempDir, + pub _cache_dir: TempDir, } impl ParamPublishSession { diff --git a/fil-proofs-param/tests/parampublish/write_json_manifest.rs b/fil-proofs-param/tests/parampublish/write_json_manifest.rs index 67e39a5eb5..a7bda1205d 100644 --- a/fil-proofs-param/tests/parampublish/write_json_manifest.rs +++ b/fil-proofs-param/tests/parampublish/write_json_manifest.rs @@ -62,6 +62,13 @@ fn writes_json_manifest() -> Result<(), failure::Error> { } } + let parent_dir = std::path::Path::new(&manifest_path) + .parent() + .expect("failed to get parent dir"); + std::fs::remove_file(&manifest_path)?; + std::fs::remove_dir(parent_dir)?; + std::fs::remove_dir_all(session._cache_dir.path())?; + Ok(()) } diff --git a/storage-proofs-porep/tests/common.rs b/storage-proofs-porep/tests/common.rs index c254e2fa6e..94e04d9c62 100644 --- a/storage-proofs-porep/tests/common.rs +++ b/storage-proofs-porep/tests/common.rs @@ -1,9 +1,41 @@ -use std::path::PathBuf; +use std::fs::remove_file; +use std::io::Result; +use std::path::{Path, PathBuf}; use filecoin_hashers::Hasher; -use storage_proofs_core::{data::Data, merkle::MerkleTreeTrait}; +use merkletree::store::StoreConfig; +use storage_proofs_core::{ + cache_key::CacheKey, + data::Data, + merkle::{get_base_tree_count, split_config, MerkleTreeTrait}, +}; use storage_proofs_porep::stacked::{PersistentAux, PublicParams, StackedDrg, Tau, TemporaryAux}; +// This method should ONLY be used in purposed test code. +#[allow(dead_code)] +pub fn remove_replica_and_tree_r(cache_path: &Path) -> Result<()> { + let replica_path = cache_path.join("replica-path"); + let tree_r_last_config = StoreConfig { + path: cache_path.to_path_buf(), + id: CacheKey::CommRLastTree.to_string(), + size: Some(0), + rows_to_discard: 0, + }; + let tree_count = get_base_tree_count::(); + if tree_count > 1 { + let configs = + split_config(tree_r_last_config, tree_count).expect("Failed to split configs"); + for config in configs { + let cur_path = StoreConfig::data_path(&config.path, &config.id); + remove_file(cur_path).expect("Failed to remove TreeR"); + } + } else { + let cur_path = StoreConfig::data_path(&tree_r_last_config.path, &tree_r_last_config.id); + remove_file(cur_path).expect("Failed to remove TreeR"); + } + remove_file(replica_path) +} + #[allow(clippy::type_complexity)] pub fn transform_and_replicate_layers( pp: &PublicParams, diff --git a/storage-proofs-porep/tests/stacked_circuit.rs b/storage-proofs-porep/tests/stacked_circuit.rs index 314878dc41..205b20f603 100644 --- a/storage-proofs-porep/tests/stacked_circuit.rs +++ b/storage-proofs-porep/tests/stacked_circuit.rs @@ -92,9 +92,12 @@ fn test_stacked_porep_circuit( replica_path.clone(), ); - let mut copied = vec![0; data.len()]; - copied.copy_from_slice(&mmapped_data); - assert_ne!(data, copied, "replication did not change data"); + { + let mut copied = vec![0; data.len()]; + copied.copy_from_slice(&mmapped_data); + assert_ne!(data, copied, "replication did not change data"); + } + drop(mmapped_data); let seed = rng.gen(); let pub_inputs = @@ -125,6 +128,10 @@ fn test_stacked_porep_circuit( // Discard cached MTs that are no longer needed. stacked::clear_cache_dir(cache_dir.path()).expect("cached files delete failed"); + // Discard normally permanent files no longer needed in testing. + common::remove_replica_and_tree_r::(cache_dir.path()) + .expect("failed to remove replica and tree_r"); + { // Verify that MetricCS returns the same metrics as TestConstraintSystem. let mut cs = MetricCS::::new(); @@ -177,5 +184,7 @@ fn test_stacked_porep_circuit( "inputs are not the same length" ); - cache_dir.close().expect("Failed to remove cache dir"); + if std::fs::remove_dir(cache_dir.path()).is_ok() && cache_dir.path().exists() { + let _ = cache_dir.close(); + } } diff --git a/storage-proofs-porep/tests/stacked_compound.rs b/storage-proofs-porep/tests/stacked_compound.rs index c288ef51a4..dc365c2290 100644 --- a/storage-proofs-porep/tests/stacked_compound.rs +++ b/storage-proofs-porep/tests/stacked_compound.rs @@ -170,6 +170,10 @@ fn test_stacked_compound() { // Discard cached MTs that are no longer needed. stacked::clear_cache_dir(cache_dir.path()).expect("cached files delete failed"); + // Discard normally permanent files no longer needed in testing. + common::remove_replica_and_tree_r::(cache_dir.path()) + .expect("failed to remove replica and tree_r"); + let proofs = StackedCompound::prove( &public_params, &public_inputs, @@ -200,5 +204,7 @@ fn test_stacked_compound() { assert!(verified); - cache_dir.close().expect("Failed to remove cache dir"); + if std::fs::remove_dir(cache_dir.path()).is_ok() && cache_dir.path().exists() { + let _ = cache_dir.close(); + } } diff --git a/storage-proofs-porep/tests/stacked_vanilla.rs b/storage-proofs-porep/tests/stacked_vanilla.rs index 4c340ee041..988757e2ea 100644 --- a/storage-proofs-porep/tests/stacked_vanilla.rs +++ b/storage-proofs-porep/tests/stacked_vanilla.rs @@ -164,7 +164,16 @@ fn test_extract_all() { assert_eq!(data, mmapped_data.as_ref()); - cache_dir.close().expect("Failed to remove cache dir"); + // Discard cached MTs that are no longer needed. + stacked::clear_cache_dir(cache_dir.path()).expect("cached files delete failed"); + + // Discard normally permanent files no longer needed in testing. + common::remove_replica_and_tree_r::(cache_dir.path()) + .expect("failed to remove replica and tree_r"); + + if std::fs::remove_dir(cache_dir.path()).is_ok() && cache_dir.path().exists() { + let _ = cache_dir.close(); + } } #[test] @@ -281,7 +290,9 @@ fn test_stacked_porep_resume_seal() { assert_eq!(data, mmapped_data1.as_ref()); - cache_dir.close().expect("Failed to remove cache dir"); + if std::fs::remove_dir(cache_dir.path()).is_ok() && cache_dir.path().exists() { + let _ = cache_dir.close(); + } } table_tests! { @@ -405,9 +416,15 @@ fn test_prove_verify(n: usize, challenges: Chal // Discard cached MTs that are no longer needed. stacked::clear_cache_dir(cache_dir.path()).expect("cached files delete failed"); + // Discard normally permanent files no longer needed in testing. + common::remove_replica_and_tree_r::(cache_dir.path()) + .expect("failed to remove replica and tree_r"); + assert!(proofs_are_valid); - cache_dir.close().expect("Failed to remove cache dir"); + if std::fs::remove_dir(cache_dir.path()).is_ok() && cache_dir.path().exists() { + let _ = cache_dir.close(); + } } // We are seeing a bug, in which setup never terminates for some sector sizes. This test is to