diff --git a/.github/workflows/test-js-packages.yml b/.github/workflows/test-js-packages.yml index 4a5d0b8179b..422a30ed08f 100644 --- a/.github/workflows/test-js-packages.yml +++ b/.github/workflows/test-js-packages.yml @@ -519,12 +519,25 @@ jobs: fail-fast: false matrix: project: - # Disabled as these are currently failing with many visibility errors - - { repo: AztecProtocol/aztec-nr, path: ./ } + - { repo: noir-lang/ec, path: ./ } + - { repo: noir-lang/eddsa, path: ./ } + - { repo: noir-lang/mimc, path: ./ } + - { repo: noir-lang/noir_sort, path: ./ } + - { repo: noir-lang/noir-edwards, path: ./ } + - { repo: noir-lang/noir-bignum, path: ./ } + - { repo: noir-lang/noir_bigcurve, path: ./ } + - { repo: noir-lang/noir_base64, path: ./ } + - { repo: noir-lang/noir_string_search, path: ./ } + - { repo: noir-lang/sparse_array, path: ./ } + - { repo: noir-lang/noir_rsa, path: ./lib } + - { repo: AztecProtocol/aztec-packages, path: ./noir-projects/aztec-nr } - { repo: AztecProtocol/aztec-packages, path: ./noir-projects/noir-contracts } - # Disabled as aztec-packages requires a setup-step in order to generate a `Nargo.toml` - #- { repo: AztecProtocol/aztec-packages, path: ./noir-projects/noir-protocol-circuits } - - { repo: noir-lang/noir-edwards, path: ./, ref: 3188ea74fe3b059219a2ea87899589c266256d74 } + - { repo: AztecProtocol/aztec-packages, path: ./noir-projects/noir-protocol-circuits/crates/parity-lib } + - { repo: AztecProtocol/aztec-packages, path: ./noir-projects/noir-protocol-circuits/crates/private-kernel-lib } + - { repo: AztecProtocol/aztec-packages, path: ./noir-projects/noir-protocol-circuits/crates/reset-kernel-lib } + - { repo: AztecProtocol/aztec-packages, path: ./noir-projects/noir-protocol-circuits/crates/rollup-lib } + - { repo: AztecProtocol/aztec-packages, path: ./noir-projects/noir-protocol-circuits/crates/types } + name: Check external repo - ${{ matrix.project.repo }} steps: - name: Checkout @@ -554,9 +567,12 @@ jobs: # Github actions seems to not expand "**" in globs by default. shopt -s globstar sed -i '/^compiler_version/d' ./**/Nargo.toml - - name: Run nargo check + + - name: Run nargo test working-directory: ./test-repo/${{ matrix.project.path }} - run: nargo check + run: nargo test --silence-warnings + env: + NARGO_IGNORE_TEST_FAILURES_FROM_FOREIGN_CALLS: true # 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/tooling/lsp/src/requests/test_run.rs b/tooling/lsp/src/requests/test_run.rs index 50c699bb6a6..937fdcc0a5e 100644 --- a/tooling/lsp/src/requests/test_run.rs +++ b/tooling/lsp/src/requests/test_run.rs @@ -101,6 +101,11 @@ fn on_test_run_request_inner( result: "fail".to_string(), message: Some(message), }, + TestStatus::Skipped => NargoTestRunResult { + id: params.id.clone(), + result: "skipped".to_string(), + message: None, + }, TestStatus::CompileError(diag) => NargoTestRunResult { id: params.id.clone(), result: "error".to_string(), diff --git a/tooling/nargo/src/foreign_calls/mod.rs b/tooling/nargo/src/foreign_calls/mod.rs index 0ef3247ee59..16ed71e11e3 100644 --- a/tooling/nargo/src/foreign_calls/mod.rs +++ b/tooling/nargo/src/foreign_calls/mod.rs @@ -8,9 +8,9 @@ use rand::Rng; use rpc::RPCForeignCallExecutor; use serde::{Deserialize, Serialize}; -mod mocker; -mod print; -mod rpc; +pub(crate) mod mocker; +pub(crate) mod print; +pub(crate) mod rpc; pub trait ForeignCallExecutor { fn execute( diff --git a/tooling/nargo/src/foreign_calls/print.rs b/tooling/nargo/src/foreign_calls/print.rs index d0befa0bd0b..92fcd65ae28 100644 --- a/tooling/nargo/src/foreign_calls/print.rs +++ b/tooling/nargo/src/foreign_calls/print.rs @@ -4,7 +4,7 @@ use noirc_printable_type::{ForeignCallError, PrintableValueDisplay}; use super::{ForeignCall, ForeignCallExecutor}; #[derive(Debug, Default)] -pub(super) struct PrintForeignCallExecutor; +pub(crate) struct PrintForeignCallExecutor; impl ForeignCallExecutor for PrintForeignCallExecutor { fn execute( diff --git a/tooling/nargo/src/foreign_calls/rpc.rs b/tooling/nargo/src/foreign_calls/rpc.rs index dab64819b56..0653eb1c7e3 100644 --- a/tooling/nargo/src/foreign_calls/rpc.rs +++ b/tooling/nargo/src/foreign_calls/rpc.rs @@ -8,7 +8,7 @@ use serde::{Deserialize, Serialize}; use super::ForeignCallExecutor; #[derive(Debug)] -pub(super) struct RPCForeignCallExecutor { +pub(crate) struct RPCForeignCallExecutor { /// A randomly generated id for this `DefaultForeignCallExecutor`. /// /// This is used so that a single `external_resolver` can distinguish between requests from multiple @@ -44,7 +44,7 @@ struct ResolveForeignCallRequest { } impl RPCForeignCallExecutor { - pub(super) fn new( + pub(crate) fn new( resolver_url: &str, id: u64, root_path: Option, diff --git a/tooling/nargo/src/ops/test.rs b/tooling/nargo/src/ops/test.rs index 51d7b8c55aa..6e7a7d1b7db 100644 --- a/tooling/nargo/src/ops/test.rs +++ b/tooling/nargo/src/ops/test.rs @@ -1,16 +1,28 @@ use std::path::PathBuf; use acvm::{ - acir::native_types::{WitnessMap, WitnessStack}, - BlackBoxFunctionSolver, FieldElement, + acir::{ + brillig::ForeignCallResult, + native_types::{WitnessMap, WitnessStack}, + }, + pwg::ForeignCallWaitInfo, + AcirField, BlackBoxFunctionSolver, FieldElement, }; use noirc_abi::Abi; use noirc_driver::{compile_no_check, CompileError, CompileOptions}; use noirc_errors::{debug_info::DebugInfo, FileDiagnostic}; use noirc_frontend::hir::{def_map::TestFunction, Context}; +use noirc_printable_type::ForeignCallError; +use rand::Rng; +use serde::{Deserialize, Serialize}; use crate::{ - errors::try_to_diagnose_runtime_error, foreign_calls::DefaultForeignCallExecutor, NargoError, + errors::try_to_diagnose_runtime_error, + foreign_calls::{ + mocker::MockForeignCallExecutor, print::PrintForeignCallExecutor, + rpc::RPCForeignCallExecutor, DefaultForeignCallExecutor, ForeignCall, ForeignCallExecutor, + }, + NargoError, }; use super::execute_program; @@ -18,12 +30,13 @@ use super::execute_program; 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::Pass | TestStatus::Skipped) } } @@ -50,23 +63,42 @@ pub fn run_test>( 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. + let mut foreign_call_executor = TestForeignCallExecutor::new( + show_output, + foreign_call_resolver_url, + root_path, + package_name, + ); + let circuit_execution = execute_program( &compiled_program.program, WitnessMap::new(), blackbox_solver, - &mut DefaultForeignCallExecutor::new( - show_output, - foreign_call_resolver_url, - root_path, - package_name, - ), + &mut foreign_call_executor, ); - test_status_program_compile_pass( + + let status = test_status_program_compile_pass( test_function, compiled_program.abi, compiled_program.debug, circuit_execution, - ) + ); + + let ignore_foreign_call_failures = + std::env::var("NARGO_IGNORE_TEST_FAILURES_FROM_FOREIGN_CALLS") + .is_ok_and(|var| &var == "true"); + + if let TestStatus::Fail { .. } = status { + if ignore_foreign_call_failures + && foreign_call_executor.encountered_unknown_foreign_call + { + TestStatus::Skipped + } else { + status + } + } else { + status + } } else { #[cfg(target_arch = "wasm32")] { @@ -217,3 +249,93 @@ fn check_expected_failure_message( error_diagnostic, } } + +/// A specialized foreign call executor which tracks whether it has encountered any unknown foreign calls +struct TestForeignCallExecutor { + /// The executor for any [`ForeignCall::Print`] calls. + printer: Option, + mocker: MockForeignCallExecutor, + external: Option, + + encountered_unknown_foreign_call: bool, +} + +impl TestForeignCallExecutor { + fn new( + show_output: bool, + resolver_url: Option<&str>, + root_path: Option, + package_name: Option, + ) -> Self { + let id = rand::thread_rng().gen(); + let printer = if show_output { Some(PrintForeignCallExecutor) } else { None }; + let external_resolver = resolver_url.map(|resolver_url| { + RPCForeignCallExecutor::new(resolver_url, id, root_path, package_name) + }); + TestForeignCallExecutor { + printer, + mocker: MockForeignCallExecutor::default(), + external: external_resolver, + encountered_unknown_foreign_call: false, + } + } +} + +impl Deserialize<'a>> ForeignCallExecutor + for TestForeignCallExecutor +{ + fn execute( + &mut self, + foreign_call: &ForeignCallWaitInfo, + ) -> Result, ForeignCallError> { + // If the circuit has reached a new foreign call opcode then it can't have failed from any previous unknown foreign calls. + self.encountered_unknown_foreign_call = false; + + let foreign_call_name = foreign_call.function.as_str(); + match ForeignCall::lookup(foreign_call_name) { + Some(ForeignCall::Print) => { + if let Some(printer) = &mut self.printer { + printer.execute(foreign_call) + } else { + Ok(ForeignCallResult::default()) + } + } + + Some( + ForeignCall::CreateMock + | ForeignCall::SetMockParams + | ForeignCall::GetMockLastParams + | ForeignCall::SetMockReturns + | ForeignCall::SetMockTimes + | ForeignCall::ClearMock, + ) => self.mocker.execute(foreign_call), + + None => { + // First check if there's any defined mock responses for this foreign call. + match self.mocker.execute(foreign_call) { + Err(ForeignCallError::NoHandler(_)) => (), + response_or_error => return response_or_error, + }; + + if let Some(external_resolver) = &mut self.external { + // If the user has registered an external resolver then we forward any remaining oracle calls there. + match external_resolver.execute(foreign_call) { + Err(ForeignCallError::NoHandler(_)) => (), + response_or_error => return response_or_error, + }; + } + + self.encountered_unknown_foreign_call = true; + + // If all executors have no handler for the given foreign call then we cannot + // return a correct response to the ACVM. The best we can do is to return an empty response, + // this allows us to ignore any foreign calls which exist solely to pass information from inside + // the circuit to the environment (e.g. custom logging) as the execution will still be able to progress. + // + // We optimistically return an empty response for all oracle calls as the ACVM will error + // should a response have been required. + Ok(ForeignCallResult::default()) + } + } + } +} diff --git a/tooling/nargo_cli/src/cli/test_cmd.rs b/tooling/nargo_cli/src/cli/test_cmd.rs index 7b0201226ef..aa0ee1bb94b 100644 --- a/tooling/nargo_cli/src/cli/test_cmd.rs +++ b/tooling/nargo_cli/src/cli/test_cmd.rs @@ -255,6 +255,12 @@ fn display_test_report( ); } } + 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"); + } TestStatus::CompileError(err) => { noirc_errors::reporter::report_all( file_manager.as_file_map(), diff --git a/tooling/nargo_cli/tests/stdlib-tests.rs b/tooling/nargo_cli/tests/stdlib-tests.rs index bdc92e625ab..99f0c9a2e7f 100644 --- a/tooling/nargo_cli/tests/stdlib-tests.rs +++ b/tooling/nargo_cli/tests/stdlib-tests.rs @@ -138,6 +138,12 @@ fn display_test_report( ); } } + 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"); + } TestStatus::CompileError(err) => { noirc_errors::reporter::report_all( file_manager.as_file_map(),