From e438f378e1d60258739abfd26ffc80cc0d6d9341 Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Fri, 18 Oct 2019 20:06:31 +0300 Subject: [PATCH 1/3] Add unit tests for inherited hooks --- unittests/test_pipeline.py | 46 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/unittests/test_pipeline.py b/unittests/test_pipeline.py index e74fa96d9b..2d6c3c5fdc 100644 --- a/unittests/test_pipeline.py +++ b/unittests/test_pipeline.py @@ -493,6 +493,52 @@ def x(self): _run(test, self.partition, self.prgenv) assert test.var == 3 + def test_inherited_hooks(self): + import unittests.resources.checks.hellocheck as mod + + class BaseTest(mod.HelloTest): + def __init__(self): + super().__init__() + self._prefix = 'unittests/resources/checks' + self.name = type(self).__name__ + self.executable = os.path.join('.', self.name) + self.var = 0 + + @rfm.run_after('setup') + def x(self): + self.var += 1 + + class MyTest(BaseTest): + pass + + test = MyTest() + _run(test, self.partition, self.prgenv) + assert test.var == 1 + + def test_overriden_hooks(self): + import unittests.resources.checks.hellocheck as mod + + class BaseTest(mod.HelloTest): + def __init__(self): + super().__init__() + self._prefix = 'unittests/resources/checks' + self.name = type(self).__name__ + self.executable = os.path.join('.', self.name) + self.var = 0 + + @rfm.run_after('setup') + def x(self): + self.var += 1 + + class MyTest(BaseTest): + @rfm.run_after('setup') + def x(self): + self.var += 5 + + test = MyTest() + _run(test, self.partition, self.prgenv) + assert test.var == 5 + def test_require_deps(self): import unittests.resources.checks.hellocheck as mod import reframe.frontend.dependency as dependency From 08b76cb98caf12c3c2a4d62d49f6cf78b19dbf4c Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Fri, 18 Oct 2019 22:41:53 +0300 Subject: [PATCH 2/3] Support inheritance in pipeline hooks --- reframe/core/pipeline.py | 16 +++++++++++++++- unittests/test_pipeline.py | 28 +++++++++++++++++++++++++--- 2 files changed, 40 insertions(+), 4 deletions(-) diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 24016e76dd..b503589b4f 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -51,7 +51,21 @@ def hooks(obj, kind): # Just any name that does not exist hook_name = 'xxx' - return obj._rfm_pipeline_hooks.get(hook_name, []) + func_names = set() + ret = [] + for cls in type(obj).mro(): + try: + funcs = cls._rfm_pipeline_hooks.get(hook_name, []) + if any(fn.__name__ in func_names for fn in funcs): + # hook has been overriden + continue + + func_names |= {fn.__name__ for fn in funcs} + ret += funcs + except AttributeError: + pass + + return ret '''Run the hooks before and after func.''' @functools.wraps(func) diff --git a/unittests/test_pipeline.py b/unittests/test_pipeline.py index 2d6c3c5fdc..204f5eb9d1 100644 --- a/unittests/test_pipeline.py +++ b/unittests/test_pipeline.py @@ -508,12 +508,23 @@ def __init__(self): def x(self): self.var += 1 - class MyTest(BaseTest): + class C(rfm.RegressionTest): + @rfm.run_before('run') + def y(self): + self.foo = 1 + + class DerivedTest(BaseTest, C): + @rfm.run_after('setup') + def z(self): + self.var += 1 + + class MyTest(DerivedTest): pass test = MyTest() _run(test, self.partition, self.prgenv) - assert test.var == 1 + assert test.var == 2 + assert test.foo == 1 def test_overriden_hooks(self): import unittests.resources.checks.hellocheck as mod @@ -525,19 +536,30 @@ def __init__(self): self.name = type(self).__name__ self.executable = os.path.join('.', self.name) self.var = 0 + self.foo = 0 @rfm.run_after('setup') def x(self): self.var += 1 - class MyTest(BaseTest): + @rfm.run_before('setup') + def y(self): + self.foo += 1 + + class DerivedTest(BaseTest): @rfm.run_after('setup') def x(self): self.var += 5 + class MyTest(DerivedTest): + @rfm.run_before('setup') + def y(self): + self.foo += 10 + test = MyTest() _run(test, self.partition, self.prgenv) assert test.var == 5 + assert test.foo == 10 def test_require_deps(self): import unittests.resources.checks.hellocheck as mod From f7223e80a22fb44cb0669d272ffe2f2cf1a16b51 Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Fri, 18 Oct 2019 23:44:28 +0300 Subject: [PATCH 3/3] Document hook inheritance --- docs/tutorial.rst | 41 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/docs/tutorial.rst b/docs/tutorial.rst index d3ba9b61db..0f02adb02d 100644 --- a/docs/tutorial.rst +++ b/docs/tutorial.rst @@ -418,6 +418,45 @@ In the following example ``var`` will be set to ``2`` after the setup phase is e def inc(self): self.var += 1 +Another important feature of the hooks syntax, is that hooks are inherited by derived tests, unless you override the function and re-hook it explicitly. +In the following example, the :func:`setflags()` will be executed before the compilation phase of the :class:`DerivedTest`: + +.. code:: python + + class BaseTest(rfm.RegressionTest): + def __init__(self): + ... + self.build_system = 'Make' + + @rfm.run_before('compile') + def setflags(self): + if self.current_environ.name == 'X': + self.build_system.cppflags = ['-Ifoo'] + + + @rfm.simple_test + class DerivedTest(BaseTest): + def __init__(self): + super().__init__() + ... + + +If you override a hooked function in a derived class, the base class' hook will not be executed, unless you explicitly call it with ``super()``. +In the following example, we completely disable the :func:`setflags()` hook of the base class: + + +.. code:: python + + @rfm.simple_test + class DerivedTest(BaseTest): + @rfm.run_before('compile') + def setflags(self): + pass + + +Notice that in order to redefine a hook, you need not only redefine the method in the derived class, but you should hook it at the same pipeline phase. +Otherwise, the base class hook will be executed. + .. note:: You may still configure your test per programming environment and per system partition by overriding the :func:`setup ` method, as in ReFrame versions prior to 2.20, but this is now discouraged since it is more error prone, as you have to memorize the signature of the pipeline methods that you override and also remember to call ``super()``. @@ -664,7 +703,7 @@ Here is the final example code that combines all the tests discussed before: This test abstracts away the common functionality found in almost all of our tutorial tests (executable options, sanity checking, etc.) to a base class, from which all the concrete regression tests derive. Each test then redefines only the parts that are specific to it. Notice also that only the actual tests, i.e., the derived classes, are made visible to the framework through the ``@simple_test`` decorator. -Decorating the base class has now meaning, because it does not correspond to an actual test. +Decorating the base class has no meaning, because it does not correspond to an actual test. The total line count of this refactored example is less than half of that of the individual tutorial tests. Another interesting thing to note here is the base class accepting additional additional parameters to its constructor, so that the concrete subclasses can initialize it based on their needs.