Skip to content

Conversation

danceratopz
Copy link
Member

@danceratopz danceratopz commented Apr 16, 2025

🗒️ Description

This PR adds a FixtureOutput class (which inherits from pydantic BaseModel) to filler.py in order to encapsulate fiil's fixture JSON output-related settings. It's instantiated from the values of the command-line arguments that modify JSON output: --output, --flat-output, --single-fixture-per-file.

🔗 Related Issues

Refactoring in preparation of:

✅ Checklist

  • All: Set appropriate labels for the changes.
  • All: Considered squashing commits to improve commit history.
  • All: Added an entry to CHANGELOG.md.

@danceratopz danceratopz added type:refactor Type: Refactor scope:fill Scope: fill command labels Apr 16, 2025
@danceratopz danceratopz marked this pull request as draft April 17, 2025 00:15
@danceratopz
Copy link
Member Author

danceratopz commented Apr 17, 2025

Looks like this is not safe with pytest-xdist, see:
https://github.com/ethereum/execution-spec-tests/actions/runs/14504935803/job/40692463739

Will take another look.

@danceratopz
Copy link
Member Author

Looks like this is not safe with pytest-xdist, see: https://github.com/ethereum/execution-spec-tests/actions/runs/14504935803/job/40692463739

This PR shouldn't have an issue with pytest-xdist, as it doesn't change the output directory behavior. It is and issue for #1473 though.

@danceratopz danceratopz marked this pull request as ready for review April 17, 2025 17:52
Copy link
Member

@marioevz marioevz left a comment

Choose a reason for hiding this comment

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

Some comments.

@danceratopz danceratopz force-pushed the refactor-fill-output branch from 6d5a92c to 9e9c26d Compare May 14, 2025 13:36
@danceratopz danceratopz requested a review from marioevz May 14, 2025 16:01
@winsvega
Copy link
Contributor

Does anyone use --flat-output?
This option was requested by me to fill .py tests from retesteth

Copy link
Member

@marioevz marioevz left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@marioevz marioevz merged commit f6bd188 into main May 14, 2025
21 checks passed
@marioevz marioevz deleted the refactor-fill-output branch May 14, 2025 17:33
felix314159 pushed a commit to felix314159/execution-spec-tests that referenced this pull request May 16, 2025
… class (ethereum#1471)

* refactor(fill): encapsulate fixture output ops in `FixtureOutput` class

* docs: update changelog

* refactor(fill): convert `FixtureOutput` to pydantic model

* refactor(fill): move `FixtureOutput` class to its own moudule

* test(fill): fix unit test

* docs: Move changelog

* fix: changelog

---------

Co-authored-by: Mario Vega <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope:fill Scope: fill command type:refactor Type: Refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants