From d44016144891d7e5b80b5053a3d17e4c6190d15a Mon Sep 17 00:00:00 2001 From: Eirini Koutsaniti Date: Wed, 11 Mar 2020 17:28:55 +0100 Subject: [PATCH 01/13] Deprecate test syntax that overrides `RegressionTest` methods --- reframe/core/decorators.py | 43 +++++++++++++++++++++++++++++++++++--- 1 file changed, 40 insertions(+), 3 deletions(-) diff --git a/reframe/core/decorators.py b/reframe/core/decorators.py index 6e399fb7de..7d6a4391ae 100644 --- a/reframe/core/decorators.py +++ b/reframe/core/decorators.py @@ -8,7 +8,7 @@ # __all__ = [ - 'parameterized_test', 'simple_test', 'required_version', + 'extend_test', 'parameterized_test', 'simple_test', 'required_version', 'require_deps', 'run_before', 'run_after' ] @@ -20,9 +20,11 @@ import traceback import reframe -from reframe.core.exceptions import ReframeSyntaxError, user_frame +from reframe.core.exceptions import (ReframeSyntaxError, + user_deprecation_warning, user_frame) from reframe.core.logging import getlogger -from reframe.core.pipeline import RegressionTest +from reframe.core.pipeline import (CompileOnlyRegressionTest, RegressionTest, + RunOnlyRegressionTest) from reframe.utility.versioning import VersionValidator @@ -72,6 +74,41 @@ def _validate_test(cls): raise ReframeSyntaxError('the decorated class must be a ' 'subclass of RegressionTest') + if not hasattr(cls, '_extend_class') or not cls._extend_class: + funcs = [ + i for i in dir(RegressionTest) if i not in ['__dict__' , + '__doc__', + '__init__', + '__init_subclass__', + '__module__', + '__subclasshook__'] + ] + if issubclass(cls, CompileOnlyRegressionTest): + test_class = CompileOnlyRegressionTest + elif issubclass(cls, RunOnlyRegressionTest): + test_class = RunOnlyRegressionTest + else: + test_class = RegressionTest + + for func in funcs: + if getattr(cls, func) != getattr(test_class, func): + msg = (f'Trying to override method {func}. The syntax ' + f'that overrides Regression test methods is ' + f'deprecated. Consider using the reframe hooks ' + f'instead or the decorator `extend_test`.') + user_deprecation_warning(msg) + + +def extend_test(cls): + '''Class decorator for allowing method overriding. + + .. versionadded:: 3.0 + + ''' + + cls._extend_class = True + return cls + def simple_test(cls): '''Class decorator for registering parameterless tests with ReFrame. From a80a6c3f83d597f80edb8e6404dbd801e5cb994f Mon Sep 17 00:00:00 2001 From: Eirini Koutsaniti Date: Wed, 11 Mar 2020 17:42:15 +0100 Subject: [PATCH 02/13] Fix whitespace --- reframe/core/decorators.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/reframe/core/decorators.py b/reframe/core/decorators.py index 7d6a4391ae..f5b3e022d2 100644 --- a/reframe/core/decorators.py +++ b/reframe/core/decorators.py @@ -76,7 +76,7 @@ def _validate_test(cls): if not hasattr(cls, '_extend_class') or not cls._extend_class: funcs = [ - i for i in dir(RegressionTest) if i not in ['__dict__' , + i for i in dir(RegressionTest) if i not in ['__dict__', '__doc__', '__init__', '__init_subclass__', From 467330db8c765fbf21028e9bee4f75b46831af06 Mon Sep 17 00:00:00 2001 From: Eirini Koutsaniti Date: Tue, 17 Mar 2020 08:56:14 +0100 Subject: [PATCH 03/13] Fix unittests warnings --- reframe/core/decorators.py | 3 ++- unittests/resources/checks/frontend_checks.py | 2 ++ unittests/resources/checks_unlisted/good.py | 2 ++ unittests/resources/checks_unlisted/kbd_interrupt.py | 1 + 4 files changed, 7 insertions(+), 1 deletion(-) diff --git a/reframe/core/decorators.py b/reframe/core/decorators.py index f5b3e022d2..a73a2e95b3 100644 --- a/reframe/core/decorators.py +++ b/reframe/core/decorators.py @@ -81,7 +81,8 @@ def _validate_test(cls): '__init__', '__init_subclass__', '__module__', - '__subclasshook__'] + '__subclasshook__', + '_rfm_pipeline_hooks'] ] if issubclass(cls, CompileOnlyRegressionTest): test_class = CompileOnlyRegressionTest diff --git a/unittests/resources/checks/frontend_checks.py b/unittests/resources/checks/frontend_checks.py index 36071bbc55..879ee6d02d 100644 --- a/unittests/resources/checks/frontend_checks.py +++ b/unittests/resources/checks/frontend_checks.py @@ -12,6 +12,7 @@ from reframe.core.exceptions import ReframeError, PerformanceError +@rfm.extend_test class BaseFrontendCheck(rfm.RunOnlyRegressionTest): def __init__(self): self.local = True @@ -34,6 +35,7 @@ def setup(self, system, environ, **job_opts): @rfm.simple_test +@rfm.extend_test class BadSetupCheckEarly(BaseFrontendCheck): def __init__(self): super().__init__() diff --git a/unittests/resources/checks_unlisted/good.py b/unittests/resources/checks_unlisted/good.py index a764ba57af..e6a9053b2a 100644 --- a/unittests/resources/checks_unlisted/good.py +++ b/unittests/resources/checks_unlisted/good.py @@ -14,6 +14,7 @@ @rfm.parameterized_test(*((x, y) for x in range(3) for y in range(2))) +@rfm.extend_test class MyBaseTest(RegressionTest): def __init__(self, a, b): self.a = a @@ -31,6 +32,7 @@ def __repr__(self): @rfm.parameterized_test(*({'a': x, 'b': y} for x in range(3) for y in range(2))) +@rfm.extend_test class AnotherBaseTest(RegressionTest): def __init__(self, a, b): self.a = a diff --git a/unittests/resources/checks_unlisted/kbd_interrupt.py b/unittests/resources/checks_unlisted/kbd_interrupt.py index 84676f9e0b..ee51dd8c20 100644 --- a/unittests/resources/checks_unlisted/kbd_interrupt.py +++ b/unittests/resources/checks_unlisted/kbd_interrupt.py @@ -14,6 +14,7 @@ @rfm.simple_test +@rfm.extend_test class KeyboardInterruptCheck(rfm.RunOnlyRegressionTest): def __init__(self): self.local = True From a5aac093db34120a0663705596c389d0bb78bd58 Mon Sep 17 00:00:00 2001 From: Eirini Koutsaniti Date: Wed, 18 Mar 2020 10:34:01 +0100 Subject: [PATCH 04/13] Add unittests for extend_test decorator --- .../resources/checks_unlisted/extend_class.py | 62 +++++++++++++++++++ unittests/test_loader.py | 8 +++ 2 files changed, 70 insertions(+) create mode 100644 unittests/resources/checks_unlisted/extend_class.py diff --git a/unittests/resources/checks_unlisted/extend_class.py b/unittests/resources/checks_unlisted/extend_class.py new file mode 100644 index 0000000000..525bb96089 --- /dev/null +++ b/unittests/resources/checks_unlisted/extend_class.py @@ -0,0 +1,62 @@ +# Copyright 2016-2020 Swiss National Supercomputing Centre (CSCS/ETH Zurich) +# ReFrame Project Developers. See the top-level LICENSE file for details. +# +# SPDX-License-Identifier: BSD-3-Clause + +import reframe as rfm + + +@rfm.simple_test +class TestSimple(rfm.RegressionTest): + # The test should not raise a deprecation warning even though + # it overrides __init__ + def __init__(self): + pass + + +@rfm.simple_test +class TestDeprecated(rfm.RegressionTest): + def setup(self, partition, environ, **job_opts): + super().setup(system, environ, **job_opts) + + +@rfm.simple_test +class TestDeprecatedRunOnly(rfm.RunOnlyRegressionTest): + def setup(self, partition, environ, **job_opts): + super().setup(system, environ, **job_opts) + + +@rfm.simple_test +class TestDeprecatedCompileOnly(rfm.CompileOnlyRegressionTest): + def setup(self, partition, environ, **job_opts): + super().setup(system, environ, **job_opts) + + +@rfm.simple_test +@rfm.extend_test +class TestExtended(rfm.RegressionTest): + def __init__(self): + pass + + def setup(self, partition, environ, **job_opts): + super().setup(system, environ, **job_opts) + + +@rfm.simple_test +@rfm.extend_test +class TestExtendedRunOnly(rfm.RunOnlyRegressionTest): + def __init__(self): + pass + + def setup(self, partition, environ, **job_opts): + super().setup(system, environ, **job_opts) + + +@rfm.simple_test +@rfm.extend_test +class TestExtendedCompileOnly(rfm.CompileOnlyRegressionTest): + def __init__(self): + pass + + def setup(self, partition, environ, **job_opts): + super().setup(system, environ, **job_opts) \ No newline at end of file diff --git a/unittests/test_loader.py b/unittests/test_loader.py index 8d0a20b3af..d7043d612f 100644 --- a/unittests/test_loader.py +++ b/unittests/test_loader.py @@ -8,6 +8,7 @@ import unittest from reframe.core.exceptions import (ConfigError, NameConflictError, + ReframeDeprecationWarning, RegressionTestLoadError) from reframe.core.systems import System from reframe.frontend.loader import RegressionCheckLoader @@ -71,3 +72,10 @@ def test_load_bad_init(self): tests = self.loader.load_from_file( 'unittests/resources/checks_unlisted/bad_init_check.py') assert 0 == len(tests) + + def test_extend_decorator(self): + with pytest.warns(ReframeDeprecationWarning) as record: + tests = self.loader.load_from_file( + 'unittests/resources/checks_unlisted/extend_class.py') + + assert len(record) == 3 From eea6832c03972ae312b5a5d69ce43f8d6e281a40 Mon Sep 17 00:00:00 2001 From: Eirini Koutsaniti Date: Wed, 8 Apr 2020 16:44:40 +0200 Subject: [PATCH 05/13] Deprecate test syntax that overrides `RegressionTest` methods --- reframe/core/decorators.py | 44 +-------- reframe/core/meta.py | 26 ++++++ reframe/core/pipeline.py | 34 ++++++- unittests/resources/checks/frontend_checks.py | 23 ++--- .../resources/checks_unlisted/extend_class.py | 62 ------------- unittests/resources/checks_unlisted/good.py | 2 - .../checks_unlisted/kbd_interrupt.py | 4 +- unittests/test_loader.py | 93 ++++++++++++++++++- 8 files changed, 163 insertions(+), 125 deletions(-) delete mode 100644 unittests/resources/checks_unlisted/extend_class.py diff --git a/reframe/core/decorators.py b/reframe/core/decorators.py index a73a2e95b3..6e399fb7de 100644 --- a/reframe/core/decorators.py +++ b/reframe/core/decorators.py @@ -8,7 +8,7 @@ # __all__ = [ - 'extend_test', 'parameterized_test', 'simple_test', 'required_version', + 'parameterized_test', 'simple_test', 'required_version', 'require_deps', 'run_before', 'run_after' ] @@ -20,11 +20,9 @@ import traceback import reframe -from reframe.core.exceptions import (ReframeSyntaxError, - user_deprecation_warning, user_frame) +from reframe.core.exceptions import ReframeSyntaxError, user_frame from reframe.core.logging import getlogger -from reframe.core.pipeline import (CompileOnlyRegressionTest, RegressionTest, - RunOnlyRegressionTest) +from reframe.core.pipeline import RegressionTest from reframe.utility.versioning import VersionValidator @@ -74,42 +72,6 @@ def _validate_test(cls): raise ReframeSyntaxError('the decorated class must be a ' 'subclass of RegressionTest') - if not hasattr(cls, '_extend_class') or not cls._extend_class: - funcs = [ - i for i in dir(RegressionTest) if i not in ['__dict__', - '__doc__', - '__init__', - '__init_subclass__', - '__module__', - '__subclasshook__', - '_rfm_pipeline_hooks'] - ] - if issubclass(cls, CompileOnlyRegressionTest): - test_class = CompileOnlyRegressionTest - elif issubclass(cls, RunOnlyRegressionTest): - test_class = RunOnlyRegressionTest - else: - test_class = RegressionTest - - for func in funcs: - if getattr(cls, func) != getattr(test_class, func): - msg = (f'Trying to override method {func}. The syntax ' - f'that overrides Regression test methods is ' - f'deprecated. Consider using the reframe hooks ' - f'instead or the decorator `extend_test`.') - user_deprecation_warning(msg) - - -def extend_test(cls): - '''Class decorator for allowing method overriding. - - .. versionadded:: 3.0 - - ''' - - cls._extend_class = True - return cls - def simple_test(cls): '''Class decorator for registering parameterless tests with ReFrame. diff --git a/reframe/core/meta.py b/reframe/core/meta.py index 8476434d7e..4c6136e883 100644 --- a/reframe/core/meta.py +++ b/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,27 @@ 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, '_final')} + + # Add the final functions from its parents + cls._final_methods.update(*[b._final_methods for b in bases + if hasattr(b, '_final_methods')]) + + # Filter the methods that are defined in this class but are not final + for v in namespace.values(): + if callable(v) and not hasattr(v, '_final'): + cls._final_methods.discard(v.__name__) + + if hasattr(cls, '_extended_test') and cls._extended_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) diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 1e804958c5..263e64c050 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -107,6 +107,16 @@ def _fn(obj, *args, **kwargs): return _deco +def final(fn): + fn._final = True + + @functools.wraps(fn) + def _wrapped(*args, **kwargs): + return fn(*args, **kwargs) + + return _wrapped + + class RegressionTest(metaclass=RegressionTestMeta): '''Base class for regression tests. @@ -681,6 +691,11 @@ def __new__(cls, *args, **kwargs): def __init__(self): pass + @classmethod + def __init_subclass__(cls, *, extended_test=False, **kwargs): + super().__init_subclass__(**kwargs) + cls._extended_test = extended_test + def _rfm_init(self, name=None, prefix=None): if name is not None: self.name = name @@ -1005,6 +1020,7 @@ def _setup_perf_logging(self): self._perf_logger = logging.getperflogger(self) @_run_hooks() + @final def setup(self, partition, environ, **job_opts): '''The setup phase of the regression test pipeline. @@ -1037,6 +1053,7 @@ def _clone_to_stagedir(self, url): os_ext.git_clone(self.sourcesdir, self._stagedir) @_run_hooks('pre_compile') + @final def compile(self): '''The compilation phase of the regression test pipeline. @@ -1125,6 +1142,7 @@ def compile(self): self._build_job.submit() @_run_hooks('post_compile') + @final def compile_wait(self): '''Wait for compilation phase to finish. @@ -1138,6 +1156,7 @@ def compile_wait(self): raise BuildError(self._build_job.stdout, self._build_job.stderr) @_run_hooks('pre_run') + @final def run(self): '''The run phase of the regression test pipeline. @@ -1226,6 +1245,7 @@ def run(self): if self.job.sched_flex_alloc_nodes: self.num_tasks = self.job.num_tasks + @final def poll(self): '''Poll the test's state. @@ -1242,6 +1262,7 @@ def poll(self): return self._job.finished() @_run_hooks('post_run') + @final def wait(self): '''Wait for this test to finish. @@ -1251,10 +1272,12 @@ def wait(self): self.logger.debug('spawned job finished') @_run_hooks() + @final def sanity(self): self.check_sanity() @_run_hooks() + @final def performance(self): try: self.check_performance() @@ -1383,6 +1406,7 @@ def _copy_to_outputdir(self): shutil.copytree(f, os.path.join(self.outputdir, f_orig)) @_run_hooks() + @final def cleanup(self, remove_files=False): '''The cleanup phase of the regression test pipeline. @@ -1487,19 +1511,21 @@ def __str__(self): self.name, self.prefix) -class RunOnlyRegressionTest(RegressionTest): +class RunOnlyRegressionTest(RegressionTest, extended_test=True): '''Base class for run-only regression tests. This class is also directly available under the top-level :mod:`reframe` module. ''' + @final def compile(self): '''The compilation phase of the regression test pipeline. This is a no-op for this type of test. ''' + @final def compile_wait(self): '''Wait for compilation phase to finish. @@ -1507,6 +1533,7 @@ def compile_wait(self): ''' @_run_hooks('pre_run') + @final def run(self): '''The run phase of the regression test pipeline. @@ -1523,7 +1550,7 @@ def run(self): super().run.__wrapped__(self) -class CompileOnlyRegressionTest(RegressionTest): +class CompileOnlyRegressionTest(RegressionTest, extended_test=True): '''Base class for compile-only regression tests. These tests are by default local and will skip the run phase of the @@ -1541,6 +1568,7 @@ def _rfm_init(self, *args, **kwargs): self.local = True @_run_hooks() + @final def setup(self, partition, environ, **job_opts): '''The setup stage of the regression test pipeline. @@ -1562,12 +1590,14 @@ def stdout(self): def stderr(self): return self._build_job.stderr + @final def run(self): '''The run stage of the regression test pipeline. Implemented as no-op. ''' + @final def wait(self): '''Wait for this test to finish. diff --git a/unittests/resources/checks/frontend_checks.py b/unittests/resources/checks/frontend_checks.py index 879ee6d02d..dcc12e423a 100644 --- a/unittests/resources/checks/frontend_checks.py +++ b/unittests/resources/checks/frontend_checks.py @@ -12,7 +12,6 @@ from reframe.core.exceptions import ReframeError, PerformanceError -@rfm.extend_test class BaseFrontendCheck(rfm.RunOnlyRegressionTest): def __init__(self): self.local = True @@ -29,13 +28,11 @@ def __init__(self): self.valid_systems = ['*'] self.valid_prog_environs = ['*'] - def setup(self, system, environ, **job_opts): - super().setup(system, environ, **job_opts) + @rfm.run_after('setup') + def raise_error(self): raise ReframeError('Setup failure') - @rfm.simple_test -@rfm.extend_test class BadSetupCheckEarly(BaseFrontendCheck): def __init__(self): super().__init__() @@ -43,7 +40,8 @@ def __init__(self): self.valid_prog_environs = ['*'] self.local = False - def setup(self, system, environ, **job_opts): + @rfm.run_before('setup') + def raise_error_early(self): raise ReframeError('Setup failure') @@ -100,7 +98,7 @@ def check_performance(self): raise PerformanceError('performance failure') -class KeyboardInterruptCheck(BaseFrontendCheck): +class KeyboardInterruptCheck(BaseFrontendCheck, extended_test=True): '''Simulate keyboard interrupt during test's execution.''' def __init__(self, phase='wait'): @@ -110,12 +108,11 @@ def __init__(self, phase='wait'): self.valid_prog_environs = ['*'] self.phase = phase - def setup(self, system, environ, **job_opts): + @rfm.run_before('setup') + def raise_before_setup(self): if self.phase == 'setup': raise KeyboardInterrupt - super().setup(system, environ, **job_opts) - def wait(self): # We do our nasty stuff in wait() to make things more complicated if self.phase == 'wait': @@ -124,7 +121,7 @@ def wait(self): super().wait() -class SystemExitCheck(BaseFrontendCheck): +class SystemExitCheck(BaseFrontendCheck, extended_test=True): '''Simulate system exit from within a check.''' def __init__(self): @@ -175,14 +172,14 @@ def __init__(self, sleep_time): SleepCheck._next_id += 1 -class SleepCheckPollFail(SleepCheck): +class SleepCheckPollFail(SleepCheck, extended_test=True): '''Emulate a test failing in the polling phase.''' def poll(self): raise ValueError -class SleepCheckPollFailLate(SleepCheck): +class SleepCheckPollFailLate(SleepCheck, extended_test=True): '''Emulate a test failing in the polling phase after the test has finished.''' diff --git a/unittests/resources/checks_unlisted/extend_class.py b/unittests/resources/checks_unlisted/extend_class.py deleted file mode 100644 index 525bb96089..0000000000 --- a/unittests/resources/checks_unlisted/extend_class.py +++ /dev/null @@ -1,62 +0,0 @@ -# Copyright 2016-2020 Swiss National Supercomputing Centre (CSCS/ETH Zurich) -# ReFrame Project Developers. See the top-level LICENSE file for details. -# -# SPDX-License-Identifier: BSD-3-Clause - -import reframe as rfm - - -@rfm.simple_test -class TestSimple(rfm.RegressionTest): - # The test should not raise a deprecation warning even though - # it overrides __init__ - def __init__(self): - pass - - -@rfm.simple_test -class TestDeprecated(rfm.RegressionTest): - def setup(self, partition, environ, **job_opts): - super().setup(system, environ, **job_opts) - - -@rfm.simple_test -class TestDeprecatedRunOnly(rfm.RunOnlyRegressionTest): - def setup(self, partition, environ, **job_opts): - super().setup(system, environ, **job_opts) - - -@rfm.simple_test -class TestDeprecatedCompileOnly(rfm.CompileOnlyRegressionTest): - def setup(self, partition, environ, **job_opts): - super().setup(system, environ, **job_opts) - - -@rfm.simple_test -@rfm.extend_test -class TestExtended(rfm.RegressionTest): - def __init__(self): - pass - - def setup(self, partition, environ, **job_opts): - super().setup(system, environ, **job_opts) - - -@rfm.simple_test -@rfm.extend_test -class TestExtendedRunOnly(rfm.RunOnlyRegressionTest): - def __init__(self): - pass - - def setup(self, partition, environ, **job_opts): - super().setup(system, environ, **job_opts) - - -@rfm.simple_test -@rfm.extend_test -class TestExtendedCompileOnly(rfm.CompileOnlyRegressionTest): - def __init__(self): - pass - - def setup(self, partition, environ, **job_opts): - super().setup(system, environ, **job_opts) \ No newline at end of file diff --git a/unittests/resources/checks_unlisted/good.py b/unittests/resources/checks_unlisted/good.py index e6a9053b2a..a764ba57af 100644 --- a/unittests/resources/checks_unlisted/good.py +++ b/unittests/resources/checks_unlisted/good.py @@ -14,7 +14,6 @@ @rfm.parameterized_test(*((x, y) for x in range(3) for y in range(2))) -@rfm.extend_test class MyBaseTest(RegressionTest): def __init__(self, a, b): self.a = a @@ -32,7 +31,6 @@ def __repr__(self): @rfm.parameterized_test(*({'a': x, 'b': y} for x in range(3) for y in range(2))) -@rfm.extend_test class AnotherBaseTest(RegressionTest): def __init__(self, a, b): self.a = a diff --git a/unittests/resources/checks_unlisted/kbd_interrupt.py b/unittests/resources/checks_unlisted/kbd_interrupt.py index ee51dd8c20..ab771eb8ce 100644 --- a/unittests/resources/checks_unlisted/kbd_interrupt.py +++ b/unittests/resources/checks_unlisted/kbd_interrupt.py @@ -14,7 +14,6 @@ @rfm.simple_test -@rfm.extend_test class KeyboardInterruptCheck(rfm.RunOnlyRegressionTest): def __init__(self): self.local = True @@ -23,5 +22,6 @@ def __init__(self): self.valid_prog_environs = ['*'] self.tags = {self.name} - def setup(self, system, environ, **job_opts): + @rfm.run_before('setup') + def raise_keyboard_interrupt (self): raise KeyboardInterrupt diff --git a/unittests/test_loader.py b/unittests/test_loader.py index d7043d612f..7ec6478187 100644 --- a/unittests/test_loader.py +++ b/unittests/test_loader.py @@ -7,6 +7,7 @@ import pytest import unittest +import reframe as rfm from reframe.core.exceptions import (ConfigError, NameConflictError, ReframeDeprecationWarning, RegressionTestLoadError) @@ -75,7 +76,93 @@ def test_load_bad_init(self): def test_extend_decorator(self): with pytest.warns(ReframeDeprecationWarning) as record: - tests = self.loader.load_from_file( - 'unittests/resources/checks_unlisted/extend_class.py') + # tests = self.loader.load_from_file( + # 'unittests/resources/checks_unlisted/extend_class.py') + @rfm.simple_test + class TestSimple(rfm.RegressionTest): + # The test should not raise a deprecation warning even though + # it overrides __init__ + def __init__(self): + pass - assert len(record) == 3 + + @rfm.simple_test + class TestDeprecated(rfm.RegressionTest): + # Should raise a warning + def setup(self, partition, environ, **job_opts): + super().setup(system, environ, **job_opts) + + + @rfm.simple_test + class TestDeprecatedRunOnly(rfm.RunOnlyRegressionTest): + # Should raise a warning + def setup(self, partition, environ, **job_opts): + super().setup(system, environ, **job_opts) + + + @rfm.simple_test + class TestDeprecatedCompileOnly(rfm.CompileOnlyRegressionTest): + # Should raise a warning + def setup(self, partition, environ, **job_opts): + super().setup(system, environ, **job_opts) + + + @rfm.simple_test + class TestDeprecatedCompileOnlyDerived(TestDeprecatedCompileOnly): + # Should not raise a warning because the setup of the parent was not set as final + def setup(self, partition, environ, **job_opts): + super().setup(system, environ, **job_opts) + + + @rfm.simple_test + class TestExtended(rfm.RegressionTest, extended_test=True): + def __init__(self): + pass + + # Should not raise a warning + def setup(self, partition, environ, **job_opts): + super().setup(system, environ, **job_opts) + + + @rfm.simple_test + class TestExtendedDerived(TestExtended): + def __init__(self): + pass + + # Should not raise a warning + def setup(self, partition, environ, **job_opts): + super().setup(system, environ, **job_opts) + + # Should raise a warning + def run(self): + super().run() + + + @rfm.simple_test + class TestExtendedRunOnly(rfm.RunOnlyRegressionTest, extended_test=True): + def __init__(self): + pass + + # Should not raise a warning + def setup(self, partition, environ, **job_opts): + super().setup(system, environ, **job_opts) + + # Should not raise a warning + def run(self): + super().run() + + + @rfm.simple_test + class TestExtendedCompileOnly(rfm.CompileOnlyRegressionTest, extended_test=True): + def __init__(self): + pass + + # Should not raise a warning + def setup(self, partition, environ, **job_opts): + super().setup(system, environ, **job_opts) + + # Should not raise a warning + def run(self): + super().run() + + assert len(record) == 4 From 4f38bd9dd9896da7cc66a5cdc7f76a39641430d6 Mon Sep 17 00:00:00 2001 From: Eirini Koutsaniti Date: Wed, 8 Apr 2020 16:50:52 +0200 Subject: [PATCH 06/13] Fix PEP8 issues --- unittests/resources/checks_unlisted/kbd_interrupt.py | 2 +- unittests/test_loader.py | 10 ---------- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/unittests/resources/checks_unlisted/kbd_interrupt.py b/unittests/resources/checks_unlisted/kbd_interrupt.py index ab771eb8ce..0010e8d777 100644 --- a/unittests/resources/checks_unlisted/kbd_interrupt.py +++ b/unittests/resources/checks_unlisted/kbd_interrupt.py @@ -23,5 +23,5 @@ def __init__(self): self.tags = {self.name} @rfm.run_before('setup') - def raise_keyboard_interrupt (self): + def raise_keyboard_interrupt(self): raise KeyboardInterrupt diff --git a/unittests/test_loader.py b/unittests/test_loader.py index 7ec6478187..527d264aa1 100644 --- a/unittests/test_loader.py +++ b/unittests/test_loader.py @@ -76,8 +76,6 @@ def test_load_bad_init(self): def test_extend_decorator(self): with pytest.warns(ReframeDeprecationWarning) as record: - # tests = self.loader.load_from_file( - # 'unittests/resources/checks_unlisted/extend_class.py') @rfm.simple_test class TestSimple(rfm.RegressionTest): # The test should not raise a deprecation warning even though @@ -85,35 +83,30 @@ class TestSimple(rfm.RegressionTest): def __init__(self): pass - @rfm.simple_test class TestDeprecated(rfm.RegressionTest): # Should raise a warning def setup(self, partition, environ, **job_opts): super().setup(system, environ, **job_opts) - @rfm.simple_test class TestDeprecatedRunOnly(rfm.RunOnlyRegressionTest): # Should raise a warning def setup(self, partition, environ, **job_opts): super().setup(system, environ, **job_opts) - @rfm.simple_test class TestDeprecatedCompileOnly(rfm.CompileOnlyRegressionTest): # Should raise a warning def setup(self, partition, environ, **job_opts): super().setup(system, environ, **job_opts) - @rfm.simple_test class TestDeprecatedCompileOnlyDerived(TestDeprecatedCompileOnly): # Should not raise a warning because the setup of the parent was not set as final def setup(self, partition, environ, **job_opts): super().setup(system, environ, **job_opts) - @rfm.simple_test class TestExtended(rfm.RegressionTest, extended_test=True): def __init__(self): @@ -123,7 +116,6 @@ def __init__(self): def setup(self, partition, environ, **job_opts): super().setup(system, environ, **job_opts) - @rfm.simple_test class TestExtendedDerived(TestExtended): def __init__(self): @@ -137,7 +129,6 @@ def setup(self, partition, environ, **job_opts): def run(self): super().run() - @rfm.simple_test class TestExtendedRunOnly(rfm.RunOnlyRegressionTest, extended_test=True): def __init__(self): @@ -151,7 +142,6 @@ def setup(self, partition, environ, **job_opts): def run(self): super().run() - @rfm.simple_test class TestExtendedCompileOnly(rfm.CompileOnlyRegressionTest, extended_test=True): def __init__(self): From aa4d65c9032051b3066450c2fe42dd30c856b7d9 Mon Sep 17 00:00:00 2001 From: Eirini Koutsaniti Date: Wed, 8 Apr 2020 16:52:55 +0200 Subject: [PATCH 07/13] Fix PEP8 issues --- unittests/test_loader.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/unittests/test_loader.py b/unittests/test_loader.py index 527d264aa1..e00b065262 100644 --- a/unittests/test_loader.py +++ b/unittests/test_loader.py @@ -103,7 +103,8 @@ def setup(self, partition, environ, **job_opts): @rfm.simple_test class TestDeprecatedCompileOnlyDerived(TestDeprecatedCompileOnly): - # Should not raise a warning because the setup of the parent was not set as final + # Should not raise a warning because the setup of the parent + # was not set as final def setup(self, partition, environ, **job_opts): super().setup(system, environ, **job_opts) @@ -130,7 +131,8 @@ def run(self): super().run() @rfm.simple_test - class TestExtendedRunOnly(rfm.RunOnlyRegressionTest, extended_test=True): + class TestExtendedRunOnly(rfm.RunOnlyRegressionTest, + extended_test=True): def __init__(self): pass @@ -143,7 +145,8 @@ def run(self): super().run() @rfm.simple_test - class TestExtendedCompileOnly(rfm.CompileOnlyRegressionTest, extended_test=True): + class TestExtendedCompileOnly(rfm.CompileOnlyRegressionTest, + extended_test=True): def __init__(self): pass From 2e6f8213eee1ba710cabc1ce246640a092c4099d Mon Sep 17 00:00:00 2001 From: Eirini Koutsaniti Date: Wed, 8 Apr 2020 18:26:05 +0200 Subject: [PATCH 08/13] Rename unittest --- unittests/test_loader.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unittests/test_loader.py b/unittests/test_loader.py index e00b065262..36ed99cc9b 100644 --- a/unittests/test_loader.py +++ b/unittests/test_loader.py @@ -74,7 +74,7 @@ def test_load_bad_init(self): 'unittests/resources/checks_unlisted/bad_init_check.py') assert 0 == len(tests) - def test_extend_decorator(self): + def test_extended_test(self): with pytest.warns(ReframeDeprecationWarning) as record: @rfm.simple_test class TestSimple(rfm.RegressionTest): From a6681df3ff34de8e2b049ee2baf6ab78540e2581 Mon Sep 17 00:00:00 2001 From: Eirini Koutsaniti Date: Tue, 14 Apr 2020 08:13:58 +0200 Subject: [PATCH 09/13] Rename variables and update unittests --- reframe/core/meta.py | 17 ++--- reframe/core/pipeline.py | 18 ++---- unittests/resources/checks/frontend_checks.py | 8 +-- unittests/test_loader.py | 64 +++++++++++++------ 4 files changed, 59 insertions(+), 48 deletions(-) diff --git a/reframe/core/meta.py b/reframe/core/meta.py index 4c6136e883..d20d1f30cc 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -37,18 +37,13 @@ def __init__(cls, name, bases, namespace, **kwargs): cls._rfm_pipeline_hooks = hooks cls._final_methods = {v.__name__ for v in namespace.values() - if hasattr(v, '_final')} + 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')]) + cls._final_methods.update(*(b._final_methods for b in bases + if hasattr(b, '_final_methods'))) - # Filter the methods that are defined in this class but are not final - for v in namespace.values(): - if callable(v) and not hasattr(v, '_final'): - cls._final_methods.discard(v.__name__) - - if hasattr(cls, '_extended_test') and cls._extended_test: + if hasattr(cls, '_rfm_special_test') and cls._rfm_special_test: return for v in namespace.values(): @@ -56,6 +51,6 @@ def __init__(cls, name, bases, namespace, **kwargs): 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.") + f"'{b.__qualname__}.{v.__name__}'; " + f"consider using the reframe hooks instead") user_deprecation_warning(msg) diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 263e64c050..2e388dc454 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -7,7 +7,7 @@ # Basic functionality for regression tests # -__all__ = ['RegressionTest', +__all__ = ['final', 'RegressionTest', 'RunOnlyRegressionTest', 'CompileOnlyRegressionTest', 'DEPEND_EXACT', 'DEPEND_BY_ENV', 'DEPEND_FULLY'] @@ -108,7 +108,7 @@ def _fn(obj, *args, **kwargs): def final(fn): - fn._final = True + fn._rfm_final = True @functools.wraps(fn) def _wrapped(*args, **kwargs): @@ -692,9 +692,9 @@ def __init__(self): pass @classmethod - def __init_subclass__(cls, *, extended_test=False, **kwargs): + def __init_subclass__(cls, *, special=False, **kwargs): super().__init_subclass__(**kwargs) - cls._extended_test = extended_test + cls._rfm_special_test = special def _rfm_init(self, name=None, prefix=None): if name is not None: @@ -1511,21 +1511,19 @@ def __str__(self): self.name, self.prefix) -class RunOnlyRegressionTest(RegressionTest, extended_test=True): +class RunOnlyRegressionTest(RegressionTest, special=True): '''Base class for run-only regression tests. This class is also directly available under the top-level :mod:`reframe` module. ''' - @final def compile(self): '''The compilation phase of the regression test pipeline. This is a no-op for this type of test. ''' - @final def compile_wait(self): '''Wait for compilation phase to finish. @@ -1533,7 +1531,6 @@ def compile_wait(self): ''' @_run_hooks('pre_run') - @final def run(self): '''The run phase of the regression test pipeline. @@ -1550,7 +1547,7 @@ def run(self): super().run.__wrapped__(self) -class CompileOnlyRegressionTest(RegressionTest, extended_test=True): +class CompileOnlyRegressionTest(RegressionTest, special=True): '''Base class for compile-only regression tests. These tests are by default local and will skip the run phase of the @@ -1568,7 +1565,6 @@ def _rfm_init(self, *args, **kwargs): self.local = True @_run_hooks() - @final def setup(self, partition, environ, **job_opts): '''The setup stage of the regression test pipeline. @@ -1590,14 +1586,12 @@ def stdout(self): def stderr(self): return self._build_job.stderr - @final def run(self): '''The run stage of the regression test pipeline. Implemented as no-op. ''' - @final def wait(self): '''Wait for this test to finish. diff --git a/unittests/resources/checks/frontend_checks.py b/unittests/resources/checks/frontend_checks.py index dcc12e423a..6b7a30c013 100644 --- a/unittests/resources/checks/frontend_checks.py +++ b/unittests/resources/checks/frontend_checks.py @@ -98,7 +98,7 @@ def check_performance(self): raise PerformanceError('performance failure') -class KeyboardInterruptCheck(BaseFrontendCheck, extended_test=True): +class KeyboardInterruptCheck(BaseFrontendCheck, special=True): '''Simulate keyboard interrupt during test's execution.''' def __init__(self, phase='wait'): @@ -121,7 +121,7 @@ def wait(self): super().wait() -class SystemExitCheck(BaseFrontendCheck, extended_test=True): +class SystemExitCheck(BaseFrontendCheck, special=True): '''Simulate system exit from within a check.''' def __init__(self): @@ -172,14 +172,14 @@ def __init__(self, sleep_time): SleepCheck._next_id += 1 -class SleepCheckPollFail(SleepCheck, extended_test=True): +class SleepCheckPollFail(SleepCheck, special=True): '''Emulate a test failing in the polling phase.''' def poll(self): raise ValueError -class SleepCheckPollFailLate(SleepCheck, extended_test=True): +class SleepCheckPollFailLate(SleepCheck, special=True): '''Emulate a test failing in the polling phase after the test has finished.''' diff --git a/unittests/test_loader.py b/unittests/test_loader.py index 36ed99cc9b..c17d122351 100644 --- a/unittests/test_loader.py +++ b/unittests/test_loader.py @@ -75,41 +75,43 @@ def test_load_bad_init(self): assert 0 == len(tests) def test_extended_test(self): - with pytest.warns(ReframeDeprecationWarning) as record: - @rfm.simple_test - class TestSimple(rfm.RegressionTest): - # The test should not raise a deprecation warning even though - # it overrides __init__ - def __init__(self): - pass - + with pytest.warns(ReframeDeprecationWarning): @rfm.simple_test class TestDeprecated(rfm.RegressionTest): - # Should raise a warning def setup(self, partition, environ, **job_opts): super().setup(system, environ, **job_opts) + with pytest.warns(ReframeDeprecationWarning): @rfm.simple_test class TestDeprecatedRunOnly(rfm.RunOnlyRegressionTest): # Should raise a warning def setup(self, partition, environ, **job_opts): super().setup(system, environ, **job_opts) + with pytest.warns(ReframeDeprecationWarning): @rfm.simple_test class TestDeprecatedCompileOnly(rfm.CompileOnlyRegressionTest): # Should raise a warning def setup(self, partition, environ, **job_opts): super().setup(system, environ, **job_opts) + with pytest.warns(ReframeDeprecationWarning): @rfm.simple_test class TestDeprecatedCompileOnlyDerived(TestDeprecatedCompileOnly): - # Should not raise a warning because the setup of the parent - # was not set as final + # Should raise a warning def setup(self, partition, environ, **job_opts): super().setup(system, environ, **job_opts) + with pytest.warns(None) as warnings: @rfm.simple_test - class TestExtended(rfm.RegressionTest, extended_test=True): + class TestSimple(rfm.RegressionTest): + # The test should not raise a deprecation warning even though + # it overrides __init__ + def __init__(self): + pass + + @rfm.simple_test + class TestExtended(rfm.RegressionTest, special=True): def __init__(self): pass @@ -118,7 +120,8 @@ def setup(self, partition, environ, **job_opts): super().setup(system, environ, **job_opts) @rfm.simple_test - class TestExtendedDerived(TestExtended): + class TestExtendedRunOnly(rfm.RunOnlyRegressionTest, + special=True): def __init__(self): pass @@ -126,13 +129,13 @@ def __init__(self): def setup(self, partition, environ, **job_opts): super().setup(system, environ, **job_opts) - # Should raise a warning + # Should not raise a warning def run(self): super().run() @rfm.simple_test - class TestExtendedRunOnly(rfm.RunOnlyRegressionTest, - extended_test=True): + class TestExtendedCompileOnly(rfm.CompileOnlyRegressionTest, + special=True): def __init__(self): pass @@ -144,18 +147,37 @@ def setup(self, partition, environ, **job_opts): def run(self): super().run() + assert not any(isinstance(w.message, ReframeDeprecationWarning) + for w in warnings) + + with pytest.warns(ReframeDeprecationWarning) as warnings: @rfm.simple_test - class TestExtendedCompileOnly(rfm.CompileOnlyRegressionTest, - extended_test=True): + class TestExtendedDerived(TestExtended): def __init__(self): pass - # Should not raise a warning + # Should raise a warning def setup(self, partition, environ, **job_opts): super().setup(system, environ, **job_opts) - # Should not raise a warning + # Should raise a warning def run(self): super().run() - assert len(record) == 4 + assert len(warnings) == 2 + + @rfm.simple_test + class TestFinal(rfm.RegressionTest): + def __init__(self): + pass + + @rfm.final + def my_new_final(seld): + pass + + with pytest.warns(ReframeDeprecationWarning): + @rfm.simple_test + class TestFinalDerived(TestFinal): + # Should raise a warning + def my_new_final(self, a, b): + pass From 77496b8061eaa50ae8272611b29c59dcb7377fd6 Mon Sep 17 00:00:00 2001 From: Eirini Koutsaniti Date: Tue, 14 Apr 2020 14:12:20 +0200 Subject: [PATCH 10/13] Rename extended test to special test --- unittests/test_loader.py | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/unittests/test_loader.py b/unittests/test_loader.py index c17d122351..69faf463a4 100644 --- a/unittests/test_loader.py +++ b/unittests/test_loader.py @@ -84,29 +84,24 @@ def setup(self, partition, environ, **job_opts): with pytest.warns(ReframeDeprecationWarning): @rfm.simple_test class TestDeprecatedRunOnly(rfm.RunOnlyRegressionTest): - # Should raise a warning def setup(self, partition, environ, **job_opts): super().setup(system, environ, **job_opts) with pytest.warns(ReframeDeprecationWarning): @rfm.simple_test class TestDeprecatedCompileOnly(rfm.CompileOnlyRegressionTest): - # Should raise a warning def setup(self, partition, environ, **job_opts): super().setup(system, environ, **job_opts) with pytest.warns(ReframeDeprecationWarning): @rfm.simple_test class TestDeprecatedCompileOnlyDerived(TestDeprecatedCompileOnly): - # Should raise a warning 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): - # The test should not raise a deprecation warning even though - # it overrides __init__ def __init__(self): pass @@ -115,7 +110,6 @@ class TestExtended(rfm.RegressionTest, special=True): def __init__(self): pass - # Should not raise a warning def setup(self, partition, environ, **job_opts): super().setup(system, environ, **job_opts) @@ -125,11 +119,9 @@ class TestExtendedRunOnly(rfm.RunOnlyRegressionTest, def __init__(self): pass - # Should not raise a warning def setup(self, partition, environ, **job_opts): super().setup(system, environ, **job_opts) - # Should not raise a warning def run(self): super().run() @@ -139,11 +131,9 @@ class TestExtendedCompileOnly(rfm.CompileOnlyRegressionTest, def __init__(self): pass - # Should not raise a warning def setup(self, partition, environ, **job_opts): super().setup(system, environ, **job_opts) - # Should not raise a warning def run(self): super().run() @@ -156,11 +146,9 @@ class TestExtendedDerived(TestExtended): def __init__(self): pass - # Should raise a warning def setup(self, partition, environ, **job_opts): super().setup(system, environ, **job_opts) - # Should raise a warning def run(self): super().run() @@ -178,6 +166,5 @@ def my_new_final(seld): with pytest.warns(ReframeDeprecationWarning): @rfm.simple_test class TestFinalDerived(TestFinal): - # Should raise a warning def my_new_final(self, a, b): pass From 078a76de8a64612f6ea79750338461ae94764087 Mon Sep 17 00:00:00 2001 From: Eirini Koutsaniti Date: Tue, 14 Apr 2020 14:15:25 +0200 Subject: [PATCH 11/13] Rename extended test to special test --- unittests/test_loader.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/unittests/test_loader.py b/unittests/test_loader.py index 69faf463a4..1864befdef 100644 --- a/unittests/test_loader.py +++ b/unittests/test_loader.py @@ -74,7 +74,7 @@ def test_load_bad_init(self): 'unittests/resources/checks_unlisted/bad_init_check.py') assert 0 == len(tests) - def test_extended_test(self): + def test_special_test(self): with pytest.warns(ReframeDeprecationWarning): @rfm.simple_test class TestDeprecated(rfm.RegressionTest): @@ -106,7 +106,7 @@ def __init__(self): pass @rfm.simple_test - class TestExtended(rfm.RegressionTest, special=True): + class TestSpecial(rfm.RegressionTest, special=True): def __init__(self): pass @@ -114,7 +114,7 @@ def setup(self, partition, environ, **job_opts): super().setup(system, environ, **job_opts) @rfm.simple_test - class TestExtendedRunOnly(rfm.RunOnlyRegressionTest, + class TestSpecialRunOnly(rfm.RunOnlyRegressionTest, special=True): def __init__(self): pass @@ -126,7 +126,7 @@ def run(self): super().run() @rfm.simple_test - class TestExtendedCompileOnly(rfm.CompileOnlyRegressionTest, + class TestSpecialCompileOnly(rfm.CompileOnlyRegressionTest, special=True): def __init__(self): pass @@ -142,7 +142,7 @@ def run(self): with pytest.warns(ReframeDeprecationWarning) as warnings: @rfm.simple_test - class TestExtendedDerived(TestExtended): + class TestSpecialDerived(TestSpecial): def __init__(self): pass From e6a95b01f4014f67c9619082132aab0957e09c57 Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Tue, 14 Apr 2020 17:57:00 +0200 Subject: [PATCH 12/13] Document the deprecation of pipeline stage overrides --- docs/index.rst | 3 +- docs/migration_2_to_3.rst | 78 ++++++++++++++++++++++++++++++++++++ docs/tutorial.rst | 10 +++-- reframe/core/pipeline.py | 83 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 170 insertions(+), 4 deletions(-) create mode 100644 docs/migration_2_to_3.rst diff --git a/docs/index.rst b/docs/index.rst index 5969b7dcf7..d348901eaf 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -46,7 +46,7 @@ Publications .. toctree:: - :caption: Table of Contents: + :caption: Table of Contents :hidden: Getting Started @@ -58,6 +58,7 @@ Publications Understanding The Mechanism Of Sanity Functions Running ReFrame Use cases + Migrating to ReFrame 3 About ReFrame Reference Guide Sanity Functions Reference diff --git a/docs/migration_2_to_3.rst b/docs/migration_2_to_3.rst new file mode 100644 index 0000000000..c14488bb0a --- /dev/null +++ b/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. + diff --git a/docs/tutorial.rst b/docs/tutorial.rst index 19bc4c7005..eb1770c962 100644 --- a/docs/tutorial.rst +++ b/docs/tutorial.rst @@ -460,12 +460,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 ` 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() ` method is deprecated. + Please refer to the `Migrate to ReFrame 3 `__ 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 diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 2e388dc454..cd7b413465 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -1030,6 +1030,15 @@ def setup(self, partition, environ, **job_opts): When overriding this method users should always pass through ``job_opts`` to the base class method. :raises reframe.core.exceptions.ReframeError: In case of errors. + + .. warning:: + + You may not override this method directly unless you are in special + test. See `here + `__ for + more details. + + .. versionchanged:: 3.0 ''' self._current_partition = partition self._current_environ = environ @@ -1058,6 +1067,15 @@ def compile(self): '''The compilation phase of the regression test pipeline. :raises reframe.core.exceptions.ReframeError: In case of errors. + + .. warning:: + + You may not override this method directly unless you are in special + test. See `here + `__ for + more details. + + .. versionchanged:: 3.0 ''' if not self._current_environ: raise PipelineError('no programming environment set') @@ -1147,6 +1165,15 @@ def compile_wait(self): '''Wait for compilation phase to finish. .. versionadded:: 2.13 + + .. warning:: + + You may not override this method directly unless you are in special + test. See `here + `__ for + more details. + + .. versionchanged:: 3.0 ''' self._build_job.wait() self.logger.debug('compilation finished') @@ -1162,6 +1189,15 @@ def run(self): This call is non-blocking. It simply submits the job associated with this test and returns. + + .. warning:: + + You may not override this method directly unless you are in special + test. See `here + `__ for + more details. + + .. versionchanged:: 3.0 ''' if not self.current_system or not self._current_partition: raise PipelineError('no system or system partition is set') @@ -1255,6 +1291,15 @@ def poll(self): If no job descriptor is yet associated with this test, :class:`True` is returned. :raises reframe.core.exceptions.ReframeError: In case of errors. + + .. warning:: + + You may not override this method directly unless you are in special + test. See `here + `__ for + more details. + + .. versionchanged:: 3.0 ''' if not self._job: return True @@ -1267,6 +1312,15 @@ def wait(self): '''Wait for this test to finish. :raises reframe.core.exceptions.ReframeError: In case of errors. + + .. warning:: + + You may not override this method directly unless you are in special + test. See `here + `__ for + more details. + + .. versionchanged:: 3.0 ''' self._job.wait() self.logger.debug('spawned job finished') @@ -1285,10 +1339,20 @@ def performance(self): if self.strict_check: raise + @final def check_sanity(self): '''The sanity checking phase of the regression test pipeline. :raises reframe.core.exceptions.SanityError: If the sanity check fails. + + .. warning:: + + You may not override this method directly unless you are in special + test. See `here + `__ for + more details. + + .. versionchanged:: 3.0 ''' if self.sanity_patterns is None: raise SanityError('sanity_patterns not set') @@ -1298,11 +1362,21 @@ def check_sanity(self): if not success: raise SanityError() + @final def check_performance(self): '''The performance checking phase of the regression test pipeline. :raises reframe.core.exceptions.SanityError: If the performance check fails. + + .. warning:: + + You may not override this method directly unless you are in special + test. See `here + `__ for + more details. + + .. versionchanged:: 3.0 ''' if self.perf_patterns is None: return @@ -1412,6 +1486,15 @@ def cleanup(self, remove_files=False): :arg remove_files: If :class:`True`, the stage directory associated with this test will be removed. + + .. warning:: + + You may not override this method directly unless you are in special + test. See `here + `__ for + more details. + + .. versionchanged:: 3.0 ''' aliased = os.path.samefile(self._stagedir, self._outputdir) if aliased: From 3dd8a87f81a643bc5e2d7339f3c77e34cb13e232 Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Tue, 14 Apr 2020 18:13:49 +0200 Subject: [PATCH 13/13] Fix PEP8 issues --- unittests/test_loader.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/unittests/test_loader.py b/unittests/test_loader.py index 1864befdef..b8e63fea44 100644 --- a/unittests/test_loader.py +++ b/unittests/test_loader.py @@ -115,7 +115,7 @@ def setup(self, partition, environ, **job_opts): @rfm.simple_test class TestSpecialRunOnly(rfm.RunOnlyRegressionTest, - special=True): + special=True): def __init__(self): pass @@ -127,7 +127,7 @@ def run(self): @rfm.simple_test class TestSpecialCompileOnly(rfm.CompileOnlyRegressionTest, - special=True): + special=True): def __init__(self): pass