🕵️ Better sad path testing with Foundry cheatcode #273
Description
Context
We could exercise sad path test cases with more specificity using the Foundry cheatcode expectRevert.
This would improve test case readability, make the overall test suite more deterministic, and potentially catch future regressions.
Proposed changes
I converted ~60 sad path tests in the token standards (ERC20, ERC721, ERC1155) from testFail... to use expectRevert(). I also appended these test case names with "ShouldFail", but this was just a stylistic choice.
Still learning, so before implementing this pattern in the fuzz tests and submitting a PR, wanted to check if it makes sense and would be a valuable addition. If it looks good, I will keep going.
LMK, thanks! 💙
Few example conversions in ERC721.t.sol
1) Testing mint to zero address
Original
function testFailMintToZero() public {
token.mint(address(0), 1337);
}Converted
function testMintToZeroShouldFail() public {
hevm.expectRevert("INVALID_RECIPIENT");
token.mint(address(0), 1337);
}2) Testing double mint
Original
function testFailDoubleMint() public {
token.mint(address(0xBEEF), 1337);
token.mint(address(0xBEEF), 1337);
}Converted
function testDoubleMintShouldFail() public {
token.mint(address(0xBEEF), 1337);
hevm.expectRevert("ALREADY_MINTED");
token.mint(address(0xBEEF), 1337);
}3) Testing safeTransferFrom to NonERC721Recipient with data
Original
function testFailSafeTransferFromToNonERC721RecipientWithData() public {
token.mint(address(this), 1337);
token.safeTransferFrom(address(this), address(new NonERC721Recipient()), 1337, "testing 123");
}Converted
function testSafeTransferFromToNonERC721RecipientWithDataShouldFail() public {
token.mint(address(this), 1337);
address to = address(new NonERC721Recipient());
hevm.expectRevert();
token.safeTransferFrom(address(this), to, 1337, "testing 123");
}