Merge pull request #1934 from vkarak/feat/deprecate-parameterized_test

[feat] Deprecate the use of the `@parameterized_test` decorator

Author: Vasileios Karakasis

reframe/core/decorators.py

@@ -20,6 +20,7 @@
 import traceback
 
 import reframe.utility.osext as osext
+import reframe.core.warnings as warn
 from reframe.core.exceptions import (ReframeSyntaxError,
                                      SkipTestError,
                                      user_frame)
@@ -60,10 +61,12 @@ def _instantiate_all():
                 getlogger().warning(f'skipping test {cls.__name__!r}: {e}')
             except Exception:
                 frame = user_frame(*sys.exc_info())
+                filename = frame.filename if frame else 'n/a'
+                lineno = frame.lineno if frame else 'n/a'
                 getlogger().warning(
                     f"skipping test {cls.__name__!r} due to errors: "
                     f"use `-v' for more information\n"
-                    f"    FILE: {frame.filename}:{frame.lineno}"
+                    f"    FILE: {filename}:{lineno}"
                 )
                 getlogger().verbose(traceback.format_exc())
 
@@ -121,7 +124,20 @@ def parameterized_test(*inst):
    .. note::
       This decorator does not instantiate any test.  It only registers them.
       The actual instantiation happens during the loading phase of the test.
+
+   .. deprecated:: 3.6.0
+
+      Please use the :func:`~reframe.core.pipeline.RegressionTest.parameter`
+      built-in instead.
+
     '''
+
+    warn.user_deprecation_warning(
+        'the @parameterized_test decorator is deprecated; '
+        'please use the parameter() built-in instead',
+        from_version='3.6.0'
+    )
+
     def _do_register(cls):
         _validate_test(cls)
         if not cls.param_space.is_empty():

reframe/frontend/loader.py

@@ -126,8 +126,7 @@ def load_from_module(self, module):
         if hasattr(module, '_get_checks'):
             getlogger().warning(
                 f'{module.__file__}: _get_checks() is no more supported '
-                f'in test files: please use @reframe.simple_test or '
-                f'@reframe.parameterized_test decorators'
+                f'in test files: please use @reframe.simple_test decorator'
             )
 
         if not hasattr(module, '_rfm_gettests'):
@@ -167,42 +166,48 @@ def load_from_module(self, module):
         getlogger().debug(f'  > Loaded {len(ret)} test(s)')
         return ret
 
-    def load_from_file(self, filename, **check_args):
+    def load_from_file(self, filename, force=False):
         if not self._validate_source(filename):
             return []
 
-        return self.load_from_module(util.import_module_from_file(filename))
+        return self.load_from_module(
+            util.import_module_from_file(filename, force)
+        )
 
-    def load_from_dir(self, dirname, recurse=False):
+    def load_from_dir(self, dirname, recurse=False, force=False):
         checks = []
         for entry in os.scandir(dirname):
             if recurse and entry.is_dir():
                 checks.extend(
-                    self.load_from_dir(entry.path, recurse)
+                    self.load_from_dir(entry.path, recurse, force)
                 )
 
             if (entry.name.startswith('.') or
                 not entry.name.endswith('.py') or
                 not entry.is_file()):
                 continue
 
-            checks.extend(self.load_from_file(entry.path))
+            checks += self.load_from_file(entry.path, force)
 
         return checks
 
-    def load_all(self):
+    def load_all(self, force=False):
         '''Load all checks in self._load_path.
 
-        If a prefix exists, it will be prepended to each path.'''
+        If a prefix exists, it will be prepended to each path.
+
+        :arg force: Force reloading of test files.
+        :returns: The list of loaded tests.
+        '''
         checks = []
         for d in self._load_path:
             getlogger().debug(f'Looking for tests in {d!r}')
             if not os.path.exists(d):
                 continue
 
             if os.path.isdir(d):
-                checks.extend(self.load_from_dir(d, self._recurse))
+                checks += self.load_from_dir(d, self._recurse, force)
             else:
-                checks.extend(self.load_from_file(d))
+                checks += self.load_from_file(d, force)
 
         return checks

reframe/utility/__init__.py

@@ -67,10 +67,11 @@ def _do_import_module_from_file(filename, module_name=None):
     return module
 
 
-def import_module_from_file(filename):
+def import_module_from_file(filename, force=False):
     '''Import module from file.
 
     :arg filename: The path to the filename of a Python module.
+    :arg force: Force reload of module in case it is already loaded.
     :returns: The loaded Python module.
     '''
 
@@ -94,6 +95,9 @@ def import_module_from_file(filename):
     if match:
         module_name = _get_module_name(match['rel_filename'])
 
+    if force:
+        sys.modules.pop(module_name, None)
+
     return importlib.import_module(module_name)
 
 

unittests/resources/checks_unlisted/deps_simple.py

@@ -18,12 +18,12 @@ def __init__(self):
         self.sanity_patterns = sn.assert_found(self.name, self.stdout)
 
 
-@rfm.parameterized_test(*([kind] for kind in ['default', 'fully',
-                                              'by_part', 'by_case',
-                                              'custom', 'any', 'all',
-                                              'nodeps']))
+@rfm.simple_test
 class Test1(rfm.RunOnlyRegressionTest):
-    def __init__(self, kind):
+    kind = parameter(['default', 'fully', 'by_part', 'by_case',
+                      'custom', 'any', 'all', 'nodeps'])
+
+    def __init__(self):
         def custom_deps(src, dst):
             return (
                 src[0] == 'p0' and
@@ -48,7 +48,7 @@ def custom_deps(src, dst):
         self.executable = 'echo'
         self.executable_opts = [self.name]
         self.sanity_patterns = sn.assert_found(self.name, self.stdout)
-        if kind == 'default':
+        if self.kind == 'default':
             self.depends_on('Test0')
         else:
-            self.depends_on('Test0', kindspec[kind])
+            self.depends_on('Test0', kindspec[self.kind])

unittests/resources/checks_unlisted/good.py

@@ -327,53 +327,55 @@ def __init__(self):
             self.executable_opts = [self.name]
             self.sanity_patterns = sn.assert_found(self.name, self.stdout)
 
-    @rfm.parameterized_test(*([kind] for kind in ['fully', 'by_case',
-                                                  'exact']))
     class Test1_deprecated(rfm.RunOnlyRegressionTest):
-        def __init__(self, kind):
-            kindspec = {
-                'fully': rfm.DEPEND_FULLY,
-                'by_case': rfm.DEPEND_BY_ENV,
-                'exact': rfm.DEPEND_EXACT,
-            }
+        kind = parameter([rfm.DEPEND_FULLY,
+                          rfm.DEPEND_BY_ENV,
+                          rfm.DEPEND_EXACT])
+
+        def __init__(self):
             self.valid_systems = ['sys0:p0', 'sys0:p1']
             self.valid_prog_environs = ['e0', 'e1']
             self.executable = 'echo'
             self.executable_opts = [self.name]
-            if kindspec[kind] == rfm.DEPEND_EXACT:
-                self.depends_on('Test0', kindspec[kind],
+            if self.kind == rfm.DEPEND_EXACT:
+                self.depends_on('Test0', self.kind,
                                 {'e0': ['e0', 'e1'], 'e1': ['e1']})
             else:
-                self.depends_on('Test0', kindspec[kind])
-
-    with pytest.warns(ReframeDeprecationWarning):
-        t1 = Test1_deprecated('fully')
-        assert(t1._userdeps == [('Test0', udeps.by_part)])
-
-    with pytest.warns(ReframeDeprecationWarning):
-        t1 = Test1_deprecated('by_case')
-        assert(t1._userdeps == [('Test0', udeps.by_case)])
-
-    with pytest.warns(ReframeDeprecationWarning):
-        t1 = Test1_deprecated('exact')
-        how = t1._userdeps[0][1]
-        t0_cases = [(p, e) for p in ['p0', 'p1']
-                    for e in ['e0', 'e1']]
-        t1_cases = [(p, e) for p in ['p0', 'p1']
-                    for e in ['e0', 'e1']]
-        deps = {(t0, t1) for t0 in t0_cases
-                for t1 in t1_cases if how(t0, t1)}
-        assert deps == {
-            (t0, t1) for t0 in t0_cases
-            for t1 in t1_cases
-            if ((t0[0] == t1[0] and t0[1] == 'e0') or
-                (t0[0] == t1[0] and t0[1] == 'e1' and t1[1] == 'e1'))
-        }
-        assert len(deps) == 6
+                self.depends_on('Test0', self.kind)
+
+        # We will do our assertions in a post-init hook
+
+        @rfm.run_after('init')
+        def assert_deps(self):
+            if self.kind == rfm.DEPEND_FULLY:
+                assert self._userdeps == [('Test0', udeps.by_part)]
+            elif self.kind == rfm.DEPEND_BY_ENV:
+                assert self._userdeps == [('Test0', udeps.by_case)]
+            else:
+                how = self._userdeps[0][1]
+                t0_cases = [(p, e) for p in ['p0', 'p1']
+                            for e in ['e0', 'e1']]
+                t1_cases = [(p, e) for p in ['p0', 'p1']
+                            for e in ['e0', 'e1']]
+                deps = {(t0, t1) for t0 in t0_cases
+                        for t1 in t1_cases if how(t0, t1)}
+                assert deps == {
+                    (t0, t1) for t0 in t0_cases
+                    for t1 in t1_cases
+                    if ((t0[0] == t1[0] and t0[1] == 'e0') or
+                        (t0[0] == t1[0] and t0[1] == 'e1' and t1[1] == 'e1'))
+                }
+                assert len(deps) == 6
+
+    with pytest.warns(ReframeDeprecationWarning) as warnings:
+        for _ in Test1_deprecated.param_space:
+            Test1_deprecated(_rfm_use_params=True)
+
+    assert len(warnings) == 3
 
 
 def test_build_deps(loader, default_exec_ctx):
-    checks = loader.load_all()
+    checks = loader.load_all(force=True)
     cases = executors.generate_testcases(checks)
 
     # Test calling getdep() before having built the graph

unittests/test_dependencies.py

@@ -48,13 +48,6 @@ def test_load_all(loader_with_path):
     assert 12 == len(checks)
 
 
-def test_load_new_syntax(loader):
-    checks = loader.load_from_file(
-        'unittests/resources/checks_unlisted/good.py'
-    )
-    assert 13 == len(checks)
-
-
 def test_conflicted_checks(loader_with_path):
     loader_with_path._ignore_conflicts = False
     with pytest.raises(NameConflictError):

unittests/test_loader.py

@@ -164,6 +164,9 @@ class MyTest(ExtendParams):
         assert test.P2 is not None
 
 
+@pytest.mark.filterwarnings(
+    'ignore::reframe.core.warnings.ReframeDeprecationWarning'
+)
 def test_parameterized_test_is_incompatible():
     with pytest.raises(ValueError):
         @rfm.parameterized_test(['var'])

unittests/test_parameters.py

@@ -927,26 +927,6 @@ def __init__(self, a, b):
     assert 'test_name_compileonly_test.<locals>.MyTest_1_2' == test.name
 
 
-def test_registration_of_tests():
-    import unittests.resources.checks_unlisted.good as mod
-
-    checks = mod._rfm_gettests()
-    assert 13 == len(checks)
-    assert [mod.MyBaseTest(0, 0),
-            mod.MyBaseTest(0, 1),
-            mod.MyBaseTest(1, 0),
-            mod.MyBaseTest(1, 1),
-            mod.MyBaseTest(2, 0),
-            mod.MyBaseTest(2, 1),
-            mod.AnotherBaseTest(0, 0),
-            mod.AnotherBaseTest(0, 1),
-            mod.AnotherBaseTest(1, 0),
-            mod.AnotherBaseTest(1, 1),
-            mod.AnotherBaseTest(2, 0),
-            mod.AnotherBaseTest(2, 1),
-            mod.MyBaseTest(10, 20)] == checks
-
-
 def test_trap_job_errors_without_sanity_patterns(local_exec_ctx):
     rt.runtime().site_config.add_sticky_option('general/trap_job_errors', True)