Address PR comments + style fixes + dead code elimination

reframe/core/decorators.py

@@ -8,7 +8,6 @@
 #
 
 __all__ = ['simple_test']
-_TREAT_WARNINGS_AS_ERRORS = False
 
 import inspect
 import sys
@@ -170,22 +169,10 @@ def _validate_test(cls):
                                  'subclass of RegressionTest')
 
     if (cls.is_abstract()):
-        params = cls.param_space.params
-        undefined_params = []
-        for param in params:
-            if params[param].is_abstract():
-                undefined_params.append(param)
-        # Raise detailed warning
-        if _TREAT_WARNINGS_AS_ERRORS:
-            raise ValueError(
-                f'skipping test {cls.__qualname__!r}: ' +
-                f'test has the following undefined parameters: ' +
-                ', '.join(undefined_params)
-            )
         getlogger().warning(
             f'skipping test {cls.__qualname__!r}: ' +
-            f'test has the following undefined parameters: ' +
-            ', '.join(undefined_params)
+            'the following parameters are undefined: ' +
+            ', '.join(cls.param_space.undefined_params())
         )
 
     conditions = [VersionValidator(v) for v in cls._rfm_required_version]

reframe/core/fixtures.py

@@ -325,12 +325,6 @@ def uninst_tests(self):
         '''Get the uninstantiated tests of this registry'''
         return self._registry.keys()
 
-    def _filter_valid_partitions(self, candidate_parts):
-        return [p for p in candidate_parts if p in self._env_by_part]
-
-    def _filter_valid_environs(self, part, candidate_environs):
-        return [e for e in cadidate_environs if e in self._env_by_part[part]]
-
     def _is_registry(self, other):
         if not isinstance(other, FixtureRegistry):
             raise TypeError('other is not a FixtureRegistry')
@@ -775,14 +769,9 @@ def __init__(self, cls, *, scope='test', action='fork', variants='all',
 
         # Check that the fixture class is not an abstract test.
         if cls.is_abstract():
-            params = cls.param_space.params
-            undefined_params = []
-            for param in params:
-                if params[param].is_abstract():
-                    undefined_params.append(param)
             raise ValueError(
-                f'class {cls.__qualname__!r} has undefined parameters: ' +
-                ', '.join(undefined_params)
+                f'fixture {cls.__qualname__!r} has undefined parameters: ' +
+                ', '.join(cls.param_space.undefined_params())
             )
 
         # Validate the scope

reframe/core/parameters.py

@@ -198,7 +198,7 @@ def update(self, other):
                 self.values = tuple(filt_vals) + self.values
             except TypeError:
                 raise ReframeSyntaxError(
-                    f"'filter_param' must return an iterable"
+                    "'filter_param' must return an iterable"
                 ) from None
 
     def is_abstract(self):
@@ -307,7 +307,7 @@ def inject(self, obj, cls=None, params_index=None):
             try:
                 # Get the parameter values for the specified variant
                 param_values = self.__param_combinations[params_index]
-            except IndexError as no_params:
+            except IndexError:
                 raise RuntimeError(
                     f'parameter space index out of range for '
                     f'{obj.__class__.__qualname__}'
@@ -333,6 +333,11 @@ def defines(self, name):
         '''
         return name in self.params and not self.params[name].is_abstract()
 
+    def undefined_params(self):
+        '''Return a list of all undefined parameters.'''
+        return [name for name, param in self.params.items()
+                if param.is_abstract()]
+
     def __iter__(self):
         '''Create a generator object to iterate over the parameter space
 

unittests/test_fixtures.py

@@ -71,12 +71,10 @@ def test_abstract_fixture():
     class Foo(rfm.RegressionTest):
         p = parameter()
 
-    with pytest.raises(ValueError) as exc_info:
+    with pytest.raises(ValueError):
         class MyTest(rfm.RegressionMixin):
             f = fixture(Foo)
 
-    assert "p" in str(exc_info.value)
-
 
 def test_fixture_variants():
     '''Fixtures must have at least one valid variant.'''

unittests/test_parameters.py

@@ -49,22 +49,13 @@ class MyTest(TwoParams):
 
 def test_abstract_param():
     class MyTest(Abstract):
-        pass
+        # Add another abstract parameter
+        P2 = parameter()
 
     assert MyTest.param_space['P0'] == ()
     assert MyTest.param_space['P1'] == ('b',)
-
-
-def test_abstract_param_warning(monkeypatch):
-    monkeypatch.setattr(decorators, '_TREAT_WARNINGS_AS_ERRORS', True)
-
-    class MyTest(Abstract):
-        pass
-
-    with pytest.raises(ValueError) as execinfo:
-        decorators._validate_test(MyTest)
-
-    assert "P0" in str(execinfo.value)
+    assert MyTest.param_space['P2'] == ()
+    assert MyTest.param_space.undefined_params() == ['P0', 'P2']
 
 
 def test_param_override():