Skip to content

Commit

Permalink
feat: properly remove temporary files leftover after running tests
Browse files Browse the repository at this point in the history
  • Loading branch information
cryptonemo committed Oct 2, 2024
1 parent 5200262 commit df83b1a
Show file tree
Hide file tree
Showing 10 changed files with 129 additions and 19 deletions.
43 changes: 36 additions & 7 deletions fil-proofs-param/tests/paramfetch/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,14 @@ fn rand_bytes_with_blake2b() -> Result<(Vec<u8>, 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<String, ParameterData> = BTreeMap::new();
Expand All @@ -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();
Expand All @@ -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(())
}

Expand All @@ -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();

Expand All @@ -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(())
}

Expand All @@ -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();
Expand All @@ -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(())
}

Expand Down Expand Up @@ -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();
Expand All @@ -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(())
}

Expand All @@ -165,22 +185,29 @@ 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(())
}

#[test]
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(())
}

Expand All @@ -203,5 +230,7 @@ fn no_json_path_uses_default_manifest() -> Result<(), FailureError> {
))?;
}

std::fs::remove_dir_all(session._cache_dir.path())?;

Ok(())
}
2 changes: 1 addition & 1 deletion fil-proofs-param/tests/paramfetch/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
6 changes: 6 additions & 0 deletions fil-proofs-param/tests/parampublish/prompts_to_publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}

Expand All @@ -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(())
}

Expand All @@ -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(())
}
4 changes: 4 additions & 0 deletions fil-proofs-param/tests/parampublish/read_metadata_files.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}

Expand All @@ -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(())
}
2 changes: 1 addition & 1 deletion fil-proofs-param/tests/parampublish/support/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
7 changes: 7 additions & 0 deletions fil-proofs-param/tests/parampublish/write_json_manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}

Expand Down
36 changes: 34 additions & 2 deletions storage-proofs-porep/tests/common.rs
Original file line number Diff line number Diff line change
@@ -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<Tree: MerkleTreeTrait + 'static>(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::<Tree>();
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<Tree: 'static + MerkleTreeTrait, G: 'static + Hasher>(
pp: &PublicParams<Tree>,
Expand Down
17 changes: 13 additions & 4 deletions storage-proofs-porep/tests/stacked_circuit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,12 @@ fn test_stacked_porep_circuit<Tree: MerkleTreeTrait + 'static>(
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 =
Expand Down Expand Up @@ -125,6 +128,10 @@ fn test_stacked_porep_circuit<Tree: MerkleTreeTrait + 'static>(
// 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::<Tree>(cache_dir.path())
.expect("failed to remove replica and tree_r");

{
// Verify that MetricCS returns the same metrics as TestConstraintSystem.
let mut cs = MetricCS::<Fr>::new();
Expand Down Expand Up @@ -177,5 +184,7 @@ fn test_stacked_porep_circuit<Tree: MerkleTreeTrait + 'static>(
"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();
}
}
8 changes: 7 additions & 1 deletion storage-proofs-porep/tests/stacked_compound.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,10 @@ fn test_stacked_compound<Tree: 'static + MerkleTreeTrait>() {
// 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::<Tree>(cache_dir.path())
.expect("failed to remove replica and tree_r");

let proofs = StackedCompound::prove(
&public_params,
&public_inputs,
Expand Down Expand Up @@ -200,5 +204,7 @@ fn test_stacked_compound<Tree: 'static + MerkleTreeTrait>() {

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();
}
}
23 changes: 20 additions & 3 deletions storage-proofs-porep/tests/stacked_vanilla.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,16 @@ fn test_extract_all<Tree: 'static + MerkleTreeTrait>() {

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::<Tree>(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]
Expand Down Expand Up @@ -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! {
Expand Down Expand Up @@ -405,9 +416,15 @@ fn test_prove_verify<Tree: 'static + MerkleTreeTrait>(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::<Tree>(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
Expand Down

0 comments on commit df83b1a

Please sign in to comment.