feat: properly remove temporary files leftover after running tests (#1762)

* feat: properly remove temporary files leftover after running tests
* feat: only test snarkpack v2 in api version 1.2.x

Author: cryptonemo

fil-proofs-param/tests/paramfetch/mod.rs

@@ -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();
@@ -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,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(())
 }
 
@@ -203,5 +230,7 @@ fn no_json_path_uses_default_manifest() -> Result<(), FailureError> {
         ))?;
     }
 
+    std::fs::remove_dir_all(session._cache_dir.path())?;
+
     Ok(())
 }

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 {

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(())
 }

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(())
 }

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 {

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(())
 }
 

filecoin-proofs/tests/api.rs

@@ -943,10 +943,16 @@ fn aggregate_seal_proofs<Tree: 'static + MerkleTreeTrait>(
     let mut prover_id = [0u8; 32];
     prover_id.copy_from_slice(AsRef::<[u8]>::as_ref(&prover_fr));
 
-    let aggregate_versions = vec![
-        groth16::aggregate::AggregateVersion::V1,
-        groth16::aggregate::AggregateVersion::V2,
-    ];
+    // Note that ApiVersion 1.2.0 only supports SnarkPack v2, so only
+    // allow that testing here.
+    let aggregate_versions = match porep_config.api_version {
+        ApiVersion::V1_2_0 => vec![groth16::aggregate::AggregateVersion::V2],
+        ApiVersion::V1_1_0 => vec![
+            groth16::aggregate::AggregateVersion::V1,
+            groth16::aggregate::AggregateVersion::V2,
+        ],
+        ApiVersion::V1_0_0 => vec![groth16::aggregate::AggregateVersion::V1],
+    };
     info!(
         "Aggregating {} seal proof with ApiVersion {}, Features {:?}, and PoRep ID {:?}",
         num_proofs_to_aggregate,

storage-proofs-porep/tests/common.rs

@@ -1,9 +1,43 @@
-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(crate) 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>,

storage-proofs-porep/tests/stacked_circuit.rs

@@ -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 =
@@ -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();
@@ -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();
+    }
 }

storage-proofs-porep/tests/stacked_compound.rs

@@ -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,
@@ -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();
+    }
 }

storage-proofs-porep/tests/stacked_vanilla.rs

@@ -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]
@@ -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<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