From 8a83fce3abb3cf492cac7b4ac39c6efab643e9e0 Mon Sep 17 00:00:00 2001 From: protolambda Date: Fri, 26 Jul 2019 23:50:11 +0200 Subject: [PATCH] fixes to decorator order, and make functions fully yield, with pytest compat. --- test_libs/pyspec/eth2spec/test/context.py | 39 ++++++++++--------- .../test/genesis/test_initialization.py | 4 +- .../eth2spec/test/genesis/test_validity.py | 14 +++---- .../test_process_attestation.py | 2 +- .../test_process_attester_slashing.py | 18 ++++----- .../test_process_block_header.py | 2 +- .../block_processing/test_process_deposit.py | 4 +- .../test_process_proposer_slashing.py | 6 +-- .../block_processing/test_process_transfer.py | 2 +- .../test_process_voluntary_exit.py | 2 +- ...est_process_early_derived_secret_reveal.py | 16 ++++---- .../pyspec/eth2spec/test/test_finality.py | 10 ++--- test_libs/pyspec/eth2spec/test/utils.py | 23 +++++++---- 13 files changed, 75 insertions(+), 67 deletions(-) diff --git a/test_libs/pyspec/eth2spec/test/context.py b/test_libs/pyspec/eth2spec/test/context.py index 71d38dcf1..5a0ddb59d 100644 --- a/test_libs/pyspec/eth2spec/test/context.py +++ b/test_libs/pyspec/eth2spec/test/context.py @@ -4,7 +4,7 @@ from eth2spec.utils import bls from .helpers.genesis import create_genesis_state -from .utils import spectest, with_meta_tags +from .utils import vector_test, with_meta_tags def with_state(fn): @@ -12,7 +12,7 @@ def with_state(fn): try: kw['state'] = create_genesis_state(spec=kw['spec'], num_validators=spec_phase0.SLOTS_PER_EPOCH * 8) except KeyError: - raise TypeError('Spec decorator must come before state decorator to inject spec into state.') + raise TypeError('Spec decorator must come within state decorator to inject spec into state.') return fn(*args, **kw) return entry @@ -27,15 +27,18 @@ def with_state(fn): DEFAULT_BLS_ACTIVE = False -def spectest_with_bls_switch(fn): - # Bls switch must be wrapped by spectest, +def spec_test(fn): + # Bls switch must be wrapped by vector_test, # to fully go through the yielded bls switch data, before setting back the BLS setting. - return spectest()(bls_switch(fn)) + # A test may apply BLS overrides such as @always_bls, + # but if it yields data (n.b. @always_bls yields the bls setting), it should be wrapped by this decorator. + # This is why @alway_bls has its own bls switch, since the override is beyond the reach of the outer switch. + return vector_test()(bls_switch(fn)) -# shorthand for decorating @with_state @spectest() +# shorthand for decorating @spectest() @with_state def spec_state_test(fn): - return with_state(spectest_with_bls_switch(fn)) + return spec_test(with_state(fn)) def expect_assertion_error(fn): @@ -52,40 +55,38 @@ def expect_assertion_error(fn): raise AssertionError('expected an assertion error, but got none.') -# Tags a test to be ignoring BLS for it to pass. -bls_ignored = with_meta_tags({'bls_setting': 2}) - - def never_bls(fn): """ Decorator to apply on ``bls_switch`` decorator to force BLS de-activation. Useful to mark tests as BLS-ignorant. + This decorator may only be applied to yielding spec test functions, and should be wrapped by vector_test, + as the yielding needs to complete before setting back the BLS setting. """ def entry(*args, **kw): # override bls setting kw['bls_active'] = False - return fn(*args, **kw) - return bls_ignored(entry) - - -# Tags a test to be requiring BLS for it to pass. -bls_required = with_meta_tags({'bls_setting': 1}) + return bls_switch(fn)(*args, **kw) + return with_meta_tags({'bls_setting': 2})(entry) def always_bls(fn): """ Decorator to apply on ``bls_switch`` decorator to force BLS activation. Useful to mark tests as BLS-dependent. + This decorator may only be applied to yielding spec test functions, and should be wrapped by vector_test, + as the yielding needs to complete before setting back the BLS setting. """ def entry(*args, **kw): # override bls setting kw['bls_active'] = True - return fn(*args, **kw) - return bls_required(entry) + return bls_switch(fn)(*args, **kw) + return with_meta_tags({'bls_setting': 1})(entry) def bls_switch(fn): """ Decorator to make a function execute with BLS ON, or BLS off. Based on an optional bool argument ``bls_active``, passed to the function at runtime. + This decorator may only be applied to yielding spec test functions, and should be wrapped by vector_test, + as the yielding needs to complete before setting back the BLS setting. """ def entry(*args, **kw): old_state = bls.bls_active diff --git a/test_libs/pyspec/eth2spec/test/genesis/test_initialization.py b/test_libs/pyspec/eth2spec/test/genesis/test_initialization.py index b95b70fef..2ff57be74 100644 --- a/test_libs/pyspec/eth2spec/test/genesis/test_initialization.py +++ b/test_libs/pyspec/eth2spec/test/genesis/test_initialization.py @@ -1,11 +1,11 @@ -from eth2spec.test.context import spectest_with_bls_switch, with_phases +from eth2spec.test.context import spec_test, with_phases from eth2spec.test.helpers.deposits import ( prepare_genesis_deposits, ) @with_phases(['phase0']) -@spectest_with_bls_switch +@spec_test def test_initialize_beacon_state_from_eth1(spec): deposit_count = spec.MIN_GENESIS_ACTIVE_VALIDATOR_COUNT deposits, deposit_root = prepare_genesis_deposits(spec, deposit_count, spec.MAX_EFFECTIVE_BALANCE, signed=True) diff --git a/test_libs/pyspec/eth2spec/test/genesis/test_validity.py b/test_libs/pyspec/eth2spec/test/genesis/test_validity.py index bb95bb2b0..07ad3a73c 100644 --- a/test_libs/pyspec/eth2spec/test/genesis/test_validity.py +++ b/test_libs/pyspec/eth2spec/test/genesis/test_validity.py @@ -1,4 +1,4 @@ -from eth2spec.test.context import spectest_with_bls_switch, with_phases +from eth2spec.test.context import spec_test, with_phases from eth2spec.test.helpers.deposits import ( prepare_genesis_deposits, ) @@ -26,7 +26,7 @@ def run_is_valid_genesis_state(spec, state, valid=True): @with_phases(['phase0']) -@spectest_with_bls_switch +@spec_test def test_is_valid_genesis_state_true(spec): state = create_valid_beacon_state(spec) @@ -34,7 +34,7 @@ def test_is_valid_genesis_state_true(spec): @with_phases(['phase0']) -@spectest_with_bls_switch +@spec_test def test_is_valid_genesis_state_false_invalid_timestamp(spec): state = create_valid_beacon_state(spec) state.genesis_time = spec.MIN_GENESIS_TIME - 1 @@ -43,7 +43,7 @@ def test_is_valid_genesis_state_false_invalid_timestamp(spec): @with_phases(['phase0']) -@spectest_with_bls_switch +@spec_test def test_is_valid_genesis_state_true_more_balance(spec): state = create_valid_beacon_state(spec) state.validators[0].effective_balance = spec.MAX_EFFECTIVE_BALANCE + 1 @@ -53,7 +53,7 @@ def test_is_valid_genesis_state_true_more_balance(spec): # TODO: not part of the genesis function yet. Erroneously merged. # @with_phases(['phase0']) -# @spectest_with_bls_switch +# @spec_test # def test_is_valid_genesis_state_false_not_enough_balance(spec): # state = create_valid_beacon_state(spec) # state.validators[0].effective_balance = spec.MAX_EFFECTIVE_BALANCE - 1 @@ -62,7 +62,7 @@ def test_is_valid_genesis_state_true_more_balance(spec): @with_phases(['phase0']) -@spectest_with_bls_switch +@spec_test def test_is_valid_genesis_state_true_one_more_validator(spec): deposit_count = spec.MIN_GENESIS_ACTIVE_VALIDATOR_COUNT + 1 deposits, _ = prepare_genesis_deposits(spec, deposit_count, spec.MAX_EFFECTIVE_BALANCE, signed=True) @@ -75,7 +75,7 @@ def test_is_valid_genesis_state_true_one_more_validator(spec): @with_phases(['phase0']) -@spectest_with_bls_switch +@spec_test def test_is_valid_genesis_state_false_not_enough_validator(spec): deposit_count = spec.MIN_GENESIS_ACTIVE_VALIDATOR_COUNT - 1 deposits, _ = prepare_genesis_deposits(spec, deposit_count, spec.MAX_EFFECTIVE_BALANCE, signed=True) diff --git a/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attestation.py b/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attestation.py index ab46a0d8c..ee1c1b397 100644 --- a/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attestation.py +++ b/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attestation.py @@ -116,8 +116,8 @@ def test_wrong_end_epoch_with_max_epochs_per_crosslink(spec, state): @with_all_phases -@always_bls @spec_state_test +@always_bls def test_invalid_attestation_signature(spec, state): attestation = get_valid_attestation(spec, state) state.slot += spec.MIN_ATTESTATION_INCLUSION_DELAY diff --git a/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attester_slashing.py b/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attester_slashing.py index 7a6030157..20a510648 100644 --- a/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attester_slashing.py +++ b/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_attester_slashing.py @@ -108,8 +108,8 @@ def test_success_surround(spec, state): @with_all_phases -@always_bls @spec_state_test +@always_bls def test_success_already_exited_recent(spec, state): attester_slashing = get_valid_attester_slashing(spec, state, signed_1=True, signed_2=True) slashed_indices = ( @@ -123,8 +123,8 @@ def test_success_already_exited_recent(spec, state): @with_all_phases -@always_bls @spec_state_test +@always_bls def test_success_already_exited_long_ago(spec, state): attester_slashing = get_valid_attester_slashing(spec, state, signed_1=True, signed_2=True) slashed_indices = ( @@ -139,24 +139,24 @@ def test_success_already_exited_long_ago(spec, state): @with_all_phases -@always_bls @spec_state_test +@always_bls def test_invalid_sig_1(spec, state): attester_slashing = get_valid_attester_slashing(spec, state, signed_1=False, signed_2=True) yield from run_attester_slashing_processing(spec, state, attester_slashing, False) @with_all_phases -@always_bls @spec_state_test +@always_bls def test_invalid_sig_2(spec, state): attester_slashing = get_valid_attester_slashing(spec, state, signed_1=True, signed_2=False) yield from run_attester_slashing_processing(spec, state, attester_slashing, False) @with_all_phases -@always_bls @spec_state_test +@always_bls def test_invalid_sig_1_and_2(spec, state): attester_slashing = get_valid_attester_slashing(spec, state, signed_1=False, signed_2=False) yield from run_attester_slashing_processing(spec, state, attester_slashing, False) @@ -212,9 +212,9 @@ def test_custody_bit_0_and_1_intersect(spec, state): yield from run_attester_slashing_processing(spec, state, attester_slashing, False) -@always_bls @with_all_phases @spec_state_test +@always_bls def test_att1_bad_extra_index(spec, state): attester_slashing = get_valid_attester_slashing(spec, state, signed_1=True, signed_2=True) @@ -228,9 +228,9 @@ def test_att1_bad_extra_index(spec, state): yield from run_attester_slashing_processing(spec, state, attester_slashing, False) -@always_bls @with_all_phases @spec_state_test +@always_bls def test_att1_bad_replaced_index(spec, state): attester_slashing = get_valid_attester_slashing(spec, state, signed_1=True, signed_2=True) @@ -244,9 +244,9 @@ def test_att1_bad_replaced_index(spec, state): yield from run_attester_slashing_processing(spec, state, attester_slashing, False) -@always_bls @with_all_phases @spec_state_test +@always_bls def test_att2_bad_extra_index(spec, state): attester_slashing = get_valid_attester_slashing(spec, state, signed_1=True, signed_2=True) @@ -260,9 +260,9 @@ def test_att2_bad_extra_index(spec, state): yield from run_attester_slashing_processing(spec, state, attester_slashing, False) -@always_bls @with_all_phases @spec_state_test +@always_bls def test_att2_bad_replaced_index(spec, state): attester_slashing = get_valid_attester_slashing(spec, state, signed_1=True, signed_2=True) diff --git a/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_block_header.py b/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_block_header.py index a2306ef4d..c790c612c 100644 --- a/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_block_header.py +++ b/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_block_header.py @@ -42,8 +42,8 @@ def test_success_block_header(spec, state): @with_all_phases -@always_bls @spec_state_test +@always_bls def test_invalid_sig_block_header(spec, state): block = build_empty_block_for_next_slot(spec, state) yield from run_block_header_processing(spec, state, block, valid=False) diff --git a/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_deposit.py b/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_deposit.py index 3dbbeedf0..d1ffbd7c9 100644 --- a/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_deposit.py +++ b/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_deposit.py @@ -94,8 +94,8 @@ def test_new_deposit_over_max(spec, state): @with_all_phases -@always_bls @spec_state_test +@always_bls def test_invalid_sig_new_deposit(spec, state): # fresh deposit = next validator index = validator appended to registry validator_index = len(state.validators) @@ -115,8 +115,8 @@ def test_success_top_up(spec, state): @with_all_phases -@always_bls @spec_state_test +@always_bls def test_invalid_sig_top_up(spec, state): validator_index = 0 amount = spec.MAX_EFFECTIVE_BALANCE // 4 diff --git a/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_proposer_slashing.py b/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_proposer_slashing.py index af34ea709..5eaec9f03 100644 --- a/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_proposer_slashing.py +++ b/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_proposer_slashing.py @@ -49,24 +49,24 @@ def test_success(spec, state): @with_all_phases -@always_bls @spec_state_test +@always_bls def test_invalid_sig_1(spec, state): proposer_slashing = get_valid_proposer_slashing(spec, state, signed_1=False, signed_2=True) yield from run_proposer_slashing_processing(spec, state, proposer_slashing, False) @with_all_phases -@always_bls @spec_state_test +@always_bls def test_invalid_sig_2(spec, state): proposer_slashing = get_valid_proposer_slashing(spec, state, signed_1=True, signed_2=False) yield from run_proposer_slashing_processing(spec, state, proposer_slashing, False) @with_all_phases -@always_bls @spec_state_test +@always_bls def test_invalid_sig_1_and_2(spec, state): proposer_slashing = get_valid_proposer_slashing(spec, state, signed_1=False, signed_2=False) yield from run_proposer_slashing_processing(spec, state, proposer_slashing, False) diff --git a/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_transfer.py b/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_transfer.py index f079ff578..1b839562e 100644 --- a/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_transfer.py +++ b/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_transfer.py @@ -81,8 +81,8 @@ def test_success_active_above_max_effective_fee(spec, state): @with_all_phases -@always_bls @spec_state_test +@always_bls def test_invalid_signature(spec, state): transfer = get_valid_transfer(spec, state) # un-activate so validator can transfer diff --git a/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_voluntary_exit.py b/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_voluntary_exit.py index 6c9298ecc..155f70621 100644 --- a/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_voluntary_exit.py +++ b/test_libs/pyspec/eth2spec/test/phase_0/block_processing/test_process_voluntary_exit.py @@ -47,8 +47,8 @@ def test_success(spec, state): @with_all_phases -@always_bls @spec_state_test +@always_bls def test_invalid_signature(spec, state): # move state forward PERSISTENT_COMMITTEE_PERIOD epochs to allow for exit state.slot += spec.PERSISTENT_COMMITTEE_PERIOD * spec.SLOTS_PER_EPOCH diff --git a/test_libs/pyspec/eth2spec/test/phase_1/block_processing/test_process_early_derived_secret_reveal.py b/test_libs/pyspec/eth2spec/test/phase_1/block_processing/test_process_early_derived_secret_reveal.py index 831ad35a5..3c7434dfc 100644 --- a/test_libs/pyspec/eth2spec/test/phase_1/block_processing/test_process_early_derived_secret_reveal.py +++ b/test_libs/pyspec/eth2spec/test/phase_1/block_processing/test_process_early_derived_secret_reveal.py @@ -42,8 +42,8 @@ def run_early_derived_secret_reveal_processing(spec, state, randao_key_reveal, v @with_all_phases_except(['phase0']) -@always_bls @spec_state_test +@always_bls def test_success(spec, state): randao_key_reveal = get_valid_early_derived_secret_reveal(spec, state) @@ -51,8 +51,8 @@ def test_success(spec, state): @with_all_phases_except(['phase0']) -@never_bls @spec_state_test +@never_bls def test_reveal_from_current_epoch(spec, state): randao_key_reveal = get_valid_early_derived_secret_reveal(spec, state, spec.get_current_epoch(state)) @@ -60,8 +60,8 @@ def test_reveal_from_current_epoch(spec, state): @with_all_phases_except(['phase0']) -@never_bls @spec_state_test +@never_bls def test_reveal_from_past_epoch(spec, state): next_epoch(spec, state) apply_empty_block(spec, state) @@ -71,8 +71,8 @@ def test_reveal_from_past_epoch(spec, state): @with_all_phases_except(['phase0']) -@always_bls @spec_state_test +@always_bls def test_reveal_with_custody_padding(spec, state): randao_key_reveal = get_valid_early_derived_secret_reveal( spec, @@ -83,8 +83,8 @@ def test_reveal_with_custody_padding(spec, state): @with_all_phases_except(['phase0']) -@always_bls @spec_state_test +@always_bls def test_reveal_with_custody_padding_minus_one(spec, state): randao_key_reveal = get_valid_early_derived_secret_reveal( spec, @@ -95,8 +95,8 @@ def test_reveal_with_custody_padding_minus_one(spec, state): @with_all_phases_except(['phase0']) -@never_bls @spec_state_test +@never_bls def test_double_reveal(spec, state): randao_key_reveal1 = get_valid_early_derived_secret_reveal( spec, @@ -120,8 +120,8 @@ def test_double_reveal(spec, state): @with_all_phases_except(['phase0']) -@never_bls @spec_state_test +@never_bls def test_revealer_is_slashed(spec, state): randao_key_reveal = get_valid_early_derived_secret_reveal(spec, state, spec.get_current_epoch(state)) state.validators[randao_key_reveal.revealed_index].slashed = True @@ -130,8 +130,8 @@ def test_revealer_is_slashed(spec, state): @with_all_phases_except(['phase0']) -@never_bls @spec_state_test +@never_bls def test_far_future_epoch(spec, state): randao_key_reveal = get_valid_early_derived_secret_reveal( spec, diff --git a/test_libs/pyspec/eth2spec/test/test_finality.py b/test_libs/pyspec/eth2spec/test/test_finality.py index 6250a685d..8ae50d436 100644 --- a/test_libs/pyspec/eth2spec/test/test_finality.py +++ b/test_libs/pyspec/eth2spec/test/test_finality.py @@ -29,8 +29,8 @@ def check_finality(spec, @with_all_phases -@never_bls @spec_state_test +@never_bls def test_finality_no_updates_at_genesis(spec, state): assert spec.get_current_epoch(state) == spec.GENESIS_EPOCH @@ -53,8 +53,8 @@ def test_finality_no_updates_at_genesis(spec, state): @with_all_phases -@never_bls @spec_state_test +@never_bls def test_finality_rule_4(spec, state): # get past first two epochs that finality does not run on next_epoch(spec, state) @@ -81,8 +81,8 @@ def test_finality_rule_4(spec, state): @with_all_phases -@never_bls @spec_state_test +@never_bls def test_finality_rule_1(spec, state): # get past first two epochs that finality does not run on next_epoch(spec, state) @@ -111,8 +111,8 @@ def test_finality_rule_1(spec, state): @with_all_phases -@never_bls @spec_state_test +@never_bls def test_finality_rule_2(spec, state): # get past first two epochs that finality does not run on next_epoch(spec, state) @@ -143,8 +143,8 @@ def test_finality_rule_2(spec, state): @with_all_phases -@never_bls @spec_state_test +@never_bls def test_finality_rule_3(spec, state): """ Test scenario described here diff --git a/test_libs/pyspec/eth2spec/test/utils.py b/test_libs/pyspec/eth2spec/test/utils.py index 4ecabb114..59289db59 100644 --- a/test_libs/pyspec/eth2spec/test/utils.py +++ b/test_libs/pyspec/eth2spec/test/utils.py @@ -3,10 +3,13 @@ from eth2spec.debug.encode import encode from eth2spec.utils.ssz.ssz_typing import SSZValue -def spectest(description: str = None): +def vector_test(description: str = None): """ - Spectest decorator, should always be the most outer decorator around functions that yield data. - to deal with silent iteration through yielding function when in a pytest context (i.e. not in generator mode). + vector_test decorator: Allow a caller to pass "generator_mode=True" to make the test yield data, + but behave like a normal test (ignoring the yield, but fully processing) a test when not in "generator_mode" + This should always be the most outer decorator around functions that yield data. + This is to deal with silent iteration through yielding function when in a pytest + context (i.e. not in generator mode). :param description: Optional description for the test to add to the metadata. :return: Decorator. """ @@ -17,10 +20,8 @@ def spectest(description: str = None): # - "ssz": raw SSZ bytes # - "data": a python structure to be encoded by the user. def entry(*args, **kw): - # check generator mode, may be None/else. - # "pop" removes it, so it is not passed to the inner function. - if kw.pop('generator_mode', False) is True: + def generator_mode(): if description is not None: # description can be explicit yield 'description', 'meta', description @@ -51,6 +52,13 @@ def spectest(description: str = None): # The data will now just be yielded as any python data, # something that should be encodeable by the generator runner. yield key, 'data', value + + # check generator mode, may be None/else. + # "pop" removes it, so it is not passed to the inner function. + if kw.pop('generator_mode', False) is True: + # return the yielding function as a generator object. + # Don't yield in this function itself, that would make pytest skip over it. + return generator_mode() else: # Just complete the function, ignore all yielded data, # we are not using it (or processing it, i.e. nearly zero efficiency loss) @@ -80,8 +88,7 @@ def with_meta_tags(tags: Dict[str, Any]): # Do not add tags if the function is not returning a dict at all (i.e. not in generator mode). # As a pytest, we do not want to be yielding anything (unsupported by pytest) if yielded_any: - for k, v in tags: + for k, v in tags.items(): yield k, 'meta', v return entry return runner -