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.

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

feat(cheatcodes): disallow usage of expectRevert with expectCall and expectEmit #5144

Merged
merged 3 commits into from
Jun 12, 2023

Conversation

Evalir
Copy link
Member

@Evalir Evalir commented Jun 12, 2023

Motivation

Closes #5138, but will go into the 1.0 branch.

When a function reverts, it's like it never happened as it gets rolled back—but we're still matching calls/events.

Solution

This completely disallows matching calls or events if the user has previously used expectRevert. This restriction will be in place until the expectRevert is "filled", meaning until after the next call, by which either the test will fail due to a non-reverting call, or end execution as it effectively reverted.

@Evalir Evalir requested a review from mattsse June 12, 2023 17:01
Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This completely disallows matching calls or events if the user has previously used expectRevert

sgtm,
tests look good

@@ -45,6 +45,16 @@ fn expect_revert(state: &mut Cheatcodes, reason: Option<Bytes>, depth: u64) -> R
Ok(Bytes::new())
}

fn expect_emit(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like having this as standalone fn

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeh my objective over time is to make handling cheatcodes a bit more state-machine like, with the ability to compose error messages and add hints and whatnot. This is a small part of it (breaking everything down into functions instead of inlining everything)

@Evalir Evalir merged commit fcbf1d4 into expect-changes Jun 12, 2023
@Evalir Evalir deleted the evalir/disallow-reverts-expects branch June 12, 2023 17:56
Evalir added a commit that referenced this pull request Jun 26, 2023
* feat(`cheatcodes`): Make expectCall only work for the next call's subcalls (#5032)

* chore: make expect call only work for the next call

* chore: make expectCall actually check only the next call's subcalls

* chore: fmt

* chore: introduce checks at the main call level, not at the subcall level

* chore: handle dangling expected calls gracefully

* chore: fix tests

* chore: fmt

* chore: forge fmt

* chore: actually exclude depth the cheatcode was called from

* chore: tests

* chore: better docs

* chore: comment out impossible to check condition on expectCall

* chore: remove unused check

* fix(cheatcodes): Correct `expectRevert` behavior (#4945)

* chore: add repro test to pass

* chore: strictly check for the depth expectRevert was called in, instead of being able to peek at function end

* chore: tests

* chore: add more repro tests

* chore: fmt

* chore: clippy

* chore: fixup problematic tests, mark them as not working properly

* chore: forge fmt

* chore: forge fmt

* Update evm/src/executor/inspector/cheatcodes/mod.rs

* chore: add more info to changelog

* chore: fmt

* chore(tests): add more cases for `expectEmit` (#5076)

* chore(tests): add more extreme cases for expectEmit

* chore(tests): add next call fail case for expectEmit

* chore(`cheatcodes`): add more edge case tests on `expect*` cheatcodes (#5135)

* chore: add edge-cases

* chore: add edge case covering #4920 (comment)

* feat(`cheatcodes`): disallow usage of `expectRevert` with `expectCall` and `expectEmit`  (#5144)

* feat(cheatcodes): disallow usage of expectCall/Emit with expectRevert

* chore: add tests

* chore: fmt

* chore: fmt
Evalir added a commit that referenced this pull request Jul 10, 2023
* feat(`cheatcodes`): `1.0` cheatcode changes (#5045)

* feat(`cheatcodes`): Make expectCall only work for the next call's subcalls (#5032)

* chore: make expect call only work for the next call

* chore: make expectCall actually check only the next call's subcalls

* chore: fmt

* chore: introduce checks at the main call level, not at the subcall level

* chore: handle dangling expected calls gracefully

* chore: fix tests

* chore: fmt

* chore: forge fmt

* chore: actually exclude depth the cheatcode was called from

* chore: tests

* chore: better docs

* chore: comment out impossible to check condition on expectCall

* chore: remove unused check

* fix(cheatcodes): Correct `expectRevert` behavior (#4945)

* chore: add repro test to pass

* chore: strictly check for the depth expectRevert was called in, instead of being able to peek at function end

* chore: tests

* chore: add more repro tests

* chore: fmt

* chore: clippy

* chore: fixup problematic tests, mark them as not working properly

* chore: forge fmt

* chore: forge fmt

* Update evm/src/executor/inspector/cheatcodes/mod.rs

* chore: add more info to changelog

* chore: fmt

* chore(tests): add more cases for `expectEmit` (#5076)

* chore(tests): add more extreme cases for expectEmit

* chore(tests): add next call fail case for expectEmit

* chore(`cheatcodes`): add more edge case tests on `expect*` cheatcodes (#5135)

* chore: add edge-cases

* chore: add edge case covering #4920 (comment)

* feat(`cheatcodes`): disallow usage of `expectRevert` with `expectCall` and `expectEmit`  (#5144)

* feat(cheatcodes): disallow usage of expectCall/Emit with expectRevert

* chore: add tests

* chore: fmt

* chore: fmt

* `foundryup`: v1 changes (#5158)

* feat(foundryup): look for v1 tag instead of nightly for normal foundryup

* feat(foundryup): add ability to download legacy nightly binary with -L flag

* feat: use latest release for figuring out the tag name

* chore(foundryup): slightly improve stable release detection

* chore: use proper repo

* make fns async

* chore: remove prb math from integration tests

* chore: forge fmt

* chore: fix some merge leftovers

* chore: last test fixes

* chore: forge fmt

* chore: uncomment etch test

* feat(docs): add `RELEASE_PROCESS.md` (#5269)

* feat(docs): add RELEASE_PROCESS.md

* chore: not include changelog changes in step

* chore: bump crates to 1.0.0 (#5346)
Evalir added a commit that referenced this pull request Jul 10, 2023
* feat(`cheatcodes`): `1.0` cheatcode changes (#5045)

* feat(`cheatcodes`): Make expectCall only work for the next call's subcalls (#5032)

* chore: make expect call only work for the next call

* chore: make expectCall actually check only the next call's subcalls

* chore: fmt

* chore: introduce checks at the main call level, not at the subcall level

* chore: handle dangling expected calls gracefully

* chore: fix tests

* chore: fmt

* chore: forge fmt

* chore: actually exclude depth the cheatcode was called from

* chore: tests

* chore: better docs

* chore: comment out impossible to check condition on expectCall

* chore: remove unused check

* fix(cheatcodes): Correct `expectRevert` behavior (#4945)

* chore: add repro test to pass

* chore: strictly check for the depth expectRevert was called in, instead of being able to peek at function end

* chore: tests

* chore: add more repro tests

* chore: fmt

* chore: clippy

* chore: fixup problematic tests, mark them as not working properly

* chore: forge fmt

* chore: forge fmt

* Update evm/src/executor/inspector/cheatcodes/mod.rs

* chore: add more info to changelog

* chore: fmt

* chore(tests): add more cases for `expectEmit` (#5076)

* chore(tests): add more extreme cases for expectEmit

* chore(tests): add next call fail case for expectEmit

* chore(`cheatcodes`): add more edge case tests on `expect*` cheatcodes (#5135)

* chore: add edge-cases

* chore: add edge case covering #4920 (comment)

* feat(`cheatcodes`): disallow usage of `expectRevert` with `expectCall` and `expectEmit`  (#5144)

* feat(cheatcodes): disallow usage of expectCall/Emit with expectRevert

* chore: add tests

* chore: fmt

* chore: fmt

* `foundryup`: v1 changes (#5158)

* feat(foundryup): look for v1 tag instead of nightly for normal foundryup

* feat(foundryup): add ability to download legacy nightly binary with -L flag

* feat: use latest release for figuring out the tag name

* chore(foundryup): slightly improve stable release detection

* chore: use proper repo

* make fns async

* chore: remove prb math from integration tests

* chore: forge fmt

* chore: fix some merge leftovers

* chore: last test fixes

* chore: forge fmt

* chore: uncomment etch test

* feat(docs): add `RELEASE_PROCESS.md` (#5269)

* feat(docs): add RELEASE_PROCESS.md

* chore: not include changelog changes in step

* chore: bump crates to 1.0.0 (#5346)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants