-
Notifications
You must be signed in to change notification settings - Fork 186
feat(consume): consume direct using evmone-state and blockchaintest
#2243
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
Conversation
961539a to
1819482
Compare
spencer-tb
left a comment
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 from my side! Thanks for adding this. Feel free to covert to ready for review :)
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.
@pdobacz wow, this is amazing!
consume direct --bin=~/bin/evmone-statetest [email protected] -v -m state_test -n 8
evmonehas two things which make things harder. One is that it has two separate binaries:evmone-statetestandevmone-blockchaintest, rather than a single one with 2 subcommands, as EEST assumes. Decision we need to make where do we adjust - on EEST end or onevmoneend.
You can specify two binaries as --bin=evmone-statetest --bin=evmone-blockchaintest, then configure the EvmoneFixtureConsumer appropriately so that it picks the relevant binary based on binary version output and fixture type.
This works right now, for example:
consume direct --bin=~/bin/evmone-statetest --bin=~/bin/evm [email protected] -v -m "state_test and Prague" -k test_chainid
Edit: Just to clarify, this parametrizes each test by Geths' evm and evmone-statetest:

I don't think we should merge this as-is. It fails by default if you provide a blockchain_test fixture. Instead, the test case should rather be skipped - then I'd be ok to merge!
|
Makes sense, will do! My initial plan for the EEST modification was to make it have some "leeway" in the way one specifies the binary. I.e. when Not sure how the dual I'll expand this PR soon, but I suppose the "granularity" issue (3.) can be fixed in a followup? |
I think the architecture should be flexible enough as-is. The The
This allows us to define two FixtureConsumer classes for evmone:
Then you use the appropriate class in
Yes, I wouldn't let 3. block this PR! Thanks for adding this! |
I agree on this point, I think this should be the way to go IMO. If we implement that, and then doing Thanks for implementing this @pdobacz ! |
|
Okay, this approach works up to this point: You'll notice the |
1819482 to
b0b01dc
Compare
b0b01dc to
333878a
Compare
consume direct using evmone-statetestconsume direct using evmone-state and blockchaintest
333878a to
99d1aad
Compare
|
blockchaintest done with the caveat as mentioned above, ready to review. |
marioevz
left a comment
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, amazing work, thanks!

🗒️ Description
First step towards supporting
consume directwithevmone.Remarks:
state_testsupport to get the ball rollingevmonehas two things which make things harder. One is that it has two separate binaries:evmone-statetestandevmone-blockchaintest, rather than a single one with 2 subcommands, as EEST assumes. Decision we need to make where do we adjust - on EEST end or onevmoneend. Gut tells me it is more elegant to adjustevmone, but overall easier to do so in EEST (assuming we can cut a corner and let EEST choose state/blockchain binary on the fly as it picks up fixtures). EDIT: using evmone-blockchaintest is handled in this PR 👇evmonetests on the granularity level of a singlejsonfile. This means a failed test case fails the entire file, making it a bit harder to figure out where the failure is (failure message provides a good hint, but is pretty messy). Also test filtering is very coarse. Something we need to tackle onevmoneend as we progress.Opening as draft - let me know if OK to proceed with such first step into
main.🔗 Related Issues or PRs
N/A.
✅ Checklist
toxchecks to avoid unnecessary CI fails, see also Code Standards and Enabling Pre-commit Checks:uvx --with=tox-uv tox -e lint,typecheck,spellcheck,markdownlinttype(scope):.