From a1aa047e5842ea9337f9de6f8d0334dfaa738441 Mon Sep 17 00:00:00 2001 From: guipublic Date: Mon, 2 Dec 2024 10:01:00 +0000 Subject: [PATCH 01/11] skip tests having oracle calls --- acvm-repo/acir/src/circuit/brillig.rs | 16 ++++++++++++ compiler/noirc_driver/src/lib.rs | 4 +++ tooling/lsp/src/requests/test_run.rs | 5 ++++ tooling/nargo/src/ops/foreign_calls.rs | 4 +++ tooling/nargo/src/ops/test.rs | 22 ++++++++++++++-- tooling/nargo_cli/src/cli/test_cmd.rs | 36 ++++++++++++++++++++------ 6 files changed, 77 insertions(+), 10 deletions(-) diff --git a/acvm-repo/acir/src/circuit/brillig.rs b/acvm-repo/acir/src/circuit/brillig.rs index a9714ce29b2..85addefce2c 100644 --- a/acvm-repo/acir/src/circuit/brillig.rs +++ b/acvm-repo/acir/src/circuit/brillig.rs @@ -28,6 +28,22 @@ pub struct BrilligBytecode { pub bytecode: Vec>, } +impl BrilligBytecode { + /// Returns true if the bytecode contains a foreign call + /// whose name matches the given predicate. + pub fn has_oracle(&self, filter: Fun) -> bool + where + Fun: Fn(&str) -> bool, + { + self.bytecode.iter().any(|op| { + if let BrilligOpcode::ForeignCall { function, .. } = op { + filter(function) + } else { + false + } + }) + } +} /// Id for the function being called. #[derive( Clone, Debug, PartialEq, Eq, Serialize, Deserialize, Hash, Copy, Default, PartialOrd, Ord, diff --git a/compiler/noirc_driver/src/lib.rs b/compiler/noirc_driver/src/lib.rs index a7cd9ff90ac..9f32377f444 100644 --- a/compiler/noirc_driver/src/lib.rs +++ b/compiler/noirc_driver/src/lib.rs @@ -139,6 +139,10 @@ pub struct CompileOptions { /// A lower value keeps the original program if it was smaller, even if it has more jumps. #[arg(long, hide = true, allow_hyphen_values = true)] pub max_bytecode_increase_percent: Option, + + /// Flag to skip tests using oracles. + #[arg(long, hide = true)] + pub skip_oracle: bool, } pub fn parse_expression_width(input: &str) -> Result { diff --git a/tooling/lsp/src/requests/test_run.rs b/tooling/lsp/src/requests/test_run.rs index 50c699bb6a6..4dcc78d59c3 100644 --- a/tooling/lsp/src/requests/test_run.rs +++ b/tooling/lsp/src/requests/test_run.rs @@ -106,6 +106,11 @@ fn on_test_run_request_inner( result: "error".to_string(), message: Some(diag.diagnostic.message), }, + TestStatus::Skipped => NargoTestRunResult { + id: params.id.clone(), + result: "skipped".to_string(), + message: None, + }, }; Ok(result) } diff --git a/tooling/nargo/src/ops/foreign_calls.rs b/tooling/nargo/src/ops/foreign_calls.rs index 30785949a46..d1a1c1d0dd7 100644 --- a/tooling/nargo/src/ops/foreign_calls.rs +++ b/tooling/nargo/src/ops/foreign_calls.rs @@ -60,6 +60,10 @@ impl ForeignCall { _ => None, } } + + pub(crate) fn invalid_name(name: &str) -> bool { + ForeignCall::lookup(name).is_none() + } } /// This struct represents an oracle mock. It can be used for testing programs that use oracles. diff --git a/tooling/nargo/src/ops/test.rs b/tooling/nargo/src/ops/test.rs index 370a4235f61..318eb45cb27 100644 --- a/tooling/nargo/src/ops/test.rs +++ b/tooling/nargo/src/ops/test.rs @@ -11,17 +11,24 @@ use noirc_frontend::hir::{def_map::TestFunction, Context}; use crate::{errors::try_to_diagnose_runtime_error, NargoError}; -use super::{execute_program, DefaultForeignCallExecutor}; +use super::{execute_program, DefaultForeignCallExecutor, ForeignCall}; pub enum TestStatus { Pass, Fail { message: String, error_diagnostic: Option }, + Skipped, CompileError(FileDiagnostic), } impl TestStatus { pub fn failed(&self) -> bool { - !matches!(self, TestStatus::Pass) + matches!(self, TestStatus::Fail { .. } | TestStatus::CompileError(_)) + } + pub fn pass(&self) -> bool { + matches!(self, TestStatus::Pass) + } + pub fn skipped(&self) -> bool { + matches!(self, TestStatus::Skipped) } } @@ -45,6 +52,17 @@ pub fn run_test>( match compile_no_check(context, config, test_function.get_id(), None, false) { Ok(compiled_program) => { + if config.skip_oracle { + let has_oracle = compiled_program + .program + .unconstrained_functions + .iter() + .any(|func| func.has_oracle(ForeignCall::invalid_name)); + if has_oracle { + return TestStatus::Skipped; + } + } + if test_function_has_no_arguments { // Run the backend to ensure the PWG evaluates functions like std::hash::pedersen, // otherwise constraints involving these expressions will not error. diff --git a/tooling/nargo_cli/src/cli/test_cmd.rs b/tooling/nargo_cli/src/cli/test_cmd.rs index 7b0201226ef..1d8d35d4e8f 100644 --- a/tooling/nargo_cli/src/cli/test_cmd.rs +++ b/tooling/nargo_cli/src/cli/test_cmd.rs @@ -263,25 +263,39 @@ fn display_test_report( compile_options.silence_warnings, ); } + TestStatus::Skipped => { + writer + .set_color(ColorSpec::new().set_fg(Some(Color::Yellow))) + .expect("Failed to set color"); + writeln!(writer, "skipped").expect("Failed to write to stderr"); + } } writer.reset().expect("Failed to reset writer"); } write!(writer, "[{}] ", package.name).expect("Failed to write to stderr"); - let count_all = test_report.len(); let count_failed = test_report.iter().filter(|(_, status)| status.failed()).count(); - let plural = if count_all == 1 { "" } else { "s" }; + let count_passed = test_report.iter().filter(|(_, status)| status.pass()).count(); + let count_skipped = test_report.iter().filter(|(_, status)| status.skipped()).count(); + let plural_failed = if count_failed == 1 { "" } else { "s" }; + let plural_passed = if count_passed == 1 { "" } else { "s" }; + let plural_skipped = if count_skipped == 1 { "" } else { "s" }; + if count_failed == 0 { writer.set_color(ColorSpec::new().set_fg(Some(Color::Green))).expect("Failed to set color"); - write!(writer, "{count_all} test{plural} passed").expect("Failed to write to stderr"); + write!(writer, "{count_passed} test{plural_passed} passed") + .expect("Failed to write to stderr"); + if count_skipped > 0 { + writer + .set_color(ColorSpec::new().set_fg(Some(Color::Yellow))) + .expect("Failed to set color"); + write!(writer, ", {count_skipped} test{plural_skipped} skipped") + .expect("Failed to write to stderr"); + } writer.reset().expect("Failed to reset writer"); writeln!(writer).expect("Failed to write to stderr"); } else { - let count_passed = count_all - count_failed; - let plural_failed = if count_failed == 1 { "" } else { "s" }; - let plural_passed = if count_passed == 1 { "" } else { "s" }; - if count_passed != 0 { writer .set_color(ColorSpec::new().set_fg(Some(Color::Green))) @@ -289,7 +303,13 @@ fn display_test_report( write!(writer, "{count_passed} test{plural_passed} passed, ",) .expect("Failed to write to stderr"); } - + if count_skipped != 0 { + writer + .set_color(ColorSpec::new().set_fg(Some(Color::Yellow))) + .expect("Failed to set color"); + write!(writer, ", {count_skipped} test{plural_skipped} skipped") + .expect("Failed to write to stderr"); + } writer.set_color(ColorSpec::new().set_fg(Some(Color::Red))).expect("Failed to set color"); writeln!(writer, "{count_failed} test{plural_failed} failed") .expect("Failed to write to stderr"); From 50da27760737cfe98b85cc13bc677e68f5d62721 Mon Sep 17 00:00:00 2001 From: guipublic Date: Mon, 2 Dec 2024 10:05:34 +0000 Subject: [PATCH 02/11] update doc --- docs/docs/how_to/how-to-oracles.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/docs/how_to/how-to-oracles.md b/docs/docs/how_to/how-to-oracles.md index 0bb8743e361..4fa80b0bb35 100644 --- a/docs/docs/how_to/how-to-oracles.md +++ b/docs/docs/how_to/how-to-oracles.md @@ -180,6 +180,8 @@ nargo test --oracle-resolver http://localhost:5555 This tells `nargo` to use your RPC Server URL whenever it finds an oracle decorator. +You can also skip the tests using oracles by using the flag `--skip-oracle` in the `nargo test` command. + ## Step 4 - Usage with NoirJS In a JS environment, an RPC server is not strictly necessary, as you may want to resolve your oracles without needing any JSON call at all. NoirJS simply expects that you pass a callback function when you generate proofs, and that callback function can be anything. From d7a3207b6c3c1c487ce29282730b0ba44481410c Mon Sep 17 00:00:00 2001 From: guipublic Date: Mon, 2 Dec 2024 10:15:22 +0000 Subject: [PATCH 03/11] fix test case --- tooling/nargo_cli/tests/stdlib-tests.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tooling/nargo_cli/tests/stdlib-tests.rs b/tooling/nargo_cli/tests/stdlib-tests.rs index bdc92e625ab..91d6e827093 100644 --- a/tooling/nargo_cli/tests/stdlib-tests.rs +++ b/tooling/nargo_cli/tests/stdlib-tests.rs @@ -146,6 +146,12 @@ fn display_test_report( compile_options.silence_warnings, ); } + TestStatus::Skipped { .. } => { + writer + .set_color(ColorSpec::new().set_fg(Some(Color::Yellow))) + .expect("Failed to set color"); + writeln!(writer, "skipped").expect("Failed to write to stderr"); + } } writer.reset().expect("Failed to reset writer"); } From 4a8019dea46beb6aedc86a9cbce3b4c31d066de6 Mon Sep 17 00:00:00 2001 From: guipublic Date: Tue, 3 Dec 2024 18:20:35 +0000 Subject: [PATCH 04/11] do not skip tests having mock oracles --- acvm-repo/acir/src/circuit/brillig.rs | 36 +++++++++++++++++++------- tooling/lsp/src/requests/test_run.rs | 5 ---- tooling/nargo/src/foreign_calls/mod.rs | 20 +++++++++++++- tooling/nargo/src/ops/test.rs | 20 ++++++++++---- tooling/nargo_cli/src/cli/test_cmd.rs | 8 +----- 5 files changed, 62 insertions(+), 27 deletions(-) diff --git a/acvm-repo/acir/src/circuit/brillig.rs b/acvm-repo/acir/src/circuit/brillig.rs index 85addefce2c..8ac3342965d 100644 --- a/acvm-repo/acir/src/circuit/brillig.rs +++ b/acvm-repo/acir/src/circuit/brillig.rs @@ -28,20 +28,38 @@ pub struct BrilligBytecode { pub bytecode: Vec>, } +/// A ForeignCall bytecode can be one of these cases: +pub enum OracleResult { + /// Bytecode which mocks oracle calls + Mocked, + /// Bytecode which calls external oracles + Unhandled, + /// Bytecode which calls internal oracles + Handled, +} impl BrilligBytecode { - /// Returns true if the bytecode contains a foreign call - /// whose name matches the given predicate. - pub fn has_oracle(&self, filter: Fun) -> bool + /// Returns + /// - Mocked: if at least one foreign call is 'mocked' + /// - Handled: if all foreign calls are 'handled' + /// - Unhandled: if at least one foreign call is 'unhandled' but none is 'mocked' + /// The foreign calls status is given by the provided filter function + pub fn get_oracle_status(&self, filter: Fun) -> OracleResult where - Fun: Fn(&str) -> bool, + Fun: Fn(&str) -> OracleResult, { - self.bytecode.iter().any(|op| { + let mut result = OracleResult::Handled; + for op in self.bytecode.iter() { if let BrilligOpcode::ForeignCall { function, .. } = op { - filter(function) - } else { - false + match filter(function) { + OracleResult::Mocked => return OracleResult::Mocked, // We assume that all unhandled oracle calls will be mocked. This is not necessarily the case. + OracleResult::Unhandled => { + result = OracleResult::Unhandled; + } + OracleResult::Handled => (), + } } - }) + } + result } } /// Id for the function being called. diff --git a/tooling/lsp/src/requests/test_run.rs b/tooling/lsp/src/requests/test_run.rs index 728fce07d1d..937fdcc0a5e 100644 --- a/tooling/lsp/src/requests/test_run.rs +++ b/tooling/lsp/src/requests/test_run.rs @@ -111,11 +111,6 @@ fn on_test_run_request_inner( result: "error".to_string(), message: Some(diag.diagnostic.message), }, - TestStatus::Skipped => NargoTestRunResult { - id: params.id.clone(), - result: "skipped".to_string(), - message: None, - }, }; Ok(result) } diff --git a/tooling/nargo/src/foreign_calls/mod.rs b/tooling/nargo/src/foreign_calls/mod.rs index 16ed71e11e3..fb4291eb4eb 100644 --- a/tooling/nargo/src/foreign_calls/mod.rs +++ b/tooling/nargo/src/foreign_calls/mod.rs @@ -1,6 +1,10 @@ use std::path::PathBuf; -use acvm::{acir::brillig::ForeignCallResult, pwg::ForeignCallWaitInfo, AcirField}; +use acvm::{ + acir::{brillig::ForeignCallResult, circuit::brillig::OracleResult}, + pwg::ForeignCallWaitInfo, + AcirField, +}; use mocker::MockForeignCallExecutor; use noirc_printable_type::ForeignCallError; use print::PrintForeignCallExecutor; @@ -62,6 +66,20 @@ impl ForeignCall { _ => None, } } + + /// Returns the foreign call status based on its name: + /// create_mock => 'mocked' + /// valid ForeignCall name => 'handled' + /// otherwise => 'unhandled' + pub(crate) fn check_oracle_status(name: &str) -> OracleResult { + if name == ForeignCall::CreateMock.name() { + OracleResult::Mocked + } else if ForeignCall::lookup(name).is_some() { + OracleResult::Handled + } else { + OracleResult::Unhandled + } + } } #[derive(Debug, Default)] diff --git a/tooling/nargo/src/ops/test.rs b/tooling/nargo/src/ops/test.rs index 92cd2166257..5cfedbc6e5f 100644 --- a/tooling/nargo/src/ops/test.rs +++ b/tooling/nargo/src/ops/test.rs @@ -3,6 +3,7 @@ use std::path::PathBuf; use acvm::{ acir::{ brillig::ForeignCallResult, + circuit::brillig::OracleResult, native_types::{WitnessMap, WitnessStack}, }, pwg::ForeignCallWaitInfo, @@ -67,11 +68,20 @@ pub fn run_test>( match compile_no_check(context, config, test_function.get_id(), None, false) { Ok(compiled_program) => { if config.skip_oracle { - let has_oracle = compiled_program - .program - .unconstrained_functions - .iter() - .any(|func| func.has_oracle(ForeignCall::invalid_name)); + let mut has_oracle = false; + for brillig_function in &compiled_program.program.unconstrained_functions { + match brillig_function.get_oracle_status(ForeignCall::check_oracle_status) { + OracleResult::Mocked => { + has_oracle = false; + break; + } + OracleResult::Unhandled => { + has_oracle = true; + } + OracleResult::Handled => (), + } + } + if has_oracle { return TestStatus::Skipped; } diff --git a/tooling/nargo_cli/src/cli/test_cmd.rs b/tooling/nargo_cli/src/cli/test_cmd.rs index d435909f731..20c80543e9c 100644 --- a/tooling/nargo_cli/src/cli/test_cmd.rs +++ b/tooling/nargo_cli/src/cli/test_cmd.rs @@ -269,12 +269,6 @@ fn display_test_report( compile_options.silence_warnings, ); } - TestStatus::Skipped => { - writer - .set_color(ColorSpec::new().set_fg(Some(Color::Yellow))) - .expect("Failed to set color"); - writeln!(writer, "skipped").expect("Failed to write to stderr"); - } } writer.reset().expect("Failed to reset writer"); } @@ -282,7 +276,7 @@ fn display_test_report( write!(writer, "[{}] ", package.name).expect("Failed to write to stderr"); let count_failed = test_report.iter().filter(|(_, status)| status.failed()).count(); - let count_passed = test_report.iter().filter(|(_, status)| status.pass()).count(); + let count_passed = test_report.iter().filter(|(_, status)| status.passed()).count(); let count_skipped = test_report.iter().filter(|(_, status)| status.skipped()).count(); let plural_failed = if count_failed == 1 { "" } else { "s" }; let plural_passed = if count_passed == 1 { "" } else { "s" }; From 96368f5ef8ace8aa1189e81f1d4098746f95c393 Mon Sep 17 00:00:00 2001 From: guipublic Date: Tue, 3 Dec 2024 18:26:16 +0000 Subject: [PATCH 05/11] fix merge issue --- tooling/nargo_cli/tests/stdlib-tests.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tooling/nargo_cli/tests/stdlib-tests.rs b/tooling/nargo_cli/tests/stdlib-tests.rs index 8d73290974e..99f0c9a2e7f 100644 --- a/tooling/nargo_cli/tests/stdlib-tests.rs +++ b/tooling/nargo_cli/tests/stdlib-tests.rs @@ -152,12 +152,6 @@ fn display_test_report( compile_options.silence_warnings, ); } - TestStatus::Skipped { .. } => { - writer - .set_color(ColorSpec::new().set_fg(Some(Color::Yellow))) - .expect("Failed to set color"); - writeln!(writer, "skipped").expect("Failed to write to stderr"); - } } writer.reset().expect("Failed to reset writer"); } From 071b2a3f981b9d8c955c6460a92c5d06e114335f Mon Sep 17 00:00:00 2001 From: guipublic Date: Wed, 4 Dec 2024 10:43:56 +0000 Subject: [PATCH 06/11] code review --- tooling/nargo_cli/src/cli/test_cmd.rs | 51 ++++++++++++--------------- 1 file changed, 22 insertions(+), 29 deletions(-) diff --git a/tooling/nargo_cli/src/cli/test_cmd.rs b/tooling/nargo_cli/src/cli/test_cmd.rs index 20c80543e9c..c998b15b7dd 100644 --- a/tooling/nargo_cli/src/cli/test_cmd.rs +++ b/tooling/nargo_cli/src/cli/test_cmd.rs @@ -281,40 +281,33 @@ fn display_test_report( let plural_failed = if count_failed == 1 { "" } else { "s" }; let plural_passed = if count_passed == 1 { "" } else { "s" }; let plural_skipped = if count_skipped == 1 { "" } else { "s" }; - - if count_failed == 0 { + let mut previous = false; + if count_passed > 0 { writer.set_color(ColorSpec::new().set_fg(Some(Color::Green))).expect("Failed to set color"); - write!(writer, "{count_passed} test{plural_passed} passed") + write!(writer, "{count_passed} test{plural_passed} passed",) .expect("Failed to write to stderr"); - if count_skipped > 0 { - writer - .set_color(ColorSpec::new().set_fg(Some(Color::Yellow))) - .expect("Failed to set color"); - write!(writer, ", {count_skipped} test{plural_skipped} skipped") - .expect("Failed to write to stderr"); - } - writer.reset().expect("Failed to reset writer"); - writeln!(writer).expect("Failed to write to stderr"); - } else { - if count_passed != 0 { - writer - .set_color(ColorSpec::new().set_fg(Some(Color::Green))) - .expect("Failed to set color"); - write!(writer, "{count_passed} test{plural_passed} passed, ",) - .expect("Failed to write to stderr"); - } - if count_skipped != 0 { - writer - .set_color(ColorSpec::new().set_fg(Some(Color::Yellow))) - .expect("Failed to set color"); - write!(writer, ", {count_skipped} test{plural_skipped} skipped") - .expect("Failed to write to stderr"); + previous = true; + } + if count_skipped > 0 { + writer + .set_color(ColorSpec::new().set_fg(Some(Color::Yellow))) + .expect("Failed to set color"); + if previous { + write!(writer, ", ").expect("Failed to write to stderr"); } + write!(writer, "{count_skipped} test{plural_skipped} skipped") + .expect("Failed to write to stderr"); + previous = true; + } + if count_failed > 0 { writer.set_color(ColorSpec::new().set_fg(Some(Color::Red))).expect("Failed to set color"); - writeln!(writer, "{count_failed} test{plural_failed} failed") + if previous { + write!(writer, ", ").expect("Failed to write to stderr"); + } + write!(writer, "{count_failed} test{plural_failed} failed") .expect("Failed to write to stderr"); - writer.reset().expect("Failed to reset writer"); } - + writeln!(writer).expect("Failed to write to stderr"); + writer.reset().expect("Failed to reset writer"); Ok(()) } From af3bc52fde7a044aa2e7a8a5dd4ae8e0f8cbd4e4 Mon Sep 17 00:00:00 2001 From: guipublic Date: Wed, 4 Dec 2024 11:16:34 +0000 Subject: [PATCH 07/11] skip_oracle as a test option --- compiler/noirc_driver/src/lib.rs | 4 ---- tooling/lsp/src/requests/test_run.rs | 1 + tooling/nargo/src/ops/test.rs | 3 ++- tooling/nargo_cli/src/cli/test_cmd.rs | 10 +++++++++- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/compiler/noirc_driver/src/lib.rs b/compiler/noirc_driver/src/lib.rs index e4348de019e..5bedefaf563 100644 --- a/compiler/noirc_driver/src/lib.rs +++ b/compiler/noirc_driver/src/lib.rs @@ -144,10 +144,6 @@ pub struct CompileOptions { /// A lower value keeps the original program if it was smaller, even if it has more jumps. #[arg(long, hide = true, allow_hyphen_values = true)] pub max_bytecode_increase_percent: Option, - - /// Flag to skip tests using oracles. - #[arg(long, hide = true)] - pub skip_oracle: bool, } pub fn parse_expression_width(input: &str) -> Result { diff --git a/tooling/lsp/src/requests/test_run.rs b/tooling/lsp/src/requests/test_run.rs index 937fdcc0a5e..a0158371243 100644 --- a/tooling/lsp/src/requests/test_run.rs +++ b/tooling/lsp/src/requests/test_run.rs @@ -88,6 +88,7 @@ fn on_test_run_request_inner( None, Some(workspace.root_dir.clone()), Some(package.name.to_string()), + false, &CompileOptions::default(), ); let result = match test_result { diff --git a/tooling/nargo/src/ops/test.rs b/tooling/nargo/src/ops/test.rs index 576175f8a57..1f3202b256c 100644 --- a/tooling/nargo/src/ops/test.rs +++ b/tooling/nargo/src/ops/test.rs @@ -56,6 +56,7 @@ pub fn run_test>( foreign_call_resolver_url: Option<&str>, root_path: Option, package_name: Option, + skip_oracle: bool, config: &CompileOptions, ) -> TestStatus { let test_function_has_no_arguments = context @@ -67,7 +68,7 @@ pub fn run_test>( match compile_no_check(context, config, test_function.get_id(), None, false) { Ok(compiled_program) => { - if config.skip_oracle { + if skip_oracle { let mut has_oracle = false; for brillig_function in &compiled_program.program.unconstrained_functions { match brillig_function.get_oracle_status(ForeignCall::check_oracle_status) { diff --git a/tooling/nargo_cli/src/cli/test_cmd.rs b/tooling/nargo_cli/src/cli/test_cmd.rs index c998b15b7dd..f868ebf9e0d 100644 --- a/tooling/nargo_cli/src/cli/test_cmd.rs +++ b/tooling/nargo_cli/src/cli/test_cmd.rs @@ -49,6 +49,10 @@ pub(crate) struct TestCommand { /// JSON RPC url to solve oracle calls #[clap(long)] oracle_resolver: Option, + + /// Flag to skip tests using oracles. + #[arg(long, hide = true)] + skip_oracle: bool, } pub(crate) fn run(args: TestCommand, config: NargoConfig) -> Result<(), CliError> { @@ -76,7 +80,6 @@ pub(crate) fn run(args: TestCommand, config: NargoConfig) -> Result<(), CliError } None => FunctionNameMatch::Anything, }; - // Configure a thread pool with a larger stack size to prevent overflowing stack in large programs. // Default is 2MB. let pool = rayon::ThreadPoolBuilder::new().stack_size(4 * 1024 * 1024).build().unwrap(); @@ -94,6 +97,7 @@ pub(crate) fn run(args: TestCommand, config: NargoConfig) -> Result<(), CliError args.oracle_resolver.as_deref(), Some(workspace.root_dir.clone()), Some(package.name.to_string()), + args.skip_oracle, &args.compile_options, ) }) @@ -133,6 +137,7 @@ fn run_tests + Default>( foreign_call_resolver_url: Option<&str>, root_path: Option, package_name: Option, + skip_oracle: bool, compile_options: &CompileOptions, ) -> Result, CliError> { let test_functions = @@ -155,6 +160,7 @@ fn run_tests + Default>( foreign_call_resolver_url, root_path.clone(), package_name.clone(), + skip_oracle, compile_options, ); @@ -176,6 +182,7 @@ fn run_test + Default>( foreign_call_resolver_url: Option<&str>, root_path: Option, package_name: Option, + skip_oracle: bool, compile_options: &CompileOptions, ) -> TestStatus { // This is really hacky but we can't share `Context` or `S` across threads. @@ -199,6 +206,7 @@ fn run_test + Default>( foreign_call_resolver_url, root_path, package_name, + skip_oracle, compile_options, ) } From fd06d9900b5b5c5b905cf877e24196a86ebddc65 Mon Sep 17 00:00:00 2001 From: guipublic Date: Wed, 4 Dec 2024 14:07:55 +0000 Subject: [PATCH 08/11] fit text case --- tooling/nargo_cli/tests/stdlib-tests.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tooling/nargo_cli/tests/stdlib-tests.rs b/tooling/nargo_cli/tests/stdlib-tests.rs index 99f0c9a2e7f..371d4676fcd 100644 --- a/tooling/nargo_cli/tests/stdlib-tests.rs +++ b/tooling/nargo_cli/tests/stdlib-tests.rs @@ -90,6 +90,7 @@ fn run_stdlib_tests(force_brillig: bool, inliner_aggressiveness: i64) { None, Some(dummy_package.root_dir.clone()), Some(dummy_package.name.to_string()), + false, &CompileOptions { force_brillig, inliner_aggressiveness, ..Default::default() }, ); (test_name, status) From 14bb5ab228fcf0f995bac4b3a07773de6ae5b644 Mon Sep 17 00:00:00 2001 From: guipublic Date: Wed, 4 Dec 2024 14:34:43 +0000 Subject: [PATCH 09/11] use skip-oracle on CI tests --- .github/workflows/test-js-packages.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test-js-packages.yml b/.github/workflows/test-js-packages.yml index 422a30ed08f..8e54c0d48f4 100644 --- a/.github/workflows/test-js-packages.yml +++ b/.github/workflows/test-js-packages.yml @@ -570,9 +570,9 @@ jobs: - name: Run nargo test working-directory: ./test-repo/${{ matrix.project.path }} - run: nargo test --silence-warnings + run: nargo test --silence-warnings --skip-oracle env: - NARGO_IGNORE_TEST_FAILURES_FROM_FOREIGN_CALLS: true + NARGO_IGNORE_TEST_FAILURES_FROM_FOREIGN_CALLS: false # This is a job which depends on all test jobs and reports the overall status. # This allows us to add/remove test jobs without having to update the required workflows. From 7b4185cfb9a8181ea99074dc4c19bc00ea027d81 Mon Sep 17 00:00:00 2001 From: guipublic Date: Thu, 5 Dec 2024 18:09:46 +0000 Subject: [PATCH 10/11] skip failing tests using oracles --- tooling/nargo/src/ops/test.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tooling/nargo/src/ops/test.rs b/tooling/nargo/src/ops/test.rs index 1f3202b256c..62fd95fa1ce 100644 --- a/tooling/nargo/src/ops/test.rs +++ b/tooling/nargo/src/ops/test.rs @@ -73,8 +73,10 @@ pub fn run_test>( for brillig_function in &compiled_program.program.unconstrained_functions { match brillig_function.get_oracle_status(ForeignCall::check_oracle_status) { OracleResult::Mocked => { - has_oracle = false; - break; + if !test_function.should_fail() { + has_oracle = false; + break; + } } OracleResult::Unhandled => { has_oracle = true; From c1fe899b677503e0d0efb8200e0d535450811d7f Mon Sep 17 00:00:00 2001 From: guipublic Date: Fri, 6 Dec 2024 18:04:25 +0000 Subject: [PATCH 11/11] skip oracle based on mock count (test) --- tooling/nargo/src/ops/test.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tooling/nargo/src/ops/test.rs b/tooling/nargo/src/ops/test.rs index 62fd95fa1ce..43d704da0d9 100644 --- a/tooling/nargo/src/ops/test.rs +++ b/tooling/nargo/src/ops/test.rs @@ -70,9 +70,12 @@ pub fn run_test>( Ok(compiled_program) => { if skip_oracle { let mut has_oracle = false; + let mut unhandled = 0; + let mut mocked = 0; for brillig_function in &compiled_program.program.unconstrained_functions { match brillig_function.get_oracle_status(ForeignCall::check_oracle_status) { OracleResult::Mocked => { + mocked += 1; if !test_function.should_fail() { has_oracle = false; break; @@ -80,11 +83,14 @@ pub fn run_test>( } OracleResult::Unhandled => { has_oracle = true; + unhandled += 1; } OracleResult::Handled => (), } } - + if mocked < unhandled { + return TestStatus::Skipped; + } if has_oracle { return TestStatus::Skipped; }