Merge pull request #2287 from vkarak/bugfix/hook-exec-order-same-class

[bugfix] Fix hook execution order for hooks in the same class

Author: Vasileios Karakasis

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):

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:

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]
 

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],
     }