Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(cheatcodes): add ability to exclude certain custom errors and revert reason strings from failing tests #4271

Open
PaulRBerg opened this issue Feb 4, 2023 · 11 comments · May be fixed by #9179
Open

feat(cheatcodes): add ability to exclude certain custom errors and revert reason strings from failing tests #4271

PaulRBerg opened this issue Feb 4, 2023 · 11 comments · May be fixed by #9179
Assignees
@emo-eth
Labels
A-cheatcodes Area: cheatcodes A-testing Area: testing C-forge Command: forge P-low Priority: low T-feature Type: feature
Milestone
v1.0.0

Comments

@PaulRBerg
Copy link
Contributor

PaulRBerg commented Feb 4, 2023 •
edited
Loading

Component

Forge

Describe the feature you would like

The use case is fork + fuzz tests - certain tokens like USDC have blacklists that prevent the transfer of assets between certain accounts (see lines 837 and 838 here). When Foundry fuzzes a blacklisted account, the Forge test will fail because of the following error:

Blacklistable: account is blacklisted

However, this is a false negative. The user's test should not have failed.

It would be great if there were two config options for filtering revert reason strings and custom errors.
@PaulRBerg PaulRBerg added the T-feature Type: feature label Feb 4, 2023
@gakonst gakonst added this to Foundry Feb 4, 2023
@github-project-automation github-project-automation bot moved this to Todo in Foundry Feb 4, 2023
@PaulRBerg PaulRBerg mentioned this issue Feb 4, 2023
Closed
@0xPhaze
Copy link

0xPhaze commented Feb 5, 2023 •
edited
Loading

Related:

@zerosnacks
Copy link
Member

For the time being, PSA: forge-std has a number of modifiers to help filter out these specific cases (I think added by OP): https://github.com/foundry-rs/forge-std/blob/75b3fcf052cc7886327e4c2eac3d1a1f36942b41/src/StdCheats.sol#L208-L278

The general request is still relevant so leaving the ticket open

@zerosnacks zerosnacks added A-testing Area: testing C-forge Command: forge A-cheatcodes Area: cheatcodes labels Jun 28, 2024
@zerosnacks zerosnacks added this to the v1.0.0 milestone Jul 26, 2024
@grandizzy grandizzy mentioned this issue Oct 15, 2024
Open
@grandizzy grandizzy added the P-low Priority: low label Oct 15, 2024
@zerosnacks zerosnacks changed the title feat(forge): ability to exclude certain custom errors and revert reason strings from failing tests feat(cheatcodes): add ability to exclude certain custom errors and revert reason strings from failing tests Oct 15, 2024
@emo-eth
Copy link
Contributor

emo-eth commented Oct 21, 2024

Interested in take a stab at this!

@jenpaff
Copy link
Collaborator

jenpaff commented Oct 21, 2024

Interested in take a stab at this!

assigned!

@emo-eth
Copy link
Contributor

emo-eth commented Oct 23, 2024

@PaulRBerg have you run into the scenario where you might want to toss-out multiple kinds of reverts for the same external call (while surfacing others)? Considering adding that functionality.

@PaulRBerg
Copy link
Contributor Author

@emo-eth I have. I think this is what I was trying to get at with the OP — reverts due to USDC blacklisted addresses should be ignored.

@emo-eth
Copy link
Contributor

emo-eth commented Oct 23, 2024

@PaulRBerg Sorry, to be more specific, I mean multiple kinds of errors with different data, to cover a set of anticipated & acceptable revert reasons that might arise from one specific call. Think I have that working either way, but curious if you've run into that scenario.

@PaulRBerg
Copy link
Contributor Author

I don't think I have.

@emo-eth emo-eth linked a pull request Oct 23, 2024 that will close this issue
Open
@grandizzy
Copy link
Collaborator

grandizzy commented Nov 6, 2024 •
edited
Loading

@PaulRBerg @mds1 as proposed in PR #9179 here are the cheatcodes planned to be added, mind to review / share thoughts? thanks

    /// Discard this run's fuzz inputs and generate new ones if next call reverted with data starting with the given revert data. Call more than once to add multiple reasons.
    #[cheatcode(group = Testing, safety = Safe)]
    function assumeNoRevert(bytes4 revertData) external pure;

    /// Discard this run's fuzz inputs and generate new ones if next call reverted with the given revert data. Call more than once to add multiple reasons.
    #[cheatcode(group = Testing, safety = Safe)]
    function assumeNoRevert(bytes calldata revertData) external pure;

    /// Discard this run's fuzz inputs and generate new ones if next call reverted with data starting with the given revert data. Call more than once to add multiple reasons.
    #[cheatcode(group = Testing, safety = Safe)]
    function assumeNoRevert(bytes4 revertData, address reverter) external pure;

    /// Discard this run's fuzz inputs and generate new ones if next call reverted with the given revert data. Call more than once to add multiple reasons.
    #[cheatcode(group = Testing, safety = Safe)]
    function assumeNoRevert(bytes calldata revertData, address reverter) external pure;

@PaulRBerg
Copy link
Contributor Author

PaulRBerg commented Nov 6, 2024 •
edited
Loading

looks good for me, tagging for @andreivladbrg and @smol-ninja for additional feedback

@smol-ninja
Copy link

Looks good to me as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@emo-eth emo-eth

Labels
A-cheatcodes Area: cheatcodes A-testing Area: testing C-forge Command: forge P-low Priority: low T-feature Type: feature
Projects
Foundry
Status: Todo
Milestone
v1.0.0
7 participants
@jenpaff @emo-eth @smol-ninja @PaulRBerg @grandizzy @zerosnacks @0xPhaze