From 9eacfe6d7f841d9e7874bd8593cb1ac1c3a3b456 Mon Sep 17 00:00:00 2001 From: Ana Perez Ghiglia Date: Wed, 7 Aug 2024 16:48:13 -0300 Subject: [PATCH] Notify if test passed or failed - add TestFunction to dap - add Abi dependency to debugger crate --- Cargo.lock | 1 + tooling/debugger/Cargo.toml | 1 + tooling/debugger/src/context.rs | 5 ++ tooling/debugger/src/dap.rs | 76 ++++++++++++++++++++++++++-- tooling/debugger/src/lib.rs | 4 +- tooling/nargo/src/ops/mod.rs | 3 +- tooling/nargo/src/ops/test.rs | 2 +- tooling/nargo_cli/src/cli/dap_cmd.rs | 25 +++++---- 8 files changed, 100 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 47b63ff2f4f..f220b412b16 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2680,6 +2680,7 @@ dependencies = [ "easy-repl", "fm", "nargo", + "noirc_abi", "noirc_artifacts", "noirc_driver", "noirc_errors", diff --git a/tooling/debugger/Cargo.toml b/tooling/debugger/Cargo.toml index 540d6d11bc0..b6ccf86f712 100644 --- a/tooling/debugger/Cargo.toml +++ b/tooling/debugger/Cargo.toml @@ -19,6 +19,7 @@ noirc_printable_type.workspace = true noirc_errors.workspace = true noirc_driver.workspace = true noirc_artifacts.workspace = true +noirc_abi.workspace = true thiserror.workspace = true codespan-reporting.workspace = true dap.workspace = true diff --git a/tooling/debugger/src/context.rs b/tooling/debugger/src/context.rs index 5b96b492824..25b4f362cf4 100644 --- a/tooling/debugger/src/context.rs +++ b/tooling/debugger/src/context.rs @@ -183,9 +183,14 @@ impl std::str::FromStr for DebugLocation { #[derive(Debug)] pub(super) enum DebugCommandResult { + // TODO: validate comments + // The debugging session is over successfully Done, + // The session is active and we should continue with the execution Ok, + // Execution should be paused since we reached a Breakpoint BreakpointReached(DebugLocation), + // Session is over with an error Error(NargoError), } diff --git a/tooling/debugger/src/dap.rs b/tooling/debugger/src/dap.rs index cfe33a61cb5..053c557a0e4 100644 --- a/tooling/debugger/src/dap.rs +++ b/tooling/debugger/src/dap.rs @@ -5,6 +5,10 @@ use acvm::acir::circuit::brillig::BrilligBytecode; use acvm::acir::circuit::Circuit; use acvm::acir::native_types::WitnessMap; use acvm::{BlackBoxFunctionSolver, FieldElement}; +use nargo::errors::try_to_diagnose_runtime_error; +use nargo::ops::check_expected_failure_message; +use noirc_abi::Abi; +use noirc_frontend::hir::def_map::TestFunction; use crate::context::DebugContext; use crate::context::{DebugCommandResult, DebugLocation}; @@ -21,8 +25,8 @@ use dap::responses::{ }; use dap::server::Server; use dap::types::{ - Breakpoint, DisassembledInstruction, Scope, Source, StackFrame, SteppingGranularity, - StoppedEventReason, Thread, Variable, + Breakpoint, DisassembledInstruction, OutputEventCategory, Scope, Source, StackFrame, + SteppingGranularity, StoppedEventReason, Thread, Variable, }; use noirc_artifacts::debug::DebugArtifact; @@ -39,6 +43,8 @@ pub struct DapSession<'a, R: Read, W: Write, B: BlackBoxFunctionSolver, source_breakpoints: BTreeMap>, + test_function: Option, + abi: &'a Abi, } enum ScopeReferences { @@ -65,6 +71,8 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession< debug_artifact: &'a DebugArtifact, initial_witness: WitnessMap, unconstrained_functions: &'a [BrilligBytecode], + test_function: Option, + abi: &'a Abi, ) -> Self { let context = DebugContext::new( solver, @@ -82,6 +90,8 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession< next_breakpoint_id: 1, instruction_breakpoints: vec![], source_breakpoints: BTreeMap::new(), + test_function, + abi, } } @@ -341,6 +351,14 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession< match result { DebugCommandResult::Done => { self.running = false; + if let Some(test_function) = self.test_function.as_ref() { + let test_result = if test_function.should_fail() { + "x Test failed: Test passed when it should have failed" + } else { + "✓ Test passed" + }; + self.send_debug_output_message(String::from(test_result))?; + } } DebugCommandResult::Ok => { self.server.send_event(Event::Stopped(StoppedEventBody { @@ -368,18 +386,65 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession< DebugCommandResult::Error(err) => { self.server.send_event(Event::Stopped(StoppedEventBody { reason: StoppedEventReason::Exception, - description: Some(format!("{err:?}")), + description: Some(String::from("Paused on exception")), // This is shown in callstack section of the debug panel thread_id: Some(0), preserve_focus_hint: Some(false), - text: None, + text: Some(format!("{err}\n{err:?}")), // This is shown in the exception panel all_threads_stopped: Some(false), hit_breakpoint_ids: None, }))?; + if let Some(test_function) = self.test_function.as_ref() { + let test_should_have_passed = !test_function.should_fail(); + if test_should_have_passed { + // Do not finish the execution of the debugger + // Since the test shouldn't have failed, so that user can + // - see the error on the exception panel + // - restart the debug session + let message = format!("x Text failed: {}", err.to_string()); + self.send_debug_output_message(message)?; + } else { + // Finish the execution of the debugger since the test reached + // the expected state (failed) + self.running = false; + let diagnostic = try_to_diagnose_runtime_error( + &err, + &self.abi, + &self.debug_artifact.debug_symbols, + ); + let message = match check_expected_failure_message( + test_function, + err.user_defined_failure_message(&self.abi.error_types), + diagnostic, + ) { + nargo::ops::TestStatus::Pass => String::from("✓ Test passed"), + nargo::ops::TestStatus::Fail { message, .. } => { + format!("x Test failed: {}", message) + } + nargo::ops::TestStatus::CompileError(..) => { + String::from("x Test failed: Something went wrong") + } // TODO: this shouldn't happen. Should we panic? + }; + self.send_debug_output_message(message)?; + }; + } } } Ok(()) } + fn send_debug_output_message(&mut self, message: String) -> Result<(), ServerError> { + self.server.send_event(Event::Output(dap::events::OutputEventBody { + category: Some(OutputEventCategory::Console), + output: message, + group: None, + variables_reference: None, + source: None, + line: None, + column: None, + data: None, + })) + } + fn get_next_breakpoint_id(&mut self) -> BreakpointId { let id = self.next_breakpoint_id; self.next_breakpoint_id += 1; @@ -607,6 +672,7 @@ pub fn run_session>( solver: &B, program: CompiledProgram, initial_witness: WitnessMap, + test_function: Option, ) -> Result<(), ServerError> { let debug_artifact = DebugArtifact { debug_symbols: program.debug, file_map: program.file_map }; let mut session = DapSession::new( @@ -616,6 +682,8 @@ pub fn run_session>( &debug_artifact, initial_witness, &program.program.unconstrained_functions, + test_function, + &program.abi, ); session.run_loop() diff --git a/tooling/debugger/src/lib.rs b/tooling/debugger/src/lib.rs index 37ac088ca35..96f6e2d66c3 100644 --- a/tooling/debugger/src/lib.rs +++ b/tooling/debugger/src/lib.rs @@ -14,6 +14,7 @@ use acvm::{BlackBoxFunctionSolver, FieldElement}; use nargo::NargoError; use noirc_driver::CompiledProgram; +use noirc_frontend::hir::def_map::TestFunction; pub fn run_repl_session>( solver: &B, @@ -28,6 +29,7 @@ pub fn run_dap_loop>( solver: &B, program: CompiledProgram, initial_witness: WitnessMap, + test_function: Option, ) -> Result<(), ServerError> { - dap::run_session(server, solver, program, initial_witness) + dap::run_session(server, solver, program, initial_witness, test_function) } diff --git a/tooling/nargo/src/ops/mod.rs b/tooling/nargo/src/ops/mod.rs index 0ff382ef1b9..7c6e48d388b 100644 --- a/tooling/nargo/src/ops/mod.rs +++ b/tooling/nargo/src/ops/mod.rs @@ -8,7 +8,8 @@ pub use self::optimize::{optimize_contract, optimize_program}; pub use self::transform::{transform_contract, transform_program}; pub use self::test::{ - run_test, test_status_program_compile_fail, test_status_program_compile_pass, TestStatus, + check_expected_failure_message, run_test, test_status_program_compile_fail, + test_status_program_compile_pass, TestStatus, }; mod compile; diff --git a/tooling/nargo/src/ops/test.rs b/tooling/nargo/src/ops/test.rs index 542a693573f..db044bf29f2 100644 --- a/tooling/nargo/src/ops/test.rs +++ b/tooling/nargo/src/ops/test.rs @@ -116,7 +116,7 @@ pub fn test_status_program_compile_pass( ) } -fn check_expected_failure_message( +pub fn check_expected_failure_message( test_function: &TestFunction, failed_assertion: Option, error_diagnostic: Option, diff --git a/tooling/nargo_cli/src/cli/dap_cmd.rs b/tooling/nargo_cli/src/cli/dap_cmd.rs index 26e3abf2065..e80ce0c091b 100644 --- a/tooling/nargo_cli/src/cli/dap_cmd.rs +++ b/tooling/nargo_cli/src/cli/dap_cmd.rs @@ -9,6 +9,7 @@ use nargo::workspace::Workspace; use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection}; use noirc_abi::input_parser::Format; use noirc_driver::{check_crate, CompileOptions, CompiledProgram, NOIR_ARTIFACT_VERSION_STRING}; +use noirc_frontend::hir::def_map::TestFunction; use noirc_frontend::{graph::CrateName, hir::FunctionNameMatch}; use std::io::{BufReader, BufWriter, Read, Write}; @@ -109,7 +110,7 @@ fn load_and_compile_project( acir_mode: bool, skip_instrumentation: bool, test_name: Option<&str>, -) -> Result<(CompiledProgram, WitnessMap), LoadError> { +) -> Result<(CompiledProgram, WitnessMap, Option), LoadError> { let workspace = find_workspace(project_folder, package) .ok_or(LoadError::Generic(workspace_not_found_error_msg(project_folder, package)))?; let package = workspace @@ -121,12 +122,15 @@ fn load_and_compile_project( let compile_options = compile_options_for_debugging(acir_mode, skip_instrumentation, CompileOptions::default()); - let compiled_program = match test_name { + let (compiled_program, test_function) = match test_name { + Some("") | None => ( + compile_bin_package_for_debugging(&workspace, &package, &compile_options) + .map_err(|_| LoadError::Generic("Failed to compile project".into()))?, + None, + ), Some(test_name) => { load_and_compile_test_function(test_name, workspace, &package, &compile_options)? } - None => compile_bin_package_for_debugging(&workspace, &package, &compile_options) - .map_err(|_| LoadError::Generic("Failed to compile project".into()))?, }; let compiled_program = nargo::ops::transform_program(compiled_program, expression_width); @@ -140,7 +144,7 @@ fn load_and_compile_project( .encode(&inputs_map, None) .map_err(|_| LoadError::Generic("Failed to encode inputs".into()))?; - Ok((compiled_program, initial_witness)) + Ok((compiled_program, initial_witness, test_function)) } fn load_and_compile_test_function( @@ -148,7 +152,7 @@ fn load_and_compile_test_function( workspace: Workspace, package: &Package, compile_options: &CompileOptions, -) -> Result { +) -> Result<(CompiledProgram, Option), LoadError> { let (workspace_file_manager, mut parsed_files) = file_manager_and_files_from(&workspace.root_dir, &workspace); @@ -183,11 +187,11 @@ fn load_and_compile_test_function( let test_functions = context .get_all_test_functions_in_crate_matching(&crate_id, FunctionNameMatch::Exact(test_name)); - let (_, test_function) = test_functions.first().expect("Test function should exist"); + let (_, test_function) = test_functions.into_iter().nth(0).expect("Test function should exist"); - let compiled = compile_no_check_for_debug(&mut context, test_function, &compile_options) + let compiled = compile_no_check_for_debug(&mut context, &test_function, &compile_options) .map_err(|_| LoadError::Generic("Failed to compile project".into()))?; - Ok(compiled) + Ok((compiled, Some(test_function))) } fn loop_uninitialized_dap( @@ -250,7 +254,7 @@ fn loop_uninitialized_dap( skip_instrumentation, test_name, ) { - Ok((compiled_program, initial_witness)) => { + Ok((compiled_program, initial_witness, test_function)) => { server.respond(req.ack()?)?; noir_debugger::run_dap_loop( @@ -258,6 +262,7 @@ fn loop_uninitialized_dap( &Bn254BlackBoxSolver, compiled_program, initial_witness, + test_function, )?; break; }