Skip to content

Commit

Permalink
Make run_test generic in ForeignCallExecutor
Browse files Browse the repository at this point in the history
  • Loading branch information
aakoshh committed Dec 18, 2024
1 parent 03fcce6 commit ab34fdc
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 54 deletions.
13 changes: 10 additions & 3 deletions tooling/lsp/src/requests/test_run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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 {
Expand Down
69 changes: 26 additions & 43 deletions tooling/nargo/src/ops/test.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
use std::path::PathBuf;

use acvm::{
acir::{
brillig::ForeignCallResult,
Expand All @@ -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,
};

Expand All @@ -41,16 +36,19 @@ impl TestStatus {
}

#[allow(clippy::too_many_arguments)]
pub fn run_test<B: BlackBoxFunctionSolver<FieldElement>>(
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<PathBuf>,
package_name: Option<String>,
output: PrintOutput<'a>,
config: &CompileOptions,
) -> TestStatus {
foreign_call_executor: F,
) -> TestStatus
where
B: BlackBoxFunctionSolver<FieldElement>,
F: Fn(PrintOutput<'a>, layers::Unhandled) -> E + 'a,
E: ForeignCallExecutor<FieldElement>,
{
let test_function_has_no_arguments = context
.def_interner
.function_meta(&test_function.get_id())
Expand All @@ -67,12 +65,8 @@ pub fn run_test<B: BlackBoxFunctionSolver<FieldElement>>(
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,
Expand Down Expand Up @@ -134,11 +128,9 @@ pub fn run_test<B: BlackBoxFunctionSolver<FieldElement>>(
program,
initial_witness,
blackbox_solver,
&mut TestForeignCallExecutor::<FieldElement>::new(
&mut TestForeignCallExecutor::new(
PrintOutput::None,
foreign_call_resolver_url,
root_path.clone(),
package_name.clone(),
&foreign_call_executor,
),
);

Expand Down Expand Up @@ -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<E> {
executor: E,
encountered_unknown_foreign_call: bool,
}

impl<'a, F> TestForeignCallExecutor<'a, F>
where
F: Default + AcirField + Serialize + for<'de> Deserialize<'de> + 'a,
{
impl<E> TestForeignCallExecutor<E> {
#[allow(clippy::new_ret_no_self)]
fn new(
output: PrintOutput<'a>,
resolver_url: Option<&str>,
root_path: Option<PathBuf>,
package_name: Option<String>,
) -> 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<F>
for TestForeignCallExecutor<'a, F>
impl<E, F> ForeignCallExecutor<F> for TestForeignCallExecutor<E>
where
F: AcirField + Serialize + for<'b> Deserialize<'b>,
E: ForeignCallExecutor<F>,
{
fn execute(
&mut self,
Expand Down
17 changes: 12 additions & 5 deletions tooling/nargo_cli/src/cli/test_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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)
}
Expand Down
13 changes: 10 additions & 3 deletions tooling/nargo_cli/tests/stdlib-tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
})
Expand Down

0 comments on commit ab34fdc

Please sign in to comment.