From ce68aea00b49e4d0ae31fa88fbed55b35e79804f Mon Sep 17 00:00:00 2001 From: Rumata888 Date: Fri, 9 May 2025 10:05:53 +0000 Subject: [PATCH 1/9] parallelisation for fuzz tests --- tooling/nargo_cli/src/cli/test_cmd.rs | 162 +++++++++++++++++--------- 1 file changed, 106 insertions(+), 56 deletions(-) diff --git a/tooling/nargo_cli/src/cli/test_cmd.rs b/tooling/nargo_cli/src/cli/test_cmd.rs index 2897f9e9ca9..3ee8486b9da 100644 --- a/tooling/nargo_cli/src/cli/test_cmd.rs +++ b/tooling/nargo_cli/src/cli/test_cmd.rs @@ -1,9 +1,13 @@ use std::{ + cmp::max, collections::{BTreeMap, HashMap}, fmt::Display, panic::{UnwindSafe, catch_unwind}, path::PathBuf, - sync::{Mutex, mpsc}, + sync::{ + Mutex, + mpsc::{self, Sender}, + }, thread, time::Duration, }; @@ -141,6 +145,7 @@ impl Display for Format { struct Test<'a> { name: String, package_name: String, + has_arguments: bool, runner: Box (TestStatus, String) + Send + UnwindSafe + 'a>, } @@ -272,6 +277,62 @@ impl<'a> TestRunner<'a> { if all_passed { Ok(()) } else { Err(CliError::Generic(String::new())) } } + fn process_chunk_of_tests(&self, iter_tests: &Mutex, thread_sender: &Sender) + where + I: Iterator>, + { + loop { + // Get next test to process from the iterator. + let Some(test) = iter_tests.lock().unwrap().next() else { + break; + }; + + self.formatter + .test_start_async(&test.name, &test.package_name) + .expect("Could not display test start"); + + let time_before_test = std::time::Instant::now(); + let (status, output) = match catch_unwind(test.runner) { + Ok((status, output)) => (status, output), + Err(err) => ( + TestStatus::Fail { + message: + // It seems `panic!("...")` makes the error be `&str`, so we handle this common case + if let Some(message) = err.downcast_ref::<&str>() { + message.to_string() + } else { + "An unexpected error happened".to_string() + }, + error_diagnostic: None, + }, + String::new(), + ), + }; + let time_to_run = time_before_test.elapsed(); + + let test_result = TestResult { + name: test.name, + package_name: test.package_name, + status, + output, + time_to_run, + }; + + self.formatter + .test_end_async( + &test_result, + self.file_manager, + self.args.show_output, + self.args.compile_options.deny_warnings, + self.args.compile_options.silence_warnings, + ) + .expect("Could not display test start"); + + if thread_sender.send(test_result).is_err() { + break; + } + } + } /// Runs all tests. Returns `true` if all tests passed, `false` otherwise. fn run_all_tests( &self, @@ -287,72 +348,54 @@ impl<'a> TestRunner<'a> { } let (sender, receiver) = mpsc::channel(); - let iter = &Mutex::new(tests.into_iter()); + let (standard_tests_finished_sender, standard_tests_finished_receiver) = mpsc::channel(); + // Partition tests into standard and fuzz tests + let (iter_tests_without_arguments, iter_tests_with_arguments): ( + Vec>, + Vec>, + ) = tests.into_iter().partition(|test| !test.has_arguments); + + let iter_tests_without_arguments = &Mutex::new(iter_tests_without_arguments.into_iter()); + let iter_tests_with_arguments = &Mutex::new(iter_tests_with_arguments.into_iter()); + thread::scope(|scope| { // Start worker threads for _ in 0..self.num_threads { // Clone sender so it's dropped once the thread finishes - let thread_sender = sender.clone(); + let test_result_thread_sender = sender.clone(); + let standard_tests_finished_thread_sender = standard_tests_finished_sender.clone(); thread::Builder::new() // Specify a larger-than-default stack size to prevent overflowing stack in large programs. // (the default is 2MB) .stack_size(STACK_SIZE) .spawn_scoped(scope, move || { - loop { - // Get next test to process from the iterator. - let Some(test) = iter.lock().unwrap().next() else { - break; - }; - - self.formatter - .test_start_async(&test.name, &test.package_name) - .expect("Could not display test start"); - - let time_before_test = std::time::Instant::now(); - let (status, output) = match catch_unwind(test.runner) { - Ok((status, output)) => (status, output), - Err(err) => ( - TestStatus::Fail { - message: - // It seems `panic!("...")` makes the error be `&str`, so we handle this common case - if let Some(message) = err.downcast_ref::<&str>() { - message.to_string() - } else { - "An unexpected error happened".to_string() - }, - error_diagnostic: None, - }, - String::new(), - ), - }; - let time_to_run = time_before_test.elapsed(); - - let test_result = TestResult { - name: test.name, - package_name: test.package_name, - status, - output, - time_to_run, - }; - - self.formatter - .test_end_async( - &test_result, - self.file_manager, - self.args.show_output, - self.args.compile_options.deny_warnings, - self.args.compile_options.silence_warnings, - ) - .expect("Could not display test start"); - - if thread_sender.send(test_result).is_err() { - break; - } - } + self.process_chunk_of_tests( + &iter_tests_without_arguments, + &test_result_thread_sender, + ); + // Signal that we've finished processing the standard tests in this thread + standard_tests_finished_thread_sender.send(()).unwrap(); }) .unwrap(); } + let test_result_thread_sender = sender.clone(); + thread::Builder::new() + .stack_size(STACK_SIZE) + .spawn_scoped(scope, move || { + // Wait for at least half of the threads to finish processing the standard tests + for _ in 0..max(1, self.num_threads / 2) { + standard_tests_finished_receiver.recv().unwrap(); + } + // Process fuzz tests sequentially + // Parallelism is handled by the fuzz tests themselves + self.process_chunk_of_tests( + &iter_tests_with_arguments, + &test_result_thread_sender, + ); + }) + .unwrap(); + // Also drop main sender so the channel closes drop(sender); @@ -505,12 +548,18 @@ impl<'a> TestRunner<'a> { package, &test_name, test_function.has_arguments, + self.num_threads, foreign_call_resolver_url, root_path, package_name_clone.clone(), ) }); - Test { name: test_name_copy, package_name: package_name_clone2, runner } + Test { + name: test_name_copy, + package_name: package_name_clone2, + runner, + has_arguments: test_function.has_arguments, + } }) .collect(); @@ -536,6 +585,7 @@ impl<'a> TestRunner<'a> { package: &Package, fn_name: &str, has_arguments: bool, + num_threads: usize, foreign_call_resolver_url: Option<&str>, root_path: Option, package_name: String, @@ -566,7 +616,7 @@ impl<'a> TestRunner<'a> { fuzzing_failure_dir: self.args.fuzzing_failure_dir.clone(), }, execution_config: FuzzExecutionConfig { - num_threads: 1, + num_threads, timeout: self.args.fuzz_timeout, show_progress: false, max_executions: 0, From 5e79f97f7712024ccf2a5363c4ef68d8ae26b772 Mon Sep 17 00:00:00 2001 From: Rumata888 Date: Fri, 9 May 2025 10:35:50 +0000 Subject: [PATCH 2/9] implemented --- tooling/nargo_cli/src/cli/test_cmd.rs | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/tooling/nargo_cli/src/cli/test_cmd.rs b/tooling/nargo_cli/src/cli/test_cmd.rs index 3ee8486b9da..e785e7a24d3 100644 --- a/tooling/nargo_cli/src/cli/test_cmd.rs +++ b/tooling/nargo_cli/src/cli/test_cmd.rs @@ -97,9 +97,17 @@ pub(crate) struct TestCommand { #[arg(long)] fuzzing_failure_dir: Option, - /// Maximum time in seconds to spend fuzzing (default: 1 second) - #[arg(long, default_value_t = 1)] + /// Maximum time in seconds to spend fuzzing (default: 10 seconds) + #[arg(long, default_value_t = 10)] fuzz_timeout: u64, + + /// Maximum number of executions to run for each fuzz test (default: 100000) + #[arg(long, default_value_t = 100000)] + fuzz_max_executions: usize, + + /// Show progress of fuzzing (default: false) + #[arg(long)] + fuzz_show_progress: bool, } impl WorkspaceCommand for TestCommand { @@ -370,7 +378,7 @@ impl<'a> TestRunner<'a> { .stack_size(STACK_SIZE) .spawn_scoped(scope, move || { self.process_chunk_of_tests( - &iter_tests_without_arguments, + iter_tests_without_arguments, &test_result_thread_sender, ); // Signal that we've finished processing the standard tests in this thread @@ -390,7 +398,7 @@ impl<'a> TestRunner<'a> { // Process fuzz tests sequentially // Parallelism is handled by the fuzz tests themselves self.process_chunk_of_tests( - &iter_tests_with_arguments, + iter_tests_with_arguments, &test_result_thread_sender, ); }) @@ -548,7 +556,6 @@ impl<'a> TestRunner<'a> { package, &test_name, test_function.has_arguments, - self.num_threads, foreign_call_resolver_url, root_path, package_name_clone.clone(), @@ -585,7 +592,6 @@ impl<'a> TestRunner<'a> { package: &Package, fn_name: &str, has_arguments: bool, - num_threads: usize, foreign_call_resolver_url: Option<&str>, root_path: Option, package_name: String, @@ -616,10 +622,10 @@ impl<'a> TestRunner<'a> { fuzzing_failure_dir: self.args.fuzzing_failure_dir.clone(), }, execution_config: FuzzExecutionConfig { - num_threads, + num_threads: self.num_threads, timeout: self.args.fuzz_timeout, - show_progress: false, - max_executions: 0, + show_progress: self.args.fuzz_show_progress, + max_executions: self.args.fuzz_max_executions, }, }; From b4294bed4c14374a1d32a16a2b79e709cac5b715 Mon Sep 17 00:00:00 2001 From: Rumata888 Date: Fri, 9 May 2025 11:10:52 +0000 Subject: [PATCH 3/9] docs --- docs/docs/tooling/testing.md | 74 +++++++++++++++++++++++++++++++++++- 1 file changed, 73 insertions(+), 1 deletion(-) diff --git a/docs/docs/tooling/testing.md b/docs/docs/tooling/testing.md index 866677da567..e5f8411e4ae 100644 --- a/docs/docs/tooling/testing.md +++ b/docs/docs/tooling/testing.md @@ -76,4 +76,76 @@ fn test_king_arthur() { fn test_bridgekeeper() { main(32); } -``` \ No newline at end of file +``` + +### Fuzz tests + +You can write fuzzing harnesses that will run on `nargo test` by using the decorator `#[test]` with a function that has arguments. For example: + +```rust +#[test] +fn test_basic(a: Field, b: Field) { + assert(a + b == b + a); +} +``` +The test above is not expected to fail. By default, the fuzzer will run for 10 seconds and use 100000 executions (whichever comes first). All available threads will be used for each fuzz test. +The fuzz tests also work with `#[test(should_fail)]` and `#[test(should_fail_with = "")]`. For example: + +```rust +#[test(should_fail)] +fn test_should_fail(a: [bool; 32]) { + let mut or_sum= false; + for i in 0..32 { + or_sum=or_sum|(a[i]==((i&1)as bool)); + } + assert(!or_sum); +} +``` +or + +```rust +#[test(should_fail_with = "This is the message that will be checked for")] +fn fuzz_should_fail_with(a: [bool; 32]) { + let mut or_sum= false; + for i in 0..32 { + or_sum=or_sum|(a[i]==((i&1)as bool)); + } + assert(or_sum); + assert(false, "This is the message that will be checked for"); +} +``` + +The underlying fuzzing mechanism is described in the [Fuzzing](../tooling/fuzzing) documentation. + +There are some fuzzing-specific options that can be used with `nargo test`: + --no-fuzz + Do not run fuzz tests (tests that have arguments) + + --only-fuzz + Only run fuzz tests (tests that have arguments) + + --corpus-dir + If given, load/store fuzzer corpus from this folder + + --minimized-corpus-dir + If given, perform corpus minimization instead of fuzzing and store results in the given folder + + --fuzzing-failure-dir + If given, store the failing input in the given folder + + --fuzz-timeout + Maximum time in seconds to spend fuzzing (default: 10 seconds) + + [default: 10] + + --fuzz-max-executions + Maximum number of executions to run for each fuzz test (default: 100000) + + [default: 100000] + + --fuzz-show-progress + Show progress of fuzzing (default: false) + + +By default, the fuzzing corpus is saved in a temporary directory, but this can be changed. This allows you to resume fuzzing from the same corpus if the process is interrupted, if you want to run continuous fuzzing on your corpus, or if you want to use previous failures for regression testing. + From 87d954598d4ce6cc39c29b0c098d18425ae80e0f Mon Sep 17 00:00:00 2001 From: Rumata888 Date: Fri, 9 May 2025 14:16:16 +0000 Subject: [PATCH 4/9] Fix --- tooling/nargo_cli/src/cli/test_cmd.rs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/tooling/nargo_cli/src/cli/test_cmd.rs b/tooling/nargo_cli/src/cli/test_cmd.rs index e785e7a24d3..c32b8d607d5 100644 --- a/tooling/nargo_cli/src/cli/test_cmd.rs +++ b/tooling/nargo_cli/src/cli/test_cmd.rs @@ -382,7 +382,7 @@ impl<'a> TestRunner<'a> { &test_result_thread_sender, ); // Signal that we've finished processing the standard tests in this thread - standard_tests_finished_thread_sender.send(()).unwrap(); + let _ = standard_tests_finished_thread_sender.send(()); }) .unwrap(); } @@ -391,10 +391,15 @@ impl<'a> TestRunner<'a> { thread::Builder::new() .stack_size(STACK_SIZE) .spawn_scoped(scope, move || { + let mut standard_tests_threads_finished = 0; // Wait for at least half of the threads to finish processing the standard tests - for _ in 0..max(1, self.num_threads / 2) { - standard_tests_finished_receiver.recv().unwrap(); + while let Ok(_) = standard_tests_finished_receiver.recv() { + standard_tests_threads_finished += 1; + if standard_tests_threads_finished >= max(1, self.num_threads / 2) { + break; + } } + // Process fuzz tests sequentially // Parallelism is handled by the fuzz tests themselves self.process_chunk_of_tests( From cf18fb7ffadf1c49243654aaaa039ddef655643a Mon Sep 17 00:00:00 2001 From: Rumata888 Date: Fri, 9 May 2025 14:20:59 +0000 Subject: [PATCH 5/9] Clippy --- tooling/nargo_cli/src/cli/test_cmd.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tooling/nargo_cli/src/cli/test_cmd.rs b/tooling/nargo_cli/src/cli/test_cmd.rs index c32b8d607d5..526c8aa0daa 100644 --- a/tooling/nargo_cli/src/cli/test_cmd.rs +++ b/tooling/nargo_cli/src/cli/test_cmd.rs @@ -285,6 +285,8 @@ impl<'a> TestRunner<'a> { if all_passed { Ok(()) } else { Err(CliError::Generic(String::new())) } } + /// Process a chunk of tests sequentially and send the results to the main thread + /// We need this functions, because first we process the standard tests, and then the fuzz tests. fn process_chunk_of_tests(&self, iter_tests: &Mutex, thread_sender: &Sender) where I: Iterator>, @@ -341,6 +343,7 @@ impl<'a> TestRunner<'a> { } } } + /// Runs all tests. Returns `true` if all tests passed, `false` otherwise. fn run_all_tests( &self, @@ -393,7 +396,7 @@ impl<'a> TestRunner<'a> { .spawn_scoped(scope, move || { let mut standard_tests_threads_finished = 0; // Wait for at least half of the threads to finish processing the standard tests - while let Ok(_) = standard_tests_finished_receiver.recv() { + while standard_tests_finished_receiver.recv().is_ok() { standard_tests_threads_finished += 1; if standard_tests_threads_finished >= max(1, self.num_threads / 2) { break; From 912812f050cba7b0a4aedef1cbcde4457c540b33 Mon Sep 17 00:00:00 2001 From: Rumata888 Date: Fri, 9 May 2025 14:34:14 +0000 Subject: [PATCH 6/9] smaller times --- docs/docs/tooling/testing.md | 4 ++-- tooling/nargo_cli/src/cli/test_cmd.rs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/docs/tooling/testing.md b/docs/docs/tooling/testing.md index e5f8411e4ae..de51f63d54a 100644 --- a/docs/docs/tooling/testing.md +++ b/docs/docs/tooling/testing.md @@ -134,9 +134,9 @@ There are some fuzzing-specific options that can be used with `nargo test`: If given, store the failing input in the given folder --fuzz-timeout - Maximum time in seconds to spend fuzzing (default: 10 seconds) + Maximum time in seconds to spend fuzzing (default: 1 second) - [default: 10] + [default: 1] --fuzz-max-executions Maximum number of executions to run for each fuzz test (default: 100000) diff --git a/tooling/nargo_cli/src/cli/test_cmd.rs b/tooling/nargo_cli/src/cli/test_cmd.rs index 526c8aa0daa..67aff3319cc 100644 --- a/tooling/nargo_cli/src/cli/test_cmd.rs +++ b/tooling/nargo_cli/src/cli/test_cmd.rs @@ -97,8 +97,8 @@ pub(crate) struct TestCommand { #[arg(long)] fuzzing_failure_dir: Option, - /// Maximum time in seconds to spend fuzzing (default: 10 seconds) - #[arg(long, default_value_t = 10)] + /// Maximum time in seconds to spend fuzzing (default: 1 seconds) + #[arg(long, default_value_t = 1)] fuzz_timeout: u64, /// Maximum number of executions to run for each fuzz test (default: 100000) From 20ab8908ce406de1be27185bce5bca8d31677fab Mon Sep 17 00:00:00 2001 From: Rumata888 Date: Fri, 9 May 2025 14:52:51 +0000 Subject: [PATCH 7/9] update aztec.nr timeout --- EXTERNAL_NOIR_LIBRARIES.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/EXTERNAL_NOIR_LIBRARIES.yml b/EXTERNAL_NOIR_LIBRARIES.yml index 3cb5b324ca6..fd92b35c780 100644 --- a/EXTERNAL_NOIR_LIBRARIES.yml +++ b/EXTERNAL_NOIR_LIBRARIES.yml @@ -76,7 +76,7 @@ libraries: repo: AztecProtocol/aztec-packages ref: *AZ_COMMIT path: noir-projects/aztec-nr - timeout: 60 + timeout: 70 critical: false noir_contracts: repo: AztecProtocol/aztec-packages From a1c9f089d196512c7c7f9a09e11a9e8c6fc8e08a Mon Sep 17 00:00:00 2001 From: Rumata888 Date: Fri, 9 May 2025 15:46:27 +0000 Subject: [PATCH 8/9] change docs --- docs/docs/tooling/testing.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/docs/tooling/testing.md b/docs/docs/tooling/testing.md index de51f63d54a..128c7dcf65f 100644 --- a/docs/docs/tooling/testing.md +++ b/docs/docs/tooling/testing.md @@ -88,7 +88,7 @@ fn test_basic(a: Field, b: Field) { assert(a + b == b + a); } ``` -The test above is not expected to fail. By default, the fuzzer will run for 10 seconds and use 100000 executions (whichever comes first). All available threads will be used for each fuzz test. +The test above is not expected to fail. By default, the fuzzer will run for 1 seconds and use 100000 executions (whichever comes first). All available threads will be used for each fuzz test. The fuzz tests also work with `#[test(should_fail)]` and `#[test(should_fail_with = "")]`. For example: ```rust From 26017064f575578fb30e216f16085f7a3111c3e9 Mon Sep 17 00:00:00 2001 From: Rumata888 Date: Fri, 9 May 2025 15:48:03 +0000 Subject: [PATCH 9/9] typo --- docs/docs/tooling/testing.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/docs/tooling/testing.md b/docs/docs/tooling/testing.md index 128c7dcf65f..a87903543bc 100644 --- a/docs/docs/tooling/testing.md +++ b/docs/docs/tooling/testing.md @@ -88,7 +88,7 @@ fn test_basic(a: Field, b: Field) { assert(a + b == b + a); } ``` -The test above is not expected to fail. By default, the fuzzer will run for 1 seconds and use 100000 executions (whichever comes first). All available threads will be used for each fuzz test. +The test above is not expected to fail. By default, the fuzzer will run for 1 second and use 100000 executions (whichever comes first). All available threads will be used for each fuzz test. The fuzz tests also work with `#[test(should_fail)]` and `#[test(should_fail_with = "")]`. For example: ```rust