[feat] Define the execution order of hooks and implement it properly

docs/regression_test_api.rst

@@ -346,7 +346,7 @@ In essence, these builtins exert control over the test creation, and they allow
     @rfm.simple_test
     class TestC(rfm.RegressionTest):
         # Parameterize TestC for each ParamFix variant
-        f = fixture(ParamFix, action='fork') 
+        f = fixture(ParamFix, action='fork')
         ...
 
         @run_after('setup')
@@ -546,7 +546,7 @@ Pipeline hooks attached to multiple stages will be executed on each pipeline sta
 Pipeline stages with multiple hooks attached will execute these hooks in the order in which they were attached to the given pipeline stage.
 A derived class will inherit all the pipeline hooks defined in its bases, except for those whose hook function is overridden by the derived class.
 A function that overrides a pipeline hook from any of the base classes will not be a pipeline hook unless the overriding function is explicitly reattached to any pipeline stage.
-In the event of a name clash arising from multiple inheritance, the inherited pipeline hook will be chosen following Python's `MRO <https://docs.python.org/3/library/stdtypes.html#class.__mro__>`_.
+In the event of a name clash arising from multiple inheritance, the inherited pipeline hook will be chosen following Python's `MRO <https://docs.python.org/3/library/stdtypes.html#class.__mro__>`__.
 
 A function may be attached to any of the following stages (listed in order of execution): ``init``, ``setup``, ``compile``, ``run``, ``sanity``, ``performance`` and ``cleanup``.
 The ``init`` stage refers to the test's instantiation and it runs before entering the execution pipeline.
@@ -556,6 +556,22 @@ So although a "post-init" and a "pre-setup" hook will both run *after* a test ha
 the post-init hook will execute *right after* the test is initialized.
 The framework will then continue with other activities and it will execute the pre-setup hook *just before* it schedules the test for executing its setup stage.
 
+Pipeline hooks are executed in reverse MRO order, i.e., the hooks of the least specialized class will be executed first.
+In the following example, :func:`BaseTest.x` will execute before :func:`DerivedTest.y`:
+
+.. code:: python
+
+   class BaseTest(rfm.RegressionTest):
+       @run_after('setup')
+       def x(self):
+           '''Hook x'''
+
+   class DerivedTest(BaseTeset):
+       @run_after('setup')
+       def y(self):
+           '''Hook y'''
+
+
 .. note::
    Pipeline hooks do not execute in the test's stage directory.
    However, the test's :attr:`~reframe.core.pipeline.RegressionTest.stagedir` can be accessed by explicitly changing the working directory from within the hook function itself (see the :class:`~reframe.utility.osext.change_dir` utility for further details):
@@ -577,6 +593,16 @@ The framework will then continue with other activities and it will execute the p
       Declaring pipeline hooks using the same name functions from the :py:mod:`reframe` or :py:mod:`reframe.core.decorators` modules is now deprecated.
       You should use the built-in functions described in this section instead.
 
+.. warning::
+   .. versionchanged:: 3.9.2
+
+      Execution of pipeline hooks until this version was implementation-defined.
+      In practice, hooks of a derived class were executed before those of its parents.
+
+      This version defines the execution order of hooks, which now follows a strict reverse MRO order, so that parent hooks will execute before those of derived classes.
+      Tests that relied on the execution order of hooks might break with this change.
+
+
 .. py:decorator:: RegressionMixin.run_before(stage)
 
   Decorator for attaching a function to a given pipeline stage.

reframe/core/meta.py

@@ -120,7 +120,7 @@ def __setitem__(self, key, value):
 
             # Register the hooks - if a value does not meet the conditions
             # it will be simply ignored
-            self['_rfm_hook_registry'].add(value)
+            self['_rfm_local_hook_registry'].add(value)
 
         def __getitem__(self, key):
             '''Expose and control access to the local namespaces.
@@ -306,6 +306,7 @@ def run_after(stage):
         namespace['run_after'] = run_after
         namespace['require_deps'] = hooks.require_deps
         namespace['_rfm_hook_registry'] = hooks.HookRegistry()
+        namespace['_rfm_local_hook_registry'] = hooks.HookRegistry()
 
         # Machinery to add a sanity function
         def sanity_function(fn):
@@ -394,7 +395,7 @@ def __init__(cls, name, bases, namespace, **kwargs):
             cls._rfm_dir.update(base._rfm_dir)
 
         used_attribute_names = set(cls._rfm_dir).union(
-            {h.__name__ for h in cls._rfm_hook_registry}
+            {h.__name__ for h in cls._rfm_local_hook_registry}
         )
 
         # Build the different global class namespaces
@@ -409,11 +410,14 @@ def __init__(cls, name, bases, namespace, **kwargs):
         # Update used names set with the local __dict__
         cls._rfm_dir.update(cls.__dict__)
 
-        # Update the hook registry with the bases
-        for base in cls._rfm_bases:
-            cls._rfm_hook_registry.update(
-                base._rfm_hook_registry, denied_hooks=namespace
-            )
+        # Populate the global hook registry with the hook registries of the
+        # parent classes in MRO order
+        cls._rfm_hook_registry.update(cls._rfm_local_hook_registry)
+        for c in cls.mro()[1:]:
+            if hasattr(c, '_rfm_local_hook_registry'):
+                cls._rfm_hook_registry.update(
+                    c._rfm_local_hook_registry, denied_hooks=namespace
+                )
 
         # Search the bases if no local sanity functions exist.
         if '_rfm_sanity' not in namespace:

reframe/core/pipeline.py

@@ -197,11 +197,15 @@ def disable_hook(self, hook_name):
 
     @classmethod
     def pipeline_hooks(cls):
+        # The hook registry stores hooks in MRO order, but we want to attach
+        # the hooks in reverse order so that the more specialized hooks (those
+        # at the beginning of the MRO list) will be executed last
+
         ret = {}
         for hook in cls._rfm_hook_registry:
             for stage in hook.stages:
                 try:
-                    ret[stage].append(hook.fn)
+                    ret[stage].insert(0, hook.fn)
                 except KeyError:
                     ret[stage] = [hook.fn]
 

unittests/test_pipeline.py

@@ -761,11 +761,60 @@ class MyTest(DerivedTest):
     assert test.var == 2
     assert test.foo == 1
     assert test.pipeline_hooks() == {
-        'post_setup': [DerivedTest.z, BaseTest.x],
+        'post_setup': [BaseTest.x, DerivedTest.z],
         'pre_run': [C.y],
     }
 
 
+@pytest.fixture
+def weird_mro_test(HelloTest):
+    # This returns a class with non-obvious MRO resolution.
+    #
+    # See example in https://www.python.org/download/releases/2.3/mro/
+    #
+    # The MRO of A is ABECDFX, which means that E is more specialized than C!
+    class X(rfm.RegressionMixin):
+        pass
+
+    class D(X):
+        @run_after('setup')
+        def d(self):
+            pass
+
+    class E(X):
+        @run_after('setup')
+        def e(self):
+            pass
+
+    class F(X):
+        @run_after('setup')
+        def f(self):
+            pass
+
+    class C(D, F):
+        @run_after('setup')
+        def c(self):
+            pass
+
+    class B(E, D):
+        @run_after('setup')
+        def b(self):
+            pass
+
+    class A(B, C, HelloTest):
+        @run_after('setup')
+        def a(self):
+            pass
+
+    return A
+
+
+def test_inherited_hooks_order(weird_mro_test, local_exec_ctx):
+    t = weird_mro_test()
+    hook_order = [fn.__name__ for fn in t.pipeline_hooks()['post_setup']]
+    assert hook_order == ['f', 'd', 'c', 'e', 'b', 'a']
+
+
 def test_inherited_hooks_from_instantiated_tests(HelloTest, local_exec_ctx):
     @test_util.custom_prefix('unittests/resources/checks')
     class T0(HelloTest):