diff --git a/reframe/core/hooks.py b/reframe/core/hooks.py index 1a02cc1ae3..e87db4d0f1 100644 --- a/reframe/core/hooks.py +++ b/reframe/core/hooks.py @@ -3,7 +3,6 @@ # # SPDX-License-Identifier: BSD-3-Clause -import contextlib import functools import inspect @@ -70,8 +69,8 @@ def select_hooks(obj, kind): if phase not in hooks: return [] - return [h for h in hooks[phase] - if h.__name__ not in obj._disabled_hooks] + return [h for h in hooks.get(phase, []) + if h.__name__ not in getattr(obj, '_disabled_hooks', [])] @functools.wraps(func) def _fn(obj, *args, **kwargs): @@ -96,6 +95,12 @@ class Hook: def __init__(self, fn): self.__fn = fn + if not hasattr(fn, '_rfm_attach'): + raise ValueError(f'{fn.__name__} is not a hook') + + @property + def stages(self): + return self._rfm_attach def __getattr__(self, attr): return getattr(self.__fn, attr) @@ -132,44 +137,30 @@ def create(cls, namespace): resolved first in the post-setup phase if not assigned elsewhere. ''' - local_hooks = {} - fn_with_deps = [] + local_hooks = util.OrderedSet() for v in namespace.values(): if hasattr(v, '_rfm_attach'): - for phase in v._rfm_attach: - try: - local_hooks[phase].append(Hook(v)) - except KeyError: - local_hooks[phase] = [Hook(v)] - - with contextlib.suppress(AttributeError): - if v._rfm_resolve_deps: - fn_with_deps.append(Hook(v)) - - if fn_with_deps: - local_hooks['post_setup'] = ( - fn_with_deps + local_hooks.get('post_setup', []) - ) + local_hooks.add(Hook(v)) + elif hasattr(v, '_rfm_resolve_deps'): + v._rfm_attach = ['post_setup'] + local_hooks.add(Hook(v)) return cls(local_hooks) def __init__(self, hooks=None): - self.__hooks = {} + self.__hooks = util.OrderedSet() if hooks is not None: self.update(hooks) - def __getitem__(self, key): - return self.__hooks[key] - - def __setitem__(self, key, name): - self.__hooks[key] = name - def __contains__(self, key): return key in self.__hooks def __getattr__(self, name): return getattr(self.__hooks, name) + def __iter__(self): + return iter(self.__hooks) + def update(self, hooks, *, denied_hooks=None): '''Update the hook registry with the hooks from another hook registry. @@ -177,11 +168,9 @@ def update(self, hooks, *, denied_hooks=None): hook names, preventing their inclusion into the current hook registry. ''' denied_hooks = denied_hooks or set() - for phase, hks in hooks.items(): - self.__hooks.setdefault(phase, util.OrderedSet()) - for h in hks: - if h.__name__ not in denied_hooks: - self.__hooks[phase].add(h) + for h in hooks: + if h.__name__ not in denied_hooks: + self.__hooks.add(h) def __repr__(self): return repr(self.__hooks) diff --git a/reframe/core/meta.py b/reframe/core/meta.py index 6176a4354c..12bda462b1 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -19,11 +19,6 @@ from reframe.core.deferrable import deferrable -_USER_PIPELINE_STAGES = ( - 'init', 'setup', 'compile', 'run', 'sanity', 'performance', 'cleanup' -) - - class RegressionTestMeta(type): class MetaNamespace(namespaces.LocalNamespace): @@ -198,40 +193,17 @@ def final(fn): # Hook-related functionality def run_before(stage): - '''Decorator for attaching a test method to a pipeline stage. + '''Decorator for attaching a test method to a given stage. See online docs for more information. ''' - - if stage not in _USER_PIPELINE_STAGES: - raise ValueError( - f'invalid pipeline stage specified: {stage!r}' - ) - - if stage == 'init': - raise ValueError('pre-init hooks are not allowed') - return hooks.attach_to('pre_' + stage) def run_after(stage): - '''Decorator for attaching a test method to a pipeline stage. + '''Decorator for attaching a test method to a given stage. See online docs for more information. ''' - - if stage not in _USER_PIPELINE_STAGES: - raise ValueError( - f'invalid pipeline stage specified: {stage!r}' - ) - - # Map user stage names to the actual pipeline functions if needed - if stage == 'init': - stage = '__init__' - elif stage == 'compile': - stage = 'compile_wait' - elif stage == 'run': - stage = 'run_wait' - return hooks.attach_to('post_' + stage) namespace['run_before'] = run_before @@ -298,11 +270,11 @@ def __init__(cls, name, bases, namespace, **kwargs): # attribute; all dependencies will be resolved first in the post-setup # phase if not assigned elsewhere hook_reg = hooks.HookRegistry.create(namespace) - for base in (b for b in bases if hasattr(b, '_rfm_pipeline_hooks')): - hook_reg.update(getattr(base, '_rfm_pipeline_hooks'), + for base in (b for b in bases if hasattr(b, '_rfm_hook_registry')): + hook_reg.update(getattr(base, '_rfm_hook_registry'), denied_hooks=namespace) - cls._rfm_pipeline_hooks = hook_reg + cls._rfm_hook_registry = hook_reg # Gather all the locally defined sanity functions based on the # _rfm_sanity_fn attribute. @@ -350,23 +322,25 @@ def __init__(cls, name, bases, namespace, **kwargs): raise ReframeSyntaxError(msg) def __call__(cls, *args, **kwargs): - '''Intercept reframe-specific constructor arguments. - - When registering a regression test using any supported decorator, - this decorator may pass additional arguments to the class constructor - to perform specific reframe-internal actions. This gives extra control - over the class instantiation process, allowing reframe to instantiate - the regression test class differently if this class was registered or - not (e.g. when deep-copying a regression test object). These internal - arguments must be intercepted before the object initialization, since - these would otherwise affect the __init__ method's signature, and these - internal mechanisms must be fully transparent to the user. + '''Inject parameter and variable spaces during object construction. + + When a class is instantiated, this method intercepts the arguments + associated to the parameter and variable spaces. This prevents both + :func:`__new__` and :func:`__init__` methods from ever seing these + arguments. + + The parameter and variable spaces are injected into the object after + construction and before initialization. ''' + # Intercept constructor arguments + _rfm_use_params = kwargs.pop('_rfm_use_params', False) + obj = cls.__new__(cls, *args, **kwargs) - # Intercept constructor arguments - kwargs.pop('_rfm_use_params', None) + # Insert the var & param spaces + cls._rfm_var_space.inject(obj, cls) + cls._rfm_param_space.inject(obj, cls, _rfm_use_params) obj.__init__(*args, **kwargs) return obj diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 7450261429..571e6e0a3a 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -90,6 +90,11 @@ ) +_USER_PIPELINE_STAGES = ( + 'init', 'setup', 'compile', 'run', 'sanity', 'performance', 'cleanup' +) + + def final(fn): fn._rfm_final = True user_deprecation_warning( @@ -195,10 +200,12 @@ def disable_hook(self, hook_name): @classmethod def pipeline_hooks(cls): ret = {} - for phase, hooks in cls._rfm_pipeline_hooks.items(): - ret[phase] = [] - for h in hooks: - ret[phase].append(h.fn) + for hook in cls._rfm_hook_registry: + for stage in hook.stages: + try: + ret[stage].append(hook.fn) + except KeyError: + ret[stage] = [hook.fn] return ret @@ -776,24 +783,9 @@ def pipeline_hooks(cls): #: :type: boolean : :default: :class:`True` build_locally = variable(bool, value=True) - def __new__(cls, *args, _rfm_use_params=False, **kwargs): + def __new__(cls, *args, **kwargs): obj = super().__new__(cls) - # Insert the var & param spaces - cls._rfm_var_space.inject(obj, cls) - cls._rfm_param_space.inject(obj, cls, _rfm_use_params) - - # Create a test name from the class name and the constructor's - # arguments - name = cls.__qualname__ - name += obj._append_parameters_to_name() - - # or alternatively, if the parameterized test was defined the old way. - if args or kwargs: - arg_names = map(lambda x: util.toalphanum(str(x)), - itertools.chain(args, kwargs.values())) - name += '_' + '_'.join(arg_names) - # Determine the prefix try: prefix = cls._rfm_custom_prefix @@ -808,37 +800,31 @@ def __new__(cls, *args, _rfm_use_params=False, **kwargs): os.path.dirname(inspect.getfile(cls)) ) + # Prepare initialization of test defaults (variables and parameters are + # injected after __new__ has returned, so we schedule this function + # call as a pre-init hook). + obj.__deferred_rfm_init = obj.__rfm_init__(*args, + name=cls.__qualname__, + prefix=prefix, **kwargs) + + # Build pipeline hook registry and add the pre-init hook + cls._rfm_pipeline_hooks = cls._process_hook_registry() + cls._rfm_pipeline_hooks['pre___init__'] = [cls.__pre_init__] + # Attach the hooks to the pipeline stages for stage in _PIPELINE_STAGES: cls._add_hooks(stage) - # Initialize the test - obj.__rfm_init__(name, prefix) return obj + @final + def __pre_init__(self): + '''Initialize the test defaults from a pre-init hook.''' + self.__deferred_rfm_init.evaluate() + def __init__(self): pass - def _append_parameters_to_name(self): - if self._rfm_param_space.params: - return '_' + '_'.join([util.toalphanum(str(self.__dict__[key])) - for key in self._rfm_param_space.params]) - else: - return '' - - @classmethod - def _add_hooks(cls, stage): - pipeline_hooks = cls._rfm_pipeline_hooks - fn = getattr(cls, stage) - new_fn = hooks.attach_hooks(pipeline_hooks)(fn) - setattr(cls, '_rfm_pipeline_fn_' + stage, new_fn) - - def __getattribute__(self, name): - if name in _PIPELINE_STAGES: - name = f'_rfm_pipeline_fn_{name}' - - return super().__getattribute__(name) - @classmethod def __init_subclass__(cls, *, special=False, pin_prefix=False, require_version=None, **kwargs): @@ -857,10 +843,20 @@ def __init_subclass__(cls, *, special=False, pin_prefix=False, os.path.dirname(inspect.getfile(cls)) ) - def __rfm_init__(self, name=None, prefix=None): + @deferrable + def __rfm_init__(self, *args, name=None, prefix=None, **kwargs): if name is not None: self.name = name + # Add the parameters to the name. + self.name += self._append_parameters_to_name() + + # Add the parameters from the parameterized_test decorator. + if args or kwargs: + arg_names = map(lambda x: util.toalphanum(str(x)), + itertools.chain(args, kwargs.values())) + self.name += '_' + '_'.join(arg_names) + # Pass if descr is a required variable. if not hasattr(self, 'descr'): self.descr = self.name @@ -917,6 +913,57 @@ def __rfm_init__(self, name=None, prefix=None): # Disabled hooks self._disabled_hooks = set() + def _append_parameters_to_name(self): + if self._rfm_param_space.params: + return '_' + '_'.join([util.toalphanum(str(self.__dict__[key])) + for key in self._rfm_param_space.params]) + else: + return '' + + @classmethod + def _process_hook_registry(cls): + '''Process and validate the pipeline hooks.''' + + _pipeline_hooks = {} + for stage, hooks in cls.pipeline_hooks().items(): + # Pop the stage pre_/post_ prefix + stage_name = stage.split('_', maxsplit=1)[1] + + if stage_name not in _USER_PIPELINE_STAGES: + raise ValueError( + f'invalid pipeline stage ({stage_name!r}) in class ' + f'{cls.__qualname__!r}' + ) + elif stage == 'pre_init': + raise ValueError( + f'{stage} hooks are not allowed ({cls.__qualname__})' + ) + elif stage == 'post_init': + stage = 'post___init__' + elif stage == 'post_compile': + stage = 'post_compile_wait' + elif stage == 'post_run': + stage = 'post_run_wait' + + _pipeline_hooks[stage] = hooks + + return _pipeline_hooks + + @classmethod + def _add_hooks(cls, stage): + '''Decorate the pipeline stages.''' + + pipeline_hooks = cls._rfm_pipeline_hooks + fn = getattr(cls, stage) + new_fn = hooks.attach_hooks(pipeline_hooks)(fn) + setattr(cls, '_rfm_pipeline_fn_' + stage, new_fn) + + def __getattribute__(self, name): + if name in _PIPELINE_STAGES: + name = f'_rfm_pipeline_fn_{name}' + + return super().__getattribute__(name) + # Export read-only views to interesting fields @property diff --git a/unittests/test_meta.py b/unittests/test_meta.py index 204489397d..36f7318abd 100644 --- a/unittests/test_meta.py +++ b/unittests/test_meta.py @@ -166,6 +166,12 @@ def my_deferrable(self): def test_hook_attachments(MyMeta): class Foo(MyMeta): + '''Base class with three random hooks. + + This class has the ``hook_in_stage`` method, which asserts that a given + hook is registered into a specified stage. + ''' + @run_after('setup') def hook_a(self): pass @@ -181,17 +187,22 @@ def hook_c(self): @classmethod def hook_in_stage(cls, hook, stage): '''Assert that a hook is in a given registry stage.''' - try: - return hook in {h.__name__ - for h in cls._rfm_pipeline_hooks[stage]} - except KeyError: - return False + for h in cls._rfm_hook_registry: + if h.__name__ == hook: + if stage in h.stages: + return True + + break + + return False assert Foo.hook_in_stage('hook_a', 'post_setup') assert Foo.hook_in_stage('hook_b', 'pre_compile') - assert Foo.hook_in_stage('hook_c', 'post_run_wait') + assert Foo.hook_in_stage('hook_c', 'post_run') class Bar(Foo): + '''Derived class that overrides and invalidates hooks from Foo.''' + @run_before('sanity') def hook_a(self): '''Convert to a pre-sanity hook''' @@ -201,9 +212,30 @@ def hook_b(self): assert not Bar.hook_in_stage('hook_a', 'post_setup') assert not Bar.hook_in_stage('hook_b', 'pre_compile') - assert Bar.hook_in_stage('hook_c', 'post_run_wait') + assert Bar.hook_in_stage('hook_c', 'post_run') assert Bar.hook_in_stage('hook_a', 'pre_sanity') + class Baz(MyMeta): + '''Class to test hook attachments with multiple inheritance.''' + + @run_before('setup') + @run_after('compile') + def hook_a(self): + '''Force a name-clash with hook_a from Bar.''' + + @run_before('run') + def hook_d(self): + '''An extra hook to attach.''' + + class MyTest(Bar, Baz): + '''Test multiple inheritance override.''' + + assert MyTest.hook_in_stage('hook_a', 'pre_sanity') + assert not MyTest.hook_in_stage('hook_a', 'pre_setup') + assert not MyTest.hook_in_stage('hook_a', 'post_compile') + assert MyTest.hook_in_stage('hook_c', 'post_run') + assert MyTest.hook_in_stage('hook_d', 'pre_run') + def test_final(MyMeta): class Base(MyMeta): diff --git a/unittests/test_pipeline.py b/unittests/test_pipeline.py index cb62e3f4fe..ff630b5791 100644 --- a/unittests/test_pipeline.py +++ b/unittests/test_pipeline.py @@ -542,27 +542,33 @@ def set_resources(self): def test_unkown_pre_hook(): + class MyTest(rfm.RunOnlyRegressionTest): + @run_before('foo') + def prepare(self): + self.x = 1 + with pytest.raises(ValueError): - class MyTest(rfm.RunOnlyRegressionTest): - @run_before('foo') - def prepare(self): - self.x = 1 + MyTest() def test_unkown_post_hook(): + class MyTest(rfm.RunOnlyRegressionTest): + @run_after('foo') + def prepare(self): + self.x = 1 + with pytest.raises(ValueError): - class MyTest(rfm.RunOnlyRegressionTest): - @run_after('foo') - def prepare(self): - self.x = 1 + MyTest() def test_pre_init_hook(): + class MyTest(rfm.RunOnlyRegressionTest): + @run_before('init') + def prepare(self): + self.x = 1 + with pytest.raises(ValueError): - class MyTest(rfm.RunOnlyRegressionTest): - @run_before('init') - def prepare(self): - self.x = 1 + MyTest() def test_post_init_hook(local_exec_ctx):