Address PR comments

Author: Theofilos Manitaras

docs/migration_2_to_3.rst

@@ -65,8 +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.
-From ReFrame 3.4, overriding pipeline stage methods is no longer allowed.
+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:
 
@@ -94,7 +93,7 @@ Alternatively, this example could have been written as follows:
            self.build_system.cflags = ['-qopenmp']
 
 
-This syntax is no longer valid and a ``ReframeSyntaxError`` is raised.
+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
@@ -112,7 +111,7 @@ However, you can't attach this function *before* the "setup" phase, because the
 
 .. warning::
    .. versionchanged:: 3.4
-      Overriding a pipeline stage method is no longer allowed and a ``ReframeSyntaxError`` is raised.
+      Overriding a pipeline stage method is no longer allowed and a reframe syntax error is raised.
 
 
 --------------------------------
@@ -130,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``, will again result in a ``ReframeSyntaxError``, 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/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
@@ -1093,8 +1093,7 @@ def setup(self, partition, environ, **job_opts):
               more details.
 
            .. versionchanged:: 3.4
-              Overriding this method directly unless you are in
-              special test. See `here
+              Overriding this method directly in no longer allowed. See `here
               <migration_2_to_3.html#force-override-a-pipeline-method>`__ for
               more details.
 
@@ -1140,8 +1139,7 @@ def compile(self):
               more details.
 
            .. versionchanged:: 3.4
-              Overriding this method directly unless you are in
-              special test. See `here
+              Overriding this method directly in no longer allowed. See `here
               <migration_2_to_3.html#force-override-a-pipeline-method>`__ for
               more details.
 
@@ -1243,8 +1241,7 @@ def compile_wait(self):
               more details.
 
            .. versionchanged:: 3.4
-              Overriding this method directly unless you are in
-              special test. See `here
+              Overriding this method directly in no longer allowed. See `here
               <migration_2_to_3.html#force-override-a-pipeline-method>`__ for
               more details.
 
@@ -1272,8 +1269,7 @@ def run(self):
               more details.
 
            .. versionchanged:: 3.4
-              Overriding this method directly unless you are in
-              special test. See `here
+              Overriding this method directly in no longer allowed. See `here
               <migration_2_to_3.html#force-override-a-pipeline-method>`__ for
               more details.
 
@@ -1384,8 +1380,7 @@ def run_complete(self):
 
 
            .. versionchanged:: 3.4
-              Overriding this method directly unless you are in
-              special test. See `here
+              Overriding this method directly in no longer allowed. See `here
               <migration_2_to_3.html#force-override-a-pipeline-method>`__ for
               more details.
 
@@ -1420,13 +1415,10 @@ def run_wait(self):
            more details.
 
            .. versionchanged:: 3.4
-              Overriding this method directly unless you are in
-              special test. See `here
+              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()
 
@@ -1469,13 +1461,10 @@ def check_sanity(self):
               more details.
 
            .. versionchanged:: 3.4
-              Overriding this method directly unless you are in
-              special test. See `here
+              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 = [
@@ -1510,13 +1499,10 @@ def check_performance(self):
               more details.
 
            .. versionchanged:: 3.4
-              Overriding this method directly unless you are in
-              special test. See `here
+              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
@@ -1636,13 +1622,10 @@ def cleanup(self, remove_files=False):
               more details.
 
            .. versionchanged:: 3.4
-              Overriding this method directly unless you are in
-              special test. See `here
+              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,7 +7,7 @@
 import pytest
 
 import reframe as rfm
-from reframe.core.exceptions import (NameConflictError, ReframeSyntaxError)
+from reframe.core.exceptions import NameConflictError, ReframeSyntaxError
 from reframe.frontend.loader import RegressionCheckLoader