From 18efd3a7357b71eb66e9da1737221e25d812b396 Mon Sep 17 00:00:00 2001 From: jeanmon Date: Thu, 8 Feb 2024 09:39:56 +0000 Subject: [PATCH 1/2] 4495 - use some matchers gtest functionalities to improve unite tests in AvmMini_execution.test.cpp --- .../vm/tests/AvmMini_execution.test.cpp | 87 +++++++++---------- 1 file changed, 40 insertions(+), 47 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/vm/tests/AvmMini_execution.test.cpp b/barretenberg/cpp/src/barretenberg/vm/tests/AvmMini_execution.test.cpp index 0dc7a4186f42..24c4d2648f76 100644 --- a/barretenberg/cpp/src/barretenberg/vm/tests/AvmMini_execution.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/tests/AvmMini_execution.test.cpp @@ -6,13 +6,14 @@ #include "barretenberg/vm/avm_trace/AvmMini_helper.hpp" #include "barretenberg/vm/avm_trace/AvmMini_opcode.hpp" #include "barretenberg/vm/tests/helpers.test.hpp" +#include "gmock/gmock.h" #include -#include #include #include #include using namespace bb; +using namespace testing; namespace { void gen_proof_and_validate(std::vector const& bytecode, std::vector&& trace, @@ -65,17 +66,17 @@ TEST_F(AvmMiniExecutionTests, basicAddReturn) auto instructions = Deserialization::parse(bytecode); // 2 instructions - EXPECT_THAT(instructions, testing::SizeIs(2)); + EXPECT_THAT(instructions, SizeIs(2)); // ADD EXPECT_EQ(instructions.at(0).op_code, OpCode::ADD); auto operands = instructions.at(0).operands; - EXPECT_EQ(operands.size(), 4); - EXPECT_EQ(std::get(operands.at(0)), AvmMemoryTag::U8); - EXPECT_EQ(std::get(operands.at(1)), 7); - EXPECT_EQ(std::get(operands.at(2)), 9); - EXPECT_EQ(std::get(operands.at(3)), 1); + EXPECT_THAT(operands, + ElementsAre(VariantWith(AvmMemoryTag::U8), + VariantWith(7), + VariantWith(9), + VariantWith(1))); // RETURN EXPECT_EQ(instructions.at(1).op_code, OpCode::RETURN); @@ -113,35 +114,34 @@ TEST_F(AvmMiniExecutionTests, setAndSubOpcodes) auto bytecode = hex_to_bytes(bytecode_hex); auto instructions = Deserialization::parse(bytecode); - EXPECT_EQ(instructions.size(), 4); + EXPECT_THAT(instructions, SizeIs(4)); // SET EXPECT_EQ(instructions.at(0).op_code, OpCode::SET); auto operands = instructions.at(0).operands; - EXPECT_EQ(operands.size(), 3); - EXPECT_EQ(std::get(operands.at(0)), AvmMemoryTag::U16); - EXPECT_EQ(std::get(operands.at(1)), 47123); - EXPECT_EQ(std::get(operands.at(2)), 170); + EXPECT_THAT(operands, + ElementsAre(VariantWith(AvmMemoryTag::U16), + VariantWith(47123), + VariantWith(170))); // SET EXPECT_EQ(instructions.at(1).op_code, OpCode::SET); operands = instructions.at(1).operands; - EXPECT_EQ(operands.size(), 3); - EXPECT_EQ(std::get(operands.at(0)), AvmMemoryTag::U16); - EXPECT_EQ(std::get(operands.at(1)), 37123); - EXPECT_EQ(std::get(operands.at(2)), 51); - + EXPECT_THAT(operands, + ElementsAre(VariantWith(AvmMemoryTag::U16), + VariantWith(37123), + VariantWith(51))); // SUB EXPECT_EQ(instructions.at(2).op_code, OpCode::SUB); operands = instructions.at(2).operands; - EXPECT_EQ(operands.size(), 4); - EXPECT_EQ(std::get(operands.at(0)), AvmMemoryTag::U16); - EXPECT_EQ(std::get(operands.at(1)), 170); - EXPECT_EQ(std::get(operands.at(2)), 51); - EXPECT_EQ(std::get(operands.at(3)), 1); + EXPECT_THAT(operands, + ElementsAre(VariantWith(AvmMemoryTag::U16), + VariantWith(170), + VariantWith(51), + VariantWith(1))); auto trace = Execution::gen_trace(instructions); @@ -191,35 +191,33 @@ TEST_F(AvmMiniExecutionTests, powerWithMulOpcodes) auto bytecode = hex_to_bytes(bytecode_hex); auto instructions = Deserialization::parse(bytecode); - EXPECT_EQ(instructions.size(), 15); + EXPECT_THAT(instructions, SizeIs(15)); // MUL first pos EXPECT_EQ(instructions.at(2).op_code, OpCode::MUL); auto operands = instructions.at(2).operands; - EXPECT_EQ(operands.size(), 4); - EXPECT_EQ(std::get(operands.at(0)), AvmMemoryTag::U64); - EXPECT_EQ(std::get(operands.at(1)), 0); - EXPECT_EQ(std::get(operands.at(2)), 1); - EXPECT_EQ(std::get(operands.at(3)), 1); + EXPECT_THAT(operands, + ElementsAre(VariantWith(AvmMemoryTag::U64), + VariantWith(0), + VariantWith(1), + VariantWith(1))); // MUL last pos EXPECT_EQ(instructions.at(13).op_code, OpCode::MUL); operands = instructions.at(13).operands; - EXPECT_EQ(operands.size(), 4); - EXPECT_EQ(std::get(operands.at(0)), AvmMemoryTag::U64); - EXPECT_EQ(std::get(operands.at(1)), 0); - EXPECT_EQ(std::get(operands.at(2)), 1); - EXPECT_EQ(std::get(operands.at(3)), 1); + EXPECT_THAT(operands, + ElementsAre(VariantWith(AvmMemoryTag::U64), + VariantWith(0), + VariantWith(1), + VariantWith(1))); // RETURN EXPECT_EQ(instructions.at(14).op_code, OpCode::RETURN); operands = instructions.at(14).operands; - EXPECT_EQ(operands.size(), 2); - EXPECT_EQ(std::get(operands.at(0)), 0); - EXPECT_EQ(std::get(operands.at(1)), 0); + EXPECT_THAT(operands, ElementsAre(VariantWith(0), VariantWith(0))); auto trace = Execution::gen_trace(instructions); @@ -266,14 +264,13 @@ TEST_F(AvmMiniExecutionTests, simpleInternalCall) auto bytecode = hex_to_bytes(bytecode_hex); auto instructions = Deserialization::parse(bytecode); - EXPECT_EQ(instructions.size(), 6); + EXPECT_THAT(instructions, SizeIs(6)); // We test parsing step for INTERNALCALL and INTERNALRETURN. // INTERNALCALL EXPECT_EQ(instructions.at(1).op_code, OpCode::INTERNALCALL); - EXPECT_EQ(instructions.at(1).operands.size(), 1); - EXPECT_EQ(std::get(instructions.at(1).operands.at(0)), 4); + EXPECT_THAT(instructions.at(1).operands, ElementsAre(VariantWith(4))); // INTERNALRETURN EXPECT_EQ(instructions.at(5).op_code, OpCode::INTERNALRETURN); @@ -343,7 +340,7 @@ TEST_F(AvmMiniExecutionTests, nestedInternalCalls) auto bytecode = hex_to_bytes(bytecode_hex); auto instructions = Deserialization::parse(bytecode); - EXPECT_EQ(instructions.size(), 12); + EXPECT_THAT(instructions, SizeIs(12)); // Expected sequence of opcodes std::vector const opcode_sequence{ OpCode::SET, OpCode::SET, @@ -406,23 +403,19 @@ TEST_F(AvmMiniExecutionTests, jumpAndCalldatacopy) auto bytecode = hex_to_bytes(bytecode_hex); auto instructions = Deserialization::parse(bytecode); - EXPECT_EQ(instructions.size(), 5); + EXPECT_THAT(instructions, SizeIs(5)); // We test parsing steps for CALLDATACOPY and JUMP. // CALLDATACOPY EXPECT_EQ(instructions.at(0).op_code, OpCode::CALLDATACOPY); - EXPECT_EQ(instructions.at(0).operands.size(), 3); auto operands = instructions.at(0).operands; - EXPECT_EQ(std::get(operands.at(0)), 0); - EXPECT_EQ(std::get(operands.at(1)), 2); - EXPECT_EQ(std::get(operands.at(2)), 10); + EXPECT_THAT(operands, ElementsAre(VariantWith(0), VariantWith(2), VariantWith(10))); // JUMP EXPECT_EQ(instructions.at(1).op_code, OpCode::JUMP); - EXPECT_EQ(instructions.at(1).operands.size(), 1); - EXPECT_EQ(std::get(instructions.at(1).operands.at(0)), 3); + EXPECT_THAT(instructions.at(1).operands, ElementsAre(VariantWith(3))); auto trace = Execution::gen_trace(instructions, std::vector{ 13, 156 }); From acfb1e9af9a3d290688ca7259a7dc0ede8b2e9f1 Mon Sep 17 00:00:00 2001 From: jeanmon Date: Thu, 8 Feb 2024 14:42:26 +0000 Subject: [PATCH 2/2] 4495 - address review comments --- .../vm/tests/AvmMini_execution.test.cpp | 134 ++++++++---------- 1 file changed, 63 insertions(+), 71 deletions(-) diff --git a/barretenberg/cpp/src/barretenberg/vm/tests/AvmMini_execution.test.cpp b/barretenberg/cpp/src/barretenberg/vm/tests/AvmMini_execution.test.cpp index 24c4d2648f76..c6680631aeca 100644 --- a/barretenberg/cpp/src/barretenberg/vm/tests/AvmMini_execution.test.cpp +++ b/barretenberg/cpp/src/barretenberg/vm/tests/AvmMini_execution.test.cpp @@ -66,28 +66,23 @@ TEST_F(AvmMiniExecutionTests, basicAddReturn) auto instructions = Deserialization::parse(bytecode); // 2 instructions - EXPECT_THAT(instructions, SizeIs(2)); + ASSERT_THAT(instructions, SizeIs(2)); // ADD - EXPECT_EQ(instructions.at(0).op_code, OpCode::ADD); - - auto operands = instructions.at(0).operands; - EXPECT_THAT(operands, - ElementsAre(VariantWith(AvmMemoryTag::U8), - VariantWith(7), - VariantWith(9), - VariantWith(1))); + EXPECT_THAT(instructions.at(0), + AllOf(Field(&Instruction::op_code, OpCode::ADD), + Field(&Instruction::operands, + ElementsAre(VariantWith(AvmMemoryTag::U8), + VariantWith(7), + VariantWith(9), + VariantWith(1))))); // RETURN - EXPECT_EQ(instructions.at(1).op_code, OpCode::RETURN); - - operands = instructions.at(1).operands; - EXPECT_EQ(operands.size(), 2); - EXPECT_EQ(std::get(operands.at(0)), 0); - EXPECT_EQ(std::get(operands.at(1)), 0); + EXPECT_THAT(instructions.at(1), + AllOf(Field(&Instruction::op_code, OpCode::RETURN), + Field(&Instruction::operands, ElementsAre(VariantWith(0), VariantWith(0))))); auto trace = Execution::gen_trace(instructions); - gen_proof_and_validate(bytecode, std::move(trace), {}); } @@ -114,34 +109,32 @@ TEST_F(AvmMiniExecutionTests, setAndSubOpcodes) auto bytecode = hex_to_bytes(bytecode_hex); auto instructions = Deserialization::parse(bytecode); - EXPECT_THAT(instructions, SizeIs(4)); + ASSERT_THAT(instructions, SizeIs(4)); // SET - EXPECT_EQ(instructions.at(0).op_code, OpCode::SET); - - auto operands = instructions.at(0).operands; - EXPECT_THAT(operands, - ElementsAre(VariantWith(AvmMemoryTag::U16), - VariantWith(47123), - VariantWith(170))); + EXPECT_THAT(instructions.at(0), + AllOf(Field(&Instruction::op_code, OpCode::SET), + Field(&Instruction::operands, + ElementsAre(VariantWith(AvmMemoryTag::U16), + VariantWith(47123), + VariantWith(170))))); // SET - EXPECT_EQ(instructions.at(1).op_code, OpCode::SET); + EXPECT_THAT(instructions.at(1), + AllOf(Field(&Instruction::op_code, OpCode::SET), + Field(&Instruction::operands, + ElementsAre(VariantWith(AvmMemoryTag::U16), + VariantWith(37123), + VariantWith(51))))); - operands = instructions.at(1).operands; - EXPECT_THAT(operands, - ElementsAre(VariantWith(AvmMemoryTag::U16), - VariantWith(37123), - VariantWith(51))); // SUB - EXPECT_EQ(instructions.at(2).op_code, OpCode::SUB); - - operands = instructions.at(2).operands; - EXPECT_THAT(operands, - ElementsAre(VariantWith(AvmMemoryTag::U16), - VariantWith(170), - VariantWith(51), - VariantWith(1))); + EXPECT_THAT(instructions.at(2), + AllOf(Field(&Instruction::op_code, OpCode::SUB), + Field(&Instruction::operands, + ElementsAre(VariantWith(AvmMemoryTag::U16), + VariantWith(170), + VariantWith(51), + VariantWith(1))))); auto trace = Execution::gen_trace(instructions); @@ -181,8 +174,7 @@ TEST_F(AvmMiniExecutionTests, powerWithMulOpcodes) "00000000" // ret offset 0 "00000000"; // ret size 0 - uint8_t num = 12; - while (num-- > 0) { + for (int i = 0; i < 12; i++) { bytecode_hex.append(mul_hex); } @@ -191,33 +183,30 @@ TEST_F(AvmMiniExecutionTests, powerWithMulOpcodes) auto bytecode = hex_to_bytes(bytecode_hex); auto instructions = Deserialization::parse(bytecode); - EXPECT_THAT(instructions, SizeIs(15)); + ASSERT_THAT(instructions, SizeIs(15)); // MUL first pos - EXPECT_EQ(instructions.at(2).op_code, OpCode::MUL); - - auto operands = instructions.at(2).operands; - EXPECT_THAT(operands, - ElementsAre(VariantWith(AvmMemoryTag::U64), - VariantWith(0), - VariantWith(1), - VariantWith(1))); + EXPECT_THAT(instructions.at(2), + AllOf(Field(&Instruction::op_code, OpCode::MUL), + Field(&Instruction::operands, + ElementsAre(VariantWith(AvmMemoryTag::U64), + VariantWith(0), + VariantWith(1), + VariantWith(1))))); // MUL last pos - EXPECT_EQ(instructions.at(13).op_code, OpCode::MUL); - - operands = instructions.at(13).operands; - EXPECT_THAT(operands, - ElementsAre(VariantWith(AvmMemoryTag::U64), - VariantWith(0), - VariantWith(1), - VariantWith(1))); + EXPECT_THAT(instructions.at(13), + AllOf(Field(&Instruction::op_code, OpCode::MUL), + Field(&Instruction::operands, + ElementsAre(VariantWith(AvmMemoryTag::U64), + VariantWith(0), + VariantWith(1), + VariantWith(1))))); // RETURN - EXPECT_EQ(instructions.at(14).op_code, OpCode::RETURN); - operands = instructions.at(14).operands; - - EXPECT_THAT(operands, ElementsAre(VariantWith(0), VariantWith(0))); + EXPECT_THAT(instructions.at(14), + AllOf(Field(&Instruction::op_code, OpCode::RETURN), + Field(&Instruction::operands, ElementsAre(VariantWith(0), VariantWith(0))))); auto trace = Execution::gen_trace(instructions); @@ -269,8 +258,9 @@ TEST_F(AvmMiniExecutionTests, simpleInternalCall) // We test parsing step for INTERNALCALL and INTERNALRETURN. // INTERNALCALL - EXPECT_EQ(instructions.at(1).op_code, OpCode::INTERNALCALL); - EXPECT_THAT(instructions.at(1).operands, ElementsAre(VariantWith(4))); + EXPECT_THAT(instructions.at(1), + AllOf(Field(&Instruction::op_code, OpCode::INTERNALCALL), + Field(&Instruction::operands, ElementsAre(VariantWith(4))))); // INTERNALRETURN EXPECT_EQ(instructions.at(5).op_code, OpCode::INTERNALRETURN); @@ -340,7 +330,7 @@ TEST_F(AvmMiniExecutionTests, nestedInternalCalls) auto bytecode = hex_to_bytes(bytecode_hex); auto instructions = Deserialization::parse(bytecode); - EXPECT_THAT(instructions, SizeIs(12)); + ASSERT_THAT(instructions, SizeIs(12)); // Expected sequence of opcodes std::vector const opcode_sequence{ OpCode::SET, OpCode::SET, @@ -403,19 +393,21 @@ TEST_F(AvmMiniExecutionTests, jumpAndCalldatacopy) auto bytecode = hex_to_bytes(bytecode_hex); auto instructions = Deserialization::parse(bytecode); - EXPECT_THAT(instructions, SizeIs(5)); + ASSERT_THAT(instructions, SizeIs(5)); // We test parsing steps for CALLDATACOPY and JUMP. // CALLDATACOPY - EXPECT_EQ(instructions.at(0).op_code, OpCode::CALLDATACOPY); - - auto operands = instructions.at(0).operands; - EXPECT_THAT(operands, ElementsAre(VariantWith(0), VariantWith(2), VariantWith(10))); + EXPECT_THAT( + instructions.at(0), + AllOf(Field(&Instruction::op_code, OpCode::CALLDATACOPY), + Field(&Instruction::operands, + ElementsAre(VariantWith(0), VariantWith(2), VariantWith(10))))); // JUMP - EXPECT_EQ(instructions.at(1).op_code, OpCode::JUMP); - EXPECT_THAT(instructions.at(1).operands, ElementsAre(VariantWith(3))); + EXPECT_THAT(instructions.at(1), + AllOf(Field(&Instruction::op_code, OpCode::JUMP), + Field(&Instruction::operands, ElementsAre(VariantWith(3))))); auto trace = Execution::gen_trace(instructions, std::vector{ 13, 156 });