From beb836e33f9a207f4927abb7cd09ad0afe4b3f9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Milo=C5=A1=20=C4=90urica?= Date: Tue, 17 Sep 2024 15:22:30 +0200 Subject: [PATCH] test: renaming tests to follow best practices from Foundry Book (#600) * Added _RevertIf_ in test/StdChains.t.sol * changed tests names in test/StdChains.t.sol * changed tests names in test/StdError.t.sol * named imports and ternary operators instead of if-else for cleaner code * named imports in test/StdStyle.t.sol * named imports in test/StdToml.t.sol * named imports and changed tests names in test/StdUtils.t.sol * named imports in test/StdAssertions.t.sol * named imports and changed tests names in test/StdCheats.t.sol * named imports in test/StdJson.t.sol * named imports and changed tests names in test/StdStorage.t.sol --- test/StdAssertions.t.sol | 2 +- test/StdChains.t.sol | 24 ++++++++++++------------ test/StdCheats.t.sol | 20 ++++++++++---------- test/StdError.t.sol | 24 ++++++++++++------------ test/StdJson.t.sol | 2 +- test/StdMath.t.sol | 18 ++++-------------- test/StdStorage.t.sol | 10 +++++----- test/StdStyle.t.sol | 2 +- test/StdToml.t.sol | 2 +- test/StdUtils.t.sol | 24 ++++++++++++------------ 10 files changed, 59 insertions(+), 69 deletions(-) diff --git a/test/StdAssertions.t.sol b/test/StdAssertions.t.sol index ea794687..9385a607 100644 --- a/test/StdAssertions.t.sol +++ b/test/StdAssertions.t.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity >=0.7.0 <0.9.0; -import "../src/StdAssertions.sol"; +import {StdAssertions} from "../src/StdAssertions.sol"; import {Vm} from "../src/Vm.sol"; interface VmInternal is Vm { diff --git a/test/StdChains.t.sol b/test/StdChains.t.sol index 783ba69c..48130da5 100644 --- a/test/StdChains.t.sol +++ b/test/StdChains.t.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity >=0.7.0 <0.9.0; -import "../src/Test.sol"; +import {Test} from "../src/Test.sol"; contract StdChainsMock is Test { function exposed_getChain(string memory chainAlias) public returns (Chain memory) { @@ -84,7 +84,7 @@ contract StdChainsTest is Test { // _testRpc("flare_coston2"); // } - function test_ChainNoDefault() public { + function test_RevertIf_ChainNotFound() public { // We deploy a mock to properly test the revert. StdChainsMock stdChainsMock = new StdChainsMock(); @@ -92,7 +92,7 @@ contract StdChainsTest is Test { stdChainsMock.exposed_getChain("does_not_exist"); } - function test_SetChainFirstFails() public { + function test_RevertIf_SetChain_ChainIdExist_FirstTest() public { // We deploy a mock to properly test the revert. StdChainsMock stdChainsMock = new StdChainsMock(); @@ -100,7 +100,7 @@ contract StdChainsTest is Test { stdChainsMock.exposed_setChain("anvil2", ChainData("Anvil", 31337, "URL")); } - function test_ChainBubbleUp() public { + function test_RevertIf_ChainBubbleUp() public { // We deploy a mock to properly test the revert. StdChainsMock stdChainsMock = new StdChainsMock(); @@ -111,7 +111,7 @@ contract StdChainsTest is Test { stdChainsMock.exposed_getChain("needs_undefined_env_var"); } - function test_CannotSetChain_ChainIdExists() public { + function test_RevertIf_SetChain_ChainIdExists_SecondTest() public { // We deploy a mock to properly test the revert. StdChainsMock stdChainsMock = new StdChainsMock(); @@ -148,7 +148,7 @@ contract StdChainsTest is Test { assertEq(chainById.chainId, 123456789); } - function test_SetNoEmptyAlias() public { + function test_RevertIf_SetEmptyAlias() public { // We deploy a mock to properly test the revert. StdChainsMock stdChainsMock = new StdChainsMock(); @@ -156,7 +156,7 @@ contract StdChainsTest is Test { stdChainsMock.exposed_setChain("", ChainData("", 123456789, "")); } - function test_SetNoChainId0() public { + function test_RevertIf_SetNoChainId0() public { // We deploy a mock to properly test the revert. StdChainsMock stdChainsMock = new StdChainsMock(); @@ -164,7 +164,7 @@ contract StdChainsTest is Test { stdChainsMock.exposed_setChain("alias", ChainData("", 0, "")); } - function test_GetNoChainId0() public { + function test_RevertIf_GetNoChainId0() public { // We deploy a mock to properly test the revert. StdChainsMock stdChainsMock = new StdChainsMock(); @@ -172,7 +172,7 @@ contract StdChainsTest is Test { stdChainsMock.exposed_getChain(0); } - function test_GetNoEmptyAlias() public { + function test_RevertIf_GetNoEmptyAlias() public { // We deploy a mock to properly test the revert. StdChainsMock stdChainsMock = new StdChainsMock(); @@ -180,7 +180,7 @@ contract StdChainsTest is Test { stdChainsMock.exposed_getChain(""); } - function test_ChainIdNotFound() public { + function test_RevertIf_ChainIdNotFound() public { // We deploy a mock to properly test the revert. StdChainsMock stdChainsMock = new StdChainsMock(); @@ -188,7 +188,7 @@ contract StdChainsTest is Test { stdChainsMock.exposed_getChain("no_such_alias"); } - function test_ChainAliasNotFound() public { + function test_RevertIf_ChainAliasNotFound() public { // We deploy a mock to properly test the revert. StdChainsMock stdChainsMock = new StdChainsMock(); @@ -214,7 +214,7 @@ contract StdChainsTest is Test { assertEq(modifiedChain.rpcUrl, "https://modified.chain/"); } - function test_DontUseDefaultRpcUrl() public { + function test_RevertIf_DontUseDefaultRpcUrl() public { // We deploy a mock to properly test the revert. StdChainsMock stdChainsMock = new StdChainsMock(); diff --git a/test/StdCheats.t.sol b/test/StdCheats.t.sol index 7bac4286..0a5a8323 100644 --- a/test/StdCheats.t.sol +++ b/test/StdCheats.t.sol @@ -1,11 +1,11 @@ // SPDX-License-Identifier: MIT pragma solidity >=0.7.0 <0.9.0; -import "../src/StdCheats.sol"; -import "../src/Test.sol"; -import "../src/StdJson.sol"; -import "../src/StdToml.sol"; -import "../src/interfaces/IERC20.sol"; +import {StdCheats} from "../src/StdCheats.sol"; +import {Test} from "../src/Test.sol"; +import {stdJson} from "../src/StdJson.sol"; +import {stdToml} from "../src/StdToml.sol"; +import {IERC20} from "../src/interfaces/IERC20.sol"; contract StdCheatsTest is Test { Bar test; @@ -205,7 +205,7 @@ contract StdCheatsTest is Test { deployCode(what); } - function test_DeployCodeFail() public { + function test_RevertIf_DeployCodeFail() public { vm.expectRevert(bytes("StdCheats deployCode(string): Deployment failed.")); this.deployCodeHelper("StdCheats.t.sol:RevertingContract"); } @@ -415,7 +415,7 @@ contract StdCheatsTest is Test { ); } - function test_CannotDeployCodeTo() external { + function test_RevertIf_CannotDeployCodeTo() external { vm.expectRevert("StdCheats deployCodeTo(string,bytes,uint256,address): Failed to create runtime bytecode."); this._revertDeployCodeTo(); } @@ -470,7 +470,7 @@ contract StdCheatsForkTest is Test { vm.createSelectFork({urlOrAlias: "mainnet", blockNumber: 16_428_900}); } - function test_CannotAssumeNoBlacklisted_EOA() external { + function test_RevertIf_CannotAssumeNoBlacklisted_EOA() external { // We deploy a mock version so we can properly test the revert. StdCheatsMock stdCheatsMock = new StdCheatsMock(); address eoa = vm.addr({privateKey: 1}); @@ -483,7 +483,7 @@ contract StdCheatsForkTest is Test { assertTrue(true); } - function test_AssumeNoBlacklisted_USDC() external { + function test_RevertIf_AssumeNoBlacklisted_USDC() external { // We deploy a mock version so we can properly test the revert. StdCheatsMock stdCheatsMock = new StdCheatsMock(); vm.expectRevert(); @@ -495,7 +495,7 @@ contract StdCheatsForkTest is Test { assertFalse(USDCLike(USDC).isBlacklisted(addr)); } - function test_AssumeNoBlacklisted_USDT() external { + function test_RevertIf_AssumeNoBlacklisted_USDT() external { // We deploy a mock version so we can properly test the revert. StdCheatsMock stdCheatsMock = new StdCheatsMock(); vm.expectRevert(); diff --git a/test/StdError.t.sol b/test/StdError.t.sol index a306eaa7..29803d5d 100644 --- a/test/StdError.t.sol +++ b/test/StdError.t.sol @@ -1,8 +1,8 @@ // SPDX-License-Identifier: MIT pragma solidity >=0.8.0 <0.9.0; -import "../src/StdError.sol"; -import "../src/Test.sol"; +import {stdError} from "../src/StdError.sol"; +import {Test} from "../src/Test.sol"; contract StdErrorsTest is Test { ErrorsTest test; @@ -11,52 +11,52 @@ contract StdErrorsTest is Test { test = new ErrorsTest(); } - function test_ExpectAssertion() public { + function test_RevertIf_AssertionError() public { vm.expectRevert(stdError.assertionError); test.assertionError(); } - function test_ExpectArithmetic() public { + function test_RevertIf_ArithmeticError() public { vm.expectRevert(stdError.arithmeticError); test.arithmeticError(10); } - function test_ExpectDiv() public { + function test_RevertIf_DivisionError() public { vm.expectRevert(stdError.divisionError); test.divError(0); } - function test_ExpectMod() public { + function test_RevertIf_ModError() public { vm.expectRevert(stdError.divisionError); test.modError(0); } - function test_ExpectEnum() public { + function test_RevertIf_EnumConversionError() public { vm.expectRevert(stdError.enumConversionError); test.enumConversion(1); } - function test_ExpectEncodeStg() public { + function test_RevertIf_EncodeStgError() public { vm.expectRevert(stdError.encodeStorageError); test.encodeStgError(); } - function test_ExpectPop() public { + function test_RevertIf_PopError() public { vm.expectRevert(stdError.popError); test.pop(); } - function test_ExpectOOB() public { + function test_RevertIf_IndexOOBError() public { vm.expectRevert(stdError.indexOOBError); test.indexOOBError(1); } - function test_ExpectMem() public { + function test_RevertIf_MemOverflowError() public { vm.expectRevert(stdError.memOverflowError); test.mem(); } - function test_ExpectIntern() public { + function test_RevertIf_InternError() public { vm.expectRevert(stdError.zeroVarError); test.intern(); } diff --git a/test/StdJson.t.sol b/test/StdJson.t.sol index e32b92ea..6bedfcc9 100644 --- a/test/StdJson.t.sol +++ b/test/StdJson.t.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity >=0.7.0 <0.9.0; -import "../src/Test.sol"; +import {Test, stdJson} from "../src/Test.sol"; contract StdJsonTest is Test { using stdJson for string; diff --git a/test/StdMath.t.sol b/test/StdMath.t.sol index ed0f9bae..d1269a02 100644 --- a/test/StdMath.t.sol +++ b/test/StdMath.t.sol @@ -1,8 +1,8 @@ // SPDX-License-Identifier: MIT pragma solidity >=0.8.0 <0.9.0; -import "../src/StdMath.sol"; -import "../src/Test.sol"; +import {stdMath} from "../src/StdMath.sol"; +import {Test, stdError} from "../src/Test.sol"; contract StdMathMock is Test { function exposed_percentDelta(uint256 a, uint256 b) public pure returns (uint256) { @@ -52,12 +52,7 @@ contract StdMathTest is Test { } function testFuzz_GetDelta_Uint(uint256 a, uint256 b) external pure { - uint256 manualDelta; - if (a > b) { - manualDelta = a - b; - } else { - manualDelta = b - a; - } + uint256 manualDelta = a > b ? a - b : b - a; uint256 delta = stdMath.delta(a, b); @@ -136,12 +131,7 @@ contract StdMathTest is Test { function testFuzz_GetPercentDelta_Uint(uint192 a, uint192 b) external pure { vm.assume(b != 0); - uint256 manualDelta; - if (a > b) { - manualDelta = a - b; - } else { - manualDelta = b - a; - } + uint256 manualDelta = a > b ? a - b : b - a; uint256 manualPercentDelta = manualDelta * 1e18 / b; uint256 percentDelta = stdMath.percentDelta(a, b); diff --git a/test/StdStorage.t.sol b/test/StdStorage.t.sol index 89984bca..fd79abaa 100644 --- a/test/StdStorage.t.sol +++ b/test/StdStorage.t.sol @@ -1,8 +1,8 @@ // SPDX-License-Identifier: MIT pragma solidity >=0.7.0 <0.9.0; -import "../src/StdStorage.sol"; -import "../src/Test.sol"; +import {stdStorage, StdStorage} from "../src/StdStorage.sol"; +import {Test} from "../src/Test.sol"; contract StdStorageTest is Test { using stdStorage for StdStorage; @@ -230,7 +230,7 @@ contract StdStorageTest is Test { assertEq(val, true); } - function test_StorageReadBool_Revert() public { + function test_RevertIf_ReadingNonBoolValue() public { vm.expectRevert("stdStorage read_bool(StdStorage): Cannot decode. Make sure you are reading a bool."); this.readNonBoolValue(); } @@ -254,7 +254,7 @@ contract StdStorageTest is Test { assertEq(val, type(int256).min); } - function testFuzzPacked(uint256 val, uint8 elemToGet) public { + function testFuzz_Packed(uint256 val, uint8 elemToGet) public { // This function tries an assortment of packed slots, shifts meaning number of elements // that are packed. Shiftsizes are the size of each element, i.e. 8 means a data type that is 8 bits, 16 == 16 bits, etc. // Combined, these determine how a slot is packed. Making it random is too hard to avoid global rejection limit @@ -296,7 +296,7 @@ contract StdStorageTest is Test { } } - function testFuzzPacked2(uint256 nvars, uint256 seed) public { + function testFuzz_Packed2(uint256 nvars, uint256 seed) public { // Number of random variables to generate. nvars = bound(nvars, 1, 20); diff --git a/test/StdStyle.t.sol b/test/StdStyle.t.sol index e12c005f..974e756f 100644 --- a/test/StdStyle.t.sol +++ b/test/StdStyle.t.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity >=0.7.0 <0.9.0; -import "../src/Test.sol"; +import {Test, console2, StdStyle} from "../src/Test.sol"; contract StdStyleTest is Test { function test_StyleColor() public pure { diff --git a/test/StdToml.t.sol b/test/StdToml.t.sol index 631b1b53..5a45f4f5 100644 --- a/test/StdToml.t.sol +++ b/test/StdToml.t.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity >=0.7.0 <0.9.0; -import "../src/Test.sol"; +import {Test, stdToml} from "../src/Test.sol"; contract StdTomlTest is Test { using stdToml for string; diff --git a/test/StdUtils.t.sol b/test/StdUtils.t.sol index 4994c6cb..aee801b2 100644 --- a/test/StdUtils.t.sol +++ b/test/StdUtils.t.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT pragma solidity >=0.7.0 <0.9.0; -import "../src/Test.sol"; +import {Test, StdUtils} from "../src/Test.sol"; contract StdUtilsMock is StdUtils { // We deploy a mock version so we can properly test expected reverts. @@ -57,7 +57,7 @@ contract StdUtilsTest is Test { assertEq(bound(type(uint256).max - 3, 50, 150), 147); } - function test_Bound_DistributionIsEven(uint256 min, uint256 size) public pure { + function testFuzz_Bound_DistributionIsEven(uint256 min, uint256 size) public pure { size = size % 100 + 1; min = bound(min, UINT256_MAX / 2, UINT256_MAX / 2 + size); uint256 max = min + size - 1; @@ -73,7 +73,7 @@ contract StdUtilsTest is Test { } } - function test_Bound(uint256 num, uint256 min, uint256 max) public pure { + function testFuzz_Bound(uint256 num, uint256 min, uint256 max) public pure { if (min > max) (min, max) = (max, min); uint256 result = bound(num, min, max); @@ -89,7 +89,7 @@ contract StdUtilsTest is Test { assertEq(bound(1, type(uint256).max - 1, type(uint256).max), type(uint256).max); } - function test_CannotBoundMaxLessThanMin() public { + function test_RevertIf_BoundMaxLessThanMin() public { // We deploy a mock version so we can properly test the revert. StdUtilsMock stdUtils = new StdUtilsMock(); @@ -97,7 +97,7 @@ contract StdUtilsTest is Test { stdUtils.exposed_bound(uint256(5), 100, 10); } - function test_CannotBoundMaxLessThanMin(uint256 num, uint256 min, uint256 max) public { + function testFuzz_RevertIf_BoundMaxLessThanMin(uint256 num, uint256 min, uint256 max) public { // We deploy a mock version so we can properly test the revert. StdUtilsMock stdUtils = new StdUtilsMock(); @@ -146,7 +146,7 @@ contract StdUtilsTest is Test { assertEq(bound(type(int256).max - 3, -50, -10), -13); } - function test_BoundInt_DistributionIsEven(int256 min, uint256 size) public pure { + function testFuzz_BoundInt_DistributionIsEven(int256 min, uint256 size) public pure { size = size % 100 + 1; min = bound(min, -int256(size / 2), int256(size - size / 2)); int256 max = min + int256(size) - 1; @@ -162,7 +162,7 @@ contract StdUtilsTest is Test { } } - function test_BoundInt(int256 num, int256 min, int256 max) public pure { + function testFuzz_BoundInt(int256 num, int256 min, int256 max) public pure { if (min > max) (min, max) = (max, min); int256 result = bound(num, min, max); @@ -183,7 +183,7 @@ contract StdUtilsTest is Test { assertEq(bound(1, type(int256).min, type(int256).min + 1), type(int256).min + 1); } - function test_CannotBoundIntMaxLessThanMin() public { + function test_RevertIf_BoundIntMaxLessThanMin() public { // We deploy a mock version so we can properly test the revert. StdUtilsMock stdUtils = new StdUtilsMock(); @@ -191,7 +191,7 @@ contract StdUtilsTest is Test { stdUtils.exposed_bound(-5, 100, 10); } - function test_CannotBoundIntMaxLessThanMin(int256 num, int256 min, int256 max) public { + function testFuzz_RevertIf_BoundIntMaxLessThanMin(int256 num, int256 min, int256 max) public { // We deploy a mock version so we can properly test the revert. StdUtilsMock stdUtils = new StdUtilsMock(); @@ -229,7 +229,7 @@ contract StdUtilsTest is Test { assertEq(bytesToUint(millionEther), 1_000_000 ether); } - function test_CannotConvertGT32Bytes() external { + function test_RevertIf_BytesLengthExceeds32() external { // We deploy a mock version so we can properly test the revert. StdUtilsMock stdUtils = new StdUtilsMock(); @@ -289,7 +289,7 @@ contract StdUtilsForkTest is Test { vm.createSelectFork({urlOrAlias: "mainnet", blockNumber: 16_428_900}); } - function test_CannotGetTokenBalances_NonTokenContract() external { + function test_RevertIf_CannotGetTokenBalances_NonTokenContract() external { // We deploy a mock version so we can properly test the revert. StdUtilsMock stdUtils = new StdUtilsMock(); @@ -303,7 +303,7 @@ contract StdUtilsForkTest is Test { stdUtils.exposed_getTokenBalances(token, addresses); } - function test_CannotGetTokenBalances_EOA() external { + function test_RevertIf_CannotGetTokenBalances_EOA() external { // We deploy a mock version so we can properly test the revert. StdUtilsMock stdUtils = new StdUtilsMock();