From 60450e4af437cae874143b71f658029bcf66758e Mon Sep 17 00:00:00 2001 From: Tom French Date: Wed, 19 Apr 2023 16:58:25 +0100 Subject: [PATCH 1/2] chore: refactor integration test code --- crates/nargo_cli/Cargo.toml | 2 +- crates/nargo_cli/src/cli/mod.rs | 12 +-- crates/nargo_cli/tests/prove_and_verify.rs | 107 ++++++++++++++------- 3 files changed, 79 insertions(+), 42 deletions(-) diff --git a/crates/nargo_cli/Cargo.toml b/crates/nargo_cli/Cargo.toml index 57de9aaa264..49c9428f546 100644 --- a/crates/nargo_cli/Cargo.toml +++ b/crates/nargo_cli/Cargo.toml @@ -34,7 +34,6 @@ const_format = "0.2.30" hex = "0.4.2" serde_json = "1.0" termcolor = "1.1.2" -tempdir = "0.3.7" color-eyre = "0.6.2" # Backends @@ -42,6 +41,7 @@ aztec_backend = { optional = true, package = "barretenberg_static_lib", git = "h aztec_wasm_backend = { optional = true, package = "barretenberg_wasm", git = "https://github.com/noir-lang/aztec_backend", rev = "26178359a2251e885f15f0a4d1a686afda04aec9" } [dev-dependencies] +tempdir = "0.3.7" assert_cmd = "2.0.8" assert_fs = "1.0.10" predicates = "2.1.5" diff --git a/crates/nargo_cli/src/cli/mod.rs b/crates/nargo_cli/src/cli/mod.rs index e713bdd47fc..2bb92925e59 100644 --- a/crates/nargo_cli/src/cli/mod.rs +++ b/crates/nargo_cli/src/cli/mod.rs @@ -6,7 +6,7 @@ use std::path::{Path, PathBuf}; use color_eyre::eyre; -use crate::find_package_root; +use crate::{constants::PROOFS_DIR, find_package_root}; mod fs; @@ -85,16 +85,14 @@ pub fn start_cli() -> eyre::Result<()> { } // helper function which tests noir programs by trying to generate a proof and verify it -pub fn prove_and_verify(proof_name: &str, prg_dir: &Path, show_ssa: bool) -> bool { - use tempdir::TempDir; - - let tmp_dir = TempDir::new("p_and_v_tests").unwrap(); +pub fn prove_and_verify(proof_name: &str, program_dir: &Path, show_ssa: bool) -> bool { let compile_options = CompileOptions { show_ssa, allow_warnings: false, show_output: false }; + let proof_dir = program_dir.join(PROOFS_DIR); match prove_cmd::prove_with_path( Some(proof_name.to_owned()), - prg_dir, - &tmp_dir.into_path(), + program_dir, + &proof_dir, None, true, &compile_options, diff --git a/crates/nargo_cli/tests/prove_and_verify.rs b/crates/nargo_cli/tests/prove_and_verify.rs index 0006dd1b986..b8147e7c23f 100644 --- a/crates/nargo_cli/tests/prove_and_verify.rs +++ b/crates/nargo_cli/tests/prove_and_verify.rs @@ -1,3 +1,5 @@ +use tempdir::TempDir; + use std::collections::BTreeMap; use std::fs; @@ -6,11 +8,15 @@ const TEST_DATA_DIR: &str = "test_data"; const CONFIG_FILE: &str = "config.toml"; mod tests { + use std::path::Path; + use super::*; - fn load_conf(conf_path: &str) -> BTreeMap> { + fn load_conf(conf_path: &Path) -> BTreeMap> { + let config_str = std::fs::read_to_string(conf_path).unwrap(); + // Parse config.toml into a BTreeMap, do not fail if config file does not exist. - let mut conf_data = match toml::from_str(conf_path) { + let mut conf_data = match toml::from_str(&config_str) { Ok(t) => t, Err(_) => BTreeMap::from([ ("exclude".to_string(), Vec::new()), @@ -26,42 +32,75 @@ mod tests { conf_data } + /// Copy files from source to destination recursively. + pub fn copy_recursively( + source: impl AsRef, + destination: impl AsRef, + ) -> std::io::Result<()> { + fs::create_dir_all(&destination)?; + for entry in fs::read_dir(source)? { + let entry = entry?; + let filetype = entry.file_type()?; + if filetype.is_dir() { + copy_recursively(entry.path(), destination.as_ref().join(entry.file_name()))?; + } else { + fs::copy(entry.path(), destination.as_ref().join(entry.file_name()))?; + } + } + Ok(()) + } + #[test] fn noir_integration() { - let mut current_dir = std::env::current_dir().unwrap(); - current_dir.push(TEST_DIR); - current_dir.push(TEST_DATA_DIR); - - //load config.tml file from test_data directory - current_dir.push(CONFIG_FILE); - let config_path = std::fs::read_to_string(current_dir).unwrap(); - let config_data: BTreeMap> = load_conf(&config_path); - let mut current_dir = std::env::current_dir().unwrap(); - current_dir.push(TEST_DIR); - current_dir.push(TEST_DATA_DIR); - - for c in fs::read_dir(current_dir.as_path()).unwrap().flatten() { - if let Ok(test_name) = c.file_name().into_string() { - println!("Running test {test_name:?}"); - if c.path().is_dir() && !config_data["exclude"].contains(&test_name) { - let verified = std::panic::catch_unwind(|| { - nargo_cli::cli::prove_and_verify("pp", &c.path(), false) - }); - - let r = match verified { - Ok(result) => result, - Err(_) => { - panic!("\n\n\nPanic occurred while running test {:?} (ignore the following panic)", c.file_name()); - } - }; - - if config_data["fail"].contains(&test_name) { - assert!(!r, "{:?} should not succeed", c.file_name()); - } else { - assert!(r, "verification fail for {:?}", c.file_name()); - } + let current_dir = std::env::current_dir().unwrap(); + + let test_data_dir = current_dir.join(TEST_DIR).join(TEST_DATA_DIR); + + // Load config.toml file from test_data directory + let config_file_path = test_data_dir.join(CONFIG_FILE); + let config_data: BTreeMap> = load_conf(&config_file_path); + + // Copy all the test cases into a temp dir so we don't leave artifacts around. + let tmp_dir = TempDir::new("p_and_v_tests").unwrap(); + copy_recursively(test_data_dir, &tmp_dir) + .expect("failed to copy test cases to temp directory"); + + let test_case_dirs = + fs::read_dir(&tmp_dir).unwrap().flatten().filter(|c| c.path().is_dir()); + + for test_dir in test_case_dirs { + let test_name = + test_dir.file_name().into_string().expect("Directory can't be converted to string"); + let test_program_dir = &test_dir.path(); + + if config_data["exclude"].contains(&test_name) { + println!("Skipping test {test_name}"); + continue; + } + + println!("Running test {test_name}"); + + let verified = std::panic::catch_unwind(|| { + nargo_cli::cli::prove_and_verify("pp", test_program_dir, false) + }); + + let r = match verified { + Ok(result) => result, + Err(_) => { + panic!( + "\n\n\nPanic occurred while running test {test_name} (ignore the following panic)" + ); } + }; + + if config_data["fail"].contains(&test_name) { + assert!(!r, "{:?} should not succeed", test_name); + } else { + assert!(r, "verification fail for {:?}", test_name); } } + + // Ensure that temp dir remains alive until all tests have run. + drop(tmp_dir); } } From 15f9b40f9510a8f5256249c4bb232b21ba796491 Mon Sep 17 00:00:00 2001 From: Tom French Date: Wed, 19 Apr 2023 18:31:22 +0100 Subject: [PATCH 2/2] chore: remove misleading comment --- crates/nargo_cli/tests/prove_and_verify.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/nargo_cli/tests/prove_and_verify.rs b/crates/nargo_cli/tests/prove_and_verify.rs index b8147e7c23f..15e860bf059 100644 --- a/crates/nargo_cli/tests/prove_and_verify.rs +++ b/crates/nargo_cli/tests/prove_and_verify.rs @@ -15,7 +15,6 @@ mod tests { fn load_conf(conf_path: &Path) -> BTreeMap> { let config_str = std::fs::read_to_string(conf_path).unwrap(); - // Parse config.toml into a BTreeMap, do not fail if config file does not exist. let mut conf_data = match toml::from_str(&config_str) { Ok(t) => t, Err(_) => BTreeMap::from([