Merge pull request #1228 from ekouts/feat/deprecate-regtest-override

[feat] Deprecate test syntax that overrides `RegressionTest` methods

Author: Vasileios Karakasis

docs/index.rst

@@ -46,7 +46,7 @@ Publications
 
 
 .. toctree::
-   :caption: Table of Contents:
+   :caption: Table of Contents
    :hidden:
 
    Getting Started <started>
@@ -58,6 +58,7 @@ Publications
    Understanding The Mechanism Of Sanity Functions <deferrables>
    Running ReFrame <running>
    Use cases <usecases>
+   Migrating to ReFrame 3 <migration_2_to_3>
    About ReFrame <about>
    Reference Guide <reference>
    Sanity Functions Reference <sanity_functions_reference>

docs/migration_2_to_3.rst

@@ -0,0 +1,78 @@
+======================
+Migrating to ReFrame 3
+======================
+
+
+Updating your tests
+-------------------
+
+
+ReFrame 2.20 introduced a new powerful mechanism for attaching arbitrary functions hooks at the different pipeline stages.
+This mechanism provides an easy way to configure and extend the functionality of a test, eliminating essentially the need to override pipeline stages for this purpose.
+ReFrame 3.0 deprecates the old practice for overriding pipeline stage methods in favor of using pipeline hooks.
+In the old syntax, it was quite common to override the ``setup()`` method, in order to configure your test based on the current programming environment or the current system partition.
+The following is a typical example of that:
+
+
+.. code:: python
+
+   def setup(self, partition, environ, **job_opts):
+       if environ.name == 'gnu':
+           self.build_system.cflags = ['-fopenmp']
+       elif environ.name == 'intel':
+           self.build_system.cflags = ['-qopenmp']
+
+       super().setup(partition, environ, **job_opts)
+
+
+Alternatively, this example could have been written as follows:
+
+.. code:: python
+
+   def setup(self, partition, environ, **job_opts):
+       super().setup(partition, environ, **job_opts)
+       if self.current_environ.name == 'gnu':
+           self.build_system.cflags = ['-fopenmp']
+       elif self.current_environ.name == 'intel':
+           self.build_system.cflags = ['-qopenmp']
+
+
+This syntax now issues a deprecation warning.
+Rewriting this using pipeline hooks is quite straightforward and leads to nicer and more intuitive code:
+
+.. code:: python
+
+   @rfm.run_before('compile')
+   def setflags(self):
+       if self.current_environ.name == 'gnu':
+           self.build_system.cflags = ['-fopenmp']
+       elif self.current_environ.name == 'intel':
+           self.build_system.cflags = ['-qopenmp']
+
+
+You could equally attach this function to run after the "setup" phase with ``@rfm.run_after('setup')``, as in the original example, but attaching it to the "compile" phase makes more sense.
+However, you can't attach this function *before* the "setup" phase, because the ``current_environ`` will not be available and it will be still ``None``.
+
+
+Force override a pipeline method
+================================
+
+Although pipeline hooks should be able to cover almost all the cases for writing tests in ReFrame, there might be corner cases that you need to override one of the pipeline methods, e.g., because you want to implement a stage differently.
+In this case, all you have to do is mark your test class as "special", and ReFrame will not issue any deprecation warning if you override pipeline stage methods:
+
+.. code:: python
+
+   class MyExtendedTest(rfm.RegressionTest, special=True):
+       def setup(self, partition, environ, **job_opts):
+           # do your custom stuff
+           super().setup(partition, environ, **job_opts)
+
+
+If you try to override the ``setup()`` method in any of the subclasses of ``MyExtendedTest``, you will still get a deprecation warning, which a desired behavior since the subclasses should be normal tests.
+
+
+Suppressing deprecation warnings
+================================
+
+You can suppress any deprecation warning issued by ReFrame by passing the ``--no-deprecation-warnings`` flag.
+

docs/tutorial.rst

@@ -461,12 +461,16 @@ Notice that in order to redefine a hook, you need not only redefine the method i
 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 <reframe.core.pipeline.RegressionTest.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()``.
+.. warning::
+   Configuring your test per programming environment and per system partition by overriding the :func:`setup() <reframe.core.pipeline.RegressionTest.setup>` method is deprecated.
+   Please refer to the `Migrate to ReFrame 3 <migration_2_to_3.html#updating-your-tests>`__ guide for more details.
 
+   .. versionchanged:: 3.0
 
 .. warning::
-   Setting the compiler flags in the programming environment has been dropped completely in version 2.17.
+   Support for setting the compiler flags in the programming environment has been dropped completely.
+
+   .. versionchanged:: 2.17
 
 
 An alternative implementation using dictionaries

reframe/core/meta.py

@@ -7,6 +7,8 @@
 # Met-class for creating regression tests.
 #
 
+from reframe.core.exceptions import user_deprecation_warning
+
 
 class RegressionTestMeta(type):
     def __init__(cls, name, bases, namespace, **kwargs):
@@ -33,3 +35,22 @@ def __init__(cls, name, bases, namespace, **kwargs):
 
         hooks['post_setup'] = fn_with_deps + hooks.get('post_setup', [])
         cls._rfm_pipeline_hooks = hooks
+
+        cls._final_methods = {v.__name__ for v in namespace.values()
+                              if hasattr(v, '_rfm_final')}
+
+        # Add the final functions from its parents
+        cls._final_methods.update(*(b._final_methods for b in bases
+                                    if hasattr(b, '_final_methods')))
+
+        if hasattr(cls, '_rfm_special_test') and cls._rfm_special_test:
+            return
+
+        for v in namespace.values():
+            for b in bases:
+                if callable(v) and v.__name__ in b._final_methods:
+                    msg = (f"'{cls.__qualname__}.{v.__name__}' attempts to "
+                           f"override final method "
+                           f"'{b.__qualname__}.{v.__name__}'; "
+                           f"consider using the reframe hooks instead")
+                    user_deprecation_warning(msg)