Skip to content

Commit

Permalink
feat: type-check program only once in nargo test
Browse files Browse the repository at this point in the history
  • Loading branch information
@asterite
asterite committed Feb 12, 2025
1 parent 1c5ae80 commit 61e0d00
Show file tree
Hide file tree
Showing 3 changed files with 150 additions and 127 deletions.
2 changes: 1 addition & 1 deletion tooling/nargo/src/ops/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ pub use self::optimize::{optimize_contract, optimize_program};
pub use self::transform::{transform_contract, transform_program};

pub use self::execute::{execute_program, execute_program_with_profiling};
pub use self::test::{run_test, TestStatus};
pub use self::test::{run_compiled_test, run_test, test_status_program_compile_fail, TestStatus};

mod check;
mod compile;
Expand Down
190 changes: 104 additions & 86 deletions tooling/nargo/src/ops/test.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ use acvm::{
AcirField, BlackBoxFunctionSolver, FieldElement,
};
use noirc_abi::Abi;
use noirc_driver::{compile_no_check, CompileError, CompileOptions, DEFAULT_EXPRESSION_WIDTH};
use noirc_driver::{
compile_no_check, CompileError, CompileOptions, CompiledProgram, DEFAULT_EXPRESSION_WIDTH,
};
use noirc_errors::{debug_info::DebugInfo, FileDiagnostic};
use noirc_frontend::hir::{def_map::TestFunction, Context};

Expand Down Expand Up @@ -54,115 +56,131 @@ where
.is_empty();

match compile_no_check(context, config, test_function.get_id(), None, false) {
Ok(compiled_program) => {
// Do the same optimizations as `compile_cmd`.
let target_width = config.expression_width.unwrap_or(DEFAULT_EXPRESSION_WIDTH);
let compiled_program = crate::ops::transform_program(compiled_program, target_width);
Ok(compiled_program) => run_compiled_test(
blackbox_solver,
test_function,
test_function_has_no_arguments,
compiled_program,
output,
config,
build_foreign_call_executor,
),
Err(err) => test_status_program_compile_fail(err, test_function),
}
}

pub fn run_compiled_test<'a, B, F, E>(
blackbox_solver: &B,
test_function: &TestFunction,
test_function_has_no_arguments: bool,
compiled_program: CompiledProgram,
output: PrintOutput<'a>,
config: &CompileOptions,
build_foreign_call_executor: F,
) -> TestStatus
where
B: BlackBoxFunctionSolver<FieldElement>,
F: Fn(PrintOutput<'a>, layers::Unhandled) -> E + 'a,
E: ForeignCallExecutor<FieldElement>,
{
// Do the same optimizations as `compile_cmd`.
let target_width = config.expression_width.unwrap_or(DEFAULT_EXPRESSION_WIDTH);
let compiled_program = crate::ops::transform_program(compiled_program, target_width);

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.
// Use a base layer that doesn't handle anything, which we handle in the `execute` below.
let inner_executor = build_foreign_call_executor(output, layers::Unhandled);
let mut foreign_call_executor = TestForeignCallExecutor::new(inner_executor);
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.
// Use a base layer that doesn't handle anything, which we handle in the `execute` below.
let inner_executor = build_foreign_call_executor(output, layers::Unhandled);
let mut foreign_call_executor = TestForeignCallExecutor::new(inner_executor);

let circuit_execution = execute_program(
&compiled_program.program,
WitnessMap::new(),
blackbox_solver,
&mut foreign_call_executor,
);
let circuit_execution = execute_program(
&compiled_program.program,
WitnessMap::new(),
blackbox_solver,
&mut foreign_call_executor,
);

let status = test_status_program_compile_pass(
test_function,
&compiled_program.abi,
&compiled_program.debug,
&circuit_execution,
);
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");
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
}
if let TestStatus::Fail { .. } = status {
if ignore_foreign_call_failures
&& foreign_call_executor.encountered_unknown_foreign_call
{
TestStatus::Skipped
} else {
use acvm::acir::circuit::Program;
use noir_fuzzer::FuzzedExecutor;
use proptest::test_runner::Config;
use proptest::test_runner::TestRunner;
status
}
} else {
status
}
} else {
use acvm::acir::circuit::Program;
use noir_fuzzer::FuzzedExecutor;
use proptest::test_runner::Config;
use proptest::test_runner::TestRunner;

let runner =
TestRunner::new(Config { failure_persistence: None, ..Config::default() });
let runner = TestRunner::new(Config { failure_persistence: None, ..Config::default() });

let abi = compiled_program.abi.clone();
let debug = compiled_program.debug.clone();
let abi = compiled_program.abi.clone();
let debug = compiled_program.debug.clone();

let executor = |program: &Program<FieldElement>,
initial_witness: WitnessMap<FieldElement>|
-> Result<WitnessStack<FieldElement>, String> {
// Use a base layer that doesn't handle anything, which we handle in the `execute` below.
let inner_executor =
build_foreign_call_executor(PrintOutput::None, layers::Unhandled);
let executor = |program: &Program<FieldElement>,
initial_witness: WitnessMap<FieldElement>|
-> Result<WitnessStack<FieldElement>, String> {
// Use a base layer that doesn't handle anything, which we handle in the `execute` below.
let inner_executor = build_foreign_call_executor(PrintOutput::None, layers::Unhandled);

let mut foreign_call_executor = TestForeignCallExecutor::new(inner_executor);
let mut foreign_call_executor = TestForeignCallExecutor::new(inner_executor);

let circuit_execution = execute_program(
program,
initial_witness,
blackbox_solver,
&mut foreign_call_executor,
);
let circuit_execution = execute_program(
program,
initial_witness,
blackbox_solver,
&mut foreign_call_executor,
);

// Check if a failure was actually expected.
let status = test_status_program_compile_pass(
test_function,
&abi,
&debug,
&circuit_execution,
);
// Check if a failure was actually expected.
let status =
test_status_program_compile_pass(test_function, &abi, &debug, &circuit_execution);

if let TestStatus::Fail { message, error_diagnostic: _ } = status {
Err(message)
} else {
// The fuzzer doesn't care about the actual result.
Ok(WitnessStack::default())
}
};
if let TestStatus::Fail { message, error_diagnostic: _ } = status {
Err(message)
} else {
// The fuzzer doesn't care about the actual result.
Ok(WitnessStack::default())
}
};

let fuzzer = FuzzedExecutor::new(compiled_program.into(), executor, runner);
let fuzzer = FuzzedExecutor::new(compiled_program.into(), executor, runner);

let result = fuzzer.fuzz();
if result.success {
TestStatus::Pass
} else {
TestStatus::Fail {
message: result.reason.unwrap_or_default(),
error_diagnostic: None,
}
}
}
let result = fuzzer.fuzz();
if result.success {
TestStatus::Pass
} else {
TestStatus::Fail { message: result.reason.unwrap_or_default(), error_diagnostic: None }
}
Err(err) => test_status_program_compile_fail(err, test_function),
}
}

/// Test function failed to compile
///
/// Note: This could be because the compiler was able to deduce
/// that a constraint was never satisfiable.
/// An example of this is the program `assert(false)`
/// In that case, we check if the test function should fail, and if so, we return `TestStatus::Pass`.
fn test_status_program_compile_fail(err: CompileError, test_function: &TestFunction) -> TestStatus {
pub fn test_status_program_compile_fail(
err: CompileError,
test_function: &TestFunction,
) -> TestStatus {
// The test has failed compilation, but it should never fail. Report error.
if !test_function.should_fail() {
return TestStatus::CompileError(err.into());
Expand Down
85 changes: 45 additions & 40 deletions tooling/nargo_cli/src/cli/test_cmd.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,17 @@ use clap::Args;
use fm::FileManager;
use formatters::{Formatter, JsonFormatter, PrettyFormatter, TerseFormatter};
use nargo::{
foreign_calls::DefaultForeignCallBuilder, insert_all_files_for_workspace_into_file_manager,
ops::TestStatus, package::Package, parse_all, prepare_package, workspace::Workspace,
foreign_calls::DefaultForeignCallBuilder,
insert_all_files_for_workspace_into_file_manager,
ops::{test_status_program_compile_fail, TestStatus},
package::Package,
parse_all, prepare_package,
workspace::Workspace,
PrintOutput,
};
use nargo_toml::PackageSelection;
use noirc_driver::{check_crate, CompileOptions};
use noirc_frontend::hir::{FunctionNameMatch, ParsedFiles};
use noirc_driver::{compile_no_check, CompileOptions, CompiledProgram};
use noirc_frontend::hir::{def_map::TestFunction, FunctionNameMatch, ParsedFiles};

use crate::{cli::check_cmd::check_crate_and_report_errors, errors::CliError};

Expand Down Expand Up @@ -455,23 +459,47 @@ impl<'a> TestRunner<'a> {
root_path: Option<PathBuf>,
package_name: String,
) -> Result<Vec<Test<'a>>, CliError> {
let test_functions = self.get_tests_in_package(package)?;
let (mut context, crate_id) =
prepare_package(self.file_manager, self.parsed_files, package);
check_crate_and_report_errors(&mut context, crate_id, &self.args.compile_options)?;

let test_functions =
context.get_all_test_functions_in_crate_matching(&crate_id, &self.pattern);

let tests: Vec<Test> = test_functions
.into_iter()
.map(|test_name| {
.map(|(test_name, test_function)| {
let compiled_program_result = compile_no_check(
&mut context,
&self.args.compile_options,
test_function.get_id(),
None,
false,
);
let compiled_program_result = compiled_program_result
.map_err(|err| test_status_program_compile_fail(err, &test_function));

let test_function_has_no_arguments = context
.def_interner
.function_meta(&test_function.get_id())
.function_signature()
.0
.is_empty();

let test_name_copy = test_name.clone();
let root_path = root_path.clone();
let package_name_clone = package_name.clone();
let package_name_clone2 = package_name.clone();
let runner = Box::new(move || {
self.run_test::<S>(
package,
&test_name,
let runner = Box::new(move || match compiled_program_result {
Ok(compiled_program) => self.run_test::<S>(
foreign_call_resolver_url,
root_path,
package_name_clone.clone(),
)
&test_function,
compiled_program,
test_function_has_no_arguments,
),
Err(test_failure) => (test_failure, String::new()),
});
Test { name: test_name_copy, package_name: package_name_clone2, runner }
})
Expand All @@ -480,48 +508,25 @@ impl<'a> TestRunner<'a> {
Ok(tests)
}

/// Compiles a single package and returns all of its test names
fn get_tests_in_package(&'a self, package: &'a Package) -> Result<Vec<String>, CliError> {
let (mut context, crate_id) =
prepare_package(self.file_manager, self.parsed_files, package);
check_crate_and_report_errors(&mut context, crate_id, &self.args.compile_options)?;

Ok(context
.get_all_test_functions_in_crate_matching(&crate_id, &self.pattern)
.into_iter()
.map(|(test_name, _)| test_name)
.collect())
}

/// Runs a single test and returns its status together with whatever was printed to stdout
/// during the test.
fn run_test<S: BlackBoxFunctionSolver<FieldElement> + Default>(
&'a self,
package: &Package,
fn_name: &str,
foreign_call_resolver_url: Option<&str>,
root_path: Option<PathBuf>,
package_name: String,
test_function: &TestFunction,
compiled_program: CompiledProgram,
test_function_has_no_arguments: bool,
) -> (TestStatus, String) {
// This is really hacky but we can't share `Context` or `S` across threads.
// We then need to construct a separate copy for each test.

let (mut context, crate_id) =
prepare_package(self.file_manager, self.parsed_files, package);
check_crate(&mut context, crate_id, &self.args.compile_options)
.expect("Any errors should have occurred when collecting test functions");

let pattern = FunctionNameMatch::Exact(vec![fn_name.to_string()]);
let test_functions = context.get_all_test_functions_in_crate_matching(&crate_id, &pattern);
let (_, test_function) = test_functions.first().expect("Test function should exist");

let blackbox_solver = S::default();
let mut output_string = String::new();

let test_status = nargo::ops::run_test(
let test_status = nargo::ops::run_compiled_test(
&blackbox_solver,
&mut context,
test_function,
test_function_has_no_arguments,
compiled_program,
PrintOutput::String(&mut output_string),
&self.args.compile_options,
|output, base| {
Expand Down

0 comments on commit 61e0d00

Please sign in to comment.