Merge pull request reframe-hpc#2574 from vkarak/feat/default-naming-s…

…cheme

[feat] Drop the old test naming scheme

docs/config_reference.rst

@@ -1340,21 +1340,6 @@ General Configuration
    The command-line option sets the configuration option to ``false``.
 
 
-.. js:attribute:: .general[].compact_test_names
-
-   :required: No
-   :default: ``false``
-
-   Use a compact test naming scheme.
-   When set to ``true``, the test parameter values will not be encoded into the test name.
-   Instead, the several test variants are differentiated by including the unique variant number into the test name.
-
-   .. warning::
-      The default value will be changed to ``true`` in version 4.0.0.
-
-   .. versionadded:: 3.9.0
-
-
 .. js:attribute:: .general[].compress_report
 
    :required: No

docs/manpage.rst

@@ -872,8 +872,8 @@ Test Naming Scheme
 
 .. versionadded:: 3.10.0
 
-This section describes the new test naming scheme which will replace the current one in ReFrame 4.0.
-It can be enabled by setting the :envvar:`RFM_COMPACT_TEST_NAMES` environment variable.
+This section describes the test naming scheme.
+This scheme has superseded the old one in ReFrame 4.0.
 
 Each ReFrame test is assigned a unique name, which will be used internally by the framework to reference the test.
 Any test-specific path component will use that name, too.
@@ -976,9 +976,6 @@ This could lead to very large and hard to read names when a test defined multipl
 Very large test names meant also very large path names which could also lead to problems and random failures.
 Fixtures followed a similar naming pattern making them hard to debug.
 
-The old naming scheme is still the default for parameterized tests (but not for fixtures) and will remain so until ReFrame 4.0, in order to ensure backward compatibility.
-However, users are advised to enable the new naming scheme by setting the :envvar:`RFM_COMPACT_TEST_NAMES` environment variable.
-
 
 Environment
 -----------
@@ -1107,21 +1104,6 @@ Here is an alphabetical list of the environment variables recognized by ReFrame:
       ================================== ==================
 
 
-.. envvar:: RFM_COMPACT_TEST_NAMES
-
-   Enable the new test naming scheme.
-
-   .. table::
-      :align: left
-
-      ================================== ==================
-      Associated command line option     N/A
-      Associated configuration parameter :js:attr:`compact_test_names` general configuration parameter
-      ================================== ==================
-
-   .. versionadded:: 3.9.0
-
-
 .. envvar:: RFM_COMPRESS_REPORT
 
    Compress the generated run report file.

reframe/core/decorators.py

@@ -10,7 +10,6 @@
 __all__ = ['simple_test']
 
 
-import collections
 import inspect
 import sys
 import traceback
@@ -42,7 +41,6 @@ class TestRegistry:
 
     def __init__(self):
         self._tests = dict()
-        self._skip_tests = set()
 
     @classmethod
     def create(cls, test, *args, **kwargs):
@@ -54,11 +52,6 @@ def add(self, test, *args, **kwargs):
         self._tests.setdefault(test, [])
         self._tests[test].append((args, kwargs))
 
-    # FIXME: To drop with the required_version decorator
-    def skip(self, test):
-        '''Add a test to the skip set.'''
-        self._skip_tests.add(test)
-
     @time_function
     def instantiate_all(self, reset_sysenv=0):
         '''Instantiate all the registered tests.
@@ -75,9 +68,6 @@ def instantiate_all(self, reset_sysenv=0):
 
         leaf_tests = []
         for test, variants in self._tests.items():
-            if test in self._skip_tests:
-                continue
-
             for args, kwargs in variants:
                 try:
                     kwargs['reset_sysenv'] = reset_sysenv
@@ -138,54 +128,6 @@ def _register_test(cls, *args, **kwargs):
         mod._rfm_test_registry.add(cls, *args, **kwargs)
 
 
-def _register_parameterized_test(cls, args=None):
-    '''Register the test.
-
-    Register the test with _rfm_use_params=True. This additional argument flags
-    this case to consume the parameter space. Otherwise, the regression test
-    parameters would simply be initialized to None.
-    '''
-    def _instantiate(cls, args):
-        if isinstance(args, collections.abc.Sequence):
-            return cls(*args)
-        elif isinstance(args, collections.abc.Mapping):
-            return cls(**args)
-        elif args is None:
-            return cls()
-
-    def _instantiate_all():
-        ret = []
-        for cls, args in mod.__rfm_test_registry:
-            try:
-                if cls in mod.__rfm_skip_tests:
-                    continue
-            except AttributeError:
-                mod.__rfm_skip_tests = set()
-
-            try:
-                ret.append(_instantiate(cls, args))
-            except SkipTestError as e:
-                getlogger().warning(f'skipping test {cls.__qualname__!r}: {e}')
-            except Exception:
-                exc_info = sys.exc_info()
-                getlogger().warning(
-                    f"skipping test {cls.__qualname__!r}: {what(*exc_info)} "
-                    f"(rerun with '-v' for more information)"
-                )
-                getlogger().verbose(traceback.format_exc())
-
-        return ret
-
-    mod = inspect.getmodule(cls)
-    if not hasattr(mod, '_rfm_gettests'):
-        mod._rfm_gettests = _instantiate_all
-
-    try:
-        mod.__rfm_test_registry.append((cls, args))
-    except AttributeError:
-        mod.__rfm_test_registry = [(cls, args)]
-
-
 def _validate_test(cls):
     if not issubclass(cls, RegressionTest):
         raise ReframeSyntaxError('the decorated class must be a '

reframe/core/fixtures.py

@@ -111,11 +111,6 @@ def __init__(self):
             p.fullname: {e.name for e in p.environs} for p in sys_part
         }
 
-        # Compact naming switch
-        self._hash = runtime.runtime().get_option(
-            'general/0/compact_test_names'
-        )
-
         # Store the system name for name-mangling purposes
         self._sys_name = runtime.runtime().system.name
 
@@ -177,10 +172,7 @@ def add(self, fixture, variant_num, parent_test):
                 (f'%{k}={utils.toalphanum(str(v))}' for k, v
                  in sorted(variables.items()))
             )
-            if self._hash:
-                vname = '_' + sha256(vname.encode('utf-8')).hexdigest()[:8]
-
-            fname += vname
+            fname += '_' + sha256(vname.encode('utf-8')).hexdigest()[:8]
 
         # Select only the valid partitions
         try:

reframe/core/logging.py

@@ -580,10 +580,6 @@ def _update_check_extras(self):
             self.extra[f'check_{extra_name}'] = val
 
         # Add special extras
-
-        # FIXME: As soon as `name` becomes a read-only property in 4.0, the
-        # following assignment will not be needed.
-        self.extra['check_name'] = self.extra['check_unique_name']
         self.extra['check_info'] = self.check.info()
         self.extra['check_job_completion_time'] = _format_time_rfc3339(
             time.localtime(self.extra['check_job_completion_time_unix']),

reframe/core/meta.py

@@ -20,7 +20,6 @@
 import reframe.utility as utils
 
 from reframe.core.exceptions import ReframeSyntaxError
-from reframe.core.runtime import runtime
 
 
 class RegressionTestMeta(type):
@@ -799,20 +798,9 @@ def variant_name(cls, variant_num=None):
         if variant_num is None:
             return name
 
-        if runtime().get_option('general/0/compact_test_names'):
-            if cls.num_variants > 1:
-                width = utils.count_digits(cls.num_variants)
-                name += f'_{variant_num:0{width}}'
-        else:
-            pid, fid = cls._map_variant_num(variant_num)
-
-            # Append the parameters to the name
-            if cls.param_space.params:
-                name += '_' + '_'.join(utils.toalphanum(str(v))
-                                       for v in cls.param_space[pid].values())
-
-            if len(cls.fixture_space) > 1:
-                name += f'_{fid}'
+        if cls.num_variants > 1:
+            width = utils.count_digits(cls.num_variants)
+            name += f'_{variant_num:0{width}}'
 
         return name
 

reframe/core/pipeline.py

@@ -914,8 +914,7 @@ def __new__(cls, *args, **kwargs):
         # Prepare initialization of test defaults (variables and parameters are
         # injected after __new__ has returned, so we schedule this function
         # call as a pre-init hook).
-        obj.__deferred_rfm_init = obj.__rfm_init__(*args,
-                                                   prefix=prefix, **kwargs)
+        obj.__deferred_rfm_init = obj.__rfm_init__(prefix)
 
         # Build pipeline hook registry and add the pre-init hook
         cls._rfm_pipeline_hooks = cls._process_hook_registry()
@@ -959,17 +958,10 @@ def __init_subclass__(cls, *, special=False, pin_prefix=False,
             )
 
     @deferrable
-    def __rfm_init__(self, *args, prefix=None, **kwargs):
+    def __rfm_init__(self, prefix=None):
         if not self.is_fixture() and not hasattr(self, '_rfm_unique_name'):
             self._rfm_unique_name = type(self).variant_name(self.variant_num)
 
-            # Add the parameters from the parameterized_test decorator.
-            if args or kwargs:
-                arg_names = map(lambda x: util.toalphanum(str(x)),
-                                itertools.chain(args, kwargs.values()))
-                self._rfm_unique_name += '_' + '_'.join(arg_names)
-                self._rfm_old_style_params = True
-
         # Pass if descr is a required variable.
         if not hasattr(self, 'descr'):
             self.descr = self.display_name
@@ -1092,6 +1084,7 @@ def unique_name(self):
         '''
         return self._rfm_unique_name
 
+    @loggable
     @property
     def name(self):
         '''The name of the test.
@@ -1136,9 +1129,6 @@ def _format_params(cls, info, prefix=' %'):
 
             return name
 
-        if hasattr(self, '_rfm_old_style_params'):
-            return self.unique_name
-
         if hasattr(self, '_rfm_display_name'):
             return self._rfm_display_name
 

reframe/frontend/cli.py

@@ -595,13 +595,6 @@ def main():
         action='store_true',
         help='Graylog server address'
     )
-    argparser.add_argument(
-        dest='compact_test_names',
-        envvar='RFM_COMPACT_TEST_NAMES',
-        configvar='general/compact_test_names',
-        action='store_true',
-        help='Use a compact test naming scheme'
-    )
     argparser.add_argument(
         dest='dump_pipeline_progress',
         envvar='RFM_DUMP_PIPELINE_PROGRESS',

reframe/frontend/filters.py

@@ -23,14 +23,11 @@ def _fn(case):
         # Match pattern, but remove spaces from the `display_name`
         display_name = case.check.display_name.replace(' ', '')
         rt = runtime()
-        if not rt.get_option('general/0/compact_test_names'):
-            return regex.match(case.check.unique_name)
+        if '@' in patt:
+            # Do an exact match on the unique name
+            return patt.replace('@', '_') == case.check.unique_name
         else:
-            if '@' in patt:
-                # Do an exact match on the unique name
-                return patt.replace('@', '_') == case.check.unique_name
-            else:
-                return regex.match(display_name)
+            return regex.match(display_name)
 
     return _fn
 
@@ -44,15 +41,13 @@ def _fn(case):
 
 def have_any_name(names):
     rt = runtime()
-    has_compact_names = rt.get_option('general/0/compact_test_names')
     exact_matches = []
     regex_matches = []
     for n in names:
-        if has_compact_names and '@' in n:
+        if '@' in n:
             test, _, variant = n.rpartition('@')
             if variant.isdigit():
                 exact_matches.append((test, int(variant)))
-
         else:
             regex_matches.append(n)
 
@@ -70,10 +65,7 @@ def _fn(case):
 
         display_name = case.check.display_name.replace(' ', '')
         if regex:
-            if has_compact_names:
-                return regex.match(display_name)
-            else:
-                return regex.match(case.check.unique_name)
+            return regex.match(display_name)
 
         return False
 

reframe/frontend/loader.py

@@ -166,53 +166,18 @@ def load_from_module(self, module):
         This method tries to load the test registry from a given module and
         instantiates all the tests in the registry. The instantiated checks
         are validated before return.
-
-        For legacy reasons, a module might have the additional legacy registry
-        `_rfm_gettests`, which is a method that instantiates all the tests
-        registered with the deprecated `parameterized_test` decorator.
         '''
-        from reframe.core.pipeline import RegressionTest
-
-        # FIXME: Remove the legacy_registry after dropping parameterized_test
         registry = getattr(module, '_rfm_test_registry', None)
-        legacy_registry = getattr(module, '_rfm_gettests', None)
-        if not any((registry, legacy_registry)):
-            getlogger().debug('No tests registered')
-            return []
-
         self._set_defaults(registry)
         reset_sysenv = self._skip_prgenv_check << 1 | self._skip_system_check
         if registry:
             candidate_tests = registry.instantiate_all(reset_sysenv)
         else:
             candidate_tests = []
 
-        legacy_tests = legacy_registry() if legacy_registry else []
-        if self._external_vars and legacy_tests:
-            getlogger().warning(
-                "variables of tests using the deprecated "
-                "'@parameterized_test' decorator cannot be set externally; "
-                "please use the 'parameter' builtin in your tests"
-            )
-
-        # Reset valid_systems and valid_prog_environs in all legacy tests
-        if reset_sysenv:
-            for t in legacy_tests:
-                if self._skip_system_check:
-                    t.valid_systems = ['*']
-
-                if self._skip_prgenv_check:
-                    t.valid_prog_environs = ['*']
-
-        # Merge tests
-        candidate_tests += legacy_tests
-
         # Post-instantiation validation of the candidate tests
         final_tests = []
         for c in candidate_tests:
-            if not isinstance(c, RegressionTest):
-                continue
-
             if not self._validate_check(c):
                 continue