Skip to content

feat(plugins): Static test filler #1336

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

Merged
merged 17 commits into from
Mar 28, 2025
Merged

feat(plugins): Static test filler #1336

merged 17 commits into from
Mar 28, 2025

Conversation

marioevz
Copy link
Member

@marioevz marioevz commented Mar 20, 2025

🗒️ Description

Introduces the refiller plugin which is capable of loading static test specs from static files.

The plugin uses static specs which inherit from base in src/ethereum_test_specs/base_static.py.

The inheriting classes must implement the fill_function method which returns the function that will be used by refiller to fill the static tests when the static file is loaded through the class.

This PR contains no implementation of classes that inherit from the base static test class, and these should be implemented by follow-up PRs separately.

An (non-working) example can be seen in the following file: https://github.com/ethereum/execution-spec-tests/blob/filler-reader/src/ethereum_test_specs/state_json.py

Refiller mimics the normal python test filler id convention and therefore the files it loads must always contain sub-tests.

I.e. the loaded static test files must be a JSON/YAML with an object at root level, of which every key is a sub-test.

Example of test IDs from a static test file:

tests/homestead/contractCreationOOGdontLeaveEmptyContractFiller.json::contractCreationOOGdontLeaveEmptyContract[fork_Berlin-state_test] FILLED
tests/homestead/contractCreationOOGdontLeaveEmptyContractFiller.json::contractCreationOOGdontLeaveEmptyContract[fork_Berlin-blockchain_test] FILLED
tests/homestead/contractCreationOOGdontLeaveEmptyContractFiller.json::contractCreationOOGdontLeaveEmptyContract[fork_Berlin-blockchain_test_engine] FILLED

🔗 Related Issues

✅ 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.

Copy link
Contributor

@winsvega winsvega left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you show an example how would it work from my test filler parser returning TestVector class. (the one that has env,pre,post,tx,id) ?

@marioevz marioevz force-pushed the static-test-loader branch from 8e28d30 to b2ee9c0 Compare March 24, 2025 18:28
@marioevz
Copy link
Member Author

can you show an example how would it work from my test filler parser returning TestVector class. (the one that has env,pre,post,tx,id) ?

Here's the example to implement EOF static file readers:
b21260c

@marioevz
Copy link
Member Author

Currently, all tests are failing due to:

Test doesn't define REFERENCE_SPEC_GIT_PATH and REFERENCE_SPEC_VERSION

I feel like this should be ok for static legacy tests and only be a concern when actually porting the test to python.

Wdyt @spencer-tb @danceratopz @winsvega ?

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request introduces the refiller plugin to load static test specifications from static files and integrates it with existing testing infrastructure, while also making refinements in validity marker processing and test item collection.

  • Introduces the new refiller plugin under src/pytest_plugins/refiller to convert static files into pytest test items
  • Updates related plugins (shared/execute_fill, filler, forks) to support parameterization and fork handling necessary for static tests
  • Extends test coverage in ethereum_test_specs by adding tests for fork range descriptor parsing

Reviewed Changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/pytest_plugins/spec_version_checker/spec_version_checker.py Adds a conditional check on request.module before returning the reference spec
src/pytest_plugins/shared/execute_fill.py Replaces cast with an isinstance check and enhances test description gathering
src/pytest_plugins/refiller/refiller.py Introduces extensive logic for collecting and parameterizing static tests from files
src/pytest_plugins/refiller/init.py Adds a minimal package docstring
src/pytest_plugins/forks/forks.py Refactors validity marker functions to use explicit test name, config, and markers
src/pytest_plugins/filler/filler.py Adjusts fixture resolution and modifies test item collection for improved robustness
src/ethereum_test_specs/tests/test_types.py Adds tests to validate fork range descriptor parsing from strings
src/ethereum_test_specs/base_static.py Introduces the BaseStaticTest class with pydantic integration for static test formats
src/ethereum_test_specs/init.py Exports BaseStaticTest for consumption by the rest of the codebase
Files not reviewed (1)
  • pytest.ini: Language not supported

Copy link
Member

@danceratopz danceratopz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing! Couple of small comments. The refiller plugin itself is pretty crazy :)

refill is catchy but I'm not sure it correctly conveys its purpose, I think that static_filler would be better. A refiller sounds like it refills the fixtures, not the fillers?

@marioevz marioevz changed the title feat(plugins): Refiller feat(plugins): Static test filler Mar 28, 2025
@marioevz marioevz merged commit b4e09b1 into main Mar 28, 2025
21 checks passed
@marioevz marioevz deleted the static-test-loader branch March 28, 2025 21:06
felix314159 pushed a commit to felix314159/execution-spec-tests that referenced this pull request May 16, 2025
* feat(specs): Introduce static spec types

* feat(plugins): Add Refiller plugin

* fix: tox

* feat(specs): Implement fork range descriptor parser

* fix(refiller): Handle `ids` and `marks`

* fix(refiller): Handle duplicate test ids

* fix(specs): Add base static helpers

* feat(plugins/forks): Add `valid_at` marker

* fix(plugins/refiller): warn instead of error for parsing failures

* fix(plugins/refiller): Check filler file name before loading

* fix(fixtures): Fix TestInfo to remove `Filler` suffix

* fix(refiller): Remove try-except to print parse errors

* fix(fixtures/collector): Remove unused function

* fix: Tox

* refactor(refiller): Rename as static-filler

* refactor(forks): Move ForkRangeDescriptor, get_fork_by_name

* docs: Add changelog
codeofcarson pushed a commit to codeofcarson/execution-spec-tests that referenced this pull request Jul 1, 2025
* feat(specs): Introduce static spec types

* feat(plugins): Add Refiller plugin

* fix: tox

* feat(specs): Implement fork range descriptor parser

* fix(refiller): Handle `ids` and `marks`

* fix(refiller): Handle duplicate test ids

* fix(specs): Add base static helpers

* feat(plugins/forks): Add `valid_at` marker

* fix(plugins/refiller): warn instead of error for parsing failures

* fix(plugins/refiller): Check filler file name before loading

* fix(fixtures): Fix TestInfo to remove `Filler` suffix

* fix(refiller): Remove try-except to print parse errors

* fix(fixtures/collector): Remove unused function

* fix: Tox

* refactor(refiller): Rename as static-filler

* refactor(forks): Move ForkRangeDescriptor, get_fork_by_name

* docs: Add changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants