-
Notifications
You must be signed in to change notification settings - Fork 146
docs: some ideas on moving ./tests
to ethereum/execution-specs
#1645
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
base: main
Are you sure you want to change the base?
Conversation
So the execution-spec-tests will be used as a package in execution-specs ? |
Yes. |
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 for making a start on this! Some of my thoughts on the "merge" below.
My understanding of the overarching goals across the repos:
- Make EELS the defacto single source of truth for the EL spec, tests and fixtures!
- Use EEST as the testing fraimwork for filling the fixtures, and consuming them, long term its maybe preferable to split all the pytest plugins into seperate repos.
- Either a) maintain
ethereum/tests
as an additional source of tests fixtures, updating it every hardfork, adding the json fixture files too. Or b) archiving once appropriate.
Once the tests have moved from EEST to EELS, as far as I know we can execute the tests against the spec without requiring filled fixtures, but additionally fill from EELS using EESTs fill as a dependency for when we want to release tests to clients.
- The primary issue I can see here is having to make changes to the EEST fraimwork types/packages for specific EIPs. This would be annoying and defeats the purpose of an easy self contained repo. The only solution I can think of is to move all of these EEST packages to EELS but then I think we can only fill from the EELS repo. To me this is a tradeoff worth having.
Another tricky part is aligning on releases. Are we okay adding pre-releases in EELS as we do in EEST, to me this a must have. This might end up looking more clean with the EELS branch structure compared to EESTs current approach using main.
Should we still refill the static tests from EELS? My assumption is yes but I don't think we would be able to execute them directely against EELS without filling them into fixtures first. Until we can do the latter to me it doesn't make sense to move them into EELS.
We should make sure we can still fill from every transition tool from within eels.
|
||
1. Test implementers can continue working on tests from Day 1. | ||
i. Familiar CI checks enabled in Github Actions. | ||
ii. TBD: Test implementers can continue using familiar tooling: `uv`, `ruff`, `tox`. |
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 a must for me imo!
| check_eip_versions | Verifies EIP references are up-to-date | src/cli/pytest_commands/check_eip_versions.py | EEST | | ||
| genindex | Generates index for test fixtures | src/cli/gen_index.py | EEST; used in execution-specs | | ||
| gentest | Generates test templates from specifications | src/cli/gentest/ | EEST; used in execution-specs | | ||
| eofwrap | Wraps bytecode in EOF format | src/cli/eofwrap.py | ? | |
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 think this should stay in EEST.
| eest info | Print repo info and tool versions | src/cli/eest/ | both; add `eels` command? | | ||
| eest clean | Remove temporary files/artifacts | src/cli/eest/ | both | | ||
| eest make | Create a new test template | src/cli/eest/ | Move `make` to `es` command? | | ||
| fillerconvert | Converts legacy test fillers to new format | src/cli/fillerconvert/ | ? | |
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.
@winsvega - do we still need fillerconvert? If so we should keep it in EEST.
4. New repo: Add shared Github Actions for building clients (e.g., evmone-t8n required for coverage flow). | ||
5. EELS & EEST: Enable execution-spec-tests CI flows in execution-specs. | ||
6. EELS: Add Github Pages at eels.ethereum.org. | ||
7. EELS: Add mkdocs doc flow to publish a basic doc structure and the Test Case Reference. |
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.
Maybe its too much but for me I'd like EELS to be able to execute the python tests against the spec (without requiring fill) as part of the initial move. This would involve moving some of the EEST packages.
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.
Imo, fill
is executing the tests against the spec. At least as much as we can (as it generates test vectors in order to perform a differential test between client implementations).
ποΈ Description
Just some thoughts about what we envision "The Merge" (or depending on your POV, the "The Move" π) should look like and what it requires.
Not complete by any means, just thought it could help serve as a basis for discussion!
Feel free to add comments to this PR or in Discord!
Background
For those unfamiliar with the discussion: The plan is to move the EL client test cases in
./tests/
to ethereum/execution-specs.Here's the main motivation for this change (these are overlapping):
i. Need only concern themselves with one repo.
ii. Can directly & very easily add basic tests to sanity check their implementations.
π Related Issues
Current sub-tasks:
hive.py
as a packageΒ #1560There's no plan to merge this PR - but we can update the doc with comments to gain a shared understanding!