fix(ci): All slow unit tests, remove run_in_serial (#1870)

* fix(ci): Speed up slow unit tests by reusing t8n server

* refactor(fill): add flag to more tests

* fix(filler): flag

Author: marioevz

CLAUDE.md

@@ -67,8 +67,7 @@ uv run ruff check --fix src tests .github/scripts
 uv run mypy src tests .github/scripts
 
 # Framework unit tests
-uv run pytest -c pytest-framework.ini -n auto -m "not run_in_serial"
-uv run pytest -c pytest-framework.ini -m run_in_serial
+uv run pytest -c pytest-framework.ini -n auto
 
 # Run specific checks (fast checks)
 uvx --with=tox-uv tox -e lint,typecheck,spellcheck

pytest-framework.ini

@@ -5,8 +5,6 @@ python_files=
     test_*.py
 testpaths =
     src
-markers =
-    run_in_serial
 addopts = 
     -p pytester
     -p pytest_plugins.eels_resolver

src/cli/gentest/tests/test_cli.py

@@ -1,13 +1,10 @@
 """Tests for the gentest CLI command."""
 
-from tempfile import TemporaryDirectory
-
 import pytest
 from click.testing import CliRunner
 
 from cli.gentest.cli import generate
 from cli.gentest.test_context_providers import StateTestProvider
-from cli.pytest_commands.fill import fill
 from ethereum_test_base_types import Account
 from ethereum_test_tools import Environment, Storage, Transaction
 
@@ -92,14 +89,18 @@ def transaction_hash(tx_type: int) -> str:  # noqa: D103
 
 
 @pytest.mark.parametrize("tx_type", list(transactions_by_type.keys()))
-def test_tx_type(tmp_path, monkeypatch, tx_type, transaction_hash):
+def test_tx_type(pytester, tmp_path, monkeypatch, tx_type, transaction_hash, default_t8n):
     """Generates a test case for any transaction type."""
     ## Arrange ##
     # This test is run in a CI environment, where connection to a node could be
     # unreliable. Therefore, we mock the RPC request to avoid any network issues.
     # This is done by patching the `get_context` method of the `StateTestProvider`.
     runner = CliRunner()
-    output_file = str(tmp_path / f"gentest_type_{tx_type}.py")
+    tmp_path_tests = tmp_path / "tests"
+    tmp_path_tests.mkdir()
+    tmp_path_output = tmp_path / "output"
+    tmp_path_output.mkdir()
+    generated_py_file = str(tmp_path_tests / f"gentest_type_{tx_type}.py")
 
     tx = transactions_by_type[tx_type]
 
@@ -109,17 +110,21 @@ def get_mock_context(self: StateTestProvider) -> dict:
     monkeypatch.setattr(StateTestProvider, "get_context", get_mock_context)
 
     ## Generate ##
-    gentest_result = runner.invoke(generate, [transaction_hash, output_file])
+    gentest_result = runner.invoke(generate, [transaction_hash, generated_py_file])
     assert gentest_result.exit_code == 0
 
     ## Fill ##
+    with open(generated_py_file, "r") as f:
+        pytester.makepyfile(f.read())
+    pytester.copy_example(name="pytest.ini")
+
     args = [
-        "-c",
-        "pytest.ini",
-        "--skip-evm-dump",
-        "--output",
-        TemporaryDirectory().name,
-        output_file,
+        "-m",
+        "state_test",
+        "--fork",
+        "Cancun",
+        "--t8n-server-url",
+        default_t8n.server_url,
     ]
-    fill_result = runner.invoke(fill, args)
-    assert fill_result.exit_code == 0, f"Fill command failed:\n{fill_result.output}"
+    result = pytester.runpytest("-v", *args)
+    assert result.ret == pytest.ExitCode.OK, f"Fill command failed:\n{result}"

src/cli/tests/test_pytest_fill_command.py

@@ -44,20 +44,25 @@ def test_fill_with_invalid_option(runner):
     assert "unrecognized arguments" in result.output
 
 
-@pytest.mark.run_in_serial
 class TestHtmlReportFlags:
     """Test html report generation and output options."""
 
     @pytest.fixture
-    def fill_args(self):
+    def fill_args(self, default_t8n):
         """
         Provide default arguments for the `fill` command when testing html report
         generation.
 
         Specifies a single existing example test case for faster fill execution,
         and to allow for tests to check for the fixture generation location.
         """
-        return ["-k", "test_dup and state_test-DUP16", "--fork", "Frontier"]
+        return [
+            "-k",
+            "test_dup and state_test-DUP16",
+            "--fork",
+            "Frontier",
+            f"--t8n-server-url={default_t8n.server_url}",
+        ]
 
     @pytest.fixture()
     def default_html_report_file_path(self):

src/conftest.py

@@ -7,15 +7,6 @@
 
 from ethereum_clis import BesuTransitionTool, ExecutionSpecsTransitionTool, TransitionTool
 
-
-def pytest_runtest_setup(item):
-    """Skip tests if running with pytest-xdist in parallel."""
-    marker = item.get_closest_marker(name="run_in_serial")
-    if marker is not None:
-        if os.getenv("PYTEST_XDIST_WORKER_COUNT") not in [None, "1"]:
-            pytest.skip("Skipping test because pytest-xdist is running with more than one worker.")
-
-
 DEFAULT_TRANSITION_TOOL_FOR_UNIT_TESTS = ExecutionSpecsTransitionTool
 
 INSTALLED_TRANSITION_TOOLS = [
@@ -37,7 +28,9 @@ def installed_transition_tool_instances() -> Generator[
     instances: Dict[str, TransitionTool | Exception] = {}
     for transition_tool_class in INSTALLED_TRANSITION_TOOLS:
         try:
-            instances[transition_tool_class.__name__] = transition_tool_class()
+            transition_tool_instance = transition_tool_class()
+            transition_tool_instance.start_server()
+            instances[transition_tool_class.__name__] = transition_tool_instance
         except Exception as e:
             # Record the exception in order to provide context when failing the appropriate test
             instances[transition_tool_class.__name__] = e

src/ethereum_clis/clis/execution_specs.py

@@ -46,12 +46,14 @@ class ExecutionSpecsTransitionTool(TransitionTool):
     detect_binary_pattern = re.compile(r"^ethereum-spec-evm-resolver\b")
     t8n_use_server: bool = True
     server_dir: Optional[TemporaryDirectory] = None
+    server_url: str | None = None
 
     def __init__(
         self,
         *,
         binary: Optional[Path] = None,
         trace: bool = False,
+        server_url: str | None = None,
     ):
         """Initialize the Ethereum Specs EVM Resolver Transition Tool interface."""
         os.environ.setdefault("NO_PROXY", "*")  # Disable proxy for local connections
@@ -71,6 +73,7 @@ def __init__(
                 f"Unexpected exception calling ethereum-spec-evm-resolver: {e}."
             ) from e
         self.help_string = result.stdout
+        self.server_url = server_url
 
     def start_server(self):
         """

src/ethereum_clis/transition_tool.py

@@ -62,7 +62,7 @@ class TransitionTool(EthereumCLI):
     cached_version: Optional[str] = None
     t8n_use_stream: bool = False
     t8n_use_server: bool = False
-    server_url: str
+    server_url: str | None = None
     process: Optional[subprocess.Popen] = None
 
     @abstractmethod
@@ -555,7 +555,7 @@ def evaluate(
         can be overridden.
         """
         if self.t8n_use_server:
-            if not self.process:
+            if not self.server_url:
                 self.start_server()
             return self._evaluate_server(
                 t8n_data=transition_tool_data,

src/pytest_plugins/filler/filler.py

@@ -128,6 +128,17 @@ def pytest_addoption(parser: pytest.Parser):
             " Default: `ethereum-spec-evm-resolver`."
         ),
     )
+    evm_group.addoption(
+        "--t8n-server-url",
+        action="store",
+        dest="t8n_server_url",
+        type=str,
+        default=None,
+        help=(
+            "[INTERNAL USE ONLY] URL of the t8n server to use. Used by framework tests/ci; not "
+            "intended for regular CLI use."
+        ),
+    )
     evm_group.addoption(
         "--traces",
         action="store_true",
@@ -557,18 +568,27 @@ def verify_fixtures_bin(request: pytest.FixtureRequest) -> Path | None:
     return request.config.getoption("verify_fixtures_bin")
 
 
+@pytest.fixture(autouse=True, scope="session")
+def t8n_server_url(request: pytest.FixtureRequest) -> str | None:
+    """Return configured t8n server url."""
+    return request.config.getoption("t8n_server_url")
+
+
 @pytest.fixture(autouse=True, scope="session")
 def t8n(
-    request: pytest.FixtureRequest, evm_bin: Path | None
+    request: pytest.FixtureRequest, evm_bin: Path | None, t8n_server_url: str | None
 ) -> Generator[TransitionTool, None, None]:
     """Return configured transition tool."""
+    kwargs = {
+        "trace": request.config.getoption("evm_collect_traces"),
+    }
+    if t8n_server_url is not None:
+        kwargs["server_url"] = t8n_server_url
     if evm_bin is None:
         assert TransitionTool.default_tool is not None, "No default transition tool found"
-        t8n = TransitionTool.default_tool(trace=request.config.getoption("evm_collect_traces"))
+        t8n = TransitionTool.default_tool(**kwargs)
     else:
-        t8n = TransitionTool.from_binary_path(
-            binary_path=evm_bin, trace=request.config.getoption("evm_collect_traces")
-        )
+        t8n = TransitionTool.from_binary_path(binary_path=evm_bin, **kwargs)
     if not t8n.exception_mapper.reliable:
         warnings.warn(
             f"The t8n tool that is currently being used to fill tests ({t8n.__class__.__name__}) "

src/pytest_plugins/filler/tests/test_filler.py

@@ -78,7 +78,6 @@ def test_shanghai_two(state_test, x):
 total_test_count = test_count_paris + test_count_shanghai
 
 
-@pytest.mark.run_in_serial
 @pytest.mark.parametrize(
     "args, expected_fixture_files, expected_fixture_counts",
     [
@@ -513,7 +512,11 @@ def test_shanghai_two(state_test, x):
     ],
 )
 def test_fixture_output_based_on_command_line_args(
-    testdir, args, expected_fixture_files, expected_fixture_counts
+    testdir,
+    args,
+    expected_fixture_files,
+    expected_fixture_counts,
+    default_t8n,
 ):
     """
     Test:
@@ -548,6 +551,8 @@ def test_fixture_output_based_on_command_line_args(
     testdir.copy_example(name="pytest.ini")
     args.append("-v")
     args.append("--no-html")
+    args.append("--t8n-server-url")
+    args.append(default_t8n.server_url)
 
     result = testdir.runpytest(*args)
     result.assert_outcomes(
@@ -600,7 +605,9 @@ def test_fixture_output_based_on_command_line_args(
 
     assert ini_file is not None, f"No {expected_ini_file} file was found in {meta_dir}"
     config = configparser.ConfigParser()
-    config.read(ini_file)
+    ini_file_text = ini_file.read_text()
+    ini_file_text = ini_file_text.replace(default_t8n.server_url, "t8n_server_path")
+    config.read_string(ini_file_text)
 
     if "--skip-index" not in args:
         assert index_file is not None, f"No {expected_index_file} file was found in {meta_dir}"
@@ -632,7 +639,6 @@ def test_max_gas_limit(state_test, pre, block_gas_limit):
 )
 
 
-@pytest.mark.run_in_serial
 @pytest.mark.parametrize(
     "args, expected_fixture_files, expected_fixture_counts, expected_gas_limit",
     [
@@ -661,7 +667,12 @@ def test_max_gas_limit(state_test, pre, block_gas_limit):
     ],
 )
 def test_fill_variables(
-    testdir, args, expected_fixture_files, expected_fixture_counts, expected_gas_limit
+    testdir,
+    args,
+    expected_fixture_files,
+    expected_fixture_counts,
+    expected_gas_limit,
+    default_t8n,
 ):
     """
     Test filling tests that depend on variables such as the max block gas limit.
@@ -679,6 +690,8 @@ def test_fill_variables(
     args.append("-m")
     args.append("state_test")
     args.append("--no-html")
+    args.append("--t8n-server-url")
+    args.append(default_t8n.server_url)
     result = testdir.runpytest(*args)
     result.assert_outcomes(
         passed=1,
@@ -730,7 +743,9 @@ def test_fill_variables(
 
     assert ini_file is not None, f"No {expected_ini_file} file was found in {meta_dir}"
     config = configparser.ConfigParser()
-    config.read(ini_file)
+    ini_file_text = ini_file.read_text()
+    ini_file_text = ini_file_text.replace(default_t8n.server_url, "t8n_server_path")
+    config.read_string(ini_file_text)
 
     if "--skip-index" not in args:
         assert index_file is not None, f"No {expected_index_file} file was found in {meta_dir}"

src/pytest_plugins/filler/tests/test_output_directory.py

@@ -30,7 +30,7 @@ def fill_fork_until() -> str:
 
 
 @pytest.fixture
-def run_fill(test_path: Path, fill_fork_from: str, fill_fork_until: str):
+def run_fill(test_path: Path, fill_fork_from: str, fill_fork_until: str, default_t8n):
     """Create a function to run the fill command with various output directory scenarios."""
 
     def _run_fill(output_dir: Path, clean: bool = False, expect_failure: bool = False):
@@ -44,6 +44,7 @@ def _run_fill(output_dir: Path, clean: bool = False, expect_failure: bool = Fals
             f"--from={fill_fork_from}",
             f"--until={fill_fork_until}",
             f"--output={str(output_dir)}",
+            f"--t8n-server-url={default_t8n.server_url}",
             str(test_path),
         ]