feat(forge) _expectRevertCheatcode
#6841
Conversation
|
Failing CI is triggered by one of the tests being false-positive earlier and failing on Windows now, not sure how to fix such things :/ |
There was a problem hiding this comment.
given that this behaviour is somewhat required to move forward with converting forge-std checks to rust native cheatcodes, I think we should proceed with this.
having an additional cheatcode for internal use makes sense to me, but we probably don't want to expose this via the forge-std interface. Perhaps there's an easy way to exclude them when we generate the interface, maybe with an additional group (Internal?)
wdyt @DaniPopes
|
@Evalir agreed that pending guidance on whether we want |
crates/cheatcodes/spec/src/vm.rs
Outdated
| @@ -680,6 +680,18 @@ interface Vm { | |||
| #[cheatcode(group = Testing, safety = Unsafe)] | |||
| function expectRevert(bytes calldata revertData) external; | |||
|
|
|||
| /// Expects an error on next cheatcode call with any revert data. | |||
| #[cheatcode(group = Testing, safety = Unsafe, status = Experimental)] | |||
There was a problem hiding this comment.
yeah I think adding an Internal variant would be appropriate
| @@ -23,10 +23,10 @@ contract FsTest is DSTest { | |||
|
|
|||
| assertEq(vm.readFile(path), "hello readable world\nthis is the second line!"); | |||
|
|
|||
| vm.expectRevert(FOUNDRY_READ_ERR); | |||
| vm._expectCheatcodeRevert(FOUNDRY_READ_ERR); | |||
There was a problem hiding this comment.
can we still use expectRevert here, or would this no longer be possible?
There was a problem hiding this comment.
We can, but the test will exit and pass right after the reverted call
That is the current behavior which resulted in some false positives
This PR will not affect existing |
|
pending @Evalir @DaniPopes |
There was a problem hiding this comment.
this lgtm, so pending @DaniPopes for any nits—ideally we'd def figure out a way to not include this in the vm interface, but we can do so later
forge-std script to create Vm.sol interface already excludes |
Motivation
Ref #6833
Considering that we need a way to write negative tests for cheatcodes internally but making existing
expectRevertcheats interact with cheatcodes as well would break a lot of thing and not really a desired feature, this PR adds_expectRevertCheatcodecheats for internal usage.Solution
ExpectedRevertcontext to allow 2 kinds of expectations -Default(expects revert from next non-cheatcode call) andCheatcode(expects revert from next cheatcode call)Experimental._expectRevertCheatcodeinstead ofexpectRevertThe logic with
pendingbool might be a little bit confusing, so maybe there is a way to improve it which I haven't figured out yet. I tried to document it clearly