diff --git a/.noir-sync-commit b/.noir-sync-commit index da88b7f93f6..c4b6d336e04 100644 --- a/.noir-sync-commit +++ b/.noir-sync-commit @@ -1 +1 @@ -1b0dd4149d9249f0ea4fb5e2228c688e0135618f +b88db67a4fa92f861329105fb732a7b1309620fe diff --git a/noir/noir-repo/.github/workflows/test-js-packages.yml b/noir/noir-repo/.github/workflows/test-js-packages.yml index 41a7008efc0..dde0deed0cf 100644 --- a/noir/noir-repo/.github/workflows/test-js-packages.yml +++ b/noir/noir-repo/.github/workflows/test-js-packages.yml @@ -534,7 +534,7 @@ jobs: - name: Build list of libraries id: get_critical_libraries run: | - LIBRARIES=$(grep -Po "^https://github.com/\K.+" ./CRITICAL_NOIR_LIBRARIES | jq -R -s -c 'split("\n") | map(select(. != "")) | map({ repo: ., path: "./"})') + LIBRARIES=$(grep -Po "^https://github.com/\K.+" ./CRITICAL_NOIR_LIBRARIES | jq -R -s -c 'split("\n") | map(select(. != "")) | map({ repo: ., path: ""})') echo "libraries=$LIBRARIES" echo "libraries=$LIBRARIES" >> $GITHUB_OUTPUT env: @@ -551,15 +551,15 @@ jobs: matrix: project: ${{ fromJson( needs.critical-library-list.outputs.libraries )}} include: - - project: { repo: AztecProtocol/aztec-packages, path: ./noir-projects/aztec-nr } - - project: { repo: AztecProtocol/aztec-packages, path: ./noir-projects/noir-contracts } - - project: { repo: AztecProtocol/aztec-packages, path: ./noir-projects/noir-protocol-circuits/crates/parity-lib } - - project: { repo: AztecProtocol/aztec-packages, path: ./noir-projects/noir-protocol-circuits/crates/private-kernel-lib } - - project: { repo: AztecProtocol/aztec-packages, path: ./noir-projects/noir-protocol-circuits/crates/reset-kernel-lib } - - project: { repo: AztecProtocol/aztec-packages, path: ./noir-projects/noir-protocol-circuits/crates/rollup-lib } - - project: { repo: AztecProtocol/aztec-packages, path: ./noir-projects/noir-protocol-circuits/crates/types } - - name: Check external repo - ${{ matrix.project.repo }} + - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/aztec-nr } + - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-contracts } + - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/parity-lib } + - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/private-kernel-lib } + - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/reset-kernel-lib } + - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/rollup-lib } + - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/types } + + name: Check external repo - ${{ matrix.project.repo }}/${{ matrix.project.path }} steps: - name: Checkout uses: actions/checkout@v4 @@ -591,7 +591,7 @@ jobs: - name: Run nargo test working-directory: ./test-repo/${{ matrix.project.path }} - run: nargo test --silence-warnings + run: nargo test -q --silence-warnings env: NARGO_IGNORE_TEST_FAILURES_FROM_FOREIGN_CALLS: true diff --git a/noir/noir-repo/acvm-repo/acvm/src/compiler/mod.rs b/noir/noir-repo/acvm-repo/acvm/src/compiler/mod.rs index af7fd108665..daedd77c4a0 100644 --- a/noir/noir-repo/acvm-repo/acvm/src/compiler/mod.rs +++ b/noir/noir-repo/acvm-repo/acvm/src/compiler/mod.rs @@ -82,15 +82,16 @@ pub fn compile( acir: Circuit, expression_width: ExpressionWidth, ) -> (Circuit, AcirTransformationMap) { - let initial_opcode_positions = (0..acir.opcodes.len()).collect::>(); + let acir_opcode_positions = (0..acir.opcodes.len()).collect::>(); + if MAX_OPTIMIZER_PASSES == 0 { - let transformation_map = AcirTransformationMap::new(&initial_opcode_positions); - return (acir, transformation_map); + return (acir, AcirTransformationMap::new(&acir_opcode_positions)); } + let mut pass = 0; let mut prev_opcodes_hash = fxhash::hash64(&acir.opcodes); let mut prev_acir = acir; - let mut prev_acir_opcode_positions = initial_opcode_positions; + let mut prev_acir_opcode_positions = acir_opcode_positions; // For most test programs it would be enough to only loop `transform_internal`, // but some of them don't stabilize unless we also repeat the backend agnostic optimizations. diff --git a/noir/noir-repo/acvm-repo/acvm/src/compiler/optimizers/mod.rs b/noir/noir-repo/acvm-repo/acvm/src/compiler/optimizers/mod.rs index 0d39b2f5100..1b743227acf 100644 --- a/noir/noir-repo/acvm-repo/acvm/src/compiler/optimizers/mod.rs +++ b/noir/noir-repo/acvm-repo/acvm/src/compiler/optimizers/mod.rs @@ -24,6 +24,7 @@ pub fn optimize(acir: Circuit) -> (Circuit, AcirTransformati // Track original acir opcode positions throughout the transformation passes of the compilation // by applying the modifications done to the circuit opcodes and also to the opcode_positions (delete and insert) let acir_opcode_positions = (0..acir.opcodes.len()).collect(); + let (mut acir, new_opcode_positions) = optimize_internal(acir, acir_opcode_positions); let transformation_map = AcirTransformationMap::new(&new_opcode_positions); diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/binary.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/binary.rs index 0f52168a30d..81f2f3b1e01 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/binary.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/binary.rs @@ -256,6 +256,10 @@ impl Binary { if rhs_is_zero { return SimplifyResult::SimplifiedTo(self.lhs); } + if operand_type == NumericType::bool() && (lhs_is_one || rhs_is_one) { + let one = dfg.make_constant(FieldElement::one(), operand_type); + return SimplifyResult::SimplifiedTo(one); + } if dfg.resolve(self.lhs) == dfg.resolve(self.rhs) { return SimplifyResult::SimplifiedTo(self.lhs); } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs index 1daa1af7907..02be0910a13 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs @@ -480,20 +480,14 @@ fn simplify_slice_push_back( } fn simplify_slice_pop_back( - element_type: Type, + slice_type: Type, arguments: &[ValueId], dfg: &mut DataFlowGraph, block: BasicBlockId, call_stack: CallStack, ) -> SimplifyResult { - let element_types = match element_type.clone() { - Type::Slice(element_types) | Type::Array(element_types, _) => element_types, - _ => { - unreachable!("ICE: Expected slice or array, but got {element_type}"); - } - }; - - let element_count = element_type.element_size(); + let element_types = slice_type.element_types(); + let element_count = element_types.len(); let mut results = VecDeque::with_capacity(element_count + 1); let new_slice_length = update_slice_length(arguments[0], dfg, BinaryOp::Sub, block); @@ -507,14 +501,17 @@ fn simplify_slice_pop_back( flattened_len = update_slice_length(flattened_len, dfg, BinaryOp::Sub, block); // We must pop multiple elements in the case of a slice of tuples - for _ in 0..element_count { + // Iterating through element types in reverse here since we're popping from the end + for element_type in element_types.iter().rev() { let get_last_elem_instr = Instruction::ArrayGet { array: arguments[1], index: flattened_len }; + + let element_type = Some(vec![element_type.clone()]); let get_last_elem = dfg .insert_instruction_and_results( get_last_elem_instr, block, - Some(element_types.to_vec()), + element_type, call_stack.clone(), ) .first(); diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs index c58264dbe84..a5de98cec7f 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs @@ -173,7 +173,11 @@ pub(super) fn simplify_msm( var_scalars.push(zero); let result_x = dfg.make_constant(result_x, NumericType::NativeField); let result_y = dfg.make_constant(result_y, NumericType::NativeField); + + // Pushing a bool here is intentional, multi_scalar_mul takes two arguments: + // `points: [(Field, Field, bool); N]` and `scalars: [(Field, Field); N]`. let result_is_infinity = dfg.make_constant(result_is_infinity, NumericType::bool()); + var_points.push(result_x); var_points.push(result_y); var_points.push(result_is_infinity); diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs index 87e7f8bcff3..7e4546083b8 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/loop_invariant.rs @@ -13,7 +13,7 @@ use fxhash::{FxHashMap as HashMap, FxHashSet as HashSet}; use crate::ssa::{ ir::{ basic_block::BasicBlockId, - function::{Function, RuntimeType}, + function::Function, function_inserter::FunctionInserter, instruction::{Instruction, InstructionId}, types::Type, @@ -27,12 +27,7 @@ use super::unrolling::{Loop, Loops}; impl Ssa { #[tracing::instrument(level = "trace", skip(self))] pub(crate) fn loop_invariant_code_motion(mut self) -> Ssa { - let brillig_functions = self - .functions - .iter_mut() - .filter(|(_, func)| matches!(func.runtime(), RuntimeType::Brillig(_))); - - for (_, function) in brillig_functions { + for function in self.functions.values_mut() { function.loop_invariant_code_motion(); } @@ -63,6 +58,7 @@ impl Loops { } context.map_dependent_instructions(); + context.inserter.map_data_bus_in_place(); } } @@ -113,6 +109,22 @@ impl<'f> LoopInvariantContext<'f> { if hoist_invariant { self.inserter.push_instruction(instruction_id, pre_header); + + // If we are hoisting a MakeArray instruction, + // we need to issue an extra inc_rc in case they are mutated afterward. + if matches!( + self.inserter.function.dfg[instruction_id], + Instruction::MakeArray { .. } + ) { + let result = + self.inserter.function.dfg.instruction_results(instruction_id)[0]; + let inc_rc = Instruction::IncrementRc { value: result }; + let call_stack = self.inserter.function.dfg.get_call_stack(instruction_id); + self.inserter + .function + .dfg + .insert_instruction_and_results(inc_rc, *block, None, call_stack); + } } else { self.inserter.push_instruction(instruction_id, *block); } @@ -190,6 +202,7 @@ impl<'f> LoopInvariantContext<'f> { }); let can_be_deduplicated = instruction.can_be_deduplicated(self.inserter.function, false) + || matches!(instruction, Instruction::MakeArray { .. }) || self.can_be_deduplicated_from_upper_bound(&instruction); is_loop_invariant && can_be_deduplicated @@ -559,4 +572,94 @@ mod test { let ssa = ssa.loop_invariant_code_motion(); assert_normalized_ssa_equals(ssa, expected); } + + #[test] + fn insert_inc_rc_when_moving_make_array() { + // SSA for the following program: + // + // unconstrained fn main(x: u32, y: u32) { + // let mut a1 = [1, 2, 3, 4, 5]; + // a1[x] = 64; + // for i in 0 .. 5 { + // let mut a2 = [1, 2, 3, 4, 5]; + // a2[y + i] = 128; + // foo(a2); + // } + // foo(a1); + // } + // + // We want to make sure move a loop invariant make_array instruction, + // to account for whether that array has been marked as mutable. + // To do so, we increment the reference counter on the array we are moving. + // In the SSA below, we want to move `v42` out of the loop. + let src = " + brillig(inline) fn main f0 { + b0(v0: u32, v1: u32): + v8 = make_array [Field 1, Field 2, Field 3, Field 4, Field 5] : [Field; 5] + v9 = allocate -> &mut [Field; 5] + v11 = array_set v8, index v0, value Field 64 + v13 = add v0, u32 1 + store v11 at v9 + jmp b1(u32 0) + b1(v2: u32): + v16 = lt v2, u32 5 + jmpif v16 then: b3, else: b2 + b2(): + v17 = load v9 -> [Field; 5] + call f1(v17) + return + b3(): + v19 = make_array [Field 1, Field 2, Field 3, Field 4, Field 5] : [Field; 5] + v20 = allocate -> &mut [Field; 5] + v21 = add v1, v2 + v23 = array_set v19, index v21, value Field 128 + call f1(v23) + v25 = add v2, u32 1 + jmp b1(v25) + } + brillig(inline) fn foo f1 { + b0(v0: [Field; 5]): + return + } + "; + + let ssa = Ssa::from_str(src).unwrap(); + + // We expect the `make_array` at the top of `b3` to be replaced with an `inc_rc` + // of the newly hoisted `make_array` at the end of `b0`. + let expected = " + brillig(inline) fn main f0 { + b0(v0: u32, v1: u32): + v8 = make_array [Field 1, Field 2, Field 3, Field 4, Field 5] : [Field; 5] + v9 = allocate -> &mut [Field; 5] + v11 = array_set v8, index v0, value Field 64 + v13 = add v0, u32 1 + store v11 at v9 + v14 = make_array [Field 1, Field 2, Field 3, Field 4, Field 5] : [Field; 5] + jmp b1(u32 0) + b1(v2: u32): + v17 = lt v2, u32 5 + jmpif v17 then: b3, else: b2 + b2(): + v18 = load v9 -> [Field; 5] + call f1(v18) + return + b3(): + inc_rc v14 + v20 = allocate -> &mut [Field; 5] + v21 = add v1, v2 + v23 = array_set v14, index v21, value Field 128 + call f1(v23) + v25 = add v2, u32 1 + jmp b1(v25) + } + brillig(inline) fn foo f1 { + b0(v0: [Field; 5]): + return + } + "; + + let ssa = ssa.loop_invariant_code_motion(); + assert_normalized_ssa_equals(ssa, expected); + } } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs index d341e5e9c4c..536f2cdb477 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs @@ -703,7 +703,9 @@ impl<'a> FunctionContext<'a> { // Don't mutate the reference count if we're assigning an array literal to a Let: // `let mut foo = [1, 2, 3];` // we consider the array to be moved, so we should have an initial rc of just 1. - let should_inc_rc = !let_expr.expression.is_array_or_slice_literal(); + // + // TODO: this exception breaks #6763 + let should_inc_rc = true; // !let_expr.expression.is_array_or_slice_literal(); values = values.map(|value| { let value = value.eval(self); diff --git a/noir/noir-repo/docs/docs/getting_started/noir_installation.md b/noir/noir-repo/docs/docs/getting_started/noir_installation.md index a5c7e649278..05f036d4f6d 100644 --- a/noir/noir-repo/docs/docs/getting_started/noir_installation.md +++ b/noir/noir-repo/docs/docs/getting_started/noir_installation.md @@ -93,7 +93,7 @@ step 2: Follow the [Noirup instructions](#installing-noirup). ## Setting up shell completions -Once `nargo` is installed, you can [set up shell completions for it](setting_up_shell_completions). +Once `nargo` is installed, you can [set up shell completions for it](setting_up_shell_completions.md). ## Uninstalling Nargo diff --git a/noir/noir-repo/scripts/check-critical-libraries.sh b/noir/noir-repo/scripts/check-critical-libraries.sh index b492cf1d4bc..f8e474d23de 100755 --- a/noir/noir-repo/scripts/check-critical-libraries.sh +++ b/noir/noir-repo/scripts/check-critical-libraries.sh @@ -31,7 +31,7 @@ for REPO in ${REPOS_TO_CHECK[@]}; do TAG=$(getLatestReleaseTagForRepo $REPO) git clone $REPO -c advice.detachedHead=false --depth 1 --branch $TAG $TMP_DIR - nargo test --program-dir $TMP_DIR + nargo test -q --program-dir $TMP_DIR rm -rf $TMP_DIR done diff --git a/noir/noir-repo/test_programs/execution_success/diamond_deps_0/Prover.toml b/noir/noir-repo/test_programs/execution_success/diamond_deps_0/Prover.toml index a713241e7dd..2bad103177c 100644 --- a/noir/noir-repo/test_programs/execution_success/diamond_deps_0/Prover.toml +++ b/noir/noir-repo/test_programs/execution_success/diamond_deps_0/Prover.toml @@ -1,3 +1,3 @@ x = 1 y = 1 -return = 5 \ No newline at end of file +return = 7 diff --git a/noir/noir-repo/test_programs/execution_success/reference_counts/src/main.nr b/noir/noir-repo/test_programs/execution_success/reference_counts/src/main.nr index 8de4d0f2508..68b9f2407b9 100644 --- a/noir/noir-repo/test_programs/execution_success/reference_counts/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/reference_counts/src/main.nr @@ -2,7 +2,7 @@ use std::mem::array_refcount; fn main() { let mut array = [0, 1, 2]; - assert_refcount(array, 1); + assert_refcount(array, 2); borrow(array, array_refcount(array)); borrow_mut(&mut array, array_refcount(array)); diff --git a/noir/noir-repo/test_programs/execution_success/return_twice/Nargo.toml b/noir/noir-repo/test_programs/execution_success/return_twice/Nargo.toml new file mode 100644 index 00000000000..defd68f37b8 --- /dev/null +++ b/noir/noir-repo/test_programs/execution_success/return_twice/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "return_twice" +version = "0.1.0" +type = "bin" +authors = [""] + +[dependencies] diff --git a/noir/noir-repo/test_programs/execution_success/return_twice/Prover.toml b/noir/noir-repo/test_programs/execution_success/return_twice/Prover.toml new file mode 100644 index 00000000000..b343065a7fc --- /dev/null +++ b/noir/noir-repo/test_programs/execution_success/return_twice/Prover.toml @@ -0,0 +1,2 @@ +return = [100, 100] +in0 = 10 diff --git a/noir/noir-repo/test_programs/execution_success/return_twice/src/main.nr b/noir/noir-repo/test_programs/execution_success/return_twice/src/main.nr new file mode 100644 index 00000000000..68bd5f916ce --- /dev/null +++ b/noir/noir-repo/test_programs/execution_success/return_twice/src/main.nr @@ -0,0 +1,5 @@ +pub fn main(in0: Field) -> pub (Field, Field) { + let out0 = (in0 * in0); + let out1 = (in0 * in0); + (out0, out1) +} diff --git a/noir/noir-repo/test_programs/gates_report_brillig_execution.sh b/noir/noir-repo/test_programs/gates_report_brillig_execution.sh index 024c7612541..b3f4a8bda98 100755 --- a/noir/noir-repo/test_programs/gates_report_brillig_execution.sh +++ b/noir/noir-repo/test_programs/gates_report_brillig_execution.sh @@ -8,8 +8,6 @@ excluded_dirs=( "double_verify_nested_proof" "overlapping_dep_and_mod" "comptime_println" - # Takes a very long time to execute as large loops do not get simplified. - "regression_4709" # bit sizes for bigint operation doesn't match up. "bigint" # Expected to fail as test asserts on which runtime it is in. diff --git a/noir/noir-repo/test_programs/noir_test_success/assert_message/Nargo.toml b/noir/noir-repo/test_programs/noir_test_success/assert_message/Nargo.toml new file mode 100644 index 00000000000..667035339bd --- /dev/null +++ b/noir/noir-repo/test_programs/noir_test_success/assert_message/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "assert_message" +type = "bin" +authors = [""] +compiler_version = ">=0.23.0" + +[dependencies] diff --git a/noir/noir-repo/test_programs/noir_test_success/assert_message/src/main.nr b/noir/noir-repo/test_programs/noir_test_success/assert_message/src/main.nr new file mode 100644 index 00000000000..073e42daf4a --- /dev/null +++ b/noir/noir-repo/test_programs/noir_test_success/assert_message/src/main.nr @@ -0,0 +1,15 @@ +// Have to use inputs otherwise the ACIR gets optimized away due to constants. +// With the original ACIR the optimizations can rearrange or merge opcodes, +// which might end up getting out of sync with assertion messages. +#[test(should_fail_with = "first")] +fn test_assert_message_preserved_during_optimization(a: Field, b: Field, c: Field) { + if (a + b) * (a - b) != c { + assert((a + b) * (a - b) == c, "first"); + assert((a - b) * (a + b) == c, "second"); + assert((a + b) * (a - b) == c, "third"); + assert((2 * a + b) * (a - b / 2) == c * c, "fourth"); + } else { + // The fuzzer might have generated a passing test. + assert((a + b) * (a - b) != c, "first"); + } +} diff --git a/noir/noir-repo/test_programs/noir_test_success/fuzzer_checks/src/main.nr b/noir/noir-repo/test_programs/noir_test_success/fuzzer_checks/src/main.nr index 2b928db092e..70e5e29b855 100644 --- a/noir/noir-repo/test_programs/noir_test_success/fuzzer_checks/src/main.nr +++ b/noir/noir-repo/test_programs/noir_test_success/fuzzer_checks/src/main.nr @@ -1,6 +1,9 @@ - #[test(should_fail_with = "42 is not allowed")] fn finds_magic_value(x: u32) { let x = x as u64; - assert(2 * x != 42, "42 is not allowed"); + if x == 21 { + assert(2 * x != 42, "42 is not allowed"); + } else { + assert(2 * x == 42, "42 is not allowed"); + } } diff --git a/noir/noir-repo/tooling/nargo/src/ops/test.rs b/noir/noir-repo/tooling/nargo/src/ops/test.rs index 7087c0de64e..1306150518d 100644 --- a/noir/noir-repo/tooling/nargo/src/ops/test.rs +++ b/noir/noir-repo/tooling/nargo/src/ops/test.rs @@ -9,7 +9,7 @@ use acvm::{ AcirField, BlackBoxFunctionSolver, FieldElement, }; use noirc_abi::Abi; -use noirc_driver::{compile_no_check, CompileError, CompileOptions}; +use noirc_driver::{compile_no_check, CompileError, CompileOptions, DEFAULT_EXPRESSION_WIDTH}; use noirc_errors::{debug_info::DebugInfo, FileDiagnostic}; use noirc_frontend::hir::{def_map::TestFunction, Context}; use noirc_printable_type::ForeignCallError; @@ -29,6 +29,7 @@ use crate::{ use super::execute_program; +#[derive(Debug)] pub enum TestStatus { Pass, Fail { message: String, error_diagnostic: Option }, @@ -62,6 +63,10 @@ pub fn run_test>( 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); + 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. @@ -81,9 +86,9 @@ pub fn run_test>( let status = test_status_program_compile_pass( test_function, - compiled_program.abi, - compiled_program.debug, - circuit_execution, + &compiled_program.abi, + &compiled_program.debug, + &circuit_execution, ); let ignore_foreign_call_failures = @@ -115,14 +120,20 @@ pub fn run_test>( { use acvm::acir::circuit::Program; use noir_fuzzer::FuzzedExecutor; + use proptest::test_runner::Config; use proptest::test_runner::TestRunner; - let runner = TestRunner::default(); + + let runner = + TestRunner::new(Config { failure_persistence: None, ..Config::default() }); + + let abi = compiled_program.abi.clone(); + let debug = compiled_program.debug.clone(); let executor = |program: &Program, initial_witness: WitnessMap| -> Result, String> { - execute_program( + let circuit_execution = execute_program( program, initial_witness, blackbox_solver, @@ -132,9 +143,23 @@ pub fn run_test>( root_path.clone(), package_name.clone(), ), - ) - .map_err(|err| err.to_string()) + ); + + 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()) + } }; + let fuzzer = FuzzedExecutor::new(compiled_program.into(), executor, runner); let result = fuzzer.fuzz(); @@ -174,9 +199,9 @@ fn test_status_program_compile_fail(err: CompileError, test_function: &TestFunct /// passed/failed to determine the test status. fn test_status_program_compile_pass( test_function: &TestFunction, - abi: Abi, - debug: Vec, - circuit_execution: Result, NargoError>, + abi: &Abi, + debug: &[DebugInfo], + circuit_execution: &Result, NargoError>, ) -> TestStatus { let circuit_execution_err = match circuit_execution { // Circuit execution was successful; ie no errors or unsatisfied constraints @@ -196,7 +221,7 @@ 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, &abi, &debug); + let diagnostic = try_to_diagnose_runtime_error(circuit_execution_err, abi, debug); let test_should_have_passed = !test_function.should_fail(); if test_should_have_passed { return TestStatus::Fail { diff --git a/noir/noir-repo/tooling/nargo_cli/build.rs b/noir/noir-repo/tooling/nargo_cli/build.rs index db6177366d3..8db2c1786d8 100644 --- a/noir/noir-repo/tooling/nargo_cli/build.rs +++ b/noir/noir-repo/tooling/nargo_cli/build.rs @@ -42,9 +42,7 @@ fn main() { /// Some tests are explicitly ignored in brillig due to them failing. /// These should be fixed and removed from this list. -const IGNORED_BRILLIG_TESTS: [&str; 11] = [ - // Takes a very long time to execute as large loops do not get simplified. - "regression_4709", +const IGNORED_BRILLIG_TESTS: [&str; 10] = [ // bit sizes for bigint operation doesn't match up. "bigint", // ICE due to looking for function which doesn't exist. diff --git a/noir/noir-repo/tooling/nargo_cli/src/cli/execute_cmd.rs b/noir/noir-repo/tooling/nargo_cli/src/cli/execute_cmd.rs index c0838743d6d..49a23a7ea62 100644 --- a/noir/noir-repo/tooling/nargo_cli/src/cli/execute_cmd.rs +++ b/noir/noir-repo/tooling/nargo_cli/src/cli/execute_cmd.rs @@ -64,8 +64,9 @@ pub(crate) fn run(args: ExecuteCommand, config: NargoConfig) -> Result<(), CliEr let program_artifact_path = workspace.package_build_path(package); let program: CompiledProgram = read_program_from_file(program_artifact_path.clone())?.into(); + let abi = program.abi.clone(); - let (return_value, witness_stack) = execute_program_and_decode( + let results = execute_program_and_decode( program, package, &args.prover_name, @@ -75,14 +76,27 @@ pub(crate) fn run(args: ExecuteCommand, config: NargoConfig) -> Result<(), CliEr )?; println!("[{}] Circuit witness successfully solved", package.name); - if let Some(return_value) = return_value { + if let Some(ref return_value) = results.actual_return { println!("[{}] Circuit output: {return_value:?}", package.name); } let package_name = package.name.clone().into(); let witness_name = args.witness_name.as_ref().unwrap_or(&package_name); - let witness_path = save_witness_to_dir(witness_stack, witness_name, target_dir)?; + let witness_path = save_witness_to_dir(results.witness_stack, witness_name, target_dir)?; println!("[{}] Witness saved to {}", package.name, witness_path.display()); + + // Sanity checks on the return value after the witness has been saved, so it can be inspected if necessary. + if let Some(expected) = results.expected_return { + if results.actual_return.as_ref() != Some(&expected) { + return Err(CliError::UnexpectedReturn { expected, actual: results.actual_return }); + } + } + // We can expect that if the circuit returns something, it should be non-empty after execution. + if let Some(ref expected) = abi.return_type { + if results.actual_return.is_none() { + return Err(CliError::MissingReturn { expected: expected.clone() }); + } + } } Ok(()) } @@ -94,18 +108,24 @@ fn execute_program_and_decode( foreign_call_resolver_url: Option<&str>, root_path: Option, package_name: Option, -) -> Result<(Option, WitnessStack), CliError> { +) -> Result { // Parse the initial witness values from Prover.toml - let (inputs_map, _) = + let (inputs_map, expected_return) = read_inputs_from_file(&package.root_dir, prover_name, Format::Toml, &program.abi)?; let witness_stack = execute_program(&program, &inputs_map, foreign_call_resolver_url, root_path, package_name)?; // Get the entry point witness for the ABI let main_witness = &witness_stack.peek().expect("Should have at least one witness on the stack").witness; - let (_, return_value) = program.abi.decode(main_witness)?; + let (_, actual_return) = program.abi.decode(main_witness)?; + + Ok(ExecutionResults { expected_return, actual_return, witness_stack }) +} - Ok((return_value, witness_stack)) +struct ExecutionResults { + expected_return: Option, + actual_return: Option, + witness_stack: WitnessStack, } pub(crate) fn execute_program( diff --git a/noir/noir-repo/tooling/nargo_cli/src/cli/test_cmd.rs b/noir/noir-repo/tooling/nargo_cli/src/cli/test_cmd.rs index 3646e28e97d..75e71818594 100644 --- a/noir/noir-repo/tooling/nargo_cli/src/cli/test_cmd.rs +++ b/noir/noir-repo/tooling/nargo_cli/src/cli/test_cmd.rs @@ -1,6 +1,6 @@ use std::{ collections::{BTreeMap, HashMap}, - io::Write, + fmt::Display, panic::{catch_unwind, UnwindSafe}, path::PathBuf, sync::{mpsc, Mutex}, @@ -12,6 +12,7 @@ use acvm::{BlackBoxFunctionSolver, FieldElement}; use bn254_blackbox_solver::Bn254BlackBoxSolver; use clap::Args; use fm::FileManager; +use formatters::{Formatter, PrettyFormatter, TerseFormatter}; use nargo::{ insert_all_files_for_workspace_into_file_manager, ops::TestStatus, package::Package, parse_all, prepare_package, workspace::Workspace, PrintOutput, @@ -19,12 +20,13 @@ use nargo::{ use nargo_toml::{get_package_manifest, resolve_workspace_from_toml}; use noirc_driver::{check_crate, CompileOptions, NOIR_ARTIFACT_VERSION_STRING}; use noirc_frontend::hir::{FunctionNameMatch, ParsedFiles}; -use termcolor::{Color, ColorChoice, ColorSpec, StandardStream, StandardStreamLock, WriteColor}; use crate::{cli::check_cmd::check_crate_and_report_errors, errors::CliError}; use super::{NargoConfig, PackageOptions}; +mod formatters; + /// Run the tests for this program #[derive(Debug, Clone, Args)] #[clap(visible_alias = "t")] @@ -53,6 +55,40 @@ pub(crate) struct TestCommand { /// Number of threads used for running tests in parallel #[clap(long, default_value_t = rayon::current_num_threads())] test_threads: usize, + + /// Configure formatting of output + #[clap(long)] + format: Option, + + /// Display one character per test instead of one line + #[clap(short = 'q', long = "quiet")] + quiet: bool, +} + +#[derive(Debug, Copy, Clone, clap::ValueEnum)] +enum Format { + /// Print verbose output + Pretty, + /// Display one character per test + Terse, +} + +impl Format { + fn formatter(&self) -> Box { + match self { + Format::Pretty => Box::new(PrettyFormatter), + Format::Terse => Box::new(TerseFormatter), + } + } +} + +impl Display for Format { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Format::Pretty => write!(f, "pretty"), + Format::Terse => write!(f, "terse"), + } + } } struct Test<'a> { @@ -95,6 +131,14 @@ pub(crate) fn run(args: TestCommand, config: NargoConfig) -> Result<(), CliError None => FunctionNameMatch::Anything, }; + let formatter: Box = if let Some(format) = args.format { + format.formatter() + } else if args.quiet { + Box::new(TerseFormatter) + } else { + Box::new(PrettyFormatter) + }; + let runner = TestRunner { file_manager: &file_manager, parsed_files: &parsed_files, @@ -102,6 +146,7 @@ pub(crate) fn run(args: TestCommand, config: NargoConfig) -> Result<(), CliError args: &args, pattern, num_threads: args.test_threads, + formatter, }; runner.run() } @@ -113,6 +158,7 @@ struct TestRunner<'a> { args: &'a TestCommand, pattern: FunctionNameMatch<'a>, num_threads: usize, + formatter: Box, } impl<'a> TestRunner<'a> { @@ -222,27 +268,31 @@ impl<'a> TestRunner<'a> { // We'll go package by package, but we might get test results from packages ahead of us. // We'll buffer those here and show them all at once when we get to those packages. let mut buffer: HashMap> = HashMap::new(); - for (package_name, test_count) in test_count_per_package { - let plural = if *test_count == 1 { "" } else { "s" }; - println!("[{package_name}] Running {test_count} test function{plural}"); - + for (package_name, total_test_count) in test_count_per_package { let mut test_report = Vec::new(); - // How many tests are left to receive for this package - let mut remaining_test_count = *test_count; + let mut current_test_count = 0; + let total_test_count = *total_test_count; + + self.formatter + .package_start(package_name, total_test_count) + .expect("Could not display package start"); // Check if we have buffered test results for this package if let Some(buffered_tests) = buffer.remove(package_name) { - remaining_test_count -= buffered_tests.len(); - for test_result in buffered_tests { - self.display_test_result(&test_result) - .expect("Could not display test status"); + self.display_test_result( + &test_result, + current_test_count + 1, + total_test_count, + ) + .expect("Could not display test status"); test_report.push(test_result); + current_test_count += 1; } } - if remaining_test_count > 0 { + if current_test_count < total_test_count { while let Ok(test_result) = receiver.recv() { if test_result.status.failed() { all_passed = false; @@ -257,17 +307,29 @@ impl<'a> TestRunner<'a> { continue; } - self.display_test_result(&test_result) - .expect("Could not display test status"); + self.display_test_result( + &test_result, + current_test_count + 1, + total_test_count, + ) + .expect("Could not display test status"); test_report.push(test_result); - remaining_test_count -= 1; - if remaining_test_count == 0 { + current_test_count += 1; + if current_test_count == total_test_count { break; } } } - display_test_report(package_name, &test_report) + self.formatter + .package_end( + package_name, + &test_report, + self.file_manager, + self.args.show_output, + self.args.compile_options.deny_warnings, + self.args.compile_options.silence_warnings, + ) .expect("Could not display test report"); } }); @@ -417,118 +479,22 @@ impl<'a> TestRunner<'a> { } /// Display the status of a single test - fn display_test_result(&'a self, test_result: &'a TestResult) -> std::io::Result<()> { - let writer = StandardStream::stderr(ColorChoice::Always); - let mut writer = writer.lock(); - - let is_slow = test_result.time_to_run >= Duration::from_secs(30); - let show_time = |writer: &mut StandardStreamLock<'_>| { - if is_slow { - write!(writer, " <{:.3}s>", test_result.time_to_run.as_secs_f64()) - } else { - Ok(()) - } - }; - - write!(writer, "[{}] Testing {}... ", &test_result.package_name, &test_result.name)?; - writer.flush()?; - - match &test_result.status { - TestStatus::Pass { .. } => { - writer.set_color(ColorSpec::new().set_fg(Some(Color::Green)))?; - write!(writer, "ok")?; - writer.reset()?; - show_time(&mut writer)?; - writeln!(writer)?; - } - TestStatus::Fail { message, error_diagnostic } => { - writer.set_color(ColorSpec::new().set_fg(Some(Color::Red)))?; - write!(writer, "FAIL\n{message}\n")?; - writer.reset()?; - show_time(&mut writer)?; - writeln!(writer)?; - if let Some(diag) = error_diagnostic { - noirc_errors::reporter::report_all( - self.file_manager.as_file_map(), - &[diag.clone()], - self.args.compile_options.deny_warnings, - self.args.compile_options.silence_warnings, - ); - } - } - TestStatus::Skipped { .. } => { - writer.set_color(ColorSpec::new().set_fg(Some(Color::Yellow)))?; - write!(writer, "skipped")?; - writer.reset()?; - show_time(&mut writer)?; - writeln!(writer)?; - } - TestStatus::CompileError(err) => { - noirc_errors::reporter::report_all( - self.file_manager.as_file_map(), - &[err.clone()], - self.args.compile_options.deny_warnings, - self.args.compile_options.silence_warnings, - ); - } - } - - if self.args.show_output && !test_result.output.is_empty() { - writeln!(writer, "--- {} stdout ---", test_result.name)?; - write!(writer, "{}", test_result.output)?; - let name_len = test_result.name.len(); - writeln!(writer, "{}", "-".repeat(name_len + "--- stdout ---".len())) - } else { - Ok(()) - } - } -} - -/// Display a report for all tests in a package -fn display_test_report(package_name: &String, test_report: &[TestResult]) -> std::io::Result<()> { - let writer = StandardStream::stderr(ColorChoice::Always); - let mut writer = writer.lock(); - - let failed_tests: Vec<_> = test_report - .iter() - .filter_map(|test_result| test_result.status.failed().then_some(&test_result.name)) - .collect(); - - if !failed_tests.is_empty() { - writeln!(writer)?; - writeln!(writer, "[{}] Failures:", package_name)?; - for failed_test in failed_tests { - writeln!(writer, " {}", failed_test)?; - } - writeln!(writer)?; - } - - write!(writer, "[{}] ", package_name)?; - - let count_all = test_report.len(); - let count_failed = test_report.iter().filter(|test_result| test_result.status.failed()).count(); - let plural = if count_all == 1 { "" } else { "s" }; - if count_failed == 0 { - writer.set_color(ColorSpec::new().set_fg(Some(Color::Green)))?; - write!(writer, "{count_all} test{plural} passed")?; - writer.reset()?; - writeln!(writer)?; - } else { - let count_passed = count_all - count_failed; - let plural_failed = if count_failed == 1 { "" } else { "s" }; - let plural_passed = if count_passed == 1 { "" } else { "s" }; - - if count_passed != 0 { - writer.set_color(ColorSpec::new().set_fg(Some(Color::Green)))?; - write!(writer, "{count_passed} test{plural_passed} passed, ")?; - } - - writer.set_color(ColorSpec::new().set_fg(Some(Color::Red)))?; - writeln!(writer, "{count_failed} test{plural_failed} failed")?; - writer.reset()?; + fn display_test_result( + &'a self, + test_result: &'a TestResult, + current_test_count: usize, + total_test_count: usize, + ) -> std::io::Result<()> { + self.formatter.test_end( + test_result, + current_test_count, + total_test_count, + self.file_manager, + self.args.show_output, + self.args.compile_options.deny_warnings, + self.args.compile_options.silence_warnings, + ) } - - Ok(()) } #[cfg(test)] diff --git a/noir/noir-repo/tooling/nargo_cli/src/cli/test_cmd/formatters.rs b/noir/noir-repo/tooling/nargo_cli/src/cli/test_cmd/formatters.rs new file mode 100644 index 00000000000..2a791930f60 --- /dev/null +++ b/noir/noir-repo/tooling/nargo_cli/src/cli/test_cmd/formatters.rs @@ -0,0 +1,324 @@ +use std::{io::Write, panic::RefUnwindSafe, time::Duration}; + +use fm::FileManager; +use nargo::ops::TestStatus; +use termcolor::{Color, ColorChoice, ColorSpec, StandardStream, StandardStreamLock, WriteColor}; + +use super::TestResult; + +pub(super) trait Formatter: Send + Sync + RefUnwindSafe { + fn package_start(&self, package_name: &str, test_count: usize) -> std::io::Result<()>; + + #[allow(clippy::too_many_arguments)] + fn test_end( + &self, + test_result: &TestResult, + current_test_count: usize, + total_test_count: usize, + file_manager: &FileManager, + show_output: bool, + deny_warnings: bool, + silence_warnings: bool, + ) -> std::io::Result<()>; + + fn package_end( + &self, + package_name: &str, + test_results: &[TestResult], + file_manager: &FileManager, + show_output: bool, + deny_warnings: bool, + silence_warnings: bool, + ) -> std::io::Result<()>; +} + +pub(super) struct PrettyFormatter; + +impl Formatter for PrettyFormatter { + fn package_start(&self, package_name: &str, test_count: usize) -> std::io::Result<()> { + package_start(package_name, test_count) + } + + fn test_end( + &self, + test_result: &TestResult, + _current_test_count: usize, + _total_test_count: usize, + file_manager: &FileManager, + show_output: bool, + deny_warnings: bool, + silence_warnings: bool, + ) -> std::io::Result<()> { + let writer = StandardStream::stderr(ColorChoice::Always); + let mut writer = writer.lock(); + + let is_slow = test_result.time_to_run >= Duration::from_secs(30); + let show_time = |writer: &mut StandardStreamLock<'_>| { + if is_slow { + write!(writer, " <{:.3}s>", test_result.time_to_run.as_secs_f64()) + } else { + Ok(()) + } + }; + + write!(writer, "[{}] Testing {}... ", &test_result.package_name, &test_result.name)?; + writer.flush()?; + + match &test_result.status { + TestStatus::Pass { .. } => { + writer.set_color(ColorSpec::new().set_fg(Some(Color::Green)))?; + write!(writer, "ok")?; + writer.reset()?; + show_time(&mut writer)?; + writeln!(writer)?; + } + TestStatus::Fail { message, error_diagnostic } => { + writer.set_color(ColorSpec::new().set_fg(Some(Color::Red)))?; + write!(writer, "FAIL\n{message}\n")?; + writer.reset()?; + show_time(&mut writer)?; + writeln!(writer)?; + if let Some(diag) = error_diagnostic { + noirc_errors::reporter::report_all( + file_manager.as_file_map(), + &[diag.clone()], + deny_warnings, + silence_warnings, + ); + } + } + TestStatus::Skipped { .. } => { + writer.set_color(ColorSpec::new().set_fg(Some(Color::Yellow)))?; + write!(writer, "skipped")?; + writer.reset()?; + show_time(&mut writer)?; + writeln!(writer)?; + } + TestStatus::CompileError(file_diagnostic) => { + noirc_errors::reporter::report_all( + file_manager.as_file_map(), + &[file_diagnostic.clone()], + deny_warnings, + silence_warnings, + ); + } + } + + if show_output && !test_result.output.is_empty() { + writeln!(writer, "--- {} stdout ---", test_result.name)?; + write!(writer, "{}", test_result.output)?; + let name_len = test_result.name.len(); + writeln!(writer, "{}", "-".repeat(name_len + "--- stdout ---".len())) + } else { + Ok(()) + } + } + + fn package_end( + &self, + package_name: &str, + test_results: &[TestResult], + _file_manager: &FileManager, + _show_output: bool, + _deny_warnings: bool, + _silence_warnings: bool, + ) -> std::io::Result<()> { + let writer = StandardStream::stderr(ColorChoice::Always); + let mut writer = writer.lock(); + + let failed_tests: Vec<_> = test_results + .iter() + .filter_map(|test_result| test_result.status.failed().then_some(&test_result.name)) + .collect(); + + if !failed_tests.is_empty() { + writeln!(writer)?; + writeln!(writer, "[{}] Failures:", package_name)?; + for failed_test in failed_tests { + writeln!(writer, " {}", failed_test)?; + } + writeln!(writer)?; + } + + write!(writer, "[{}] ", package_name)?; + + let count_all = test_results.len(); + let count_failed = + test_results.iter().filter(|test_result| test_result.status.failed()).count(); + let plural = if count_all == 1 { "" } else { "s" }; + if count_failed == 0 { + writer.set_color(ColorSpec::new().set_fg(Some(Color::Green)))?; + write!(writer, "{count_all} test{plural} passed")?; + writer.reset()?; + writeln!(writer)?; + } else { + let count_passed = count_all - count_failed; + let plural_failed = if count_failed == 1 { "" } else { "s" }; + let plural_passed = if count_passed == 1 { "" } else { "s" }; + + if count_passed != 0 { + writer.set_color(ColorSpec::new().set_fg(Some(Color::Green)))?; + write!(writer, "{count_passed} test{plural_passed} passed, ")?; + } + + writer.set_color(ColorSpec::new().set_fg(Some(Color::Red)))?; + writeln!(writer, "{count_failed} test{plural_failed} failed")?; + writer.reset()?; + } + + Ok(()) + } +} + +pub(super) struct TerseFormatter; + +impl Formatter for TerseFormatter { + fn package_start(&self, package_name: &str, test_count: usize) -> std::io::Result<()> { + package_start(package_name, test_count) + } + + fn test_end( + &self, + test_result: &TestResult, + current_test_count: usize, + total_test_count: usize, + _file_manager: &FileManager, + _show_output: bool, + _deny_warnings: bool, + _silence_warnings: bool, + ) -> std::io::Result<()> { + let writer = StandardStream::stderr(ColorChoice::Always); + let mut writer = writer.lock(); + + match &test_result.status { + TestStatus::Pass => { + writer.set_color(ColorSpec::new().set_fg(Some(Color::Green)))?; + write!(writer, ".")?; + writer.reset()?; + } + TestStatus::Fail { .. } | TestStatus::CompileError(_) => { + writer.set_color(ColorSpec::new().set_fg(Some(Color::Red)))?; + write!(writer, "F")?; + writer.reset()?; + } + TestStatus::Skipped => { + writer.set_color(ColorSpec::new().set_fg(Some(Color::Yellow)))?; + write!(writer, "s")?; + writer.reset()?; + } + } + + // How many tests ('.', 'F', etc.) to print per line. + // We use 88 which is a bit more than the traditional 80 columns (screens are larger these days) + // but we also want the output to be readable in case the terminal isn't maximized. + const MAX_TESTS_PER_LINE: usize = 88; + + if current_test_count % MAX_TESTS_PER_LINE == 0 && current_test_count < total_test_count { + writeln!(writer, " {}/{}", current_test_count, total_test_count)?; + } + + Ok(()) + } + + fn package_end( + &self, + package_name: &str, + test_results: &[TestResult], + file_manager: &FileManager, + show_output: bool, + deny_warnings: bool, + silence_warnings: bool, + ) -> std::io::Result<()> { + let writer = StandardStream::stderr(ColorChoice::Always); + let mut writer = writer.lock(); + + if !test_results.is_empty() { + writeln!(writer)?; + } + + for test_result in test_results { + if (show_output && !test_result.output.is_empty()) || test_result.status.failed() { + writeln!(writer, "--- {} stdout ---", test_result.name)?; + if !test_result.output.is_empty() { + write!(writer, "{}", test_result.output)?; + } + + match &test_result.status { + TestStatus::Pass | TestStatus::Skipped => (), + TestStatus::Fail { message, error_diagnostic } => { + writer.set_color(ColorSpec::new().set_fg(Some(Color::Red)))?; + writeln!(writer, "{message}")?; + writer.reset()?; + if let Some(diag) = error_diagnostic { + noirc_errors::reporter::report_all( + file_manager.as_file_map(), + &[diag.clone()], + deny_warnings, + silence_warnings, + ); + } + } + TestStatus::CompileError(file_diagnostic) => { + noirc_errors::reporter::report_all( + file_manager.as_file_map(), + &[file_diagnostic.clone()], + deny_warnings, + silence_warnings, + ); + } + } + + let name_len = test_result.name.len(); + writeln!(writer, "{}", "-".repeat(name_len + "--- stdout ---".len()))?; + } + } + + let failed_tests: Vec<_> = test_results + .iter() + .filter_map(|test_result| test_result.status.failed().then_some(&test_result.name)) + .collect(); + + if !failed_tests.is_empty() { + writeln!(writer)?; + writeln!(writer, "[{}] Failures:", package_name)?; + for failed_test in failed_tests { + writeln!(writer, " {}", failed_test)?; + } + writeln!(writer)?; + } + + write!(writer, "[{}] ", package_name)?; + + let count_all = test_results.len(); + let count_failed = + test_results.iter().filter(|test_result| test_result.status.failed()).count(); + let plural = if count_all == 1 { "" } else { "s" }; + if count_failed == 0 { + writer.set_color(ColorSpec::new().set_fg(Some(Color::Green)))?; + write!(writer, "{count_all} test{plural} passed")?; + writer.reset()?; + writeln!(writer)?; + } else { + let count_passed = count_all - count_failed; + let plural_failed = if count_failed == 1 { "" } else { "s" }; + let plural_passed = if count_passed == 1 { "" } else { "s" }; + + if count_passed != 0 { + writer.set_color(ColorSpec::new().set_fg(Some(Color::Green)))?; + write!(writer, "{count_passed} test{plural_passed} passed, ")?; + } + + writer.set_color(ColorSpec::new().set_fg(Some(Color::Red)))?; + writeln!(writer, "{count_failed} test{plural_failed} failed")?; + writer.reset()?; + } + + Ok(()) + } +} + +fn package_start(package_name: &str, test_count: usize) -> std::io::Result<()> { + let plural = if test_count == 1 { "" } else { "s" }; + println!("[{package_name}] Running {test_count} test function{plural}"); + Ok(()) +} diff --git a/noir/noir-repo/tooling/nargo_cli/src/errors.rs b/noir/noir-repo/tooling/nargo_cli/src/errors.rs index b28012ae7aa..9255d6fc6a6 100644 --- a/noir/noir-repo/tooling/nargo_cli/src/errors.rs +++ b/noir/noir-repo/tooling/nargo_cli/src/errors.rs @@ -2,7 +2,11 @@ use acvm::{acir::native_types::WitnessStackError, FieldElement}; use nargo::{errors::CompileError, NargoError}; use nargo_toml::ManifestError; use noir_debugger::errors::DapError; -use noirc_abi::errors::{AbiError, InputParserError}; +use noirc_abi::{ + errors::{AbiError, InputParserError}, + input_parser::InputValue, + AbiReturnType, +}; use std::path::PathBuf; use thiserror::Error; @@ -32,6 +36,7 @@ pub(crate) enum FilesystemError { pub(crate) enum CliError { #[error("{0}")] Generic(String), + #[error("Error: destination {} already exists", .0.display())] DestinationAlreadyExists(PathBuf), @@ -63,4 +68,10 @@ pub(crate) enum CliError { /// Error from the compilation pipeline #[error(transparent)] CompileError(#[from] CompileError), + + #[error("Unexpected return value: expected {expected:?}; got {actual:?}")] + UnexpectedReturn { expected: InputValue, actual: Option }, + + #[error("Missing return witnesses; expected {expected:?}")] + MissingReturn { expected: AbiReturnType }, }