-
Notifications
You must be signed in to change notification settings - Fork 20.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
cmd/evm: don't reuse state between iterations, show errors #30780
Conversation
…enchmark errors are captured and printed to output.
Co-authored-by: Martin HS <[email protected]>
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.
Some nits
Co-authored-by: Martin HS <[email protected]>
Co-authored-by: Martin HS <[email protected]>
Co-authored-by: Martin HS <[email protected]>
Co-authored-by: Martin HS <[email protected]>
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.
LGTM
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.
Even though you worked around the panic due to b.Fatalf
, I think it would still be more "correct" to also invoke testing.Init()
when you enter the if bench
clause.
Reusing state between benchmark iterations resulted in inconsistent results across runs, which surfaced in #30778 .
If these errors are triggered again, they should be printed to output. To ensure that the code invoking
testing.Benchmark()
can catch and output these errors, and then exit: I replace calls tot.B.Fatalf
with setting an error which is consumed by the calling code.The same consistency checks should probably be applied to the state test benchmarker. I am figuring that out now.
closes #30778 .