From b75f5b914ce9a6c57c304f5f526aa8860ee23cae Mon Sep 17 00:00:00 2001
From: Vasileios Karakasis <karakasis@cscs.ch>
Date: Wed, 24 Nov 2021 21:17:06 +0100
Subject: [PATCH] Fix hook execution order in the same class

---
 reframe/core/hooks.py      | 8 ++++++--
 reframe/core/meta.py       | 7 ++++---
 reframe/core/pipeline.py   | 6 +-----
 unittests/test_pipeline.py | 8 ++++++--
 4 files changed, 17 insertions(+), 12 deletions(-)

diff --git a/reframe/core/hooks.py b/reframe/core/hooks.py
index 99aef67809..a1e7f19174 100644
--- a/reframe/core/hooks.py
+++ b/reframe/core/hooks.py
@@ -156,15 +156,19 @@ def add(self, v):
             v._rfm_attach = ['post_setup']
             self.__hooks.add(Hook(v))
 
-    def update(self, hooks, *, denied_hooks=None):
+    def update(self, other, *, 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.
         '''
+
+        assert isinstance(other, HookRegistry)
         denied_hooks = denied_hooks or set()
-        for h in hooks:
+        for h in other:
             if h.__name__ not in denied_hooks:
+                # Hooks in `other` override ours
+                self.__hooks.discard(h)
                 self.__hooks.add(h)
 
     def __repr__(self):
diff --git a/reframe/core/meta.py b/reframe/core/meta.py
index 87471dbdf7..66d8327c1a 100644
--- a/reframe/core/meta.py
+++ b/reframe/core/meta.py
@@ -411,14 +411,15 @@ def __init__(cls, name, bases, namespace, **kwargs):
         cls._rfm_dir.update(cls.__dict__)
 
         # 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:]:
+        # parent classes in reverse MRO order
+        for c in list(reversed(cls.mro()))[:-1]:
             if hasattr(c, '_rfm_local_hook_registry'):
                 cls._rfm_hook_registry.update(
                     c._rfm_local_hook_registry, denied_hooks=namespace
                 )
 
+        cls._rfm_hook_registry.update(cls._rfm_local_hook_registry)
+
         # Search the bases if no local sanity functions exist.
         if '_rfm_sanity' not in namespace:
             for base in cls._rfm_bases:
diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py
index acd85fafb6..f29f2161b6 100644
--- a/reframe/core/pipeline.py
+++ b/reframe/core/pipeline.py
@@ -197,15 +197,11 @@ 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].insert(0, hook.fn)
+                    ret[stage].append(hook.fn)
                 except KeyError:
                     ret[stage] = [hook.fn]
 
diff --git a/unittests/test_pipeline.py b/unittests/test_pipeline.py
index d76da6be5a..2d5ad83d8f 100644
--- a/unittests/test_pipeline.py
+++ b/unittests/test_pipeline.py
@@ -753,15 +753,19 @@ class DerivedTest(BaseTest, C):
         def z(self):
             self.var += 1
 
+        @run_after('setup')
+        def w(self):
+            self.var += 1
+
     class MyTest(DerivedTest):
         pass
 
     test = MyTest()
     _run(test, *local_exec_ctx)
-    assert test.var == 2
+    assert test.var == 3
     assert test.foo == 1
     assert test.pipeline_hooks() == {
-        'post_setup': [BaseTest.x, DerivedTest.z],
+        'post_setup': [BaseTest.x, DerivedTest.z, DerivedTest.w],
         'pre_run': [C.y],
     }