[bugfix] Fix hook override with multiple inheritance

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,56 +137,40 @@ 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.
 
         The optional ``denied_hooks`` argument takes a set of disallowed
         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)

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

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