Merge pull request #3554 from vkarak/feat/reparameterize

[feat] Allow the `-P` option to redefine existing parameters

Author: vkarak

docs/manpage.rst

@@ -1062,13 +1062,14 @@ The way the tests are generated and how they interact with the test filtering op
 
 .. option:: -P, --parameterize=[TEST.]VAR=VAL0,VAL1,...
 
-   Parameterize a test on an existing variable.
+   Parameterize a test on an existing variable or parameter.
 
-   The test will behave as if the variable ``VAR`` was a parameter taking the values ``VAL0,VAL1,...``.
+   In case of variables, the test will behave as if the variable ``VAR`` was a parameter taking the values ``VAL0,VAL1,...``.
    The values will be converted based on the type of the target variable ``VAR``.
+   In case of parameters, the test will behave is the parameter had been defined with the values ``VAL0,VAL1,...``.
    The ``TEST.`` prefix will only parameterize the variable ``VAR`` of test ``TEST``.
 
-   The :option:`-P` can be specified multiple times in order to parameterize multiple variables.
+   The :option:`-P` can be specified multiple times in order to parameterize multiple variables or redefine multiple parameters.
 
    .. note::
 
@@ -1083,6 +1084,10 @@ The way the tests are generated and how they interact with the test filtering op
 
    .. versionadded:: 4.3
 
+   .. versionchanged:: 4.9
+
+      It is now possible to use the :option:`-P` option to redefine existing test parameters.
+
 .. option:: --param-values-delim=<delim>
 
    Use the given delimiter to separate the parameter values passed with :option:`--parameterize`.

reframe/core/parameters.py

@@ -137,7 +137,7 @@ class AbstractB(Bar):
 
     '''
 
-    def __init__(self, values=None, inherit_params=False,
+    def __init__(self, values=None, *, type=None, inherit_params=False,
                  filter_params=None, fmt=None, loggable=True):
         if values is None:
             values = []
@@ -175,6 +175,7 @@ def fmt(x):
 
         self.__fmt_fn = fmt
         self.__loggable = loggable
+        self.__type = type
         self.__owner = None
         self.__name = None
 
@@ -184,6 +185,10 @@ def __set_name__(self, owner, name):
     def __rfm_set_owner__(self, name):
         self.__owner = name
 
+    @property
+    def type(self):
+        return self.__type
+
     @property
     def name(self):
         return self.__name

reframe/frontend/testgenerators.py

@@ -3,12 +3,14 @@
 #
 # SPDX-License-Identifier: BSD-3-Clause
 
+from enum import IntEnum
 
 import reframe.core.builtins as builtins
 import reframe.core.runtime as runtime
 import reframe.utility as util
 
 from reframe.core.decorators import TestRegistry
+from reframe.core.exceptions import ReframeError
 from reframe.core.fields import make_convertible
 from reframe.core.logging import getlogger, time_function
 from reframe.core.meta import make_test
@@ -42,33 +44,76 @@ def getallnodes(state, jobs_cli_options=None):
     return nodes
 
 
-def _generate_tests(testcases, gen_fn):
+class _GenKind(IntEnum):
+    BY_CHECK = 1
+    BY_PARTITION = 2
+
+
+def _generate_tests(testcases, gen_fn, kind: _GenKind):
     tmp_registry = TestRegistry()
 
+    def _testcase_key(tc):
+        check, partition, _ = tc
+        if kind == _GenKind.BY_CHECK:
+            return check.unique_name
+        elif kind == _GenKind.BY_PARTITION:
+            return (check.unique_name, partition.fullname)
+
+        assert False, '[BUG] unknown _GenKind'
+
+    def _variant_key(cls, partition):
+        if kind == _GenKind.BY_CHECK:
+            return cls.__name__
+        if kind == _GenKind.BY_PARTITION:
+            return (cls.__name__, partition)
+
+        assert False, '[BUG] unknown _GenKind'
+
+    def _remove_params(params, variant_info):
+        for p in params:
+            variant_info['params'].pop(p, None)
+
     # We don't want to register the same check for every environment
     # per partition
-    check_part_combs = set()
+    known_testcases = set()
+    registered_variants = {}
     for tc in testcases:
         check, partition, _ = tc
-        candidate_comb = (check.unique_name, partition.fullname)
-        if check.is_fixture() or candidate_comb in check_part_combs:
+        tc_key = _testcase_key(tc)
+        if check.is_fixture() or tc_key in known_testcases:
             continue
 
-        check_part_combs.add(candidate_comb)
+        known_testcases.add(tc_key)
+
+        # We want to instantiate only the variants of the original test cases.
+        # For this reason, we store the original test case variant info and
+        # compare it with the newly generated one after having removed its new
+        # parameters. In case of reparameterizations, we remove the redefined
+        # parameter also from the original test case info. We then instantiate
+        # only the test cases that have a matching variant info. This
+        # technique essentially re-applies any parameter filtering that has
+        # happened previously in the CLI, e.g., with `-n MyTest%param=3`
         cls = type(check)
-        variant_info = cls.get_variant_info(
-            check.variant_num, recurse=True
-        )
+        tc_info = cls.get_variant_info(check.variant_num, recurse=True)
         nc, params = gen_fn(tc)
+
+        # Remove any redefined parameters
+        _remove_params(params, tc_info)
+
+        nc_key = _variant_key(nc, partition.fullname)
+        registered_variants.setdefault(nc_key, set())
         nc._rfm_custom_prefix = check.prefix
         for i in range(nc.num_variants):
-            # Check if this variant should be instantiated
-            vinfo = nc.get_variant_info(i, recurse=True)
-            for p in params:
-                vinfo['params'].pop(p)
-
-            if vinfo == variant_info:
+            nc_info = nc.get_variant_info(i, recurse=True)
+
+            # Remove all the parameters that we have (re)defined and compare
+            # it with the original info; we will only instantiate test that
+            # have a matching remaining info and that we have not already
+            # registered.
+            _remove_params(params, nc_info)
+            if nc_info == tc_info and i not in registered_variants[nc_key]:
                 tmp_registry.add(nc, variant_num=i)
+                registered_variants[nc_key].add(i)
 
     new_checks = tmp_registry.instantiate_all()
     return generate_testcases(new_checks)
@@ -115,7 +160,7 @@ def _rfm_set_valid_systems(obj):
             ]
         ), ['.part', '.nid']
 
-    return _generate_tests(testcases, _make_dist_test)
+    return _generate_tests(testcases, _make_dist_test, _GenKind.BY_PARTITION)
 
 
 @time_function
@@ -132,7 +177,7 @@ def _make_repeat_test(testcase):
             }
         ), ['.repeat_no']
 
-    return _generate_tests(testcases, _make_repeat_test)
+    return _generate_tests(testcases, _make_repeat_test, _GenKind.BY_CHECK)
 
 
 @time_function
@@ -145,6 +190,7 @@ def _make_parameterized_test(testcase):
 
         # Check that all the requested variables exist
         body = {}
+        methods = []
         for var, values in paramvars.items():
             var_parts = var.split('.')
             if len(var_parts) == 1:
@@ -157,24 +203,37 @@ def _make_parameterized_test(testcase):
                 getlogger().warning('cannot set a variable in a fixture')
                 continue
 
-            if var not in cls.var_space:
+            if var not in cls.var_space and var not in cls.param_space:
                 getlogger().warning(
-                    f'variable {var!r} not defined for test '
+                    f'{var!r} is neither a variable nor a parameter of test '
                     f'{check.display_name!r}; ignoring parameterization'
                 )
                 continue
 
-            body[f'.{var}'] = builtins.parameter(values, loggable=False)
-
-        def _set_vars(self):
-            for var in body.keys():
-                setattr(self, var[1:],
-                        make_convertible(getattr(self, f'{var}')))
+            if var in cls.var_space:
+                body[f'.{var}'] = builtins.parameter(values, loggable=False)
+                def _set_vars(self):
+                    for var in body.keys():
+                        setattr(self, var[1:],
+                                make_convertible(getattr(self, f'{var}')))
+
+                methods = [builtins.run_after('init')(_set_vars)]
+            elif var in cls.param_space:
+                p = cls.param_space.params[var]
+                if p.type is None:
+                    raise ReframeError(
+                        f'cannot parameterize test {cls.__name__!r}: '
+                        f'no type information associated with parameter {var!r}: '  # noqa: E501
+                        'consider defining the parameter as follows:\n'
+                        f'    {var} = parameter({list(p.values)}, type=<type>, '    # noqa: E501
+                        f'loggable={p.is_loggable()})'
+                    )
+
+                body[var] = builtins.parameter([p.type(v) for v in values], type=p.type)
+            else:
+                assert 0, f'[BUG] {var} cannot be defined as both a variable and a parameter'
 
-        return make_test(
-            cls.__name__, (cls,),
-            body=body,
-            methods=[builtins.run_after('init')(_set_vars)]
-        ), body.keys()
+        return (make_test(cls.__name__, (cls,), body=body, methods=methods),
+                body.keys())
 
-    return _generate_tests(testcases, _make_parameterized_test)
+    return _generate_tests(testcases, _make_parameterized_test, _GenKind.BY_CHECK)

unittests/test_testgenerators.py

@@ -8,6 +8,7 @@
 import reframe as rfm
 import reframe.frontend.executors as executors
 import reframe.frontend.filters as filters
+import reframe.utility.sanity as sn
 from reframe.frontend.testgenerators import (distribute_tests,
                                              parameterize_tests, repeat_tests)
 from reframe.frontend.loader import RegressionCheckLoader
@@ -74,10 +75,11 @@ def test_repeat_testcases():
 
 
 @pytest.fixture
-def hello_test_cls():
+def HelloTest():
     class _HelloTest(rfm.RunOnlyRegressionTest):
         message = variable(str, value='world')
         number = variable(int, value=1)
+        x = parameter([1, 2], type=int)
         valid_systems = ['*']
         valid_prog_environs = ['*']
         executable = 'echo'
@@ -95,14 +97,16 @@ def validate(self):
     return _HelloTest
 
 
-def test_parameterize_tests(hello_test_cls):
-    testcases = executors.generate_testcases([hello_test_cls()])
-    assert len(testcases) == 1
-
+def test_parameterize_tests(HelloTest):
+    testcases = executors.generate_testcases(
+        [HelloTest(variant_num=i) for i in range(HelloTest.num_variants)]
+    )
+    assert len(testcases) == 2
     testcases = parameterize_tests(
         testcases, {'message': ['x', 'y'],
                     '_HelloTest.number': [1, '2', 3],
+                    '_HelloTest.x': [3, 4, 5],
                     'UnknownTest.var': 3,
                     'unknown': 1}
     )
-    assert len(testcases) == 6
+    assert len(testcases) == 18