Merge pull request #1636 from teojgo/enhancement/drop_override_support

[feat] Drop support of pipeline stage method override

Author: Vasileios Karakasis

docs/migration_2_to_3.rst

@@ -65,7 +65,7 @@ Pipeline methods and hooks
 
 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 of overriding pipeline stage methods in favor of using pipeline hooks.
+ReFrame 3.0 deprecates the old practice of overriding pipeline stage methods in favor of using pipeline hooks and ReFrame 3.4 disables that by default.
 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:
 
@@ -93,7 +93,7 @@ Alternatively, this example could have been written as follows:
            self.build_system.cflags = ['-qopenmp']
 
 
-This syntax now issues a deprecation warning.
+This syntax is no longer valid and it will raise a deprecation warning for ReFrame versions >= 3.0 and a reframe syntax error for versions >=3.4.
 Rewriting this using pipeline hooks is quite straightforward and leads to nicer and more intuitive code:
 
 .. code:: python
@@ -109,6 +109,10 @@ Rewriting this using pipeline hooks is quite straightforward and leads to nicer
 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``.
 
+.. warning::
+   .. versionchanged:: 3.4
+      Overriding a pipeline stage method is no longer allowed and a reframe syntax error is raised.
+
 
 --------------------------------
 Force override a pipeline method
@@ -125,7 +129,7 @@ In this case, all you have to do is mark your test class as "special", and ReFra
            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.
+If you try to override the ``setup()`` method in any of the subclasses of ``MyExtendedTest``, it will again result in a reframe syntax error, which is a desired behavior since the subclasses should be normal tests.
 
 
 Getting schedulers and launchers by name

reframe/core/meta.py

@@ -4,10 +4,10 @@
 # SPDX-License-Identifier: BSD-3-Clause
 
 #
-# Met-class for creating regression tests.
+# Meta-class for creating regression tests.
 #
 
-from reframe.core.warnings import user_deprecation_warning
+from reframe.core.exceptions import ReframeSyntaxError
 
 
 class RegressionTestMeta(type):
@@ -57,5 +57,5 @@ def __init__(cls, name, bases, namespace, **kwargs):
                     msg = (f"'{cls.__qualname__}.{v.__name__}' attempts to "
                            f"override final method "
                            f"'{b.__qualname__}.{v.__name__}'; "
-                           f"consider using the pipeline hooks instead")
-                    user_deprecation_warning(msg)
+                           f"you should use the pipeline hooks instead")
+                    raise ReframeSyntaxError(msg)

reframe/core/pipeline.py

@@ -30,7 +30,7 @@
 import reframe.utility.sanity as sn
 import reframe.utility.typecheck as typ
 import reframe.utility.udeps as udeps
-from reframe.core.backends import (getlauncher, getscheduler)
+from reframe.core.backends import getlauncher, getscheduler
 from reframe.core.buildsystems import BuildSystemField
 from reframe.core.containers import ContainerPlatformField
 from reframe.core.deferrable import _DeferredExpression
@@ -1094,6 +1094,11 @@ def setup(self, partition, environ, **job_opts):
               <migration_2_to_3.html#force-override-a-pipeline-method>`__ for
               more details.
 
+           .. versionchanged:: 3.4
+              Overriding this method directly in no longer allowed. See `here
+              <migration_2_to_3.html#force-override-a-pipeline-method>`__ for
+              more details.
+
         '''
         self._current_partition = partition
         self._current_environ = environ
@@ -1135,6 +1140,11 @@ def compile(self):
               <migration_2_to_3.html#force-override-a-pipeline-method>`__ for
               more details.
 
+           .. versionchanged:: 3.4
+              Overriding this method directly in no longer allowed. See `here
+              <migration_2_to_3.html#force-override-a-pipeline-method>`__ for
+              more details.
+
         '''
         if not self._current_environ:
             raise PipelineError('no programming environment set')
@@ -1232,6 +1242,11 @@ def compile_wait(self):
               <migration_2_to_3.html#force-override-a-pipeline-method>`__ for
               more details.
 
+           .. versionchanged:: 3.4
+              Overriding this method directly in no longer allowed. See `here
+              <migration_2_to_3.html#force-override-a-pipeline-method>`__ for
+              more details.
+
         '''
         self._build_job.wait()
 
@@ -1254,6 +1269,12 @@ def run(self):
               special test. See `here
               <migration_2_to_3.html#force-override-a-pipeline-method>`__ for
               more details.
+
+           .. versionchanged:: 3.4
+              Overriding this method directly in no longer allowed. See `here
+              <migration_2_to_3.html#force-override-a-pipeline-method>`__ for
+              more details.
+
         '''
         if not self.current_system or not self._current_partition:
             raise PipelineError('no system or system partition is set')
@@ -1359,6 +1380,12 @@ def run_complete(self):
            <migration_2_to_3.html#force-override-a-pipeline-method>`__ for
            more details.
 
+
+           .. versionchanged:: 3.4
+              Overriding this method directly in no longer allowed. See `here
+              <migration_2_to_3.html#force-override-a-pipeline-method>`__ for
+              more details.
+
         '''
         if not self._job:
             return True
@@ -1389,6 +1416,11 @@ def run_wait(self):
            <migration_2_to_3.html#force-override-a-pipeline-method>`__ for
            more details.
 
+           .. versionchanged:: 3.4
+              Overriding this method directly in no longer allowed. See `here
+              <migration_2_to_3.html#force-override-a-pipeline-method>`__ for
+              more details.
+
         '''
         self._job.wait()
 
@@ -1430,6 +1462,11 @@ def check_sanity(self):
               <migration_2_to_3.html#force-override-a-pipeline-method>`__ for
               more details.
 
+           .. versionchanged:: 3.4
+              Overriding this method directly in no longer allowed. See `here
+              <migration_2_to_3.html#force-override-a-pipeline-method>`__ for
+              more details.
+
         '''
         if rt.runtime().get_option('general/0/trap_job_errors'):
             sanity_patterns = [
@@ -1463,6 +1500,11 @@ def check_performance(self):
               <migration_2_to_3.html#force-override-a-pipeline-method>`__ for
               more details.
 
+           .. versionchanged:: 3.4
+              Overriding this method directly in no longer allowed. See `here
+              <migration_2_to_3.html#force-override-a-pipeline-method>`__ for
+              more details.
+
         '''
         if self.perf_patterns is None:
             return
@@ -1581,6 +1623,11 @@ def cleanup(self, remove_files=False):
               <migration_2_to_3.html#force-override-a-pipeline-method>`__ for
               more details.
 
+           .. versionchanged:: 3.4
+              Overriding this method directly in no longer allowed. See `here
+              <migration_2_to_3.html#force-override-a-pipeline-method>`__ for
+              more details.
+
         '''
         aliased = os.path.samefile(self._stagedir, self._outputdir)
         if aliased:

unittests/test_loader.py

@@ -7,10 +7,7 @@
 import pytest
 
 import reframe as rfm
-from reframe.core.exceptions import (ConfigError, NameConflictError,
-                                     RegressionTestLoadError)
-from reframe.core.systems import System
-from reframe.core.warnings import ReframeDeprecationWarning
+from reframe.core.exceptions import NameConflictError, ReframeSyntaxError
 from reframe.frontend.loader import RegressionCheckLoader
 
 
@@ -83,72 +80,62 @@ def test_load_bad_init(loader):
 
 
 def test_special_test():
-    with pytest.warns(ReframeDeprecationWarning):
+    with pytest.raises(ReframeSyntaxError):
         @rfm.simple_test
-        class TestDeprecated(rfm.RegressionTest):
+        class TestOverride(rfm.RegressionTest):
             def setup(self, partition, environ, **job_opts):
                 super().setup(system, environ, **job_opts)
 
-    with pytest.warns(ReframeDeprecationWarning):
+    with pytest.raises(ReframeSyntaxError):
         @rfm.simple_test
-        class TestDeprecatedRunOnly(rfm.RunOnlyRegressionTest):
+        class TestOverrideRunOnly(rfm.RunOnlyRegressionTest):
             def setup(self, partition, environ, **job_opts):
                 super().setup(system, environ, **job_opts)
 
-    with pytest.warns(ReframeDeprecationWarning):
+    with pytest.raises(ReframeSyntaxError):
         @rfm.simple_test
-        class TestDeprecatedCompileOnly(rfm.CompileOnlyRegressionTest):
+        class TestOverrideCompileOnly(rfm.CompileOnlyRegressionTest):
             def setup(self, partition, environ, **job_opts):
                 super().setup(system, environ, **job_opts)
 
-    with pytest.warns(ReframeDeprecationWarning):
-        @rfm.simple_test
-        class TestDeprecatedCompileOnlyDerived(TestDeprecatedCompileOnly):
-            def setup(self, partition, environ, **job_opts):
-                super().setup(system, environ, **job_opts)
-
-    with pytest.warns(None) as warnings:
-        @rfm.simple_test
-        class TestSimple(rfm.RegressionTest):
-            def __init__(self):
-                pass
+    @rfm.simple_test
+    class TestSimple(rfm.RegressionTest):
+        def __init__(self):
+            pass
 
-        @rfm.simple_test
-        class TestSpecial(rfm.RegressionTest, special=True):
-            def __init__(self):
-                pass
+    @rfm.simple_test
+    class TestSpecial(rfm.RegressionTest, special=True):
+        def __init__(self):
+            pass
 
-            def setup(self, partition, environ, **job_opts):
-                super().setup(system, environ, **job_opts)
+        def setup(self, partition, environ, **job_opts):
+            super().setup(system, environ, **job_opts)
 
-        @rfm.simple_test
-        class TestSpecialRunOnly(rfm.RunOnlyRegressionTest,
-                                 special=True):
-            def __init__(self):
-                pass
+    @rfm.simple_test
+    class TestSpecialRunOnly(rfm.RunOnlyRegressionTest,
+                             special=True):
+        def __init__(self):
+            pass
 
-            def setup(self, partition, environ, **job_opts):
-                super().setup(system, environ, **job_opts)
+        def setup(self, partition, environ, **job_opts):
+            super().setup(system, environ, **job_opts)
 
-            def run(self):
-                super().run()
+        def run(self):
+            super().run()
 
-        @rfm.simple_test
-        class TestSpecialCompileOnly(rfm.CompileOnlyRegressionTest,
-                                     special=True):
-            def __init__(self):
-                pass
-
-            def setup(self, partition, environ, **job_opts):
-                super().setup(system, environ, **job_opts)
+    @rfm.simple_test
+    class TestSpecialCompileOnly(rfm.CompileOnlyRegressionTest,
+                                 special=True):
+        def __init__(self):
+            pass
 
-            def run(self):
-                super().run()
+        def setup(self, partition, environ, **job_opts):
+            super().setup(system, environ, **job_opts)
 
-    assert not any(isinstance(w.message, ReframeDeprecationWarning)
-                   for w in warnings)
+        def run(self):
+            super().run()
 
-    with pytest.warns(ReframeDeprecationWarning) as warnings:
+    with pytest.raises(ReframeSyntaxError):
         @rfm.simple_test
         class TestSpecialDerived(TestSpecial):
             def __init__(self):
@@ -157,21 +144,16 @@ def __init__(self):
             def setup(self, partition, environ, **job_opts):
                 super().setup(system, environ, **job_opts)
 
-            def run(self):
-                super().run()
-
-    assert len(warnings) == 2
-
     @rfm.simple_test
     class TestFinal(rfm.RegressionTest):
         def __init__(self):
             pass
 
         @rfm.final
-        def my_new_final(seld):
+        def my_new_final(self):
             pass
 
-    with pytest.warns(ReframeDeprecationWarning):
+    with pytest.raises(ReframeSyntaxError):
         @rfm.simple_test
         class TestFinalDerived(TestFinal):
             def my_new_final(self, a, b):