-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
MTCannon: add EmptyThreadStack test #12082
base: develop
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #12082 +/- ##
===========================================
- Coverage 79.10% 78.87% -0.24%
===========================================
Files 41 41
Lines 3437 3437
===========================================
- Hits 2719 2711 -8
- Misses 548 557 +9
+ Partials 170 169 -1
Flags with carried forward coverage won't be shown. Click here to find out more. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@joohhnnn Thanks for working on this.
We also want to assert that the EVM (MIPS2.sol) reverts when the stack is empty. You can use AssertEVMReverts
to do this as it checks for EVM reverts in MIPS2.sol.
} else { | ||
require.NotPanics(t, func() { | ||
goVm.Step(true) | ||
testutil.AssertEVMReverts(t, state, contracts, tracer) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the case where shouldPanic == false
, you should use testutil.ValidateEVM
as is done here. AssertEVMReverts
doesn't actually panic. But rather runs a require
assertion against the EVM execution status.
I've added tests for the EmptyThreadStack scenario based on Issue 11977. I'm using
defer
and strings for checking because I didn't see panic handling inAssertEVMReverts
, and I'm unsure if modifying AssertEVMReverts directly is appropriate.