[bugfix] Fix pipeline hook override

docs/regression_test_api.rst

@@ -204,7 +204,8 @@ You can attach arbitrary functions to run before or after any pipeline stage, wh
 Multiple hooks can be attached before or after the same pipeline stage, in which case the order of execution will match the order in which the functions are defined in the class body of the test.
 A single hook can also be applied to multiple stages and it will be executed multiple times.
 All pipeline hooks of a test class are inherited by its subclasses.
-Subclasses may override a pipeline hook of their parents by redefining the hook function and re-attaching it at the same pipeline stage.
+Subclasses may override a pipeline hook of their parents by redefining the hook function.
+The overriding function will not be a pipeline hook unless explicitly decorated, and it may be reattached to any pipeline stage.
 There are seven pipeline stages where you can attach test methods: ``init``, ``setup``, ``compile``, ``run``, ``sanity``, ``performance`` and ``cleanup``.
 The ``init`` stage is not a real pipeline stage, but it refers to the test initialization.
 

reframe/core/hooks.py

@@ -170,11 +170,18 @@ def __contains__(self, key):
     def __getattr__(self, name):
         return getattr(self.__hooks, name)
 
-    def update(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:
-                self.__hooks[phase].add(h)
+                if h.__name__ not in denied_hooks:
+                    self.__hooks[phase].add(h)
 
     def __repr__(self):
         return repr(self.__hooks)

reframe/core/meta.py

@@ -257,11 +257,11 @@ class was created or even at the instance level (e.g. doing
         constructed.
         '''
 
-        blacklist = [
+        directives = [
             'parameter', 'variable', 'bind', 'run_before', 'run_after',
             'require_deps', 'required', 'deferrable', 'sanity_function'
         ]
-        for b in blacklist:
+        for b in directives:
             namespace.pop(b, None)
 
         return super().__new__(metacls, name, bases, dict(namespace), **kwargs)
@@ -291,7 +291,8 @@ def __init__(cls, name, bases, namespace, **kwargs):
         # 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'))
+            hook_reg.update(getattr(base, '_rfm_pipeline_hooks'),
+                            denied_hooks=namespace)
 
         cls._rfm_pipeline_hooks = hook_reg
 

unittests/test_meta.py

@@ -160,3 +160,44 @@ def my_deferrable(self):
             pass
 
     assert type(MyTest.my_deferrable()) is deferrable._DeferredExpression
+
+
+def test_hook_attachments(MyMeta):
+    class Foo(MyMeta):
+        @run_after('setup')
+        def hook_a(self):
+            pass
+
+        @run_before('compile')
+        def hook_b(self):
+            pass
+
+        @run_after('run')
+        def hook_c(self):
+            pass
+
+        @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
+
+    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')
+
+    class Bar(Foo):
+        @run_before('sanity')
+        def hook_a(self):
+            '''Convert to a pre-sanity hook'''
+
+        def hook_b(self):
+            '''No longer a hook'''
+
+    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_a', 'pre_sanity')