refactor(ethereum_clis): move generic code from TransitionTool to `…

…EthereumCLI`#894

.flake8

@@ -23,11 +23,11 @@ extend-ignore = E203, D107, D200, D203, D205,
     # Ignore N806: Variable names with all caps (ALL_CAPS)
 max-line-length = 99
 per-file-ignore =
-    tests/evm_transition_tool/test_evaluate.py:E501
+    tests/ethereum_clis/test_evaluate.py:E501
 
 extend-exclude =
     setup.py
-    src/evm_transition_tool/tests/
+    src/ethereum_clis/tests/
     src/ethereum_test_tools/tests/
     src/ethereum_test_forks/tests/
 

docs/CHANGELOG.md

@@ -51,6 +51,7 @@ Test fixtures for use by clients are available for each release on the [Github r
 ### 🔧 EVM Tools
 
 - ✨ Fill test fixtures using EELS by default. EEST now uses the [`ethereum-specs-evm-resolver`](https://github.com/petertdavies/ethereum-spec-evm-resolver) with the EELS daemon ([#792](https://github.com/ethereum/execution-spec-tests/pull/792)).
+- 🔀 Move the `evm_transition_tool` package to `ethereum_clis` and derive the transition tool CL interfaces from a shared `EthereumCLI` class that can be reused for other sub-commands ([#894](https://github.com/ethereum/execution-spec-tests/pull/894)).
 
 ### 📋 Misc
 

docs/library/ethereum_clis.md

@@ -0,0 +1,3 @@
+# Ethereum CLIs Package
+
+::: ethereum_clis

docs/library/index.md

@@ -10,5 +10,5 @@ Execution spec tests consists of several packages that implement helper classes
 - [`ethereum_test_tools`](./ethereum_test_tools.md) - provides primitives and helpers to test Ethereum execution clients.
 - [`ethereum_test_types`](./ethereum_test_types.md) - provides Ethereum types built on top of the base types which are used to define test cases and interact with other libraries.
 - [`ethereum_test_vm`](./ethereum_test_vm.md) - provides definitions for the Ethereum Virtual Machine (EVM) as used to define bytecode in test cases.
-- [`evm_transition_tool`](./evm_transition_tool.md) - a wrapper for the transition (`t8n`) tool.
+- [`ethereum_clis`](./ethereum_clis.md) - a wrapper for the transition (`t8n`) tool.
 - [`pytest_plugins`](./pytest_plugins/index.md) - contains pytest customizations that provide additional functionality for generating test fixtures.

docs/navigation.md

@@ -36,7 +36,7 @@
     * [Running Github Actions Locally](dev/test_actions_locally.md)
   * [Changelog](CHANGELOG.md)
   * [Library Reference](library/index.md)
-    * [Miscellaneous CLI Tools](library/cli/index.md)
+    * [EEST CLI Tools](library/cli/index.md)
     * [Ethereum Test Base Types Package](library/ethereum_test_base_types.md)
     * [Ethereum Test Exceptions Package](library/ethereum_test_exceptions.md)
     * [Ethereum Test Fixtures Package](library/ethereum_test_fixtures.md)
@@ -45,5 +45,5 @@
     * [Ethereum Test Tools Package](library/ethereum_test_tools.md)
     * [Ethereum Test Types Package](library/ethereum_test_types.md)
     * [Ethereum Test VM Package](library/ethereum_test_vm.md)
-    * [EVM Transition Tool Package](library/evm_transition_tool.md)
+    * [Ethereum CLIs Package](library/ethereum_clis.md)
     * [Pytest Plugins](library/pytest_plugins/index.md)

src/evm_transition_tool/__init__.py → src/ethereum_clis/__init__.py

@@ -2,13 +2,14 @@
 Library of Python wrappers for the different implementations of transition tools.
 """
 
-from .besu import BesuTransitionTool
-from .ethereumjs import EthereumJSTransitionTool
-from .evmone import EvmOneTransitionTool
-from .execution_specs import ExecutionSpecsTransitionTool
-from .geth import GethTransitionTool
-from .nimbus import NimbusTransitionTool
-from .transition_tool import TransitionTool, TransitionToolNotFoundInPath, UnknownTransitionTool
+from .clis.besu import BesuTransitionTool
+from .clis.ethereumjs import EthereumJSTransitionTool
+from .clis.evmone import EvmOneTransitionTool
+from .clis.execution_specs import ExecutionSpecsTransitionTool
+from .clis.geth import GethTransitionTool
+from .clis.nimbus import NimbusTransitionTool
+from .ethereum_cli import CLINotFoundInPath, UnknownCLI
+from .transition_tool import TransitionTool
 from .types import Result, TransitionToolOutput
 
 TransitionTool.set_default_tool(ExecutionSpecsTransitionTool)
@@ -23,6 +24,6 @@
     "Result",
     "TransitionTool",
     "TransitionToolOutput",
-    "TransitionToolNotFoundInPath",
-    "UnknownTransitionTool",
+    "CLINotFoundInPath",
+    "UnknownCLI",
 )

src/ethereum_clis/clis/__init__.py

@@ -0,0 +1,3 @@
+"""
+Package containing concrete implementations of Ethereum CL interfaces.
+"""

src/evm_transition_tool/besu.py → src/ethereum_clis/clis/besu.py

@@ -17,8 +17,8 @@
 from ethereum_test_forks import Fork
 from ethereum_test_types import Alloc, Environment, Transaction
 
-from .transition_tool import TransitionTool, dump_files_to_directory, model_dump_config
-from .types import TransitionToolInput, TransitionToolOutput
+from ..transition_tool import TransitionTool, dump_files_to_directory, model_dump_config
+from ..types import TransitionToolInput, TransitionToolOutput
 
 
 class BesuTransitionTool(TransitionTool):

src/evm_transition_tool/ethereumjs.py → src/ethereum_clis/clis/ethereumjs.py

@@ -1,13 +1,14 @@
 """
 EthereumJS Transition tool interface.
 """
+
 from pathlib import Path
 from re import compile
 from typing import Optional
 
 from ethereum_test_forks import Fork
 
-from .transition_tool import TransitionTool
+from ..transition_tool import TransitionTool
 
 
 class EthereumJSTransitionTool(TransitionTool):

src/evm_transition_tool/evmone.py → src/ethereum_clis/clis/evmone.py

@@ -1,13 +1,14 @@
 """
 Evmone Transition tool interface.
 """
+
 from pathlib import Path
 from re import compile
 from typing import Optional
 
 from ethereum_test_forks import Fork
 
-from .transition_tool import TransitionTool
+from ..transition_tool import TransitionTool
 
 
 class EvmOneTransitionTool(TransitionTool):

src/evm_transition_tool/execution_specs.py → src/ethereum_clis/clis/execution_specs.py

@@ -13,7 +13,7 @@
 
 from ethereum_test_forks import Fork
 
-from .transition_tool import TransitionTool
+from ..transition_tool import TransitionTool
 
 DAEMON_STARTUP_TIMEOUT_SECONDS = 5
 

src/evm_transition_tool/geth.py → src/ethereum_clis/clis/geth.py

@@ -13,7 +13,7 @@
 from ethereum_test_fixtures import BlockchainFixture, StateFixture
 from ethereum_test_forks import Fork
 
-from .transition_tool import FixtureFormat, TransitionTool, dump_files_to_directory
+from ..transition_tool import FixtureFormat, TransitionTool, dump_files_to_directory
 
 
 class GethTransitionTool(TransitionTool):

src/evm_transition_tool/nimbus.py → src/ethereum_clis/clis/nimbus.py

@@ -10,7 +10,7 @@
 
 from ethereum_test_forks import Fork
 
-from .transition_tool import TransitionTool
+from ..transition_tool import TransitionTool
 
 
 class NimbusTransitionTool(TransitionTool):

src/ethereum_clis/ethereum_cli.py

@@ -0,0 +1,155 @@
+"""
+Abstract base class to help create Python interfaces to Ethereum CLIs.
+"""
+
+import os
+import shutil
+import subprocess
+from abc import ABC, abstractmethod
+from itertools import groupby
+from pathlib import Path
+from re import Pattern
+from typing import Any, List, Optional, Type
+
+
+class UnknownCLI(Exception):
+    """Exception raised if an unknown CLI is encountered"""
+
+    pass
+
+
+class CLINotFoundInPath(Exception):
+    """Exception raised if the specified CLI binary is not found in the path"""
+
+    def __init__(self, message="The CLI binary was not found in the path", binary=None):
+        if binary:
+            message = f"{message} ({binary})"
+        super().__init__(message)
+
+
+class EthereumCLI(ABC):
+    """
+    Abstract base class to help create Python interfaces to Ethereum CLIs.
+
+    This base class helps handle the special case of EVM subcommands, such as
+    the EVM transition tool `t8n`, which have multiple implementations, one
+    from each client team. In the case of these tools, this class mainly serves
+    to help instantiate the correct subclass based on the output of the CLI's
+    version flag.
+    """
+
+    registered_tools: List[Type[Any]] = []
+    default_tool: Optional[Type[Any]] = None
+    default_binary: Path
+    detect_binary_pattern: Pattern
+    version_flag: str = "-v"
+    cached_version: Optional[str] = None
+
+    @abstractmethod
+    def __init__(self, *, binary: Optional[Path] = None, trace: bool = False):
+        """
+        Abstract initialization method that all subclasses must implement.
+        """
+        if binary is None:
+            binary = self.default_binary
+        else:
+            # improve behavior of which by resolving the path: ~/relative paths don't work
+            resolved_path = Path(os.path.expanduser(binary)).resolve()
+            if resolved_path.exists():
+                binary = resolved_path
+        binary = shutil.which(binary)  # type: ignore
+        if not binary:
+            raise CLINotFoundInPath(binary=binary)
+        self.binary = Path(binary)
+        self.trace = trace
+
+    @classmethod
+    def register_tool(cls, tool_subclass: Type[Any]):
+        """
+        Registers a given subclass as tool option.
+        """
+        cls.registered_tools.append(tool_subclass)  # raise NotImplementedError
+
+    @classmethod
+    def set_default_tool(cls, tool_subclass: Type[Any]):
+        """
+        Registers the default tool subclass.
+        """
+        cls.default_tool = tool_subclass
+
+    @classmethod
+    def from_binary_path(cls, *, binary_path: Optional[Path], **kwargs) -> Any:
+        """
+        Instantiates the appropriate CLI subclass derived from the CLI's `binary_path`.
+
+        This method will attempt to detect the CLI version and instantiate the appropriate
+        subclass based on the version output by running hte CLI with the version flag.
+        """
+        assert cls.default_tool is not None, "default CLI implementation was never set"
+
+        if binary_path is None:
+            return cls.default_tool(binary=binary_path, **kwargs)
+
+        resolved_path = Path(os.path.expanduser(binary_path)).resolve()
+        if resolved_path.exists():
+            binary_path = resolved_path
+        binary = shutil.which(binary_path)  # type: ignore
+
+        if not binary:
+            raise CLINotFoundInPath(binary=binary)
+
+        binary = Path(binary)
+
+        # Group the tools by version flag, so we only have to call the tool once for all the
+        # classes that share the same version flag
+        for version_flag, subclasses in groupby(
+            cls.registered_tools, key=lambda x: x.version_flag
+        ):
+            try:
+                result = subprocess.run(
+                    [binary, version_flag], stdout=subprocess.PIPE, stderr=subprocess.PIPE
+                )
+                if result.returncode != 0:
+                    raise Exception(f"Non-zero return code: {result.returncode}")
+
+                if result.stderr:
+                    raise Exception(f"Tool wrote to stderr: {result.stderr.decode()}")
+
+                binary_output = ""
+                if result.stdout:
+                    binary_output = result.stdout.decode().strip()
+            except Exception:
+                # If the tool doesn't support the version flag,
+                # we'll get an non-zero exit code.
+                continue
+            for subclass in subclasses:
+                if subclass.detect_binary(binary_output):
+                    return subclass(binary=binary, **kwargs)
+
+        raise UnknownCLI(f"Unknown CLI: {binary_path}")
+
+    @classmethod
+    def detect_binary(cls, binary_output: str) -> bool:
+        """
+        Returns True if a CLI's `binary_output` matches the class's expected output.
+        """
+        assert cls.detect_binary_pattern is not None
+
+        return cls.detect_binary_pattern.match(binary_output) is not None
+
+    def version(self) -> str:
+        """
+        Returns the name and version of the CLI as reported by the CLI's version flag.
+        """
+        if self.cached_version is None:
+            result = subprocess.run(
+                [str(self.binary), self.version_flag],
+                stdout=subprocess.PIPE,
+            )
+
+            if result.returncode != 0:
+                raise Exception("failed to evaluate: " + result.stderr.decode())
+
+            self.cached_version = result.stdout.decode().strip()
+
+        return self.cached_version

src/evm_transition_tool/tests/test_evaluate.py → src/ethereum_clis/tests/test_evaluate.py

@@ -8,12 +8,12 @@
 import pytest
 from pydantic import TypeAdapter
 
+from ethereum_clis import ExecutionSpecsTransitionTool, TransitionTool
 from ethereum_test_base_types import to_json
 from ethereum_test_forks import Berlin, Fork, Istanbul, London
 from ethereum_test_types import Alloc, Environment, Transaction
-from evm_transition_tool import ExecutionSpecsTransitionTool, TransitionTool
 
-FIXTURES_ROOT = Path(os.path.join("src", "evm_transition_tool", "tests", "fixtures"))
+FIXTURES_ROOT = Path(os.path.join("src", "ethereum_clis", "tests", "fixtures"))
 DEFAULT_EVM_T8N_BINARY_NAME = "ethereum-spec-evm-resolver"
 
 

src/evm_transition_tool/tests/test_transition_tool.py → src/ethereum_clis/tests/test_transition_tool.py

@@ -9,13 +9,13 @@
 
 import pytest
 
-from evm_transition_tool import (
+from ethereum_clis import (
+    CLINotFoundInPath,
     EvmOneTransitionTool,
     ExecutionSpecsTransitionTool,
     GethTransitionTool,
     NimbusTransitionTool,
     TransitionTool,
-    TransitionToolNotFoundInPath,
 )
 
 
@@ -86,8 +86,8 @@ def mock_run(args, **kwargs):
 
 def test_unknown_binary_path():
     """
-    Test that `from_binary_path` raises `UnknownTransitionTool` for unknown
+    Test that `from_binary_path` raises `UnknownCLI` for unknown
     binary paths.
     """
-    with pytest.raises(TransitionToolNotFoundInPath):
+    with pytest.raises(CLINotFoundInPath):
         TransitionTool.from_binary_path(binary_path=Path("unknown_binary_path"))