Skip to content

Commit 919fd3d

Browse files
committed
chore: Remove explicit fill flag for sync; add tests for sync
- ``fill`` doesn't work with ``-m blockchain_test_only`` nor for ``-m blockchain_test_engine_only``. Only ``consume`` works with this. Remove the logic that adds this to filler for blockchain sync tests. - Add tests for the sync marker to make sure it's behaving as expected. - Prevent fixture filling altogether for sync tests marked with exceptions. This makes it so we don't have ``skipped`` pytest tests for these if we can avoid it. We can't easily do this with just a basic ``verify_sync=False`` check but we CAN check whether the marker ``exception_test`` exists and skip filling if it does.
1 parent 2d11b62 commit 919fd3d

File tree

5 files changed

+159
-27
lines changed

5 files changed

+159
-27
lines changed

src/ethereum_test_fixtures/blockchain.py

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -580,7 +580,7 @@ class BlockchainEngineSyncFixture(BlockchainEngineFixtureCommon):
580580
- Both client types are parametrized from hive client config
581581
"""
582582

583-
format_name: ClassVar[str] = "blockchain_engine_sync_test"
583+
format_name: ClassVar[str] = "blockchain_test_sync"
584584
description: ClassVar[str] = (
585585
"Tests that generate a blockchain test fixture for Engine API testing with client sync."
586586
)
@@ -589,8 +589,3 @@ class BlockchainEngineSyncFixture(BlockchainEngineFixtureCommon):
589589
post_state: Alloc | None = Field(None)
590590
payloads: List[FixtureEngineNewPayload] = Field(..., alias="engineNewPayloads")
591591
sync_payload: FixtureEngineNewPayload | None = None
592-
593-
@classmethod
594-
def output_base_dir_name(cls) -> str:
595-
"""Return the proper output directory name for sync fixtures."""
596-
return "blockchain_engine_sync_tests"

src/ethereum_test_specs/blockchain.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -448,9 +448,10 @@ def discard_fixture_format_by_marks(
448448
return fixture_format != BlockchainFixture
449449
if "blockchain_test_engine_only" in marker_names:
450450
return fixture_format != BlockchainEngineFixture
451-
# Note: Don't check for ``blockchain_test_sync_only`` here because the marker
452-
# is added dynamically for tests that use ``verify_sync``. The ``generate()``
453-
# method handles skipping sync fixtures when ``verify_sync=False``.
451+
452+
# Exception tests cannot be used for sync testing
453+
if "exception_test" in marker_names and fixture_format == BlockchainEngineSyncFixture:
454+
return True
454455

455456
return False
456457

src/pytest_plugins/consume/simulators/sync/conftest.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
"""
2-
Pytest fixtures for the `consume engine_sync` simulator.
2+
Pytest fixtures for the `consume sync` simulator.
33
44
Configures the hive back-end & EL clients for each individual test execution.
55
"""

src/pytest_plugins/filler/filler.py

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88

99
import configparser
1010
import datetime
11-
import inspect
1211
import os
1312
import warnings
1413
from enum import Enum
@@ -27,7 +26,6 @@
2726
from ethereum_test_base_types import Account, Address, Alloc, ReferenceSpec
2827
from ethereum_test_fixtures import (
2928
BaseFixture,
30-
BlockchainEngineSyncFixture,
3129
BlockchainEngineXFixture,
3230
FixtureCollector,
3331
FixtureConsumer,
@@ -339,10 +337,6 @@ def pytest_configure(config):
339337
it uses the modified `htmlpath` option.
340338
"""
341339
# Register custom markers
342-
config.addinivalue_line(
343-
"markers", "blockchain_test_sync_only: Only generate blockchain sync test fixtures"
344-
)
345-
346340
# Modify the block gas limit if specified.
347341
if config.getoption("block_gas_limit"):
348342
EnvironmentDefaults.gas_limit = config.getoption("block_gas_limit")
@@ -554,6 +548,7 @@ def pytest_runtest_makereport(item, call):
554548
"state_test",
555549
"blockchain_test",
556550
"blockchain_test_engine",
551+
"blockchain_test_sync",
557552
]:
558553
report.user_properties.append(("evm_dump_dir", item.config.evm_dump_dir))
559554
else:
@@ -1120,10 +1115,6 @@ def pytest_collection_modifyitems(
11201115
11211116
These can't be handled in this plugins pytest_generate_tests() as the fork
11221117
parametrization occurs in the forks plugin.
1123-
1124-
Also dynamically adds custom markers to tests based on their parameters (e.g.,
1125-
`blockchain_test_sync_only` for tests that have `verify_sync` in their
1126-
`blockchain_test` parameters).
11271118
"""
11281119
items_for_removal = []
11291120
for i, item in enumerate(items):
@@ -1145,13 +1136,6 @@ def pytest_collection_modifyitems(
11451136
items_for_removal.append(i)
11461137
continue
11471138

1148-
# Add ``blockchain_test_sync_only`` marker to blockchain fixture if it uses
1149-
# ``verify_sync``
1150-
if fixture_format == BlockchainEngineSyncFixture and isinstance(item, pytest.Function):
1151-
source = inspect.getsource(item.function)
1152-
if "verify_sync" in source:
1153-
item.add_marker(pytest.mark.blockchain_test_sync_only)
1154-
11551139
markers = list(item.iter_markers())
11561140
if spec_type.discard_fixture_format_by_marks(fixture_format, fork, markers):
11571141
items_for_removal.append(i)
Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
"""Test blockchain sync fixture generation with verify_sync parameter."""
2+
3+
import textwrap
4+
5+
from ethereum_clis import TransitionTool
6+
7+
test_module_with_sync = textwrap.dedent(
8+
"""\
9+
import pytest
10+
from ethereum_test_tools import (
11+
Account,
12+
BlockException,
13+
Block,
14+
Environment,
15+
Header,
16+
TestAddress,
17+
Transaction,
18+
)
19+
20+
TEST_ADDRESS = Account(balance=1_000_000)
21+
22+
@pytest.mark.valid_at("Cancun")
23+
def test_sync_default(blockchain_test):
24+
# verify_sync defaults to False
25+
blockchain_test(
26+
pre={TestAddress: TEST_ADDRESS},
27+
post={},
28+
blocks=[Block(txs=[Transaction()])]
29+
)
30+
31+
@pytest.mark.valid_at("Cancun")
32+
def test_sync_true(blockchain_test):
33+
blockchain_test(
34+
verify_sync=True,
35+
pre={TestAddress: TEST_ADDRESS},
36+
post={},
37+
blocks=[Block(txs=[Transaction()])]
38+
)
39+
40+
@pytest.mark.valid_at("Cancun")
41+
def test_sync_false(blockchain_test):
42+
blockchain_test(
43+
verify_sync=False,
44+
pre={TestAddress: TEST_ADDRESS},
45+
post={},
46+
blocks=[Block(txs=[Transaction()])]
47+
)
48+
49+
@pytest.mark.valid_at("Cancun")
50+
@pytest.mark.parametrize("sync", [True, False])
51+
def test_sync_conditional(blockchain_test, sync):
52+
blockchain_test(
53+
pre={TestAddress: TEST_ADDRESS},
54+
post={},
55+
blocks=[Block(txs=[Transaction()])],
56+
verify_sync=sync
57+
)
58+
59+
@pytest.mark.valid_at("Cancun")
60+
@pytest.mark.parametrize(
61+
"has_exception",
62+
[
63+
pytest.param(False, id="no_exception"),
64+
pytest.param(
65+
True, id="with_exception", marks=pytest.mark.exception_test
66+
),
67+
]
68+
)
69+
def test_sync_with_exception(blockchain_test, has_exception):
70+
blockchain_test(
71+
pre={TestAddress: TEST_ADDRESS},
72+
post={},
73+
blocks=[
74+
Block(
75+
txs=[Transaction()],
76+
rlp_modifier=Header(gas_limit=0) if has_exception else None,
77+
exception=BlockException.INCORRECT_BLOCK_FORMAT if has_exception else None,
78+
)
79+
],
80+
verify_sync=not has_exception,
81+
)
82+
83+
"""
84+
)
85+
86+
87+
def test_blockchain_sync_marker(
88+
pytester,
89+
default_t8n: TransitionTool,
90+
):
91+
"""
92+
Test blockchain sync fixture generation with exception_test marker.
93+
94+
The test module has 5 test functions (7 test cases with parametrization):
95+
- test_sync_default: generates all formats except sync (verify_sync defaults to False)
96+
- test_sync_true: generates all formats including sync (verify_sync=True)
97+
- test_sync_false: generates all formats except sync (verify_sync=False)
98+
- test_sync_conditional: generates formats based on the sync parameter (2 cases)
99+
- test_sync_with_exception: tests exception_test marker behavior (2 cases)
100+
101+
Each test generates fixture formats:
102+
- BlockchainFixture (always)
103+
- BlockchainEngineFixture (always)
104+
- BlockchainEngineSyncFixture (only when verify_sync=True AND not marked with exception_test)
105+
106+
Expected outcomes:
107+
- 7 test cases total
108+
- Each generates BlockchainFixture (7) and BlockchainEngineFixture (7) = 14 fixtures
109+
- Sync fixtures:
110+
- test_sync_true: 1 sync fixture ✓
111+
- test_sync_conditional[True]: 1 sync fixture ✓
112+
- test_sync_with_exception[no_exception]: 1 sync fixture ✓
113+
- Total sync fixtures: 3
114+
- Skipped sync fixtures:
115+
- test_sync_default: 1 skipped
116+
- test_sync_false: 1 skipped
117+
- test_sync_conditional[False]: 1 skipped
118+
- Total skipped: 3
119+
- Not generated (due to exception_test marker):
120+
- test_sync_with_exception[with_exception]: sync fixture not generated at all
121+
122+
Final counts:
123+
- Passed: 14 (base fixtures) + 3 (sync fixtures) = 17 passed
124+
- Skipped: 3 skipped
125+
- Failed: 0 failed
126+
"""
127+
# Create proper directory structure for tests
128+
tests_dir = pytester.mkdir("tests")
129+
cancun_tests_dir = tests_dir / "cancun"
130+
cancun_tests_dir.mkdir()
131+
sync_test_dir = cancun_tests_dir / "sync_test_module"
132+
sync_test_dir.mkdir()
133+
test_module = sync_test_dir / "test_sync_marker.py"
134+
test_module.write_text(test_module_with_sync)
135+
136+
pytester.copy_example(name="src/cli/pytest_commands/pytest_ini_files/pytest-fill.ini")
137+
138+
# Add the test directory to the arguments
139+
args = [
140+
"-c",
141+
"pytest-fill.ini",
142+
"-v",
143+
"--no-html",
144+
"--t8n-server-url",
145+
default_t8n.server_url,
146+
"tests/cancun/sync_test_module/",
147+
]
148+
149+
expected_outcomes = {"passed": 17, "failed": 0, "skipped": 3, "errors": 0}
150+
151+
result = pytester.runpytest(*args)
152+
result.assert_outcomes(**expected_outcomes)

0 commit comments

Comments
 (0)