Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Jul 8, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 0 additions & 2 deletions pytest-framework.ini
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@ python_files=
test_*.py
testpaths =
src
markers =
run_in_serial
addopts =
-p pytester
-p pytest_plugins.eels_resolver
Expand Down
33 changes: 19 additions & 14 deletions src/cli/gentest/tests/test_cli.py
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -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]

Expand All @@ -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}"
11 changes: 8 additions & 3 deletions src/cli/tests/test_pytest_fill_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
13 changes: 3 additions & 10 deletions src/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand All @@ -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
Expand Down
3 changes: 3 additions & 0 deletions src/ethereum_clis/clis/execution_specs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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):
"""
Expand Down
4 changes: 2 additions & 2 deletions src/ethereum_clis/transition_tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
27 changes: 22 additions & 5 deletions src/pytest_plugins/filler/filler.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,14 @@ 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="URL of the t8n server to use.",
)
evm_group.addoption(
"--traces",
action="store_true",
Expand Down Expand Up @@ -557,18 +565,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__}) "
Expand Down
27 changes: 21 additions & 6 deletions src/pytest_plugins/filler/tests/test_filler.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
[
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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}"
Expand Down Expand Up @@ -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",
[
Expand Down Expand Up @@ -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.
Expand All @@ -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,
Expand Down Expand Up @@ -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}"
Expand Down
3 changes: 2 additions & 1 deletion src/pytest_plugins/filler/tests/test_output_directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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),
]

Expand Down
3 changes: 1 addition & 2 deletions tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,7 @@ extras =
test
lint # Required `gentest` for formatting tests
commands =
pytest -c ./pytest-framework.ini -n auto -m "not run_in_serial"
pytest -c ./pytest-framework.ini -m run_in_serial
pytest -c ./pytest-framework.ini -n auto


[forks]
Expand Down
Loading