Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Nargo test runtime callstacks and assert messages without string matching #2953

Merged
merged 3 commits into from
Oct 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 5 additions & 3 deletions compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use fm::FileId;
use noirc_abi::{AbiParameter, AbiType, ContractEvent};
use noirc_errors::{CustomDiagnostic, FileDiagnostic};
use noirc_evaluator::errors::RuntimeError;
use noirc_evaluator::{create_circuit, into_abi_params};
use noirc_frontend::graph::{CrateId, CrateName};
use noirc_frontend::hir::def_map::{Contract, CrateDefMap};
Expand Down Expand Up @@ -164,10 +165,11 @@
}
};

let compiled_program = compile_no_check(context, options, main, cached_program, force_compile)?;
let compiled_program = compile_no_check(context, options, main, cached_program, force_compile)
.map_err(FileDiagnostic::from)?;

if options.print_acir {
println!("Compiled ACIR for main (unoptimized):");

Check warning on line 172 in compiler/noirc_driver/src/lib.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (unoptimized)
println!("{}", compiled_program.circuit);
}

Expand Down Expand Up @@ -216,7 +218,7 @@
if options.print_acir {
for contract_function in &compiled_contract.functions {
println!(
"Compiled ACIR for {}::{} (unoptimized):",

Check warning on line 221 in compiler/noirc_driver/src/lib.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (unoptimized)
compiled_contract.name, contract_function.name
);
println!("{}", contract_function.bytecode);
Expand Down Expand Up @@ -261,7 +263,7 @@
let function = match compile_no_check(context, options, function_id, None, true) {
Ok(function) => function,
Err(new_error) => {
errors.push(new_error);
errors.push(FileDiagnostic::from(new_error));
continue;
}
};
Expand Down Expand Up @@ -316,10 +318,10 @@
main_function: FuncId,
cached_program: Option<CompiledProgram>,
force_compile: bool,
) -> Result<CompiledProgram, FileDiagnostic> {
) -> Result<CompiledProgram, RuntimeError> {
let program = monomorphize(main_function, &context.def_interner);

let hash = fxhash::hash64(&program);

Check warning on line 324 in compiler/noirc_driver/src/lib.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (fxhash)

// If user has specified that they want to see intermediate steps printed then we should
// force compilation even if the program hasn't changed.
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_evaluator/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
#![warn(unreachable_pub)]
#![warn(clippy::semicolon_if_nothing_returned)]

mod errors;
pub mod errors;

// SSA code to create the SSA based IR
// for functions and execute different optimizations.
Expand Down
2 changes: 1 addition & 1 deletion tooling/lsp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@
result: "pass".to_string(),
message: None,
},
TestStatus::Fail { message } => NargoTestRunResult {
TestStatus::Fail { message, .. } => NargoTestRunResult {
id: params.id.clone(),
result: "fail".to_string(),
message: Some(message),
Expand Down Expand Up @@ -806,7 +806,7 @@
}

cfg_if::cfg_if! {
if #[cfg(all(target_arch = "wasm32", not(target_os = "wasi")))] {

Check warning on line 809 in tooling/lsp/src/lib.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck

Unknown word (wasi)
use wasm_bindgen::{prelude::*, JsValue};

#[wasm_bindgen(module = "@noir-lang/source-resolver")]
Expand Down
1 change: 1 addition & 0 deletions tooling/nargo/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ fm.workspace = true
noirc_abi.workspace = true
noirc_driver.workspace = true
noirc_errors.workspace = true
noirc_evaluator.workspace = true
noirc_frontend.workspace = true
noirc_printable_type.workspace = true
iter-extended.workspace = true
Expand Down
82 changes: 81 additions & 1 deletion tooling/nargo/src/errors.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
use acvm::{acir::circuit::OpcodeLocation, pwg::OpcodeResolutionError};
use acvm::{
acir::circuit::OpcodeLocation,
pwg::{ErrorLocation, OpcodeResolutionError},
};
use noirc_errors::{debug_info::DebugInfo, CustomDiagnostic, FileDiagnostic, Location};
use noirc_printable_type::ForeignCallError;
use thiserror::Error;

Expand Down Expand Up @@ -57,3 +61,79 @@ pub enum ExecutionError {
#[error(transparent)]
SolvingError(#[from] OpcodeResolutionError),
}

/// Extracts the opcode locations from a nargo error.
fn extract_locations_from_error(
error: &ExecutionError,
debug: &DebugInfo,
) -> Option<Vec<Location>> {
let mut opcode_locations = match error {
ExecutionError::SolvingError(OpcodeResolutionError::BrilligFunctionFailed {
call_stack,
..
})
| ExecutionError::AssertionFailed(_, call_stack) => Some(call_stack.clone()),
ExecutionError::SolvingError(OpcodeResolutionError::IndexOutOfBounds {
opcode_location: error_location,
..
})
| ExecutionError::SolvingError(OpcodeResolutionError::UnsatisfiedConstrain {
opcode_location: error_location,
}) => match error_location {
ErrorLocation::Unresolved => {
unreachable!("Cannot resolve index for unsatisfied constraint")
}
ErrorLocation::Resolved(opcode_location) => Some(vec![*opcode_location]),
},
_ => None,
}?;

if let Some(OpcodeLocation::Brillig { acir_index, .. }) = opcode_locations.get(0) {
opcode_locations.insert(0, OpcodeLocation::Acir(*acir_index));
}

Some(
opcode_locations
.iter()
.flat_map(|opcode_location| debug.opcode_location(opcode_location).unwrap_or_default())
.collect(),
)
}

/// Tries to generate a runtime diagnostic from a nargo error. It will successfully do so if it's a runtime error with a call stack.
pub fn try_to_diagnose_runtime_error(
nargo_err: &NargoError,
debug: &DebugInfo,
) -> Option<FileDiagnostic> {
let execution_error = match nargo_err {
NargoError::ExecutionError(execution_error) => execution_error,
_ => return None,
};

let source_locations = extract_locations_from_error(execution_error, debug)?;

// The location of the error itself will be the location at the top
// of the call stack (the last item in the Vec).
let location = source_locations.last()?;

let message = match nargo_err {
NargoError::ExecutionError(ExecutionError::AssertionFailed(message, _)) => {
format!("Assertion failed: '{message}'")
}
NargoError::ExecutionError(ExecutionError::SolvingError(
OpcodeResolutionError::IndexOutOfBounds { index, array_size, .. },
)) => {
format!("Index out of bounds, array has size {array_size:?}, but index was {index:?}")
}
NargoError::ExecutionError(ExecutionError::SolvingError(
OpcodeResolutionError::UnsatisfiedConstrain { .. },
)) => "Failed constraint".into(),
_ => nargo_err.to_string(),
};

Some(
CustomDiagnostic::simple_error(message, String::new(), location.span)
.in_file(location.file)
.with_call_stack(source_locations),
)
}
68 changes: 33 additions & 35 deletions tooling/nargo/src/ops/test.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
use acvm::{acir::native_types::WitnessMap, BlackBoxFunctionSolver};
use noirc_driver::{compile_no_check, CompileOptions};
use noirc_errors::FileDiagnostic;
use noirc_errors::{debug_info::DebugInfo, FileDiagnostic};
use noirc_evaluator::errors::RuntimeError;
use noirc_frontend::hir::{def_map::TestFunction, Context};

use crate::NargoError;
use crate::{errors::try_to_diagnose_runtime_error, NargoError};

use super::execute_circuit;

pub enum TestStatus {
Pass,
Fail { message: String },
Fail { message: String, error_diagnostic: Option<FileDiagnostic> },
CompileError(FileDiagnostic),
}

Expand All @@ -27,9 +28,9 @@ pub fn run_test<B: BlackBoxFunctionSolver>(
// otherwise constraints involving these expressions will not error.
let circuit_execution =
execute_circuit(blackbox_solver, program.circuit, WitnessMap::new(), show_output);
test_status_program_compile_pass(test_function, circuit_execution)
test_status_program_compile_pass(test_function, program.debug, circuit_execution)
}
Err(diag) => test_status_program_compile_fail(diag, test_function),
Err(err) => test_status_program_compile_fail(err, test_function),
}
}

Expand All @@ -39,33 +40,19 @@ pub fn run_test<B: BlackBoxFunctionSolver>(
/// 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(
diag: FileDiagnostic,
test_function: TestFunction,
) -> TestStatus {
fn test_status_program_compile_fail(err: RuntimeError, test_function: TestFunction) -> TestStatus {
// The test has failed compilation, but it should never fail. Report error.
if !test_function.should_fail() {
return TestStatus::CompileError(diag);
}

// The test has failed compilation, check if it is because the program is never satisfiable.
// If it is never satisfiable, then this is the expected behavior.
let program_is_never_satisfiable = diag.diagnostic.message.contains("Failed constraint");
if !program_is_never_satisfiable {
// The test has failed compilation, but its a compilation error. Report error
return TestStatus::CompileError(diag);
return TestStatus::CompileError(err.into());
}

// Given "Failed constraint: 'reason'"
// This method will return "reason"
fn extract_constraint_error_message(input: &str) -> Option<String> {
input.split(": '").nth(1)?.split('\'').next().map(str::to_string)
kevaundray marked this conversation as resolved.
Show resolved Hide resolved
// The test has failed compilation, extract the assertion message if present and check if it's expected.
if let RuntimeError::FailedConstraint { assert_message: Some(assert_message), .. } = &err {
let assert_message = assert_message.clone();
check_expected_failure_message(test_function, &assert_message, Some(err.into()))
} else {
TestStatus::CompileError(err.into())
}

check_expected_failure_message(
test_function,
&extract_constraint_error_message(&diag.diagnostic.message).unwrap(),
)
}

/// The test function compiled successfully.
Expand All @@ -74,6 +61,7 @@ fn test_status_program_compile_fail(
/// passed/failed to determine the test status.
fn test_status_program_compile_pass(
test_function: TestFunction,
debug: DebugInfo,
circuit_execution: Result<WitnessMap, NargoError>,
) -> TestStatus {
let circuit_execution_err = match circuit_execution {
Expand All @@ -83,6 +71,7 @@ fn test_status_program_compile_pass(
if test_function.should_fail() {
return TestStatus::Fail {
message: "error: Test passed when it should have failed".to_string(),
error_diagnostic: None,
};
}
return TestStatus::Pass;
Expand All @@ -93,18 +82,26 @@ fn test_status_program_compile_pass(
// If we reach here, then the circuit execution failed.
//
// Check if the function should have passed
let diagnostic = try_to_diagnose_runtime_error(&circuit_execution_err, &debug);
let test_should_have_passed = !test_function.should_fail();
if test_should_have_passed {
return TestStatus::Fail { message: circuit_execution_err.to_string() };
return TestStatus::Fail {
message: circuit_execution_err.to_string(),
error_diagnostic: diagnostic,
};
}

check_expected_failure_message(
test_function,
circuit_execution_err.user_defined_failure_message().unwrap_or_default(),
)
let assertion_message =
circuit_execution_err.user_defined_failure_message().unwrap_or_default().to_string();

check_expected_failure_message(test_function, &assertion_message, diagnostic)
}

fn check_expected_failure_message(test_function: TestFunction, got_error: &str) -> TestStatus {
fn check_expected_failure_message(
test_function: TestFunction,
failed_assertion: &str,
error_diagnostic: Option<FileDiagnostic>,
) -> TestStatus {
// Extract the expected failure message, if there was one
//
// #[test(should_fail)] will not produce any message
Expand All @@ -115,7 +112,7 @@ fn check_expected_failure_message(test_function: TestFunction, got_error: &str)
None => return TestStatus::Pass,
};

let expected_failure_message_matches = got_error == expected_failure_message;
let expected_failure_message_matches = failed_assertion == expected_failure_message;
if expected_failure_message_matches {
return TestStatus::Pass;
}
Expand All @@ -125,7 +122,8 @@ fn check_expected_failure_message(test_function: TestFunction, got_error: &str)
message: format!(
"\nerror: Test failed with the wrong message. \nExpected: {} \nGot: {}",
test_function.failure_reason().unwrap_or_default(),
got_error.trim_matches('\'')
failed_assertion.trim_matches('\'')
),
error_diagnostic,
}
}
Loading