-
Notifications
You must be signed in to change notification settings - Fork 90
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
improvement: Refactor of spec for blockchain & state test generation #307
Conversation
8048044
to
4d3a153
Compare
9ce1538
to
a33ef95
Compare
a035e3d
to
574ccab
Compare
4fbafe2
to
a760464
Compare
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.
Nice, looks cleaner to me!
Nice to see engine
/ "NoProof" removed from the pytest interface... that seemed unnecessary back when we did the pytest port.
578a211
to
52f8514
Compare
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.
I really like the changes, I'd like to diff the fixtures against the ones currently generated on main, just to double check we are not breaking anything :)
I diff'd 52f8514.
diff '--color=auto' -r fixtures-geth-main-hive-ordered/shanghai/eip4895_withdrawals/withdrawals/use_value_in_tx.json fixtures-refactor-spec-hive-ordered/shanghai/eip4895_withdrawals/withdrawals/use_value_in_tx.json
7a8
> "engineFcuVersion": "2",
187a189
> "engineFcuVersion": "3", |
This undoes any changes in the fixture names used in JSON fixture files.
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!
I decided not to use nodeid
for fixture names in JSON within the scope of this PR, and created a ticket instead, see:
- The comment in this PR for more info: improvement: Refactor of spec for blockchain & state test generation #307 (comment)
- Follow-up issue: feat(fw): use pytest nodeids as test names in json fixtures #329
🗒️ Description
The refactor proposal is to use
make_fixture()
andmake_hive_fixture()
withinfill.py
, to abstract all the filling logic to bothstate_test.py
andblockchain_test.py
. These additions should make it easier to add the correct StateTest fixture format in the future.🔗 Related Issues
This is a follow up PR to address some comments on #301.
Resolves the following issue #43.
✅ Checklist
mkdocs serve
locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.