[feat] Allow combining explicit system partitions and features/extras in valid_systems

reframe/core/pipeline.py

@@ -92,7 +92,17 @@ def launch_command(self, stagedir):
 _VALID_ENV_SYNTAX = rf'^({_NW}|{_FKV}(\s+{_FKV})*)$'
 
 _S = rf'({_NW}(:{_NW})?)'   # system/partition
-_VALID_SYS_SYNTAX = rf'^({_S}|{_FKV}(\s+{_FKV})*)$'
+_SE = rf'({_N}(:{_N})?)'    # system/partition (exact match; no wildcards)
+
+# Valid syntax variants
+#
+# _SV1: system/partition combinations w/ wildcards
+# _SV2: features and extras only
+# _SV3: exact system/partition w/ features and extras
+_SV1 = rf'{_S}'
+_SV2 = rf'{_FKV}(\s+{_FKV})*'
+_SV3 = rf'({_FKV}\s+)*{_SE}(\s+{_FKV})*'
+_VALID_SYS_SYNTAX = rf'^({_SV1}|{_SV2}|{_SV3})$'
 
 
 _PIPELINE_STAGES = (
@@ -681,6 +691,22 @@ def pipeline_hooks(cls):
     #:
     #:    valid_systems = [r'+feat1 +feat2 %foo=1']
     #:
+    #: Features and key/value pairs can also be combined with an explicit
+    #: system partition combination in a single :attr:`valid_systems` entry. In
+    #: this case, the resulting constraint means that the requested system
+    #: partition combination is valid only if it also satisfies the feature and
+    #: key/value pair specification. In the following example, the ``sys:part``
+    #: system will only be selected if it also defines the ``foo`` feature:
+    #:
+    #: .. code-block:: python
+    #:
+    #:    valid_systems = [r'sys:part +foo']
+    #:
+    #: This case is generally useful in cases of parameterization of tests
+    #: based on configuration settings as well as in cases where additional
+    #: constraints can only be determined during the test initialization after
+    #: its parameterization.
+    #:
     #: Any partition/environment extra or
     #: :ref:`partition resource <scheduler-resources>` can be specified as a
     #: feature constraint without having to explicitly state this in the
@@ -720,6 +746,10 @@ def pipeline_hooks(cls):
     #:
     #:  .. versionchanged:: 3.11.0
     #:     Extend syntax to support features and key/value pairs.
+    #:
+    #:   .. versionchanged:: 4.10
+    #:      Support for combining an explicit system partition combination with
+    #:      features and extras.
     valid_systems = variable(typ.List[typ.Str[_VALID_SYS_SYNTAX]])
 
     #: A detailed description of the test.

reframe/core/runtime.py

@@ -289,50 +289,61 @@ def is_env_loaded(environ):
 
 
 def _is_valid_part(part, valid_systems):
+    # Get sysname and partname for the partition being checked and construct
+    # all system:partition patterns that would match the sysname and partname
+    # being checked
+    sysname, partname = part.fullname.split(':')
+    syspart_matches = ['*', '*:*', sysname, f'{sysname}:*', f'*:{partname}',
+                       f'{part.fullname}']
+
+    # If any of the specs in valid_systems matches, this is a valid partition
     for spec in valid_systems:
-        if spec[0] not in ('+', '-', '%'):
-            # This is the standard case
-            sysname, partname = part.fullname.split(':')
-            valid_matches = ['*', '*:*', sysname, f'{sysname}:*',
-                             f'*:{partname}', f'{part.fullname}']
-            if spec in valid_matches:
-                return True
-        else:
-            plus_feats = []
-            minus_feats = []
-            props = {}
-            for subspec in spec.split(' '):
-                if subspec.startswith('+'):
-                    plus_feats.append(subspec[1:])
-                elif subspec.startswith('-'):
-                    minus_feats.append(subspec[1:])
-                elif subspec.startswith('%'):
-                    key, val = subspec[1:].split('=')
-                    props[key] = val
-
-            have_plus_feats = all(
-                (ft in part.features or
-                 ft in part.resources or ft in part.extras)
-                for ft in plus_feats
-            )
-            have_minus_feats = any(
-                (ft in part.features or
-                 ft in part.resources or ft in part.extras)
-                for ft in minus_feats
-            )
-            try:
-                have_props = True
-                for k, v in props.items():
-                    extra_value = part.extras[k]
-                    extra_type  = type(extra_value)
-                    if extra_value != extra_type(v):
-                        have_props = False
-                        break
-            except (KeyError, ValueError):
-                have_props = False
-
-            if have_plus_feats and not have_minus_feats and have_props:
-                return True
+        plus_feats = []
+        minus_feats = []
+        props = {}
+        valid_match = True
+        for subspec in spec.split(' '):
+            if subspec.startswith('+'):
+                plus_feats.append(subspec[1:])
+            elif subspec.startswith('-'):
+                minus_feats.append(subspec[1:])
+            elif subspec.startswith('%'):
+                key, val = subspec[1:].split('=')
+                props[key] = val
+            else:
+                # If there is a system:partition specified, make sure it
+                # matches one of the items in syspart_matches
+                valid_match = True if subspec in syspart_matches else False
+
+        have_plus_feats = all(
+            (ft in part.features or
+             ft in part.resources or ft in part.extras)
+            for ft in plus_feats
+        )
+        have_minus_feats = any(
+            (ft in part.features or
+             ft in part.resources or ft in part.extras)
+            for ft in minus_feats
+        )
+        try:
+            have_props = True
+            for k, v in props.items():
+                extra_value = part.extras[k]
+                extra_type  = type(extra_value)
+                if extra_value != extra_type(v):
+                    have_props = False
+                    break
+        except (KeyError, ValueError):
+            have_props = False
+
+        # If the partition has all the plus features, none of the minus
+        # all of the properties and the system:partition spec (if any)
+        # matched, this partition is valid
+        if (have_plus_feats and
+            not have_minus_feats and
+            have_props and
+            valid_match):
+            return True
 
     return False
 

unittests/test_pipeline.py

@@ -362,6 +362,10 @@ def test_valid_systems_syntax(hellotest):
     hellotest.valid_systems = ['+x0 -y0 %z0=w0']
     hellotest.valid_systems = ['-y0 +x0 %z0=w0']
     hellotest.valid_systems = ['%z0=w0 +x0 -y0']
+    hellotest.valid_systems = ['sys:part +x0 +y0']
+    hellotest.valid_systems = ['sys:part +x0 +y0 %z0=w0']
+    hellotest.valid_systems = ['+x0 sys:part']
+    hellotest.valid_systems = ['+x0 sys:part +y0 %z0=w0']
 
     with pytest.raises(TypeError):
         hellotest.valid_systems = ['']
@@ -405,6 +409,27 @@ def test_valid_systems_syntax(hellotest):
     with pytest.raises(TypeError):
         hellotest.valid_systems = ['%']
 
+    with pytest.raises(TypeError):
+        hellotest.valid_systems = ['sys:* +foo']
+
+    with pytest.raises(TypeError):
+        hellotest.valid_systems = ['*:part +foo']
+
+    with pytest.raises(TypeError):
+        hellotest.valid_systems = ['*:* +foo']
+
+    with pytest.raises(TypeError):
+        hellotest.valid_systems = ['* +foo']
+
+    with pytest.raises(TypeError):
+        hellotest.valid_systems = ['sys0:part0 sys0:part1 +foo']
+
+    with pytest.raises(TypeError):
+        hellotest.valid_systems = ['sys0:part0 +foo sys0:part1']
+
+    with pytest.raises(TypeError):
+        hellotest.valid_systems = ['+foo sys0:part0 sys0:part1']
+
     for sym in '!@#$^&()=<>':
         with pytest.raises(TypeError):
             hellotest.valid_systems = [f'{sym}foo']