From f42da8d00b803c00a8f275322723e68cc0280ec2 Mon Sep 17 00:00:00 2001 From: Etan Kissling Date: Sat, 19 Nov 2022 12:57:13 +0100 Subject: [PATCH 1/6] Apply `with_config_overrides` to all phases When defining a fork transition test, additional spec forks are made available through `@with_phases(..., other_phases=...)`. The `with_config_overrides` decorator only applies to the primary phase so far, which can be unexpected. `with_config_overrides` is adjusted to override config in subsequent `other_phases` as well. --- .../altair/unittests/test_config_override.py | 14 +++++- tests/core/pyspec/eth2spec/test/context.py | 50 ++++++++++++------- 2 files changed, 44 insertions(+), 20 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/altair/unittests/test_config_override.py b/tests/core/pyspec/eth2spec/test/altair/unittests/test_config_override.py index 5448781ec..cd186e62f 100644 --- a/tests/core/pyspec/eth2spec/test/altair/unittests/test_config_override.py +++ b/tests/core/pyspec/eth2spec/test/altair/unittests/test_config_override.py @@ -1,10 +1,13 @@ from eth2spec.test.context import ( spec_configured_state_test, spec_state_test_with_matching_config, + spec_test, with_all_phases, + with_matching_spec_config, with_phases, + with_state, ) -from eth2spec.test.helpers.constants import ALTAIR +from eth2spec.test.helpers.constants import ALTAIR, BELLATRIX from eth2spec.test.helpers.forks import ( is_post_capella, is_post_eip4844, ) @@ -56,3 +59,12 @@ def test_override_config_fork_epoch(spec, state): return assert False # Fork is missing + + +@with_phases(phases=[ALTAIR], other_phases=[BELLATRIX]) +@spec_test +@with_state +@with_matching_spec_config +def test_capella_store_with_legacy_data(spec, phases, state): + assert state.fork.current_version == spec.config.ALTAIR_FORK_VERSION + assert phases[BELLATRIX].ALTAIR_FORK_EPOCH == spec.config.ALTAIR_FORK_EPOCH diff --git a/tests/core/pyspec/eth2spec/test/context.py b/tests/core/pyspec/eth2spec/test/context.py index 94910fa47..be8f04ae1 100644 --- a/tests/core/pyspec/eth2spec/test/context.py +++ b/tests/core/pyspec/eth2spec/test/context.py @@ -318,14 +318,16 @@ def config_fork_epoch_overrides(spec, state): return overrides +def with_matching_spec_config(fn): + def decorator(*args, spec: Spec, **kw): + conf = config_fork_epoch_overrides(spec, kw['state']) + overrides = with_config_overrides(conf) + return overrides(fn)(*args, spec=spec, **kw) + return decorator + + def spec_state_test_with_matching_config(fn): - def decorator(fn): - def wrapper(*args, spec: Spec, **kw): - conf = config_fork_epoch_overrides(spec, kw['state']) - overrides = with_config_overrides(conf) - return overrides(fn)(*args, spec=spec, **kw) - return wrapper - return spec_test(with_state(decorator(single_phase(fn)))) + return spec_test(with_state(with_matching_spec_config(single_phase(fn)))) def expect_assertion_error(fn): @@ -569,6 +571,22 @@ def _get_copy_of_spec(spec): return module +def spec_with_config_overrides(spec, config_overrides): + # apply our overrides to a copy of it, and apply it to the spec + config = spec.config._asdict() + config.update(config_overrides) + config_types = spec.Configuration.__annotations__ + modified_config = {k: config_types[k](v) for k, v in config.items()} + + spec.config = spec.Configuration(**modified_config) + + # To output the changed config in a format compatible with yaml test vectors, + # the dict SSZ objects have to be converted into Python built-in types. + output_config = _get_basic_dict(modified_config) + + return spec, output_config + + def with_config_overrides(config_overrides): """ WARNING: the spec_test decorator must wrap this, to ensure the decorated test actually runs. @@ -579,20 +597,14 @@ def with_config_overrides(config_overrides): """ def decorator(fn): def wrapper(*args, spec: Spec, **kw): - spec = _get_copy_of_spec(spec) - - # apply our overrides to a copy of it, and apply it to the spec - config = spec.config._asdict() - config.update(config_overrides) - config_types = spec.Configuration.__annotations__ - modified_config = {k: config_types[k](v) for k, v in config.items()} - - # To output the changed config to could be serialized with yaml test vectors, - # the dict SSZ objects have to be converted into Python built-in types. - output_config = _get_basic_dict(modified_config) + spec, output_config = spec_with_config_overrides(_get_copy_of_spec(spec), config_overrides) yield 'config', 'cfg', output_config - spec.config = spec.Configuration(**modified_config) + if 'phases' in kw: + for fork in kw['phases']: + if is_post_fork(fork, spec.fork): + kw['phases'][fork], _ = \ + spec_with_config_overrides(_get_copy_of_spec(kw['phases'][fork]), config_overrides) # Run the function out = fn(*args, spec=spec, **kw) From db796f70ad597ae8d7b188534259911b889a33d9 Mon Sep 17 00:00:00 2001 From: Etan Kissling Date: Sun, 20 Nov 2022 14:25:50 +0100 Subject: [PATCH 2/6] Add missing `.config` --- .../eth2spec/test/altair/unittests/test_config_override.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/core/pyspec/eth2spec/test/altair/unittests/test_config_override.py b/tests/core/pyspec/eth2spec/test/altair/unittests/test_config_override.py index cd186e62f..6748fac76 100644 --- a/tests/core/pyspec/eth2spec/test/altair/unittests/test_config_override.py +++ b/tests/core/pyspec/eth2spec/test/altair/unittests/test_config_override.py @@ -67,4 +67,4 @@ def test_override_config_fork_epoch(spec, state): @with_matching_spec_config def test_capella_store_with_legacy_data(spec, phases, state): assert state.fork.current_version == spec.config.ALTAIR_FORK_VERSION - assert phases[BELLATRIX].ALTAIR_FORK_EPOCH == spec.config.ALTAIR_FORK_EPOCH + assert phases[BELLATRIX].config.ALTAIR_FORK_EPOCH == spec.config.ALTAIR_FORK_EPOCH From 0649e0662c5baf1aa4d954c5375d1880bf7af369 Mon Sep 17 00:00:00 2001 From: Etan Kissling Date: Wed, 7 Dec 2022 21:10:09 +0100 Subject: [PATCH 3/6] Allow selecting phase to emit, and fix combi with overrides --- .../altair/unittests/test_config_override.py | 24 ++++++++++++--- tests/core/pyspec/eth2spec/test/context.py | 30 ++++++++++++------- 2 files changed, 40 insertions(+), 14 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/altair/unittests/test_config_override.py b/tests/core/pyspec/eth2spec/test/altair/unittests/test_config_override.py index 6748fac76..ed6f63868 100644 --- a/tests/core/pyspec/eth2spec/test/altair/unittests/test_config_override.py +++ b/tests/core/pyspec/eth2spec/test/altair/unittests/test_config_override.py @@ -3,6 +3,7 @@ from eth2spec.test.context import ( spec_state_test_with_matching_config, spec_test, with_all_phases, + with_config_overrides, with_matching_spec_config, with_phases, with_state, @@ -32,7 +33,7 @@ def test_config_override(spec, state): @with_all_phases @spec_state_test_with_matching_config -def test_override_config_fork_epoch(spec, state): +def test_config_override_matching_fork_epochs(spec, state): if state.fork.current_version == spec.config.GENESIS_FORK_VERSION: return @@ -63,8 +64,23 @@ def test_override_config_fork_epoch(spec, state): @with_phases(phases=[ALTAIR], other_phases=[BELLATRIX]) @spec_test +@with_config_overrides({ + 'ALTAIR_FORK_VERSION': '0x11111111', + 'BELLATRIX_FORK_EPOCH': 4, +}, emit=False) @with_state -@with_matching_spec_config -def test_capella_store_with_legacy_data(spec, phases, state): +@with_matching_spec_config(emitted_fork=BELLATRIX) +def test_config_override_across_phases(spec, phases, state): assert state.fork.current_version == spec.config.ALTAIR_FORK_VERSION - assert phases[BELLATRIX].config.ALTAIR_FORK_EPOCH == spec.config.ALTAIR_FORK_EPOCH + + assert spec.config.ALTAIR_FORK_VERSION == spec.Version('0x11111111') + assert spec.config.ALTAIR_FORK_EPOCH == 0 + assert not hasattr(spec.config, 'BELLATRIX_FORK_EPOCH') + + assert phases[ALTAIR].config.ALTAIR_FORK_VERSION == spec.Version('0x11111111') + assert phases[ALTAIR].config.ALTAIR_FORK_EPOCH == 0 + assert not hasattr(phases[ALTAIR].config, 'BELLATRIX_FORK_EPOCH') + + assert phases[ALTAIR].config.ALTAIR_FORK_VERSION == spec.Version('0x11111111') + assert phases[BELLATRIX].config.ALTAIR_FORK_EPOCH == 0 + assert phases[BELLATRIX].config.BELLATRIX_FORK_EPOCH == 4 diff --git a/tests/core/pyspec/eth2spec/test/context.py b/tests/core/pyspec/eth2spec/test/context.py index be8f04ae1..d80d841cc 100644 --- a/tests/core/pyspec/eth2spec/test/context.py +++ b/tests/core/pyspec/eth2spec/test/context.py @@ -1,4 +1,5 @@ import pytest +from copy import deepcopy from dataclasses import dataclass import importlib @@ -318,16 +319,18 @@ def config_fork_epoch_overrides(spec, state): return overrides -def with_matching_spec_config(fn): - def decorator(*args, spec: Spec, **kw): - conf = config_fork_epoch_overrides(spec, kw['state']) - overrides = with_config_overrides(conf) - return overrides(fn)(*args, spec=spec, **kw) +def with_matching_spec_config(emitted_fork=None): + def decorator(fn): + def wrapper(*args, spec: Spec, **kw): + overrides = config_fork_epoch_overrides(spec, kw['state']) + deco = with_config_overrides(overrides, emitted_fork) + return deco(fn)(*args, spec=spec, **kw) + return wrapper return decorator def spec_state_test_with_matching_config(fn): - return spec_test(with_state(with_matching_spec_config(single_phase(fn)))) + return spec_test(with_state(with_matching_spec_config()(single_phase(fn)))) def expect_assertion_error(fn): @@ -568,13 +571,17 @@ def _get_copy_of_spec(spec): module_spec = importlib.util.find_spec(module_path) module = importlib.util.module_from_spec(module_spec) module_spec.loader.exec_module(module) + + # Preserve existing config overrides + module.config = deepcopy(spec.config) + return module def spec_with_config_overrides(spec, config_overrides): # apply our overrides to a copy of it, and apply it to the spec config = spec.config._asdict() - config.update(config_overrides) + config.update((k, config_overrides[k]) for k in config.keys() & config_overrides.keys()) config_types = spec.Configuration.__annotations__ modified_config = {k: config_types[k](v) for k, v in config.items()} @@ -587,7 +594,7 @@ def spec_with_config_overrides(spec, config_overrides): return spec, output_config -def with_config_overrides(config_overrides): +def with_config_overrides(config_overrides, emitted_fork=None, emit=True): """ WARNING: the spec_test decorator must wrap this, to ensure the decorated test actually runs. This decorator forces the test to yield, and pytest doesn't run generator tests, and instead silently passes it. @@ -598,13 +605,16 @@ def with_config_overrides(config_overrides): def decorator(fn): def wrapper(*args, spec: Spec, **kw): spec, output_config = spec_with_config_overrides(_get_copy_of_spec(spec), config_overrides) - yield 'config', 'cfg', output_config + if emit and emitted_fork is None: + yield 'config', 'cfg', output_config if 'phases' in kw: for fork in kw['phases']: if is_post_fork(fork, spec.fork): - kw['phases'][fork], _ = \ + kw['phases'][fork], output_config = \ spec_with_config_overrides(_get_copy_of_spec(kw['phases'][fork]), config_overrides) + if emit and emitted_fork == fork: + yield 'config', 'cfg', output_config # Run the function out = fn(*args, spec=spec, **kw) From 0c3853e95952b61ce64f05a651f0e32b858f15ad Mon Sep 17 00:00:00 2001 From: Etan Kissling Date: Sun, 11 Dec 2022 23:41:08 +0100 Subject: [PATCH 4/6] Avoid modifying caller `phases` (`kw` is shallow copy) --- tests/core/pyspec/eth2spec/test/context.py | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/context.py b/tests/core/pyspec/eth2spec/test/context.py index d80d841cc..b1f608d7b 100644 --- a/tests/core/pyspec/eth2spec/test/context.py +++ b/tests/core/pyspec/eth2spec/test/context.py @@ -604,20 +604,26 @@ def with_config_overrides(config_overrides, emitted_fork=None, emit=True): """ def decorator(fn): def wrapper(*args, spec: Spec, **kw): + # Apply config overrides to spec spec, output_config = spec_with_config_overrides(_get_copy_of_spec(spec), config_overrides) - if emit and emitted_fork is None: - yield 'config', 'cfg', output_config + # Apply config overrides to additional phases, if present if 'phases' in kw: + phases = {} for fork in kw['phases']: - if is_post_fork(fork, spec.fork): - kw['phases'][fork], output_config = \ - spec_with_config_overrides(_get_copy_of_spec(kw['phases'][fork]), config_overrides) - if emit and emitted_fork == fork: - yield 'config', 'cfg', output_config + phases[fork], output = \ + spec_with_config_overrides(_get_copy_of_spec(kw['phases'][fork]), config_overrides) + if emitted_fork == fork: + output_config = output + kw['phases'] = phases # Run the function out = fn(*args, spec=spec, **kw) + + # Emit requested spec (with overrides) + if emit: + yield 'config', 'cfg', output_config + # If it's not returning None like a normal test function, # it's generating things, and we need to complete it before setting back the config. if out is not None: From 82ff974090638cb966771defd6ee36d3e4210065 Mon Sep 17 00:00:00 2001 From: Etan Kissling Date: Mon, 12 Dec 2022 12:15:27 +0100 Subject: [PATCH 5/6] Emit config before calling test to ignore changes --- tests/core/pyspec/eth2spec/test/context.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/context.py b/tests/core/pyspec/eth2spec/test/context.py index b1f608d7b..4bddaba1c 100644 --- a/tests/core/pyspec/eth2spec/test/context.py +++ b/tests/core/pyspec/eth2spec/test/context.py @@ -617,13 +617,13 @@ def with_config_overrides(config_overrides, emitted_fork=None, emit=True): output_config = output kw['phases'] = phases - # Run the function - out = fn(*args, spec=spec, **kw) - # Emit requested spec (with overrides) if emit: yield 'config', 'cfg', output_config + # Run the function + out = fn(*args, spec=spec, **kw) + # If it's not returning None like a normal test function, # it's generating things, and we need to complete it before setting back the config. if out is not None: From c84862bfaee574f4fa2fa8ad8334ff2c707144aa Mon Sep 17 00:00:00 2001 From: Etan Kissling Date: Tue, 13 Dec 2022 12:34:14 +0100 Subject: [PATCH 6/6] Avoid line continuation syntax --- tests/core/pyspec/eth2spec/test/context.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/core/pyspec/eth2spec/test/context.py b/tests/core/pyspec/eth2spec/test/context.py index 4bddaba1c..39b889901 100644 --- a/tests/core/pyspec/eth2spec/test/context.py +++ b/tests/core/pyspec/eth2spec/test/context.py @@ -611,8 +611,8 @@ def with_config_overrides(config_overrides, emitted_fork=None, emit=True): if 'phases' in kw: phases = {} for fork in kw['phases']: - phases[fork], output = \ - spec_with_config_overrides(_get_copy_of_spec(kw['phases'][fork]), config_overrides) + phases[fork], output = spec_with_config_overrides( + _get_copy_of_spec(kw['phases'][fork]), config_overrides) if emitted_fork == fork: output_config = output kw['phases'] = phases