Skip to content

Commit 8eeda4a

Browse files
committed
Use verify_sync flag appropriately for sync tests:
- With `-m blockchain_test_sync_only`, only generate the sync fixtures - By default, fill blockchain, blockchain_engine, and IF verify_sync=True fill blockchain_engine_sync test.
1 parent 97a8a70 commit 8eeda4a

File tree

3 files changed

+64
-37
lines changed

3 files changed

+64
-37
lines changed

src/ethereum_test_specs/blockchain.py

Lines changed: 31 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
"""Ethereum blockchain test spec definition and filler."""
22

33
from pprint import pprint
4-
from typing import Any, Callable, ClassVar, Dict, Generator, List, Optional, Sequence, Tuple, Type
4+
from typing import Any, Callable, ClassVar, Dict, Generator, List, Sequence, Tuple, Type
55

66
import pytest
77
from pydantic import ConfigDict, Field, field_validator
@@ -404,6 +404,8 @@ class BlockchainTest(BaseTest):
404404
post: Alloc
405405
blocks: List[Block]
406406
genesis_environment: Environment = Field(default_factory=Environment)
407+
# If set to True, generate a BlockchainEngineSyncFixture to
408+
# verify syncing between clients.
407409
verify_sync: bool = False
408410
chain_id: int = 1
409411
exclude_full_post_state_in_output: bool = False
@@ -429,7 +431,7 @@ class BlockchainTest(BaseTest):
429431
supported_markers: ClassVar[Dict[str, str]] = {
430432
"blockchain_test_engine_only": "Only generate a blockchain test engine fixture",
431433
"blockchain_test_only": "Only generate a blockchain test fixture",
432-
"blockchain_engine_sync_test": "Generate a blockchain engine sync test fixture",
434+
"blockchain_test_sync_only": "Only generate a blockchain test sync fixture",
433435
}
434436

435437
@classmethod
@@ -446,12 +448,11 @@ def discard_fixture_format_by_marks(
446448
return fixture_format != BlockchainFixture
447449
if "blockchain_test_engine_only" in marker_names:
448450
return fixture_format != BlockchainEngineFixture
449-
if "blockchain_engine_sync_test" in marker_names:
450-
# Only generate sync fixture when marker is present
451-
return fixture_format != BlockchainEngineSyncFixture
452-
else:
453-
# When sync marker is NOT present, exclude sync fixture
454-
return fixture_format == BlockchainEngineSyncFixture
451+
# Note: Don't check for ``blockchain_test_sync_only`` here because the marker
452+
# is added dynamically for tests with ``verify_sync=True``. The ``generate()``
453+
# method handles skipping sync fixtures when ``verify_sync=False``.
454+
455+
return False
455456

456457
def get_genesis_environment(self, fork: Fork) -> Environment:
457458
"""Get the genesis environment for pre-allocation groups."""
@@ -764,27 +765,11 @@ def make_hive_fixture(
764765

765766
self.verify_post_state(t8n, t8n_state=alloc)
766767

767-
sync_payload: Optional[FixtureEngineNewPayload] = None
768-
# Only include sync_payload for BlockchainEngineSyncFixture
769-
if fixture_format == BlockchainEngineSyncFixture:
770-
# Test is marked for syncing verification.
771-
# Most clients require the header to start the sync process, so we create an empty
772-
# block on top of the last block of the test to send it as new payload and trigger
773-
# the sync process.
774-
sync_built_block = self.generate_block_data(
775-
t8n=t8n,
776-
fork=fork,
777-
block=Block(),
778-
previous_env=env,
779-
previous_alloc=alloc,
780-
last_block=False,
781-
)
782-
sync_payload = sync_built_block.get_fixture_engine_new_payload()
783-
784-
# Create base fixture data
768+
# Create base fixture data, common to all fixture formats
785769
fixture_data = {
786770
"fork": fork,
787771
"genesis": genesis.header,
772+
"payloads": fixture_payloads,
788773
"last_block_hash": head_hash,
789774
"post_state_hash": alloc.state_root()
790775
if self.exclude_full_post_state_in_output
@@ -802,19 +787,30 @@ def make_hive_fixture(
802787
# and prepare for state diff optimization
803788
fixture_data.update(
804789
{
805-
"payloads": fixture_payloads,
806-
"sync_payload": sync_payload,
807790
"post_state": alloc if not self.exclude_full_post_state_in_output else None,
808791
"pre_hash": "", # Will be set by BaseTestWrapper
809792
}
810793
)
811794
return BlockchainEngineXFixture(**fixture_data)
812795
elif fixture_format == BlockchainEngineSyncFixture:
813-
# For sync fixture format
796+
# Sync fixture format
797+
assert genesis.header.block_hash != head_hash, (
798+
"Invalid payload tests negative test via sync is not supported yet."
799+
)
800+
# Most clients require the header to start the sync process, so we create an empty
801+
# block on top of the last block of the test to send it as new payload and trigger the
802+
# sync process.
803+
sync_built_block = self.generate_block_data(
804+
t8n=t8n,
805+
fork=fork,
806+
block=Block(),
807+
previous_env=env,
808+
previous_alloc=alloc,
809+
last_block=False,
810+
)
814811
fixture_data.update(
815812
{
816-
"payloads": fixture_payloads,
817-
"sync_payload": sync_payload,
813+
"sync_payload": sync_built_block.get_fixture_engine_new_payload(),
818814
"pre": pre,
819815
"post_state": alloc if not self.exclude_full_post_state_in_output else None,
820816
}
@@ -824,8 +820,6 @@ def make_hive_fixture(
824820
# Standard engine fixture
825821
fixture_data.update(
826822
{
827-
"payloads": fixture_payloads,
828-
"sync_payload": sync_payload,
829823
"pre": pre,
830824
"post_state": alloc if not self.exclude_full_post_state_in_output else None,
831825
}
@@ -839,6 +833,10 @@ def generate(
839833
fixture_format: FixtureFormat,
840834
) -> BaseFixture:
841835
"""Generate the BlockchainTest fixture."""
836+
# Skip sync fixture generation if verify_sync is False
837+
if fixture_format == BlockchainEngineSyncFixture and not self.verify_sync:
838+
raise pytest.skip("Skipping sync fixture for test without verify_sync=True")
839+
842840
t8n.reset_traces()
843841
if fixture_format in [
844842
BlockchainEngineFixture,

src/pytest_plugins/filler/filler.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
from ethereum_test_base_types import Account, Address, Alloc, ReferenceSpec
2727
from ethereum_test_fixtures import (
2828
BaseFixture,
29+
BlockchainEngineSyncFixture,
2930
BlockchainEngineXFixture,
3031
FixtureCollector,
3132
FixtureConsumer,
@@ -336,6 +337,11 @@ def pytest_configure(config):
336337
called before the pytest-html plugin's pytest_configure to ensure that
337338
it uses the modified `htmlpath` option.
338339
"""
340+
# Register custom markers
341+
config.addinivalue_line(
342+
"markers", "blockchain_test_sync_only: Only generate blockchain sync test fixtures"
343+
)
344+
339345
# Modify the block gas limit if specified.
340346
if config.getoption("block_gas_limit"):
341347
EnvironmentDefaults.gas_limit = config.getoption("block_gas_limit")
@@ -1113,7 +1119,13 @@ def pytest_collection_modifyitems(
11131119
11141120
These can't be handled in this plugins pytest_generate_tests() as the fork
11151121
parametrization occurs in the forks plugin.
1122+
1123+
Also dynamically adds custom markers to tests based on their parameters (e.g.,
1124+
`blockchain_test_sync_only` for tests that have `verify_sync=True` in their
1125+
`blockchain_test` parameters).
11161126
"""
1127+
import inspect
1128+
11171129
items_for_removal = []
11181130
for i, item in enumerate(items):
11191131
params: Dict[str, Any] | None = None
@@ -1133,7 +1145,28 @@ def pytest_collection_modifyitems(
11331145
if not fixture_format.supports_fork(fork):
11341146
items_for_removal.append(i)
11351147
continue
1148+
1149+
# Check if test has ``verify_sync=True`` and add marker if it does
1150+
# This needs to happen before we check ``discard_fixture_format_by_marks``
1151+
if hasattr(item, "function"):
1152+
try:
1153+
test_func = item.function
1154+
source = inspect.getsource(test_func)
1155+
if "verify_sync=True" in source:
1156+
# Add the marker so pytest can filter it with `-m`
1157+
item.add_marker(pytest.mark.blockchain_test_sync_only)
1158+
except Exception:
1159+
pass # If we can't inspect, just continue
1160+
11361161
markers = list(item.iter_markers())
1162+
# Check for ``-m blockchain_test_sync_only`` and only fill sync tests
1163+
if (
1164+
config.getoption("-m") == "blockchain_test_sync_only"
1165+
and "blockchain_test_sync_only" in [m.name for m in markers]
1166+
and fixture_format != BlockchainEngineSyncFixture
1167+
):
1168+
items_for_removal.append(i)
1169+
continue
11371170
if spec_type.discard_fixture_format_by_marks(fixture_format, fork, markers):
11381171
items_for_removal.append(i)
11391172
continue

tests/osaka/eip7934_block_rlp_limit/test_max_block_rlp_size.py

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -389,7 +389,6 @@ def _exact_size_transactions_impl(
389389
pytest.param(1, id="max_rlp_size_plus_1_byte", marks=pytest.mark.exception_test),
390390
],
391391
)
392-
@pytest.mark.blockchain_engine_sync_test
393392
def test_block_at_rlp_size_limit_boundary(
394393
blockchain_test: BlockchainTestFiller,
395394
pre: Alloc,
@@ -443,7 +442,6 @@ def test_block_at_rlp_size_limit_boundary(
443442

444443

445444
@pytest.mark.with_all_typed_transactions
446-
@pytest.mark.blockchain_engine_sync_test
447445
def test_block_rlp_size_at_limit_with_all_typed_transactions(
448446
blockchain_test: BlockchainTestFiller,
449447
pre: Alloc,
@@ -453,7 +451,6 @@ def test_block_rlp_size_at_limit_with_all_typed_transactions(
453451
block_size_limit: int,
454452
env: Environment,
455453
typed_transaction: Transaction,
456-
request: pytest.FixtureRequest,
457454
) -> None:
458455
"""Test the block RLP size limit with all transaction types."""
459456
transactions, gas_used = exact_size_transactions(
@@ -483,7 +480,6 @@ def test_block_rlp_size_at_limit_with_all_typed_transactions(
483480
)
484481

485482

486-
@pytest.mark.blockchain_engine_sync_test
487483
def test_block_at_rlp_limit_with_logs(
488484
blockchain_test: BlockchainTestFiller,
489485
pre: Alloc,

0 commit comments

Comments
 (0)