-
Notifications
You must be signed in to change notification settings - Fork 107
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
π docs: A guide to porting tests #1165
base: main
Are you sure you want to change the base?
Conversation
docs/dev/porting_legacy_tests.md
Outdated
|
||
## Background | ||
|
||
EEST is the successor to [ethereum/tests](https://github.com/ethereum/tests) (aka "legacy tests"), a repository that defined EVM test cases from the [Frontier](https://ethereum.org/en/history/#frontier) phase up to and including [The Merge](https://ethereum.org/en/history/#paris). These test cases are specified as YAML (and occasionally JSON) files in the [`./src/`](https://github.com/ethereum/tests/tree/develop/src) sub-directory. JSON test fixtures, which are fully-populated tests that can be executed against clients, are generated using [ethereum/retesteth](https://github.com/ethereum/retesteth). These JSON artifacts are regenerated when needed and added to the repository, typically in the [`./GeneralStateTests/`](https://github.com/ethereum/tests/tree/develop/GeneralStateTests) sub-directory. |
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.
confusion here as legacy tests are in ethereum/legacytests. (actual tests from Frontier to Cancun)
in ethereum/tests are the most recent version of test vectors currently being filled for cancun and possibly prague.
it is actually mostly json fillers than yaml.
docs/dev/porting_legacy_tests.md
Outdated
|
||
!!! note "Expected coverage loss" | ||
|
||
EEST uses [pytest](https://docs.pytest.org/en/stable/), a Python testing library, for testing Ethereum specifications, while legacy tests relied on the EVM to handle much of the execution. This difference can lead to coverage gaps. EEST favors dynamic contract creation for each test vector, while legacy tests preferred a single static contract with multiple test vectors determined by transaction input data. |
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.
as a note here that yul tests or some tests used calldataload that might no longer needed when designing a test with python. but we should always investigate if an opcode is not covered anymore to see if its ok
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.
Added clarification comments
I have address the comments @winsvega! PTAL @danceratopz. Thanks. |
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.
This is very helpful, thank you very much for this addition @raxhvl!
I think it should be in more prominent than the "dev" docs though, which mainly target EEST core devs. What do you think about moving it from
docs/dev/porting_legacy_tests.md
to docs/writing_tests/porting_ethereum_tests.md
If so, please add a menu item to docs/navigation.md
, so it shows up in the menu: I think it can go after the tutorial here, at the bottom of this list?
docs/dev/porting_legacy_tests.md
Outdated
3. Submit a PR with the ported tests: | ||
|
||
1. Add the list of ported YAML files to [`converted-ethereum-tests.txt`](https://github.com/ethereum/execution-spec-tests/blob/1b30c336eae6b0746ea4db441ac74406f2fb2322/converted-ethereum-tests.txt). | ||
2. Open a PR to remove the ported tests from the _original tests_ repository. |
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.
Additionally, I think we can write and link to ethereum/tests
here instead of referring to "original tests".
|
||
## Debugging tests | ||
|
||
By default, EVM logs are stored in the `logs` folder at the repository root. You can check the `output` folder to review transaction results. If needed, review a previous PR that ported tests (e.g., [the PR porting the `PUSH` opcode](https://github.com/ethereum/execution-spec-tests/pull/975)). |
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 just added a new label port
for ported tests and labelled the ones I could find. Could you add a link to that so people can find related PRs easily?
https://github.com/ethereum/execution-spec-tests/pulls?q=is%3Apr+label%3Aport
But do please leave a link to this as an example (which is not tooo long or complicated).
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.
If you think the label should be more descriptive, let me know.
|
||
By default, EVM logs are stored in the `logs` folder at the repository root. You can check the `output` folder to review transaction results. If needed, review a previous PR that ported tests (e.g., [the PR porting the `PUSH` opcode](https://github.com/ethereum/execution-spec-tests/pull/975)). | ||
|
||
## Test coverage |
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.
Extremely helpful section, thank uuuuuu!
Co-authored-by: danceratopz <[email protected]>
Co-authored-by: danceratopz <[email protected]>
Co-authored-by: danceratopz <[email protected]>
ποΈ Description
Everything I know about porting tests.
π Related Issues
Closes #1153
β Checklist
mkdocs serve
locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.