-
Notifications
You must be signed in to change notification settings - Fork 72
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
new(tests): EOF - EIP-3540: Migrate validation tests: EIP3540/validInvalidFiller.yml #598
Conversation
717bf1f
to
a892469
Compare
a892469
to
1a9c38b
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.
Some comments. I think the end-goal should be to get rid of tests/prague/eip7692_eof_v1/eip3540_eof_v1/container.py
and place the tests it contains into several better categorized tests.
tests/prague/eip7692_eof_v1/eip3540_eof_v1/test_migrated_valid_invalid.py
Outdated
Show resolved
Hide resolved
tests/prague/eip7692_eof_v1/eip3540_eof_v1/test_migrated_valid_invalid.py
Outdated
Show resolved
Hide resolved
tests/prague/eip7692_eof_v1/eip3540_eof_v1/test_migrated_valid_invalid.py
Show resolved
Hide resolved
tests/prague/eip7692_eof_v1/eip3540_eof_v1/test_migrated_valid_invalid.py
Show resolved
Hide resolved
tests/prague/eip7692_eof_v1/eip3540_eof_v1/test_migrated_valid_invalid.py
Show resolved
Hide resolved
tests/prague/eip7692_eof_v1/eip3540_eof_v1/test_migrated_valid_invalid.py
Outdated
Show resolved
Hide resolved
tests/prague/eip7692_eof_v1/eip3540_eof_v1/test_migrated_valid_invalid.py
Outdated
Show resolved
Hide resolved
tests/prague/eip7692_eof_v1/eip3540_eof_v1/test_migrated_valid_invalid.py
Outdated
Show resolved
Hide resolved
tests/prague/eip7692_eof_v1/eip3540_eof_v1/test_migrated_valid_invalid.py
Outdated
Show resolved
Hide resolved
tests/prague/eip7692_eof_v1/eip3540_eof_v1/test_migrated_valid_invalid.py
Outdated
Show resolved
Hide resolved
tests/prague/eip7692_eof_v1/eip3540_eof_v1/test_migrated_valid_invalid.py
Show resolved
Hide resolved
1864f0f
to
b5359a2
Compare
I put it on hold because this is too fragile: it requires evmone, ethereum/tests and EEST to full agree. |
1bc4bd1
to
342ab44
Compare
4331a25
to
9f3b678
Compare
Because the coverage comparison only considers the newly added python file removing duplicated tests is impractical. I'm not sure how do you want to handle this. Moreover, the coverage report also points to the lost coverage in the "bytes to hex" helper functions. I don't know why exactly this happens but these are irrelevant. Quick fix is probably to exclude EVMC from the coverage. |
9b0fc42
to
29bd9b1
Compare
Should I push these tests further? |
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!
29bd9b1
to
cccb738
Compare
- EOFTests/EIP3540/validInvalid.json: ethereum/execution-spec-tests#598 - EOFTests/efValidation/EOF1_truncated_section_.json: ethereum/execution-spec-tests#740 - EOFTests/efValidation/EOF1_eofcreate_valid_.json: ethereum/execution-spec-tests#738
- EOFTests/EIP3540/validInvalid.json: ethereum/execution-spec-tests#598 - EOFTests/efValidation/EOF1_truncated_section_.json: ethereum/execution-spec-tests#740 - EOFTests/efValidation/EOF1_eofcreate_valid_.json: ethereum/execution-spec-tests#738
🗒️ Description
Migrate https://github.com/ethereum/tests/blob/develop/src/EOFTestsFiller/EIP3540/validInvalidFiller.yml.
✅ Checklist
mkdocs serve
locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.