refactor: Abstract fixture type collection; Parametrize tests over fi…

…xture formats; Per-format sub-folder output (ethereum#358)

* tools: Move fixture collector to spec

* refactor: Fixtures collector

* all: remove `BaseTestConfig`

* fix: all filler tests

* docs: update

* tox: spelling

* feat: Parametrize the supported fixture types for each test type

* fix: many tests

* fix: remaining tests

* fix: fixture name case

* fix: Indirect parametrize using fixture format

* feat: Dynamic generation of spec test fixtures

* fix: type-checking

* fix: imports

* feat(pytest): allow filtering by fixture format via pytest mark (#12)

* feat(pytest): allow filtering by fixture format via pytest mark

* feat(fw): give unset fixture format a more verbose name

* feat(pytest): only paramatrize post-merge forks with hive format (#13)

* feat(pytest): remove items which are pre-merge and have hive format

* chore: make exception string clearer; fix whitespace

* fix(pytest): test_filler: remove `enable_hive` completely

* fix: Remove unused `STATE_TEST_HIVE`

---------

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

docs/CHANGELOG.md

@@ -19,6 +19,7 @@ Test fixtures for use by clients are available for each release on the [Github r
 - 🔀 Locally calculate the transactions list's root instead of using the one returned by t8n when producing BlockchainTests ([#353](https://github.com/ethereum/execution-spec-tests/pull/353))
 - ✨ Fork objects used to write tests can now be compared using the `>`, `>=`, `<`, `<=` operators, to check for a fork being newer than, newer than or equal, older than, older than or equal, respectively when compared against other fork ([#367](https://github.com/ethereum/execution-spec-tests/pull/367))
 - 🐞 Storage type iterator is now fixed ([#369](https://github.com/ethereum/execution-spec-tests/pull/369))
+- 💥 Removed `--enable-hive` parameter, now all test types are generated by default ([#358](https://github.com/ethereum/execution-spec-tests/pull/358))
 
 ### 🔧 EVM Tools
 
@@ -28,7 +29,11 @@ Test fixtures for use by clients are available for each release on the [Github r
 
 ## Breaking Changes
 
-1. In this release the pytest node ID is now used for fixture names (previously only the test parameters were used), this should not be breaking. However, "=" in both node IDs and therefore fixture names, have been replaced with "_", which may break tooling that depends on the "=" character.
+1. Fixture output, including release tarballs, now contain subdirectories for different test types:
+   - `blockchain_tests`: Contains BlockchainTest formatted tests
+   - `blockchain_tests_hive`: Contains BlockchainTest with Engine API call directives for use in hive
+   - `state_tests`: Contains StateTest formatted tests
+2. In this release the pytest node ID is now used for fixture names (previously only the test parameters were used), this should not be breaking. However, "=" in both node IDs and therefore fixture names, have been replaced with "_", which may break tooling that depends on the "=" character.
 
    Pytest node ID example:
 

docs/getting_started/debugging_t8n_tools.md

@@ -22,38 +22,39 @@ will produce the directory structure:
 
 ```text
 📂 /tmp/evm-dump
-└── 📂 berlin__eip2930_access_list__test_acl__test_access_list
-    └── 📂 fork_Berlin
-        ├── 📂 0
-        │   ├── 📄 args.py
-        │   ├── 📂 input
-        │   │   ├── 📄 alloc.json
-        │   │   ├── 📄 env.json
-        │   │   └── 📄 txs.json
-        │   ├── 📂 output
-        │   │   ├── 📄 alloc.json
-        │   │   ├── 📄 result.json
-        │   │   └── 📄 txs.rlp
-        │   ├── 📄 returncode.txt
-        │   ├── 📄 stderr.txt
-        │   ├── 📄 stdin.txt
-        │   ├── 📄 stdout.txt
-        │   └── 📄 t8n.sh
-        └── 📂 1
-            ├── 📄 args.py
-            ├── 📂 input
-            │   ├── 📄 alloc.json
-            │   ├── 📄 env.json
-            │   └── 📄 txs.json
-            ├── 📂 output
-            │   ├── 📄 alloc.json
-            │   ├── 📄 result.json
-            │   └── 📄 txs.rlp
-            ├── 📄 returncode.txt
-            ├── 📄 stderr.txt
-            ├── 📄 stdin.txt
-            ├── 📄 stdout.txt
-            └── 📄 t8n.sh
+└── 📂 blockchain_tests
+    └── 📂 berlin__eip2930_access_list__test_acl__test_access_list
+        └── 📂 fork_Berlin
+            ├── 📂 0
+            │   ├── 📄 args.py
+            │   ├── 📂 input
+            │   │   ├── 📄 alloc.json
+            │   │   ├── 📄 env.json
+            │   │   └── 📄 txs.json
+            │   ├── 📂 output
+            │   │   ├── 📄 alloc.json
+            │   │   ├── 📄 result.json
+            │   │   └── 📄 txs.rlp
+            │   ├── 📄 returncode.txt
+            │   ├── 📄 stderr.txt
+            │   ├── 📄 stdin.txt
+            │   ├── 📄 stdout.txt
+            │   └── 📄 t8n.sh
+            └── 📂 1
+                ├── 📄 args.py
+                ├── 📂 input
+                │   ├── 📄 alloc.json
+                │   ├── 📄 env.json
+                │   └── 📄 txs.json
+                ├── 📂 output
+                │   ├── 📄 alloc.json
+                │   ├── 📄 result.json
+                │   └── 📄 txs.rlp
+                ├── 📄 returncode.txt
+                ├── 📄 stderr.txt
+                ├── 📄 stdin.txt
+                ├── 📄 stdout.txt
+                └── 📄 t8n.sh
 ```
 
 where the directories `0` and `1` correspond to the different calls made to the `t8n` tool executed during the test:
@@ -120,24 +121,25 @@ will additionally run the `evm blocktest` command on every JSON fixture file and
 
 ```text
 📂 /tmp/evm-dump
-└── 📂 berlin__eip2930_access_list__test_acl__test_access_list
-    ├── 📄 fixtures.json
-    ├── 📂 fork_Berlin
-    │   ├── 📂 0
-    │   │   ├── 📄 args.py
-    │   │   ├── 📂 input
-    │   │   │   ├── 📄 alloc.json
-    │   │   │   ├── 📄 env.json
-    │   │   │   └── 📄 txs.json
-    │   │   ├── 📂 output
-    │   │   │   ├── 📄 alloc.json
-    │   ... ... ...
-    │
-    ├── 📄 verify_fixtures_args.py
-    ├── 📄 verify_fixtures_returncode.txt
-    ├── 📄 verify_fixtures.sh
-    ├── 📄 verify_fixtures_stderr.txt
-    └── 📄 verify_fixtures_stdout.txt
+└── 📂 blockchain_tests
+    └── 📂 berlin__eip2930_access_list__test_acl__test_access_list
+        ├── 📄 fixtures.json
+        ├── 📂 fork_Berlin
+        │   ├── 📂 0
+        │   │   ├── 📄 args.py
+        │   │   ├── 📂 input
+        │   │   │   ├── 📄 alloc.json
+        │   │   │   ├── 📄 env.json
+        │   │   │   └── 📄 txs.json
+        │   │   ├── 📂 output
+        │   │   │   ├── 📄 alloc.json
+        │   ... ... ...
+        │
+        ├── 📄 verify_fixtures_args.py
+        ├── 📄 verify_fixtures_returncode.txt
+        ├── 📄 verify_fixtures.sh
+        ├── 📄 verify_fixtures_stderr.txt
+        └── 📄 verify_fixtures_stdout.txt
 ```
 
 where the `verify_fixtures.sh` script can be used to reproduce the `evm blocktest` command.

docs/getting_started/executing_tests_command_line.md

@@ -152,7 +152,6 @@ Arguments defining filler location and output:
                         Don't group fixtures in JSON files by test function;
                         write each fixture to its own file. This can be used to
                         increase the granularity of --verify-fixtures.
-  --enable-hive         Output test fixtures with the hive-specific properties.
 
 Arguments defining debug behavior:
   --evm-dump-dir EVM_DUMP_DIR, --t8n-dump-dir EVM_DUMP_DIR

docs/getting_started/quick_start.md

@@ -84,7 +84,7 @@ The following requires a Python 3.10, 3.11 or 3.12 installation.
         2. The corresponding fixture file has been generated:
 
             ```console
-            head fixtures/berlin/eip2930_access_list/acl/access_list.json
+            head fixtures/blockchain_tests/berlin/eip2930_access_list/acl/access_list.json
             ```
 
 ## Next Steps

docs/getting_started/repository_overview.md

@@ -10,8 +10,9 @@ The most relevant folders and files in the repo are:
 │   ├── 📁 vm/
 │   └── 📁 ...
 ├─╴📁 fixtures/                  # default fixture output dir
-│   ├── 📁 eips/
-│   ├── 📁 vm/
+│   ├── 📁 blockchain_tests/
+│   ├── 📁 blockchain_tests_hive/
+│   ├── 📁 state_tests/
 │   └── 📁 ...
 ├─╴📁 src/                       # library & framework packages
 │   ├── 📁 ethereum_test_fork/

docs/getting_started/using_fixtures.md

@@ -37,10 +37,8 @@ The @ethereum/execution-spec-tests repository provides [releases](https://github
 | ------------------------------ | -------- | ------------------ |
 | `fixtures.tar.gz`              | Clients  | All tests until the last stable fork | "Must pass" |
 | `fixtures_develop.tar.gz`      | Clients  | All tests until the last development fork |
-| `fixtures_hive.tar.gaz`        | Hive     | All tests until the last stable fork in hive format |
-| `fixtures_develop_hive.tar.gz` | Hive     | All tests until the last development fork in hive format |
 
-The Hive format uses Engine API directives instead of the usual BlockchainTest format.
+The Hive format tests are included in subdirectory `blockchain_tests_hive` and these use Engine API directives instead of the usual BlockchainTest format.
 
 ## Obtaining the Most Recent Release Artifacts
 

docs/tutorials/state_transition.md

@@ -188,7 +188,7 @@ In this case, we look at the storage of the contract we called and add to it wha
     state_test(env=env, pre=pre, post=post, txs=[tx])
 ```
 
-This line calls the wrapper to the `StateTest` object that provides all the objects required (for example, the fork parameter) in order to fill the test, generate the test fixtures and write them to file (by default, `./fixtures/example/yul_example/test_yul.json`).
+This line calls the wrapper to the `StateTest` object that provides all the objects required (for example, the fork parameter) in order to fill the test, generate the test fixtures and write them to file (by default, `./fixtures/<blockchain,state>_tests/example/yul_example/test_yul.json`).
 
 ## Conclusion
 

src/ethereum_test_tools/__init__.py

@@ -44,24 +44,26 @@
 )
 from .reference_spec import ReferenceSpec, ReferenceSpecTypes
 from .spec import (
+    SPEC_TYPES,
     BaseFixture,
     BaseTest,
-    BaseTestConfig,
     BlockchainTest,
     BlockchainTestFiller,
+    FixtureCollector,
     StateTest,
     StateTestFiller,
+    TestInfo,
 )
 from .spec.blockchain.types import Block, Header
 from .vm import Opcode, OpcodeCallArg, Opcodes
 
 __all__ = (
+    "SPEC_TYPES",
     "AccessList",
     "Account",
     "Auto",
     "BaseFixture",
     "BaseTest",
-    "BaseTestConfig",
     "Block",
     "BlockchainTest",
     "BlockchainTestFiller",
@@ -73,6 +75,7 @@
     "EngineAPIError",
     "Environment",
     "Fixture",
+    "FixtureCollector",
     "FixtureEngineNewPayload",
     "Header",
     "HistoryStorageAddress",
@@ -91,6 +94,7 @@
     "Switch",
     "TestAddress",
     "TestAddress2",
+    "TestInfo",
     "TestPrivateKey",
     "TestPrivateKey2",
     "Transaction",

src/ethereum_test_tools/spec/__init__.py

@@ -1,20 +1,27 @@
 """
 Test spec definitions and utilities.
 """
-from .base.base_test import BaseFixture, BaseTest, BaseTestConfig, TestSpec, verify_post_alloc
+from typing import List, Type
+
+from .base.base_test import BaseFixture, BaseTest, TestSpec, verify_post_alloc
 from .blockchain.blockchain_test import BlockchainTest, BlockchainTestFiller, BlockchainTestSpec
+from .fixture_collector import FixtureCollector, TestInfo
 from .state.state_test import StateTest, StateTestFiller, StateTestSpec
 
+SPEC_TYPES: List[Type[BaseTest]] = [BlockchainTest, StateTest]
+
 __all__ = (
+    "SPEC_TYPES",
     "BaseFixture",
     "BaseTest",
-    "BaseTestConfig",
     "BlockchainTest",
     "BlockchainTestFiller",
     "BlockchainTestSpec",
+    "FixtureCollector",
     "StateTest",
     "StateTestFiller",
     "StateTestSpec",
+    "TestInfo",
     "TestSpec",
     "verify_post_alloc",
 )

src/ethereum_test_tools/spec/base/base_test.py

@@ -5,10 +5,11 @@
 from dataclasses import dataclass, field
 from itertools import count
 from os import path
+from pathlib import Path
 from typing import Any, Callable, Dict, Generator, Iterator, List, Mapping, Optional
 
 from ethereum_test_forks import Fork
-from evm_transition_tool import TransitionTool
+from evm_transition_tool import FixtureFormats, TransitionTool
 
 from ...common import Account, Address, Environment, Transaction, withdrawals_root
 from ...common.conversions import to_hex
@@ -72,26 +73,6 @@ def verify_result(result: Mapping, env: Environment):
         assert result["withdrawalsRoot"] == to_hex(withdrawals_root(env.withdrawals))
 
 
-@dataclass(kw_only=True)
-class BaseTestConfig:
-    """
-    General configuration that all tests must support.
-    """
-
-    enable_hive: bool = False
-    """
-    Enable any hive-related properties that the output could contain.
-    """
-    blockchain_test: bool = False
-    """
-    Enable BlockchainTest type tests production.
-    """
-    state_test: bool = False
-    """
-    Enable StateTest type tests production.
-    """
-
-
 @dataclass(kw_only=True)
 class BaseFixture:
     """
@@ -118,27 +99,52 @@ def fill_info(
         if ref_spec is not None:
             ref_spec.write_info(self.info)
 
-    def to_json(self) -> Dict[str, Any]:
+    @classmethod
+    @abstractmethod
+    def format(cls) -> FixtureFormats:
+        """
+        Returns the fixture format which the evm tool can use to determine how to verify the
+        fixture.
         """
-        Convert to JSON.
+        pass
 
-        TODO: This has to be replaced in the future to allow for different fixtures to do whatever
-        they want in the output file, and somehow be able to merge themselves automatically with
-        other fixtures of the same type.
+    @classmethod
+    @abstractmethod
+    def collect_into_file(cls, fixture_file_path: Path, fixtures: Dict[str, "BaseFixture"]):
+        """
+        Returns the name of the subdirectory where this type of fixture should be dumped to.
         """
         pass
 
+    @classmethod
+    @abstractmethod
+    def output_base_dir_name(cls) -> Path:
+        """
+        Returns the name of the subdirectory where this type of fixture should be dumped to.
+        """
+        pass
+
+    @classmethod
+    def output_file_extension(cls) -> str:
+        """
+        Returns the file extension for this type of fixture.
+
+        By default, fixtures are dumped as JSON files.
+        """
+        return ".json"
+
 
 @dataclass(kw_only=True)
 class BaseTest:
     """
-    Represents a base Ethereum test which must return a genesis and a
-    blockchain.
+    Represents a base Ethereum test which must return a single test fixture.
     """
 
     pre: Mapping
     tag: str = ""
-    base_test_config: BaseTestConfig = field(default_factory=BaseTestConfig)
+    # Setting a default here is just for type checking, the correct value is automatically set
+    # by pytest.
+    fixture_format: FixtureFormats = FixtureFormats.UNSET_TEST_FORMAT
 
     # Transition tool specific fields
     t8n_dump_dir: Optional[str] = ""
@@ -150,9 +156,9 @@ def generate(
         t8n: TransitionTool,
         fork: Fork,
         eips: Optional[List[int]] = None,
-    ) -> Optional[BaseFixture]:
+    ) -> BaseFixture:
         """
-        Generate the test fixture.
+        Generate the list of test fixtures.
         """
         pass
 
@@ -165,6 +171,23 @@ def pytest_parameter_name(cls) -> str:
         """
         pass
 
+    @classmethod
+    @abstractmethod
+    def fixture_formats(cls) -> List[FixtureFormats]:
+        """
+        Returns a list of fixture formats that can be output to the test spec.
+        """
+        pass
+
+    def __post_init__(self) -> None:
+        """
+        Validate the fixture format.
+        """
+        if self.fixture_format not in self.fixture_formats():
+            raise ValueError(
+                f"Invalid fixture format {self.fixture_format} for {self.__class__.__name__}."
+            )
+
     def get_next_transition_tool_output_path(self) -> str:
         """
         Returns the path to the next transition tool output file.