-
Notifications
You must be signed in to change notification settings - Fork 74
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(tests): eof section order tests #592
Conversation
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 comments, thanks!
tests/prague/eip7692_eof_v1/eip3540_eof_v1/test_section_order.py
Outdated
Show resolved
Hide resolved
Another comment is that, since there are some repeated cases with the current parametrization approach, and we are actually listing each case in @pytest.mark.parametrize("section_kind,section_test,test_position,expected_bytecode,expected_exception",
[
pytest.param(
SectionKind.TYPE,
SectionTest.MISSING,
CasePosition.HEADER,
"ef000102000100030400010000800001305000ef",
EOFException.MISSING_TYPE_HEADER,
), |
hm I was thinking what if we are to add more combinations. |
Code verification is totally fine, I like that we are not blindly trusting our EOF bytecode constructor. What I like about this second approach is that it's easier to read for someone that comes into the file and does not have that much knowledge about how parametrization combinations work, even if we add more cases, I think it's a bit better to be explicit IMO. |
what happend. now you prove me that the way json fillers expect sections were organized is better, and I prove you that python scripting is superior ) |
Lol, I guess I just felt it was confusing in this context, because we still need a switch-case implemented to make it work. I think having combinations handled by parametrize is still possible, it's just that we need to figure out the logic to fill the exceptions maybe? |
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.
Thanks!
🗒️ Description
Construct eof container with skipped sections combinations
Example:
🔗 Related Issues
#550
✅ Checklist
mkdocs serve
locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.