feat(fill,tests): add all static `GeneralStateTestsFiller` from `ethereum/tests` by winsvega · Pull Request #1442 · ethereum/execution-spec-tests

winsvega · 2025-04-11T13:39:56Z

🗒️ Description

All ethereum/tests state test fillers

🔗 Related Issues

To rebase on:
#1439

✅ Checklist

All: Set appropriate labels for the changes.
All: Considered squashing commits to improve commit history.
All: Added an entry to CHANGELOG.md.
All: Considered updating the online docs in the ./docs/ directory.
Tests: All converted JSON/YML tests from ethereum/tests have been added to converted-ethereum-tests.txt.
Tests: A PR with removal of converted JSON/YML tests from ethereum/tests have been opened.
Tests: Included the type and version of evm t8n tool used to locally execute test cases: e.g., ref with commit hash or geth 1.13.1-stable-3f40e65.
Tests: Ran mkdocs serve locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.

marioevz

LGTM!

A couple of things in the commits I made after yours:

Some of the modules were causing issues due to use of hyphens in the name, so I renamed them to use underscores.
I marked stTimeConsuming as slow, just so we know that these tests take time to fill when using EELS, although in #1491 I recommend we use evmone to fill all legacy tests
I marked all tests/legacy/state_tests/Cancun/stEIP4844_blobtransactions/blobhashListBounds*Filler.yml tests with the skip marker, since these were the only tests in the entire set that expected no failures in Cancun, and a failure in Prague, and since these are already covered and parametrized for future forks in ./tests/cancun/eip4844*`, they can be considered redundant.
Modified several tests to start from block 1, because higher blocks were causing issues, and the block number was irrelevant to the tests.

spencer-tb

Approved from myside, would be happy to merge!

But just a thought: should we not use a submodule or attach the tests to a release in ethereum/tests as a tar?

And is the word legacy correct here as we also have a legacy tests repo?

winsvega · 2025-04-24T09:05:31Z

yes it is a bit misleading. the legacy keyword.
the plan is following:

ethereum/tests stays as is untill we convert all json/yml into python, then we remove all static tests in python
releases are mamanged by execution spec tests releases
ethereum/legacytests remains as is as legacy tests optional to run. perhaps we also commit there execution spec tests test generation as we won't eventually be able to support all forks for all tests.
ethereum/tests repo once converted can become the main repo for test release process?

danceratopz

Can we add the tests at tests/static instead of tests/legacy? Happy to hear other suggestions, but seems more logical/reads better?

fill --until=Prague --fill-static-tests ./tests/static

Does the static filler need the __init__.py file, btw? If not, we could leave them out? If so, then I would update all the __init__.py to remove "legacy" and use "static".

And can we add ./tests/static/README.md which states the ethereum/tests reference (with link) that these tests were taken from?

chfast · 2025-04-24T13:06:31Z

Are you going to check-in all these files to the repo? This is one of the main pain points for ethereum/tests: there are a lot of commits changing a lot of files in a bulk mode so ethereum/tests is unusable by default git clone. You need some fancy git tricks to reduce the download size.

winsvega · 2025-04-24T13:18:05Z

Submodule it to a separate repo?
This files only needed to generate a release

spencer-tb · 2025-04-24T13:43:10Z

I'm realising its okay here with all the files as they will never change, as they are just the fillers not the json fixtures! So from my side its fine without the submodules!

Plus on Dan's comment: legacy -> static

chfast · 2025-04-24T13:47:47Z

But are you going to remove them gradually?

spencer-tb · 2025-04-24T13:54:16Z

Yeah I believe so, once a filler is ported into python the filler will be removed. This could make the git history unusable but I'm not 100% sure

marioevz · 2025-04-24T14:03:52Z

Submodule it to a separate repo? This files only needed to generate a release

We talked about this and the nice thing about having all of this in the same repository is that we can now have atomic PRs that update a static yml/json filler to python.

chfast · 2025-04-24T14:08:10Z

Fine to me, I won't block it.

fix(tests): Rename modules to comply with python naming system fix(tests): Mark stTimeConsuming as slow fix(tests): Skip deprecated/duplicated blob tests fix(tests/legacy): All state tests start on block 1 docs: Changelog fix(tests): Remove static blob tests altogether refactor: rename rename feat(tests): Add tests/static/README.md readme changes

marioevz · 2025-04-24T16:15:24Z

@danceratopz Change is in, let me know what you think.

danceratopz

Thanks, LGTM.

~~Ideally, we'd add an ethereum/tests permalink (with commit hash) from which the files were taken from.~~
Edit: It's there!

And just out of interest, do we need the __init__.py files? For the static filler?

danceratopz · 2025-04-24T17:34:52Z

Ideally, we'd add an ethereum/tests permalink (with commit hash) from which the files were taken from.

Ah sorry, just saw you added this, thanks! So many files!

danceratopz

Minor nit, not important.

winsvega · 2025-04-24T17:38:21Z

Submodule it to a separate repo? This files only needed to generate a release

We talked about this and the nice thing about having all of this in the same repository is that we can now have atomic PRs that update a static yml/json filler to python.

I would still double check the coverage with ported ethereum test.txt file

marioevz · 2025-04-24T17:38:25Z

Thanks, LGTM.

~~Ideally, we'd add an ethereum/tests permalink (with commit hash) from which the files were taken from.~~ Edit: It's there!

And just out of interest, do we need the __init__.py files? For the static filler?

This line fails if we don't add these :(

execution-spec-tests/src/pytest_plugins/filler/static_filler.py

Line 322 in ab9817d

self.params[fixture_name] = request.getfixturevalue(fixture_name)

I'd rather not add them too and I hope we find a workaround at some point.

winsvega · 2025-04-24T17:39:17Z

Thanks, LGTM.

~~Ideally, we'd add an ethereum/tests permalink (with commit hash) from which the files were taken from.~~
Edit: It's there!

And just out of interest, do we need the __init__.py files? For the static filler?

Yes so it will scan Filler files for pydantic scheme.

Co-authored-by: danceratopz <danceratopz@gmail.com>

…reum/tests` (ethereum#1442) * feat(tests/static): all ethereum/tests state_test fillers fix(tests): Rename modules to comply with python naming system fix(tests): Mark stTimeConsuming as slow fix(tests): Skip deprecated/duplicated blob tests fix(tests/legacy): All state tests start on block 1 docs: Changelog fix(tests): Remove static blob tests altogether refactor: rename rename feat(tests): Add tests/static/README.md readme changes * Update docs/CHANGELOG.md Co-authored-by: danceratopz <danceratopz@gmail.com> * Update CHANGELOG.md --------- Co-authored-by: Mario Vega <marioevz@gmail.com> Co-authored-by: danceratopz <danceratopz@gmail.com>

spencer-tb added type:feat type: Feature scope:fill Scope: fill command labels Apr 14, 2025

spencer-tb changed the title ~~feat(json fillers): Legacy state tests~~ feat(fill): refill all legacy state tests Apr 14, 2025

marioevz force-pushed the legacy_state_tests branch from dbd0898 to fdcf43f Compare April 23, 2025 21:56

This was referenced Apr 23, 2025

feat(static tests) : do not compile test fillers source code while loading the models #1439

Merged

feat(ci): Add static feature to produce fixtures_static #1491

Merged

marioevz force-pushed the legacy_state_tests branch from fdcf43f to 6f8a939 Compare April 23, 2025 23:28

marioevz approved these changes Apr 23, 2025

View reviewed changes

spencer-tb approved these changes Apr 24, 2025

View reviewed changes

danceratopz requested changes Apr 24, 2025

View reviewed changes

marioevz force-pushed the legacy_state_tests branch from 6f8a939 to 2cfd334 Compare April 24, 2025 16:14

danceratopz approved these changes Apr 24, 2025

View reviewed changes

danceratopz changed the title ~~feat(fill): refill all legacy state tests~~ feat(fill,tests): add legacy state tests Apr 24, 2025

danceratopz added the scope:tests Scope: Changes EL client test cases in `./tests` label Apr 24, 2025

danceratopz reviewed Apr 24, 2025

View reviewed changes

Comment thread docs/CHANGELOG.md Outdated

Update docs/CHANGELOG.md

3f91410

Co-authored-by: danceratopz <danceratopz@gmail.com>

danceratopz changed the title ~~feat(fill,tests): add legacy state tests~~ feat(fill,tests): add all static GeneralStateTestsFiller from ethereum/tests Apr 24, 2025

Update CHANGELOG.md

26a7c23

marioevz merged commit 6203535 into main Apr 24, 2025
22 checks passed

marioevz deleted the legacy_state_tests branch April 24, 2025 17:55

Conversation

winsvega commented Apr 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🗒️ Description

🔗 Related Issues

✅ Checklist

Uh oh!

marioevz left a comment

Choose a reason for hiding this comment

Uh oh!

spencer-tb left a comment

Choose a reason for hiding this comment

Uh oh!

winsvega commented Apr 24, 2025

Uh oh!

danceratopz left a comment

Choose a reason for hiding this comment

Uh oh!

chfast commented Apr 24, 2025

Uh oh!

winsvega commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

spencer-tb commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chfast commented Apr 24, 2025

Uh oh!

spencer-tb commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

marioevz commented Apr 24, 2025

Uh oh!

chfast commented Apr 24, 2025

Uh oh!

marioevz commented Apr 24, 2025

Uh oh!

danceratopz left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danceratopz commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danceratopz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

winsvega commented Apr 24, 2025

Uh oh!

marioevz commented Apr 24, 2025

Uh oh!

winsvega commented Apr 24, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

winsvega commented Apr 11, 2025 •

edited

Loading

winsvega commented Apr 24, 2025 •

edited

Loading

spencer-tb commented Apr 24, 2025 •

edited

Loading

spencer-tb commented Apr 24, 2025 •

edited

Loading

danceratopz left a comment •

edited

Loading

danceratopz commented Apr 24, 2025 •

edited

Loading