diff --git a/.claude/commands/write-test.md b/.claude/commands/write-test.md index 018317a7446..cbc8db3b601 100644 --- a/.claude/commands/write-test.md +++ b/.claude/commands/write-test.md @@ -61,6 +61,10 @@ Conventions and patterns for writing consensus tests. Run this skill before writ - `@pytest.mark.parametrize("name", [pytest.param(val, id="label"), ...])` with descriptive `id=` strings - Stack parametrize decorators for multiple dimensions +## Unit Tests (execution_testing package) + +Plain pytest. Tests are co-located with each module under `packages/testing/src/execution_testing/` in a sibling `tests/` directory. When adding a guardrail or validation, verify the tests fail without the change and pass with it. + ## After Writing Tests After writing or modifying tests, ask the user: "Would you like me to load the `/fill-tests` skill to verify the new tests fill correctly? (This loads an additional skill into context.)" If they agree, run `/fill-tests`, fill the new tests, then inspect the generated fixture JSON to verify the fixture contents match what the test intends. diff --git a/packages/testing/src/execution_testing/test_types/block_access_list/modifiers.py b/packages/testing/src/execution_testing/test_types/block_access_list/modifiers.py index 121ad26d13c..00903266251 100644 --- a/packages/testing/src/execution_testing/test_types/block_access_list/modifiers.py +++ b/packages/testing/src/execution_testing/test_types/block_access_list/modifiers.py @@ -28,10 +28,9 @@ def _remove_field_from_accounts( ) -> Callable[[BlockAccessList], BlockAccessList]: """Abstracted helper to remove a field from specified accounts.""" len_addresses = len(addresses) - found_addresses = set() def transform(bal: BlockAccessList) -> BlockAccessList: - nonlocal found_addresses + found_addresses: set[Address] = set() new_root = [] for account_change in bal.root: if account_change.address in addresses: @@ -70,10 +69,10 @@ def _modify_field_value( Abstracted helper to modify a field value for a specific account and transaction. """ - found_address = False def transform(bal: BlockAccessList) -> BlockAccessList: - nonlocal found_address + found_address = False + found_index = False new_root = [] for account_change in bal.root: if account_change.address == address: @@ -84,8 +83,10 @@ def transform(bal: BlockAccessList) -> BlockAccessList: if changes: if nested and slot is not None: # nested structure (storage) + found_slot = False for storage_slot in changes: if storage_slot.slot == slot: + found_slot = True for j, change in enumerate( storage_slot.slot_changes ): @@ -93,6 +94,7 @@ def transform(bal: BlockAccessList) -> BlockAccessList: change.block_access_index == block_access_index ): + found_index = True kwargs = { "block_access_index": ( block_access_index @@ -104,10 +106,16 @@ def transform(bal: BlockAccessList) -> BlockAccessList: ) break break + if not found_slot: + raise ValueError( + f"Storage slot {slot} not found in " + f"storage_changes of account {address}" + ) else: # flat structure (nonce, balance, code) for i, change in enumerate(changes): if change.block_access_index == block_access_index: + found_index = True kwargs = { "block_access_index": block_access_index, value_field: new_value, @@ -120,10 +128,14 @@ def transform(bal: BlockAccessList) -> BlockAccessList: new_root.append(account_change) if not found_address: - # sanity check that we actually found the address raise ValueError( f"Address {address} not found in BAL to modify {field_name}" ) + if not found_index: + raise ValueError( + f"Block access index {block_access_index} not found in " + f"{field_name} of account {address}" + ) return BlockAccessList(root=new_root) @@ -247,13 +259,12 @@ def swap_bal_indices( idx1: int, idx2: int ) -> Callable[[BlockAccessList], BlockAccessList]: """Swap block access indices throughout the BAL, modifying ordering.""" - nonce_indices = {idx1: False, idx2: False} - balance_indices = nonce_indices.copy() - storage_indices = nonce_indices.copy() - code_indices = nonce_indices.copy() def transform(bal: BlockAccessList) -> BlockAccessList: - nonlocal nonce_indices, balance_indices, storage_indices, code_indices + nonce_indices = {idx1: False, idx2: False} + balance_indices = nonce_indices.copy() + storage_indices = nonce_indices.copy() + code_indices = nonce_indices.copy() new_root = [] for account_change in bal.root: new_account = account_change.model_copy(deep=True) @@ -385,10 +396,8 @@ def append_change( else: raise TypeError(f"Unsupported change type: {type(change)}") - found_address = False - def transform(bal: BlockAccessList) -> BlockAccessList: - nonlocal found_address + found_address = False new_root = [] for account_change in bal.root: if account_change.address == account: @@ -427,10 +436,9 @@ def append_storage( slot_changes - If change provided and slot new: creates new BalStorageSlot """ - found_address = False def transform(bal: BlockAccessList) -> BlockAccessList: - nonlocal found_address + found_address = False new_root = [] for account_change in bal.root: if account_change.address == address: @@ -477,10 +485,9 @@ def duplicate_account( address: Address, ) -> Callable[[BlockAccessList], BlockAccessList]: """Duplicate an account entry in the BAL.""" - address_present = False def transform(bal: BlockAccessList) -> BlockAccessList: - nonlocal address_present + address_present = False new_root = [] for account_change in bal.root: new_root.append(account_change) @@ -514,7 +521,6 @@ def _duplicate_in_field( When sub_field and sub_match_fn are provided, find the parent entry via match_fn then duplicate within sub_field using sub_match_fn. """ - found = False def _copy(entry: Any) -> Any: if hasattr(entry, "model_copy"): @@ -522,7 +528,7 @@ def _copy(entry: Any) -> Any: return ZeroPaddedHexNumber(entry) def transform(bal: BlockAccessList) -> BlockAccessList: - nonlocal found + found = False new_root = [] for account_change in bal.root: if account_change.address == address: @@ -663,10 +669,9 @@ def insert_storage_read( Useful for testing that a key must not appear in both storage_changes and storage_reads. """ - found_address = False def transform(bal: BlockAccessList) -> BlockAccessList: - nonlocal found_address + found_address = False new_root = [] for account_change in bal.root: if account_change.address == address: @@ -721,8 +726,14 @@ def reorder_accounts( """Reorder accounts according to the provided index list.""" def transform(bal: BlockAccessList) -> BlockAccessList: - if len(indices) != len(bal.root): + n = len(bal.root) + if len(indices) != n: raise ValueError("Index list length must match number of accounts") + if sorted(indices) != list(range(n)): + raise ValueError( + f"Indices must be a valid permutation of 0..{n - 1}, " + f"got {indices}" + ) new_root = [bal.root[i] for i in indices] return BlockAccessList(root=new_root) diff --git a/packages/testing/src/execution_testing/test_types/tests/test_block_access_list_modifiers.py b/packages/testing/src/execution_testing/test_types/tests/test_block_access_list_modifiers.py index 50ca7d9dc83..4ac837be51d 100644 --- a/packages/testing/src/execution_testing/test_types/tests/test_block_access_list_modifiers.py +++ b/packages/testing/src/execution_testing/test_types/tests/test_block_access_list_modifiers.py @@ -1,5 +1,7 @@ """Unit tests for BAL modifier functions.""" +from typing import Callable + import pytest from execution_testing.base_types import Address @@ -13,6 +15,8 @@ BlockAccessList, ) from execution_testing.test_types.block_access_list.modifiers import ( + append_change, + append_storage, duplicate_account, duplicate_balance_change, duplicate_code_change, @@ -21,6 +25,13 @@ duplicate_storage_read, duplicate_storage_slot, insert_storage_read, + modify_balance, + modify_code, + modify_nonce, + modify_storage, + remove_nonces, + reorder_accounts, + swap_bal_indices, ) ALICE = Address(0xA) @@ -217,3 +228,143 @@ def test_insert_storage_read_missing_address_raises() -> None: bal = BlockAccessList([BalAccountChange(address=ALICE, nonce_changes=[])]) with pytest.raises(ValueError, match="not found"): insert_storage_read(CONTRACT, 1)(bal) + + +def test_modify_nonce_missing_index_raises( + sample_bal: BlockAccessList, +) -> None: + """Raise when the block_access_index is absent from nonce_changes.""" + with pytest.raises(ValueError, match="not found"): + modify_nonce(ALICE, 99, 42)(sample_bal) + + +def test_modify_balance_missing_index_raises( + sample_bal: BlockAccessList, +) -> None: + """Raise when the block_access_index is absent from balance_changes.""" + with pytest.raises(ValueError, match="not found"): + modify_balance(ALICE, 99, 9999)(sample_bal) + + +def test_modify_code_missing_index_raises(sample_bal: BlockAccessList) -> None: + """Raise when the block_access_index is absent from code_changes.""" + with pytest.raises(ValueError, match="not found"): + modify_code(ALICE, 99, b"\x00")(sample_bal) + + +def test_modify_storage_missing_index_raises( + sample_bal: BlockAccessList, +) -> None: + """Raise when block_access_index is absent within the storage slot.""" + with pytest.raises(ValueError, match="not found"): + modify_storage(CONTRACT, 99, 1, 0xFF)(sample_bal) + + +def test_modify_storage_missing_slot_raises( + sample_bal: BlockAccessList, +) -> None: + """Raise when the storage slot itself is absent.""" + with pytest.raises(ValueError, match="not found"): + modify_storage(CONTRACT, 1, 99, 0xFF)(sample_bal) + + +def test_modify_nonce_reused_callable_missing_index_still_raises() -> None: + """Raise even when the same modifier callable is reused across BALs.""" + modifier = modify_nonce(ALICE, 1, 42) + valid_bal = BlockAccessList( + [ + BalAccountChange( + address=ALICE, + nonce_changes=[ + BalNonceChange(block_access_index=1, post_nonce=1), + ], + ) + ] + ) + missing_index_bal = BlockAccessList( + [ + BalAccountChange( + address=ALICE, + nonce_changes=[], + ) + ] + ) + + modifier(valid_bal) + + with pytest.raises(ValueError, match="not found"): + modifier(missing_index_bal) + + +def test_reorder_accounts_duplicate_index_raises( + sample_bal: BlockAccessList, +) -> None: + """Raise when indices contain duplicates (not a valid permutation).""" + with pytest.raises(ValueError, match="valid permutation"): + reorder_accounts([0, 0])(sample_bal) + + +def test_reorder_accounts_out_of_range_raises( + sample_bal: BlockAccessList, +) -> None: + """Raise when indices are not a valid permutation (skipped index).""" + with pytest.raises(ValueError, match="valid permutation"): + reorder_accounts([0, 2])(sample_bal) + + +_EMPTY_BAL = BlockAccessList([]) +_ALICE_ONLY_BAL = BlockAccessList( + [BalAccountChange(address=ALICE, nonce_changes=[])] +) + + +@pytest.mark.parametrize( + "modifier_factory, missing_bal", + [ + pytest.param( + lambda: remove_nonces(ALICE), _EMPTY_BAL, id="remove_nonces" + ), + pytest.param( + lambda: swap_bal_indices(1, 1), _EMPTY_BAL, id="swap_bal_indices" + ), + pytest.param( + lambda: append_change( + ALICE, BalNonceChange(block_access_index=2, post_nonce=5) + ), + _EMPTY_BAL, + id="append_change", + ), + pytest.param( + lambda: append_storage(CONTRACT, slot=7, read=True), + _EMPTY_BAL, + id="append_storage", + ), + pytest.param( + lambda: duplicate_account(ALICE), + _EMPTY_BAL, + id="duplicate_account", + ), + pytest.param( + lambda: duplicate_nonce_change(ALICE, 1), + _ALICE_ONLY_BAL, + id="duplicate_nonce_change", + ), + pytest.param( + lambda: insert_storage_read(CONTRACT, 99), + _EMPTY_BAL, + id="insert_storage_read", + ), + ], +) +def test_reused_callable_does_not_carry_found_state( + sample_bal: BlockAccessList, + modifier_factory: Callable[ + [], Callable[[BlockAccessList], BlockAccessList] + ], + missing_bal: BlockAccessList, +) -> None: + """A modifier's found-state must not persist across calls.""" + modifier = modifier_factory() + modifier(sample_bal) + with pytest.raises(ValueError, match="not found"): + modifier(missing_bal)