diff --git a/.github/workflows/test-js-packages.yml b/.github/workflows/test-js-packages.yml index 36ece11b1bf..4ed28c4e68f 100644 --- a/.github/workflows/test-js-packages.yml +++ b/.github/workflows/test-js-packages.yml @@ -591,9 +591,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. diff --git a/acvm-repo/acir/src/circuit/brillig.rs b/acvm-repo/acir/src/circuit/brillig.rs index ef75d088f8c..5d32f91bfcd 100644 --- a/acvm-repo/acir/src/circuit/brillig.rs +++ b/acvm-repo/acir/src/circuit/brillig.rs @@ -28,6 +28,40 @@ 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 + /// - 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) -> OracleResult, + { + let mut result = OracleResult::Handled; + for op in self.bytecode.iter() { + if let BrilligOpcode::ForeignCall { function, .. } = op { + 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. #[derive( Clone, Debug, PartialEq, Eq, Serialize, Deserialize, Hash, Copy, Default, PartialOrd, Ord, 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. 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/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 e258627b522..43d704da0d9 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, @@ -36,7 +37,13 @@ pub enum TestStatus { impl TestStatus { pub fn failed(&self) -> bool { - !matches!(self, TestStatus::Pass | TestStatus::Skipped) + matches!(self, TestStatus::Fail { .. } | TestStatus::CompileError(_)) + } + pub fn passed(&self) -> bool { + matches!(self, TestStatus::Pass) + } + pub fn skipped(&self) -> bool { + matches!(self, TestStatus::Skipped) } } @@ -49,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 @@ -60,6 +68,34 @@ pub fn run_test>( match compile_no_check(context, config, test_function.get_id(), None, false) { 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; + } + } + OracleResult::Unhandled => { + has_oracle = true; + unhandled += 1; + } + OracleResult::Handled => (), + } + } + if mocked < unhandled { + return TestStatus::Skipped; + } + 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 aa0ee1bb94b..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, ) } @@ -275,32 +283,39 @@ fn display_test_report( 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" }; - if count_failed == 0 { + 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" }; + let plural_skipped = if count_skipped == 1 { "" } else { "s" }; + 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_all} test{plural} passed").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))) - .expect("Failed to set color"); - write!(writer, "{count_passed} test{plural_passed} passed, ",) - .expect("Failed to write to stderr"); + write!(writer, "{count_passed} test{plural_passed} passed",) + .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(()) } 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)