Merge pull request #1819 from jjotero/feature/improve-error-handling

[feat] Improve access to the namespaces of both parameter and variable built-ins

Author: Vasileios Karakasis

reframe/core/meta.py

@@ -17,16 +17,40 @@
 class RegressionTestMeta(type):
 
     class MetaNamespace(namespaces.LocalNamespace):
-        '''Custom namespace to control the cls attribute assignment.'''
+        '''Custom namespace to control the cls attribute assignment.
+
+        Regular Python class attributes can be overriden by either
+        parameters or variables respecting the order of execution.
+        A variable or a parameter may not be declared more than once in the
+        same class body. Overriding a variable with a parameter or the other
+        way around has an undefined behaviour. A variable's value may be
+        updated multiple times within the same class body. A parameter's
+        value may not be updated more than once within the same class body.
+        '''
+
         def __setitem__(self, key, value):
-            if isinstance(value, variables.VarDirective):
+            if isinstance(value, variables.TestVar):
                 # Insert the attribute in the variable namespace
                 self['_rfm_local_var_space'][key] = value
+
+                # Override the regular class attribute (if present)
+                self._namespace.pop(key, None)
+
             elif isinstance(value, parameters.TestParam):
                 # Insert the attribute in the parameter namespace
                 self['_rfm_local_param_space'][key] = value
+
+                # Override the regular class attribute (if present)
+                self._namespace.pop(key, None)
+
+            elif key in self['_rfm_local_param_space']:
+                raise ValueError(
+                    f'cannot override parameter {key!r}'
+                )
             else:
-                super().__setitem__(key, value)
+                # Insert the items manually to overide the namespace clash
+                # check from the base namespace.
+                self._namespace[key] = value
 
         def __getitem__(self, key):
             '''Expose and control access to the local namespaces.
@@ -40,22 +64,31 @@ def __getitem__(self, key):
             except KeyError as err:
                 try:
                     # Handle variable access
-                    var = self['_rfm_local_var_space'][key]
-                    if var.is_defined():
-                        return var.default_value
-                    else:
-                        raise ValueError(
-                            f'variable {key!r} is not assigned a value'
-                        )
+                    v = self['_rfm_local_var_space'][key]
+                    v.__set_name__(self, key)
+                    return v
 
                 except KeyError:
                     # Handle parameter access
                     if key in self['_rfm_local_param_space']:
                         raise ValueError(
                             'accessing a test parameter from the class '
                             'body is disallowed'
-                        )
+                        ) from None
                     else:
+                        # As the last resource, look if key is a variable in
+                        # any of the base classes. If so, make its value
+                        # available in the current class' namespace.
+                        for b in self['_rfm_bases']:
+                            if key in b._rfm_var_space:
+                                v = b._rfm_var_space[key]
+                                v.__set_name__(self, key)
+
+                                # Store a deep-copy of the variable's
+                                # value and return.
+                                self._namespace[key] = v.default_value
+                                return self._namespace[key]
+
                         # If 'key' is neither a variable nor a parameter,
                         # raise the exception from the base __getitem__.
                         raise err from None
@@ -64,6 +97,11 @@ def __getitem__(self, key):
     def __prepare__(metacls, name, bases, **kwargs):
         namespace = super().__prepare__(name, bases, **kwargs)
 
+        # Keep reference to the bases inside the namespace
+        namespace['_rfm_bases'] = [
+            b for b in bases if hasattr(b, '_rfm_var_space')
+        ]
+
         # Regression test parameter space defined at the class level
         local_param_space = namespaces.LocalNamespace()
         namespace['_rfm_local_param_space'] = local_param_space
@@ -78,7 +116,7 @@ def __prepare__(metacls, name, bases, **kwargs):
 
         # Directives to add/modify a regression test variable
         namespace['variable'] = variables.TestVar
-        namespace['required'] = variables.UndefineVar()
+        namespace['required'] = variables.Undefined
         return metacls.MetaNamespace(namespace)
 
     def __new__(metacls, name, bases, namespace, **kwargs):
@@ -172,7 +210,7 @@ def __call__(cls, *args, **kwargs):
         obj.__init__(*args, **kwargs)
         return obj
 
-    def __getattribute__(cls, name):
+    def __getattr__(cls, name):
         ''' Attribute lookup method for the MetaNamespace.
 
         This metaclass implements a custom namespace, where built-in `variable`
@@ -183,15 +221,14 @@ def __getattribute__(cls, name):
         requested class attribute.
         '''
         try:
-            return super().__getattribute__(name)
-        except AttributeError:
+            return cls._rfm_var_space.vars[name]
+        except KeyError:
             try:
-                return cls._rfm_local_var_space[name]
+                return cls._rfm_param_space.params[name]
             except KeyError:
-                try:
-                    return cls._rfm_local_param_space[name]
-                except KeyError:
-                    return super().__getattr__(name)
+                raise AttributeError(
+                    f'class {cls.__qualname__!r} has no attribute {name!r}'
+                ) from None
 
     @property
     def param_space(cls):

reframe/core/namespaces.py

@@ -63,8 +63,11 @@ def _raise_namespace_clash(self, name):
             f'{name!r} is already present in the current namespace'
         )
 
+    def clear(self):
+        self._namespace = {}
+
 
-class Namespace(metaclass=abc.ABCMeta):
+class Namespace(LocalNamespace, metaclass=abc.ABCMeta):
     '''Namespace of a regression test.
 
     The final namespace may be built by inheriting namespaces from
@@ -111,7 +114,7 @@ def namespace_name(self):
         '''
 
     def __init__(self, target_cls=None, illegal_names=None):
-        self._namespace = {}
+        super().__init__()
         if target_cls:
             # Assert the Namespace can be built for the target_cls
             self.assert_target_cls(target_cls)
@@ -173,5 +176,7 @@ def inject(self, obj, objtype=None):
            ``obj``.
         '''
 
-    def items(self):
-        return self._namespace.items()
+    def __setitem__(self, key, value):
+        raise ValueError(
+            f'cannot set item {key!r} into a {type(self).__qualname__} object'
+        )

reframe/core/parameters.py

@@ -124,7 +124,8 @@ def join(self, other, cls):
     def extend(self, cls):
         '''Extend the parameter space with the local parameter space.'''
 
-        for name, p in cls._rfm_local_param_space.items():
+        local_param_space = getattr(cls, self.local_namespace_name)
+        for name, p in local_param_space.items():
             self.params[name] = (
                 p.filter_params(self.params.get(name, ())) + p.values
             )
@@ -139,6 +140,9 @@ def extend(self, cls):
                     f'parameter type'
                 )
 
+        # Clear the local param space
+        local_param_space.clear()
+
     def inject(self, obj, cls=None, use_params=False):
         '''Insert the params in the regression test.
 

reframe/core/pipeline.py

@@ -149,19 +149,17 @@ class RegressionMixin(metaclass=RegressionTestMeta):
     .. versionadded:: 3.4.2
     '''
 
-    def __getattribute__(self, name):
-        try:
-            return super().__getattribute__(name)
-        except AttributeError:
-            # Intercept the AttributeError if the name corresponds to a
-            # required variable.
-            if (name in self._rfm_var_space.vars and
-                not self._rfm_var_space.vars[name].is_defined()):
-                raise AttributeError(
-                    f'required variable {name!r} has not been set'
-                ) from None
-            else:
-                super().__getattr__(name)
+    def __getattr__(self, name):
+        ''' Intercept the AttributeError if the name is a required variable.'''
+        if (name in self._rfm_var_space and
+            not self._rfm_var_space[name].is_defined()):
+            raise AttributeError(
+                f'required variable {name!r} has not been set'
+            ) from None
+        else:
+            raise AttributeError(
+                f'{type(self).__qualname__} object has no attribute {name!r}'
+            )
 
 
 class RegressionTest(RegressionMixin, jsonext.JSONSerializable):