From ab34fdc5653415325480b46cf3edfbde45fdd588 Mon Sep 17 00:00:00 2001 From: Akosh Farkash Date: Wed, 18 Dec 2024 16:06:44 +0000 Subject: [PATCH] Make run_test generic in ForeignCallExecutor --- tooling/lsp/src/requests/test_run.rs | 13 +++-- tooling/nargo/src/ops/test.rs | 69 ++++++++++--------------- tooling/nargo_cli/src/cli/test_cmd.rs | 17 ++++-- tooling/nargo_cli/tests/stdlib-tests.rs | 13 +++-- 4 files changed, 58 insertions(+), 54 deletions(-) diff --git a/tooling/lsp/src/requests/test_run.rs b/tooling/lsp/src/requests/test_run.rs index 72ae6695b82..56fcb0325c3 100644 --- a/tooling/lsp/src/requests/test_run.rs +++ b/tooling/lsp/src/requests/test_run.rs @@ -3,6 +3,7 @@ use std::future::{self, Future}; use crate::insert_all_files_for_workspace_into_file_manager; use async_lsp::{ErrorCode, ResponseError}; use nargo::{ + foreign_calls::DefaultForeignCallExecutor, ops::{run_test, TestStatus}, PrintOutput, }; @@ -88,10 +89,16 @@ fn on_test_run_request_inner( &mut context, &test_function, PrintOutput::Stdout, - None, - Some(workspace.root_dir.clone()), - Some(package.name.to_string()), &CompileOptions::default(), + |output, base| { + DefaultForeignCallExecutor::with_base( + base, + output, + None, + Some(workspace.root_dir.clone()), + Some(package.name.to_string()), + ) + }, ); let result = match test_result { TestStatus::Pass => NargoTestRunResult { diff --git a/tooling/nargo/src/ops/test.rs b/tooling/nargo/src/ops/test.rs index 56afbbf29ec..2b4bd080d26 100644 --- a/tooling/nargo/src/ops/test.rs +++ b/tooling/nargo/src/ops/test.rs @@ -1,5 +1,3 @@ -use std::path::PathBuf; - use acvm::{ acir::{ brillig::ForeignCallResult, @@ -17,10 +15,7 @@ use serde::{Deserialize, Serialize}; use crate::{ errors::try_to_diagnose_runtime_error, - foreign_calls::{ - layers, print::PrintOutput, DefaultForeignCallExecutor, DefaultForeignCallLayers, - ForeignCallExecutor, - }, + foreign_calls::{layers, print::PrintOutput, ForeignCallExecutor}, NargoError, }; @@ -41,16 +36,19 @@ impl TestStatus { } #[allow(clippy::too_many_arguments)] -pub fn run_test>( +pub fn run_test<'a, B, F, E>( blackbox_solver: &B, context: &mut Context, test_function: &TestFunction, - output: PrintOutput<'_>, - foreign_call_resolver_url: Option<&str>, - root_path: Option, - package_name: Option, + output: PrintOutput<'a>, config: &CompileOptions, -) -> TestStatus { + foreign_call_executor: F, +) -> TestStatus +where + B: BlackBoxFunctionSolver, + F: Fn(PrintOutput<'a>, layers::Unhandled) -> E + 'a, + E: ForeignCallExecutor, +{ let test_function_has_no_arguments = context .def_interner .function_meta(&test_function.get_id()) @@ -67,12 +65,8 @@ 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( - output, - foreign_call_resolver_url, - root_path, - package_name, - ); + let mut foreign_call_executor = + TestForeignCallExecutor::new(output, &foreign_call_executor); let circuit_execution = execute_program( &compiled_program.program, @@ -134,11 +128,9 @@ pub fn run_test>( program, initial_witness, blackbox_solver, - &mut TestForeignCallExecutor::::new( + &mut TestForeignCallExecutor::new( PrintOutput::None, - foreign_call_resolver_url, - root_path.clone(), - package_name.clone(), + &foreign_call_executor, ), ); @@ -275,37 +267,28 @@ fn check_expected_failure_message( } /// A specialized foreign call executor which tracks whether it has encountered any unknown foreign calls -struct TestForeignCallExecutor<'a, F> { - executor: DefaultForeignCallLayers<'a, layers::Unhandled, F>, +struct TestForeignCallExecutor { + executor: E, encountered_unknown_foreign_call: bool, } -impl<'a, F> TestForeignCallExecutor<'a, F> -where - F: Default + AcirField + Serialize + for<'de> Deserialize<'de> + 'a, -{ +impl TestForeignCallExecutor { #[allow(clippy::new_ret_no_self)] - fn new( - output: PrintOutput<'a>, - resolver_url: Option<&str>, - root_path: Option, - package_name: Option, - ) -> Self { + fn new<'a, F>(output: PrintOutput<'a>, foreign_call_executor: &F) -> Self + where + F: Fn(PrintOutput<'a>, layers::Unhandled) -> E + 'a, + { // Use a base layer that doesn't handle anything, which we handle in the `execute` below. - let executor = DefaultForeignCallExecutor::with_base( - layers::Unhandled, - output, - resolver_url, - root_path, - package_name, - ); + let executor = foreign_call_executor(output, layers::Unhandled); Self { executor, encountered_unknown_foreign_call: false } } } -impl<'a, F: AcirField + Serialize + for<'b> Deserialize<'b>> ForeignCallExecutor - for TestForeignCallExecutor<'a, F> +impl ForeignCallExecutor for TestForeignCallExecutor +where + F: AcirField + Serialize + for<'b> Deserialize<'b>, + E: ForeignCallExecutor, { fn execute( &mut self, diff --git a/tooling/nargo_cli/src/cli/test_cmd.rs b/tooling/nargo_cli/src/cli/test_cmd.rs index 1fd4ed2d873..f4f50acd37c 100644 --- a/tooling/nargo_cli/src/cli/test_cmd.rs +++ b/tooling/nargo_cli/src/cli/test_cmd.rs @@ -14,8 +14,9 @@ use clap::Args; use fm::FileManager; use formatters::{Formatter, JsonFormatter, PrettyFormatter, TerseFormatter}; use nargo::{ - insert_all_files_for_workspace_into_file_manager, ops::TestStatus, package::Package, parse_all, - prepare_package, workspace::Workspace, PrintOutput, + foreign_calls::DefaultForeignCallExecutor, insert_all_files_for_workspace_into_file_manager, + ops::TestStatus, package::Package, parse_all, prepare_package, workspace::Workspace, + PrintOutput, }; use nargo_toml::{get_package_manifest, resolve_workspace_from_toml}; use noirc_driver::{check_crate, CompileOptions, NOIR_ARTIFACT_VERSION_STRING}; @@ -494,10 +495,16 @@ impl<'a> TestRunner<'a> { &mut context, test_function, PrintOutput::String(&mut output_string), - foreign_call_resolver_url, - root_path, - Some(package_name), &self.args.compile_options, + |output, base| { + DefaultForeignCallExecutor::with_base( + base, + output, + foreign_call_resolver_url, + root_path.clone(), + Some(package_name.clone()), + ) + }, ); (test_status, output_string) } diff --git a/tooling/nargo_cli/tests/stdlib-tests.rs b/tooling/nargo_cli/tests/stdlib-tests.rs index 29b871814b8..bd08b0fd156 100644 --- a/tooling/nargo_cli/tests/stdlib-tests.rs +++ b/tooling/nargo_cli/tests/stdlib-tests.rs @@ -2,6 +2,7 @@ #![allow(clippy::items_after_test_module)] use clap::Parser; use fm::FileManager; +use nargo::foreign_calls::DefaultForeignCallExecutor; use nargo::PrintOutput; use noirc_driver::{check_crate, file_manager_with_stdlib, CompileOptions}; use noirc_frontend::hir::FunctionNameMatch; @@ -88,10 +89,16 @@ fn run_stdlib_tests(force_brillig: bool, inliner_aggressiveness: i64) { &mut context, &test_function, PrintOutput::Stdout, - None, - Some(dummy_package.root_dir.clone()), - Some(dummy_package.name.to_string()), &CompileOptions { force_brillig, inliner_aggressiveness, ..Default::default() }, + |output, base| { + DefaultForeignCallExecutor::with_base( + base, + output, + None, + Some(dummy_package.root_dir.clone()), + Some(dummy_package.name.to_string()), + ) + }, ); (test_name, status) })