From 381fd43f51246f4e43f56bd3c81dcd19ce0bc9b5 Mon Sep 17 00:00:00 2001 From: Mario Vega Date: Wed, 9 Apr 2025 20:52:06 +0000 Subject: [PATCH 1/8] feat(specs): Introduce/enforce `negative` marker --- src/ethereum_test_specs/blockchain.py | 43 ++++++++++++++++++++--- src/ethereum_test_specs/helpers.py | 7 ++++ src/ethereum_test_specs/state.py | 12 ++++++- src/ethereum_test_specs/transaction.py | 11 ++++++ src/pytest_plugins/shared/execute_fill.py | 4 +++ 5 files changed, 72 insertions(+), 5 deletions(-) diff --git a/src/ethereum_test_specs/blockchain.py b/src/ethereum_test_specs/blockchain.py index f7bcd60fbdd..67499fc798b 100644 --- a/src/ethereum_test_specs/blockchain.py +++ b/src/ethereum_test_specs/blockchain.py @@ -49,7 +49,7 @@ from .base import BaseTest, verify_result from .debugging import print_traces -from .helpers import is_slow_test, verify_block, verify_transactions +from .helpers import is_negative_test, is_slow_test, verify_block, verify_transactions def environment_from_parent_header(parent: "FixtureHeader") -> "Environment": @@ -563,6 +563,7 @@ def make_fixture( fork: Fork, eips: Optional[List[int]] = None, slow: bool = False, + negative_test: bool = False, ) -> BlockchainFixture: """Create a fixture from the blockchain test definition.""" fixture_blocks: List[FixtureBlock | InvalidFixtureBlock] = [] @@ -572,7 +573,7 @@ def make_fixture( alloc = pre env = environment_from_parent_header(genesis.header) head = genesis.header.block_hash - + invalid_blocks = 0 for block in self.blocks: if block.rlp is None: # This is the most common case, the RLP needs to be constructed @@ -605,6 +606,10 @@ def make_fixture( env = apply_new_parent(new_env, header) head = header.block_hash else: + assert negative_test, ( + "unmarked negative test: the test case produces an invalid block but " + + "does not specify the `pytest.mark.negative` marker" + ) fixture_blocks.append( InvalidFixtureBlock( rlp=fixture_block.rlp, @@ -616,23 +621,35 @@ def make_fixture( ), ), ) + invalid_blocks += 1 else: assert block.exception is not None, ( "test correctness: if the block's rlp is hard-coded, " + "the block is expected to produce an exception" ) + assert negative_test, ( + "unmarked negative test: the test case produces an invalid block but " + + "does not specify the `pytest.mark.negative` marker" + ) fixture_blocks.append( InvalidFixtureBlock( rlp=block.rlp, expect_exception=block.exception, ), ) + invalid_blocks += 1 if block.expected_post_state: self.verify_post_state( t8n, t8n_state=alloc, expected_state=block.expected_post_state ) + if invalid_blocks == 0 and negative_test: + raise Exception( + "test correctness: the test case is marked as negative but " + + "no invalid blocks were found" + ) + self.verify_post_state(t8n, t8n_state=alloc) network_info = BlockchainTest.network_info(fork, eips) return BlockchainFixture( @@ -656,6 +673,7 @@ def make_hive_fixture( fork: Fork, eips: Optional[List[int]] = None, slow: bool = False, + negative_test: bool = False, ) -> BlockchainEngineFixture: """Create a hive fixture from the blocktest definition.""" fixture_payloads: List[FixtureEngineNewPayload] = [] @@ -691,6 +709,11 @@ def make_hive_fixture( alloc = new_alloc env = apply_new_parent(env, header) head_hash = header.block_hash + else: + assert negative_test, ( + "unmarked negative test: the test case produces an invalid block but " + + "does not specify the `pytest.mark.negative` marker" + ) if block.expected_post_state: self.verify_post_state( @@ -761,9 +784,21 @@ def generate( """Generate the BlockchainTest fixture.""" t8n.reset_traces() if fixture_format == BlockchainEngineFixture: - return self.make_hive_fixture(t8n, fork, eips, slow=is_slow_test(request)) + return self.make_hive_fixture( + t8n, + fork, + eips, + slow=is_slow_test(request), + negative_test=is_negative_test(request), + ) elif fixture_format == BlockchainFixture: - return self.make_fixture(t8n, fork, eips, slow=is_slow_test(request)) + return self.make_fixture( + t8n, + fork, + eips, + slow=is_slow_test(request), + negative_test=is_negative_test(request), + ) raise Exception(f"Unknown fixture format: {fixture_format}") diff --git a/src/ethereum_test_specs/helpers.py b/src/ethereum_test_specs/helpers.py index 0c184182ff3..4f4fe191e5b 100644 --- a/src/ethereum_test_specs/helpers.py +++ b/src/ethereum_test_specs/helpers.py @@ -313,3 +313,10 @@ def is_slow_test(request: pytest.FixtureRequest) -> bool: if hasattr(request, "node"): return request.node.get_closest_marker("slow") is not None return False + + +def is_negative_test(request: pytest.FixtureRequest) -> bool: + """Check if the test is negative (invalid block, invalid transaction).""" + if hasattr(request, "node"): + return request.node.get_closest_marker("negative") is not None + return False diff --git a/src/ethereum_test_specs/state.py b/src/ethereum_test_specs/state.py index a29223d4c1b..d0b5c71726e 100644 --- a/src/ethereum_test_specs/state.py +++ b/src/ethereum_test_specs/state.py @@ -34,7 +34,7 @@ from .base import BaseTest from .blockchain import Block, BlockchainTest, Header from .debugging import print_traces -from .helpers import is_slow_test, verify_transactions +from .helpers import is_negative_test, is_slow_test, verify_transactions class StateTest(BaseTest): @@ -233,6 +233,16 @@ def generate( eips: Optional[List[int]] = None, ) -> BaseFixture: """Generate the BlockchainTest fixture.""" + if self.tx.error is not None and not is_negative_test(request): + raise Exception( + "State tests with an error code should be marked with the " + "`pytest.mark.negative` test marker" + ) + elif self.tx.error is None and is_negative_test(request): + raise Exception( + "State tests without an error code should not be marked with the " + "`pytest.mark.negative` test marker" + ) if fixture_format in BlockchainTest.supported_fixture_formats: return self.generate_blockchain_test(fork=fork).generate( request=request, t8n=t8n, fork=fork, fixture_format=fixture_format, eips=eips diff --git a/src/ethereum_test_specs/transaction.py b/src/ethereum_test_specs/transaction.py index 7bc3ff21156..d6a737f16d2 100644 --- a/src/ethereum_test_specs/transaction.py +++ b/src/ethereum_test_specs/transaction.py @@ -22,6 +22,7 @@ from ethereum_test_types import Alloc, Transaction from .base import BaseTest +from .helpers import is_negative_test class TransactionTest(BaseTest): @@ -85,6 +86,16 @@ def generate( eips: Optional[List[int]] = None, ) -> BaseFixture: """Generate the TransactionTest fixture.""" + if self.tx.error is not None and not is_negative_test(request): + raise Exception( + "Transaction tests with an error code should be marked with the " + "`pytest.mark.negative` test marker." + ) + elif self.tx.error is None and is_negative_test(request): + raise Exception( + "Transaction tests without an error code should not be marked with " + "`pytest.mark.negative` test marker." + ) if fixture_format == TransactionFixture: return self.make_transaction_test_fixture(fork, eips) diff --git a/src/pytest_plugins/shared/execute_fill.py b/src/pytest_plugins/shared/execute_fill.py index 80e1df87467..1e817138755 100644 --- a/src/pytest_plugins/shared/execute_fill.py +++ b/src/pytest_plugins/shared/execute_fill.py @@ -85,6 +85,10 @@ def pytest_configure(config: pytest.Config): "markers", "zkevm: Tests that are relevant to zkEVM.", ) + config.addinivalue_line( + "markers", + "negative: Negative tests that include an invalid block or transaction.", + ) @pytest.fixture(autouse=True) From cad053f8f978c1bd01d8ad5083277808da074031 Mon Sep 17 00:00:00 2001 From: Mario Vega Date: Wed, 9 Apr 2025 20:53:24 +0000 Subject: [PATCH 2/8] feat(tests): Add marker to all invalid transaction/block tests --- tests/cancun/eip4844_blobs/test_blob_txs.py | 13 +++++++++++++ tests/cancun/eip4844_blobs/test_blob_txs_full.py | 1 + tests/cancun/eip4844_blobs/test_excess_blob_gas.py | 9 +++++++++ .../test_excess_blob_gas_fork_transition.py | 2 ++ tests/prague/eip6110_deposits/test_deposits.py | 1 + .../test_withdrawal_requests.py | 1 + .../eip7251_consolidations/test_consolidations.py | 1 + .../test_transaction_validity.py | 2 +- .../test_multi_type_requests.py | 2 ++ tests/prague/eip7702_set_code_tx/test_gas.py | 2 +- tests/prague/eip7702_set_code_tx/test_invalid_tx.py | 2 +- .../prague/eip7702_set_code_tx/test_set_code_txs.py | 5 +++++ tests/shanghai/eip3860_initcode/test_initcode.py | 10 +++++++--- .../eip4895_withdrawals/test_withdrawals.py | 8 ++++++-- 14 files changed, 51 insertions(+), 8 deletions(-) diff --git a/tests/cancun/eip4844_blobs/test_blob_txs.py b/tests/cancun/eip4844_blobs/test_blob_txs.py index 9ac2c51045d..88a59cdb531 100644 --- a/tests/cancun/eip4844_blobs/test_blob_txs.py +++ b/tests/cancun/eip4844_blobs/test_blob_txs.py @@ -424,6 +424,7 @@ def generate_invalid_tx_max_fee_per_blob_gas_tests( min_base_fee_per_blob_gas, # tx max_blob_gas_cost is the minimum TransactionException.INSUFFICIENT_MAX_FEE_PER_BLOB_GAS, id="insufficient_max_fee_per_blob_gas", + marks=pytest.mark.negative, ) ) if (next_base_fee_per_blob_gas - min_base_fee_per_blob_gas) > 1: @@ -436,6 +437,7 @@ def generate_invalid_tx_max_fee_per_blob_gas_tests( - 1, # tx max_blob_gas_cost is one less than the minimum TransactionException.INSUFFICIENT_MAX_FEE_PER_BLOB_GAS, id="insufficient_max_fee_per_blob_gas_one_less_than_next", + marks=pytest.mark.negative, ) ) if min_base_fee_per_blob_gas > 1: @@ -446,6 +448,7 @@ def generate_invalid_tx_max_fee_per_blob_gas_tests( min_base_fee_per_blob_gas - 1, # tx max_blob_gas_cost is one less than the minimum TransactionException.INSUFFICIENT_MAX_FEE_PER_BLOB_GAS, id="insufficient_max_fee_per_blob_gas_one_less_than_min", + marks=pytest.mark.negative, ) ) @@ -456,6 +459,7 @@ def generate_invalid_tx_max_fee_per_blob_gas_tests( 0, # tx max_blob_gas_cost is 0 TransactionException.INSUFFICIENT_MAX_FEE_PER_BLOB_GAS, id="invalid_max_fee_per_blob_gas", + marks=pytest.mark.negative, ) ) return tests @@ -533,6 +537,7 @@ def test_invalid_tx_max_fee_per_blob_gas_state( ], ids=["insufficient_max_fee_per_gas"], ) +@pytest.mark.negative @pytest.mark.valid_from("Cancun") def test_invalid_normal_gas( state_test: StateTestFiller, @@ -572,6 +577,7 @@ def test_invalid_normal_gas( ], ids=[""], ) +@pytest.mark.negative @pytest.mark.valid_from("Cancun") def test_invalid_block_blob_count( blockchain_test: BlockchainTestFiller, @@ -611,6 +617,7 @@ def test_invalid_block_blob_count( @pytest.mark.parametrize("tx_max_fee_per_blob_gas_multiplier", [1, 100, 10000]) @pytest.mark.parametrize("account_balance_modifier", [-1], ids=["exact_balance_minus_1"]) @pytest.mark.parametrize("tx_error", [TransactionException.INSUFFICIENT_ACCOUNT_FUNDS], ids=[""]) +@pytest.mark.negative @pytest.mark.valid_from("Cancun") def test_insufficient_balance_blob_tx( state_test: StateTestFiller, @@ -831,6 +838,7 @@ def test_blob_gas_subtraction_tx( ) @pytest.mark.parametrize("account_balance_modifier", [-1], ids=["exact_balance_minus_1"]) @pytest.mark.parametrize("tx_error", [TransactionException.INSUFFICIENT_ACCOUNT_FUNDS], ids=[""]) +@pytest.mark.negative @pytest.mark.valid_from("Cancun") def test_insufficient_balance_blob_tx_combinations( blockchain_test: BlockchainTestFiller, @@ -877,6 +885,7 @@ def generate_invalid_tx_blob_count_tests( "blobs_per_tx,tx_error", generate_invalid_tx_blob_count_tests, ) +@pytest.mark.negative @pytest.mark.valid_from("Cancun") def test_invalid_tx_blob_count( state_test: StateTestFiller, @@ -921,6 +930,7 @@ def test_invalid_tx_blob_count( @pytest.mark.parametrize( "tx_error", [TransactionException.TYPE_3_TX_INVALID_BLOB_VERSIONED_HASH], ids=[""] ) +@pytest.mark.negative @pytest.mark.valid_from("Cancun") def test_invalid_blob_hash_versioning_single_tx( state_test: StateTestFiller, @@ -979,6 +989,7 @@ def test_invalid_blob_hash_versioning_single_tx( @pytest.mark.parametrize( "tx_error", [TransactionException.TYPE_3_TX_INVALID_BLOB_VERSIONED_HASH], ids=[""] ) +@pytest.mark.negative @pytest.mark.valid_from("Cancun") def test_invalid_blob_hash_versioning_multiple_txs( blockchain_test: BlockchainTestFiller, @@ -1005,6 +1016,7 @@ def test_invalid_blob_hash_versioning_multiple_txs( @pytest.mark.parametrize( "tx_gas", [500_000], ids=[""] ) # Increase gas to account for contract creation +@pytest.mark.negative @pytest.mark.valid_from("Cancun") def test_invalid_blob_tx_contract_creation( blockchain_test: BlockchainTestFiller, @@ -1347,6 +1359,7 @@ def test_blob_tx_attribute_gasprice_opcode( ], ids=["no_blob_tx", "one_blob_tx"], ) +@pytest.mark.negative @pytest.mark.valid_at_transition_to("Cancun") def test_blob_type_tx_pre_fork( state_test: StateTestFiller, diff --git a/tests/cancun/eip4844_blobs/test_blob_txs_full.py b/tests/cancun/eip4844_blobs/test_blob_txs_full.py index da7d1ca0dfc..067142261b6 100644 --- a/tests/cancun/eip4844_blobs/test_blob_txs_full.py +++ b/tests/cancun/eip4844_blobs/test_blob_txs_full.py @@ -293,6 +293,7 @@ def generate_full_blob_tests( "txs_blobs,txs_wrapped_blobs", generate_full_blob_tests, ) +@pytest.mark.negative @pytest.mark.valid_from("Cancun") def test_reject_valid_full_blob_in_block_rlp( blockchain_test: BlockchainTestFiller, diff --git a/tests/cancun/eip4844_blobs/test_excess_blob_gas.py b/tests/cancun/eip4844_blobs/test_excess_blob_gas.py index f90f275e93e..7f1e8754834 100644 --- a/tests/cancun/eip4844_blobs/test_excess_blob_gas.py +++ b/tests/cancun/eip4844_blobs/test_excess_blob_gas.py @@ -387,6 +387,7 @@ def test_correct_decreasing_blob_gas_costs( "parent_blobs", lambda fork: range(0, fork.max_blobs_per_block() + 1), ) +@pytest.mark.negative def test_invalid_zero_excess_blob_gas_in_header( blockchain_test: BlockchainTestFiller, env: Environment, @@ -435,6 +436,7 @@ def all_invalid_blob_gas_used_combinations(fork: Fork) -> Iterator[Tuple[int, in all_invalid_blob_gas_used_combinations, ) @pytest.mark.parametrize("parent_blobs", [0]) +@pytest.mark.negative def test_invalid_blob_gas_used_in_header( blockchain_test: BlockchainTestFiller, env: Environment, @@ -479,6 +481,7 @@ def generate_invalid_excess_blob_gas_above_target_change_tests(fork: Fork) -> Li generate_invalid_excess_blob_gas_above_target_change_tests, ) @pytest.mark.parametrize("new_blobs", [1]) +@pytest.mark.negative def test_invalid_excess_blob_gas_above_target_change( blockchain_test: BlockchainTestFiller, env: Environment, @@ -523,6 +526,7 @@ def test_invalid_excess_blob_gas_above_target_change( "parent_excess_blobs", lambda fork: [1, fork.target_blobs_per_block()] ) @pytest.mark.parametrize("new_blobs", [1]) +@pytest.mark.negative def test_invalid_static_excess_blob_gas( blockchain_test: BlockchainTestFiller, env: Environment, @@ -564,6 +568,7 @@ def test_invalid_static_excess_blob_gas( ) @pytest.mark.parametrize("parent_excess_blobs", [0]) # Start at 0 @pytest.mark.parametrize("new_blobs", [1]) +@pytest.mark.negative def test_invalid_excess_blob_gas_target_blobs_increase_from_zero( blockchain_test: BlockchainTestFiller, env: Environment, @@ -605,6 +610,7 @@ def test_invalid_excess_blob_gas_target_blobs_increase_from_zero( ) @pytest.mark.parametrize("parent_excess_blobs", [0]) # Start at 0 @pytest.mark.parametrize("new_blobs", [1]) +@pytest.mark.negative def test_invalid_static_excess_blob_gas_from_zero_on_blobs_above_target( blockchain_test: BlockchainTestFiller, env: Environment, @@ -653,6 +659,7 @@ def test_invalid_static_excess_blob_gas_from_zero_on_blobs_above_target( ), ) @pytest.mark.parametrize("new_blobs", [1]) +@pytest.mark.negative def test_invalid_excess_blob_gas_change( blockchain_test: BlockchainTestFiller, env: Environment, @@ -704,6 +711,7 @@ def test_invalid_excess_blob_gas_change( "parent_excess_blobs", lambda fork: range(fork.target_blobs_per_block()), ) +@pytest.mark.negative def test_invalid_negative_excess_blob_gas( blockchain_test: BlockchainTestFiller, env: Environment, @@ -753,6 +761,7 @@ def test_invalid_negative_excess_blob_gas( "parent_excess_blobs", lambda fork: [fork.target_blobs_per_block() + 1], ) +@pytest.mark.negative def test_invalid_non_multiple_excess_blob_gas( blockchain_test: BlockchainTestFiller, env: Environment, diff --git a/tests/cancun/eip4844_blobs/test_excess_blob_gas_fork_transition.py b/tests/cancun/eip4844_blobs/test_excess_blob_gas_fork_transition.py index fd75d24c35b..90e804fdca1 100644 --- a/tests/cancun/eip4844_blobs/test_excess_blob_gas_fork_transition.py +++ b/tests/cancun/eip4844_blobs/test_excess_blob_gas_fork_transition.py @@ -213,6 +213,7 @@ def post( # noqa: D103 (True, True), ], ) +@pytest.mark.negative def test_invalid_pre_fork_block_with_blob_fields( blockchain_test: BlockchainTestFiller, env: Environment, @@ -257,6 +258,7 @@ def test_invalid_pre_fork_block_with_blob_fields( (True, True), ], ) +@pytest.mark.negative def test_invalid_post_fork_block_without_blob_fields( blockchain_test: BlockchainTestFiller, env: Environment, diff --git a/tests/prague/eip6110_deposits/test_deposits.py b/tests/prague/eip6110_deposits/test_deposits.py index 5be5457e8a8..55d9b9eb007 100644 --- a/tests/prague/eip6110_deposits/test_deposits.py +++ b/tests/prague/eip6110_deposits/test_deposits.py @@ -951,6 +951,7 @@ def test_deposit( ), ], ) +@pytest.mark.negative def test_deposit_negative( blockchain_test: BlockchainTestFiller, pre: Alloc, diff --git a/tests/prague/eip7002_el_triggerable_withdrawals/test_withdrawal_requests.py b/tests/prague/eip7002_el_triggerable_withdrawals/test_withdrawal_requests.py index 5b7bb6b4f67..59d6f8ce622 100644 --- a/tests/prague/eip7002_el_triggerable_withdrawals/test_withdrawal_requests.py +++ b/tests/prague/eip7002_el_triggerable_withdrawals/test_withdrawal_requests.py @@ -679,6 +679,7 @@ def test_withdrawal_requests( ), ], ) +@pytest.mark.negative def test_withdrawal_requests_negative( pre: Alloc, fork: Fork, diff --git a/tests/prague/eip7251_consolidations/test_consolidations.py b/tests/prague/eip7251_consolidations/test_consolidations.py index 3f84cbd3fc3..1afbbdf53fc 100644 --- a/tests/prague/eip7251_consolidations/test_consolidations.py +++ b/tests/prague/eip7251_consolidations/test_consolidations.py @@ -730,6 +730,7 @@ def test_consolidation_requests( ), ], ) +@pytest.mark.negative def test_consolidation_requests_negative( pre: Alloc, fork: Fork, diff --git a/tests/prague/eip7623_increase_calldata_cost/test_transaction_validity.py b/tests/prague/eip7623_increase_calldata_cost/test_transaction_validity.py index 1d2c6433f71..161179d497a 100644 --- a/tests/prague/eip7623_increase_calldata_cost/test_transaction_validity.py +++ b/tests/prague/eip7623_increase_calldata_cost/test_transaction_validity.py @@ -37,7 +37,7 @@ # the data floor does not consume more gas than it should. pytest.param(1, id="extra_gas"), pytest.param(0, id="exact_gas"), - pytest.param(-1, id="insufficient_gas"), + pytest.param(-1, id="insufficient_gas", marks=pytest.mark.negative), ], ), pytest.mark.parametrize( diff --git a/tests/prague/eip7685_general_purpose_el_requests/test_multi_type_requests.py b/tests/prague/eip7685_general_purpose_el_requests/test_multi_type_requests.py index 68a6796a7ff..d5920fe4c3c 100644 --- a/tests/prague/eip7685_general_purpose_el_requests/test_multi_type_requests.py +++ b/tests/prague/eip7685_general_purpose_el_requests/test_multi_type_requests.py @@ -604,6 +604,7 @@ def invalid_requests_block_combinations(fork: Fork) -> List[ParameterSet]: "requests,block_body_override_requests,exception", invalid_requests_block_combinations, ) +@pytest.mark.negative def test_invalid_deposit_withdrawal_consolidation_requests( blockchain_test: BlockchainTestFiller, pre: Alloc, @@ -631,6 +632,7 @@ def test_invalid_deposit_withdrawal_consolidation_requests( ) @pytest.mark.parametrize("correct_requests_hash_in_header", [True]) @pytest.mark.blockchain_test_engine_only +@pytest.mark.negative def test_invalid_deposit_withdrawal_consolidation_requests_engine( blockchain_test: BlockchainTestFiller, pre: Alloc, diff --git a/tests/prague/eip7702_set_code_tx/test_gas.py b/tests/prague/eip7702_set_code_tx/test_gas.py index 1306542678b..05d93d71d03 100644 --- a/tests/prague/eip7702_set_code_tx/test_gas.py +++ b/tests/prague/eip7702_set_code_tx/test_gas.py @@ -894,7 +894,7 @@ def test_account_warming( @pytest.mark.parametrize(**gas_test_parameter_args(include_pre_authorized=False)) @pytest.mark.parametrize( "valid", - [True, False], + [True, pytest.param(False, marks=pytest.mark.negative)], ) def test_intrinsic_gas_cost( state_test: StateTestFiller, diff --git a/tests/prague/eip7702_set_code_tx/test_invalid_tx.py b/tests/prague/eip7702_set_code_tx/test_invalid_tx.py index 4e46cb0d12d..635ae7bd99c 100644 --- a/tests/prague/eip7702_set_code_tx/test_invalid_tx.py +++ b/tests/prague/eip7702_set_code_tx/test_invalid_tx.py @@ -23,7 +23,7 @@ REFERENCE_SPEC_GIT_PATH = ref_spec_7702.git_path REFERENCE_SPEC_VERSION = ref_spec_7702.version -pytestmark = pytest.mark.valid_from("Prague") +pytestmark = [pytest.mark.valid_from("Prague"), pytest.mark.negative] auth_account_start_balance = 0 diff --git a/tests/prague/eip7702_set_code_tx/test_set_code_txs.py b/tests/prague/eip7702_set_code_tx/test_set_code_txs.py index f0f5ee94204..b2913bcfa1d 100644 --- a/tests/prague/eip7702_set_code_tx/test_set_code_txs.py +++ b/tests/prague/eip7702_set_code_tx/test_set_code_txs.py @@ -2973,6 +2973,7 @@ def test_reset_code( ) +@pytest.mark.negative def test_contract_create( state_test: StateTestFiller, pre: Alloc, @@ -3000,6 +3001,7 @@ def test_contract_create( ) +@pytest.mark.negative def test_empty_authorization_list( state_test: StateTestFiller, pre: Alloc, @@ -3488,6 +3490,7 @@ def test_many_delegations( ) +@pytest.mark.negative def test_invalid_transaction_after_authorization( blockchain_test: BlockchainTestFiller, pre: Alloc, @@ -3589,6 +3592,7 @@ def test_authorization_reusing_nonce( "self_sponsored", [True, False], ) +@pytest.mark.negative @pytest.mark.execute(pytest.mark.skip(reason="Requires contract-eoa address collision")) def test_set_code_from_account_with_non_delegating_code( state_test: StateTestFiller, @@ -3669,6 +3673,7 @@ def test_set_code_from_account_with_non_delegating_code( "priority_greater_than_max_fee_per_gas", ], ) +@pytest.mark.negative def test_set_code_transaction_fee_validations( state_test: StateTestFiller, pre: Alloc, diff --git a/tests/shanghai/eip3860_initcode/test_initcode.py b/tests/shanghai/eip3860_initcode/test_initcode.py index c64ab035513..7b46d4c0fba 100644 --- a/tests/shanghai/eip3860_initcode/test_initcode.py +++ b/tests/shanghai/eip3860_initcode/test_initcode.py @@ -122,8 +122,8 @@ [ INITCODE_ZEROS_MAX_LIMIT, INITCODE_ONES_MAX_LIMIT, - INITCODE_ZEROS_OVER_LIMIT, - INITCODE_ONES_OVER_LIMIT, + pytest.param(INITCODE_ZEROS_OVER_LIMIT, marks=pytest.mark.negative), + pytest.param(INITCODE_ONES_OVER_LIMIT, marks=pytest.mark.negative), ], ids=get_initcode_name, ) @@ -181,7 +181,11 @@ def valid_gas_test_case(initcode: Initcode, gas_test_case: str) -> bool: @pytest.mark.parametrize( "initcode,gas_test_case", [ - (i, g) + pytest.param( + i, + g, + marks=([pytest.mark.negative] if g == "too_little_intrinsic_gas" else []), + ) for i in [ INITCODE_ZEROS_MAX_LIMIT, INITCODE_ONES_MAX_LIMIT, diff --git a/tests/shanghai/eip4895_withdrawals/test_withdrawals.py b/tests/shanghai/eip4895_withdrawals/test_withdrawals.py index 87cc1501016..df4fb0f4270 100644 --- a/tests/shanghai/eip4895_withdrawals/test_withdrawals.py +++ b/tests/shanghai/eip4895_withdrawals/test_withdrawals.py @@ -37,8 +37,12 @@ @pytest.mark.parametrize( "test_case", - ["tx_in_withdrawals_block", "tx_after_withdrawals_block"], - ids=lambda x: x, + [ + pytest.param( + "tx_in_withdrawals_block", id="tx_in_withdrawals_block", marks=pytest.mark.negative + ), + pytest.param("tx_after_withdrawals_block", id="tx_after_withdrawals_block"), + ], ) class TestUseValueInTx: """ From 26b912b28c91ef30e11b173a3d2e289621233dc4 Mon Sep 17 00:00:00 2001 From: Mario Vega Date: Thu, 10 Apr 2025 16:58:09 +0000 Subject: [PATCH 3/8] changelog --- docs/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 8266f804ca1..2d84cdca316 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -14,6 +14,7 @@ Test fixtures for use by clients are available for each release on the [Github r - 🐞 Fix `DeprecationWarning: Pickle, copy, and deepcopy support will be removed from itertools in Python 3.14.` by avoiding use `itertools` object in the spec `BaseTest` pydantic model ([#1414](https://github.com/ethereum/execution-spec-tests/pull/1414)). - ✨ The `static_filler` plug-in now has support for static state tests (from [GeneralStateTests](https://github.com/ethereum/tests/tree/develop/src/GeneralStateTestsFiller)) ([#1362](https://github.com/ethereum/execution-spec-tests/pull/1362)). +- ✨ Introduce `pytest.mark.negative` to mark tests that contain an invalid transaction or block ([#1436](https://github.com/ethereum/execution-spec-tests/pull/1436)). ### 📋 Misc From 684ae377039dd07c6bdf15eb56e308e55d49519a Mon Sep 17 00:00:00 2001 From: Mario Vega Date: Thu, 10 Apr 2025 21:29:38 +0000 Subject: [PATCH 4/8] refactor(specs): Rename marker to `pytest.mark.exception_test` --- src/ethereum_test_specs/blockchain.py | 14 +++++++------- src/ethereum_test_specs/helpers.py | 4 ++-- src/ethereum_test_specs/state.py | 4 ++-- src/ethereum_test_specs/transaction.py | 4 ++-- src/pytest_plugins/shared/execute_fill.py | 2 +- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/ethereum_test_specs/blockchain.py b/src/ethereum_test_specs/blockchain.py index 67499fc798b..59331d2f2eb 100644 --- a/src/ethereum_test_specs/blockchain.py +++ b/src/ethereum_test_specs/blockchain.py @@ -607,8 +607,8 @@ def make_fixture( head = header.block_hash else: assert negative_test, ( - "unmarked negative test: the test case produces an invalid block but " - + "does not specify the `pytest.mark.negative` marker" + "unmarked exception test: the test case produces an invalid block but " + + "does not specify the `pytest.mark.exception_test` marker" ) fixture_blocks.append( InvalidFixtureBlock( @@ -628,8 +628,8 @@ def make_fixture( + "the block is expected to produce an exception" ) assert negative_test, ( - "unmarked negative test: the test case produces an invalid block but " - + "does not specify the `pytest.mark.negative` marker" + "unmarked exception test: the test case produces an invalid block but " + + "does not specify the `pytest.mark.exception_test` marker" ) fixture_blocks.append( InvalidFixtureBlock( @@ -646,7 +646,7 @@ def make_fixture( if invalid_blocks == 0 and negative_test: raise Exception( - "test correctness: the test case is marked as negative but " + "test correctness: the test case is marked as an exception test but " + "no invalid blocks were found" ) @@ -711,8 +711,8 @@ def make_hive_fixture( head_hash = header.block_hash else: assert negative_test, ( - "unmarked negative test: the test case produces an invalid block but " - + "does not specify the `pytest.mark.negative` marker" + "unmarked exception test: the test case produces an invalid block but " + + "does not specify the `pytest.mark.exception_test` marker" ) if block.expected_post_state: diff --git a/src/ethereum_test_specs/helpers.py b/src/ethereum_test_specs/helpers.py index 4f4fe191e5b..3bc07185d6a 100644 --- a/src/ethereum_test_specs/helpers.py +++ b/src/ethereum_test_specs/helpers.py @@ -316,7 +316,7 @@ def is_slow_test(request: pytest.FixtureRequest) -> bool: def is_negative_test(request: pytest.FixtureRequest) -> bool: - """Check if the test is negative (invalid block, invalid transaction).""" + """Check if the test is an exception test (invalid block, invalid transaction).""" if hasattr(request, "node"): - return request.node.get_closest_marker("negative") is not None + return request.node.get_closest_marker("exception_test") is not None return False diff --git a/src/ethereum_test_specs/state.py b/src/ethereum_test_specs/state.py index d0b5c71726e..9c93fb23c6c 100644 --- a/src/ethereum_test_specs/state.py +++ b/src/ethereum_test_specs/state.py @@ -236,12 +236,12 @@ def generate( if self.tx.error is not None and not is_negative_test(request): raise Exception( "State tests with an error code should be marked with the " - "`pytest.mark.negative` test marker" + "`pytest.mark.exception_test` test marker" ) elif self.tx.error is None and is_negative_test(request): raise Exception( "State tests without an error code should not be marked with the " - "`pytest.mark.negative` test marker" + "`pytest.mark.exception_test` test marker" ) if fixture_format in BlockchainTest.supported_fixture_formats: return self.generate_blockchain_test(fork=fork).generate( diff --git a/src/ethereum_test_specs/transaction.py b/src/ethereum_test_specs/transaction.py index d6a737f16d2..94e9472ae0e 100644 --- a/src/ethereum_test_specs/transaction.py +++ b/src/ethereum_test_specs/transaction.py @@ -89,12 +89,12 @@ def generate( if self.tx.error is not None and not is_negative_test(request): raise Exception( "Transaction tests with an error code should be marked with the " - "`pytest.mark.negative` test marker." + "`pytest.mark.exception_test` test marker." ) elif self.tx.error is None and is_negative_test(request): raise Exception( "Transaction tests without an error code should not be marked with " - "`pytest.mark.negative` test marker." + "`pytest.mark.exception_test` test marker." ) if fixture_format == TransactionFixture: return self.make_transaction_test_fixture(fork, eips) diff --git a/src/pytest_plugins/shared/execute_fill.py b/src/pytest_plugins/shared/execute_fill.py index 1e817138755..7ac49c3f190 100644 --- a/src/pytest_plugins/shared/execute_fill.py +++ b/src/pytest_plugins/shared/execute_fill.py @@ -87,7 +87,7 @@ def pytest_configure(config: pytest.Config): ) config.addinivalue_line( "markers", - "negative: Negative tests that include an invalid block or transaction.", + "exception_test: Negative tests that include an invalid block or transaction.", ) From 2dd4b3ddd9702c0464fde1fec17a234c11c437a9 Mon Sep 17 00:00:00 2001 From: Mario Vega Date: Thu, 10 Apr 2025 21:31:03 +0000 Subject: [PATCH 5/8] refactor(tests): Rename marker to `pytest.mark.exception_test` --- tests/cancun/eip4844_blobs/test_blob_txs.py | 26 +++++++++---------- .../eip4844_blobs/test_blob_txs_full.py | 2 +- .../eip4844_blobs/test_excess_blob_gas.py | 18 ++++++------- .../test_excess_blob_gas_fork_transition.py | 4 +-- .../prague/eip6110_deposits/test_deposits.py | 2 +- .../test_withdrawal_requests.py | 2 +- .../test_consolidations.py | 2 +- .../test_transaction_validity.py | 2 +- .../test_multi_type_requests.py | 4 +-- tests/prague/eip7702_set_code_tx/test_gas.py | 2 +- .../eip7702_set_code_tx/test_invalid_tx.py | 2 +- .../eip7702_set_code_tx/test_set_code_txs.py | 10 +++---- .../eip3860_initcode/test_initcode.py | 6 ++--- .../eip4895_withdrawals/test_withdrawals.py | 4 ++- 14 files changed, 44 insertions(+), 42 deletions(-) diff --git a/tests/cancun/eip4844_blobs/test_blob_txs.py b/tests/cancun/eip4844_blobs/test_blob_txs.py index 88a59cdb531..0a7dd7c9ec8 100644 --- a/tests/cancun/eip4844_blobs/test_blob_txs.py +++ b/tests/cancun/eip4844_blobs/test_blob_txs.py @@ -424,7 +424,7 @@ def generate_invalid_tx_max_fee_per_blob_gas_tests( min_base_fee_per_blob_gas, # tx max_blob_gas_cost is the minimum TransactionException.INSUFFICIENT_MAX_FEE_PER_BLOB_GAS, id="insufficient_max_fee_per_blob_gas", - marks=pytest.mark.negative, + marks=pytest.mark.exception_test, ) ) if (next_base_fee_per_blob_gas - min_base_fee_per_blob_gas) > 1: @@ -437,7 +437,7 @@ def generate_invalid_tx_max_fee_per_blob_gas_tests( - 1, # tx max_blob_gas_cost is one less than the minimum TransactionException.INSUFFICIENT_MAX_FEE_PER_BLOB_GAS, id="insufficient_max_fee_per_blob_gas_one_less_than_next", - marks=pytest.mark.negative, + marks=pytest.mark.exception_test, ) ) if min_base_fee_per_blob_gas > 1: @@ -448,7 +448,7 @@ def generate_invalid_tx_max_fee_per_blob_gas_tests( min_base_fee_per_blob_gas - 1, # tx max_blob_gas_cost is one less than the minimum TransactionException.INSUFFICIENT_MAX_FEE_PER_BLOB_GAS, id="insufficient_max_fee_per_blob_gas_one_less_than_min", - marks=pytest.mark.negative, + marks=pytest.mark.exception_test, ) ) @@ -459,7 +459,7 @@ def generate_invalid_tx_max_fee_per_blob_gas_tests( 0, # tx max_blob_gas_cost is 0 TransactionException.INSUFFICIENT_MAX_FEE_PER_BLOB_GAS, id="invalid_max_fee_per_blob_gas", - marks=pytest.mark.negative, + marks=pytest.mark.exception_test, ) ) return tests @@ -537,7 +537,7 @@ def test_invalid_tx_max_fee_per_blob_gas_state( ], ids=["insufficient_max_fee_per_gas"], ) -@pytest.mark.negative +@pytest.mark.exception_test @pytest.mark.valid_from("Cancun") def test_invalid_normal_gas( state_test: StateTestFiller, @@ -577,7 +577,7 @@ def test_invalid_normal_gas( ], ids=[""], ) -@pytest.mark.negative +@pytest.mark.exception_test @pytest.mark.valid_from("Cancun") def test_invalid_block_blob_count( blockchain_test: BlockchainTestFiller, @@ -617,7 +617,7 @@ def test_invalid_block_blob_count( @pytest.mark.parametrize("tx_max_fee_per_blob_gas_multiplier", [1, 100, 10000]) @pytest.mark.parametrize("account_balance_modifier", [-1], ids=["exact_balance_minus_1"]) @pytest.mark.parametrize("tx_error", [TransactionException.INSUFFICIENT_ACCOUNT_FUNDS], ids=[""]) -@pytest.mark.negative +@pytest.mark.exception_test @pytest.mark.valid_from("Cancun") def test_insufficient_balance_blob_tx( state_test: StateTestFiller, @@ -838,7 +838,7 @@ def test_blob_gas_subtraction_tx( ) @pytest.mark.parametrize("account_balance_modifier", [-1], ids=["exact_balance_minus_1"]) @pytest.mark.parametrize("tx_error", [TransactionException.INSUFFICIENT_ACCOUNT_FUNDS], ids=[""]) -@pytest.mark.negative +@pytest.mark.exception_test @pytest.mark.valid_from("Cancun") def test_insufficient_balance_blob_tx_combinations( blockchain_test: BlockchainTestFiller, @@ -885,7 +885,7 @@ def generate_invalid_tx_blob_count_tests( "blobs_per_tx,tx_error", generate_invalid_tx_blob_count_tests, ) -@pytest.mark.negative +@pytest.mark.exception_test @pytest.mark.valid_from("Cancun") def test_invalid_tx_blob_count( state_test: StateTestFiller, @@ -930,7 +930,7 @@ def test_invalid_tx_blob_count( @pytest.mark.parametrize( "tx_error", [TransactionException.TYPE_3_TX_INVALID_BLOB_VERSIONED_HASH], ids=[""] ) -@pytest.mark.negative +@pytest.mark.exception_test @pytest.mark.valid_from("Cancun") def test_invalid_blob_hash_versioning_single_tx( state_test: StateTestFiller, @@ -989,7 +989,7 @@ def test_invalid_blob_hash_versioning_single_tx( @pytest.mark.parametrize( "tx_error", [TransactionException.TYPE_3_TX_INVALID_BLOB_VERSIONED_HASH], ids=[""] ) -@pytest.mark.negative +@pytest.mark.exception_test @pytest.mark.valid_from("Cancun") def test_invalid_blob_hash_versioning_multiple_txs( blockchain_test: BlockchainTestFiller, @@ -1016,7 +1016,7 @@ def test_invalid_blob_hash_versioning_multiple_txs( @pytest.mark.parametrize( "tx_gas", [500_000], ids=[""] ) # Increase gas to account for contract creation -@pytest.mark.negative +@pytest.mark.exception_test @pytest.mark.valid_from("Cancun") def test_invalid_blob_tx_contract_creation( blockchain_test: BlockchainTestFiller, @@ -1359,7 +1359,7 @@ def test_blob_tx_attribute_gasprice_opcode( ], ids=["no_blob_tx", "one_blob_tx"], ) -@pytest.mark.negative +@pytest.mark.exception_test @pytest.mark.valid_at_transition_to("Cancun") def test_blob_type_tx_pre_fork( state_test: StateTestFiller, diff --git a/tests/cancun/eip4844_blobs/test_blob_txs_full.py b/tests/cancun/eip4844_blobs/test_blob_txs_full.py index 067142261b6..5a2420dfc8b 100644 --- a/tests/cancun/eip4844_blobs/test_blob_txs_full.py +++ b/tests/cancun/eip4844_blobs/test_blob_txs_full.py @@ -293,7 +293,7 @@ def generate_full_blob_tests( "txs_blobs,txs_wrapped_blobs", generate_full_blob_tests, ) -@pytest.mark.negative +@pytest.mark.exception_test @pytest.mark.valid_from("Cancun") def test_reject_valid_full_blob_in_block_rlp( blockchain_test: BlockchainTestFiller, diff --git a/tests/cancun/eip4844_blobs/test_excess_blob_gas.py b/tests/cancun/eip4844_blobs/test_excess_blob_gas.py index 7f1e8754834..c7107b2057b 100644 --- a/tests/cancun/eip4844_blobs/test_excess_blob_gas.py +++ b/tests/cancun/eip4844_blobs/test_excess_blob_gas.py @@ -387,7 +387,7 @@ def test_correct_decreasing_blob_gas_costs( "parent_blobs", lambda fork: range(0, fork.max_blobs_per_block() + 1), ) -@pytest.mark.negative +@pytest.mark.exception_test def test_invalid_zero_excess_blob_gas_in_header( blockchain_test: BlockchainTestFiller, env: Environment, @@ -436,7 +436,7 @@ def all_invalid_blob_gas_used_combinations(fork: Fork) -> Iterator[Tuple[int, in all_invalid_blob_gas_used_combinations, ) @pytest.mark.parametrize("parent_blobs", [0]) -@pytest.mark.negative +@pytest.mark.exception_test def test_invalid_blob_gas_used_in_header( blockchain_test: BlockchainTestFiller, env: Environment, @@ -481,7 +481,7 @@ def generate_invalid_excess_blob_gas_above_target_change_tests(fork: Fork) -> Li generate_invalid_excess_blob_gas_above_target_change_tests, ) @pytest.mark.parametrize("new_blobs", [1]) -@pytest.mark.negative +@pytest.mark.exception_test def test_invalid_excess_blob_gas_above_target_change( blockchain_test: BlockchainTestFiller, env: Environment, @@ -526,7 +526,7 @@ def test_invalid_excess_blob_gas_above_target_change( "parent_excess_blobs", lambda fork: [1, fork.target_blobs_per_block()] ) @pytest.mark.parametrize("new_blobs", [1]) -@pytest.mark.negative +@pytest.mark.exception_test def test_invalid_static_excess_blob_gas( blockchain_test: BlockchainTestFiller, env: Environment, @@ -568,7 +568,7 @@ def test_invalid_static_excess_blob_gas( ) @pytest.mark.parametrize("parent_excess_blobs", [0]) # Start at 0 @pytest.mark.parametrize("new_blobs", [1]) -@pytest.mark.negative +@pytest.mark.exception_test def test_invalid_excess_blob_gas_target_blobs_increase_from_zero( blockchain_test: BlockchainTestFiller, env: Environment, @@ -610,7 +610,7 @@ def test_invalid_excess_blob_gas_target_blobs_increase_from_zero( ) @pytest.mark.parametrize("parent_excess_blobs", [0]) # Start at 0 @pytest.mark.parametrize("new_blobs", [1]) -@pytest.mark.negative +@pytest.mark.exception_test def test_invalid_static_excess_blob_gas_from_zero_on_blobs_above_target( blockchain_test: BlockchainTestFiller, env: Environment, @@ -659,7 +659,7 @@ def test_invalid_static_excess_blob_gas_from_zero_on_blobs_above_target( ), ) @pytest.mark.parametrize("new_blobs", [1]) -@pytest.mark.negative +@pytest.mark.exception_test def test_invalid_excess_blob_gas_change( blockchain_test: BlockchainTestFiller, env: Environment, @@ -711,7 +711,7 @@ def test_invalid_excess_blob_gas_change( "parent_excess_blobs", lambda fork: range(fork.target_blobs_per_block()), ) -@pytest.mark.negative +@pytest.mark.exception_test def test_invalid_negative_excess_blob_gas( blockchain_test: BlockchainTestFiller, env: Environment, @@ -761,7 +761,7 @@ def test_invalid_negative_excess_blob_gas( "parent_excess_blobs", lambda fork: [fork.target_blobs_per_block() + 1], ) -@pytest.mark.negative +@pytest.mark.exception_test def test_invalid_non_multiple_excess_blob_gas( blockchain_test: BlockchainTestFiller, env: Environment, diff --git a/tests/cancun/eip4844_blobs/test_excess_blob_gas_fork_transition.py b/tests/cancun/eip4844_blobs/test_excess_blob_gas_fork_transition.py index 90e804fdca1..ee2b48ede47 100644 --- a/tests/cancun/eip4844_blobs/test_excess_blob_gas_fork_transition.py +++ b/tests/cancun/eip4844_blobs/test_excess_blob_gas_fork_transition.py @@ -213,7 +213,7 @@ def post( # noqa: D103 (True, True), ], ) -@pytest.mark.negative +@pytest.mark.exception_test def test_invalid_pre_fork_block_with_blob_fields( blockchain_test: BlockchainTestFiller, env: Environment, @@ -258,7 +258,7 @@ def test_invalid_pre_fork_block_with_blob_fields( (True, True), ], ) -@pytest.mark.negative +@pytest.mark.exception_test def test_invalid_post_fork_block_without_blob_fields( blockchain_test: BlockchainTestFiller, env: Environment, diff --git a/tests/prague/eip6110_deposits/test_deposits.py b/tests/prague/eip6110_deposits/test_deposits.py index 55d9b9eb007..6354fe21228 100644 --- a/tests/prague/eip6110_deposits/test_deposits.py +++ b/tests/prague/eip6110_deposits/test_deposits.py @@ -951,7 +951,7 @@ def test_deposit( ), ], ) -@pytest.mark.negative +@pytest.mark.exception_test def test_deposit_negative( blockchain_test: BlockchainTestFiller, pre: Alloc, diff --git a/tests/prague/eip7002_el_triggerable_withdrawals/test_withdrawal_requests.py b/tests/prague/eip7002_el_triggerable_withdrawals/test_withdrawal_requests.py index 59d6f8ce622..f5fd8434dd5 100644 --- a/tests/prague/eip7002_el_triggerable_withdrawals/test_withdrawal_requests.py +++ b/tests/prague/eip7002_el_triggerable_withdrawals/test_withdrawal_requests.py @@ -679,7 +679,7 @@ def test_withdrawal_requests( ), ], ) -@pytest.mark.negative +@pytest.mark.exception_test def test_withdrawal_requests_negative( pre: Alloc, fork: Fork, diff --git a/tests/prague/eip7251_consolidations/test_consolidations.py b/tests/prague/eip7251_consolidations/test_consolidations.py index 1afbbdf53fc..43f88bd7253 100644 --- a/tests/prague/eip7251_consolidations/test_consolidations.py +++ b/tests/prague/eip7251_consolidations/test_consolidations.py @@ -730,7 +730,7 @@ def test_consolidation_requests( ), ], ) -@pytest.mark.negative +@pytest.mark.exception_test def test_consolidation_requests_negative( pre: Alloc, fork: Fork, diff --git a/tests/prague/eip7623_increase_calldata_cost/test_transaction_validity.py b/tests/prague/eip7623_increase_calldata_cost/test_transaction_validity.py index 161179d497a..c2952c70f51 100644 --- a/tests/prague/eip7623_increase_calldata_cost/test_transaction_validity.py +++ b/tests/prague/eip7623_increase_calldata_cost/test_transaction_validity.py @@ -37,7 +37,7 @@ # the data floor does not consume more gas than it should. pytest.param(1, id="extra_gas"), pytest.param(0, id="exact_gas"), - pytest.param(-1, id="insufficient_gas", marks=pytest.mark.negative), + pytest.param(-1, id="insufficient_gas", marks=pytest.mark.exception_test), ], ), pytest.mark.parametrize( diff --git a/tests/prague/eip7685_general_purpose_el_requests/test_multi_type_requests.py b/tests/prague/eip7685_general_purpose_el_requests/test_multi_type_requests.py index d5920fe4c3c..5181dc50258 100644 --- a/tests/prague/eip7685_general_purpose_el_requests/test_multi_type_requests.py +++ b/tests/prague/eip7685_general_purpose_el_requests/test_multi_type_requests.py @@ -604,7 +604,7 @@ def invalid_requests_block_combinations(fork: Fork) -> List[ParameterSet]: "requests,block_body_override_requests,exception", invalid_requests_block_combinations, ) -@pytest.mark.negative +@pytest.mark.exception_test def test_invalid_deposit_withdrawal_consolidation_requests( blockchain_test: BlockchainTestFiller, pre: Alloc, @@ -632,7 +632,7 @@ def test_invalid_deposit_withdrawal_consolidation_requests( ) @pytest.mark.parametrize("correct_requests_hash_in_header", [True]) @pytest.mark.blockchain_test_engine_only -@pytest.mark.negative +@pytest.mark.exception_test def test_invalid_deposit_withdrawal_consolidation_requests_engine( blockchain_test: BlockchainTestFiller, pre: Alloc, diff --git a/tests/prague/eip7702_set_code_tx/test_gas.py b/tests/prague/eip7702_set_code_tx/test_gas.py index 05d93d71d03..47e51367ef9 100644 --- a/tests/prague/eip7702_set_code_tx/test_gas.py +++ b/tests/prague/eip7702_set_code_tx/test_gas.py @@ -894,7 +894,7 @@ def test_account_warming( @pytest.mark.parametrize(**gas_test_parameter_args(include_pre_authorized=False)) @pytest.mark.parametrize( "valid", - [True, pytest.param(False, marks=pytest.mark.negative)], + [True, pytest.param(False, marks=pytest.mark.exception_test)], ) def test_intrinsic_gas_cost( state_test: StateTestFiller, diff --git a/tests/prague/eip7702_set_code_tx/test_invalid_tx.py b/tests/prague/eip7702_set_code_tx/test_invalid_tx.py index 635ae7bd99c..690579f7df3 100644 --- a/tests/prague/eip7702_set_code_tx/test_invalid_tx.py +++ b/tests/prague/eip7702_set_code_tx/test_invalid_tx.py @@ -23,7 +23,7 @@ REFERENCE_SPEC_GIT_PATH = ref_spec_7702.git_path REFERENCE_SPEC_VERSION = ref_spec_7702.version -pytestmark = [pytest.mark.valid_from("Prague"), pytest.mark.negative] +pytestmark = [pytest.mark.valid_from("Prague"), pytest.mark.exception_test] auth_account_start_balance = 0 diff --git a/tests/prague/eip7702_set_code_tx/test_set_code_txs.py b/tests/prague/eip7702_set_code_tx/test_set_code_txs.py index b2913bcfa1d..f8014ead269 100644 --- a/tests/prague/eip7702_set_code_tx/test_set_code_txs.py +++ b/tests/prague/eip7702_set_code_tx/test_set_code_txs.py @@ -2973,7 +2973,7 @@ def test_reset_code( ) -@pytest.mark.negative +@pytest.mark.exception_test def test_contract_create( state_test: StateTestFiller, pre: Alloc, @@ -3001,7 +3001,7 @@ def test_contract_create( ) -@pytest.mark.negative +@pytest.mark.exception_test def test_empty_authorization_list( state_test: StateTestFiller, pre: Alloc, @@ -3490,7 +3490,7 @@ def test_many_delegations( ) -@pytest.mark.negative +@pytest.mark.exception_test def test_invalid_transaction_after_authorization( blockchain_test: BlockchainTestFiller, pre: Alloc, @@ -3592,7 +3592,7 @@ def test_authorization_reusing_nonce( "self_sponsored", [True, False], ) -@pytest.mark.negative +@pytest.mark.exception_test @pytest.mark.execute(pytest.mark.skip(reason="Requires contract-eoa address collision")) def test_set_code_from_account_with_non_delegating_code( state_test: StateTestFiller, @@ -3673,7 +3673,7 @@ def test_set_code_from_account_with_non_delegating_code( "priority_greater_than_max_fee_per_gas", ], ) -@pytest.mark.negative +@pytest.mark.exception_test def test_set_code_transaction_fee_validations( state_test: StateTestFiller, pre: Alloc, diff --git a/tests/shanghai/eip3860_initcode/test_initcode.py b/tests/shanghai/eip3860_initcode/test_initcode.py index 7b46d4c0fba..da8faac541d 100644 --- a/tests/shanghai/eip3860_initcode/test_initcode.py +++ b/tests/shanghai/eip3860_initcode/test_initcode.py @@ -122,8 +122,8 @@ [ INITCODE_ZEROS_MAX_LIMIT, INITCODE_ONES_MAX_LIMIT, - pytest.param(INITCODE_ZEROS_OVER_LIMIT, marks=pytest.mark.negative), - pytest.param(INITCODE_ONES_OVER_LIMIT, marks=pytest.mark.negative), + pytest.param(INITCODE_ZEROS_OVER_LIMIT, marks=pytest.mark.exception_test), + pytest.param(INITCODE_ONES_OVER_LIMIT, marks=pytest.mark.exception_test), ], ids=get_initcode_name, ) @@ -184,7 +184,7 @@ def valid_gas_test_case(initcode: Initcode, gas_test_case: str) -> bool: pytest.param( i, g, - marks=([pytest.mark.negative] if g == "too_little_intrinsic_gas" else []), + marks=([pytest.mark.exception_test] if g == "too_little_intrinsic_gas" else []), ) for i in [ INITCODE_ZEROS_MAX_LIMIT, diff --git a/tests/shanghai/eip4895_withdrawals/test_withdrawals.py b/tests/shanghai/eip4895_withdrawals/test_withdrawals.py index df4fb0f4270..fc9305a83e9 100644 --- a/tests/shanghai/eip4895_withdrawals/test_withdrawals.py +++ b/tests/shanghai/eip4895_withdrawals/test_withdrawals.py @@ -39,7 +39,9 @@ "test_case", [ pytest.param( - "tx_in_withdrawals_block", id="tx_in_withdrawals_block", marks=pytest.mark.negative + "tx_in_withdrawals_block", + id="tx_in_withdrawals_block", + marks=pytest.mark.exception_test, ), pytest.param("tx_after_withdrawals_block", id="tx_after_withdrawals_block"), ], From fd8f023f70fb5e8856aa0078cabb903db16eab2a Mon Sep 17 00:00:00 2001 From: Mario Vega Date: Thu, 10 Apr 2025 22:54:38 +0000 Subject: [PATCH 6/8] refactor(specs): Make `request` a `PrivateAttr` --- src/cli/eofwrap.py | 1 - .../tests/test_transition_tools_support.py | 1 - src/ethereum_test_specs/base.py | 72 ++++++++++++++++++- src/ethereum_test_specs/blockchain.py | 52 +++----------- src/ethereum_test_specs/eof.py | 23 +++--- src/ethereum_test_specs/helpers.py | 16 ----- src/ethereum_test_specs/state.py | 25 ++----- src/ethereum_test_specs/tests/test_expect.py | 30 ++++---- .../tests/test_fixtures.py | 28 ++------ .../tests/test_transaction.py | 1 - src/ethereum_test_specs/transaction.py | 15 +--- src/ethereum_test_tools/tests/test_code.py | 1 - src/pytest_plugins/execute/execute.py | 1 + src/pytest_plugins/filler/filler.py | 2 +- 14 files changed, 115 insertions(+), 153 deletions(-) diff --git a/src/cli/eofwrap.py b/src/cli/eofwrap.py index 323ebdf9d2e..246ac303fb0 100644 --- a/src/cli/eofwrap.py +++ b/src/cli/eofwrap.py @@ -308,7 +308,6 @@ def _wrap_fixture(self, fixture: BlockchainFixture, traces: bool): raise TypeError("not a FixtureBlock") result = test.generate( - request=None, # type: ignore t8n=t8n, fork=Osaka, fixture_format=BlockchainFixture, diff --git a/src/ethereum_clis/tests/test_transition_tools_support.py b/src/ethereum_clis/tests/test_transition_tools_support.py index 48259efb9b3..3c6fd625c14 100644 --- a/src/ethereum_clis/tests/test_transition_tools_support.py +++ b/src/ethereum_clis/tests/test_transition_tools_support.py @@ -236,7 +236,6 @@ def test_t8n_support(fork: Fork, installed_t8n: TransitionTool): blocks=[block_1, block_2], ) test.generate( - request=None, # type: ignore t8n=installed_t8n, fork=fork, fixture_format=BlockchainFixture, diff --git a/src/ethereum_test_specs/base.py b/src/ethereum_test_specs/base.py index 6562ded765d..30381adf744 100644 --- a/src/ethereum_test_specs/base.py +++ b/src/ethereum_test_specs/base.py @@ -4,10 +4,10 @@ from functools import reduce from os import path from pathlib import Path -from typing import Callable, ClassVar, Dict, Generator, List, Optional, Sequence +from typing import Callable, ClassVar, Dict, Generator, List, Optional, Sequence, Type, TypeVar import pytest -from pydantic import BaseModel, Field +from pydantic import BaseModel, Field, PrivateAttr from ethereum_clis import Result, TransitionTool from ethereum_test_base_types import to_hex @@ -41,11 +41,16 @@ def verify_result(result: Result, env: Environment): assert result.withdrawals_root == to_hex(Withdrawal.list_root(env.withdrawals)) +T = TypeVar("T", bound="BaseTest") + + class BaseTest(BaseModel): """Represents a base Ethereum test which must return a single test fixture.""" tag: str = "" + _request: pytest.FixtureRequest | None = PrivateAttr(None) + # Transition tool specific fields t8n_dump_dir: Path | None = Field(None, exclude=True) t8n_call_counter: int = Field(0, exclude=True) @@ -65,6 +70,22 @@ def discard_fixture_format_by_marks( """Discard a fixture format from filling if the appropriate marker is used.""" return False + @classmethod + def from_test( + cls: Type[T], + *, + base_test: "BaseTest", + **kwargs, + ) -> T: + """Create a test in a different format from a base test.""" + new_instance = cls( + tag=base_test.tag, + t8n_dump_dir=base_test.t8n_dump_dir, + **kwargs, + ) + new_instance._request = base_test._request + return new_instance + @classmethod def discard_execute_format_by_marks( cls, @@ -79,7 +100,6 @@ def discard_execute_format_by_marks( def generate( self, *, - request: pytest.FixtureRequest, t8n: TransitionTool, fork: Fork, fixture_format: FixtureFormat, @@ -119,5 +139,51 @@ def get_next_transition_tool_output_path(self) -> str: str(current_value), ) + def is_slow_test(self) -> bool: + """Check if the test is slow.""" + if self._request is not None and hasattr(self._request, "node"): + return self._request.node.get_closest_marker("slow") is not None + return False + + def is_negative_test(self) -> bool | None: + """ + Check if the test is an exception test (invalid block, invalid transaction). + + `None` is returned if it's not possible to determine if the test is negative or not. + This is the case when the test is not run in pytest. + """ + if self._request is not None and hasattr(self._request, "node"): + return self._request.node.get_closest_marker("exception_test") is not None + return None + + def node_id(self) -> str: + """Return the node ID of the test.""" + if self._request is not None and hasattr(self._request, "node"): + return self._request.node.nodeid + return "" + + def check_negative_test( + self, + *, + exception: bool, + ): + """Compare the test marker against the outcome of the test.""" + negative_test_marker = self.is_negative_test() + if negative_test_marker is None: + return + if negative_test_marker != exception: + if exception: + raise Exception( + "Test produced an invalid block or transaction but was not marked with the " + "`exception_test` marker. Add the `@pytest.mark.exception_test` decorator " + "to the test." + ) + else: + raise Exception( + "Test didn't produce an invalid block or transaction but was marked with the " + "`exception_test` marker. Remove the `@pytest.mark.exception_test` decorator " + "from the test." + ) + TestSpec = Callable[[Fork], Generator[BaseTest, None, None]] diff --git a/src/ethereum_test_specs/blockchain.py b/src/ethereum_test_specs/blockchain.py index 59331d2f2eb..9218613c288 100644 --- a/src/ethereum_test_specs/blockchain.py +++ b/src/ethereum_test_specs/blockchain.py @@ -49,7 +49,7 @@ from .base import BaseTest, verify_result from .debugging import print_traces -from .helpers import is_negative_test, is_slow_test, verify_block, verify_transactions +from .helpers import verify_block, verify_transactions def environment_from_parent_header(parent: "FixtureHeader") -> "Environment": @@ -562,8 +562,6 @@ def make_fixture( t8n: TransitionTool, fork: Fork, eips: Optional[List[int]] = None, - slow: bool = False, - negative_test: bool = False, ) -> BlockchainFixture: """Create a fixture from the blockchain test definition.""" fixture_blocks: List[FixtureBlock | InvalidFixtureBlock] = [] @@ -586,7 +584,7 @@ def make_fixture( previous_env=env, previous_alloc=alloc, eips=eips, - slow=slow, + slow=self.is_slow_test(), ) fixture_block = FixtureBlockBase( header=header, @@ -606,10 +604,6 @@ def make_fixture( env = apply_new_parent(new_env, header) head = header.block_hash else: - assert negative_test, ( - "unmarked exception test: the test case produces an invalid block but " - + "does not specify the `pytest.mark.exception_test` marker" - ) fixture_blocks.append( InvalidFixtureBlock( rlp=fixture_block.rlp, @@ -627,10 +621,6 @@ def make_fixture( "test correctness: if the block's rlp is hard-coded, " + "the block is expected to produce an exception" ) - assert negative_test, ( - "unmarked exception test: the test case produces an invalid block but " - + "does not specify the `pytest.mark.exception_test` marker" - ) fixture_blocks.append( InvalidFixtureBlock( rlp=block.rlp, @@ -643,13 +633,7 @@ def make_fixture( self.verify_post_state( t8n, t8n_state=alloc, expected_state=block.expected_post_state ) - - if invalid_blocks == 0 and negative_test: - raise Exception( - "test correctness: the test case is marked as an exception test but " - + "no invalid blocks were found" - ) - + self.check_negative_test(exception=invalid_blocks > 0) self.verify_post_state(t8n, t8n_state=alloc) network_info = BlockchainTest.network_info(fork, eips) return BlockchainFixture( @@ -672,8 +656,6 @@ def make_hive_fixture( t8n: TransitionTool, fork: Fork, eips: Optional[List[int]] = None, - slow: bool = False, - negative_test: bool = False, ) -> BlockchainEngineFixture: """Create a hive fixture from the blocktest definition.""" fixture_payloads: List[FixtureEngineNewPayload] = [] @@ -682,7 +664,7 @@ def make_hive_fixture( alloc = pre env = environment_from_parent_header(genesis.header) head_hash = genesis.header.block_hash - + invalid_blocks = 0 for block in self.blocks: header, txs, requests, new_alloc, new_env = self.generate_block_data( t8n=t8n, @@ -691,7 +673,7 @@ def make_hive_fixture( previous_env=env, previous_alloc=alloc, eips=eips, - slow=slow, + slow=self.is_slow_test(), ) if block.rlp is None: fixture_payloads.append( @@ -710,16 +692,13 @@ def make_hive_fixture( env = apply_new_parent(env, header) head_hash = header.block_hash else: - assert negative_test, ( - "unmarked exception test: the test case produces an invalid block but " - + "does not specify the `pytest.mark.exception_test` marker" - ) + invalid_blocks += 1 if block.expected_post_state: self.verify_post_state( t8n, t8n_state=alloc, expected_state=block.expected_post_state ) - + self.check_negative_test(exception=invalid_blocks > 0) fcu_version = fork.engine_forkchoice_updated_version(header.number, header.timestamp) assert fcu_version is not None, ( "A hive fixture was requested but no forkchoice update is defined." @@ -775,7 +754,6 @@ def make_hive_fixture( def generate( self, - request: pytest.FixtureRequest, t8n: TransitionTool, fork: Fork, fixture_format: FixtureFormat, @@ -784,21 +762,9 @@ def generate( """Generate the BlockchainTest fixture.""" t8n.reset_traces() if fixture_format == BlockchainEngineFixture: - return self.make_hive_fixture( - t8n, - fork, - eips, - slow=is_slow_test(request), - negative_test=is_negative_test(request), - ) + return self.make_hive_fixture(t8n, fork, eips) elif fixture_format == BlockchainFixture: - return self.make_fixture( - t8n, - fork, - eips, - slow=is_slow_test(request), - negative_test=is_negative_test(request), - ) + return self.make_fixture(t8n, fork, eips) raise Exception(f"Unknown fixture format: {fixture_format}") diff --git a/src/ethereum_test_specs/eof.py b/src/ethereum_test_specs/eof.py index 5c409e40839..33357ffba02 100644 --- a/src/ethereum_test_specs/eof.py +++ b/src/ethereum_test_specs/eof.py @@ -305,7 +305,6 @@ def model_post_init(self, __context): def make_eof_test_fixture( self, *, - request: pytest.FixtureRequest, fork: Fork, eips: Optional[List[int]], ) -> EOFFixture: @@ -316,7 +315,7 @@ def make_eof_test_fixture( f"Duplicate EOF test: {container_bytes}, " f"existing test: {existing_tests[container_bytes]}" ) - existing_tests[container_bytes] = request.node.nodeid + existing_tests[container_bytes] = self.node_id() vectors = [ Vector( code=container_bytes, @@ -438,18 +437,17 @@ def generate_eof_contract_create_transaction(self) -> Transaction: def generate_state_test(self, fork: Fork) -> StateTest: """Generate the StateTest filler.""" - return StateTest( + return StateTest.from_test( + base_test=self, pre=self.pre, tx=self.generate_eof_contract_create_transaction(), env=Environment(), post=self.post, - t8n_dump_dir=self.t8n_dump_dir, ) def generate( self, *, - request: pytest.FixtureRequest, t8n: TransitionTool, fork: Fork, eips: Optional[List[int]] = None, @@ -458,10 +456,10 @@ def generate( ) -> BaseFixture: """Generate the BlockchainTest fixture.""" if fixture_format == EOFFixture: - return self.make_eof_test_fixture(request=request, fork=fork, eips=eips) + return self.make_eof_test_fixture(fork=fork, eips=eips) elif fixture_format in StateTest.supported_fixture_formats: return self.generate_state_test(fork).generate( - request=request, t8n=t8n, fork=fork, fixture_format=fixture_format, eips=eips + t8n=t8n, fork=fork, fixture_format=fixture_format, eips=eips ) raise Exception(f"Unknown fixture format: {fixture_format}") @@ -583,18 +581,17 @@ def generate_state_test(self, fork: Fork) -> StateTest: assert self.pre is not None, "pre must be set to generate a StateTest." assert self.post is not None, "post must be set to generate a StateTest." - return StateTest( + return StateTest.from_test( + base_test=self, pre=self.pre, tx=self, env=self.env, post=self.post, - t8n_dump_dir=self.t8n_dump_dir, ) def generate( self, *, - request: pytest.FixtureRequest, t8n: TransitionTool, fork: Fork, eips: Optional[List[int]] = None, @@ -606,11 +603,11 @@ def generate( if Bytes(self.container) in existing_tests: # Gracefully skip duplicate tests because one EOFStateTest can generate multiple # state fixtures with the same data. - pytest.skip(f"Duplicate EOF container on EOFStateTest: {request.node.nodeid}") - return self.make_eof_test_fixture(request=request, fork=fork, eips=eips) + pytest.skip(f"Duplicate EOF container on EOFStateTest: {self.node_id()}") + return self.make_eof_test_fixture(fork=fork, eips=eips) elif fixture_format in StateTest.supported_fixture_formats: return self.generate_state_test(fork).generate( - request=request, t8n=t8n, fork=fork, fixture_format=fixture_format, eips=eips + t8n=t8n, fork=fork, fixture_format=fixture_format, eips=eips ) raise Exception(f"Unknown fixture format: {fixture_format}") diff --git a/src/ethereum_test_specs/helpers.py b/src/ethereum_test_specs/helpers.py index 3bc07185d6a..7537b9240a9 100644 --- a/src/ethereum_test_specs/helpers.py +++ b/src/ethereum_test_specs/helpers.py @@ -4,8 +4,6 @@ from enum import Enum from typing import Any, Dict, List -import pytest - from ethereum_clis import Result from ethereum_test_exceptions import ( BlockException, @@ -306,17 +304,3 @@ def verify_block( got_exception=result.block_exception, ) info.verify(strict_match=transition_tool_exceptions_reliable) - - -def is_slow_test(request: pytest.FixtureRequest) -> bool: - """Check if the test is slow.""" - if hasattr(request, "node"): - return request.node.get_closest_marker("slow") is not None - return False - - -def is_negative_test(request: pytest.FixtureRequest) -> bool: - """Check if the test is an exception test (invalid block, invalid transaction).""" - if hasattr(request, "node"): - return request.node.get_closest_marker("exception_test") is not None - return False diff --git a/src/ethereum_test_specs/state.py b/src/ethereum_test_specs/state.py index 9c93fb23c6c..85e23ec3753 100644 --- a/src/ethereum_test_specs/state.py +++ b/src/ethereum_test_specs/state.py @@ -34,7 +34,7 @@ from .base import BaseTest from .blockchain import Block, BlockchainTest, Header from .debugging import print_traces -from .helpers import is_negative_test, is_slow_test, verify_transactions +from .helpers import verify_transactions class StateTest(BaseTest): @@ -142,12 +142,12 @@ def _generate_blockchain_blocks(self, *, fork: Fork) -> List[Block]: def generate_blockchain_test(self, *, fork: Fork) -> BlockchainTest: """Generate a BlockchainTest fixture from this StateTest fixture.""" - return BlockchainTest( + return BlockchainTest.from_test( + base_test=self, genesis_environment=self._generate_blockchain_genesis_environment(fork=fork), pre=self.pre, post=self.post, blocks=self._generate_blockchain_blocks(fork=fork), - t8n_dump_dir=self.t8n_dump_dir, ) def make_state_test_fixture( @@ -155,7 +155,6 @@ def make_state_test_fixture( t8n: TransitionTool, fork: Fork, eips: Optional[List[int]] = None, - slow: bool = False, ) -> StateFixture: """Create a fixture from the state test definition.""" # We can't generate a state test fixture that names a transition fork, @@ -182,7 +181,7 @@ def make_state_test_fixture( eips=eips, debug_output_path=self.get_next_transition_tool_output_path(), state_test=True, - slow_request=slow, + slow_request=self.is_slow_test(), ) try: @@ -226,29 +225,19 @@ def make_state_test_fixture( def generate( self, - request: pytest.FixtureRequest, t8n: TransitionTool, fork: Fork, fixture_format: FixtureFormat, eips: Optional[List[int]] = None, ) -> BaseFixture: """Generate the BlockchainTest fixture.""" - if self.tx.error is not None and not is_negative_test(request): - raise Exception( - "State tests with an error code should be marked with the " - "`pytest.mark.exception_test` test marker" - ) - elif self.tx.error is None and is_negative_test(request): - raise Exception( - "State tests without an error code should not be marked with the " - "`pytest.mark.exception_test` test marker" - ) + self.check_negative_test(exception=self.tx.error is not None) if fixture_format in BlockchainTest.supported_fixture_formats: return self.generate_blockchain_test(fork=fork).generate( - request=request, t8n=t8n, fork=fork, fixture_format=fixture_format, eips=eips + t8n=t8n, fork=fork, fixture_format=fixture_format, eips=eips ) elif fixture_format == StateFixture: - return self.make_state_test_fixture(t8n, fork, eips, slow=is_slow_test(request)) + return self.make_state_test_fixture(t8n, fork, eips) raise Exception(f"Unknown fixture format: {fixture_format}") diff --git a/src/ethereum_test_specs/tests/test_expect.py b/src/ethereum_test_specs/tests/test_expect.py index 500e7e3e8ee..24ae4002a79 100644 --- a/src/ethereum_test_specs/tests/test_expect.py +++ b/src/ethereum_test_specs/tests/test_expect.py @@ -120,7 +120,7 @@ def state_test( # noqa: D103 def test_post_storage_value_mismatch(expected_exception, state_test, default_t8n, fork): """Test post state `Account.storage` exceptions during state test fixture generation.""" with pytest.raises(Storage.KeyValueMismatchError) as e_info: - state_test.generate(request=None, t8n=default_t8n, fork=fork, fixture_format=StateFixture) + state_test.generate(t8n=default_t8n, fork=fork, fixture_format=StateFixture) assert e_info.value == expected_exception @@ -146,10 +146,10 @@ def test_post_nonce_value_mismatch(pre: Alloc, post: Alloc, state_test, default_ pre_nonce = pre_account.nonce post_nonce = post_account.nonce if "nonce" not in post_account.model_fields_set: # no exception - state_test.generate(request=None, t8n=default_t8n, fork=fork, fixture_format=StateFixture) + state_test.generate(t8n=default_t8n, fork=fork, fixture_format=StateFixture) return with pytest.raises(Account.NonceMismatchError) as e_info: - state_test.generate(request=None, t8n=default_t8n, fork=fork, fixture_format=StateFixture) + state_test.generate(t8n=default_t8n, fork=fork, fixture_format=StateFixture) assert e_info.value == Account.NonceMismatchError( address=ADDRESS_UNDER_TEST, want=post_nonce, got=pre_nonce ) @@ -177,10 +177,10 @@ def test_post_code_value_mismatch(pre: Alloc, post: Alloc, state_test, default_t pre_code = pre_account.code post_code = post_account.code if "code" not in post_account.model_fields_set: # no exception - state_test.generate(request=None, t8n=default_t8n, fork=fork, fixture_format=StateFixture) + state_test.generate(t8n=default_t8n, fork=fork, fixture_format=StateFixture) return with pytest.raises(Account.CodeMismatchError) as e_info: - state_test.generate(request=None, t8n=default_t8n, fork=fork, fixture_format=StateFixture) + state_test.generate(t8n=default_t8n, fork=fork, fixture_format=StateFixture) assert e_info.value == Account.CodeMismatchError( address=ADDRESS_UNDER_TEST, want=post_code, got=pre_code ) @@ -208,10 +208,10 @@ def test_post_balance_value_mismatch(pre: Alloc, post: Alloc, state_test, defaul pre_balance = pre_account.balance post_balance = post_account.balance if "balance" not in post_account.model_fields_set: # no exception - state_test.generate(request=None, t8n=default_t8n, fork=fork, fixture_format=StateFixture) + state_test.generate(t8n=default_t8n, fork=fork, fixture_format=StateFixture) return with pytest.raises(Account.BalanceMismatchError) as e_info: - state_test.generate(request=None, t8n=default_t8n, fork=fork, fixture_format=StateFixture) + state_test.generate(t8n=default_t8n, fork=fork, fixture_format=StateFixture) assert e_info.value == Account.BalanceMismatchError( address=ADDRESS_UNDER_TEST, want=post_balance, got=pre_balance ) @@ -252,10 +252,10 @@ def test_post_account_mismatch( fixture generation. """ if exception_type is None: - state_test.generate(request=None, t8n=default_t8n, fork=fork, fixture_format=StateFixture) + state_test.generate(t8n=default_t8n, fork=fork, fixture_format=StateFixture) return with pytest.raises(exception_type) as _: - state_test.generate(request=None, t8n=default_t8n, fork=fork, fixture_format=StateFixture) + state_test.generate(t8n=default_t8n, fork=fork, fixture_format=StateFixture) # Transaction result mismatch tests @@ -344,14 +344,10 @@ def test_transaction_expectation( f"({default_t8n.__class__.__name__})." ) if exception_type is None: - state_test.generate( - request=None, t8n=default_t8n, fork=fork, fixture_format=fixture_format - ) + state_test.generate(t8n=default_t8n, fork=fork, fixture_format=fixture_format) else: with pytest.raises(exception_type) as _: - state_test.generate( - request=None, t8n=default_t8n, fork=fork, fixture_format=fixture_format - ) + state_test.generate(t8n=default_t8n, fork=fork, fixture_format=fixture_format) @pytest.mark.parametrize( @@ -428,7 +424,7 @@ def test_block_intermediate_state( post=block_3.expected_post_state, blocks=[block_1, block_2, block_3], ).generate( - request=None, # type: ignore + # type: ignore t8n=default_t8n, fork=fork, fixture_format=fixture_format, @@ -443,7 +439,7 @@ def test_block_intermediate_state( post=block_3.expected_post_state, blocks=[block_1, block_2, block_3], ).generate( - request=None, # type: ignore + # type: ignore t8n=default_t8n, fork=fork, fixture_format=fixture_format, diff --git a/src/ethereum_test_specs/tests/test_fixtures.py b/src/ethereum_test_specs/tests/test_fixtures.py index 85178c07601..5d9cc8eac50 100644 --- a/src/ethereum_test_specs/tests/test_fixtures.py +++ b/src/ethereum_test_specs/tests/test_fixtures.py @@ -87,12 +87,7 @@ def test_make_genesis(fork: Fork, fixture_hash: bytes, default_t8n: TransitionTo post={}, blocks=[], tag="some_state_test", - ).generate( - request=None, # type: ignore - t8n=default_t8n, - fork=fork, - fixture_format=BlockchainFixture, - ) + ).generate(t8n=default_t8n, fork=fork, fixture_format=BlockchainFixture) assert isinstance(fixture, BlockchainFixture) assert fixture.genesis is not None @@ -182,12 +177,7 @@ def test_fill_state_test( post=post, tx=tx, tag="my_chain_id_test", - ).generate( - request=None, # type: ignore - t8n=default_t8n, - fork=fork, - fixture_format=fixture_format, - ) + ).generate(t8n=default_t8n, fork=fork, fixture_format=fixture_format) assert generated_fixture.__class__ == fixture_format fixture = { f"000/my_chain_id_test/{fork}/tx_type_{tx_type}": generated_fixture.json_dict_with_info( @@ -515,12 +505,7 @@ def blockchain_test_fixture( # noqa: D102 blocks=blocks, genesis_environment=genesis_environment, tag="my_blockchain_test_valid_txs", - ).generate( - request=None, # type: ignore - t8n=default_t8n, - fork=fork, - fixture_format=fixture_format, - ) + ).generate(t8n=default_t8n, fork=fork, fixture_format=fixture_format) @pytest.mark.parametrize("fork", [London, Shanghai], indirect=True) def test_fill_blockchain_valid_txs( # noqa: D102 @@ -896,12 +881,7 @@ def test_fill_blockchain_invalid_txs( post=post, blocks=blocks, genesis_environment=genesis_environment, - ).generate( - request=None, # type: ignore - t8n=default_t8n, - fork=fork, - fixture_format=fixture_format, - ) + ).generate(t8n=default_t8n, fork=fork, fixture_format=fixture_format) assert generated_fixture.__class__ == fixture_format assert isinstance(generated_fixture, BlockchainFixtureCommon) diff --git a/src/ethereum_test_specs/tests/test_transaction.py b/src/ethereum_test_specs/tests/test_transaction.py index fbc33907177..a161a06db2f 100644 --- a/src/ethereum_test_specs/tests/test_transaction.py +++ b/src/ethereum_test_specs/tests/test_transaction.py @@ -22,7 +22,6 @@ def test_transaction_test_filling(name: str, tx: Transaction, fork: Fork): """Test the transaction test filling.""" generated_fixture = TransactionTest(tx=tx.with_signature_and_sender()).generate( - request=None, # type: ignore t8n=None, # type: ignore fork=fork, fixture_format=TransactionFixture, diff --git a/src/ethereum_test_specs/transaction.py b/src/ethereum_test_specs/transaction.py index 94e9472ae0e..fa22dcd1056 100644 --- a/src/ethereum_test_specs/transaction.py +++ b/src/ethereum_test_specs/transaction.py @@ -2,8 +2,6 @@ from typing import Callable, ClassVar, Generator, List, Optional, Sequence, Type -import pytest - from ethereum_clis import TransitionTool from ethereum_test_execution import ( BaseExecute, @@ -22,7 +20,6 @@ from ethereum_test_types import Alloc, Transaction from .base import BaseTest -from .helpers import is_negative_test class TransactionTest(BaseTest): @@ -79,23 +76,13 @@ def make_transaction_test_fixture( def generate( self, - request: pytest.FixtureRequest, t8n: TransitionTool, fork: Fork, fixture_format: FixtureFormat, eips: Optional[List[int]] = None, ) -> BaseFixture: """Generate the TransactionTest fixture.""" - if self.tx.error is not None and not is_negative_test(request): - raise Exception( - "Transaction tests with an error code should be marked with the " - "`pytest.mark.exception_test` test marker." - ) - elif self.tx.error is None and is_negative_test(request): - raise Exception( - "Transaction tests without an error code should not be marked with " - "`pytest.mark.exception_test` test marker." - ) + self.check_negative_test(exception=self.tx.error is not None) if fixture_format == TransactionFixture: return self.make_transaction_test_fixture(fork, eips) diff --git a/src/ethereum_test_tools/tests/test_code.py b/src/ethereum_test_tools/tests/test_code.py index f4974ba5dfd..70ec9c3899a 100644 --- a/src/ethereum_test_tools/tests/test_code.py +++ b/src/ethereum_test_tools/tests/test_code.py @@ -630,7 +630,6 @@ def test_switch( post=post, ) state_test.generate( - request=None, # type: ignore t8n=default_t8n, fork=Cancun, fixture_format=BlockchainFixture, diff --git a/src/pytest_plugins/execute/execute.py b/src/pytest_plugins/execute/execute.py index 45fe60efc5a..24ece89e077 100644 --- a/src/pytest_plugins/execute/execute.py +++ b/src/pytest_plugins/execute/execute.py @@ -268,6 +268,7 @@ def __init__(self, *args, **kwargs): request.node.config.sender_address = str(pre._sender) super(BaseTestWrapper, self).__init__(*args, **kwargs) + self._request = request # wait for pre-requisite transactions to be included in blocks pre.wait_for_transactions() diff --git a/src/pytest_plugins/filler/filler.py b/src/pytest_plugins/filler/filler.py index d9893f51df4..c1201911a56 100644 --- a/src/pytest_plugins/filler/filler.py +++ b/src/pytest_plugins/filler/filler.py @@ -717,8 +717,8 @@ def __init__(self, *args, **kwargs): if "pre" not in kwargs: kwargs["pre"] = pre super(BaseTestWrapper, self).__init__(*args, **kwargs) + self._request = request fixture = self.generate( - request=request, t8n=t8n, fork=fork, fixture_format=fixture_format, From 3b886c0ce794dc50b73eec3578fd1b50af77dc65 Mon Sep 17 00:00:00 2001 From: Mario Vega Date: Thu, 10 Apr 2025 23:03:44 +0000 Subject: [PATCH 7/8] nit --- src/ethereum_test_specs/tests/test_expect.py | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/src/ethereum_test_specs/tests/test_expect.py b/src/ethereum_test_specs/tests/test_expect.py index 24ae4002a79..94df1c9bbea 100644 --- a/src/ethereum_test_specs/tests/test_expect.py +++ b/src/ethereum_test_specs/tests/test_expect.py @@ -423,12 +423,7 @@ def test_block_intermediate_state( pre=pre, post=block_3.expected_post_state, blocks=[block_1, block_2, block_3], - ).generate( - # type: ignore - t8n=default_t8n, - fork=fork, - fixture_format=fixture_format, - ) + ).generate(t8n=default_t8n, fork=fork, fixture_format=fixture_format) return else: BlockchainTest( @@ -438,9 +433,4 @@ def test_block_intermediate_state( pre=pre, post=block_3.expected_post_state, blocks=[block_1, block_2, block_3], - ).generate( - # type: ignore - t8n=default_t8n, - fork=fork, - fixture_format=fixture_format, - ) + ).generate(t8n=default_t8n, fork=fork, fixture_format=fixture_format) From fe4c01f1029704aa24abfe3adbe67c0a952a3d08 Mon Sep 17 00:00:00 2001 From: Mario Vega Date: Fri, 11 Apr 2025 17:49:57 +0000 Subject: [PATCH 8/8] fix(specs): Review suggestions Co-authored-by: spencer --- src/ethereum_test_specs/base.py | 6 +++--- src/ethereum_test_specs/blockchain.py | 4 ++-- src/ethereum_test_specs/state.py | 2 +- src/ethereum_test_specs/transaction.py | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/ethereum_test_specs/base.py b/src/ethereum_test_specs/base.py index 30381adf744..d60556d5bd1 100644 --- a/src/ethereum_test_specs/base.py +++ b/src/ethereum_test_specs/base.py @@ -145,7 +145,7 @@ def is_slow_test(self) -> bool: return self._request.node.get_closest_marker("slow") is not None return False - def is_negative_test(self) -> bool | None: + def is_exception_test(self) -> bool | None: """ Check if the test is an exception test (invalid block, invalid transaction). @@ -162,13 +162,13 @@ def node_id(self) -> str: return self._request.node.nodeid return "" - def check_negative_test( + def check_exception_test( self, *, exception: bool, ): """Compare the test marker against the outcome of the test.""" - negative_test_marker = self.is_negative_test() + negative_test_marker = self.is_exception_test() if negative_test_marker is None: return if negative_test_marker != exception: diff --git a/src/ethereum_test_specs/blockchain.py b/src/ethereum_test_specs/blockchain.py index 9218613c288..b72d84b22cd 100644 --- a/src/ethereum_test_specs/blockchain.py +++ b/src/ethereum_test_specs/blockchain.py @@ -633,7 +633,7 @@ def make_fixture( self.verify_post_state( t8n, t8n_state=alloc, expected_state=block.expected_post_state ) - self.check_negative_test(exception=invalid_blocks > 0) + self.check_exception_test(exception=invalid_blocks > 0) self.verify_post_state(t8n, t8n_state=alloc) network_info = BlockchainTest.network_info(fork, eips) return BlockchainFixture( @@ -698,7 +698,7 @@ def make_hive_fixture( self.verify_post_state( t8n, t8n_state=alloc, expected_state=block.expected_post_state ) - self.check_negative_test(exception=invalid_blocks > 0) + self.check_exception_test(exception=invalid_blocks > 0) fcu_version = fork.engine_forkchoice_updated_version(header.number, header.timestamp) assert fcu_version is not None, ( "A hive fixture was requested but no forkchoice update is defined." diff --git a/src/ethereum_test_specs/state.py b/src/ethereum_test_specs/state.py index 85e23ec3753..be4e03c0c4e 100644 --- a/src/ethereum_test_specs/state.py +++ b/src/ethereum_test_specs/state.py @@ -231,7 +231,7 @@ def generate( eips: Optional[List[int]] = None, ) -> BaseFixture: """Generate the BlockchainTest fixture.""" - self.check_negative_test(exception=self.tx.error is not None) + self.check_exception_test(exception=self.tx.error is not None) if fixture_format in BlockchainTest.supported_fixture_formats: return self.generate_blockchain_test(fork=fork).generate( t8n=t8n, fork=fork, fixture_format=fixture_format, eips=eips diff --git a/src/ethereum_test_specs/transaction.py b/src/ethereum_test_specs/transaction.py index fa22dcd1056..b4f96d8b8fa 100644 --- a/src/ethereum_test_specs/transaction.py +++ b/src/ethereum_test_specs/transaction.py @@ -82,7 +82,7 @@ def generate( eips: Optional[List[int]] = None, ) -> BaseFixture: """Generate the TransactionTest fixture.""" - self.check_negative_test(exception=self.tx.error is not None) + self.check_exception_test(exception=self.tx.error is not None) if fixture_format == TransactionFixture: return self.make_transaction_test_fixture(fork, eips)