diff --git a/docs/regression_test_api.rst b/docs/regression_test_api.rst index 9b839362c7..4635da5c43 100644 --- a/docs/regression_test_api.rst +++ b/docs/regression_test_api.rst @@ -111,10 +111,10 @@ 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. +Pipeline hooks are normally 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 +.. code-block:: python class BaseTest(rfm.RegressionTest): @run_after('setup') @@ -127,6 +127,34 @@ In the following example, :func:`BaseTest.x` will execute before :func:`DerivedT '''Hook y''' +This order can be altered using the ``always_last`` argument of the :func:`@run_before ` and :func:`@run_after ` decorators. +In this case, all hooks of the same stage defined with ``always_last=True`` will be executed in MRO order at the end of the stage's hook chain. +For example, given the following hierarchy: + + .. code-block:: python + + class X(rfm.RunOnlyRegressionTest): + @run_before('run', always_last=True) + def hook_a(self): pass + + @run_before('run') + def hook_b(self): pass + + class Y(X): + @run_before('run', always_last=True) + def hook_c(self): pass + + @run_before('run') + def hook_d(self): pass + + +the run hooks of :class:`Y` will be executed as follows: + +.. code-block:: console + + X.hook_b, Y.hook_d, Y.hook_c, X.hook_a + + .. seealso:: - :func:`@run_before `, :func:`@run_after ` decorators @@ -146,6 +174,12 @@ In the following example, :func:`BaseTest.x` will execute before :func:`DerivedT with osext.change_dir(self.stagedir): ... +.. note:: + In versions prior to 4.3.4, overriding a pipeline hook in a subclass would re-attach it from scratch in the stage, therefore changing its execution order relative to other hooks of the superclass. + This is fixed in versions >= 4.3.4 where the execution order of the hook is not changed. + However, the fix may break existing workaround code that used to call explicitly the base class' hook from the derived one. + Check issue `#3012 `__ for details on how to properly address this. + .. warning:: .. versionchanged:: 3.7.0 Declaring pipeline hooks using the same name functions from the :py:mod:`reframe` or :py:mod:`reframe.core.decorators` modules is now deprecated. diff --git a/reframe/core/builtins.py b/reframe/core/builtins.py index 6f32c3a7f4..4b5d4c8e4b 100644 --- a/reframe/core/builtins.py +++ b/reframe/core/builtins.py @@ -48,15 +48,18 @@ def run_before(stage, *, always_last=False): :param stage: The pipeline stage where this function will be attached to. See :ref:`pipeline-hooks` for the list of valid stage values. - :param always_last: Run this hook always as the last one of the stage. In - a whole test hierarchy, only a single hook can be explicitly pinned at - the end of the same-stage sequence of hooks. If another hook is - declared as ``always_last`` in the same stage, an error will be - issued. + :param always_last: Run this hook at the end of the stage's hook chain + instead of the beginning. If multiple tests set this flag for a hook + in the same stage, then all ``always_last`` hooks will be executed in + MRO order at the end of stage's hook chain. See :ref:`pipeline-hooks` + for an example execution. .. versionchanged:: 4.4 The ``always_last`` argument was added. + .. versionchanged:: 4.5 + Multiple tests can set ``always_last`` in the same stage. + ''' return hooks.attach_to('pre_' + stage, always_last) diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index db53c4f2e5..8da9377d61 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -183,16 +183,10 @@ def pipeline_hooks(cls): for hook in cls._rfm_hook_registry: for stage, always_last in hook.stages: if always_last: - if stage in last: - hook_name = hook.__qualname__ - pinned_name = last[stage].__qualname__ - raise ReframeSyntaxError( - f'cannot pin hook {hook_name!r} as last ' - f'of stage {stage!r} as {pinned_name!r} ' - f'is already pinned last' - ) - - last[stage] = hook + try: + last[stage].append(hook.fn) + except KeyError: + last[stage] = [hook.fn] else: try: ret[stage].append(hook.fn) @@ -200,11 +194,12 @@ def pipeline_hooks(cls): ret[stage] = [hook.fn] # Append the last hooks - for stage, hook in last.items(): - try: - ret[stage].append(hook.fn) - except KeyError: - ret[stage] = [hook.fn] + for stage, hooks in last.items(): + for fn in reversed(hooks): + try: + ret[stage].append(fn) + except KeyError: + ret[stage] = [fn] return ret diff --git a/unittests/test_pipeline.py b/unittests/test_pipeline.py index 05cc9f8f8c..35ac202bd3 100644 --- a/unittests/test_pipeline.py +++ b/unittests/test_pipeline.py @@ -1183,7 +1183,6 @@ def foo(self): def test_pinned_hooks(): - @test_util.custom_prefix('unittests/resources/checks') class X(rfm.RunOnlyRegressionTest): @run_before('run', always_last=True) def foo(self): @@ -1207,34 +1206,43 @@ def bar(self): def test_pinned_hooks_multiple_last(): - @test_util.custom_prefix('unittests/resources/checks') class X(rfm.RunOnlyRegressionTest): @run_before('run', always_last=True) - def foo(self): + def hook_a(self): + pass + + @run_before('run') + def hook_b(self): pass class Y(X): @run_before('run', always_last=True) - def bar(self): + def hook_c(self): pass - with pytest.raises(ReframeSyntaxError): - test = Y() + @run_before('run') + def hook_d(self): + pass + + test = Y() + assert test.pipeline_hooks() == { + 'pre_run': [X.hook_b, Y.hook_d, Y.hook_c, X.hook_a] + } def test_pinned_hooks_multiple_last_inherited(): - @test_util.custom_prefix('unittests/resources/checks') class X(rfm.RunOnlyRegressionTest): @run_before('run', always_last=True) def foo(self): pass + class Y(X): @run_before('run', always_last=True) def bar(self): pass - with pytest.raises(ReframeSyntaxError): - test = X() + test = Y() + assert test.pipeline_hooks() == {'pre_run': [Y.bar, X.foo]} def test_disabled_hooks(HelloTest, local_exec_ctx):