Merge pull request #2647 from vkarak/feat/variable-aliases

[feat] Introduce variable aliases

Author: vkarak

docs/conf.py

@@ -38,6 +38,16 @@
 import reframe                           # noqa: F401, F403
 import reframe.utility.osext as osext    # noqa: F401, F403
 
+# Sphinx erroneously uses `repr()` to display values and not the
+# human-readable `str()`. For this reason we define the following variable in
+# the `reframe` module so that objects can redirect to their `str()` method
+# from `repr()` if they are invoked in the context of Sphinx when building the
+# documentation. See discussion and the related workaround here:
+#
+# https://github.com/sphinx-doc/sphinx/issues/3857
+
+reframe.__build_docs__ = True
+
 # -- General configuration ------------------------------------------------
 
 # If your documentation needs a minimal Sphinx version, state it here.

reframe/core/meta.py

@@ -140,11 +140,9 @@ def __getitem__(self, key):
                         # available in the current class' namespace.
                         for b in self['_rfm_bases']:
                             if key in b._rfm_var_space:
-                                # Store a deep-copy of the variable's
-                                # value and return.
-                                v = b._rfm_var_space[key].default_value
+                                v = variables.ShadowVar(b._rfm_var_space[key])
                                 self._namespace[key] = v
-                                return self._namespace[key]
+                                return v
 
                         # If 'key' is neither a variable nor a parameter,
                         # raise the exception from the base __getitem__.

reframe/core/variables.py

@@ -158,13 +158,24 @@ class MyRequiredTest(HelloTest):
     from :class:`EchoBaseTest` to throw an error indicating that the variable
     ``what`` has not been set.
 
+    Finally, variables may alias each other. If a variable is an alias of
+    another one it behaves in the exact same way as its target. If a change is
+    made to the target variable, this is reflected to the alias and vice
+    versa. However, alias variables are independently loggable: an alias may
+    be logged but not its target and vice versa. Aliased variables are useful
+    when you want to rename a variable and you want to keep the old one for
+    compatibility reasons.
+
     :param `types`: the supported types for the variable.
     :param value: the default value assigned to the variable. If no value is
         provided, the variable is set as ``required``.
     :param field: the field validator to be used for this variable. If no
         field argument is provided, it defaults to
         :attr:`reframe.core.fields.TypedField`. The provided field validator
         by this argument must derive from :attr:`reframe.core.fields.Field`.
+    :param alias: the target variable if this variable is an alias. This must
+        refer to an already declared variable and neither default value nor a
+        field can be specified for an alias variable.
     :param loggable: Mark this variable as loggable. If :obj:`True`, this
         variable will become a log record attribute under the name
         ``check_NAME``, where ``NAME`` is the name of the variable.
@@ -175,37 +186,71 @@ class MyRequiredTest(HelloTest):
     .. versionadded:: 3.10.2
        The ``loggable`` argument is added.
 
+    .. versionadded:: 4.0.0
+       Alias variable are introduced.
+
     '''
 
-    __slots__ = ('_default_value', '_field', '_loggable', '_name')
+    # NOTE: We can't use truly private fields in `__slots__`, because
+    # `__setattr__()` will be called with their mangled name and we cannot
+    # match them in the `__slots__` without making implementation-defined
+    # assumptions about the mangled name. So we just add the `_p_` prefix for
+    # to denote the "private" fields.
+
+    __slots__ = ('_p_default_value', '_p_field',
+                 '_loggable', '_name', '_target')
+
+    __mutable_props = ('_default_value',)
 
     def __init__(self, *args, **kwargs):
+        alias = kwargs.pop('alias', None)
+        if alias is not None and 'field' in kwargs:
+            raise ValueError(f"'field' cannot be set for an alias variable")
+
+        if alias is not None and 'value' in kwargs:
+            raise ValueError('alias variables do not accept default values')
+
+        if alias is not None and not isinstance(alias, TestVar):
+            raise TypeError(f"'alias' must refer to a variable; "
+                            f"found {type(alias).__name__!r}")
+
         field_type = kwargs.pop('field', fields.TypedField)
-        self._default_value = kwargs.pop('value', Undefined)
-        self._loggable = kwargs.pop('loggable', False)
+        if alias is not None:
+            self._p_default_value = alias._default_value
+        else:
+            self._p_default_value = kwargs.pop('value', Undefined)
 
+        self._loggable = kwargs.pop('loggable', False)
         if not issubclass(field_type, fields.Field):
             raise TypeError(
                 f'field {field_type!r} is not derived from '
                 f'{fields.Field.__qualname__}'
             )
 
-        self._field = field_type(*args, **kwargs)
+        if alias is not None:
+            self._p_field = alias._field
+        else:
+            self._p_field = field_type(*args, **kwargs)
+
+        self._target = alias
 
     @classmethod
     def create_deprecated(cls, var, message,
                           kind=DEPRECATE_RDWR, from_version='0.0.0'):
         ret = TestVar.__new__(TestVar)
-        ret._field = fields.DeprecatedField(var.field, message,
-                                            kind, from_version)
-        ret._default_value = var._default_value
+        ret._p_field = fields.DeprecatedField(var.field, message,
+                                              kind, from_version)
+        ret._p_default_value = var._default_value
         ret._loggable = var._loggable
+        ret._target = var._target
         return ret
 
-    def _check_deprecation(self, kind):
-        if isinstance(self.field, fields.DeprecatedField):
-            if self.field.op & kind:
-                user_deprecation_warning(self.field.message)
+    def _warn_deprecation(self, kind):
+        if self.is_deprecated() and self.field.op & kind:
+            user_deprecation_warning(self.field.message)
+
+    def is_deprecated(self):
+        return isinstance(self._p_field, fields.DeprecatedField)
 
     def is_loggable(self):
         return self._loggable
@@ -217,29 +262,41 @@ def undefine(self):
         self._default_value = Undefined
 
     def define(self, value):
-        if value != self._default_value:
-            # We only issue a deprecation warning if the write attempt changes
-            # the value. This is a workaround to the fact that if a variable
-            # defined in parent classes is accessed by the current class, then
-            # the definition of the variable is "copied" in the class body as
-            # an assignment (see `MetaNamespace.__getitem__()`). The
-            # `VarSpace.extend()` method then checks all local class body
-            # assignments and if they refer to a variable (inherited or not),
-            # they call `define()` on it. So, practically, in this case, the
-            # `_default_value` is set redundantly once per class in the
-            # hierarchy.
-            self._check_deprecation(DEPRECATE_WR)
-
+        self._warn_deprecation(DEPRECATE_WR)
         self._default_value = value
 
+    @property
+    def _default_value(self):
+        if self._target:
+            return self._target._default_value
+        else:
+            return self._p_default_value
+
+    @_default_value.setter
+    def _default_value(self, value):
+        if self._target:
+            self._target._default_value = value
+        else:
+            self._p_default_value = value
+
     @property
     def default_value(self):
         # Variables must be returned by-value to prevent an instance from
         # modifying the class variable space.
         self._check_is_defined()
-        self._check_deprecation(DEPRECATE_RD)
+        self._warn_deprecation(DEPRECATE_RD)
         return copy.deepcopy(self._default_value)
 
+    @property
+    def _field(self):
+        if self.is_deprecated():
+            return self._p_field
+
+        if self._target:
+            return self._target._field
+        else:
+            return self._p_field
+
     @property
     def field(self):
         return self._field
@@ -253,14 +310,14 @@ def __set_name__(self, owner, name):
 
     def __setattr__(self, name, value):
         '''Set any additional variable attribute into the default value.'''
-        if name in self.__slots__:
+        if name in self.__slots__ or name in self.__mutable_props:
             super().__setattr__(name, value)
         else:
             setattr(self._default_value, name, value)
 
     def __getattr__(self, name):
         '''Attribute lookup into the variable's value.'''
-        def_val = self.__getattribute__('_default_value')
+        def_val = self.__getattribute__('_p_default_value')
 
         # NOTE: This if below is necessary to avoid breaking the deepcopy
         # of instances of this class. Without it, a deepcopy of instances of
@@ -284,12 +341,26 @@ def _check_is_defined(self):
                 f'variable {self._name!r} is not assigned a value'
             )
 
-    def __repr__(self):
+    def __str__(self):
         self._check_is_defined()
-        return repr(self._default_value)
+        return str(self._default_value)
 
-    def __str__(self):
-        return self.__repr__()
+    def __repr__(self):
+        import reframe
+        if hasattr(reframe, '__build_docs__'):
+            return str(self)
+
+        try:
+            name = self.name
+        except AttributeError:
+            name = '<undef>'
+
+        if self.is_defined():
+            value = self._default_value
+        else:
+            value = '<undef>'
+
+        return f'TestVar(name={name!r}, value={value!r})'
 
     def __bytes__(self):
         self._check_is_defined()
@@ -593,6 +664,30 @@ def __ceil__(self):
         return math.ceil(self._default_value)
 
 
+class ShadowVar(TestVar):
+    '''A shadow instance of another variable.
+
+    This is essentially a fully-fledged shallow copy of another variable. It
+    is used during the construction of the class namespace to bring in scope a
+    requested variable that is defined in a base class (see
+    `MetaNamespace.__getitem__()`)
+
+    We could not simply create a reference of the original variable in the
+    current namespace, because we need a mechanism to differentiate the
+    lowered variable from any redefinition, which is illegal.
+
+    Also, we don't need a deep copy, since the shadow variable will replace
+    the original variable in the newly constructed `VarSpace`.
+
+    '''
+
+    def __init__(self, other):
+        for name in self.__slots__:
+            setattr(self, name, getattr(other, name))
+
+        self._warn_deprecation(DEPRECATE_RD)
+
+
 class VarSpace(namespaces.Namespace):
     '''Variable space of a regression test.
 
@@ -645,8 +740,7 @@ def extend(self, cls):
         is disallowed.
         '''
         local_varspace = getattr(cls, self.local_namespace_name, False)
-        while local_varspace:
-            key, var = local_varspace.popitem()
+        for key, var in local_varspace.items():
             if isinstance(var, TestVar):
                 # Disable redeclaring a variable
                 if key in self.vars:
@@ -657,13 +751,19 @@ def extend(self, cls):
                 # Add a new var
                 self.vars[key] = var
 
+        local_varspace.clear()
+
         # If any previously declared variable was defined in the class body
         # by directly assigning it a value, retrieve this value from the class
         # namespace and update it into the variable space.
         _assigned_vars = set()
         for key, value in cls.__dict__.items():
             if key in self.vars:
-                self.vars[key].define(value)
+                if isinstance(value, ShadowVar):
+                    self.vars[key] = value
+                else:
+                    self.vars[key].define(value)
+
                 _assigned_vars.add(key)
             elif value is Undefined:
                 # Cannot be set as Undefined if not a variable

unittests/test_logging.py

@@ -29,6 +29,7 @@ def fake_check():
     class _FakeCheck(rfm.RegressionTest):
         param = parameter(range(3), loggable=True, fmt=lambda x: 10*x)
         custom = variable(str, value='hello extras', loggable=True)
+        custom2 = variable(alias=custom)
         custom_list = variable(list,
                                value=['custom', 3.0, ['hello', 'world']],
                                loggable=True)
@@ -161,13 +162,13 @@ def test_logger_levels(logfile, logger_with_check):
 
 def test_logger_loggable_attributes(logfile, logger_with_check):
     formatter = rlog.RFC3339Formatter(
-        '%(check_custom)s|%(check_custom_list)s|'
+        '%(check_custom)s|%(check_custom2)s|%(check_custom_list)s|'
         '%(check_foo)s|%(check_custom_dict)s|%(check_param)s|%(check_x)s'
     )
     logger_with_check.logger.handlers[0].setFormatter(formatter)
     logger_with_check.info('xxx')
     assert _pattern_in_logfile(
-        r'hello extras\|custom,3.0,\["hello", "world"\]\|null\|'
+        r'hello extras\|null\|custom,3.0,\["hello", "world"\]\|null\|'
         r'{"a": 1, "b": 2}\|10\|null', logfile
     )