Repository: reframe-hpc/reframe

Issue:
Allow the parameterisation of a test on an existing variable from the command line
It's a common pattern if you have a test with a variable that can take multiple values to be able to parametrise the test on that variable. Currently, this cannot be done from the command line and we have to write some boilerplate code. For example,

```python
class MyTest(...):
    x = variable(int, value=0)

class MyTestWithX(MyTest):
    xx = parameter(range(10))

    @run_after('init')
    def set_x(self):
        self.x = self.xx
```

Ideally, users should be able to do `--parameterise=x:1,2,3,4`

Discussion:

Patch:
diff --git a/docs/manpage.rst b/docs/manpage.rst
index 2320c26f..9de42be8 100644
--- a/docs/manpage.rst
+++ b/docs/manpage.rst
@@ -545,6 +545,24 @@ Options controlling ReFrame execution
    .. versionchanged:: 4.1
       Options that can be specified multiple times are now combined between execution modes and the command line.
 
+.. option:: -P, --parameterize=[TEST.]VAR=VAL0,VAL1,...
+
+   Parameterize a test on an existing variable.
+
+   This option will create a new test with a parameter named ``$VAR`` with the values given in the comma-separated list ``VAL0,VAL1,...``.
+   The values will be converted based on the type of the target variable ``VAR``.
+   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.
+
+   .. note::
+
+      Conversely to the :option:`-S` option that can set a variable in an arbitrarily nested fixture,
+      the :option:`-P` option can only parameterize the leaf test:
+      it cannot be used to parameterize a fixture of the test.
+
+   .. versionadded:: 4.3
+
 .. option:: --repeat=N
 
    Repeat the selected tests ``N`` times.
diff --git a/reframe/core/meta.py b/reframe/core/meta.py
index 95c35e7b..9654a531 100644
--- a/reframe/core/meta.py
+++ b/reframe/core/meta.py
@@ -936,20 +936,18 @@ def make_test(name, bases, body, methods=None, module=None, **kwargs):
     namespace = RegressionTestMeta.__prepare__(name, bases, **kwargs)
     methods = methods or []
 
-    # Add methods to the body
+    # Update the namespace with the body elements
+    #
+    # NOTE: We do not use `update()` here so as to force the `__setitem__`
+    # logic
+    for k, v in body.items():
+        namespace[k] = v
+
+    # Add methods to the namespace
     for m in methods:
-        body[m.__name__] = m
-
-    # We update the namespace with the body of the class and we explicitly
-    # call reset on each namespace key to trigger the functionality of
-    # `__setitem__()` as if the body elements were actually being typed in the
-    # class definition
-    namespace.update(body)
-    for k in list(namespace.keys()):
-        namespace.reset(k)
+        namespace[m.__name__] = m
 
     cls = RegressionTestMeta(name, bases, namespace, **kwargs)
-
     if not module:
         # Set the test's module to be that of our callers
         caller = inspect.currentframe().f_back
diff --git a/reframe/frontend/cli.py b/reframe/frontend/cli.py
index 55dc6fb7..b3ea5af7 100644
--- a/reframe/frontend/cli.py
+++ b/reframe/frontend/cli.py
@@ -30,7 +30,8 @@ import reframe.utility.osext as osext
 import reframe.utility.typecheck as typ
 
 from reframe.frontend.testgenerators import (distribute_tests,
-                                             getallnodes, repeat_tests)
+                                             getallnodes, repeat_tests,
+                                             parameterize_tests)
 from reframe.frontend.executors.policies import (SerialExecutionPolicy,
                                                  AsynchronousExecutionPolicy)
 from reframe.frontend.executors import Runner, generate_testcases
@@ -436,6 +437,10 @@ def main():
     run_options.add_argument(
         '--mode', action='store', help='Execution mode to use'
     )
+    run_options.add_argument(
+        '-P', '--parameterize', action='append', metavar='VAR:VAL0,VAL1,...',
+        default=[], help='Parameterize a test on a set of variables'
+    )
     run_options.add_argument(
         '--repeat', action='store', metavar='N',
         help='Repeat selected tests N times'
@@ -1096,6 +1101,22 @@ def main():
                 f'{len(testcases)} remaining'
             )
 
+        if options.parameterize:
+            # Prepare parameters
+            params = {}
+            for param_spec in options.parameterize:
+                try:
+                    var, values_spec = param_spec.split('=')
+                except ValueError:
+                    raise errors.CommandLineError(
+                        f'invalid parameter spec: {param_spec}'
+                    ) from None
+                else:
+                    params[var] = values_spec.split(',')
+
+            testcases_all = parameterize_tests(testcases, params)
+            testcases = testcases_all
+
         if options.repeat is not None:
             try:
                 num_repeats = int(options.repeat)


Test patch:
diff --git a/reframe/frontend/testgenerators.py b/reframe/frontend/testgenerators.py
index 3b827898..49510536 100644
--- a/reframe/frontend/testgenerators.py
+++ b/reframe/frontend/testgenerators.py
@@ -9,6 +9,7 @@ import reframe.core.runtime as runtime
 import reframe.utility as util
 
 from reframe.core.decorators import TestRegistry
+from reframe.core.fields import make_convertible
 from reframe.core.logging import getlogger, time_function
 from reframe.core.meta import make_test
 from reframe.core.schedulers import Job
@@ -48,36 +49,7 @@ def getallnodes(state='all', jobs_cli_options=None):
     return nodes
 
 
-def _rfm_pin_run_nodes(obj):
-    nodelist = getattr(obj, '$nid')
-    if not obj.local:
-        obj.job.pin_nodes = nodelist
-
-
-def _rfm_pin_build_nodes(obj):
-    pin_nodes = getattr(obj, '$nid')
-    if not obj.local and not obj.build_locally:
-        obj.build_job.pin_nodes = pin_nodes
-
-
-def make_valid_systems_hook(systems):
-    '''Returns a function to be used as a hook that sets the
-    valid systems.
-
-    Since valid_systems change for each generated test, we need to
-    generate different post-init hooks for each one of them.
-    '''
-    def _rfm_set_valid_systems(obj):
-        obj.valid_systems = systems
-
-    return _rfm_set_valid_systems
-
-
-@time_function
-def distribute_tests(testcases, node_map):
-    '''Returns new testcases that will be parameterized to run in node of
-    their partitions based on the nodemap
-    '''
+def _generate_tests(testcases, gen_fn):
     tmp_registry = TestRegistry()
 
     # We don't want to register the same check for every environment
@@ -94,7 +66,41 @@ def distribute_tests(testcases, node_map):
         variant_info = cls.get_variant_info(
             check.variant_num, recurse=True
         )
-        nc = make_test(
+        nc, params = gen_fn(tc)
+        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:
+                tmp_registry.add(nc, variant_num=i)
+
+    new_checks = tmp_registry.instantiate_all()
+    return generate_testcases(new_checks)
+
+
+@time_function
+def distribute_tests(testcases, node_map):
+    def _rfm_pin_run_nodes(obj):
+        nodelist = getattr(obj, '$nid')
+        if not obj.local:
+            obj.job.pin_nodes = nodelist
+
+    def _rfm_pin_build_nodes(obj):
+        pin_nodes = getattr(obj, '$nid')
+        if not obj.local and not obj.build_locally:
+            obj.build_job.pin_nodes = pin_nodes
+
+    def _make_dist_test(testcase):
+        check, partition, _ = testcase
+        cls = type(check)
+
+        def _rfm_set_valid_systems(obj):
+            obj.valid_systems = [partition.fullname]
+
+        return make_test(
             f'{cls.__name__}_{partition.fullname.replace(":", "_")}',
             (cls,),
             {
@@ -108,55 +114,70 @@ def distribute_tests(testcases, node_map):
                 builtins.run_before('run')(_rfm_pin_run_nodes),
                 builtins.run_before('compile')(_rfm_pin_build_nodes),
                 # We re-set the valid system in a hook to make sure that it
-                # will not be overwriten by a parent post-init hook
-                builtins.run_after('init')(
-                    make_valid_systems_hook([partition.fullname])
-                ),
+                # will not be overwritten by a parent post-init hook
+                builtins.run_after('init')(_rfm_set_valid_systems),
             ]
-        )
-
-        # We have to set the prefix manually
-        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)
-            vinfo['params'].pop('$nid')
-            if vinfo == variant_info:
-                tmp_registry.add(nc, variant_num=i)
+        ), ['$nid']
 
-    new_checks = tmp_registry.instantiate_all()
-    return generate_testcases(new_checks)
+    return _generate_tests(testcases, _make_dist_test)
 
 
 @time_function
 def repeat_tests(testcases, num_repeats):
     '''Returns new test cases parameterized over their repetition number'''
 
-    tmp_registry = TestRegistry()
-    unique_checks = set()
-    for tc in testcases:
-        check = tc.check
-        if check.is_fixture() or check in unique_checks:
-            continue
-
-        unique_checks.add(check)
-        cls = type(check)
-        variant_info = cls.get_variant_info(
-            check.variant_num, recurse=True
-        )
-        nc = make_test(
+    def _make_repeat_test(testcase):
+        cls = type(testcase.check)
+        return make_test(
             f'{cls.__name__}', (cls,),
             {
                 '$repeat_no': builtins.parameter(range(num_repeats))
             }
-        )
-        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)
-            vinfo['params'].pop('$repeat_no')
-            if vinfo == variant_info:
-                tmp_registry.add(nc, variant_num=i)
+        ), ['$repeat_no']
 
-    new_checks = tmp_registry.instantiate_all()
-    return generate_testcases(new_checks)
+    return _generate_tests(testcases, _make_repeat_test)
+
+
+@time_function
+def parameterize_tests(testcases, paramvars):
+    '''Returns new test cases parameterized over specific variables.'''
+
+    def _make_parameterized_test(testcase):
+        check = testcase.check
+        cls = type(check)
+
+        # Check that all the requested variables exist
+        body = {}
+        for var, values in paramvars.items():
+            var_parts = var.split('.')
+            if len(var_parts) == 1:
+                var = var_parts[0]
+            elif len(var_parts) == 2:
+                var_check, var = var_parts
+                if var_check != cls.__name__:
+                    continue
+            else:
+                getlogger().warning(f'cannot set a variable in a fixture')
+                continue
+
+            if not var in cls.var_space:
+                getlogger().warning(
+                    f'variable {var!r} not defined for test '
+                    f'{check.display_name!r}; ignoring parameterization'
+                )
+                continue
+
+            body[f'${var}'] = builtins.parameter(values)
+
+        def _set_vars(self):
+            for var in body.keys():
+                setattr(self, var[1:],
+                        make_convertible(getattr(self, f'{var}')))
+
+        return make_test(
+            f'{cls.__name__}', (cls,),
+            body=body,
+            methods=[builtins.run_after('init')(_set_vars)]
+        ), body.keys()
+
+    return _generate_tests(testcases, _make_parameterized_test)
diff --git a/unittests/test_cli.py b/unittests/test_cli.py
index 22d80868..d2e9589e 100644
--- a/unittests/test_cli.py
+++ b/unittests/test_cli.py
@@ -105,6 +105,8 @@ def run_reframe(tmp_path, monkeypatch):
             argv += ['--list-tags']
         elif action == 'help':
             argv += ['-h']
+        elif action == 'describe':
+            argv += ['--describe']
 
         if perflogdir:
             argv += ['--perflogdir', perflogdir]
@@ -957,6 +959,32 @@ def test_repeat_negative(run_reframe):
     assert returncode == 1
 
 
+def test_parameterize_tests(run_reframe):
+    returncode, stdout, _ = run_reframe(
+        more_options=['-P', 'num_tasks=2,4,8', '-n', '^HelloTest'],
+        checkpath=['unittests/resources/checks/hellocheck.py'],
+        action='describe'
+    )
+    assert returncode == 0
+
+    test_descr = json.loads(stdout)
+    print(json.dumps(test_descr, indent=2))
+    num_tasks = {t['num_tasks'] for t in test_descr}
+    assert num_tasks == {2, 4, 8}
+
+
+def test_parameterize_tests_invalid_params(run_reframe):
+    returncode, stdout, stderr = run_reframe(
+        more_options=['-P', 'num_tasks', '-n', '^HelloTest'],
+        checkpath=['unittests/resources/checks/hellocheck.py'],
+        action='list'
+    )
+    assert returncode == 1
+    assert 'Traceback' not in stdout
+    assert 'Traceback' not in stderr
+    assert 'invalid parameter spec' in stdout
+
+
 def test_reruns_negative(run_reframe):
     returncode, stdout, stderr = run_reframe(
         more_options=['--reruns', '-1'],
diff --git a/unittests/test_testgenerators.py b/unittests/test_testgenerators.py
index 8c787dc0..810ddde3 100644
--- a/unittests/test_testgenerators.py
+++ b/unittests/test_testgenerators.py
@@ -5,9 +5,11 @@
 
 import pytest
 
+import reframe as rfm
 import reframe.frontend.executors as executors
 import reframe.frontend.filters as filters
-from reframe.frontend.testgenerators import (distribute_tests, repeat_tests)
+from reframe.frontend.testgenerators import (distribute_tests,
+                                             parameterize_tests, repeat_tests)
 from reframe.frontend.loader import RegressionCheckLoader
 
 
@@ -69,3 +71,38 @@ def test_repeat_testcases():
 
     testcases = repeat_tests(testcases, 10)
     assert len(testcases) == 20
+
+
+@pytest.fixture
+def hello_test_cls():
+    class _HelloTest(rfm.RunOnlyRegressionTest):
+        message = variable(str, value='world')
+        number = variable(int, value=1)
+        valid_systems = ['*']
+        valid_prog_environs = ['*']
+        executable = 'echo'
+        executable_opts = ['hello']
+
+        @run_before('run')
+        def set_message(self):
+            self.executable_opts += [self.message, str(self.number)]
+
+        @sanity_function
+        def validate(self):
+            return sn.assert_found(rf'hello {self.message} {self.number}',
+                                   self.stdout)
+
+    return _HelloTest
+
+
+def test_parameterize_tests(hello_test_cls):
+    testcases = executors.generate_testcases([hello_test_cls()])
+    assert len(testcases) == 1
+
+    testcases = parameterize_tests(
+        testcases, {'message': ['x', 'y'],
+                    '_HelloTest.number': [1, '2', 3],
+                    'UnknownTest.var': 3,
+                    'unknown': 1}
+    )
+    assert len(testcases) == 6