refactor(fill): encapsulate fixture output options in FixtureOutput class (#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]>

Author: danceratopz

docs/CHANGELOG.md

@@ -8,6 +8,22 @@ Test fixtures for use by clients are available for each release on the [Github r
 
 ### 💥 Breaking Change
 
+### 🛠️ Framework
+
+#### `fill`
+
+- 🔀 Refactor: Encapsulate `fill`'s fixture output options (`--output`, `--flat-output`, `--single-fixture-per-file`) into a `FixtureOutput` class ([#1471](https://github.com/ethereum/execution-spec-tests/pull/1471)).
+
+#### `consume`
+
+### 📋 Misc
+
+### 🧪 Test Cases
+
+## [v4.5.0](https://github.com/ethereum/execution-spec-tests/releases/tag/v4.5.0) - 2025-05-14
+
+### 💥 Breaking Change
+
 #### EOF removed from Osaka
 
 Following ["Interop Testing Call 34"](https://github.com/ethereum/pm/issues/1499) and the procedural EIPs [PR](https://github.com/ethereum/EIPs/pull/9703) the decision to remove EOF from Osaka was made.

src/pytest_plugins/filler/__init__.py

@@ -1,4 +1,7 @@
-"""
-A pytest plugin that provides fixtures that fill tests and generate
-fixtures.
-"""
+"""A pytest plugin to fill tests and generate JSON fixtures."""
+
+from .fixture_output import FixtureOutput
+
+__all__ = [
+    "FixtureOutput",
+]

src/pytest_plugins/filler/filler.py

@@ -9,7 +9,6 @@
 import configparser
 import datetime
 import os
-import tarfile
 import warnings
 from pathlib import Path
 from typing import Any, Dict, Generator, List, Type
@@ -35,6 +34,7 @@
 
 from ..shared.helpers import get_spec_format_for_item, labeled_format_parameter_set
 from ..spec_version_checker.spec_version_checker import get_ref_spec_from_module
+from .fixture_output import FixtureOutput
 
 
 def default_output_directory() -> str:
@@ -53,18 +53,6 @@ def default_html_report_file_path() -> str:
     return ".meta/report_fill.html"
 
 
-def strip_output_tarball_suffix(output: Path) -> Path:
-    """Strip the '.tar.gz' suffix from the output path."""
-    if str(output).endswith(".tar.gz"):
-        return output.with_suffix("").with_suffix("")
-    return output
-
-
-def is_output_stdout(output: Path) -> bool:
-    """Return True if the fixture output is configured to be stdout."""
-    return strip_output_tarball_suffix(output).name == "stdout"
-
-
 def pytest_addoption(parser: pytest.Parser):
     """Add command-line options to pytest."""
     evm_group = parser.getgroup("evm", "Arguments defining evm executable behavior")
@@ -229,12 +217,14 @@ def pytest_configure(config):
         EnvironmentDefaults.gas_limit = config.getoption("block_gas_limit")
     if config.option.collectonly:
         return
+
+    # Initialize fixture output configuration
+    config.fixture_output = FixtureOutput.from_config(config)
+
     if not config.getoption("disable_html") and config.getoption("htmlpath") is None:
         # generate an html report by default, unless explicitly disabled
-        config.option.htmlpath = (
-            strip_output_tarball_suffix(config.getoption("output"))
-            / default_html_report_file_path()
-        )
+        config.option.htmlpath = config.fixture_output.directory / default_html_report_file_path()
+
     # Instantiate the transition tool here to check that the binary path/trace option is valid.
     # This ensures we only raise an error once, if appropriate, instead of for every test.
     t8n = TransitionTool.from_binary_path(
@@ -288,7 +278,7 @@ def pytest_report_teststatus(report, config: pytest.Config):
     ...x...
     ```
     """
-    if is_output_stdout(config.getoption("output")):
+    if config.fixture_output.is_stdout:  # type: ignore[attr-defined]
         return report.outcome, "", report.outcome.upper()
 
 
@@ -303,12 +293,12 @@ def pytest_terminal_summary(
     actually run the tests.
     """
     yield
-    if is_output_stdout(config.getoption("output")):
+    if config.fixture_output.is_stdout:  # type: ignore[attr-defined]
         return
     stats = terminalreporter.stats
     if "passed" in stats and stats["passed"]:
         # append / to indicate this is a directory
-        output_dir = str(strip_output_tarball_suffix(config.getoption("output"))) + "/"
+        output_dir = str(config.fixture_output.directory) + "/"  # type: ignore[attr-defined]
         terminalreporter.write_sep(
             "=",
             (
@@ -505,45 +495,33 @@ def base_dump_dir(request: pytest.FixtureRequest) -> Path | None:
 
 
 @pytest.fixture(scope="session")
-def is_output_tarball(request: pytest.FixtureRequest) -> bool:
-    """Return True if the output directory is a tarball."""
-    output: Path = request.config.getoption("output")
-    if output.suffix == ".gz" and output.with_suffix("").suffix == ".tar":
-        return True
-    return False
+def fixture_output(request: pytest.FixtureRequest) -> FixtureOutput:
+    """Return the fixture output configuration."""
+    return request.config.fixture_output  # type: ignore[attr-defined]
 
 
 @pytest.fixture(scope="session")
-def output_dir(request: pytest.FixtureRequest, is_output_tarball: bool) -> Path:
-    """Return directory to store the generated test fixtures."""
-    output = request.config.getoption("output")
-    if is_output_tarball:
-        return strip_output_tarball_suffix(output)
-    return output
+def is_output_tarball(fixture_output: FixtureOutput) -> bool:
+    """Return True if the output directory is a tarball."""
+    return fixture_output.is_tarball
 
 
 @pytest.fixture(scope="session")
-def output_metadata_dir(output_dir: Path) -> Path:
-    """Return metadata directory to store fixture meta files."""
-    if is_output_stdout(output_dir):
-        return output_dir
-    return output_dir / ".meta"
+def output_dir(fixture_output: FixtureOutput) -> Path:
+    """Return directory to store the generated test fixtures."""
+    return fixture_output.directory
 
 
 @pytest.fixture(scope="session", autouse=True)
-def create_properties_file(
-    request: pytest.FixtureRequest, output_dir: Path, output_metadata_dir: Path
-) -> None:
+def create_properties_file(request: pytest.FixtureRequest, fixture_output: FixtureOutput) -> None:
     """
     Create ini file with fixture build properties in the fixture output
     directory.
     """
-    if is_output_stdout(request.config.getoption("output")):
+    if fixture_output.is_stdout:
         return
-    if not output_dir.exists():
-        output_dir.mkdir(parents=True)
-    if not output_metadata_dir.exists():
-        output_metadata_dir.mkdir(parents=True)
+
+    fixture_output.create_directories()
 
     fixture_properties = {
         "timestamp": datetime.datetime.now().isoformat(),
@@ -574,7 +552,7 @@ def create_properties_file(
             )
     config["environment"] = environment_properties
 
-    ini_filename = output_metadata_dir / "fixtures.ini"
+    ini_filename = fixture_output.metadata_dir / "fixtures.ini"
     with open(ini_filename, "w") as f:
         f.write("; This file describes fixture build properties\n\n")
         config.write(f)
@@ -610,9 +588,9 @@ def get_fixture_collection_scope(fixture_name, config):
 
     See: https://docs.pytest.org/en/stable/how-to/fixtures.html#dynamic-scope
     """
-    if is_output_stdout(config.getoption("output")):
+    if config.fixture_output.is_stdout:
         return "session"
-    if config.getoption("single_fixture_per_file"):
+    if config.fixture_output.single_fixture_per_file:
         return "function"
     return "module"
 
@@ -636,17 +614,17 @@ def fixture_collector(
     evm_fixture_verification: FixtureConsumer,
     filler_path: Path,
     base_dump_dir: Path | None,
-    output_dir: Path,
+    fixture_output: FixtureOutput,
 ) -> Generator[FixtureCollector, None, None]:
     """
     Return configured fixture collector instance used for all tests
     in one test module.
     """
     fixture_collector = FixtureCollector(
-        output_dir=output_dir,
-        flat_output=request.config.getoption("flat_output"),
+        output_dir=fixture_output.directory,
+        flat_output=fixture_output.flat_output,
         fill_static_tests=request.config.getoption("fill_static_tests_enabled"),
-        single_fixture_per_file=request.config.getoption("single_fixture_per_file"),
+        single_fixture_per_file=fixture_output.single_fixture_per_file,
         filler_path=filler_path,
         base_dump_dir=base_dump_dir,
     )
@@ -875,29 +853,20 @@ def pytest_sessionfinish(session: pytest.Session, exitstatus: int):
     if xdist.is_xdist_worker(session):
         return
 
-    output: Path = session.config.getoption("output")
+    fixture_output = session.config.fixture_output  # type: ignore[attr-defined]
     # When using --collect-only it should not matter whether fixtures folder exists or not
-    if is_output_stdout(output) or session.config.option.collectonly:
+    if fixture_output.is_stdout or session.config.option.collectonly:
         return
 
-    output_dir = strip_output_tarball_suffix(output)
     # Remove any lock files that may have been created.
-    for file in output_dir.rglob("*.lock"):
+    for file in fixture_output.directory.rglob("*.lock"):
         file.unlink()
 
     # Generate index file for all produced fixtures.
     if session.config.getoption("generate_index"):
         generate_fixtures_index(
-            output_dir, quiet_mode=True, force_flag=False, disable_infer_format=False
+            fixture_output.directory, quiet_mode=True, force_flag=False, disable_infer_format=False
         )
 
     # Create tarball of the output directory if the output is a tarball.
-    is_output_tarball = output.suffix == ".gz" and output.with_suffix("").suffix == ".tar"
-    if is_output_tarball:
-        source_dir = output_dir
-        tarball_filename = output
-        with tarfile.open(tarball_filename, "w:gz") as tar:
-            for file in source_dir.rglob("*"):
-                if file.suffix in {".json", ".ini"}:
-                    arcname = Path("fixtures") / file.relative_to(source_dir)
-                    tar.add(file, arcname=arcname)
+    fixture_output.create_tarball()

src/pytest_plugins/filler/fixture_output.py

@@ -0,0 +1,93 @@
+"""Fixture output configuration for generated test fixtures."""
+
+import tarfile
+from pathlib import Path
+
+import pytest
+from pydantic import BaseModel, Field
+
+
+class FixtureOutput(BaseModel):
+    """Represents the output destination for generated test fixtures."""
+
+    output_path: Path = Field(description="Directory path to store the generated test fixtures")
+    flat_output: bool = Field(
+        default=False,
+        description="Output each test case in the directory without the folder structure",
+    )
+    single_fixture_per_file: bool = Field(
+        default=False,
+        description=(
+            "Don't group fixtures in JSON files by test function; "
+            "write each fixture to its own file"
+        ),
+    )
+
+    @property
+    def directory(self) -> Path:
+        """Return the actual directory path where fixtures will be written."""
+        return self.strip_tarball_suffix(self.output_path)
+
+    @property
+    def metadata_dir(self) -> Path:
+        """Return metadata directory to store fixture meta files."""
+        if self.is_stdout:
+            return self.directory
+        return self.directory / ".meta"
+
+    @property
+    def is_tarball(self) -> bool:
+        """Return True if the output should be packaged as a tarball."""
+        path = self.output_path
+        return path.suffix == ".gz" and path.with_suffix("").suffix == ".tar"
+
+    @property
+    def is_stdout(self) -> bool:
+        """Return True if the fixture output is configured to be stdout."""
+        return self.directory.name == "stdout"
+
+    @staticmethod
+    def strip_tarball_suffix(path: Path) -> Path:
+        """Strip the '.tar.gz' suffix from the output path."""
+        if str(path).endswith(".tar.gz"):
+            return path.with_suffix("").with_suffix("")
+        return path
+
+    def create_directories(self) -> None:
+        """Create output and metadata directories if needed."""
+        if self.is_stdout:
+            return
+
+        self.directory.mkdir(parents=True, exist_ok=True)
+        self.metadata_dir.mkdir(parents=True, exist_ok=True)
+
+    def create_tarball(self) -> None:
+        """Create tarball of the output directory if configured to do so."""
+        if not self.is_tarball:
+            return
+
+        with tarfile.open(self.output_path, "w:gz") as tar:
+            for file in self.directory.rglob("*"):
+                if file.suffix in {".json", ".ini"}:
+                    arcname = Path("fixtures") / file.relative_to(self.directory)
+                    tar.add(file, arcname=arcname)
+
+    @classmethod
+    def from_options(
+        cls, output_path: Path, flat_output: bool, single_fixture_per_file: bool
+    ) -> "FixtureOutput":
+        """Create a FixtureOutput instance from pytest options."""
+        return cls(
+            output_path=output_path,
+            flat_output=flat_output,
+            single_fixture_per_file=single_fixture_per_file,
+        )
+
+    @classmethod
+    def from_config(cls, config: pytest.Config) -> "FixtureOutput":
+        """Create a FixtureOutput instance from pytest configuration."""
+        return cls(
+            output_path=config.getoption("output"),
+            flat_output=config.getoption("flat_output"),
+            single_fixture_per_file=config.getoption("single_fixture_per_file"),
+        )

src/pytest_plugins/filler/tests/test_filler.py

@@ -569,6 +569,7 @@ def test_fixture_output_based_on_command_line_args(
     expected_ini_file = "fixtures.ini"
     expected_index_file = "index.json"
     expected_resolver_file = None
+    resolver_file = None
     if TransitionTool.default_tool == ExecutionSpecsTransitionTool:
         expected_resolver_file = "eels_resolutions.json"
 
@@ -698,6 +699,7 @@ def test_fill_variables(
     expected_ini_file = "fixtures.ini"
     expected_index_file = "index.json"
     expected_resolver_file = None
+    resolver_file = None
     if TransitionTool.default_tool == ExecutionSpecsTransitionTool:
         expected_resolver_file = "eels_resolutions.json"