Merge pull request reframe-hpc#3115 from vkarak/feat/vars-implicit-conversions

[feat] Enable default value type checking and implicit conversions at the variable declaration level

Author: vkarak

docs/regression_test_api.rst

@@ -41,6 +41,13 @@ These are called *builtins* because they are directly available for use inside t
 However, almost all of these builtins are also available from the :obj:`reframe.core.builtins` module.
 The use of this module is required only when creating new tests programmatically using the :func:`~reframe.core.meta.make_test` function.
 
+.. versionchanged:: 3.7.0
+   Expose :func:`@deferrable <reframe.core.builtins.deferrable>` as a builtin.
+
+.. versionchanged:: 3.11.0
+   Builtins are now available also through the :obj:`reframe.core.builtins` module.
+
+
 .. py:method:: reframe.core.pipeline.RegressionMixin.bind(func, name=None)
 
    Bind a free function to a regression test.
@@ -80,13 +87,6 @@ The use of this module is required only when creating new tests programmatically
 .. autofunction:: reframe.core.builtins.variable
 
 
-.. versionchanged:: 3.7.0
-   Expose :func:`@deferrable <reframe.core.builtins.deferrable>` as a builtin.
-
-.. versionchanged:: 3.11.0
-   Builtins are now available also through the :obj:`reframe.core.builtins` module.
-
-
 .. _pipeline-hooks:
 
 --------------

reframe/core/buildsystems.py

@@ -981,4 +981,4 @@ def __set__(self, obj, value):
             except KeyError:
                 raise ValueError(f'unknown build system: {value}') from None
 
-        super().__set__(obj, value)
+        return super().__set__(obj, value)

reframe/core/containers.py

@@ -287,4 +287,4 @@ def __set__(self, obj, value):
         if isinstance(value, str):
             value = ContainerPlatform.create(value)
 
-        super().__set__(obj, value)
+        return super().__set__(obj, value)

reframe/core/decorators.py

@@ -123,7 +123,6 @@ def instantiate_all(self, reset_sysenv=0, external_vars=None):
         # candidate tests; the leaf tests are consumed at the end of the
         # traversal and all instantiated tests (including fixtures) are stored
         # in `final_tests`.
-        unset_vars = {}
         final_tests = []
         fixture_registry = FixtureRegistry()
         while leaf_tests:

reframe/core/fields.py

@@ -56,7 +56,16 @@ def __get__(self, obj, objtype):
                                  (objtype.__name__, self._name)) from None
 
     def __set__(self, obj, value):
-        obj.__dict__[self._name] = remove_convertible(value)
+        # NOTE: We extend the Python data model by returning the value that
+        # would be otherwise written to the field, if ``obj`` is :obj:`None`.
+        # Subclasses must make sure to return to the caller the value returned
+        # from ``super().__set__(...)``.
+
+        value = remove_convertible(value)
+        if obj is None:
+            return value
+
+        obj.__dict__[self._name] = value
 
 
 class TypedField(Field):
@@ -79,7 +88,7 @@ def _check_type(self, value):
         if not any(isinstance(value, t) for t in self._types):
             typedescr = '|'.join(t.__name__ for t in self._types)
             raise TypeError(
-                "failed to set field '%s': '%s' is not of type '%s'" %
+                "failed to set variable '%s': '%s' is not of type '%s'" %
                 (self._name, value, typedescr))
 
     def __set__(self, obj, value):
@@ -100,18 +109,17 @@ def __set__(self, obj, value):
                 except TypeError:
                     continue
                 else:
-                    super().__set__(obj, value)
-                    return
+                    return super().__set__(obj, value)
 
             # Conversion failed
             typenames = [t.__name__ for t in self._types]
             raise TypeError(
-                f'failed to set field {self._name!r}: '
+                f'failed to set variable {self._name!r}: '
                 f'could not convert to any of the supported types: '
                 f'{typenames}'
             )
         else:
-            super().__set__(obj, value)
+            return super().__set__(obj, value)
 
 
 class ConstantField(Field):
@@ -152,7 +160,7 @@ def __set__(self, obj, value):
         if not isinstance(value, ScopedDict):
             value = ScopedDict(value) if value is not None else value
 
-        Field.__set__(self, obj, value)
+        return Field.__set__(self, obj, value)
 
 
 class DeprecatedField(Field):
@@ -187,7 +195,7 @@ def __set__(self, obj, value):
         if self._op & DeprecatedField.OP_SET:
             user_deprecation_warning(self._message, self._from_version)
 
-        self._target_field.__set__(obj, value)
+        return self._target_field.__set__(obj, value)
 
     def __get__(self, obj, objtype):
         if self._op & DeprecatedField.OP_GET:

reframe/core/variables.py

@@ -343,8 +343,10 @@ def _default_value(self):
     def _default_value(self, value):
         if self.is_alias():
             self._target._default_value = value
-        else:
+        elif value is Undefined:
             self._p_default_value = value
+        else:
+            self._p_default_value = self._p_field.__set__(None, value)
 
     @property
     def default_value(self):
@@ -386,6 +388,18 @@ def reset_target(self, new_target):
 
     def __set_name__(self, owner, name):
         self._name = name
+        self._p_field.__set_name__(owner, name)
+
+        # Type check and convert the variable's value if defined
+        if self.is_defined():
+            if isinstance(self._p_default_value, TestVar):
+                # Treat shadow variables
+                value = self._p_default_value._p_default_value
+            else:
+                value = self._p_default_value
+
+            with suppress_deprecations():
+                self._p_default_value = self._p_field.__set__(None, value)
 
     def __setattr__(self, name, value):
         '''Set any additional variable attribute into the default value.'''
@@ -918,12 +932,16 @@ def inject(self, obj, cls):
 
     def _inject(self, obj, cls):
         for name, var in self.items():
+            # Replace the variable with its descriptor
             setattr(cls, name, var.field)
             getattr(cls, name).__set_name__(obj, name)
 
             # If the var is defined, set its value
             if var.is_defined():
-                setattr(obj, name, var.default_value)
+                # Variable's value is already validated and converted,
+                # so we bypass completely the descriptor logic by not calling
+                # `setattr()`
+                obj.__dict__[name] = var.default_value
 
             # Track the variables that have been injected.
             self._injected_vars.add(name)

unittests/test_pipeline.py

@@ -1607,11 +1607,9 @@ def test_reference_deferrable(dummy_perftest):
     with pytest.raises(TypeError):
         dummy_perftest.reference = {'*': {'value1': (sn.defer(1), -0.1, -0.1)}}
 
-    class T(rfm.RegressionTest):
-        reference = {'*': {'value1': (sn.defer(1), -0.1, -0.1)}}
-
     with pytest.raises(TypeError):
-        T()
+        class T(rfm.RegressionTest):
+            reference = {'*': {'value1': (sn.defer(1), -0.1, -0.1)}}
 
 
 def test_performance_invalid_value(dummytest, sanity_file,

unittests/test_variables.py

@@ -146,11 +146,9 @@ class MyTest(OneVarTest):
 
 
 def test_var_type(OneVarTest):
-    class MyTest(OneVarTest):
-        foo = 'bananas'
-
     with pytest.raises(TypeError):
-        MyTest()
+        class MyTest(OneVarTest):
+            foo = 'bananas'
 
 
 def test_require_var(OneVarTest):
@@ -358,7 +356,7 @@ class A(rfm.RegressionTest):
         v = variable(int, value=4)
         assert (v / 3) == 4/3
         assert (3 / v) == 3/4
-        v /= 2
+        v //= 2
         assert v == 2