🕵️ Better sad path testing with Foundry cheatcode

[neodaoist]

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");
}

[transmissions11]

glad you opened an issue before jumping straight into a PR, while this is a great change I believe these cases should already be covered in @abigger87's PR #214? lmk if he's missing something and we can upstream into there if that's cool w/ you!

[neodaoist]

Okay totally. I missed that PR.

Sounds good, I will check it out in more depth and upstream into there, looks like there's at least a few test cases I could contribute.

Nice work @abigger87! 🙌