From 8e52d53ab003bb4ed31c8d7bb617d0e2c725d964 Mon Sep 17 00:00:00 2001 From: Sujith Date: Thu, 12 Oct 2023 11:02:25 -0400 Subject: [PATCH] fix: pr comments --- .../libraries/StringAddressConversion.t.sol | 46 ++++++++++--------- .../libraries/EIP5164/ExecutorAware.t.sol | 6 +-- test/unit-tests/libraries/Message.t.sol | 16 ++++--- test/unit-tests/libraries/TypeCasts.t.sol | 14 +++--- 4 files changed, 44 insertions(+), 38 deletions(-) diff --git a/test/unit-tests/adapters/axelar/libraries/StringAddressConversion.t.sol b/test/unit-tests/adapters/axelar/libraries/StringAddressConversion.t.sol index 7629aff..8898c5f 100644 --- a/test/unit-tests/adapters/axelar/libraries/StringAddressConversion.t.sol +++ b/test/unit-tests/adapters/axelar/libraries/StringAddressConversion.t.sol @@ -8,7 +8,9 @@ import {Test, Vm} from "forge-std/Test.sol"; import "src/adapters/axelar/libraries/StringAddressConversion.sol"; /// @dev helper for testing StringAddressConversion library -contract StringAddressConversionHelper { +/// @dev library testing using foundry can only be done through helper contracts +/// @dev see https://github.com/foundry-rs/foundry/issues/2567 +contract StringAddressConversionTestClient { function toString(address _addr) external pure returns (string memory) { return StringAddressConversion.toString(_addr); } @@ -19,13 +21,13 @@ contract StringAddressConversionHelper { } contract StringAddressConversionTest is Test { - StringAddressConversionHelper public conversionHelper; + StringAddressConversionTestClient public conversionHelper; /*/////////////////////////////////////////////////////////////// SETUP //////////////////////////////////////////////////////////////*/ function setUp() public { - conversionHelper = new StringAddressConversionHelper(); + conversionHelper = new StringAddressConversionTestClient(); } /*/////////////////////////////////////////////////////////////// @@ -33,85 +35,85 @@ contract StringAddressConversionTest is Test { //////////////////////////////////////////////////////////////*/ /// @dev tests conversion of address to string - function testToString() public { + function test_to_string() public { address testAddr = address(0x1234567890123456789012345678901234567890); string memory result = conversionHelper.toString(testAddr); string memory expected = "0x1234567890123456789012345678901234567890"; - assertTrue( - keccak256(bytes(result)) == keccak256(bytes(expected)), "Converted string does not match expected value" + assertEq( + keccak256(bytes(result)), keccak256(bytes(expected)) ); } /// @dev tests conversion of string to address - function testToAddress() public { + function test_to_address() public { string memory testString = "0x1234567890123456789012345678901234567890"; address result = conversionHelper.toAddress(testString); address expected = address(0x1234567890123456789012345678901234567890); - assertTrue(result == expected, "Converted address does not match expected value"); + assertEq(result,expected); } /// @dev tests invalid address conversion - function testInvalidAddressStringConversion() public { + function test_invalid_address_string_conversion() public { string memory invalidAddressString = "1234567890123456789012345678901234567892"; - bytes4 selector = bytes4(keccak256(bytes("InvalidAddressString()"))); + bytes4 selector = StringAddressConversion.InvalidAddressString.selector; vm.expectRevert(selector); conversionHelper.toAddress(invalidAddressString); } /// @dev tests short address string - function testShortAddressStringConversion() public { + function test_short_address_string_conversion() public { string memory shortAddressString = "0x12345678901234567890123456789012345678"; - bytes4 selector = bytes4(keccak256(bytes("InvalidAddressString()"))); + bytes4 selector = StringAddressConversion.InvalidAddressString.selector; vm.expectRevert(selector); conversionHelper.toAddress(shortAddressString); } /// @dev tests long address string - function testLongAddressStringConversion() public { + function test_long_address_string_conversion() public { string memory longAddressString = "0x123456789012345678901234567890123456789012"; - bytes4 selector = bytes4(keccak256(bytes("InvalidAddressString()"))); + bytes4 selector = StringAddressConversion.InvalidAddressString.selector; vm.expectRevert(selector); conversionHelper.toAddress(longAddressString); } /// @dev tests invalid prefix in address string - function testInvalidPrefixAddressStringConversion() public { + function test_invalid_prefix_address_string_conversion() public { string memory invalidPrefixAddressString = "1x1234567890123456789012345678901234567890"; - bytes4 selector = bytes4(keccak256(bytes("InvalidAddressString()"))); + bytes4 selector = StringAddressConversion.InvalidAddressString.selector; vm.expectRevert(selector); conversionHelper.toAddress(invalidPrefixAddressString); } /// @dev tests address string with invalid characters - function testInvalidCharacterAddressStringConversion() public { + function test_invalid_character_address_string_conversion() public { string memory invalidCharacterAddressString = "0x12345678901234567890123456789012345678g0"; // 'g' is an invalid character - bytes4 selector = bytes4(keccak256(bytes("InvalidAddressString()"))); + bytes4 selector = StringAddressConversion.InvalidAddressString.selector; vm.expectRevert(selector); conversionHelper.toAddress(invalidCharacterAddressString); } /// @dev tests conversion of string with lowercase hex characters to address - function testLowercaseHexCharacterToAddress() public { + function test_lowercase_hex_character_to_address() public { string memory testString = "0xabcdefabcdefabcdefabcdefabcdefabcdefabcd"; address result = conversionHelper.toAddress(testString); address expected = address(0xABcdEFABcdEFabcdEfAbCdefabcdeFABcDEFabCD); - assertTrue(result == expected, "Converted address with lowercase hex characters does not match expected value"); + assertEq(result,expected); } /// @dev tests conversion of string with uppercase hex characters to address - function testUppercaseHexCharacterToAddress() public { + function test_ppercase_hex_character_to_address() public { string memory testString = "0xABCDEFABCDEFABCDEFABCDEFABCDEFABCDEFABCD"; address result = conversionHelper.toAddress(testString); address expected = address(0xABcdEFABcdEFabcdEfAbCdefabcdeFABcDEFabCD); - assertTrue(result == expected, "Converted address with uppercase hex characters does not match expected value"); + assertEq(result, expected); } } diff --git a/test/unit-tests/libraries/EIP5164/ExecutorAware.t.sol b/test/unit-tests/libraries/EIP5164/ExecutorAware.t.sol index b0e60bd..4fe9bae 100644 --- a/test/unit-tests/libraries/EIP5164/ExecutorAware.t.sol +++ b/test/unit-tests/libraries/EIP5164/ExecutorAware.t.sol @@ -8,7 +8,7 @@ import {Test, Vm} from "forge-std/Test.sol"; import "src/libraries/EIP5164/ExecutorAware.sol"; /// @dev helper to test abstract contract -contract ExecutorAwareHelper is ExecutorAware { +contract ExecutorAwareTestClient is ExecutorAware { function addTrustedExecutor(address _executor) external returns (bool) { return _addTrustedExecutor(_executor); } @@ -19,13 +19,13 @@ contract ExecutorAwareHelper is ExecutorAware { } contract ExecutorAwareTest is Test { - ExecutorAwareHelper public executorAware; + ExecutorAwareTestClient public executorAware; /*/////////////////////////////////////////////////////////////// SETUP //////////////////////////////////////////////////////////////*/ function setUp() public { - executorAware = new ExecutorAwareHelper(); + executorAware = new ExecutorAwareTestClient(); } /*/////////////////////////////////////////////////////////////// diff --git a/test/unit-tests/libraries/Message.t.sol b/test/unit-tests/libraries/Message.t.sol index 3818d81..cd37884 100644 --- a/test/unit-tests/libraries/Message.t.sol +++ b/test/unit-tests/libraries/Message.t.sol @@ -8,7 +8,9 @@ import {Test, Vm} from "forge-std/Test.sol"; import "src/libraries/Message.sol"; /// @dev is a helper function to test library contracts -contract MessageHelper { +/// @dev library testing using foundry can only be done through helper contracts +/// @dev see https://github.com/foundry-rs/foundry/issues/2567 +contract MessageLibraryTestClient { using MessageLibrary for MessageLibrary.Message; function computeMsgId(MessageLibrary.Message memory _message) external pure returns (bytes32) { @@ -41,10 +43,10 @@ contract MessageHelper { } contract MessageLibraryTest is Test { - MessageHelper messageHelper; + MessageLibraryTestClient messageLibraryTestClient; function setUp() public { - messageHelper = new MessageHelper(); + messageLibraryTestClient = new MessageLibraryTestClient(); } /// @dev tests computation of message id @@ -62,7 +64,7 @@ contract MessageLibraryTest is Test { expiration: 10000 }); - bytes32 computedId = messageHelper.computeMsgId(message); + bytes32 computedId = messageLibraryTestClient.computeMsgId(message); // Update the expectedId calculation to use the bytes constant bytes32 expectedId = keccak256( @@ -92,7 +94,7 @@ contract MessageLibraryTest is Test { expiration: 10000 }); - MessageLibrary.MessageExecutionParams memory params = messageHelper.extractExecutionParams(message); + MessageLibrary.MessageExecutionParams memory params = messageLibraryTestClient.extractExecutionParams(message); assertEq(params.target, address(0x1234567890123456789012345678901234567890)); assertEq(keccak256(params.callData), keccak256(hex"abcdef")); @@ -111,7 +113,7 @@ contract MessageLibraryTest is Test { expiration: 10000 }); - bytes32 computedHash = messageHelper.computeExecutionParamsHash(params); + bytes32 computedHash = messageLibraryTestClient.computeExecutionParamsHash(params); bytes32 expectedHash = keccak256( abi.encodePacked( address(0x1234567890123456789012345678901234567890), @@ -137,7 +139,7 @@ contract MessageLibraryTest is Test { expiration: 10000 }); - bytes32 computedHash = messageHelper.computeExecutionParamsHashFromMessage(message); + bytes32 computedHash = messageLibraryTestClient.computeExecutionParamsHashFromMessage(message); bytes32 expectedHash = keccak256( abi.encodePacked( address(0x1234567890123456789012345678901234567890), diff --git a/test/unit-tests/libraries/TypeCasts.t.sol b/test/unit-tests/libraries/TypeCasts.t.sol index e748e2f..483fe5d 100644 --- a/test/unit-tests/libraries/TypeCasts.t.sol +++ b/test/unit-tests/libraries/TypeCasts.t.sol @@ -8,7 +8,9 @@ import {Test, Vm} from "forge-std/Test.sol"; import "src/libraries/TypeCasts.sol"; /// @dev helper to test TypeCasts library -contract TypeCastsHelper { +/// @dev library testing using foundry can only be done through helper contracts +/// @dev see https://github.com/foundry-rs/foundry/issues/2567 +contract TypeCastsLibraryTestClient { function addressToBytes32(address _addr) external pure returns (bytes32) { return TypeCasts.addressToBytes32(_addr); } @@ -19,13 +21,13 @@ contract TypeCastsHelper { } contract TypeCastsTest is Test { - TypeCastsHelper public typeCastsHelper; + TypeCastsLibraryTestClient public typeCastsLibraryTestClient; /*/////////////////////////////////////////////////////////////// SETUP //////////////////////////////////////////////////////////////*/ function setUp() public { - typeCastsHelper = new TypeCastsHelper(); + typeCastsLibraryTestClient = new TypeCastsLibraryTestClient(); } /*/////////////////////////////////////////////////////////////// @@ -36,17 +38,17 @@ contract TypeCastsTest is Test { function test_address_to_bytes32() public { address testAddr = address(0x1234567890123456789012345678901234567890); bytes32 expected = bytes32(uint256(uint160(testAddr))); // Correct casting here - bytes32 result = typeCastsHelper.addressToBytes32(testAddr); + bytes32 result = typeCastsLibraryTestClient.addressToBytes32(testAddr); assertEq(result, expected); } /// @dev tests conversion of bytes32 to address - function testBytes32ToAddress() public { + function test_bytes32_to_address() public { bytes32 testBytes = bytes32(uint256(uint160(0x1234567890123456789012345678901234567890))); address expected = address(uint160(uint256(testBytes))); // Correct casting here - address result = typeCastsHelper.bytes32ToAddress(testBytes); + address result = typeCastsLibraryTestClient.bytes32ToAddress(testBytes); assertEq(result, expected); }