From 0e38c82301322d80dc1dc32735347563c7cfac93 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Tue, 12 Jan 2021 14:28:28 +0100 Subject: [PATCH 01/49] Add var directive --- reframe/core/attributes.py | 62 ++++++++++++++++++++++++++++++++++++++ reframe/core/meta.py | 8 +++++ reframe/core/variables.py | 24 +++++++++++++++ 3 files changed, 94 insertions(+) create mode 100644 reframe/core/attributes.py create mode 100644 reframe/core/variables.py diff --git a/reframe/core/attributes.py b/reframe/core/attributes.py new file mode 100644 index 0000000000..b9d391ea86 --- /dev/null +++ b/reframe/core/attributes.py @@ -0,0 +1,62 @@ +# Copyright 2016-2020 Swiss National Supercomputing Centre (CSCS/ETH Zurich) +# ReFrame Project Developers. See the top-level LICENSE file for details. +# +# SPDX-License-Identifier: BSD-3-Clause + +# +# Functionality to build extensible attribute spaces through class directives. +# + +import abc + +class LocalAttrSpace(metaclass=abc.ABCMeta): + '''Local attribute space of a regression test. + + Stores the regression test attributes defined in the test class body. + In the context of this class, a regression test attribute is an instance + of the class _TOBESET#########. This local attribute space is populated + during the test class body execution through the add_attr method, and the + different attributes are stored under the _attr member. This class + should be thought of as a temporary storage for this local attribute space, + before the full final space of the attribute is built. + + Example: In the pseudo-code below, the local parameter space of A is {P0}, + and the local parameter space of B is {P1}. However, the final parameter + space of A is still {P0}, and the final parameter space of B is {P0, P1}. + + .. code:: python + + class A(RegressionTest): + -> define parameter P0 with value X. + + class B(A): + -> define parameter P1 with value Y. + ''' + + def __init__(self): + self._attr = {} + + def __getattr__(self, name): + return getattr(self._attr, name) + + def __setitem__(self, name, value): + if name not in self._attr: + self._attr[name] = value + else: + raise ValueError( + f'attribute {name!r} already defined in this class' + ) + + @abc.abstractmethod + def add_attr(self, name, *args, **kwargs): + '''Insert a new attribute in the local attribute space.''' + pass + + @property + def attr(self): + return self._attr + + def items(self): + return self._attr.items() + + diff --git a/reframe/core/meta.py b/reframe/core/meta.py index 49376a9875..840bae5c48 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -10,6 +10,7 @@ from reframe.core.exceptions import ReframeSyntaxError import reframe.core.parameters as parameters +import reframe.core.variables as variables class RegressionTestMeta(type): @@ -25,6 +26,13 @@ def __prepare__(cls, name, bases, **kwargs): # class body as: `parameter('P0', 0,1,2,3)`. namespace['parameter'] = local_param_space.add_param + # Regression test var space defined at the class level + local_var_space = variables.LocalVarSpace() + namespace['_rfm_local_var_space'] = local_var_space + + # Directive to add a regression test variable + namespace['var'] = local_var_space.add_attr + return namespace def __init__(cls, name, bases, namespace, **kwargs): diff --git a/reframe/core/variables.py b/reframe/core/variables.py new file mode 100644 index 0000000000..519f28d48c --- /dev/null +++ b/reframe/core/variables.py @@ -0,0 +1,24 @@ +# Copyright 2016-2020 Swiss National Supercomputing Centre (CSCS/ETH Zurich) +# ReFrame Project Developers. See the top-level LICENSE file for details. +# +# SPDX-License-Identifier: BSD-3-Clause + +# +# Functionality to build extensible attribute spaces into ReFrame tests. +# + +import reframe.core.attributes as ReframeAttributes + + +class _TestVar: + def __init__(self, name, *types, required=False): + self.name = name + self.types = types + self.required = required + + +class LocalVarSpace(ReframeAttributes.LocalAttrSpace): + def add_attr(self, name, *args, **kwargs): + self[name] = _TestVar(name, *args, **kwargs) + + From 52d074bcfc0e603ee78f19920b821bf54f34e000 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Thu, 14 Jan 2021 16:40:21 +0100 Subject: [PATCH 02/49] Add support for an extensible var space --- reframe/core/attributes.py | 56 ++++++++++++++++++++++++++++--- reframe/core/meta.py | 3 +- reframe/core/variables.py | 67 +++++++++++++++++++++++++++++++++++++- 3 files changed, 120 insertions(+), 6 deletions(-) diff --git a/reframe/core/attributes.py b/reframe/core/attributes.py index b9d391ea86..822acf9664 100644 --- a/reframe/core/attributes.py +++ b/reframe/core/attributes.py @@ -52,11 +52,59 @@ def add_attr(self, name, *args, **kwargs): '''Insert a new attribute in the local attribute space.''' pass - @property - def attr(self): - return self._attr - def items(self): return self._attr.items() +class AttrSpace(metaclass=abc.ABCMeta): + + @property + @abc.abstractmethod + def localAttrSpaceName(self): + pass + + @property + @abc.abstractmethod + def localAttrSpaceCls(self): + pass + + @property + @abc.abstractmethod + def attrSpaceName(self): + pass + + def __init__(self, target_cls=None): + self._attr = {} + if target_cls: + + # Assert the AttrSpace can be built for the target_cls + self.assert_target_cls(target_cls) + + # Inherit AttrSpaces from the base clases + self.inherit(target_cls) + + # Extend the AttrSpace with the LocalAttrSpace + self.extend(target_cls) + + # Sanity checkings on the resulting AttrSpace + self.validate(target_cls) + + def assert_target_cls(self, cls): + assert hasattr(cls, self.localAttrSpaceName) + assert isinstance(getattr(cls, self.localAttrSpaceName), + self.localAttrSpaceCls) + + @abc.abstractmethod + def inherit(self, cls): + pass + + @abc.abstractmethod + def extend(self, cls): + pass + + @abc.abstractmethod + def validate(self, cls): + pass + + def items(self): + return self._attr.items() diff --git a/reframe/core/meta.py b/reframe/core/meta.py index 840bae5c48..806834c35b 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -38,8 +38,9 @@ def __prepare__(cls, name, bases, **kwargs): def __init__(cls, name, bases, namespace, **kwargs): super().__init__(name, bases, namespace, **kwargs) - # Build the regression test parameter space + # Build the parameter and variable spaces cls._rfm_param_space = parameters.ParamSpace(cls) + cls._rfm_var_space = variables.VarSpace(cls) # Set up the hooks for the pipeline stages based on the _rfm_attach # attribute; all dependencies will be resolved first in the post-setup diff --git a/reframe/core/variables.py b/reframe/core/variables.py index 519f28d48c..4de71c855c 100644 --- a/reframe/core/variables.py +++ b/reframe/core/variables.py @@ -11,14 +11,79 @@ class _TestVar: - def __init__(self, name, *types, required=False): + def __init__(self, name, *types, required=True): self.name = name self.types = types self.required = required + def is_required(self): + return self.required + class LocalVarSpace(ReframeAttributes.LocalAttrSpace): def add_attr(self, name, *args, **kwargs): self[name] = _TestVar(name, *args, **kwargs) + @property + def vars(self): + return self._attr + + +class VarSpace(ReframeAttributes.AttrSpace): + + localAttrSpaceName = '_rfm_local_var_space' + localAttrSpaceCls = LocalVarSpace + attrSpaceName = '_rfm_var_space' + + def __init__(self, target_cls=None): + self._requiredVars = set() + super().__init__(target_cls) + + + def inherit(self, cls): + for base in filter(lambda x: hasattr(x, self.attrSpaceName), + cls.__bases__): + assert isinstance(getattr(base, self.attrSpaceName), VarSpace) + self.join(getattr(base, self.attrSpaceName)) + + def join(self, other): + '''Join the variable spaces from the base clases + + Incorporate the variable space form a base class into the current + variable space. This follows standard inheritance rules, so if more + than two base clases have the same variable defined, the one imported + last will prevail. + + :param other: variable space from a base class. + ''' + for key, val in other.items(): + + # Override the required set + if key in other.required_vars: + self._requiredVars.add(key) + elif key in self._requiredVars: + self._requiredVars.remove(key) + + self._attr[key] = val + + def extend(self, cls): + for key, var in getattr(cls, self.localAttrSpaceName).items(): + + # Override the required set + if var.is_required(): + self._requiredVars.add(key) + elif key in self._requiredVars: + self._requiredVars.remove(key) + + self._attr[key] = var.types + + @property + def vars(self): + return self._attr + + @property + def required_vars(self): + return self._requiredVars + def validate(self, cls): + pass From 94bcd890a4d896511fe5b82c0099764ab7b78936 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Mon, 18 Jan 2021 18:25:29 +0100 Subject: [PATCH 03/49] Update dynamic var space --- reframe/core/attributes.py | 15 ++++-- reframe/core/fields.py | 7 ++- reframe/core/meta.py | 4 +- reframe/core/pipeline.py | 13 +++++ reframe/core/variables.py | 98 +++++++++++++++++++++++++------------- 5 files changed, 98 insertions(+), 39 deletions(-) diff --git a/reframe/core/attributes.py b/reframe/core/attributes.py index 822acf9664..882c4e816a 100644 --- a/reframe/core/attributes.py +++ b/reframe/core/attributes.py @@ -87,7 +87,7 @@ def __init__(self, target_cls=None): self.extend(target_cls) # Sanity checkings on the resulting AttrSpace - self.validate(target_cls) + self.sanity(target_cls) def assert_target_cls(self, cls): assert hasattr(cls, self.localAttrSpaceName) @@ -102,9 +102,16 @@ def inherit(self, cls): def extend(self, cls): pass - @abc.abstractmethod - def validate(self, cls): - pass + def sanity(self, cls): + + # Namespace clashing with cls disallowed + target_namespace = set(dir(cls)) + for key in self._attr: + if key in target_namespace: + raise ValueError( + f'{key!r} clashes with the namespace from ' + f'{cls.__qualname__!r}' + ) def items(self): return self._attr.items() diff --git a/reframe/core/fields.py b/reframe/core/fields.py index 91b8254b7c..6a0356d2fa 100644 --- a/reframe/core/fields.py +++ b/reframe/core/fields.py @@ -62,8 +62,8 @@ def __set__(self, obj, value): class TypedField(Field): '''Stores a field of predefined type''' - def __init__(self, main_type, *other_types): - self._types = (main_type,) + other_types + def __init__(self, *types): + self._types = types if not all(isinstance(t, type) for t in self._types): raise TypeError('{0} is not a sequence of types'. format(self._types)) @@ -79,6 +79,9 @@ def __set__(self, obj, value): self._check_type(value) super().__set__(obj, value) + def supports_type(self, value): + return value in self._types + class CopyOnWriteField(Field): '''Holds a copy of the variable that is assigned to it the first time''' diff --git a/reframe/core/meta.py b/reframe/core/meta.py index 806834c35b..7371779aa3 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -30,8 +30,10 @@ def __prepare__(cls, name, bases, **kwargs): local_var_space = variables.LocalVarSpace() namespace['_rfm_local_var_space'] = local_var_space - # Directive to add a regression test variable + # Directives to add/modify a regression test variable namespace['var'] = local_var_space.add_attr + namespace['require'] = local_var_space.undefine_attr + namespace['set_var'] = local_var_space.define_attr return namespace diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 25b1a6135a..dbac9f04ff 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -702,6 +702,9 @@ def __new__(cls, *args, _rfm_use_params=False, **kwargs): # Set the test parameters in the object cls._init_params(obj, _rfm_use_params) + # Insert the user variables + cls._insert_vars(obj) + # Create a test name from the class name and the constructor's # arguments name = cls.__qualname__ @@ -763,6 +766,16 @@ def _init_params(cls, obj, use_params=False): for key in cls._rfm_param_space.params: setattr(obj, key, None) + @classmethod + def _insert_vars(cls, obj): + '''Insert the vars in the regression test.''' + + for name, var in cls._rfm_var_space.items(): + setattr(cls, name, var.field(*var.types)) + getattr(cls, name).__set_name__(obj, name) + if not var.is_undef(): + setattr(obj, name, var.value) + def _append_parameters_to_name(self): if self._rfm_param_space.params: return '_' + '_'.join([str(self.__dict__[key]) diff --git a/reframe/core/variables.py b/reframe/core/variables.py index 4de71c855c..7db7c2ab7e 100644 --- a/reframe/core/variables.py +++ b/reframe/core/variables.py @@ -8,22 +8,49 @@ # import reframe.core.attributes as ReframeAttributes - +import reframe.core.fields as fields class _TestVar: - def __init__(self, name, *types, required=True): + def __init__(self, name, *types, field=fields.TypedField, **kwargs): + if 'value' in kwargs: + self.define(kwargs.get('value')) + else: + self.undefine() + self.name = name self.types = types - self.required = required + self.field = field + + def is_undef(self): + return self.value == '_rfm_undef' + + def undefine(self): + self.value = '_rfm_undef' - def is_required(self): - return self.required + def define(self, value): + self.value = value class LocalVarSpace(ReframeAttributes.LocalAttrSpace): + def __init__(self): + super().__init__() + self.undefined = set() + self.definitions = {} + def add_attr(self, name, *args, **kwargs): self[name] = _TestVar(name, *args, **kwargs) + def undefine_attr(self, name): + self.undefined.add(name) + + def define_attr(self, name, value): + if name not in self.definitions: + self.definitions[name] = value + else: + raise ValueError( + f'default value for var {name!r} are already set' + ) + @property def vars(self): return self._attr @@ -36,7 +63,6 @@ class VarSpace(ReframeAttributes.AttrSpace): attrSpaceName = '_rfm_var_space' def __init__(self, target_cls=None): - self._requiredVars = set() super().__init__(target_cls) @@ -47,43 +73,51 @@ def inherit(self, cls): self.join(getattr(base, self.attrSpaceName)) def join(self, other): - '''Join the variable spaces from the base clases - - Incorporate the variable space form a base class into the current - variable space. This follows standard inheritance rules, so if more - than two base clases have the same variable defined, the one imported - last will prevail. - - :param other: variable space from a base class. - ''' for key, val in other.items(): - # Override the required set - if key in other.required_vars: - self._requiredVars.add(key) - elif key in self._requiredVars: - self._requiredVars.remove(key) + # Multiple inheritance is NOT allowed + if key in self._attr: + raise ValueError( + f'var {key!r} is already present in the var space' + ) self._attr[key] = val def extend(self, cls): - for key, var in getattr(cls, self.localAttrSpaceName).items(): + localVarSpace = getattr(cls, self.localAttrSpaceName) + + # Extend the var space + for key, var in localVarSpace.items(): + + # Disable redeclaring a variable + if key in self._attr: + raise ValueError( + f'cannot redeclare a variable ({key!r})' + ) + + self._attr[key] = var - # Override the required set - if var.is_required(): - self._requiredVars.add(key) - elif key in self._requiredVars: - self._requiredVars.remove(key) + # Undefine the vars as indicated by the local var space + for key in localVarSpace.undefined: + self._check_var_is_declared(key) + self._attr[key].undefine() - self._attr[key] = var.types + # Define the vars as indicated by the local var space + for key, val in localVarSpace.definitions.items(): + self._check_var_is_declared(key) + self._attr[key].define(val) + + def _check_var_is_declared(self, key): + if key not in self._attr: + raise ValueError( + f'var {key!r} has not been declared' + ) @property def vars(self): return self._attr - @property - def required_vars(self): - return self._requiredVars + def undefined_vars(self): + return list(filter(lambda x: self._attr[x].is_undef(), self._attr)) + - def validate(self, cls): - pass From a305789b10ec7119fa6f824a1ff88b3a978e87c6 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Wed, 20 Jan 2021 16:24:21 +0100 Subject: [PATCH 04/49] Add docstring to var-related routines --- reframe/core/meta.py | 2 +- reframe/core/variables.py | 105 ++++++++++++++++++++++++++++++-------- 2 files changed, 85 insertions(+), 22 deletions(-) diff --git a/reframe/core/meta.py b/reframe/core/meta.py index 7371779aa3..a37b51b02a 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -32,7 +32,7 @@ def __prepare__(cls, name, bases, **kwargs): # Directives to add/modify a regression test variable namespace['var'] = local_var_space.add_attr - namespace['require'] = local_var_space.undefine_attr + namespace['require_var'] = local_var_space.undefine_attr namespace['set_var'] = local_var_space.define_attr return namespace diff --git a/reframe/core/variables.py b/reframe/core/variables.py index 7db7c2ab7e..a5fc590875 100644 --- a/reframe/core/variables.py +++ b/reframe/core/variables.py @@ -11,45 +11,88 @@ import reframe.core.fields as fields class _TestVar: + '''Regression test variable. + + Buffer to store a regression test variable into either a VarSpace or a + LocalVarSpace. + ''' def __init__(self, name, *types, field=fields.TypedField, **kwargs): if 'value' in kwargs: self.define(kwargs.get('value')) else: self.undefine() + if not issubclass(field, fields.Field): + raise ValueError( + f'field {field!r} is not derived from ' + f'{fields.Field.__qualname__}' + ) + self.name = name self.types = types self.field = field def is_undef(self): - return self.value == '_rfm_undef' + return self.value == '_rfm_undef_var' def undefine(self): - self.value = '_rfm_undef' + self.value = '_rfm_undef_var' def define(self, value): self.value = value class LocalVarSpace(ReframeAttributes.LocalAttrSpace): + '''Local variable space of a regression test. + + Stores the input from the var directives executed in the class body of + the regression test. This local variable space is later used during the + instantiation of the VarSpace class to extend the final variable space. + ''' def __init__(self): super().__init__() self.undefined = set() self.definitions = {} - def add_attr(self, name, *args, **kwargs): - self[name] = _TestVar(name, *args, **kwargs) + def add_attr(self, name, *types, **kwargs): + '''Declare a new regression test variable. + + If the ``value`` argument is not provided, the variable is considered + *declared* but not *defined*. Note that a variable must be defined + before is referenced in the regression test. + + :param name: the variable name. + :param types: the supported types for the variable. + :param value: the default value assigned to the variable. + :params field: the field validator to be used for this variable. + If no field argument is provided, it defaults to a TypedField + (see :class `reframe.core.fields`). Note that the field validator + provided by this argument must derive from + :class `reframe.core.fields.Field`. + ''' + self[name] = _TestVar(name, *types, **kwargs) def undefine_attr(self, name): + '''Undefine a variable previously declared in a parent class. + + This method is particularly useful when writing a test library, + since it permits to remove any default values that may have been + defined for a variable in any of the parent classes. Effectively, this + will force the user of the library to provide the required value for a + variable. However, a variable flagged as ``required`` which is not + referenced in the regression test is implemented as a no-op. + + :param name: the name of the required variable. + ''' self.undefined.add(name) def define_attr(self, name, value): - if name not in self.definitions: - self.definitions[name] = value - else: - raise ValueError( - f'default value for var {name!r} are already set' - ) + '''Assign a value to a regression test variable. + + :param name: the variable name. + :param value: the value assigned to the variable. + ''' + self.definitions[name] = value @property def vars(self): @@ -57,7 +100,19 @@ def vars(self): class VarSpace(ReframeAttributes.AttrSpace): - + '''Variable space of a regression test. + + Store the variables of a regression test. This variable space is stored + in the regression test class under the class attribute `_rfm_var_space`. + A target class can be provided to the + :func:`__init__` method, which is the regression test where the + VarSpace is to be built. During this call to + :func:`__init__`, the VarSpace inherits all the VarSpace from the base + classes of the target class. After this, the VarSpace is extended with + the information from the local variable space, which is stored under the + target class' attribute '_rfm_local_var_space'. If no target class is + provided, the VarSpace is simply initialized as empty. + ''' localAttrSpaceName = '_rfm_local_var_space' localAttrSpaceCls = LocalVarSpace attrSpaceName = '_rfm_var_space' @@ -65,34 +120,44 @@ class VarSpace(ReframeAttributes.AttrSpace): def __init__(self, target_cls=None): super().__init__(target_cls) - def inherit(self, cls): + '''Inherit the VarSpace from the bases.''' for base in filter(lambda x: hasattr(x, self.attrSpaceName), cls.__bases__): assert isinstance(getattr(base, self.attrSpaceName), VarSpace) self.join(getattr(base, self.attrSpaceName)) - def join(self, other): - for key, val in other.items(): + def join(self, other_var_space): + '''Join an existing VarSpace into the current one.''' + for key, var in other_var_space.items(): - # Multiple inheritance is NOT allowed + # Make doubly declared vars illegal. if key in self._attr: raise ValueError( - f'var {key!r} is already present in the var space' + f'cannot redeclare a variable ({key})' ) - self._attr[key] = val + self._attr[key] = var def extend(self, cls): + '''Extend the VarSpace with the content in the LocalVarSpace + + Merge the VarSpace inherited from the base classes with the + LocalVarSpace. Note that the LocalVarSpace can also contain + define and undefine actions on existing vars. Thus, since it + does not make sense to define and undefine a var in the same + class, the order on which the define and undefine functions + are called is not preserved. + ''' localVarSpace = getattr(cls, self.localAttrSpaceName) - # Extend the var space + # Extend the VarSpace for key, var in localVarSpace.items(): # Disable redeclaring a variable if key in self._attr: raise ValueError( - f'cannot redeclare a variable ({key!r})' + f'cannot redeclare a variable ({key})' ) self._attr[key] = var @@ -119,5 +184,3 @@ def vars(self): def undefined_vars(self): return list(filter(lambda x: self._attr[x].is_undef(), self._attr)) - - From ec675b1b04d123eb2fba5742e198a363c217c83a Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Wed, 20 Jan 2021 16:31:19 +0100 Subject: [PATCH 05/49] Update copyright --- reframe/core/attributes.py | 2 +- reframe/core/parameters.py | 2 +- reframe/core/variables.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/reframe/core/attributes.py b/reframe/core/attributes.py index 882c4e816a..87256c4ef7 100644 --- a/reframe/core/attributes.py +++ b/reframe/core/attributes.py @@ -1,4 +1,4 @@ -# Copyright 2016-2020 Swiss National Supercomputing Centre (CSCS/ETH Zurich) +# Copyright 2016-2021 Swiss National Supercomputing Centre (CSCS/ETH Zurich) # ReFrame Project Developers. See the top-level LICENSE file for details. # # SPDX-License-Identifier: BSD-3-Clause diff --git a/reframe/core/parameters.py b/reframe/core/parameters.py index 24320be7c8..6f6642c1c9 100644 --- a/reframe/core/parameters.py +++ b/reframe/core/parameters.py @@ -1,4 +1,4 @@ -# Copyright 2016-2020 Swiss National Supercomputing Centre (CSCS/ETH Zurich) +# Copyright 2016-2021 Swiss National Supercomputing Centre (CSCS/ETH Zurich) # ReFrame Project Developers. See the top-level LICENSE file for details. # # SPDX-License-Identifier: BSD-3-Clause diff --git a/reframe/core/variables.py b/reframe/core/variables.py index a5fc590875..9aaf9fbe23 100644 --- a/reframe/core/variables.py +++ b/reframe/core/variables.py @@ -1,4 +1,4 @@ -# Copyright 2016-2020 Swiss National Supercomputing Centre (CSCS/ETH Zurich) +# Copyright 2016-2021 Swiss National Supercomputing Centre (CSCS/ETH Zurich) # ReFrame Project Developers. See the top-level LICENSE file for details. # # SPDX-License-Identifier: BSD-3-Clause From b855eb45a91551b2611b7c38dea30b35035a4611 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Wed, 20 Jan 2021 18:00:54 +0100 Subject: [PATCH 06/49] Use the var directive in the pipeline --- reframe/core/pipeline.py | 130 ++++++++++++--------------------------- 1 file changed, 41 insertions(+), 89 deletions(-) diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 775e0ca284..de67a72149 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -180,7 +180,7 @@ def pipeline_hooks(cls): #: .. versionchanged:: 2.17 #: Support for wildcards is dropped. #: - valid_prog_environs = fields.TypedField(typ.List[str], type(None)) + var('valid_prog_environs', typ.List[str], type(None), value=None) #: List of systems supported by this test. #: The general syntax for systems is ``[:]``. @@ -189,13 +189,13 @@ def pipeline_hooks(cls): #: #: :type: :class:`List[str]` #: :default: ``[]`` - valid_systems = fields.TypedField(typ.List[str], type(None)) + var('valid_systems', typ.List[str], type(None), value=None) #: A detailed description of the test. #: #: :type: :class:`str` #: :default: ``self.name`` - descr = fields.TypedField(str) + var('descr', str) #: The path to the source file or source directory of the test. #: @@ -213,7 +213,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`str` #: :default: ``''`` - sourcepath = fields.TypedField(str) + var('sourcepath', str, value='') #: The directory containing the test's resources. #: @@ -243,7 +243,7 @@ def pipeline_hooks(cls): #: .. versionchanged:: 3.0 #: Default value is now conditionally set to either ``'src'`` or #: :class:`None`. - sourcesdir = fields.TypedField(str, type(None)) + var('sourcesdir', str, type(None), value='src') #: .. versionadded:: 2.14 #: @@ -260,7 +260,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`str` or :class:`reframe.core.buildsystems.BuildSystem`. #: :default: :class:`None`. - build_system = BuildSystemField(type(None)) + var('build_system', type(None), field=BuildSystemField, value=None) #: .. versionadded:: 3.0 #: @@ -272,7 +272,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`List[str]` #: :default: ``[]`` - prebuild_cmds = fields.TypedField(typ.List[str]) + var('prebuild_cmds', typ.List[str], value=[]) #: .. versionadded:: 3.0 #: @@ -284,19 +284,19 @@ def pipeline_hooks(cls): #: #: :type: :class:`List[str]` #: :default: ``[]`` - postbuild_cmds = fields.TypedField(typ.List[str]) + var('postbuild_cmds', typ.List[str], value=[]) #: The name of the executable to be launched during the run phase. #: #: :type: :class:`str` #: :default: ``os.path.join('.', self.name)`` - executable = fields.TypedField(str) + var('executable', str) #: List of options to be passed to the :attr:`executable`. #: #: :type: :class:`List[str]` #: :default: ``[]`` - executable_opts = fields.TypedField(typ.List[str]) + var('executable_opts', typ.List[str], value=[]) #: .. versionadded:: 2.20 #: @@ -320,7 +320,9 @@ def pipeline_hooks(cls): #: :type: :class:`str` or #: :class:`reframe.core.containers.ContainerPlatform`. #: :default: :class:`None`. - container_platform = ContainerPlatformField(type(None)) + var('container_platform', type(None), + field=ContainerPlatformField, value=None + ) #: .. versionadded:: 3.0 #: @@ -332,7 +334,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`List[str]` #: :default: ``[]`` - prerun_cmds = fields.TypedField(typ.List[str]) + var('prerun_cmds', typ.List[str], value=[]) #: .. versionadded:: 3.0 #: @@ -343,7 +345,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`List[str]` #: :default: ``[]`` - postrun_cmds = fields.TypedField(typ.List[str]) + var('postrun_cmds', typ.List[str], value=[]) #: List of files to be kept after the test finishes. #: @@ -363,7 +365,7 @@ def pipeline_hooks(cls): #: .. versionchanged:: 3.3 #: This field accepts now also file glob patterns. #: - keep_files = fields.TypedField(typ.List[str]) + var('keep_files', typ.List[str], value=[]) #: List of files or directories (relative to the :attr:`sourcesdir`) that #: will be symlinked in the stage directory and not copied. @@ -373,7 +375,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`List[str]` #: :default: ``[]`` - readonly_files = fields.TypedField(typ.List[str]) + var('readonly_files', typ.List[str], value=[]) #: Set of tags associated with this test. #: @@ -381,7 +383,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`Set[str]` #: :default: an empty set - tags = fields.TypedField(typ.Set[str]) + var('tags', typ.Set[str], value=set()) #: List of people responsible for this test. #: @@ -389,7 +391,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`List[str]` #: :default: ``[]`` - maintainers = fields.TypedField(typ.List[str]) + var('maintainers', typ.List[str], value=[]) #: Mark this test as a strict performance test. #: @@ -399,7 +401,7 @@ def pipeline_hooks(cls): #: #: :type: boolean #: :default: :class:`True` - strict_check = fields.TypedField(bool) + var('strict_check', bool, value=True) #: Number of tasks required by this test. #: @@ -425,7 +427,7 @@ def pipeline_hooks(cls): #: #: .. |--flex-alloc-nodes| replace:: :attr:`--flex-alloc-nodes` #: .. _--flex-alloc-nodes: manpage.html#cmdoption-flex-alloc-nodes - num_tasks = fields.TypedField(int) + var('num_tasks', int, value=1) #: Number of tasks per node required by this test. #: @@ -433,7 +435,7 @@ def pipeline_hooks(cls): #: #: :type: integral or :class:`None` #: :default: :class:`None` - num_tasks_per_node = fields.TypedField(int, type(None)) + var('num_tasks_per_node', int, type(None), value=None) #: Number of GPUs per node required by this test. #: This attribute is translated internally to the ``_rfm_gpu`` resource. @@ -442,7 +444,7 @@ def pipeline_hooks(cls): #: #: :type: integral #: :default: ``0`` - num_gpus_per_node = fields.TypedField(int) + var('num_gpus_per_node', int, value=0) #: Number of CPUs per task required by this test. #: @@ -450,7 +452,7 @@ def pipeline_hooks(cls): #: #: :type: integral or :class:`None` #: :default: :class:`None` - num_cpus_per_task = fields.TypedField(int, type(None)) + var('num_cpus_per_task', int, type(None), value=None) #: Number of tasks per core required by this test. #: @@ -458,7 +460,7 @@ def pipeline_hooks(cls): #: #: :type: integral or :class:`None` #: :default: :class:`None` - num_tasks_per_core = fields.TypedField(int, type(None)) + var('num_tasks_per_core', int, type(None), value=None) #: Number of tasks per socket required by this test. #: @@ -466,7 +468,7 @@ def pipeline_hooks(cls): #: #: :type: integral or :class:`None` #: :default: :class:`None` - num_tasks_per_socket = fields.TypedField(int, type(None)) + var('num_tasks_per_socket', int, type(None), value=None) #: Specify whether this tests needs simultaneous multithreading enabled. #: @@ -474,7 +476,7 @@ def pipeline_hooks(cls): #: #: :type: boolean or :class:`None` #: :default: :class:`None` - use_multithreading = fields.TypedField(bool, type(None)) + var('use_multithreading', bool, type(None), value=None) #: .. versionadded:: 3.0 #: @@ -484,19 +486,19 @@ def pipeline_hooks(cls): #: #: :type: :class:`str` or :class:`datetime.timedelta` #: :default: :class:`None` - max_pending_time = fields.TimerField(type(None)) + var('max_pending_time', type(None), field=fields.TimerField, value=None) #: Specify whether this test needs exclusive access to nodes. #: #: :type: boolean #: :default: :class:`False` - exclusive_access = fields.TypedField(bool) + var('exclusive_access', bool, value=False) #: Always execute this test locally. #: #: :type: boolean #: :default: :class:`False` - local = fields.TypedField(bool) + var('local', bool, value=False) #: The set of reference values for this test. #: @@ -528,8 +530,9 @@ def pipeline_hooks(cls): #: .. versionchanged:: 3.0 #: The measurement unit is required. The user should explicitly #: specify :class:`None` if no unit is available. - reference = fields.ScopedDictField( - typ.Tuple[object, object, object, object]) + var('reference', typ.Tuple[object, object, object, object], + field=fields.ScopedDictField, value={} + ) # FIXME: There is not way currently to express tuples of `float`s or # `None`s, so we just use the very generic `object` @@ -555,7 +558,7 @@ def pipeline_hooks(cls): #: :: #: #: self.sanity_patterns = sn.assert_found(r'.*', self.stdout) - sanity_patterns = fields.TypedField(_DeferredExpression, type(None)) + var('sanity_patterns', _DeferredExpression, type(None), value=None) #: Patterns for verifying the performance of this test. #: @@ -569,8 +572,7 @@ def pipeline_hooks(cls): #: `) as values. #: :class:`None` is also allowed. #: :default: :class:`None` - perf_patterns = fields.TypedField( - typ.Dict[str, _DeferredExpression], type(None)) + var('perf_patterns', typ.Dict[str, _DeferredExpression], type(None), value=None) #: List of modules to be loaded before running this test. #: @@ -578,7 +580,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`List[str]` #: :default: ``[]`` - modules = fields.TypedField(typ.List[str]) + var('modules', typ.List[str], value=[]) #: Environment variables to be set before running this test. #: @@ -586,7 +588,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`Dict[str, str]` #: :default: ``{}`` - variables = fields.TypedField(typ.Dict[str, str]) + var('variables', typ.Dict[str, str], value={}) #: Time limit for this test. #: @@ -610,7 +612,7 @@ def pipeline_hooks(cls): #: - The old syntax using a ``(h, m, s)`` tuple is dropped. #: - Support of `timedelta` objects is dropped. #: - Number values are now accepted. - time_limit = fields.TimerField(type(None)) + var('time_limit', type(None), field=fields.TimerField, value='10m') #: .. versionadded:: 2.8 #: @@ -678,8 +680,7 @@ def pipeline_hooks(cls): #: .. versionchanged:: 2.9 #: A new more powerful syntax was introduced #: that allows also custom job script directive prefixes. - extra_resources = fields.TypedField( - typ.Dict[str, typ.Dict[str, object]]) + var('extra_resources', typ.Dict[str, typ.Dict[str, object]], value={}) #: .. versionadded:: 3.3 #: @@ -694,7 +695,7 @@ def pipeline_hooks(cls): #: appropriate sanity check. #: #: :type: boolean : :default: :class:`True` - build_locally = fields.TypedField(bool) + var('build_locally', bool, value=True) def __new__(cls, *args, _rfm_use_params=False, **kwargs): obj = super().__new__(cls) @@ -800,68 +801,20 @@ def _rfm_init(self, name=None, prefix=None): self.name = name self.descr = self.name - self.valid_prog_environs = None - self.valid_systems = None - self.sourcepath = '' - self.prebuild_cmds = [] - self.postbuild_cmds = [] self.executable = os.path.join('.', self.name) - self.executable_opts = [] - self.prerun_cmds = [] - self.postrun_cmds = [] - self.keep_files = [] - self.readonly_files = [] - self.tags = set() - self.maintainers = [] self._perfvalues = {} - self.container_platform = None - - # Strict performance check, if applicable - self.strict_check = True - - # Default is a single node check - self.num_tasks = 1 - self.num_tasks_per_node = None - self.num_gpus_per_node = 0 - self.num_cpus_per_task = None - self.num_tasks_per_core = None - self.num_tasks_per_socket = None - self.use_multithreading = None - self.exclusive_access = False - self.max_pending_time = None - - # True only if check is to be run locally - self.local = False - self.build_locally = True # Static directories of the regression check self._prefix = os.path.abspath(prefix) - if os.path.isdir(os.path.join(self._prefix, 'src')): - self.sourcesdir = 'src' - else: + if not os.path.isdir(os.path.join(self._prefix, self.sourcesdir)): self.sourcesdir = None - # Output patterns - self.sanity_patterns = None - - # Performance patterns: None -> no performance checking - self.perf_patterns = None - self.reference = {} - - # Environment setup - self.modules = [] - self.variables = {} - - # Time limit for the check - self.time_limit = '10m' - # Runtime information of the test self._current_partition = None self._current_environ = None # Associated job self._job = None - self.extra_resources = {} # Dynamic paths of the regression check; will be set in setup() self._stagedir = None @@ -872,7 +825,6 @@ def _rfm_init(self, name=None, prefix=None): # Compilation process output self._build_job = None self._compile_proc = None - self.build_system = None # Performance logging self._perf_logger = logging.null_logger From 2541a929ad74e29c2fff51305a7c75d0ca59654d Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Thu, 21 Jan 2021 14:16:15 +0100 Subject: [PATCH 07/49] Add RegressionMixin class --- reframe/core/pipeline.py | 14 +++++++++++++- reframe/core/variables.py | 3 ++- unittests/test_pipeline.py | 2 +- 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index de67a72149..b55c9c2e94 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -9,7 +9,7 @@ __all__ = [ 'CompileOnlyRegressionTest', 'RegressionTest', 'RunOnlyRegressionTest', - 'DEPEND_BY_ENV', 'DEPEND_EXACT', 'DEPEND_FULLY', 'final' + 'DEPEND_BY_ENV', 'DEPEND_EXACT', 'DEPEND_FULLY', 'final', 'RegressionMixin' ] @@ -126,6 +126,18 @@ def _wrapped(*args, **kwargs): return _wrapped +class RegressionMixin(metaclass=RegressionTestMeta): + '''Base mixin class for regression tests. + + Multiple inheritance from more than one + :class:`RegressionTest` class is not allowed in ReFrame. Hence, mixin + classes provide the flexibility to bundle reusable test add-ons, leveraging + the metaclass magic implemented in + :class:`RegressionTestMeta`. Using this metaclass allows mixin classes to + use powerful ReFrame features, such as hooks, parameters or variables. + ''' + + class RegressionTest(jsonext.JSONSerializable, metaclass=RegressionTestMeta): '''Base class for regression tests. diff --git a/reframe/core/variables.py b/reframe/core/variables.py index 9aaf9fbe23..9f60e703bd 100644 --- a/reframe/core/variables.py +++ b/reframe/core/variables.py @@ -131,7 +131,8 @@ def join(self, other_var_space): '''Join an existing VarSpace into the current one.''' for key, var in other_var_space.items(): - # Make doubly declared vars illegal. + # Make doubly declared vars illegal. Note that this will be + # triggered when inheriting from multiple RegressionTest classes. if key in self._attr: raise ValueError( f'cannot redeclare a variable ({key})' diff --git a/unittests/test_pipeline.py b/unittests/test_pipeline.py index bdd0611244..5d266f5388 100644 --- a/unittests/test_pipeline.py +++ b/unittests/test_pipeline.py @@ -609,7 +609,7 @@ def __init__(self): def x(self): self.var += 1 - class C(rfm.RegressionTest): + class C(rfm.RegressionMixin): @rfm.run_before('run') def y(self): self.foo = 1 From f2f696e538ca6939d5e9b212b4545d38d26a3c34 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Thu, 21 Jan 2021 19:17:43 +0100 Subject: [PATCH 08/49] Fix module import leak in pipeline tests --- unittests/test_pipeline.py | 242 ++++++++++++++++++++----------------- 1 file changed, 130 insertions(+), 112 deletions(-) diff --git a/unittests/test_pipeline.py b/unittests/test_pipeline.py index 5d266f5388..2a9cf0dfc3 100644 --- a/unittests/test_pipeline.py +++ b/unittests/test_pipeline.py @@ -6,6 +6,7 @@ import os import pytest import re +import sys import reframe as rfm import reframe.core.runtime as rt @@ -14,9 +15,6 @@ import unittests.fixtures as fixtures from reframe.core.exceptions import (BuildError, PipelineError, ReframeError, PerformanceError, SanityError) -from reframe.frontend.loader import RegressionCheckLoader -from unittests.resources.checks.hellocheck import HelloTest -from unittests.resources.checks.pinnedcheck import PinnedTest def _run(test, partition, prgenv): @@ -30,9 +28,25 @@ def _run(test, partition, prgenv): test.cleanup(remove_files=True) -def load_test(testfile): - loader = RegressionCheckLoader(['unittests/resources/checks']) - return loader.load_from_file(testfile) +@pytest.fixture +def hellotest(): + from unittests.resources.checks.hellocheck import HelloTest + yield HelloTest + del sys.modules['unittests.resources.checks.hellocheck'] + + +@pytest.fixture +def hellomaketest(): + from unittests.resources.checks.hellocheck_make import HelloMakeTest + yield HelloMakeTest + del sys.modules['unittests.resources.checks.hellocheck_make'] + + +@pytest.fixture +def pinnedtest(): + from unittests.resources.checks.pinnedcheck import PinnedTest + yield PinnedTest + del sys.modules['unittests.resources.checks.pinnedcheck'] @pytest.fixture @@ -64,10 +78,6 @@ def user_system(temp_runtime): yield generic_system -@pytest.fixture -def hellotest(): - yield load_test('unittests/resources/checks/hellocheck.py')[0] - @pytest.fixture def local_exec_ctx(generic_system): @@ -149,48 +159,52 @@ def __init__(self): def test_environ_setup(hellotest, local_exec_ctx): + inst = hellotest() + # Use test environment for the regression check - hellotest.variables = {'_FOO_': '1', '_BAR_': '2'} - hellotest.setup(*local_exec_ctx) - for k in hellotest.variables.keys(): + inst.variables = {'_FOO_': '1', '_BAR_': '2'} + inst.setup(*local_exec_ctx) + for k in inst.variables.keys(): assert k not in os.environ def test_hellocheck(hellotest, remote_exec_ctx): - _run(hellotest, *remote_exec_ctx) + _run(hellotest(), *remote_exec_ctx) -def test_hellocheck_make(remote_exec_ctx): - test = load_test('unittests/resources/checks/hellocheck_make.py')[0] - _run(test, *remote_exec_ctx) +def test_hellocheck_make(hellomaketest, remote_exec_ctx): + _run(hellomaketest(), *remote_exec_ctx) def test_hellocheck_local(hellotest, local_exec_ctx): + inst = hellotest() + # Test also the prebuild/postbuild functionality - hellotest.prebuild_cmds = ['touch prebuild', 'mkdir -p prebuild_dir/foo'] - hellotest.postbuild_cmds = ['touch postbuild', 'mkdir postbuild_dir'] - hellotest.keep_files = ['prebuild', 'postbuild', '*dir'] + inst.prebuild_cmds = ['touch prebuild', 'mkdir -p prebuild_dir/foo'] + inst.postbuild_cmds = ['touch postbuild', 'mkdir postbuild_dir'] + inst.keep_files = ['prebuild', 'postbuild', '*dir'] # Force local execution of the test; just for testing .local - hellotest.local = True - _run(hellotest, *local_exec_ctx) + inst.local = True + _run(inst, *local_exec_ctx) must_keep = [ - hellotest.stdout.evaluate(), - hellotest.stderr.evaluate(), - hellotest.build_stdout.evaluate(), - hellotest.build_stderr.evaluate(), - hellotest.job.script_filename, + inst.stdout.evaluate(), + inst.stderr.evaluate(), + inst.build_stdout.evaluate(), + inst.build_stderr.evaluate(), + inst.job.script_filename, 'prebuild', 'postbuild', 'prebuild_dir', 'prebuild_dir/foo', 'postbuild_dir' ] for f in must_keep: - assert os.path.exists(os.path.join(hellotest.outputdir, f)) + assert os.path.exists(os.path.join(inst.outputdir, f)) def test_hellocheck_build_remotely(hellotest, remote_exec_ctx): - hellotest.build_locally = False - _run(hellotest, *remote_exec_ctx) - assert not hellotest.build_job.scheduler.is_local + inst = hellotest() + inst.build_locally = False + _run(inst, *remote_exec_ctx) + assert not inst.build_job.scheduler.is_local def test_hellocheck_local_prepost_run(hellotest, local_exec_ctx): @@ -198,16 +212,18 @@ def test_hellocheck_local_prepost_run(hellotest, local_exec_ctx): def stagedir(test): return test.stagedir + inst = hellotest() + # Test also the prebuild/postbuild functionality - hellotest.prerun_cmds = ['echo prerun: `pwd`'] - hellotest.postrun_cmds = ['echo postrun: `pwd`'] - pre_run_path = sn.extractsingle(r'^prerun: (\S+)', hellotest.stdout, 1) - post_run_path = sn.extractsingle(r'^postrun: (\S+)', hellotest.stdout, 1) - hellotest.sanity_patterns = sn.all([ - sn.assert_eq(stagedir(hellotest), pre_run_path), - sn.assert_eq(stagedir(hellotest), post_run_path), + inst.prerun_cmds = ['echo prerun: `pwd`'] + inst.postrun_cmds = ['echo postrun: `pwd`'] + pre_run_path = sn.extractsingle(r'^prerun: (\S+)', inst.stdout, 1) + post_run_path = sn.extractsingle(r'^postrun: (\S+)', inst.stdout, 1) + inst.sanity_patterns = sn.all([ + sn.assert_eq(stagedir(inst), pre_run_path), + sn.assert_eq(stagedir(inst), post_run_path), ]) - _run(hellotest, *local_exec_ctx) + _run(inst, *local_exec_ctx) def test_run_only_sanity(local_exec_ctx): @@ -269,8 +285,8 @@ def __init__(self): _run(MyTest(), *local_exec_ctx) -def test_pinned_test(local_exec_ctx): - class MyTest(PinnedTest): +def test_pinned_test(pinnedtest, local_exec_ctx): + class MyTest(pinnedtest): pass pinned = MyTest() @@ -279,59 +295,61 @@ class MyTest(PinnedTest): def test_supports_system(hellotest, testsys_system): - hellotest.valid_systems = ['*'] - assert hellotest.supports_system('gpu') - assert hellotest.supports_system('login') - assert hellotest.supports_system('testsys:gpu') - assert hellotest.supports_system('testsys:login') - - hellotest.valid_systems = ['*:*'] - assert hellotest.supports_system('gpu') - assert hellotest.supports_system('login') - assert hellotest.supports_system('testsys:gpu') - assert hellotest.supports_system('testsys:login') - - hellotest.valid_systems = ['testsys'] - assert hellotest.supports_system('gpu') - assert hellotest.supports_system('login') - assert hellotest.supports_system('testsys:gpu') - assert hellotest.supports_system('testsys:login') - - hellotest.valid_systems = ['testsys:gpu'] - assert hellotest.supports_system('gpu') - assert not hellotest.supports_system('login') - assert hellotest.supports_system('testsys:gpu') - assert not hellotest.supports_system('testsys:login') - - hellotest.valid_systems = ['testsys:login'] - assert not hellotest.supports_system('gpu') - assert hellotest.supports_system('login') - assert not hellotest.supports_system('testsys:gpu') - assert hellotest.supports_system('testsys:login') - - hellotest.valid_systems = ['foo'] - assert not hellotest.supports_system('gpu') - assert not hellotest.supports_system('login') - assert not hellotest.supports_system('testsys:gpu') - assert not hellotest.supports_system('testsys:login') - - hellotest.valid_systems = ['*:gpu'] - assert hellotest.supports_system('testsys:gpu') - assert hellotest.supports_system('foo:gpu') - assert not hellotest.supports_system('testsys:cpu') - assert not hellotest.supports_system('testsys:login') - - hellotest.valid_systems = ['testsys:*'] - assert hellotest.supports_system('testsys:login') - assert hellotest.supports_system('gpu') - assert not hellotest.supports_system('foo:gpu') + inst = hellotest() + inst.valid_systems = ['*'] + assert inst.supports_system('gpu') + assert inst.supports_system('login') + assert inst.supports_system('testsys:gpu') + assert inst.supports_system('testsys:login') + + inst.valid_systems = ['*:*'] + assert inst.supports_system('gpu') + assert inst.supports_system('login') + assert inst.supports_system('testsys:gpu') + assert inst.supports_system('testsys:login') + + inst.valid_systems = ['testsys'] + assert inst.supports_system('gpu') + assert inst.supports_system('login') + assert inst.supports_system('testsys:gpu') + assert inst.supports_system('testsys:login') + + inst.valid_systems = ['testsys:gpu'] + assert inst.supports_system('gpu') + assert not inst.supports_system('login') + assert inst.supports_system('testsys:gpu') + assert not inst.supports_system('testsys:login') + + inst.valid_systems = ['testsys:login'] + assert not inst.supports_system('gpu') + assert inst.supports_system('login') + assert not inst.supports_system('testsys:gpu') + assert inst.supports_system('testsys:login') + + inst.valid_systems = ['foo'] + assert not inst.supports_system('gpu') + assert not inst.supports_system('login') + assert not inst.supports_system('testsys:gpu') + assert not inst.supports_system('testsys:login') + + inst.valid_systems = ['*:gpu'] + assert inst.supports_system('testsys:gpu') + assert inst.supports_system('foo:gpu') + assert not inst.supports_system('testsys:cpu') + assert not inst.supports_system('testsys:login') + + inst.valid_systems = ['testsys:*'] + assert inst.supports_system('testsys:login') + assert inst.supports_system('gpu') + assert not inst.supports_system('foo:gpu') def test_supports_environ(hellotest, generic_system): - hellotest.valid_prog_environs = ['*'] - assert hellotest.supports_environ('foo1') - assert hellotest.supports_environ('foo-env') - assert hellotest.supports_environ('*') + inst = hellotest() + inst.valid_prog_environs = ['*'] + assert inst.supports_environ('foo1') + assert inst.supports_environ('foo-env') + assert inst.supports_environ('*') def test_sourcesdir_none(local_exec_ctx): @@ -450,9 +468,9 @@ def __init__(self): test.compile_wait() -def test_extra_resources(testsys_system): +def test_extra_resources(hellotest, testsys_system): @fixtures.custom_prefix('unittests/resources/checks') - class MyTest(HelloTest): + class MyTest(hellotest): def __init__(self): super().__init__() self.name = type(self).__name__ @@ -479,9 +497,9 @@ def set_resources(self): assert expected_job_options == set(test.job.options) -def test_setup_hooks(local_exec_ctx): +def test_setup_hooks(hellotest, local_exec_ctx): @fixtures.custom_prefix('unittests/resources/checks') - class MyTest(HelloTest): + class MyTest(hellotest): def __init__(self): super().__init__() self.name = type(self).__name__ @@ -503,9 +521,9 @@ def postfoo(self): assert test.count == 2 -def test_compile_hooks(local_exec_ctx): +def test_compile_hooks(hellotest, local_exec_ctx): @fixtures.custom_prefix('unittests/resources/checks') - class MyTest(HelloTest): + class MyTest(hellotest): def __init__(self): super().__init__() self.name = type(self).__name__ @@ -528,9 +546,9 @@ def check_executable(self): assert test.count == 1 -def test_run_hooks(local_exec_ctx): +def test_run_hooks(hellotest, local_exec_ctx): @fixtures.custom_prefix('unittests/resources/checks') - class MyTest(HelloTest): + class MyTest(hellotest): def __init__(self): super().__init__() self.name = type(self).__name__ @@ -550,9 +568,9 @@ def check_executable(self): _run(MyTest(), *local_exec_ctx) -def test_multiple_hooks(local_exec_ctx): +def test_multiple_hooks(hellotest, local_exec_ctx): @fixtures.custom_prefix('unittests/resources/checks') - class MyTest(HelloTest): + class MyTest(hellotest): def __init__(self): super().__init__() self.name = type(self).__name__ @@ -576,9 +594,9 @@ def z(self): assert test.var == 3 -def test_stacked_hooks(local_exec_ctx): +def test_stacked_hooks(hellotest, local_exec_ctx): @fixtures.custom_prefix('unittests/resources/checks') - class MyTest(HelloTest): + class MyTest(hellotest): def __init__(self): super().__init__() self.name = type(self).__name__ @@ -596,9 +614,9 @@ def x(self): assert test.var == 3 -def test_inherited_hooks(local_exec_ctx): +def test_inherited_hooks(hellotest, local_exec_ctx): @fixtures.custom_prefix('unittests/resources/checks') - class BaseTest(HelloTest): + class BaseTest(hellotest): def __init__(self): super().__init__() self.name = type(self).__name__ @@ -632,9 +650,9 @@ class MyTest(DerivedTest): } -def test_overriden_hooks(local_exec_ctx): +def test_overriden_hooks(hellotest, local_exec_ctx): @fixtures.custom_prefix('unittests/resources/checks') - class BaseTest(HelloTest): + class BaseTest(hellotest): def __init__(self): super().__init__() self.name = type(self).__name__ @@ -666,9 +684,9 @@ def y(self): assert test.foo == 10 -def test_disabled_hooks(local_exec_ctx): +def test_disabled_hooks(hellotest, local_exec_ctx): @fixtures.custom_prefix('unittests/resources/checks') - class BaseTest(HelloTest): + class BaseTest(hellotest): def __init__(self): super().__init__() self.name = type(self).__name__ @@ -696,12 +714,12 @@ def x(self): assert test.foo == 0 -def test_require_deps(local_exec_ctx): +def test_require_deps(hellotest, local_exec_ctx): import reframe.frontend.dependencies as dependencies import reframe.frontend.executors as executors @fixtures.custom_prefix('unittests/resources/checks') - class T0(HelloTest): + class T0(hellotest): def __init__(self): super().__init__() self.name = type(self).__name__ @@ -709,7 +727,7 @@ def __init__(self): self.x = 1 @fixtures.custom_prefix('unittests/resources/checks') - class T1(HelloTest): + class T1(hellotest): def __init__(self): super().__init__() self.name = type(self).__name__ From c0f2f535ad9826d7fe0518cde7094896a2f18b5e Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Fri, 22 Jan 2021 13:13:08 +0100 Subject: [PATCH 09/49] Streamline vars framework --- reframe/core/attributes.py | 72 ++++++++++++++++++++++++++++---------- reframe/core/meta.py | 2 +- reframe/core/pipeline.py | 12 +------ reframe/core/variables.py | 57 ++++++++++++++++++++---------- 4 files changed, 93 insertions(+), 50 deletions(-) diff --git a/reframe/core/attributes.py b/reframe/core/attributes.py index 87256c4ef7..c881b1903c 100644 --- a/reframe/core/attributes.py +++ b/reframe/core/attributes.py @@ -4,33 +4,36 @@ # SPDX-License-Identifier: BSD-3-Clause # -# Functionality to build extensible attribute spaces through class directives. +# Abstract base classes to build extensible attribute spaces through class +# directives. # + import abc + class LocalAttrSpace(metaclass=abc.ABCMeta): '''Local attribute space of a regression test. - Stores the regression test attributes defined in the test class body. - In the context of this class, a regression test attribute is an instance - of the class _TOBESET#########. This local attribute space is populated - during the test class body execution through the add_attr method, and the - different attributes are stored under the _attr member. This class - should be thought of as a temporary storage for this local attribute space, - before the full final space of the attribute is built. + Temporary storage for test attributes defined in the test class body + through directives. This local attribute space is populated during the + test class body execution through the add_attr method, which must be + exposed as a directive in the + :class:`reframe.core.pipeline.RegressionTest`. - Example: In the pseudo-code below, the local parameter space of A is {P0}, - and the local parameter space of B is {P1}. However, the final parameter - space of A is still {P0}, and the final parameter space of B is {P0, P1}. + Example: In the pseudo-code below, the local attribute space of A is {P0}, + and the local attribute space of B is {P1}. However, the final attribute + space of A is still {P0}, and the final attribute space of B is {P0, P1}. + The :func:`new_attr` directive is simply a function pointer to the + :func:`add_attr` method. .. code:: python class A(RegressionTest): - -> define parameter P0 with value X. + new_attr('P0') class B(A): - -> define parameter P1 with value Y. + new_attr('P1') ''' def __init__(self): @@ -57,21 +60,39 @@ def items(self): class AttrSpace(metaclass=abc.ABCMeta): + '''Attribute space of a regression test. + The final attribute space may be built by inheriting attribute spaces from + the base classes, and extended with the information stored in the local + attribute space of the target class. In this context, the target class is + simply the regression test class where the attribute space is to be built. + + To allow for this inheritance and extension of the attribute space, this + class must define the names under which the local and final attribute + spaces are inserted in the target classes. + ''' @property @abc.abstractmethod def localAttrSpaceName(self): - pass + '''Name of the local attribute space in the target class. + + Name under which the local attribute space is stored in the + :class`reframe.core.pipeline.RegressionTest` class. + ''' @property @abc.abstractmethod def localAttrSpaceCls(self): - pass + '''Type of the expected local attribute space.''' @property @abc.abstractmethod def attrSpaceName(self): - pass + '''Name of the attribute space in the target class. + + Name under which the attribute space is stored in the + :class`reframe.core.pipeline.RegressionTest` class. + ''' def __init__(self, target_cls=None): self._attr = {} @@ -89,22 +110,31 @@ def __init__(self, target_cls=None): # Sanity checkings on the resulting AttrSpace self.sanity(target_cls) + # Attach the AttrSpace to the target class + if target_cls: + setattr(target_cls, self.attrSpaceName, self) + def assert_target_cls(self, cls): + '''Assert the target class has a valid local attribute space.''' + assert hasattr(cls, self.localAttrSpaceName) assert isinstance(getattr(cls, self.localAttrSpaceName), self.localAttrSpaceCls) @abc.abstractmethod def inherit(self, cls): - pass + '''Inherit the attribute spaces from the parent classes of cls.''' @abc.abstractmethod def extend(self, cls): - pass + '''Extend the attribute space with the local attribute space.''' def sanity(self, cls): + '''Sanity checks post-creation of the attribute space. - # Namespace clashing with cls disallowed + By default, we make illegal to have the any attribute in the AttrSpace + that clashes with a member of the target class. + ''' target_namespace = set(dir(cls)) for key in self._attr: if key in target_namespace: @@ -113,5 +143,9 @@ def sanity(self, cls): f'{cls.__qualname__!r}' ) + @abc.abstractmethod + def insert(self, obj, objtype=None): + '''Insert the attributes from the AttrSpace as members of the test.''' + def items(self): return self._attr.items() diff --git a/reframe/core/meta.py b/reframe/core/meta.py index 2154fe83d6..6391529048 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -42,7 +42,7 @@ def __init__(cls, name, bases, namespace, **kwargs): # Build the parameter and variable spaces cls._rfm_param_space = parameters.ParamSpace(cls) - cls._rfm_var_space = variables.VarSpace(cls) + variables.VarSpace(cls) # Set up the hooks for the pipeline stages based on the _rfm_attach # attribute; all dependencies will be resolved first in the post-setup diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index b55c9c2e94..c6e6b2056b 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -716,7 +716,7 @@ def __new__(cls, *args, _rfm_use_params=False, **kwargs): cls._init_params(obj, _rfm_use_params) # Insert the user variables - cls._insert_vars(obj) + cls._rfm_var_space.insert(obj, cls) # Create a test name from the class name and the constructor's # arguments @@ -779,16 +779,6 @@ def _init_params(cls, obj, use_params=False): for key in cls._rfm_param_space.params: setattr(obj, key, None) - @classmethod - def _insert_vars(cls, obj): - '''Insert the vars in the regression test.''' - - for name, var in cls._rfm_var_space.items(): - setattr(cls, name, var.field(*var.types)) - getattr(cls, name).__set_name__(obj, name) - if not var.is_undef(): - setattr(obj, name, var.value) - def _append_parameters_to_name(self): if self._rfm_param_space.params: return '_' + '_'.join([str(self.__dict__[key]) diff --git a/reframe/core/variables.py b/reframe/core/variables.py index 9f60e703bd..9c153e6feb 100644 --- a/reframe/core/variables.py +++ b/reframe/core/variables.py @@ -7,7 +7,7 @@ # Functionality to build extensible attribute spaces into ReFrame tests. # -import reframe.core.attributes as ReframeAttributes +import reframe.core.attributes as attributes import reframe.core.fields as fields class _TestVar: @@ -42,7 +42,7 @@ def define(self, value): self.value = value -class LocalVarSpace(ReframeAttributes.LocalAttrSpace): +class LocalVarSpace(attributes.LocalAttrSpace): '''Local variable space of a regression test. Stores the input from the var directives executed in the class body of @@ -60,6 +60,8 @@ def add_attr(self, name, *types, **kwargs): If the ``value`` argument is not provided, the variable is considered *declared* but not *defined*. Note that a variable must be defined before is referenced in the regression test. + This method may only be called in the main class body, otherwise its + behavior is undefined. :param name: the variable name. :param types: the supported types for the variable. @@ -68,7 +70,7 @@ def add_attr(self, name, *types, **kwargs): If no field argument is provided, it defaults to a TypedField (see :class `reframe.core.fields`). Note that the field validator provided by this argument must derive from - :class `reframe.core.fields.Field`. + :class:`reframe.core.fields.Field`. ''' self[name] = _TestVar(name, *types, **kwargs) @@ -79,8 +81,10 @@ def undefine_attr(self, name): since it permits to remove any default values that may have been defined for a variable in any of the parent classes. Effectively, this will force the user of the library to provide the required value for a - variable. However, a variable flagged as ``required`` which is not + variable. However, a variable flagged as *required* which is not referenced in the regression test is implemented as a no-op. + This method may only be called in the main class body, otherwise its + behavior is undefined. :param name: the name of the required variable. ''' @@ -89,6 +93,9 @@ def undefine_attr(self, name): def define_attr(self, name, value): '''Assign a value to a regression test variable. + This method may only be called in the main class body, otherwise its + behavior is undefined. + :param name: the variable name. :param value: the value assigned to the variable. ''' @@ -99,27 +106,24 @@ def vars(self): return self._attr -class VarSpace(ReframeAttributes.AttrSpace): +class VarSpace(attributes.AttrSpace): '''Variable space of a regression test. Store the variables of a regression test. This variable space is stored - in the regression test class under the class attribute `_rfm_var_space`. + in the regression test class under the class attribute ``_rfm_var_space``. A target class can be provided to the :func:`__init__` method, which is the regression test where the VarSpace is to be built. During this call to :func:`__init__`, the VarSpace inherits all the VarSpace from the base classes of the target class. After this, the VarSpace is extended with the information from the local variable space, which is stored under the - target class' attribute '_rfm_local_var_space'. If no target class is + target class' attribute ``_rfm_local_var_space``. If no target class is provided, the VarSpace is simply initialized as empty. ''' localAttrSpaceName = '_rfm_local_var_space' localAttrSpaceCls = LocalVarSpace attrSpaceName = '_rfm_var_space' - def __init__(self, target_cls=None): - super().__init__(target_cls) - def inherit(self, cls): '''Inherit the VarSpace from the bases.''' for base in filter(lambda x: hasattr(x, self.attrSpaceName), @@ -133,15 +137,15 @@ def join(self, other_var_space): # Make doubly declared vars illegal. Note that this will be # triggered when inheriting from multiple RegressionTest classes. - if key in self._attr: + if key in self.vars: raise ValueError( f'cannot redeclare a variable ({key})' ) - self._attr[key] = var + self.vars[key] = var def extend(self, cls): - '''Extend the VarSpace with the content in the LocalVarSpace + '''Extend the VarSpace with the content in the LocalVarSpace. Merge the VarSpace inherited from the base classes with the LocalVarSpace. Note that the LocalVarSpace can also contain @@ -156,32 +160,47 @@ def extend(self, cls): for key, var in localVarSpace.items(): # Disable redeclaring a variable - if key in self._attr: + if key in self.vars: raise ValueError( f'cannot redeclare a variable ({key})' ) - self._attr[key] = var + self.vars[key] = var # Undefine the vars as indicated by the local var space for key in localVarSpace.undefined: self._check_var_is_declared(key) - self._attr[key].undefine() + self.vars[key].undefine() # Define the vars as indicated by the local var space for key, val in localVarSpace.definitions.items(): self._check_var_is_declared(key) - self._attr[key].define(val) + self.vars[key].define(val) def _check_var_is_declared(self, key): - if key not in self._attr: + if key not in self.vars: raise ValueError( f'var {key!r} has not been declared' ) + def insert(self, obj, cls): + '''Insert the vars in the regression test. + + :param obj: The test object. + :param cls: The test class. + ''' + + for name, var in self.items(): + setattr(cls, name, var.field(*var.types)) + getattr(cls, name).__set_name__(obj, name) + + # If the var is defined, set its value + if not var.is_undef(): + setattr(obj, name, var.value) + @property def vars(self): return self._attr def undefined_vars(self): - return list(filter(lambda x: self._attr[x].is_undef(), self._attr)) + return list(filter(lambda x: self.vars[x].is_undef(), self.vars)) From d3bceb975e5ec2810fd6a84ad912c90fe8cc75c9 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Fri, 22 Jan 2021 13:19:46 +0100 Subject: [PATCH 10/49] Fix PEP8 complaints --- reframe/core/attributes.py | 24 ++++++++++++------------ reframe/core/pipeline.py | 9 ++++----- reframe/core/variables.py | 2 ++ 3 files changed, 18 insertions(+), 17 deletions(-) diff --git a/reframe/core/attributes.py b/reframe/core/attributes.py index c881b1903c..ec9ecc2775 100644 --- a/reframe/core/attributes.py +++ b/reframe/core/attributes.py @@ -98,21 +98,21 @@ def __init__(self, target_cls=None): self._attr = {} if target_cls: - # Assert the AttrSpace can be built for the target_cls - self.assert_target_cls(target_cls) + # Assert the AttrSpace can be built for the target_cls + self.assert_target_cls(target_cls) - # Inherit AttrSpaces from the base clases - self.inherit(target_cls) + # Inherit AttrSpaces from the base clases + self.inherit(target_cls) - # Extend the AttrSpace with the LocalAttrSpace - self.extend(target_cls) + # Extend the AttrSpace with the LocalAttrSpace + self.extend(target_cls) - # Sanity checkings on the resulting AttrSpace - self.sanity(target_cls) + # Sanity checkings on the resulting AttrSpace + self.sanity(target_cls) - # Attach the AttrSpace to the target class - if target_cls: - setattr(target_cls, self.attrSpaceName, self) + # Attach the AttrSpace to the target class + if target_cls: + setattr(target_cls, self.attrSpaceName, self) def assert_target_cls(self, cls): '''Assert the target class has a valid local attribute space.''' @@ -145,7 +145,7 @@ def sanity(self, cls): @abc.abstractmethod def insert(self, obj, objtype=None): - '''Insert the attributes from the AttrSpace as members of the test.''' + '''Insert the attributes from the AttrSpace as members of the test.''' def items(self): return self._attr.items() diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index c6e6b2056b..1053e9d401 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -333,8 +333,7 @@ def pipeline_hooks(cls): #: :class:`reframe.core.containers.ContainerPlatform`. #: :default: :class:`None`. var('container_platform', type(None), - field=ContainerPlatformField, value=None - ) + field=ContainerPlatformField, value=None) #: .. versionadded:: 3.0 #: @@ -543,8 +542,7 @@ def pipeline_hooks(cls): #: The measurement unit is required. The user should explicitly #: specify :class:`None` if no unit is available. var('reference', typ.Tuple[object, object, object, object], - field=fields.ScopedDictField, value={} - ) + field=fields.ScopedDictField, value={}) # FIXME: There is not way currently to express tuples of `float`s or # `None`s, so we just use the very generic `object` @@ -584,7 +582,8 @@ def pipeline_hooks(cls): #: `) as values. #: :class:`None` is also allowed. #: :default: :class:`None` - var('perf_patterns', typ.Dict[str, _DeferredExpression], type(None), value=None) + var('perf_patterns', typ.Dict[str, _DeferredExpression], + type(None), value=None) #: List of modules to be loaded before running this test. #: diff --git a/reframe/core/variables.py b/reframe/core/variables.py index 9c153e6feb..4ea119456e 100644 --- a/reframe/core/variables.py +++ b/reframe/core/variables.py @@ -7,9 +7,11 @@ # Functionality to build extensible attribute spaces into ReFrame tests. # + import reframe.core.attributes as attributes import reframe.core.fields as fields + class _TestVar: '''Regression test variable. From 38b6d6e65cfc1289d9fb572b03ead908d66312b7 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Fri, 22 Jan 2021 16:25:38 +0100 Subject: [PATCH 11/49] Update parameters to work with the attribute bases --- reframe/core/attributes.py | 28 ++++-- reframe/core/meta.py | 4 +- reframe/core/parameters.py | 178 ++++++++++++++++--------------------- reframe/core/pipeline.py | 38 +------- reframe/core/variables.py | 30 ++++--- 5 files changed, 122 insertions(+), 156 deletions(-) diff --git a/reframe/core/attributes.py b/reframe/core/attributes.py index ec9ecc2775..90d83e3775 100644 --- a/reframe/core/attributes.py +++ b/reframe/core/attributes.py @@ -19,7 +19,7 @@ class LocalAttrSpace(metaclass=abc.ABCMeta): through directives. This local attribute space is populated during the test class body execution through the add_attr method, which must be exposed as a directive in the - :class:`reframe.core.pipeline.RegressionTest`. + :class `reframe.core.pipeline.RegressionTest`. Example: In the pseudo-code below, the local attribute space of A is {P0}, and the local attribute space of B is {P1}. However, the final attribute @@ -70,14 +70,19 @@ class AttrSpace(metaclass=abc.ABCMeta): To allow for this inheritance and extension of the attribute space, this class must define the names under which the local and final attribute spaces are inserted in the target classes. + + If a target class is provided, the constructor will attach the AttrSpace + instance into the target class with the class attribute name as defined + in ``attrSpaceName``. ''' + @property @abc.abstractmethod def localAttrSpaceName(self): '''Name of the local attribute space in the target class. Name under which the local attribute space is stored in the - :class`reframe.core.pipeline.RegressionTest` class. + :class `reframe.core.pipeline.RegressionTest` class. ''' @property @@ -91,7 +96,7 @@ def attrSpaceName(self): '''Name of the attribute space in the target class. Name under which the attribute space is stored in the - :class`reframe.core.pipeline.RegressionTest` class. + :class `reframe.core.pipeline.RegressionTest` class. ''' def __init__(self, target_cls=None): @@ -121,9 +126,17 @@ def assert_target_cls(self, cls): assert isinstance(getattr(cls, self.localAttrSpaceName), self.localAttrSpaceCls) - @abc.abstractmethod def inherit(self, cls): - '''Inherit the attribute spaces from the parent classes of cls.''' + '''Inherit the AttrSpace from the bases.''' + + for base in filter(lambda x: hasattr(x, self.attrSpaceName), + cls.__bases__): + assert isinstance(getattr(base, self.attrSpaceName), type(self)) + self.join(getattr(base, self.attrSpaceName)) + + @abc.abstractmethod + def join(self, other): + '''Join other AttrSpace with the current one.''' @abc.abstractmethod def extend(self, cls): @@ -135,12 +148,13 @@ def sanity(self, cls): By default, we make illegal to have the any attribute in the AttrSpace that clashes with a member of the target class. ''' + target_namespace = set(dir(cls)) for key in self._attr: if key in target_namespace: raise ValueError( - f'{key!r} clashes with the namespace from ' - f'{cls.__qualname__!r}' + f'{key!r} clashes with other attribute present in the' + f' namespace of {cls.__qualname__!r}' ) @abc.abstractmethod diff --git a/reframe/core/meta.py b/reframe/core/meta.py index 6391529048..0586da4a31 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -24,7 +24,7 @@ def __prepare__(cls, name, bases, **kwargs): # Directive to add a regression test parameter directly in the # class body as: `parameter('P0', 0,1,2,3)`. - namespace['parameter'] = local_param_space.add_param + namespace['parameter'] = local_param_space.add_attr # Regression test var space defined at the class level local_var_space = variables.LocalVarSpace() @@ -41,8 +41,8 @@ def __init__(cls, name, bases, namespace, **kwargs): super().__init__(name, bases, namespace, **kwargs) # Build the parameter and variable spaces - cls._rfm_param_space = parameters.ParamSpace(cls) variables.VarSpace(cls) + parameters.ParamSpace(cls) # Set up the hooks for the pipeline stages based on the _rfm_attach # attribute; all dependencies will be resolved first in the post-setup diff --git a/reframe/core/parameters.py b/reframe/core/parameters.py index 6f6642c1c9..57990de8f3 100644 --- a/reframe/core/parameters.py +++ b/reframe/core/parameters.py @@ -10,7 +10,7 @@ import functools import itertools -from reframe.core.exceptions import ReframeSyntaxError +import reframe.core.attributes as attributes class _TestParameter: @@ -42,46 +42,13 @@ def filter_params(x): self.filter_params = filter_params -class LocalParamSpace: +class LocalParamSpace(attributes.LocalAttrSpace): '''Local parameter space of a regression test. Stores all the regression test parameters defined in the test class body. - In the context of this class, a regression test parameter is an instance - of the class _TestParameter. This local parameter space is populated - during the test class body execution through the add_param method, and the - different parameters are stored under the _params attribute. This class - should be thought of as a temporary storage for this local parameter space, - before the full final parameter space is built. - - Example: In the pseudo-code below, the local parameter space of A is {P0}, - and the local parameter space of B is {P1}. However, the final parameter - space of A is still {P0}, and the final parameter space of B is {P0, P1}. - - .. code:: python - - class A(RegressionTest): - -> define parameter P0 with value X. - - class B(A): - -> define parameter P1 with value Y. ''' - def __init__(self): - self._params = {} - - def __getattr__(self, name): - # Delegate any unknown attribute access to the actual parameter space - return getattr(self._params, name) - - def __setitem__(self, name, value): - if name not in self._params: - self._params[name] = value - else: - raise ValueError( - f'parameter {name!r} already defined in this class' - ) - - def add_param(self, name, *values, **kwargs): + def add_attr(self, name, *values, **kwargs): '''Insert or modify a regression test parameter. This method must be called directly in the class body. For each @@ -98,40 +65,25 @@ def add_param(self, name, *values, **kwargs): @property def params(self): - return self._params + return self._attr - def items(self): - return self._params.items() - -class ParamSpace: +class ParamSpace(attributes.AttrSpace): ''' Regression test parameter space Host class for the parameter space of a regresion test. The parameter - space is stored as a dictionary (self._params), where the keys are the + space is stored as a dictionary (self.params), where the keys are the parameter names and the values are tuples with all the available values for each parameter. The __init__ method in this class takes an optional argument (target_class), which is the regression test class where the - parameter space is to be built. If this target class is provided, the - __init__ method performs three main steps. These are (in order of exec) - the inheritance of the parameter spaces from the direct parent classes, - the extension of the inherited parameter space with the local parameter - space (this must be an instance of - :class `reframe.core.parameters.LocalParamSpace`), and lastly, a check to - ensure that none of the parameter names clashes with any of the class - attributes existing in the target class. If no target class is provided, - the parameter space is initialized as empty. After the parameter space is - set, a parameter space iterator is created under self.__unique_iter, which - acts as an internal control variable that tracks the usage of this - parameter space. This iterator walks through all possible parameter - combinations and cannot be restored after reaching exhaustion. This unique - iterator is made available as read-only through cls.unique_iterator and it - may be used by an external class to track the usage of the parameter space - (i.e. the - :class `reframe.core.pipeline.RegressionTest` can use this unique iterator - to ensure that each point of the parameter space has only been instantiated - once). The length of this iterator matches the value returned by the member - function __len__. + parameter space is to e inserted as the ``_rfm_param_space`` class + attribute. If no target class is provided, the parameter space is + initialized as empty. After the parameter space is set, a parameter space + iterator is created under self.__unique_iter, which acts as an internal + control variable that tracks the usage of this parameter space. This + iterator walks through all possible parameter combinations and cannot be + restored after reaching exhaustion. The length of this iterator matches + the value returned by the member function __len__. :param target_cls: the class where the full parameter space is to be built. @@ -142,42 +94,18 @@ class ParamSpace: the target class. ''' + localAttrSpaceName = '_rfm_local_param_space' + localAttrSpaceCls = LocalParamSpace + attrSpaceName = '_rfm_param_space' + def __init__(self, target_cls=None): - self._params = {} - - # If a target class is provided, build the param space for it - if target_cls: - assert hasattr(target_cls, '_rfm_local_param_space') - assert isinstance(target_cls._rfm_local_param_space, - LocalParamSpace) - - # Inherit the parameter spaces from the direct parent classes - for base in filter(lambda x: hasattr(x, 'param_space'), - target_cls.__bases__): - self.join(base._rfm_param_space) - - # Extend the parameter space with the local parameter space - for name, p in target_cls._rfm_local_param_space.items(): - self._params[name] = ( - p.filter_params(self._params.get(name, ())) + p.values - ) - - # Make sure that none of the parameters clashes with the target - # class namespace - target_namespace = set(dir(target_cls)) - for key in self._params: - if key in target_namespace: - raise ReframeSyntaxError( - f'parameter {key!r} clashes with other variables' - f' present in the namespace from class ' - f'{target_cls.__qualname__!r}' - ) + super().__init__(target_cls) # Internal parameter space usage tracker self.__unique_iter = iter(self) def join(self, other): - '''Join two parameter spaces into one + '''Join other parameter space into the current one. Join two different parameter spaces into a single one. Both parameter spaces must be an instance ot the ParamSpace class. This method will @@ -190,28 +118,73 @@ def join(self, other): # With multiple inheritance, a single parameter # could be doubly defined and lead to repeated # values - if (key in self._params and - self._params[key] != () and + if (key in self.params and + self.params[key] != () and other.params[key] != ()): raise ReframeSyntaxError(f'parameter space conflict: ' f'parameter {key!r} already defined ' f'in {b.__qualname__!r}') - self._params[key] = ( - other.params.get(key, ()) + self._params.get(key, ()) + self.params[key] = ( + other.params.get(key, ()) + self.params.get(key, ()) ) + def extend(self, cls): + '''Extend the parameter space with the local parameter space.''' + + for name, p in cls._rfm_local_param_space.items(): + self.params[name] = ( + p.filter_params(self.params.get(name, ())) + p.values + ) + + def insert(self, obj, cls=None, use_params=False): + '''Insert the params in the regression test. + + Create and initialize the regression test parameters as object + attributes. The values assigned to these parameters exclusively depend + on the use_params argument. If this is set to True, the current object + uses the parameter space iterator (see + :class `reframe.core.pipeline.RegressionTest` and consumes a set of + parameter values (i.e. a point in the parameter space). Contrarily, if + use_params is False, the regression test parameters are initialized as + None. + + :param obj: The test object. + :param cls: The test class. + :param use_param: bool that dictates whether an instance of the + :class `reframe.core.pipeline.RegressionTest` is to use the + parameter values defined in the parameter space. + + ''' + # Set the values of the test parameters (if any) + if use_params and self.params: + try: + # Consume the parameter space iterator + param_values = next(self.unique_iter) + for index, key in enumerate(self.params): + setattr(obj, key, param_values[index]) + + except StopIteration as no_params: + raise RuntimeError( + f'exhausted parameter space' + ) from no_params + + else: + # Otherwise init the params as None + for key in self.params: + setattr(obj, key, None) + def __iter__(self): '''Create a generator object to iterate over the parameter space :return: generator object to iterate over the parameter space. ''' - yield from itertools.product(*(p for p in self._params.values())) + yield from itertools.product(*(p for p in self.params.values())) @property def params(self): - return self._params + return self._attr @property def unique_iter(self): @@ -230,17 +203,18 @@ def __len__(self): the parameter space), the returned parameter space length is 0. :return: length of the parameter space + ''' - if not self._params: + if not self.params: return 1 return functools.reduce( lambda x, y: x*y, - (len(p) for p in self._params.values()) + (len(p) for p in self.params.values()) ) def __getitem__(self, key): - return self._params.get(key, ()) + return self.params.get(key, ()) def is_empty(self): - return self._params == {} + return self.params == {} diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 1053e9d401..6677b42a6f 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -175,7 +175,7 @@ def pipeline_hooks(cls): #: The name of the test. #: #: :type: string that can contain any character except ``/`` - name = fields.TypedField(typ.Str[r'[^\/]+']) + var('name', typ.Str[r'[^\/]+']) #: List of programming environments supported by this test. #: @@ -711,11 +711,9 @@ def pipeline_hooks(cls): def __new__(cls, *args, _rfm_use_params=False, **kwargs): obj = super().__new__(cls) - # Set the test parameters in the object - cls._init_params(obj, _rfm_use_params) - - # Insert the user variables + # Insert the var & param spaces cls._rfm_var_space.insert(obj, cls) + cls._rfm_param_space.insert(obj, cls, _rfm_use_params) # Create a test name from the class name and the constructor's # arguments @@ -748,36 +746,6 @@ def __new__(cls, *args, _rfm_use_params=False, **kwargs): def __init__(self): pass - @classmethod - def _init_params(cls, obj, use_params=False): - '''Attach the test parameters as class attributes. - - Create and initialize the regression test parameters as object - attributes. The values assigned to these parameters exclusively depend - on the use_params argument. If this is set to True, the current object - uses the parameter space iterator (see - :class `reframe.core.pipeline.RegressionTest` and consumes a set of - parameter values (i.e. a point in the parameter space). Contrarily, if - use_params is False, the regression test parameters are initialized as - None. - - :param use_param: bool that dictates whether an instance of the - :class `reframe.core.pipeline.RegressionTest` is to use the - parameter values defined in the parameter space. - - :meta private: - ''' - # Set the values of the test parameters (if any) - if use_params and cls._rfm_param_space.params: - # Consume the parameter space iterator - param_values = next(cls._rfm_param_space.unique_iter) - for index, key in enumerate(cls._rfm_param_space.params): - setattr(obj, key, param_values[index]) - else: - # Otherwise init the params as None - for key in cls._rfm_param_space.params: - setattr(obj, key, None) - def _append_parameters_to_name(self): if self._rfm_param_space.params: return '_' + '_'.join([str(self.__dict__[key]) diff --git a/reframe/core/variables.py b/reframe/core/variables.py index 4ea119456e..1887b77eb2 100644 --- a/reframe/core/variables.py +++ b/reframe/core/variables.py @@ -51,6 +51,7 @@ class LocalVarSpace(attributes.LocalAttrSpace): the regression test. This local variable space is later used during the instantiation of the VarSpace class to extend the final variable space. ''' + def __init__(self): super().__init__() self.undefined = set() @@ -72,7 +73,12 @@ def add_attr(self, name, *types, **kwargs): If no field argument is provided, it defaults to a TypedField (see :class `reframe.core.fields`). Note that the field validator provided by this argument must derive from - :class:`reframe.core.fields.Field`. + :class `reframe.core.fields.Field`. + + .. seealso:: + + :ref:`directives` + ''' self[name] = _TestVar(name, *types, **kwargs) @@ -89,6 +95,11 @@ def undefine_attr(self, name): behavior is undefined. :param name: the name of the required variable. + + .. seealso:: + + :ref:`directives` + ''' self.undefined.add(name) @@ -100,6 +111,11 @@ def define_attr(self, name, value): :param name: the variable name. :param value: the value assigned to the variable. + + .. seealso:: + + :ref:`directives` + ''' self.definitions[name] = value @@ -122,20 +138,14 @@ class VarSpace(attributes.AttrSpace): target class' attribute ``_rfm_local_var_space``. If no target class is provided, the VarSpace is simply initialized as empty. ''' + localAttrSpaceName = '_rfm_local_var_space' localAttrSpaceCls = LocalVarSpace attrSpaceName = '_rfm_var_space' - def inherit(self, cls): - '''Inherit the VarSpace from the bases.''' - for base in filter(lambda x: hasattr(x, self.attrSpaceName), - cls.__bases__): - assert isinstance(getattr(base, self.attrSpaceName), VarSpace) - self.join(getattr(base, self.attrSpaceName)) - - def join(self, other_var_space): + def join(self, other): '''Join an existing VarSpace into the current one.''' - for key, var in other_var_space.items(): + for key, var in other.items(): # Make doubly declared vars illegal. Note that this will be # triggered when inheriting from multiple RegressionTest classes. From 3a9560033c229e1fce7548c864018bbe47f37967 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Fri, 22 Jan 2021 18:03:36 +0100 Subject: [PATCH 12/49] Extend pipeline unit tests --- unittests/test_parameters.py | 5 +++-- unittests/test_pipeline.py | 12 ++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/unittests/test_parameters.py b/unittests/test_parameters.py index 56e85e955e..3c9dbf76b4 100644 --- a/unittests/test_parameters.py +++ b/unittests/test_parameters.py @@ -1,4 +1,4 @@ -# Copyright 2016-2020 Swiss National Supercomputing Centre (CSCS/ETH Zurich) +# Copyright 2016-2021 Swiss National Supercomputing Centre (CSCS/ETH Zurich) # ReFrame Project Developers. See the top-level LICENSE file for details. # # SPDX-License-Identifier: BSD-3-Clause @@ -7,6 +7,7 @@ import pytest import inspect + import reframe as rfm @@ -137,7 +138,7 @@ class MyTest(ExtendParams): assert test.P1 is None assert test.P2 is None - with pytest.raises(StopIteration): + with pytest.raises(RuntimeError): test = MyTest(_rfm_use_params=True) diff --git a/unittests/test_pipeline.py b/unittests/test_pipeline.py index 2a9cf0dfc3..406626f3e2 100644 --- a/unittests/test_pipeline.py +++ b/unittests/test_pipeline.py @@ -614,6 +614,18 @@ def x(self): assert test.var == 3 +def test_multiple_inheritance(hellotest): + with pytest.raises(ValueError): + class MyTest(rfm.RunOnlyRegressionTest, hellotest): + pass + + +def test_extend_after_instantiation(hellotest): + inst = hellotest() + with pytest.raises(ValueError): + class MyTest(hellotest): + pass + def test_inherited_hooks(hellotest, local_exec_ctx): @fixtures.custom_prefix('unittests/resources/checks') class BaseTest(hellotest): From a46f1a9fc27ba2550c5eb320af345f37de504a76 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Fri, 22 Jan 2021 19:19:09 +0100 Subject: [PATCH 13/49] Add unit tests for vars --- reframe/core/attributes.py | 2 +- unittests/test_variables.py | 116 ++++++++++++++++++++++++++++++++++++ 2 files changed, 117 insertions(+), 1 deletion(-) create mode 100644 unittests/test_variables.py diff --git a/reframe/core/attributes.py b/reframe/core/attributes.py index 90d83e3775..c9bb085ce5 100644 --- a/reframe/core/attributes.py +++ b/reframe/core/attributes.py @@ -152,7 +152,7 @@ def sanity(self, cls): target_namespace = set(dir(cls)) for key in self._attr: if key in target_namespace: - raise ValueError( + raise NameError( f'{key!r} clashes with other attribute present in the' f' namespace of {cls.__qualname__!r}' ) diff --git a/unittests/test_variables.py b/unittests/test_variables.py new file mode 100644 index 0000000000..64c71ec20a --- /dev/null +++ b/unittests/test_variables.py @@ -0,0 +1,116 @@ +# Copyright 2016-2021 Swiss National Supercomputing Centre (CSCS/ETH Zurich) +# ReFrame Project Developers. See the top-level LICENSE file for details. +# +# SPDX-License-Identifier: BSD-3-Clause + + +import pytest + + +import reframe as rfm +from reframe.core.fields import Field + + +@pytest.fixture +def novars(): + class NoVars(rfm.RegressionTest): + pass + yield NoVars + + +@pytest.fixture +def onevar(novars): + class OneVar(novars): + var('foo', int, value=10) + yield OneVar + + +def test_custom_var(onevar): + assert not hasattr(onevar, 'foo') + inst = onevar() + assert hasattr(onevar, 'foo') + assert isinstance(onevar.foo, Field) + assert hasattr(inst, 'foo') + assert inst.foo == 10 + + +def test_instantiate_and_inherit(novars): + inst = novars() + with pytest.raises(NameError): + class MyTest(novars): + pass + + +def test_redeclare_var_clash(novars): + with pytest.raises(ValueError): + class MyTest(novars): + var('name', str) + + +def test_inheritance_clash(novars): + class MyMixin(rfm.RegressionMixin): + var('name', str) + + with pytest.raises(ValueError): + class MyTest(novars, MyMixin): + pass + + +def test_namespace_clash(novars): + with pytest.raises(NameError): + class MyTest(novars): + var('current_environ', str) + + +def test_set_var(onevar): + class MyTest(onevar): + set_var('foo', 4) + + inst = MyTest() + assert not hasattr(onevar, 'foo') + assert hasattr(MyTest, 'foo') + assert hasattr(inst, 'foo') + assert inst.foo == 4 + + +def test_var_type(onevar): + class MyTest(onevar): + set_var('foo', 'bananas') + + with pytest.raises(TypeError): + inst = MyTest() + + +def test_set_undef(novars): + with pytest.raises(ValueError): + class MyTest(novars): + set_var('foo', 4) + + +def test_require_var(onevar): + class MyTest(onevar): + require_var('foo') + def __init__(self): + print(self.foo) + + with pytest.raises(AttributeError): + inst = MyTest() + + +def test_required_var_not_present(onevar): + class MyTest(onevar): + require_var('foo') + def __init__(self): + pass + + try: + inst = MyTest() + except Exception: + pytest.fail('class instantiation failed') + + +def test_require_undef(novars): + with pytest.raises(ValueError): + class MyTest(novars): + require_var('foo') + From 650d06470d64962fbea2994fe522ae2db198fd57 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Fri, 22 Jan 2021 20:05:10 +0100 Subject: [PATCH 14/49] Update pipeline test to catch NameError --- unittests/test_pipeline.py | 4 ++-- unittests/test_variables.py | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/unittests/test_pipeline.py b/unittests/test_pipeline.py index 406626f3e2..48529a63ba 100644 --- a/unittests/test_pipeline.py +++ b/unittests/test_pipeline.py @@ -78,7 +78,6 @@ def user_system(temp_runtime): yield generic_system - @pytest.fixture def local_exec_ctx(generic_system): partition = fixtures.partition_by_name('default') @@ -622,10 +621,11 @@ class MyTest(rfm.RunOnlyRegressionTest, hellotest): def test_extend_after_instantiation(hellotest): inst = hellotest() - with pytest.raises(ValueError): + with pytest.raises(NameError): class MyTest(hellotest): pass + def test_inherited_hooks(hellotest, local_exec_ctx): @fixtures.custom_prefix('unittests/resources/checks') class BaseTest(hellotest): diff --git a/unittests/test_variables.py b/unittests/test_variables.py index 64c71ec20a..26fd454005 100644 --- a/unittests/test_variables.py +++ b/unittests/test_variables.py @@ -90,6 +90,7 @@ class MyTest(novars): def test_require_var(onevar): class MyTest(onevar): require_var('foo') + def __init__(self): print(self.foo) @@ -100,6 +101,7 @@ def __init__(self): def test_required_var_not_present(onevar): class MyTest(onevar): require_var('foo') + def __init__(self): pass @@ -113,4 +115,3 @@ def test_require_undef(novars): with pytest.raises(ValueError): class MyTest(novars): require_var('foo') - From b2e03e7276cebd9eb9e25ebe057037467637cc91 Mon Sep 17 00:00:00 2001 From: Javier Otero <71280927+jjotero@users.noreply.github.com> Date: Fri, 22 Jan 2021 23:21:37 +0100 Subject: [PATCH 15/49] First doc draft for the var directives --- docs/regression_test_api.rst | 34 ++++++++++++++++++++++++++++ reframe/core/custom_vars.py | 44 ++++++++++++++++++++++++++++++++++++ reframe/core/parameters.py | 10 ++++---- reframe/core/pipeline.py | 4 ++++ reframe/core/variables.py | 40 ++++++++------------------------ 5 files changed, 96 insertions(+), 36 deletions(-) create mode 100644 reframe/core/custom_vars.py diff --git a/docs/regression_test_api.rst b/docs/regression_test_api.rst index decdb82ba5..6c5abdcde9 100644 --- a/docs/regression_test_api.rst +++ b/docs/regression_test_api.rst @@ -87,6 +87,40 @@ For instance, continuing with the example above, one could override the :func:`_ This only has an effect if used with ``inherit_params=True``. +.. py:function:: reframe.core.pipeline.RegressionTest.var(name, *types, value=None, field=None) + + Inserts a new regression test variable. + If the ``value`` argument is not provided, the variable is considered *declared* but not *defined*. + Thus, a variable may not be declared more than once. However, it is possible to alter a variable's value after it was declared by using the :func:`set_var` and :func:`require_var` directives. + Note that a variable must be defined before is referenced in the regression test. Otherwise, an :py:exc:`AttributeError` will be raised. + + :param name: the variable name. + :param types: the supported types for the variable. + :param value: the default value assigned to the variable. + :param field: the field validator to be used for this variable. + If no field argument is provided, it defaults to + :class:`reframe.core.fields.TypedField`. + Note that the field validator provided by this argument must derive from + :class:`reframe.core.fields.Field`. + + +.. py:function:: reframe.core.pipeline.RegressionTest.set_var(name, value) + + Assign a value to a regression test variable. The variable must have been defined in a parent class using the :func:`var` directive. + + :param name: the variable name. + :param value: the value assigned to the variable. + + +.. py:function:: reframe.core.pipeline.RegressionTest.require_var(name) + + Undefine a regression test variable. The variable must have been defined in a parent class using the :func:`var` directive. + This method is particularly useful when writing a test library, since it permits to remove any default values that may have been defined for a variable in any of the parent classes. + Effectively, this will force the user of the library to provide the required value for a variable. + However, a variable flagged as *required* which is not referenced in the regression test is implemented as a no-op. + + :param name: the name of the required variable. + Environments and Systems diff --git a/reframe/core/custom_vars.py b/reframe/core/custom_vars.py new file mode 100644 index 0000000000..fb853d2025 --- /dev/null +++ b/reframe/core/custom_vars.py @@ -0,0 +1,44 @@ +# Copyright 2016-2020 Swiss National Supercomputing Centre (CSCS/ETH Zurich) +# ReFrame Project Developers. See the top-level LICENSE file for details. +# +# SPDX-License-Identifier: BSD-3-Clause + +# +# Functionality to add custom variables into a regression test. +# + +class _TestVar: + '''Regression test variable class.''' + def __init__(self, name, *types, required=False): + self.name = name + self.types = types + self.required = required + + +class LocalVarSpace: + '''This is the equivalent to LocalParamSpace in the Perhaps we could inherit this from the LocalParamSpace class. + ''' + def __init__(self): + self._vars = {} + + def __getattr__(self, name, value): + if name not in self._vars: + self._vars[name] = value + else: + raise ValueError( + f'var {name!r} already defined in this class' + ) + + def add_var(self, name, *types, **kwargs): + self[name] = _TestVar(name, *types, **kwargs) + + @property + def vars(self): + return self._vars + + def items(self): + return self._vars.items() + + +class VarSpace: + pass diff --git a/reframe/core/parameters.py b/reframe/core/parameters.py index 57990de8f3..665541d62c 100644 --- a/reframe/core/parameters.py +++ b/reframe/core/parameters.py @@ -51,10 +51,8 @@ class LocalParamSpace(attributes.LocalAttrSpace): def add_attr(self, name, *values, **kwargs): '''Insert or modify a regression test parameter. - This method must be called directly in the class body. For each - regression test class definition, this function may only be called - once per parameter. Calling this method during or after the class - instantiation has an undefined behavior. + This method may only be called in the main class body. Otherwise, its + behavior is undefined. .. seealso:: @@ -145,7 +143,7 @@ def insert(self, obj, cls=None, use_params=False): attributes. The values assigned to these parameters exclusively depend on the use_params argument. If this is set to True, the current object uses the parameter space iterator (see - :class `reframe.core.pipeline.RegressionTest` and consumes a set of + :class:`reframe.core.pipeline.RegressionTest` and consumes a set of parameter values (i.e. a point in the parameter space). Contrarily, if use_params is False, the regression test parameters are initialized as None. @@ -153,7 +151,7 @@ def insert(self, obj, cls=None, use_params=False): :param obj: The test object. :param cls: The test class. :param use_param: bool that dictates whether an instance of the - :class `reframe.core.pipeline.RegressionTest` is to use the + :class:`reframe.core.pipeline.RegressionTest` is to use the parameter values defined in the parameter space. ''' diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 6677b42a6f..f3a671c70b 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -145,6 +145,10 @@ class RegressionTest(jsonext.JSONSerializable, metaclass=RegressionTestMeta): This class provides the implementation of the pipeline phases that the regression test goes through during its lifetime. + .. warning:: + .. versionchanged:: 3.5 + Multiple inheritance with a shared common ancestor is not allowed. + .. note:: .. versionchanged:: 2.19 Base constructor takes no arguments. diff --git a/reframe/core/variables.py b/reframe/core/variables.py index 1887b77eb2..97c98581d7 100644 --- a/reframe/core/variables.py +++ b/reframe/core/variables.py @@ -18,9 +18,12 @@ class _TestVar: Buffer to store a regression test variable into either a VarSpace or a LocalVarSpace. ''' - def __init__(self, name, *types, field=fields.TypedField, **kwargs): - if 'value' in kwargs: - self.define(kwargs.get('value')) + def __init__(self, name, *types, field=None, value=None): + if field is None: + field = fields.TypedField + + if value is not None: + self.define(value) else: self.undefine() @@ -60,21 +63,9 @@ def __init__(self): def add_attr(self, name, *types, **kwargs): '''Declare a new regression test variable. - If the ``value`` argument is not provided, the variable is considered - *declared* but not *defined*. Note that a variable must be defined - before is referenced in the regression test. - This method may only be called in the main class body, otherwise its + This method may only be called in the main class body. Otherwise, its behavior is undefined. - :param name: the variable name. - :param types: the supported types for the variable. - :param value: the default value assigned to the variable. - :params field: the field validator to be used for this variable. - If no field argument is provided, it defaults to a TypedField - (see :class `reframe.core.fields`). Note that the field validator - provided by this argument must derive from - :class `reframe.core.fields.Field`. - .. seealso:: :ref:`directives` @@ -85,17 +76,9 @@ def add_attr(self, name, *types, **kwargs): def undefine_attr(self, name): '''Undefine a variable previously declared in a parent class. - This method is particularly useful when writing a test library, - since it permits to remove any default values that may have been - defined for a variable in any of the parent classes. Effectively, this - will force the user of the library to provide the required value for a - variable. However, a variable flagged as *required* which is not - referenced in the regression test is implemented as a no-op. - This method may only be called in the main class body, otherwise its + This method may only be called in the main class body. Otherwise, its behavior is undefined. - :param name: the name of the required variable. - .. seealso:: :ref:`directives` @@ -104,14 +87,11 @@ def undefine_attr(self, name): self.undefined.add(name) def define_attr(self, name, value): - '''Assign a value to a regression test variable. + '''Assign a value to a previously declared regression test variable. - This method may only be called in the main class body, otherwise its + This method may only be called in the main class body. Otherwise, its behavior is undefined. - :param name: the variable name. - :param value: the value assigned to the variable. - .. seealso:: :ref:`directives` From 49bc4c5214fa4fbd0911cb611f769e9dea7671b3 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Mon, 25 Jan 2021 18:09:36 +0100 Subject: [PATCH 16/49] Add nameclash check across all attributes --- reframe/core/attributes.py | 9 ++++---- reframe/core/custom_vars.py | 44 ------------------------------------ reframe/core/meta.py | 12 +++++++--- reframe/core/parameters.py | 4 ++-- reframe/core/variables.py | 9 ++++---- unittests/test_parameters.py | 9 ++++++++ 6 files changed, 30 insertions(+), 57 deletions(-) delete mode 100644 reframe/core/custom_vars.py diff --git a/reframe/core/attributes.py b/reframe/core/attributes.py index c9bb085ce5..39db4e1da1 100644 --- a/reframe/core/attributes.py +++ b/reframe/core/attributes.py @@ -99,7 +99,7 @@ def attrSpaceName(self): :class `reframe.core.pipeline.RegressionTest` class. ''' - def __init__(self, target_cls=None): + def __init__(self, target_cls=None, target_namespace=None): self._attr = {} if target_cls: @@ -113,7 +113,7 @@ def __init__(self, target_cls=None): self.extend(target_cls) # Sanity checkings on the resulting AttrSpace - self.sanity(target_cls) + self.sanity(target_cls, target_namespace) # Attach the AttrSpace to the target class if target_cls: @@ -142,14 +142,15 @@ def join(self, other): def extend(self, cls): '''Extend the attribute space with the local attribute space.''' - def sanity(self, cls): + def sanity(self, cls, target_namespace=None): '''Sanity checks post-creation of the attribute space. By default, we make illegal to have the any attribute in the AttrSpace that clashes with a member of the target class. ''' + if target_namespace is None: + target_namespace = set(dir(cls)) - target_namespace = set(dir(cls)) for key in self._attr: if key in target_namespace: raise NameError( diff --git a/reframe/core/custom_vars.py b/reframe/core/custom_vars.py deleted file mode 100644 index fb853d2025..0000000000 --- a/reframe/core/custom_vars.py +++ /dev/null @@ -1,44 +0,0 @@ -# Copyright 2016-2020 Swiss National Supercomputing Centre (CSCS/ETH Zurich) -# ReFrame Project Developers. See the top-level LICENSE file for details. -# -# SPDX-License-Identifier: BSD-3-Clause - -# -# Functionality to add custom variables into a regression test. -# - -class _TestVar: - '''Regression test variable class.''' - def __init__(self, name, *types, required=False): - self.name = name - self.types = types - self.required = required - - -class LocalVarSpace: - '''This is the equivalent to LocalParamSpace in the Perhaps we could inherit this from the LocalParamSpace class. - ''' - def __init__(self): - self._vars = {} - - def __getattr__(self, name, value): - if name not in self._vars: - self._vars[name] = value - else: - raise ValueError( - f'var {name!r} already defined in this class' - ) - - def add_var(self, name, *types, **kwargs): - self[name] = _TestVar(name, *types, **kwargs) - - @property - def vars(self): - return self._vars - - def items(self): - return self._vars.items() - - -class VarSpace: - pass diff --git a/reframe/core/meta.py b/reframe/core/meta.py index 0586da4a31..2bd41fab6f 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -40,9 +40,15 @@ def __prepare__(cls, name, bases, **kwargs): def __init__(cls, name, bases, namespace, **kwargs): super().__init__(name, bases, namespace, **kwargs) - # Build the parameter and variable spaces - variables.VarSpace(cls) - parameters.ParamSpace(cls) + # Create a target namespace to test for attribute name clashes + target_namespace = set(dir(cls)) + + # Build the var space and extend the target namespace + variables.VarSpace(cls, target_namespace) + target_namespace.update(cls._rfm_var_space.vars) + + # Build the parameter space + parameters.ParamSpace(cls, target_namespace) # Set up the hooks for the pipeline stages based on the _rfm_attach # attribute; all dependencies will be resolved first in the post-setup diff --git a/reframe/core/parameters.py b/reframe/core/parameters.py index 665541d62c..2e4b9669bc 100644 --- a/reframe/core/parameters.py +++ b/reframe/core/parameters.py @@ -96,8 +96,8 @@ class ParamSpace(attributes.AttrSpace): localAttrSpaceCls = LocalParamSpace attrSpaceName = '_rfm_param_space' - def __init__(self, target_cls=None): - super().__init__(target_cls) + def __init__(self, target_cls=None, target_namespace=None): + super().__init__(target_cls, target_namespace) # Internal parameter space usage tracker self.__unique_iter = iter(self) diff --git a/reframe/core/variables.py b/reframe/core/variables.py index 97c98581d7..614ced3e0e 100644 --- a/reframe/core/variables.py +++ b/reframe/core/variables.py @@ -18,12 +18,12 @@ class _TestVar: Buffer to store a regression test variable into either a VarSpace or a LocalVarSpace. ''' - def __init__(self, name, *types, field=None, value=None): + def __init__(self, name, *types, field=None, **kwargs): if field is None: field = fields.TypedField - if value is not None: - self.define(value) + if 'value' in kwargs: + self.define(kwargs.get('value')) else: self.undefine() @@ -131,7 +131,8 @@ def join(self, other): # triggered when inheriting from multiple RegressionTest classes. if key in self.vars: raise ValueError( - f'cannot redeclare a variable ({key})' + f'attribute {key!r} is declared in more than one of the ' + f'parent classes' ) self.vars[key] = var diff --git a/unittests/test_parameters.py b/unittests/test_parameters.py index 3c9dbf76b4..640348eb7f 100644 --- a/unittests/test_parameters.py +++ b/unittests/test_parameters.py @@ -169,3 +169,12 @@ def test_parameterized_test_is_incompatible(): class MyTest(TwoParams): def __init__(self, var): pass + + +def test_namespace_clash(): + class Spam(rfm.RegressionTest): + var('foo', int, 1) + + with pytest.raises(NameError): + class Ham(Spam): + parameter('foo', 1) From 6941c41606091e9d3747ca17f54cc254015e1912 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Tue, 26 Jan 2021 12:13:42 +0100 Subject: [PATCH 17/49] Extend unit tests --- reframe/core/parameters.py | 6 +++--- reframe/core/variables.py | 5 ++--- unittests/test_parameters.py | 19 +++++++++++++++++++ unittests/test_variables.py | 32 ++++++++++++++++++++++++++++++-- 4 files changed, 54 insertions(+), 8 deletions(-) diff --git a/reframe/core/parameters.py b/reframe/core/parameters.py index 2e4b9669bc..53d60674a4 100644 --- a/reframe/core/parameters.py +++ b/reframe/core/parameters.py @@ -120,9 +120,9 @@ def join(self, other): self.params[key] != () and other.params[key] != ()): - raise ReframeSyntaxError(f'parameter space conflict: ' - f'parameter {key!r} already defined ' - f'in {b.__qualname__!r}') + raise ValueError(f'parameter space conflict: ' + f'parameter {key!r} is defined in more than ' + f'one base class') self.params[key] = ( other.params.get(key, ()) + self.params.get(key, ()) diff --git a/reframe/core/variables.py b/reframe/core/variables.py index 614ced3e0e..d117dea93a 100644 --- a/reframe/core/variables.py +++ b/reframe/core/variables.py @@ -4,7 +4,7 @@ # SPDX-License-Identifier: BSD-3-Clause # -# Functionality to build extensible attribute spaces into ReFrame tests. +# Functionality to build extensible variable spaces into ReFrame tests. # @@ -15,8 +15,7 @@ class _TestVar: '''Regression test variable. - Buffer to store a regression test variable into either a VarSpace or a - LocalVarSpace. + Buffer to store a regression test variable declared through directives. ''' def __init__(self, name, *types, field=None, **kwargs): if field is None: diff --git a/unittests/test_parameters.py b/unittests/test_parameters.py index 640348eb7f..c8aae1f98c 100644 --- a/unittests/test_parameters.py +++ b/unittests/test_parameters.py @@ -171,6 +171,18 @@ def __init__(self, var): pass +def test_param_space_clash(): + class Spam(rfm.RegressionMixin): + parameter('P0', 1) + + class Ham(rfm.RegressionMixin): + parameter('P0', 2) + + with pytest.raises(ValueError): + class Eggs(Spam, Ham): + '''Trigger error from param name clashing.''' + + def test_namespace_clash(): class Spam(rfm.RegressionTest): var('foo', int, 1) @@ -178,3 +190,10 @@ class Spam(rfm.RegressionTest): with pytest.raises(NameError): class Ham(Spam): parameter('foo', 1) + + +def test_double_declare(): + with pytest.raises(ValueError): + class MyTest(rfm.RegressionTest): + parameter('P0', 1, 2, 3) + parameter('P0') diff --git a/unittests/test_variables.py b/unittests/test_variables.py index 26fd454005..8a112c9bf8 100644 --- a/unittests/test_variables.py +++ b/unittests/test_variables.py @@ -38,7 +38,7 @@ def test_instantiate_and_inherit(novars): inst = novars() with pytest.raises(NameError): class MyTest(novars): - pass + '''Error from name clashing''' def test_redeclare_var_clash(novars): @@ -53,7 +53,26 @@ class MyMixin(rfm.RegressionMixin): with pytest.raises(ValueError): class MyTest(novars, MyMixin): - pass + '''Trigger error from inheritance clash.''' + + +def test_var_space_clash(): + class Spam(rfm.RegressionMixin): + var('v0', int, value=1) + + class Ham(rfm.RegressionMixin): + var('v0', int, value=2) + + with pytest.raises(ValueError): + class Eggs(Spam, Ham): + '''Trigger error from var name clashing.''' + + +def test_double_declare(): + with pytest.raises(ValueError): + class MyTest(rfm.RegressionTest): + var('v0', int, value=1) + var('v0', float, value=0.5) def test_namespace_clash(novars): @@ -115,3 +134,12 @@ def test_require_undef(novars): with pytest.raises(ValueError): class MyTest(novars): require_var('foo') + + +def test_invalid_field(): + class Foo: + '''An invalid descriptor''' + + with pytest.raises(ValueError): + class MyTest(rfm.RegressionTest): + var('a', int, value=4, field=Foo) From 73689dab5a3cced884835d7cb850ab494600326b Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Tue, 26 Jan 2021 14:27:12 +0100 Subject: [PATCH 18/49] Add error catch --- reframe/core/variables.py | 18 +++++++++++++++++- unittests/test_variables.py | 7 +++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/reframe/core/variables.py b/reframe/core/variables.py index d117dea93a..2fadaf9cb3 100644 --- a/reframe/core/variables.py +++ b/reframe/core/variables.py @@ -70,6 +70,7 @@ def add_attr(self, name, *types, **kwargs): :ref:`directives` ''' + self._is_logged(name) self[name] = _TestVar(name, *types, **kwargs) def undefine_attr(self, name): @@ -83,6 +84,7 @@ def undefine_attr(self, name): :ref:`directives` ''' + self._is_logged(name) self.undefined.add(name) def define_attr(self, name, value): @@ -96,8 +98,20 @@ def define_attr(self, name, value): :ref:`directives` ''' + self._is_logged(name) self.definitions[name] = value + def _is_logged(self, name): + ''' Check if an action has been registered for this variable. + + Calling more than one of the directives above on the same variable + does not make sense. + ''' + if any(name in x for x in [self.vars, self.undefined, self.definitions]): + raise ValueError(f'cannot specify more than one action on variable' + f' {name!r} in the same class' + ) + @property def vars(self): return self._attr @@ -144,7 +158,9 @@ def extend(self, cls): define and undefine actions on existing vars. Thus, since it does not make sense to define and undefine a var in the same class, the order on which the define and undefine functions - are called is not preserved. + are called is not preserved. In fact, applying more than one + of these actions on the same var for the same local var space + is disallowed. ''' localVarSpace = getattr(cls, self.localAttrSpaceName) diff --git a/unittests/test_variables.py b/unittests/test_variables.py index 8a112c9bf8..9a1fdfa95e 100644 --- a/unittests/test_variables.py +++ b/unittests/test_variables.py @@ -75,6 +75,13 @@ class MyTest(rfm.RegressionTest): var('v0', float, value=0.5) +def test_double_action_on_var(): + with pytest.raises(ValueError): + class MyTest(rfm.RegressionTest): + set_var('v0', 2) + var('v0', int, value=2) + + def test_namespace_clash(novars): with pytest.raises(NameError): class MyTest(novars): From e124e40f90ad281d237928820a8ec84ff30779a2 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Tue, 26 Jan 2021 16:42:25 +0100 Subject: [PATCH 19/49] Fix PEP8 complaints --- reframe/core/variables.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/reframe/core/variables.py b/reframe/core/variables.py index 2fadaf9cb3..d7ce901e83 100644 --- a/reframe/core/variables.py +++ b/reframe/core/variables.py @@ -107,9 +107,12 @@ def _is_logged(self, name): Calling more than one of the directives above on the same variable does not make sense. ''' - if any(name in x for x in [self.vars, self.undefined, self.definitions]): - raise ValueError(f'cannot specify more than one action on variable' - f' {name!r} in the same class' + if any(name in x for x in [self.vars, + self.undefined, + self.definitions]): + raise ValueError( + f'cannot specify more than one action on variable' + f' {name!r} in the same class' ) @property From a1724a72f3cf1235199906f80beef7bf9da02555 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Fri, 5 Feb 2021 11:38:28 +0100 Subject: [PATCH 20/49] Rename attribute base classes as namespace --- reframe/core/attributes.py | 166 ------------------------------------ reframe/core/meta.py | 8 +- reframe/core/namespaces.py | 168 +++++++++++++++++++++++++++++++++++++ reframe/core/parameters.py | 25 ++++-- reframe/core/variables.py | 37 ++++---- 5 files changed, 209 insertions(+), 195 deletions(-) delete mode 100644 reframe/core/attributes.py create mode 100644 reframe/core/namespaces.py diff --git a/reframe/core/attributes.py b/reframe/core/attributes.py deleted file mode 100644 index 39db4e1da1..0000000000 --- a/reframe/core/attributes.py +++ /dev/null @@ -1,166 +0,0 @@ -# Copyright 2016-2021 Swiss National Supercomputing Centre (CSCS/ETH Zurich) -# ReFrame Project Developers. See the top-level LICENSE file for details. -# -# SPDX-License-Identifier: BSD-3-Clause - -# -# Abstract base classes to build extensible attribute spaces through class -# directives. -# - - -import abc - - -class LocalAttrSpace(metaclass=abc.ABCMeta): - '''Local attribute space of a regression test. - - Temporary storage for test attributes defined in the test class body - through directives. This local attribute space is populated during the - test class body execution through the add_attr method, which must be - exposed as a directive in the - :class `reframe.core.pipeline.RegressionTest`. - - Example: In the pseudo-code below, the local attribute space of A is {P0}, - and the local attribute space of B is {P1}. However, the final attribute - space of A is still {P0}, and the final attribute space of B is {P0, P1}. - The :func:`new_attr` directive is simply a function pointer to the - :func:`add_attr` method. - - .. code:: python - - class A(RegressionTest): - new_attr('P0') - - class B(A): - new_attr('P1') - ''' - - def __init__(self): - self._attr = {} - - def __getattr__(self, name): - return getattr(self._attr, name) - - def __setitem__(self, name, value): - if name not in self._attr: - self._attr[name] = value - else: - raise ValueError( - f'attribute {name!r} already defined in this class' - ) - - @abc.abstractmethod - def add_attr(self, name, *args, **kwargs): - '''Insert a new attribute in the local attribute space.''' - pass - - def items(self): - return self._attr.items() - - -class AttrSpace(metaclass=abc.ABCMeta): - '''Attribute space of a regression test. - - The final attribute space may be built by inheriting attribute spaces from - the base classes, and extended with the information stored in the local - attribute space of the target class. In this context, the target class is - simply the regression test class where the attribute space is to be built. - - To allow for this inheritance and extension of the attribute space, this - class must define the names under which the local and final attribute - spaces are inserted in the target classes. - - If a target class is provided, the constructor will attach the AttrSpace - instance into the target class with the class attribute name as defined - in ``attrSpaceName``. - ''' - - @property - @abc.abstractmethod - def localAttrSpaceName(self): - '''Name of the local attribute space in the target class. - - Name under which the local attribute space is stored in the - :class `reframe.core.pipeline.RegressionTest` class. - ''' - - @property - @abc.abstractmethod - def localAttrSpaceCls(self): - '''Type of the expected local attribute space.''' - - @property - @abc.abstractmethod - def attrSpaceName(self): - '''Name of the attribute space in the target class. - - Name under which the attribute space is stored in the - :class `reframe.core.pipeline.RegressionTest` class. - ''' - - def __init__(self, target_cls=None, target_namespace=None): - self._attr = {} - if target_cls: - - # Assert the AttrSpace can be built for the target_cls - self.assert_target_cls(target_cls) - - # Inherit AttrSpaces from the base clases - self.inherit(target_cls) - - # Extend the AttrSpace with the LocalAttrSpace - self.extend(target_cls) - - # Sanity checkings on the resulting AttrSpace - self.sanity(target_cls, target_namespace) - - # Attach the AttrSpace to the target class - if target_cls: - setattr(target_cls, self.attrSpaceName, self) - - def assert_target_cls(self, cls): - '''Assert the target class has a valid local attribute space.''' - - assert hasattr(cls, self.localAttrSpaceName) - assert isinstance(getattr(cls, self.localAttrSpaceName), - self.localAttrSpaceCls) - - def inherit(self, cls): - '''Inherit the AttrSpace from the bases.''' - - for base in filter(lambda x: hasattr(x, self.attrSpaceName), - cls.__bases__): - assert isinstance(getattr(base, self.attrSpaceName), type(self)) - self.join(getattr(base, self.attrSpaceName)) - - @abc.abstractmethod - def join(self, other): - '''Join other AttrSpace with the current one.''' - - @abc.abstractmethod - def extend(self, cls): - '''Extend the attribute space with the local attribute space.''' - - def sanity(self, cls, target_namespace=None): - '''Sanity checks post-creation of the attribute space. - - By default, we make illegal to have the any attribute in the AttrSpace - that clashes with a member of the target class. - ''' - if target_namespace is None: - target_namespace = set(dir(cls)) - - for key in self._attr: - if key in target_namespace: - raise NameError( - f'{key!r} clashes with other attribute present in the' - f' namespace of {cls.__qualname__!r}' - ) - - @abc.abstractmethod - def insert(self, obj, objtype=None): - '''Insert the attributes from the AttrSpace as members of the test.''' - - def items(self): - return self._attr.items() diff --git a/reframe/core/meta.py b/reframe/core/meta.py index 2bd41fab6f..8b68446359 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -24,16 +24,16 @@ def __prepare__(cls, name, bases, **kwargs): # Directive to add a regression test parameter directly in the # class body as: `parameter('P0', 0,1,2,3)`. - namespace['parameter'] = local_param_space.add_attr + namespace['parameter'] = local_param_space.add # Regression test var space defined at the class level local_var_space = variables.LocalVarSpace() namespace['_rfm_local_var_space'] = local_var_space # Directives to add/modify a regression test variable - namespace['var'] = local_var_space.add_attr - namespace['require_var'] = local_var_space.undefine_attr - namespace['set_var'] = local_var_space.define_attr + namespace['var'] = local_var_space.add + namespace['require_var'] = local_var_space.undefine + namespace['set_var'] = local_var_space.define return namespace diff --git a/reframe/core/namespaces.py b/reframe/core/namespaces.py new file mode 100644 index 0000000000..a4cb15e6bc --- /dev/null +++ b/reframe/core/namespaces.py @@ -0,0 +1,168 @@ +# Copyright 2016-2021 Swiss National Supercomputing Centre (CSCS/ETH Zurich) +# ReFrame Project Developers. See the top-level LICENSE file for details. +# +# SPDX-License-Identifier: BSD-3-Clause + +# +# Abstract base classes to build extensible namespaces through class +# directives. +# + + +import abc + + +class LocalNamespace(metaclass=abc.ABCMeta): + '''Local namespace of a regression test. + + Temporary storage for test attributes defined in the test class body + through directives. This local namespace is populated during the + test class body execution through the add method, which must be + exposed as a directive in the + :class `reframe.core.pipeline.RegressionTest`. + + Example: In the pseudo-code below, the local namespace of A is {P0}, + and the local namespace of B is {P1}. However, the final namespace + of A is still {P0}, and the final namespace of B is {P0, P1}. + The :func:`new` directive is simply an alias to the + :func:`add` method. + + .. code:: python + + class A(RegressionTest): + new('P0') + + class B(A): + new('P1') + ''' + + def __init__(self): + self._namespace = {} + + def __getattr__(self, name): + return getattr(self._namespace, name) + + def __setitem__(self, name, value): + if name not in self._namespace: + self._namespace[name] = value + else: + self._raise_namespace_clash(name) + + def _raise_namespace_clash(self, name): + raise ValueError( + f'{name!r} is already present in the local namespace' + ) + + @abc.abstractmethod + def add(self, name, *args, **kwargs): + '''Insert a new item in the local namespace.''' + + def items(self): + return self._namespace.items() + + +class Namespace(metaclass=abc.ABCMeta): + '''Namespace of a regression test. + + The final namespace may be built by inheriting namespaces from + the base classes, and extended with the information stored in the local + namespace of the target class. In this context, the target class is + simply the regression test class where the namespace is to be built. + + To allow for this inheritance and extension of the namespace, this + class must define the names under which the local and final namespaces + are inserted in the target classes. + + If a target class is provided, the constructor will attach the Namespace + instance into the target class with the class attribute name as defined + in ``namespace_name``. + ''' + + @property + @abc.abstractmethod + def local_namespace_name(self): + '''Name of the local namespace in the target class. + + Name under which the local namespace is stored in the + :class `reframe.core.pipeline.RegressionTest` class. + ''' + + @property + @abc.abstractmethod + def local_namespace_class(self): + '''Type of the expected local namespace.''' + + @property + @abc.abstractmethod + def namespace_name(self): + '''Name of the namespace in the target class. + + Name under which the namespace is stored in the + :class `reframe.core.pipeline.RegressionTest` class. + ''' + + def __init__(self, target_cls=None, target_namespace=None): + self._namespace = {} + if target_cls: + + # Assert the Namespace can be built for the target_cls + self.assert_target_cls(target_cls) + + # Inherit Namespaces from the base clases + self.inherit(target_cls) + + # Extend the Namespace with the LocalNamespace + self.extend(target_cls) + + # Sanity checkings on the resulting Namespace + self.sanity(target_cls, target_namespace) + + # Attach the Namespace to the target class + if target_cls: + setattr(target_cls, self.namespace_name, self) + + def assert_target_cls(self, cls): + '''Assert the target class has a valid local namespace.''' + + assert hasattr(cls, self.local_namespace_name) + assert isinstance(getattr(cls, self.local_namespace_name), + self.local_namespace_class) + + def inherit(self, cls): + '''Inherit the Namespaces from the bases.''' + + for base in filter(lambda x: hasattr(x, self.namespace_name), + cls.__bases__): + assert isinstance(getattr(base, self.namespace_name), type(self)) + self.join(getattr(base, self.namespace_name)) + + @abc.abstractmethod + def join(self, other): + '''Join other Namespace with the current one.''' + + @abc.abstractmethod + def extend(self, cls): + '''Extend the namespace with the local namespace.''' + + def sanity(self, cls, target_namespace=None): + '''Sanity checks post-creation of the namespace. + + By default, we make illegal to have the any item in the namespace + that clashes with a member of the target class. + ''' + if target_namespace is None: + target_namespace = set(dir(cls)) + + for key in self._namespace: + if key in target_namespace: + raise NameError( + f'{key!r} clashes with other class attribute from' + f' {cls.__qualname__!r}' + ) + + @abc.abstractmethod + def insert(self, obj, objtype=None): + '''Insert the items from the namespace as attributes of the test object.''' + + def items(self): + return self._namespace.items() diff --git a/reframe/core/parameters.py b/reframe/core/parameters.py index 08f76e9c50..d0af1bc1e3 100644 --- a/reframe/core/parameters.py +++ b/reframe/core/parameters.py @@ -10,7 +10,7 @@ import functools import itertools -import reframe.core.attributes as attributes +import reframe.core.namespaces as namespaces class _TestParameter: @@ -45,13 +45,13 @@ def filter_params(x): self.filter_params = filter_params -class LocalParamSpace(attributes.LocalAttrSpace): +class LocalParamSpace(namespaces.LocalNamespace): '''Local parameter space of a regression test. Stores all the regression test parameters defined in the test class body. ''' - def add_attr(self, name, values=None, **kwargs): + def add(self, name, values=None, **kwargs): '''Insert or modify a regression test parameter. This method may only be called in the main class body. Otherwise, its @@ -66,10 +66,15 @@ def add_attr(self, name, values=None, **kwargs): @property def params(self): - return self._attr + return self._namespace + + def _raise_namespace_clash(self, name): + raise ValueError( + f'{name!r} is already present in the local parameter space' + ) -class ParamSpace(attributes.AttrSpace): +class ParamSpace(namespaces.Namespace): ''' Regression test parameter space Host class for the parameter space of a regresion test. The parameter @@ -87,6 +92,8 @@ class ParamSpace(attributes.AttrSpace): the value returned by the member function __len__. :param target_cls: the class where the full parameter space is to be built. + :param target_namespace: a reference namespace to ensure that no name + clashes occur (see :class:`reframe.core.namespaces.Namespace`). .. note:: The __init__ method is aware of the implementation details of the @@ -95,9 +102,9 @@ class ParamSpace(attributes.AttrSpace): the target class. ''' - localAttrSpaceName = '_rfm_local_param_space' - localAttrSpaceCls = LocalParamSpace - attrSpaceName = '_rfm_param_space' + local_namespace_name = '_rfm_local_param_space' + local_namespace_class = LocalParamSpace + namespace_name = '_rfm_param_space' def __init__(self, target_cls=None, target_namespace=None): super().__init__(target_cls, target_namespace) @@ -185,7 +192,7 @@ def __iter__(self): @property def params(self): - return self._attr + return self._namespace @property def unique_iter(self): diff --git a/reframe/core/variables.py b/reframe/core/variables.py index d7ce901e83..4d22958b6f 100644 --- a/reframe/core/variables.py +++ b/reframe/core/variables.py @@ -8,7 +8,7 @@ # -import reframe.core.attributes as attributes +import reframe.core.namespaces as namespaces import reframe.core.fields as fields @@ -46,7 +46,7 @@ def define(self, value): self.value = value -class LocalVarSpace(attributes.LocalAttrSpace): +class LocalVarSpace(namespaces.LocalNamespace): '''Local variable space of a regression test. Stores the input from the var directives executed in the class body of @@ -59,7 +59,7 @@ def __init__(self): self.undefined = set() self.definitions = {} - def add_attr(self, name, *types, **kwargs): + def add(self, name, *types, **kwargs): '''Declare a new regression test variable. This method may only be called in the main class body. Otherwise, its @@ -73,7 +73,7 @@ def add_attr(self, name, *types, **kwargs): self._is_logged(name) self[name] = _TestVar(name, *types, **kwargs) - def undefine_attr(self, name): + def undefine(self, name): '''Undefine a variable previously declared in a parent class. This method may only be called in the main class body. Otherwise, its @@ -87,7 +87,7 @@ def undefine_attr(self, name): self._is_logged(name) self.undefined.add(name) - def define_attr(self, name, value): + def define(self, name, value): '''Assign a value to a previously declared regression test variable. This method may only be called in the main class body. Otherwise, its @@ -117,10 +117,15 @@ def _is_logged(self, name): @property def vars(self): - return self._attr + return self._namespace + def _raise_namespace_clash(self, name): + raise ValueError( + f'{name!r} is already present in the local variable space' + ) -class VarSpace(attributes.AttrSpace): + +class VarSpace(namespaces.Namespace): '''Variable space of a regression test. Store the variables of a regression test. This variable space is stored @@ -135,9 +140,9 @@ class VarSpace(attributes.AttrSpace): provided, the VarSpace is simply initialized as empty. ''' - localAttrSpaceName = '_rfm_local_var_space' - localAttrSpaceCls = LocalVarSpace - attrSpaceName = '_rfm_var_space' + local_namespace_name = '_rfm_local_var_space' + local_namespace_class = LocalVarSpace + namespace_name = '_rfm_var_space' def join(self, other): '''Join an existing VarSpace into the current one.''' @@ -147,7 +152,7 @@ def join(self, other): # triggered when inheriting from multiple RegressionTest classes. if key in self.vars: raise ValueError( - f'attribute {key!r} is declared in more than one of the ' + f'variable {key!r} is declared in more than one of the ' f'parent classes' ) @@ -165,10 +170,10 @@ def extend(self, cls): of these actions on the same var for the same local var space is disallowed. ''' - localVarSpace = getattr(cls, self.localAttrSpaceName) + local_varspace = getattr(cls, self.local_namespace_name) # Extend the VarSpace - for key, var in localVarSpace.items(): + for key, var in local_varspace.items(): # Disable redeclaring a variable if key in self.vars: @@ -179,12 +184,12 @@ def extend(self, cls): self.vars[key] = var # Undefine the vars as indicated by the local var space - for key in localVarSpace.undefined: + for key in local_varspace.undefined: self._check_var_is_declared(key) self.vars[key].undefine() # Define the vars as indicated by the local var space - for key, val in localVarSpace.definitions.items(): + for key, val in local_varspace.definitions.items(): self._check_var_is_declared(key) self.vars[key].define(val) @@ -211,7 +216,7 @@ def insert(self, obj, cls): @property def vars(self): - return self._attr + return self._namespace def undefined_vars(self): return list(filter(lambda x: self.vars[x].is_undef(), self.vars)) From 0dc8556b8eb6792952ad200e08cfc63f85a50e81 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Fri, 5 Feb 2021 18:54:25 +0100 Subject: [PATCH 21/49] Address PR comments --- reframe/core/fields.py | 7 ++-- reframe/core/meta.py | 17 +++++---- reframe/core/namespaces.py | 64 +++++++++++++++++---------------- reframe/core/parameters.py | 24 ++++++++----- reframe/core/pipeline.py | 19 +++++++--- reframe/core/variables.py | 68 ++++++++++++++++++------------------ unittests/test_parameters.py | 2 +- unittests/test_pipeline.py | 2 +- unittests/test_variables.py | 4 +-- 9 files changed, 111 insertions(+), 96 deletions(-) diff --git a/reframe/core/fields.py b/reframe/core/fields.py index 119ec91cb6..8001abbb22 100644 --- a/reframe/core/fields.py +++ b/reframe/core/fields.py @@ -40,8 +40,8 @@ def __set__(self, obj, value): class TypedField(Field): '''Stores a field of predefined type''' - def __init__(self, *types): - self._types = types + def __init__(self, main_type, *other_types): + self._types = (main_type,) + other_types if not all(isinstance(t, type) for t in self._types): raise TypeError('{0} is not a sequence of types'. format(self._types)) @@ -57,9 +57,6 @@ def __set__(self, obj, value): self._check_type(value) super().__set__(obj, value) - def supports_type(self, value): - return value in self._types - class ConstantField(Field): '''Holds a constant. diff --git a/reframe/core/meta.py b/reframe/core/meta.py index 8b68446359..76c8097dcb 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -22,33 +22,32 @@ def __prepare__(cls, name, bases, **kwargs): local_param_space = parameters.LocalParamSpace() namespace['_rfm_local_param_space'] = local_param_space - # Directive to add a regression test parameter directly in the + # Directive to insert a regression test parameter directly in the # class body as: `parameter('P0', 0,1,2,3)`. - namespace['parameter'] = local_param_space.add + namespace['parameter'] = local_param_space.insert # Regression test var space defined at the class level local_var_space = variables.LocalVarSpace() namespace['_rfm_local_var_space'] = local_var_space # Directives to add/modify a regression test variable - namespace['var'] = local_var_space.add + namespace['var'] = local_var_space.declare namespace['require_var'] = local_var_space.undefine namespace['set_var'] = local_var_space.define - return namespace def __init__(cls, name, bases, namespace, **kwargs): super().__init__(name, bases, namespace, **kwargs) - # Create a target namespace to test for attribute name clashes - target_namespace = set(dir(cls)) + # Create a set with the attribute names already in use. + used_attribute_names = set(dir(cls)) # Build the var space and extend the target namespace - variables.VarSpace(cls, target_namespace) - target_namespace.update(cls._rfm_var_space.vars) + variables.VarSpace(cls, used_attribute_names) + used_attribute_names.update(cls._rfm_var_space.vars) # Build the parameter space - parameters.ParamSpace(cls, target_namespace) + parameters.ParamSpace(cls, used_attribute_names) # Set up the hooks for the pipeline stages based on the _rfm_attach # attribute; all dependencies will be resolved first in the post-setup diff --git a/reframe/core/namespaces.py b/reframe/core/namespaces.py index a4cb15e6bc..96154778ae 100644 --- a/reframe/core/namespaces.py +++ b/reframe/core/namespaces.py @@ -19,21 +19,21 @@ class LocalNamespace(metaclass=abc.ABCMeta): through directives. This local namespace is populated during the test class body execution through the add method, which must be exposed as a directive in the - :class `reframe.core.pipeline.RegressionTest`. + :class:`reframe.core.pipeline.RegressionTest`. Example: In the pseudo-code below, the local namespace of A is {P0}, and the local namespace of B is {P1}. However, the final namespace of A is still {P0}, and the final namespace of B is {P0, P1}. - The :func:`new` directive is simply an alias to the + The :func:`var` directive is simply an alias to the :func:`add` method. .. code:: python class A(RegressionTest): - new('P0') + var('P0') class B(A): - new('P1') + var('P1') ''' def __init__(self): @@ -48,17 +48,9 @@ def __setitem__(self, name, value): else: self._raise_namespace_clash(name) - def _raise_namespace_clash(self, name): - raise ValueError( - f'{name!r} is already present in the local namespace' - ) - @abc.abstractmethod - def add(self, name, *args, **kwargs): - '''Insert a new item in the local namespace.''' - - def items(self): - return self._namespace.items() + def _raise_namespace_clash(self, name): + '''Raise an error if there is a namespace clash.''' class Namespace(metaclass=abc.ABCMeta): @@ -76,6 +68,17 @@ class must define the names under which the local and final namespaces If a target class is provided, the constructor will attach the Namespace instance into the target class with the class attribute name as defined in ``namespace_name``. + + Eventually, the items from a Namespace are injected as attributes of + the target class instance by the :func:`inject` method, which must be + called by the target class during its instantiation process. Also, a target + class may use more that one Namespace, which raises the need for name + checking across namespaces. Thus, the :func:`__init__` method accepts the + additional argument ``illegal_names``, which is a set of class attribute + names already in use by the target class or other namespaces from this + target class. Then, after the Namespace is built, if ``illegal_names`` is + provided, a sanity check is performed, ensuring that no name clashing + will occur during the target class instantiation process. ''' @property @@ -84,7 +87,7 @@ def local_namespace_name(self): '''Name of the local namespace in the target class. Name under which the local namespace is stored in the - :class `reframe.core.pipeline.RegressionTest` class. + :class:`reframe.core.pipeline.RegressionTest` class. ''' @property @@ -98,13 +101,12 @@ def namespace_name(self): '''Name of the namespace in the target class. Name under which the namespace is stored in the - :class `reframe.core.pipeline.RegressionTest` class. + :class:`reframe.core.pipeline.RegressionTest` class. ''' - def __init__(self, target_cls=None, target_namespace=None): + def __init__(self, target_cls=None, illegal_names=None): self._namespace = {} if target_cls: - # Assert the Namespace can be built for the target_cls self.assert_target_cls(target_cls) @@ -115,7 +117,7 @@ def __init__(self, target_cls=None, target_namespace=None): self.extend(target_cls) # Sanity checkings on the resulting Namespace - self.sanity(target_cls, target_namespace) + self.sanity(target_cls, illegal_names) # Attach the Namespace to the target class if target_cls: @@ -134,35 +136,37 @@ def inherit(self, cls): for base in filter(lambda x: hasattr(x, self.namespace_name), cls.__bases__): assert isinstance(getattr(base, self.namespace_name), type(self)) - self.join(getattr(base, self.namespace_name)) + self.join(getattr(base, self.namespace_name), cls) @abc.abstractmethod - def join(self, other): + def join(self, other, cls): '''Join other Namespace with the current one.''' @abc.abstractmethod def extend(self, cls): '''Extend the namespace with the local namespace.''' - def sanity(self, cls, target_namespace=None): + def sanity(self, cls, illegal_names=None): '''Sanity checks post-creation of the namespace. - By default, we make illegal to have the any item in the namespace + By default, we make illegal to have any item in the namespace that clashes with a member of the target class. ''' - if target_namespace is None: - target_namespace = set(dir(cls)) + if illegal_names is None: + illegal_names = set(dir(cls)) for key in self._namespace: - if key in target_namespace: - raise NameError( - f'{key!r} clashes with other class attribute from' + if key in illegal_names: + raise ValueError( + f'{key!r} already defined in class' f' {cls.__qualname__!r}' ) @abc.abstractmethod - def insert(self, obj, objtype=None): - '''Insert the items from the namespace as attributes of the test object.''' + def inject(self, obj, objtype=None): + '''Insert the items from the namespace as attributes of the object + ``obj``. + ''' def items(self): return self._namespace.items() diff --git a/reframe/core/parameters.py b/reframe/core/parameters.py index d0af1bc1e3..b7f1ec82d2 100644 --- a/reframe/core/parameters.py +++ b/reframe/core/parameters.py @@ -51,7 +51,7 @@ class LocalParamSpace(namespaces.LocalNamespace): Stores all the regression test parameters defined in the test class body. ''' - def add(self, name, values=None, **kwargs): + def insert(self, name, values=None, **kwargs): '''Insert or modify a regression test parameter. This method may only be called in the main class body. Otherwise, its @@ -61,6 +61,7 @@ def add(self, name, values=None, **kwargs): :ref:`directives` + .. versionadded:: 3.4 ''' self[name] = _TestParameter(name, values, **kwargs) @@ -112,7 +113,7 @@ def __init__(self, target_cls=None, target_namespace=None): # Internal parameter space usage tracker self.__unique_iter = iter(self) - def join(self, other): + def join(self, other, cls): '''Join other parameter space into the current one. Join two different parameter spaces into a single one. Both parameter @@ -120,7 +121,8 @@ def join(self, other): raise an error if a parameter is defined in the two parameter spaces to be merged. - :param other: instance of the ParamSpace class + :param other: instance of the ParamSpace class. + :param cls: the target class. ''' for key in other.params: # With multiple inheritance, a single parameter @@ -130,9 +132,11 @@ def join(self, other): self.params[key] != () and other.params[key] != ()): - raise ValueError(f'parameter space conflict: ' - f'parameter {key!r} is defined in more than ' - f'one base class') + raise ValueError( + f'parameter space conflict: ' + f'parameter {key!r} is defined in more than ' + f'one base class of class {cls.__qualname__!r}' + ) self.params[key] = ( other.params.get(key, ()) + self.params.get(key, ()) @@ -146,7 +150,7 @@ def extend(self, cls): p.filter_params(self.params.get(name, ())) + p.values ) - def insert(self, obj, cls=None, use_params=False): + def inject(self, obj, cls=None, use_params=False): '''Insert the params in the regression test. Create and initialize the regression test parameters as object @@ -175,8 +179,10 @@ def insert(self, obj, cls=None, use_params=False): except StopIteration as no_params: raise RuntimeError( - f'exhausted parameter space' - ) from no_params + f'exhausted parameter space: all possible parameter value' + f' combinations have been used for ' + f'{obj.__class__.__qualname__}' + ) from None else: # Otherwise init the params as None diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index f3a671c70b..e75311c5e4 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -135,6 +135,8 @@ class RegressionMixin(metaclass=RegressionTestMeta): the metaclass magic implemented in :class:`RegressionTestMeta`. Using this metaclass allows mixin classes to use powerful ReFrame features, such as hooks, parameters or variables. + + .. versionadded:: 3.5 ''' @@ -147,7 +149,7 @@ class RegressionTest(jsonext.JSONSerializable, metaclass=RegressionTestMeta): .. warning:: .. versionchanged:: 3.5 - Multiple inheritance with a shared common ancestor is not allowed. + Multiple inheritance with a shared common ancestor is not allowed. .. note:: .. versionchanged:: 2.19 @@ -187,7 +189,7 @@ def pipeline_hooks(cls): #: by this test. #: #: :type: :class:`List[str]` - #: :default: ``[]`` + #: :default: ``None`` #: #: .. note:: #: .. versionchanged:: 2.12 @@ -196,6 +198,9 @@ def pipeline_hooks(cls): #: .. versionchanged:: 2.17 #: Support for wildcards is dropped. #: + #: .. versionchanged:: 3.3 + #: Default value changed from ``[]`` to ``None``. + #: var('valid_prog_environs', typ.List[str], type(None), value=None) #: List of systems supported by this test. @@ -204,7 +209,11 @@ def pipeline_hooks(cls): #: ``*`` is an alias of ``*:*`` #: #: :type: :class:`List[str]` - #: :default: ``[]`` + #: :default: ``None`` + #: + #: .. versionchanged:: 3.3 + #: Default value changed from ``[]`` to ``None``. + #: var('valid_systems', typ.List[str], type(None), value=None) #: A detailed description of the test. @@ -716,8 +725,8 @@ def __new__(cls, *args, _rfm_use_params=False, **kwargs): obj = super().__new__(cls) # Insert the var & param spaces - cls._rfm_var_space.insert(obj, cls) - cls._rfm_param_space.insert(obj, cls, _rfm_use_params) + cls._rfm_var_space.inject(obj, cls) + cls._rfm_param_space.inject(obj, cls, _rfm_use_params) # Create a test name from the class name and the constructor's # arguments diff --git a/reframe/core/variables.py b/reframe/core/variables.py index 4d22958b6f..4bc64b242e 100644 --- a/reframe/core/variables.py +++ b/reframe/core/variables.py @@ -11,39 +11,36 @@ import reframe.core.namespaces as namespaces import reframe.core.fields as fields +class _UndefVar: + '''Custom type to flag a variable as undefined.''' class _TestVar: '''Regression test variable. Buffer to store a regression test variable declared through directives. ''' - def __init__(self, name, *types, field=None, **kwargs): - if field is None: - field = fields.TypedField + def __init__(self, name, *args, **kwargs): + self.field = kwargs.pop('field', fields.TypedField) + self.default_value = kwargs.pop('value', _UndefVar) - if 'value' in kwargs: - self.define(kwargs.get('value')) - else: - self.undefine() - - if not issubclass(field, fields.Field): + if not issubclass(self.field, fields.TypedField): raise ValueError( - f'field {field!r} is not derived from ' + f'field {self.field!r} is not derived from ' f'{fields.Field.__qualname__}' ) self.name = name - self.types = types - self.field = field + self.args = args + self.kwargs = kwargs - def is_undef(self): - return self.value == '_rfm_undef_var' + def is_defined(self): + return self.default_value is not _UndefVar def undefine(self): - self.value = '_rfm_undef_var' + self.default_value = _UndefVar def define(self, value): - self.value = value + self.default_value = value class LocalVarSpace(namespaces.LocalNamespace): @@ -59,7 +56,7 @@ def __init__(self): self.undefined = set() self.definitions = {} - def add(self, name, *types, **kwargs): + def declare(self, name, *args, **kwargs): '''Declare a new regression test variable. This method may only be called in the main class body. Otherwise, its @@ -69,9 +66,10 @@ def add(self, name, *types, **kwargs): :ref:`directives` + .. versionadded:: 3.5 ''' - self._is_logged(name) - self[name] = _TestVar(name, *types, **kwargs) + self._is_present(name) + self[name] = _TestVar(name, *args, **kwargs) def undefine(self, name): '''Undefine a variable previously declared in a parent class. @@ -83,8 +81,9 @@ def undefine(self, name): :ref:`directives` + .. versionadded:: 3.5 ''' - self._is_logged(name) + self._is_present(name) self.undefined.add(name) def define(self, name, value): @@ -97,11 +96,12 @@ def define(self, name, value): :ref:`directives` + .. versionadded:: 3.5 ''' - self._is_logged(name) + self._is_present(name) self.definitions[name] = value - def _is_logged(self, name): + def _is_present(self, name): ''' Check if an action has been registered for this variable. Calling more than one of the directives above on the same variable @@ -144,8 +144,12 @@ class VarSpace(namespaces.Namespace): local_namespace_class = LocalVarSpace namespace_name = '_rfm_var_space' - def join(self, other): - '''Join an existing VarSpace into the current one.''' + def join(self, other, cls): + '''Join an existing VarSpace into the current one. + + :param other: instance of the VarSpace class. + :param cls: the target class. + ''' for key, var in other.items(): # Make doubly declared vars illegal. Note that this will be @@ -153,7 +157,7 @@ def join(self, other): if key in self.vars: raise ValueError( f'variable {key!r} is declared in more than one of the ' - f'parent classes' + f'parent classes of class {cls.__qualname__!r}' ) self.vars[key] = var @@ -174,7 +178,6 @@ def extend(self, cls): # Extend the VarSpace for key, var in local_varspace.items(): - # Disable redeclaring a variable if key in self.vars: raise ValueError( @@ -196,10 +199,10 @@ def extend(self, cls): def _check_var_is_declared(self, key): if key not in self.vars: raise ValueError( - f'var {key!r} has not been declared' + f'variable {key!r} has not been declared' ) - def insert(self, obj, cls): + def inject(self, obj, cls): '''Insert the vars in the regression test. :param obj: The test object. @@ -207,16 +210,13 @@ def insert(self, obj, cls): ''' for name, var in self.items(): - setattr(cls, name, var.field(*var.types)) + setattr(cls, name, var.field(*var.args, **var.kwargs)) getattr(cls, name).__set_name__(obj, name) # If the var is defined, set its value - if not var.is_undef(): - setattr(obj, name, var.value) + if var.is_defined(): + setattr(obj, name, var.default_value) @property def vars(self): return self._namespace - - def undefined_vars(self): - return list(filter(lambda x: self.vars[x].is_undef(), self.vars)) diff --git a/unittests/test_parameters.py b/unittests/test_parameters.py index fbbe8cdfd9..0396e0c349 100644 --- a/unittests/test_parameters.py +++ b/unittests/test_parameters.py @@ -187,7 +187,7 @@ def test_namespace_clash(): class Spam(rfm.RegressionTest): var('foo', int, 1) - with pytest.raises(NameError): + with pytest.raises(ValueError): class Ham(Spam): parameter('foo', [1]) diff --git a/unittests/test_pipeline.py b/unittests/test_pipeline.py index 48529a63ba..be5e752aff 100644 --- a/unittests/test_pipeline.py +++ b/unittests/test_pipeline.py @@ -621,7 +621,7 @@ class MyTest(rfm.RunOnlyRegressionTest, hellotest): def test_extend_after_instantiation(hellotest): inst = hellotest() - with pytest.raises(NameError): + with pytest.raises(ValueError): class MyTest(hellotest): pass diff --git a/unittests/test_variables.py b/unittests/test_variables.py index 9a1fdfa95e..27b163782f 100644 --- a/unittests/test_variables.py +++ b/unittests/test_variables.py @@ -36,7 +36,7 @@ def test_custom_var(onevar): def test_instantiate_and_inherit(novars): inst = novars() - with pytest.raises(NameError): + with pytest.raises(ValueError): class MyTest(novars): '''Error from name clashing''' @@ -83,7 +83,7 @@ class MyTest(rfm.RegressionTest): def test_namespace_clash(novars): - with pytest.raises(NameError): + with pytest.raises(ValueError): class MyTest(novars): var('current_environ', str) From 3daac84277cb53dbf35f3e3d07ba1cfba9f09c85 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Fri, 5 Feb 2021 19:59:43 +0100 Subject: [PATCH 22/49] PEP8 fixes --- reframe/core/variables.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/reframe/core/variables.py b/reframe/core/variables.py index 4bc64b242e..06708f851c 100644 --- a/reframe/core/variables.py +++ b/reframe/core/variables.py @@ -11,14 +11,17 @@ import reframe.core.namespaces as namespaces import reframe.core.fields as fields + class _UndefVar: '''Custom type to flag a variable as undefined.''' + class _TestVar: '''Regression test variable. Buffer to store a regression test variable declared through directives. ''' + def __init__(self, name, *args, **kwargs): self.field = kwargs.pop('field', fields.TypedField) self.default_value = kwargs.pop('value', _UndefVar) From ee2dd35db6ddf5b650783c5def611c6cb062b05e Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Wed, 10 Feb 2021 18:41:13 +0100 Subject: [PATCH 23/49] Improve var assignment in the RegressionTest class --- reframe/core/meta.py | 3 ++ reframe/core/pipeline.py | 74 +++++++++++++++++++-------------------- reframe/core/variables.py | 5 +++ 3 files changed, 45 insertions(+), 37 deletions(-) diff --git a/reframe/core/meta.py b/reframe/core/meta.py index 76c8097dcb..4d0d7cd692 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -30,6 +30,9 @@ def __prepare__(cls, name, bases, **kwargs): local_var_space = variables.LocalVarSpace() namespace['_rfm_local_var_space'] = local_var_space + # The test var class + namespace['_TestVar'] = variables._TestVar + # Directives to add/modify a regression test variable namespace['var'] = local_var_space.declare namespace['require_var'] = local_var_space.undefine diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index e75311c5e4..ca142a2ae3 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -181,7 +181,7 @@ def pipeline_hooks(cls): #: The name of the test. #: #: :type: string that can contain any character except ``/`` - var('name', typ.Str[r'[^\/]+']) + name = _TestVar('/', typ.Str[r'[^\/]+']) #: List of programming environments supported by this test. #: @@ -201,7 +201,7 @@ def pipeline_hooks(cls): #: .. versionchanged:: 3.3 #: Default value changed from ``[]`` to ``None``. #: - var('valid_prog_environs', typ.List[str], type(None), value=None) + valid_prog_environgs = _TestVar('/', typ.List[str], type(None), value=None) #: List of systems supported by this test. #: The general syntax for systems is ``[:]``. @@ -214,13 +214,13 @@ def pipeline_hooks(cls): #: .. versionchanged:: 3.3 #: Default value changed from ``[]`` to ``None``. #: - var('valid_systems', typ.List[str], type(None), value=None) + valid_systems = _TestVar('/', typ.List[str], type(None), value=None) #: A detailed description of the test. #: #: :type: :class:`str` #: :default: ``self.name`` - var('descr', str) + descr = _TestVar('/', str) #: The path to the source file or source directory of the test. #: @@ -238,7 +238,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`str` #: :default: ``''`` - var('sourcepath', str, value='') + sourcepath = _TestVar('/', str, value='') #: The directory containing the test's resources. #: @@ -268,7 +268,7 @@ def pipeline_hooks(cls): #: .. versionchanged:: 3.0 #: Default value is now conditionally set to either ``'src'`` or #: :class:`None`. - var('sourcesdir', str, type(None), value='src') + sourcesdir = _TestVar('/', str, type(None), value='src') #: .. versionadded:: 2.14 #: @@ -285,7 +285,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`str` or :class:`reframe.core.buildsystems.BuildSystem`. #: :default: :class:`None`. - var('build_system', type(None), field=BuildSystemField, value=None) + build_system = _TestVar('/', type(None), field=BuildSystemField, value=None) #: .. versionadded:: 3.0 #: @@ -297,7 +297,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`List[str]` #: :default: ``[]`` - var('prebuild_cmds', typ.List[str], value=[]) + prebuild_cmds = _TestVar('/', typ.List[str], value=[]) #: .. versionadded:: 3.0 #: @@ -309,19 +309,19 @@ def pipeline_hooks(cls): #: #: :type: :class:`List[str]` #: :default: ``[]`` - var('postbuild_cmds', typ.List[str], value=[]) + postbuild_cmds = _TestVar('/', typ.List[str], value=[]) #: The name of the executable to be launched during the run phase. #: #: :type: :class:`str` #: :default: ``os.path.join('.', self.name)`` - var('executable', str) + executable = _TestVar('/', str) #: List of options to be passed to the :attr:`executable`. #: #: :type: :class:`List[str]` #: :default: ``[]`` - var('executable_opts', typ.List[str], value=[]) + executable_opts = _TestVar('/', typ.List[str], value=[]) #: .. versionadded:: 2.20 #: @@ -345,7 +345,7 @@ def pipeline_hooks(cls): #: :type: :class:`str` or #: :class:`reframe.core.containers.ContainerPlatform`. #: :default: :class:`None`. - var('container_platform', type(None), + container_platform = _TestVar('/', type(None), field=ContainerPlatformField, value=None) #: .. versionadded:: 3.0 @@ -358,7 +358,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`List[str]` #: :default: ``[]`` - var('prerun_cmds', typ.List[str], value=[]) + prerun_cmds = _TestVar('/', typ.List[str], value=[]) #: .. versionadded:: 3.0 #: @@ -369,7 +369,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`List[str]` #: :default: ``[]`` - var('postrun_cmds', typ.List[str], value=[]) + postrun_cmds = _TestVar('/', typ.List[str], value=[]) #: List of files to be kept after the test finishes. #: @@ -389,7 +389,7 @@ def pipeline_hooks(cls): #: .. versionchanged:: 3.3 #: This field accepts now also file glob patterns. #: - var('keep_files', typ.List[str], value=[]) + keep_files = _TestVar('/', typ.List[str], value=[]) #: List of files or directories (relative to the :attr:`sourcesdir`) that #: will be symlinked in the stage directory and not copied. @@ -399,7 +399,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`List[str]` #: :default: ``[]`` - var('readonly_files', typ.List[str], value=[]) + readonly_files = _TestVar('/', typ.List[str], value=[]) #: Set of tags associated with this test. #: @@ -407,7 +407,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`Set[str]` #: :default: an empty set - var('tags', typ.Set[str], value=set()) + tags = _TestVar('/', typ.Set[str], value=set()) #: List of people responsible for this test. #: @@ -415,7 +415,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`List[str]` #: :default: ``[]`` - var('maintainers', typ.List[str], value=[]) + maintainers = _TestVar('/', typ.List[str], value=[]) #: Mark this test as a strict performance test. #: @@ -425,7 +425,7 @@ def pipeline_hooks(cls): #: #: :type: boolean #: :default: :class:`True` - var('strict_check', bool, value=True) + strict_check = _TestVar('/', bool, value=True) #: Number of tasks required by this test. #: @@ -451,7 +451,7 @@ def pipeline_hooks(cls): #: #: .. |--flex-alloc-nodes| replace:: :attr:`--flex-alloc-nodes` #: .. _--flex-alloc-nodes: manpage.html#cmdoption-flex-alloc-nodes - var('num_tasks', int, value=1) + num_tasks = _TestVar('/', int, value=1) #: Number of tasks per node required by this test. #: @@ -459,7 +459,7 @@ def pipeline_hooks(cls): #: #: :type: integral or :class:`None` #: :default: :class:`None` - var('num_tasks_per_node', int, type(None), value=None) + num_tasks_per_node = _TestVar('/', int, type(None), value=None) #: Number of GPUs per node required by this test. #: This attribute is translated internally to the ``_rfm_gpu`` resource. @@ -468,7 +468,7 @@ def pipeline_hooks(cls): #: #: :type: integral #: :default: ``0`` - var('num_gpus_per_node', int, value=0) + num_gpus_per_node = _TestVar('/', int, value=0) #: Number of CPUs per task required by this test. #: @@ -476,7 +476,7 @@ def pipeline_hooks(cls): #: #: :type: integral or :class:`None` #: :default: :class:`None` - var('num_cpus_per_task', int, type(None), value=None) + num_cpus_per_task = _TestVar('/', int, type(None), value=None) #: Number of tasks per core required by this test. #: @@ -484,7 +484,7 @@ def pipeline_hooks(cls): #: #: :type: integral or :class:`None` #: :default: :class:`None` - var('num_tasks_per_core', int, type(None), value=None) + num_tasks_per_core = _TestVar('/', int, type(None), value=None) #: Number of tasks per socket required by this test. #: @@ -492,7 +492,7 @@ def pipeline_hooks(cls): #: #: :type: integral or :class:`None` #: :default: :class:`None` - var('num_tasks_per_socket', int, type(None), value=None) + num_tasks_per_socket = _TestVar('/', int, type(None), value=None) #: Specify whether this tests needs simultaneous multithreading enabled. #: @@ -500,7 +500,7 @@ def pipeline_hooks(cls): #: #: :type: boolean or :class:`None` #: :default: :class:`None` - var('use_multithreading', bool, type(None), value=None) + use_multithreading = _TestVar('/', bool, type(None), value=None) #: .. versionadded:: 3.0 #: @@ -510,19 +510,19 @@ def pipeline_hooks(cls): #: #: :type: :class:`str` or :class:`datetime.timedelta` #: :default: :class:`None` - var('max_pending_time', type(None), field=fields.TimerField, value=None) + max_pending_time = _TestVar('/', type(None), field=fields.TimerField, value=None) #: Specify whether this test needs exclusive access to nodes. #: #: :type: boolean #: :default: :class:`False` - var('exclusive_access', bool, value=False) + exclusive_access = _TestVar('/', bool, value=False) #: Always execute this test locally. #: #: :type: boolean #: :default: :class:`False` - var('local', bool, value=False) + local = _TestVar('/', bool, value=False) #: The set of reference values for this test. #: @@ -554,7 +554,7 @@ def pipeline_hooks(cls): #: .. versionchanged:: 3.0 #: The measurement unit is required. The user should explicitly #: specify :class:`None` if no unit is available. - var('reference', typ.Tuple[object, object, object, object], + reference = _TestVar('/', typ.Tuple[object, object, object, object], field=fields.ScopedDictField, value={}) # FIXME: There is not way currently to express tuples of `float`s or # `None`s, so we just use the very generic `object` @@ -581,7 +581,7 @@ def pipeline_hooks(cls): #: :: #: #: self.sanity_patterns = sn.assert_found(r'.*', self.stdout) - var('sanity_patterns', _DeferredExpression, type(None), value=None) + sanity_patterns = _TestVar('/', _DeferredExpression, type(None), value=None) #: Patterns for verifying the performance of this test. #: @@ -595,7 +595,7 @@ def pipeline_hooks(cls): #: `) as values. #: :class:`None` is also allowed. #: :default: :class:`None` - var('perf_patterns', typ.Dict[str, _DeferredExpression], + perf_patterns = _TestVar('/', typ.Dict[str, _DeferredExpression], type(None), value=None) #: List of modules to be loaded before running this test. @@ -604,7 +604,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`List[str]` #: :default: ``[]`` - var('modules', typ.List[str], value=[]) + modules = _TestVar('/', typ.List[str], value=[]) #: Environment variables to be set before running this test. #: @@ -612,7 +612,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`Dict[str, str]` #: :default: ``{}`` - var('variables', typ.Dict[str, str], value={}) + variables = _TestVar('/', typ.Dict[str, str], value={}) #: Time limit for this test. #: @@ -636,7 +636,7 @@ def pipeline_hooks(cls): #: - The old syntax using a ``(h, m, s)`` tuple is dropped. #: - Support of `timedelta` objects is dropped. #: - Number values are now accepted. - var('time_limit', type(None), field=fields.TimerField, value='10m') + time_limit = _TestVar('/', type(None), field=fields.TimerField, value='10m') #: .. versionadded:: 2.8 #: @@ -704,7 +704,7 @@ def pipeline_hooks(cls): #: .. versionchanged:: 2.9 #: A new more powerful syntax was introduced #: that allows also custom job script directive prefixes. - var('extra_resources', typ.Dict[str, typ.Dict[str, object]], value={}) + extra_resources = _TestVar('/', typ.Dict[str, typ.Dict[str, object]], value={}) #: .. versionadded:: 3.3 #: @@ -719,7 +719,7 @@ def pipeline_hooks(cls): #: appropriate sanity check. #: #: :type: boolean : :default: :class:`True` - var('build_locally', bool, value=True) + build_locally = _TestVar('/', bool, value=True) def __new__(cls, *args, _rfm_use_params=False, **kwargs): obj = super().__new__(cls) diff --git a/reframe/core/variables.py b/reframe/core/variables.py index 06708f851c..f4c9f75327 100644 --- a/reframe/core/variables.py +++ b/reframe/core/variables.py @@ -45,6 +45,11 @@ def undefine(self): def define(self, value): self.default_value = value + def __set_name__(self, owner, name): + self.name = name + owner._rfm_local_var_space[name] = self + delattr(owner, name) + class LocalVarSpace(namespaces.LocalNamespace): '''Local variable space of a regression test. From 32dcb2435ee0f1942656dda10de920f147cf62f3 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Wed, 10 Feb 2021 19:14:11 +0100 Subject: [PATCH 24/49] Rename _TestVar to _var --- reframe/core/meta.py | 4 +- reframe/core/pipeline.py | 81 ++++++++++++++++++++------------------- reframe/core/variables.py | 11 ++++++ 3 files changed, 54 insertions(+), 42 deletions(-) diff --git a/reframe/core/meta.py b/reframe/core/meta.py index 4d0d7cd692..502b84cd4a 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -30,8 +30,8 @@ def __prepare__(cls, name, bases, **kwargs): local_var_space = variables.LocalVarSpace() namespace['_rfm_local_var_space'] = local_var_space - # The test var class - namespace['_TestVar'] = variables._TestVar + # var declaration by assignment + namespace['_var'] = variables._TestVar # Directives to add/modify a regression test variable namespace['var'] = local_var_space.declare diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index ca142a2ae3..88f932e438 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -181,7 +181,7 @@ def pipeline_hooks(cls): #: The name of the test. #: #: :type: string that can contain any character except ``/`` - name = _TestVar('/', typ.Str[r'[^\/]+']) + name = _var('/', typ.Str[r'[^\/]+']) #: List of programming environments supported by this test. #: @@ -201,7 +201,7 @@ def pipeline_hooks(cls): #: .. versionchanged:: 3.3 #: Default value changed from ``[]`` to ``None``. #: - valid_prog_environgs = _TestVar('/', typ.List[str], type(None), value=None) + valid_prog_environgs = _var('/', typ.List[str], type(None), value=None) #: List of systems supported by this test. #: The general syntax for systems is ``[:]``. @@ -214,13 +214,13 @@ def pipeline_hooks(cls): #: .. versionchanged:: 3.3 #: Default value changed from ``[]`` to ``None``. #: - valid_systems = _TestVar('/', typ.List[str], type(None), value=None) + valid_systems = _var('/', typ.List[str], type(None), value=None) #: A detailed description of the test. #: #: :type: :class:`str` #: :default: ``self.name`` - descr = _TestVar('/', str) + descr = _var('/', str) #: The path to the source file or source directory of the test. #: @@ -238,7 +238,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`str` #: :default: ``''`` - sourcepath = _TestVar('/', str, value='') + sourcepath = _var('/', str, value='') #: The directory containing the test's resources. #: @@ -268,7 +268,7 @@ def pipeline_hooks(cls): #: .. versionchanged:: 3.0 #: Default value is now conditionally set to either ``'src'`` or #: :class:`None`. - sourcesdir = _TestVar('/', str, type(None), value='src') + sourcesdir = _var('/', str, type(None), value='src') #: .. versionadded:: 2.14 #: @@ -285,7 +285,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`str` or :class:`reframe.core.buildsystems.BuildSystem`. #: :default: :class:`None`. - build_system = _TestVar('/', type(None), field=BuildSystemField, value=None) + build_system = _var('/', type(None), field=BuildSystemField, value=None) #: .. versionadded:: 3.0 #: @@ -297,7 +297,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`List[str]` #: :default: ``[]`` - prebuild_cmds = _TestVar('/', typ.List[str], value=[]) + prebuild_cmds = _var('/', typ.List[str], value=[]) #: .. versionadded:: 3.0 #: @@ -309,19 +309,19 @@ def pipeline_hooks(cls): #: #: :type: :class:`List[str]` #: :default: ``[]`` - postbuild_cmds = _TestVar('/', typ.List[str], value=[]) + postbuild_cmds = _var('/', typ.List[str], value=[]) #: The name of the executable to be launched during the run phase. #: #: :type: :class:`str` #: :default: ``os.path.join('.', self.name)`` - executable = _TestVar('/', str) + executable = _var('/', str) #: List of options to be passed to the :attr:`executable`. #: #: :type: :class:`List[str]` #: :default: ``[]`` - executable_opts = _TestVar('/', typ.List[str], value=[]) + executable_opts = _var('/', typ.List[str], value=[]) #: .. versionadded:: 2.20 #: @@ -345,8 +345,8 @@ def pipeline_hooks(cls): #: :type: :class:`str` or #: :class:`reframe.core.containers.ContainerPlatform`. #: :default: :class:`None`. - container_platform = _TestVar('/', type(None), - field=ContainerPlatformField, value=None) + container_platform = _var('/', type(None), + field=ContainerPlatformField, value=None) #: .. versionadded:: 3.0 #: @@ -358,7 +358,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`List[str]` #: :default: ``[]`` - prerun_cmds = _TestVar('/', typ.List[str], value=[]) + prerun_cmds = _var('/', typ.List[str], value=[]) #: .. versionadded:: 3.0 #: @@ -369,7 +369,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`List[str]` #: :default: ``[]`` - postrun_cmds = _TestVar('/', typ.List[str], value=[]) + postrun_cmds = _var('/', typ.List[str], value=[]) #: List of files to be kept after the test finishes. #: @@ -389,7 +389,7 @@ def pipeline_hooks(cls): #: .. versionchanged:: 3.3 #: This field accepts now also file glob patterns. #: - keep_files = _TestVar('/', typ.List[str], value=[]) + keep_files = _var('/', typ.List[str], value=[]) #: List of files or directories (relative to the :attr:`sourcesdir`) that #: will be symlinked in the stage directory and not copied. @@ -399,7 +399,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`List[str]` #: :default: ``[]`` - readonly_files = _TestVar('/', typ.List[str], value=[]) + readonly_files = _var('/', typ.List[str], value=[]) #: Set of tags associated with this test. #: @@ -407,7 +407,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`Set[str]` #: :default: an empty set - tags = _TestVar('/', typ.Set[str], value=set()) + tags = _var('/', typ.Set[str], value=set()) #: List of people responsible for this test. #: @@ -415,7 +415,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`List[str]` #: :default: ``[]`` - maintainers = _TestVar('/', typ.List[str], value=[]) + maintainers = _var('/', typ.List[str], value=[]) #: Mark this test as a strict performance test. #: @@ -425,7 +425,7 @@ def pipeline_hooks(cls): #: #: :type: boolean #: :default: :class:`True` - strict_check = _TestVar('/', bool, value=True) + strict_check = _var('/', bool, value=True) #: Number of tasks required by this test. #: @@ -451,7 +451,7 @@ def pipeline_hooks(cls): #: #: .. |--flex-alloc-nodes| replace:: :attr:`--flex-alloc-nodes` #: .. _--flex-alloc-nodes: manpage.html#cmdoption-flex-alloc-nodes - num_tasks = _TestVar('/', int, value=1) + num_tasks = _var('/', int, value=1) #: Number of tasks per node required by this test. #: @@ -459,7 +459,7 @@ def pipeline_hooks(cls): #: #: :type: integral or :class:`None` #: :default: :class:`None` - num_tasks_per_node = _TestVar('/', int, type(None), value=None) + num_tasks_per_node = _var('/', int, type(None), value=None) #: Number of GPUs per node required by this test. #: This attribute is translated internally to the ``_rfm_gpu`` resource. @@ -468,7 +468,7 @@ def pipeline_hooks(cls): #: #: :type: integral #: :default: ``0`` - num_gpus_per_node = _TestVar('/', int, value=0) + num_gpus_per_node = _var('/', int, value=0) #: Number of CPUs per task required by this test. #: @@ -476,7 +476,7 @@ def pipeline_hooks(cls): #: #: :type: integral or :class:`None` #: :default: :class:`None` - num_cpus_per_task = _TestVar('/', int, type(None), value=None) + num_cpus_per_task = _var('/', int, type(None), value=None) #: Number of tasks per core required by this test. #: @@ -484,7 +484,7 @@ def pipeline_hooks(cls): #: #: :type: integral or :class:`None` #: :default: :class:`None` - num_tasks_per_core = _TestVar('/', int, type(None), value=None) + num_tasks_per_core = _var('/', int, type(None), value=None) #: Number of tasks per socket required by this test. #: @@ -492,7 +492,7 @@ def pipeline_hooks(cls): #: #: :type: integral or :class:`None` #: :default: :class:`None` - num_tasks_per_socket = _TestVar('/', int, type(None), value=None) + num_tasks_per_socket = _var('/', int, type(None), value=None) #: Specify whether this tests needs simultaneous multithreading enabled. #: @@ -500,7 +500,7 @@ def pipeline_hooks(cls): #: #: :type: boolean or :class:`None` #: :default: :class:`None` - use_multithreading = _TestVar('/', bool, type(None), value=None) + use_multithreading = _var('/', bool, type(None), value=None) #: .. versionadded:: 3.0 #: @@ -510,19 +510,20 @@ def pipeline_hooks(cls): #: #: :type: :class:`str` or :class:`datetime.timedelta` #: :default: :class:`None` - max_pending_time = _TestVar('/', type(None), field=fields.TimerField, value=None) + max_pending_time = _var( + '/', type(None), field=fields.TimerField, value=None) #: Specify whether this test needs exclusive access to nodes. #: #: :type: boolean #: :default: :class:`False` - exclusive_access = _TestVar('/', bool, value=False) + exclusive_access = _var('/', bool, value=False) #: Always execute this test locally. #: #: :type: boolean #: :default: :class:`False` - local = _TestVar('/', bool, value=False) + local = _var('/', bool, value=False) #: The set of reference values for this test. #: @@ -554,8 +555,8 @@ def pipeline_hooks(cls): #: .. versionchanged:: 3.0 #: The measurement unit is required. The user should explicitly #: specify :class:`None` if no unit is available. - reference = _TestVar('/', typ.Tuple[object, object, object, object], - field=fields.ScopedDictField, value={}) + reference = _var('/', typ.Tuple[object, object, object, object], + field=fields.ScopedDictField, value={}) # FIXME: There is not way currently to express tuples of `float`s or # `None`s, so we just use the very generic `object` @@ -581,7 +582,7 @@ def pipeline_hooks(cls): #: :: #: #: self.sanity_patterns = sn.assert_found(r'.*', self.stdout) - sanity_patterns = _TestVar('/', _DeferredExpression, type(None), value=None) + sanity_patterns = _var('/', _DeferredExpression, type(None), value=None) #: Patterns for verifying the performance of this test. #: @@ -595,8 +596,8 @@ def pipeline_hooks(cls): #: `) as values. #: :class:`None` is also allowed. #: :default: :class:`None` - perf_patterns = _TestVar('/', typ.Dict[str, _DeferredExpression], - type(None), value=None) + perf_patterns = _var('/', typ.Dict[str, _DeferredExpression], + type(None), value=None) #: List of modules to be loaded before running this test. #: @@ -604,7 +605,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`List[str]` #: :default: ``[]`` - modules = _TestVar('/', typ.List[str], value=[]) + modules = _var('/', typ.List[str], value=[]) #: Environment variables to be set before running this test. #: @@ -612,7 +613,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`Dict[str, str]` #: :default: ``{}`` - variables = _TestVar('/', typ.Dict[str, str], value={}) + variables = _var('/', typ.Dict[str, str], value={}) #: Time limit for this test. #: @@ -636,7 +637,7 @@ def pipeline_hooks(cls): #: - The old syntax using a ``(h, m, s)`` tuple is dropped. #: - Support of `timedelta` objects is dropped. #: - Number values are now accepted. - time_limit = _TestVar('/', type(None), field=fields.TimerField, value='10m') + time_limit = _var('/', type(None), field=fields.TimerField, value='10m') #: .. versionadded:: 2.8 #: @@ -704,7 +705,7 @@ def pipeline_hooks(cls): #: .. versionchanged:: 2.9 #: A new more powerful syntax was introduced #: that allows also custom job script directive prefixes. - extra_resources = _TestVar('/', typ.Dict[str, typ.Dict[str, object]], value={}) + extra_resources = _var('/', typ.Dict[str, typ.Dict[str, object]], value={}) #: .. versionadded:: 3.3 #: @@ -719,7 +720,7 @@ def pipeline_hooks(cls): #: appropriate sanity check. #: #: :type: boolean : :default: :class:`True` - build_locally = _TestVar('/', bool, value=True) + build_locally = _var('/', bool, value=True) def __new__(cls, *args, _rfm_use_params=False, **kwargs): obj = super().__new__(cls) diff --git a/reframe/core/variables.py b/reframe/core/variables.py index f4c9f75327..a423ee3160 100644 --- a/reframe/core/variables.py +++ b/reframe/core/variables.py @@ -32,6 +32,9 @@ def __init__(self, name, *args, **kwargs): f'{fields.Field.__qualname__}' ) + if not isinstance(name, str): + raise ValueError("the argument 'name' must be a string") + self.name = name self.args = args self.kwargs = kwargs @@ -46,6 +49,14 @@ def define(self, value): self.default_value = value def __set_name__(self, owner, name): + '''Overwrite the dummy name. + + If the variable was created directly by assignment in the test class, + this function assigns the variable the name used in the test class body + and inserts the variable in the test's local variable space. To avoid + any namespace collisions, this function also disowns the test class + (owner argument) from this variable. + ''' self.name = name owner._rfm_local_var_space[name] = self delattr(owner, name) From 813083e1e551537c56acd86cc8cce1267233812a Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Thu, 11 Feb 2021 13:38:32 +0100 Subject: [PATCH 25/49] Address PR comments --- reframe/core/namespaces.py | 12 +- reframe/core/parameters.py | 14 ++- reframe/core/variables.py | 35 ++++-- unittests/test_pipeline.py | 225 ++++++++++++++++++------------------ unittests/test_variables.py | 79 +++++++------ 5 files changed, 193 insertions(+), 172 deletions(-) diff --git a/reframe/core/namespaces.py b/reframe/core/namespaces.py index 96154778ae..0000856ae7 100644 --- a/reframe/core/namespaces.py +++ b/reframe/core/namespaces.py @@ -17,15 +17,12 @@ class LocalNamespace(metaclass=abc.ABCMeta): Temporary storage for test attributes defined in the test class body through directives. This local namespace is populated during the - test class body execution through the add method, which must be - exposed as a directive in the + test class body execution through directives available in the class :class:`reframe.core.pipeline.RegressionTest`. Example: In the pseudo-code below, the local namespace of A is {P0}, and the local namespace of B is {P1}. However, the final namespace of A is still {P0}, and the final namespace of B is {P0, P1}. - The :func:`var` directive is simply an alias to the - :func:`add` method. .. code:: python @@ -120,8 +117,7 @@ def __init__(self, target_cls=None, illegal_names=None): self.sanity(target_cls, illegal_names) # Attach the Namespace to the target class - if target_cls: - setattr(target_cls, self.namespace_name, self) + setattr(target_cls, self.namespace_name, self) def assert_target_cls(self, cls): '''Assert the target class has a valid local namespace.''' @@ -158,8 +154,8 @@ def sanity(self, cls, illegal_names=None): for key in self._namespace: if key in illegal_names: raise ValueError( - f'{key!r} already defined in class' - f' {cls.__qualname__!r}' + f'{key!r} already defined in class ' + f'{cls.__qualname__!r}' ) @abc.abstractmethod diff --git a/reframe/core/parameters.py b/reframe/core/parameters.py index b7f1ec82d2..cf1f7be101 100644 --- a/reframe/core/parameters.py +++ b/reframe/core/parameters.py @@ -103,9 +103,17 @@ class ParamSpace(namespaces.Namespace): the target class. ''' - local_namespace_name = '_rfm_local_param_space' - local_namespace_class = LocalParamSpace - namespace_name = '_rfm_param_space' + @property + def local_namespace_name(self): + return '_rfm_local_param_space' + + @property + def local_namespace_class(self): + return LocalParamSpace + + @property + def namespace_name(self): + return '_rfm_param_space' def __init__(self, target_cls=None, target_namespace=None): super().__init__(target_cls, target_namespace) diff --git a/reframe/core/variables.py b/reframe/core/variables.py index a423ee3160..09770ad452 100644 --- a/reframe/core/variables.py +++ b/reframe/core/variables.py @@ -12,8 +12,12 @@ import reframe.core.fields as fields -class _UndefVar: +class _UndefinedType: '''Custom type to flag a variable as undefined.''' + __slots__ = () + + +_Undefined = _UndefinedType() class _TestVar: @@ -21,14 +25,13 @@ class _TestVar: Buffer to store a regression test variable declared through directives. ''' - def __init__(self, name, *args, **kwargs): - self.field = kwargs.pop('field', fields.TypedField) - self.default_value = kwargs.pop('value', _UndefVar) + self.field_type = kwargs.pop('field', fields.TypedField) + self.default_value = kwargs.pop('value', _Undefined) - if not issubclass(self.field, fields.TypedField): + if not issubclass(self.field_type, fields.Field): raise ValueError( - f'field {self.field!r} is not derived from ' + f'field {self.field_type!r} is not derived from ' f'{fields.Field.__qualname__}' ) @@ -40,10 +43,10 @@ def __init__(self, name, *args, **kwargs): self.kwargs = kwargs def is_defined(self): - return self.default_value is not _UndefVar + return self.default_value is not _Undefined def undefine(self): - self.default_value = _UndefVar + self.default_value = _Undefined def define(self, value): self.default_value = value @@ -159,9 +162,17 @@ class VarSpace(namespaces.Namespace): provided, the VarSpace is simply initialized as empty. ''' - local_namespace_name = '_rfm_local_var_space' - local_namespace_class = LocalVarSpace - namespace_name = '_rfm_var_space' + @property + def local_namespace_name(self): + return '_rfm_local_var_space' + + @property + def local_namespace_class(self): + return LocalVarSpace + + @property + def namespace_name(self): + return '_rfm_var_space' def join(self, other, cls): '''Join an existing VarSpace into the current one. @@ -229,7 +240,7 @@ def inject(self, obj, cls): ''' for name, var in self.items(): - setattr(cls, name, var.field(*var.args, **var.kwargs)) + setattr(cls, name, var.field_type(*var.args, **var.kwargs)) getattr(cls, name).__set_name__(obj, name) # If the var is defined, set its value diff --git a/unittests/test_pipeline.py b/unittests/test_pipeline.py index be5e752aff..d87fd41a0e 100644 --- a/unittests/test_pipeline.py +++ b/unittests/test_pipeline.py @@ -29,12 +29,17 @@ def _run(test, partition, prgenv): @pytest.fixture -def hellotest(): +def HelloTest(): from unittests.resources.checks.hellocheck import HelloTest yield HelloTest del sys.modules['unittests.resources.checks.hellocheck'] +@pytest.fixture +def hellotest(HelloTest): + yield HelloTest() + + @pytest.fixture def hellomaketest(): from unittests.resources.checks.hellocheck_make import HelloMakeTest @@ -158,17 +163,15 @@ def __init__(self): def test_environ_setup(hellotest, local_exec_ctx): - inst = hellotest() - # Use test environment for the regression check - inst.variables = {'_FOO_': '1', '_BAR_': '2'} - inst.setup(*local_exec_ctx) - for k in inst.variables.keys(): + hellotest.variables = {'_FOO_': '1', '_BAR_': '2'} + hellotest.setup(*local_exec_ctx) + for k in hellotest.variables.keys(): assert k not in os.environ def test_hellocheck(hellotest, remote_exec_ctx): - _run(hellotest(), *remote_exec_ctx) + _run(hellotest, *remote_exec_ctx) def test_hellocheck_make(hellomaketest, remote_exec_ctx): @@ -176,34 +179,31 @@ def test_hellocheck_make(hellomaketest, remote_exec_ctx): def test_hellocheck_local(hellotest, local_exec_ctx): - inst = hellotest() - # Test also the prebuild/postbuild functionality - inst.prebuild_cmds = ['touch prebuild', 'mkdir -p prebuild_dir/foo'] - inst.postbuild_cmds = ['touch postbuild', 'mkdir postbuild_dir'] - inst.keep_files = ['prebuild', 'postbuild', '*dir'] + hellotest.prebuild_cmds = ['touch prebuild', 'mkdir -p prebuild_dir/foo'] + hellotest.postbuild_cmds = ['touch postbuild', 'mkdir postbuild_dir'] + hellotest.keep_files = ['prebuild', 'postbuild', '*dir'] # Force local execution of the test; just for testing .local - inst.local = True - _run(inst, *local_exec_ctx) + hellotest.local = True + _run(hellotest, *local_exec_ctx) must_keep = [ - inst.stdout.evaluate(), - inst.stderr.evaluate(), - inst.build_stdout.evaluate(), - inst.build_stderr.evaluate(), - inst.job.script_filename, + hellotest.stdout.evaluate(), + hellotest.stderr.evaluate(), + hellotest.build_stdout.evaluate(), + hellotest.build_stderr.evaluate(), + hellotest.job.script_filename, 'prebuild', 'postbuild', 'prebuild_dir', 'prebuild_dir/foo', 'postbuild_dir' ] for f in must_keep: - assert os.path.exists(os.path.join(inst.outputdir, f)) + assert os.path.exists(os.path.join(hellotest.outputdir, f)) def test_hellocheck_build_remotely(hellotest, remote_exec_ctx): - inst = hellotest() - inst.build_locally = False - _run(inst, *remote_exec_ctx) - assert not inst.build_job.scheduler.is_local + hellotest.build_locally = False + _run(hellotest, *remote_exec_ctx) + assert not hellotest.build_job.scheduler.is_local def test_hellocheck_local_prepost_run(hellotest, local_exec_ctx): @@ -211,18 +211,16 @@ def test_hellocheck_local_prepost_run(hellotest, local_exec_ctx): def stagedir(test): return test.stagedir - inst = hellotest() - # Test also the prebuild/postbuild functionality - inst.prerun_cmds = ['echo prerun: `pwd`'] - inst.postrun_cmds = ['echo postrun: `pwd`'] - pre_run_path = sn.extractsingle(r'^prerun: (\S+)', inst.stdout, 1) - post_run_path = sn.extractsingle(r'^postrun: (\S+)', inst.stdout, 1) - inst.sanity_patterns = sn.all([ - sn.assert_eq(stagedir(inst), pre_run_path), - sn.assert_eq(stagedir(inst), post_run_path), + hellotest.prerun_cmds = ['echo prerun: `pwd`'] + hellotest.postrun_cmds = ['echo postrun: `pwd`'] + pre_run_path = sn.extractsingle(r'^prerun: (\S+)', hellotest.stdout, 1) + post_run_path = sn.extractsingle(r'^postrun: (\S+)', hellotest.stdout, 1) + hellotest.sanity_patterns = sn.all([ + sn.assert_eq(stagedir(hellotest), pre_run_path), + sn.assert_eq(stagedir(hellotest), post_run_path), ]) - _run(inst, *local_exec_ctx) + _run(hellotest, *local_exec_ctx) def test_run_only_sanity(local_exec_ctx): @@ -294,61 +292,59 @@ class MyTest(pinnedtest): def test_supports_system(hellotest, testsys_system): - inst = hellotest() - inst.valid_systems = ['*'] - assert inst.supports_system('gpu') - assert inst.supports_system('login') - assert inst.supports_system('testsys:gpu') - assert inst.supports_system('testsys:login') - - inst.valid_systems = ['*:*'] - assert inst.supports_system('gpu') - assert inst.supports_system('login') - assert inst.supports_system('testsys:gpu') - assert inst.supports_system('testsys:login') - - inst.valid_systems = ['testsys'] - assert inst.supports_system('gpu') - assert inst.supports_system('login') - assert inst.supports_system('testsys:gpu') - assert inst.supports_system('testsys:login') - - inst.valid_systems = ['testsys:gpu'] - assert inst.supports_system('gpu') - assert not inst.supports_system('login') - assert inst.supports_system('testsys:gpu') - assert not inst.supports_system('testsys:login') - - inst.valid_systems = ['testsys:login'] - assert not inst.supports_system('gpu') - assert inst.supports_system('login') - assert not inst.supports_system('testsys:gpu') - assert inst.supports_system('testsys:login') - - inst.valid_systems = ['foo'] - assert not inst.supports_system('gpu') - assert not inst.supports_system('login') - assert not inst.supports_system('testsys:gpu') - assert not inst.supports_system('testsys:login') - - inst.valid_systems = ['*:gpu'] - assert inst.supports_system('testsys:gpu') - assert inst.supports_system('foo:gpu') - assert not inst.supports_system('testsys:cpu') - assert not inst.supports_system('testsys:login') - - inst.valid_systems = ['testsys:*'] - assert inst.supports_system('testsys:login') - assert inst.supports_system('gpu') - assert not inst.supports_system('foo:gpu') + hellotest.valid_systems = ['*'] + assert hellotest.supports_system('gpu') + assert hellotest.supports_system('login') + assert hellotest.supports_system('testsys:gpu') + assert hellotest.supports_system('testsys:login') + + hellotest.valid_systems = ['*:*'] + assert hellotest.supports_system('gpu') + assert hellotest.supports_system('login') + assert hellotest.supports_system('testsys:gpu') + assert hellotest.supports_system('testsys:login') + + hellotest.valid_systems = ['testsys'] + assert hellotest.supports_system('gpu') + assert hellotest.supports_system('login') + assert hellotest.supports_system('testsys:gpu') + assert hellotest.supports_system('testsys:login') + + hellotest.valid_systems = ['testsys:gpu'] + assert hellotest.supports_system('gpu') + assert not hellotest.supports_system('login') + assert hellotest.supports_system('testsys:gpu') + assert not hellotest.supports_system('testsys:login') + + hellotest.valid_systems = ['testsys:login'] + assert not hellotest.supports_system('gpu') + assert hellotest.supports_system('login') + assert not hellotest.supports_system('testsys:gpu') + assert hellotest.supports_system('testsys:login') + + hellotest.valid_systems = ['foo'] + assert not hellotest.supports_system('gpu') + assert not hellotest.supports_system('login') + assert not hellotest.supports_system('testsys:gpu') + assert not hellotest.supports_system('testsys:login') + + hellotest.valid_systems = ['*:gpu'] + assert hellotest.supports_system('testsys:gpu') + assert hellotest.supports_system('foo:gpu') + assert not hellotest.supports_system('testsys:cpu') + assert not hellotest.supports_system('testsys:login') + + hellotest.valid_systems = ['testsys:*'] + assert hellotest.supports_system('testsys:login') + assert hellotest.supports_system('gpu') + assert not hellotest.supports_system('foo:gpu') def test_supports_environ(hellotest, generic_system): - inst = hellotest() - inst.valid_prog_environs = ['*'] - assert inst.supports_environ('foo1') - assert inst.supports_environ('foo-env') - assert inst.supports_environ('*') + hellotest.valid_prog_environs = ['*'] + assert hellotest.supports_environ('foo1') + assert hellotest.supports_environ('foo-env') + assert hellotest.supports_environ('*') def test_sourcesdir_none(local_exec_ctx): @@ -467,9 +463,9 @@ def __init__(self): test.compile_wait() -def test_extra_resources(hellotest, testsys_system): +def test_extra_resources(HelloTest, testsys_system): @fixtures.custom_prefix('unittests/resources/checks') - class MyTest(hellotest): + class MyTest(HelloTest): def __init__(self): super().__init__() self.name = type(self).__name__ @@ -496,9 +492,9 @@ def set_resources(self): assert expected_job_options == set(test.job.options) -def test_setup_hooks(hellotest, local_exec_ctx): +def test_setup_hooks(HelloTest, local_exec_ctx): @fixtures.custom_prefix('unittests/resources/checks') - class MyTest(hellotest): + class MyTest(HelloTest): def __init__(self): super().__init__() self.name = type(self).__name__ @@ -520,9 +516,9 @@ def postfoo(self): assert test.count == 2 -def test_compile_hooks(hellotest, local_exec_ctx): +def test_compile_hooks(HelloTest, local_exec_ctx): @fixtures.custom_prefix('unittests/resources/checks') - class MyTest(hellotest): + class MyTest(HelloTest): def __init__(self): super().__init__() self.name = type(self).__name__ @@ -545,9 +541,9 @@ def check_executable(self): assert test.count == 1 -def test_run_hooks(hellotest, local_exec_ctx): +def test_run_hooks(HelloTest, local_exec_ctx): @fixtures.custom_prefix('unittests/resources/checks') - class MyTest(hellotest): + class MyTest(HelloTest): def __init__(self): super().__init__() self.name = type(self).__name__ @@ -567,9 +563,9 @@ def check_executable(self): _run(MyTest(), *local_exec_ctx) -def test_multiple_hooks(hellotest, local_exec_ctx): +def test_multiple_hooks(HelloTest, local_exec_ctx): @fixtures.custom_prefix('unittests/resources/checks') - class MyTest(hellotest): + class MyTest(HelloTest): def __init__(self): super().__init__() self.name = type(self).__name__ @@ -593,9 +589,9 @@ def z(self): assert test.var == 3 -def test_stacked_hooks(hellotest, local_exec_ctx): +def test_stacked_hooks(HelloTest, local_exec_ctx): @fixtures.custom_prefix('unittests/resources/checks') - class MyTest(hellotest): + class MyTest(HelloTest): def __init__(self): super().__init__() self.name = type(self).__name__ @@ -613,22 +609,27 @@ def x(self): assert test.var == 3 -def test_multiple_inheritance(hellotest): +def test_multiple_inheritance(HelloTest): with pytest.raises(ValueError): - class MyTest(rfm.RunOnlyRegressionTest, hellotest): + class MyTest(rfm.RunOnlyRegressionTest, HelloTest): pass -def test_extend_after_instantiation(hellotest): - inst = hellotest() +def test_extend_after_instantiation(HelloTest): + '''Instantiation will inject the vars as class attributes. + + Therefore, inheriting from this class after the instantiation will create + a namespace clash with the vars. + ''' + hellotest = HelloTest() with pytest.raises(ValueError): - class MyTest(hellotest): + class MyTest(HelloTest): pass -def test_inherited_hooks(hellotest, local_exec_ctx): +def test_inherited_hooks(HelloTest, local_exec_ctx): @fixtures.custom_prefix('unittests/resources/checks') - class BaseTest(hellotest): + class BaseTest(HelloTest): def __init__(self): super().__init__() self.name = type(self).__name__ @@ -662,9 +663,9 @@ class MyTest(DerivedTest): } -def test_overriden_hooks(hellotest, local_exec_ctx): +def test_overriden_hooks(HelloTest, local_exec_ctx): @fixtures.custom_prefix('unittests/resources/checks') - class BaseTest(hellotest): + class BaseTest(HelloTest): def __init__(self): super().__init__() self.name = type(self).__name__ @@ -696,9 +697,9 @@ def y(self): assert test.foo == 10 -def test_disabled_hooks(hellotest, local_exec_ctx): +def test_disabled_hooks(HelloTest, local_exec_ctx): @fixtures.custom_prefix('unittests/resources/checks') - class BaseTest(hellotest): + class BaseTest(HelloTest): def __init__(self): super().__init__() self.name = type(self).__name__ @@ -726,12 +727,12 @@ def x(self): assert test.foo == 0 -def test_require_deps(hellotest, local_exec_ctx): +def test_require_deps(HelloTest, local_exec_ctx): import reframe.frontend.dependencies as dependencies import reframe.frontend.executors as executors @fixtures.custom_prefix('unittests/resources/checks') - class T0(hellotest): + class T0(HelloTest): def __init__(self): super().__init__() self.name = type(self).__name__ @@ -739,7 +740,7 @@ def __init__(self): self.x = 1 @fixtures.custom_prefix('unittests/resources/checks') - class T1(hellotest): + class T1(HelloTest): def __init__(self): super().__init__() self.name = type(self).__name__ diff --git a/unittests/test_variables.py b/unittests/test_variables.py index 27b163782f..6c5d6a4faa 100644 --- a/unittests/test_variables.py +++ b/unittests/test_variables.py @@ -12,47 +12,55 @@ @pytest.fixture -def novars(): - class NoVars(rfm.RegressionTest): +def NoVarsTest(): + class NoVarsTest(rfm.RegressionTest): pass - yield NoVars + + yield NoVarsTest @pytest.fixture -def onevar(novars): - class OneVar(novars): +def OneVarTest(NoVarsTest): + class OneVarTest(NoVarsTest): var('foo', int, value=10) - yield OneVar + + yield OneVarTest -def test_custom_var(onevar): - assert not hasattr(onevar, 'foo') - inst = onevar() - assert hasattr(onevar, 'foo') - assert isinstance(onevar.foo, Field) +def test_custom_var(OneVarTest): + assert not hasattr(OneVarTest, 'foo') + inst = OneVarTest() + assert hasattr(OneVarTest, 'foo') + assert isinstance(OneVarTest.foo, Field) assert hasattr(inst, 'foo') assert inst.foo == 10 -def test_instantiate_and_inherit(novars): - inst = novars() +def test_instantiate_and_inherit(NoVarsTest): + inst = NoVarsTest() with pytest.raises(ValueError): - class MyTest(novars): + class MyTest(NoVarsTest): '''Error from name clashing''' -def test_redeclare_var_clash(novars): +def test_redeclare_builtin_var_clash(NoVarsTest): with pytest.raises(ValueError): - class MyTest(novars): + class MyTest(NoVarsTest): var('name', str) -def test_inheritance_clash(novars): +def test_redeclare_var_clash(OneVarTest): + with pytest.raises(ValueError): + class MyTest(OneVarTest): + var('foo', str) + + +def test_inheritance_clash(NoVarsTest): class MyMixin(rfm.RegressionMixin): var('name', str) with pytest.raises(ValueError): - class MyTest(novars, MyMixin): + class MyTest(NoVarsTest, MyMixin): '''Trigger error from inheritance clash.''' @@ -82,39 +90,39 @@ class MyTest(rfm.RegressionTest): var('v0', int, value=2) -def test_namespace_clash(novars): +def test_namespace_clash(NoVarsTest): with pytest.raises(ValueError): - class MyTest(novars): + class MyTest(NoVarsTest): var('current_environ', str) -def test_set_var(onevar): - class MyTest(onevar): +def test_set_var(OneVarTest): + class MyTest(OneVarTest): set_var('foo', 4) inst = MyTest() - assert not hasattr(onevar, 'foo') + assert not hasattr(OneVarTest, 'foo') assert hasattr(MyTest, 'foo') assert hasattr(inst, 'foo') assert inst.foo == 4 -def test_var_type(onevar): - class MyTest(onevar): +def test_var_type(OneVarTest): + class MyTest(OneVarTest): set_var('foo', 'bananas') with pytest.raises(TypeError): inst = MyTest() -def test_set_undef(novars): +def test_set_undef(NoVarsTest): with pytest.raises(ValueError): - class MyTest(novars): + class MyTest(NoVarsTest): set_var('foo', 4) -def test_require_var(onevar): - class MyTest(onevar): +def test_require_var(OneVarTest): + class MyTest(OneVarTest): require_var('foo') def __init__(self): @@ -124,22 +132,19 @@ def __init__(self): inst = MyTest() -def test_required_var_not_present(onevar): - class MyTest(onevar): +def test_required_var_not_present(OneVarTest): + class MyTest(OneVarTest): require_var('foo') def __init__(self): pass - try: - inst = MyTest() - except Exception: - pytest.fail('class instantiation failed') + mytest = MyTest() -def test_require_undef(novars): +def test_require_undeclared_var(NoVarsTest): with pytest.raises(ValueError): - class MyTest(novars): + class MyTest(NoVarsTest): require_var('foo') From a07a3c6700f4dcf880fe34b02fe693e97ca3365f Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Thu, 11 Feb 2021 18:05:39 +0100 Subject: [PATCH 26/49] Extend metaclass to work with custom namespace --- reframe/core/meta.py | 8 ++++++-- reframe/core/namespaces.py | 42 +++++++++++++++++++++++++------------- 2 files changed, 34 insertions(+), 16 deletions(-) diff --git a/reframe/core/meta.py b/reframe/core/meta.py index 502b84cd4a..b7b3739ee3 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -9,13 +9,14 @@ from reframe.core.exceptions import ReframeSyntaxError +import reframe.core.namespaces as namespaces import reframe.core.parameters as parameters import reframe.core.variables as variables class RegressionTestMeta(type): @classmethod - def __prepare__(cls, name, bases, **kwargs): + def __prepare__(metacls, name, bases, **kwargs): namespace = super().__prepare__(name, bases, **kwargs) # Regression test parameter space defined at the class level @@ -37,7 +38,10 @@ def __prepare__(cls, name, bases, **kwargs): namespace['var'] = local_var_space.declare namespace['require_var'] = local_var_space.undefine namespace['set_var'] = local_var_space.define - return namespace + return namespaces.LocalNamespace(namespace) + + def __new__(metacls, name, bases, namespace, **kwargs): + return super().__new__(metacls, name, bases, dict(namespace), **kwargs) def __init__(cls, name, bases, namespace, **kwargs): super().__init__(name, bases, namespace, **kwargs) diff --git a/reframe/core/namespaces.py b/reframe/core/namespaces.py index 0000856ae7..d7f2949233 100644 --- a/reframe/core/namespaces.py +++ b/reframe/core/namespaces.py @@ -4,21 +4,18 @@ # SPDX-License-Identifier: BSD-3-Clause # -# Abstract base classes to build extensible namespaces through class -# directives. +# Base classes to manage the namespace of a regression test. # import abc -class LocalNamespace(metaclass=abc.ABCMeta): +class LocalNamespace: '''Local namespace of a regression test. - Temporary storage for test attributes defined in the test class body - through directives. This local namespace is populated during the - test class body execution through directives available in the class - :class:`reframe.core.pipeline.RegressionTest`. + Temporary storage for test attributes defined in the test class body. + This local namespace is populated during the test class body execution. Example: In the pseudo-code below, the local namespace of A is {P0}, and the local namespace of B is {P1}. However, the final namespace @@ -33,21 +30,38 @@ class B(A): var('P1') ''' - def __init__(self): - self._namespace = {} + def __init__(self, namespace=None): + self._namespace = namespace if namespace else {} def __getattr__(self, name): return getattr(self._namespace, name) - def __setitem__(self, name, value): - if name not in self._namespace: - self._namespace[name] = value + def __getitem__(self, key): + return self._namespace[key] + + def __setitem__(self, key, value): + if key not in self._namespace: + self._namespace[key] = value else: - self._raise_namespace_clash(name) + self._raise_namespace_clash(key) + + def __contains__(self, key): + return key in self._namespace + + def __iter__(self): + return iter(self._namespace) + + def __len__(self): + return len(self._namespace) + + def __repr__(self): + return '{}({!r})'.format(type(self).__name__, self._namespace) - @abc.abstractmethod def _raise_namespace_clash(self, name): '''Raise an error if there is a namespace clash.''' + raise ValueError( + f'{name!r} is already present in the current namespace' + ) class Namespace(metaclass=abc.ABCMeta): From 880a8acb930a4a1a9197986ddd400abe19b5a380 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Thu, 11 Feb 2021 21:51:48 +0100 Subject: [PATCH 27/49] Port var directives to new syntax --- reframe/core/meta.py | 26 ++++--- reframe/core/pipeline.py | 76 ++++++++++---------- reframe/core/variables.py | 138 ++++++++++-------------------------- unittests/test_variables.py | 64 ++++++++--------- 4 files changed, 124 insertions(+), 180 deletions(-) diff --git a/reframe/core/meta.py b/reframe/core/meta.py index b7b3739ee3..7b4a340e97 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -14,6 +14,17 @@ import reframe.core.variables as variables +class MetaNamespace(namespaces.LocalNamespace): + def __setitem__(self, key, value): + if isinstance(value, variables.VarDirective): + self['_rfm_local_var_space'][key] = value + + #elif isinstance(value, parameters._TestParam): + # self['_rfm_local_param_space'][key] = value + + else: + super().__setitem__(key, value) + class RegressionTestMeta(type): @classmethod def __prepare__(metacls, name, bases, **kwargs): @@ -28,17 +39,14 @@ def __prepare__(metacls, name, bases, **kwargs): namespace['parameter'] = local_param_space.insert # Regression test var space defined at the class level - local_var_space = variables.LocalVarSpace() + local_var_space = namespaces.LocalNamespace() namespace['_rfm_local_var_space'] = local_var_space - # var declaration by assignment - namespace['_var'] = variables._TestVar - # Directives to add/modify a regression test variable - namespace['var'] = local_var_space.declare - namespace['require_var'] = local_var_space.undefine - namespace['set_var'] = local_var_space.define - return namespaces.LocalNamespace(namespace) + namespace['variable'] = variables.TestVar + namespace['required_variable'] = variables.UndefineVar() + #namespace['set_var'] = variables.DefineVar + return MetaNamespace(namespace) def __new__(metacls, name, bases, namespace, **kwargs): return super().__new__(metacls, name, bases, dict(namespace), **kwargs) @@ -50,7 +58,7 @@ def __init__(cls, name, bases, namespace, **kwargs): used_attribute_names = set(dir(cls)) # Build the var space and extend the target namespace - variables.VarSpace(cls, used_attribute_names) + variables.VarSpace(cls, set()) used_attribute_names.update(cls._rfm_var_space.vars) # Build the parameter space diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 88f932e438..4798f14bf1 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -181,7 +181,7 @@ def pipeline_hooks(cls): #: The name of the test. #: #: :type: string that can contain any character except ``/`` - name = _var('/', typ.Str[r'[^\/]+']) + name = variable(typ.Str[r'[^\/]+']) #: List of programming environments supported by this test. #: @@ -201,7 +201,7 @@ def pipeline_hooks(cls): #: .. versionchanged:: 3.3 #: Default value changed from ``[]`` to ``None``. #: - valid_prog_environgs = _var('/', typ.List[str], type(None), value=None) + valid_prog_environgs = variable(typ.List[str], type(None), value=None) #: List of systems supported by this test. #: The general syntax for systems is ``[:]``. @@ -214,13 +214,13 @@ def pipeline_hooks(cls): #: .. versionchanged:: 3.3 #: Default value changed from ``[]`` to ``None``. #: - valid_systems = _var('/', typ.List[str], type(None), value=None) + valid_systems = variable(typ.List[str], type(None), value=None) #: A detailed description of the test. #: #: :type: :class:`str` #: :default: ``self.name`` - descr = _var('/', str) + descr = variable(str) #: The path to the source file or source directory of the test. #: @@ -238,7 +238,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`str` #: :default: ``''`` - sourcepath = _var('/', str, value='') + sourcepath = variable(str, value='') #: The directory containing the test's resources. #: @@ -268,7 +268,7 @@ def pipeline_hooks(cls): #: .. versionchanged:: 3.0 #: Default value is now conditionally set to either ``'src'`` or #: :class:`None`. - sourcesdir = _var('/', str, type(None), value='src') + sourcesdir = variable(str, type(None), value='src') #: .. versionadded:: 2.14 #: @@ -285,7 +285,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`str` or :class:`reframe.core.buildsystems.BuildSystem`. #: :default: :class:`None`. - build_system = _var('/', type(None), field=BuildSystemField, value=None) + build_system = variable(type(None), field=BuildSystemField, value=None) #: .. versionadded:: 3.0 #: @@ -297,7 +297,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`List[str]` #: :default: ``[]`` - prebuild_cmds = _var('/', typ.List[str], value=[]) + prebuild_cmds = variable(typ.List[str], value=[]) #: .. versionadded:: 3.0 #: @@ -309,19 +309,19 @@ def pipeline_hooks(cls): #: #: :type: :class:`List[str]` #: :default: ``[]`` - postbuild_cmds = _var('/', typ.List[str], value=[]) + postbuild_cmds = variable(typ.List[str], value=[]) #: The name of the executable to be launched during the run phase. #: #: :type: :class:`str` #: :default: ``os.path.join('.', self.name)`` - executable = _var('/', str) + executable = variable(str) #: List of options to be passed to the :attr:`executable`. #: #: :type: :class:`List[str]` #: :default: ``[]`` - executable_opts = _var('/', typ.List[str], value=[]) + executable_opts = variable(typ.List[str], value=[]) #: .. versionadded:: 2.20 #: @@ -345,7 +345,7 @@ def pipeline_hooks(cls): #: :type: :class:`str` or #: :class:`reframe.core.containers.ContainerPlatform`. #: :default: :class:`None`. - container_platform = _var('/', type(None), + container_platform = variable(type(None), field=ContainerPlatformField, value=None) #: .. versionadded:: 3.0 @@ -358,7 +358,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`List[str]` #: :default: ``[]`` - prerun_cmds = _var('/', typ.List[str], value=[]) + prerun_cmds = variable(typ.List[str], value=[]) #: .. versionadded:: 3.0 #: @@ -369,7 +369,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`List[str]` #: :default: ``[]`` - postrun_cmds = _var('/', typ.List[str], value=[]) + postrun_cmds = variable(typ.List[str], value=[]) #: List of files to be kept after the test finishes. #: @@ -389,7 +389,7 @@ def pipeline_hooks(cls): #: .. versionchanged:: 3.3 #: This field accepts now also file glob patterns. #: - keep_files = _var('/', typ.List[str], value=[]) + keep_files = variable(typ.List[str], value=[]) #: List of files or directories (relative to the :attr:`sourcesdir`) that #: will be symlinked in the stage directory and not copied. @@ -399,7 +399,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`List[str]` #: :default: ``[]`` - readonly_files = _var('/', typ.List[str], value=[]) + readonly_files = variable(typ.List[str], value=[]) #: Set of tags associated with this test. #: @@ -407,7 +407,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`Set[str]` #: :default: an empty set - tags = _var('/', typ.Set[str], value=set()) + tags = variable(typ.Set[str], value=set()) #: List of people responsible for this test. #: @@ -415,7 +415,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`List[str]` #: :default: ``[]`` - maintainers = _var('/', typ.List[str], value=[]) + maintainers = variable(typ.List[str], value=[]) #: Mark this test as a strict performance test. #: @@ -425,7 +425,7 @@ def pipeline_hooks(cls): #: #: :type: boolean #: :default: :class:`True` - strict_check = _var('/', bool, value=True) + strict_check = variable(bool, value=True) #: Number of tasks required by this test. #: @@ -451,7 +451,7 @@ def pipeline_hooks(cls): #: #: .. |--flex-alloc-nodes| replace:: :attr:`--flex-alloc-nodes` #: .. _--flex-alloc-nodes: manpage.html#cmdoption-flex-alloc-nodes - num_tasks = _var('/', int, value=1) + num_tasks = variable(int, value=1) #: Number of tasks per node required by this test. #: @@ -459,7 +459,7 @@ def pipeline_hooks(cls): #: #: :type: integral or :class:`None` #: :default: :class:`None` - num_tasks_per_node = _var('/', int, type(None), value=None) + num_tasks_per_node = variable(int, type(None), value=None) #: Number of GPUs per node required by this test. #: This attribute is translated internally to the ``_rfm_gpu`` resource. @@ -468,7 +468,7 @@ def pipeline_hooks(cls): #: #: :type: integral #: :default: ``0`` - num_gpus_per_node = _var('/', int, value=0) + num_gpus_per_node = variable(int, value=0) #: Number of CPUs per task required by this test. #: @@ -476,7 +476,7 @@ def pipeline_hooks(cls): #: #: :type: integral or :class:`None` #: :default: :class:`None` - num_cpus_per_task = _var('/', int, type(None), value=None) + num_cpus_per_task = variable(int, type(None), value=None) #: Number of tasks per core required by this test. #: @@ -484,7 +484,7 @@ def pipeline_hooks(cls): #: #: :type: integral or :class:`None` #: :default: :class:`None` - num_tasks_per_core = _var('/', int, type(None), value=None) + num_tasks_per_core = variable(int, type(None), value=None) #: Number of tasks per socket required by this test. #: @@ -492,7 +492,7 @@ def pipeline_hooks(cls): #: #: :type: integral or :class:`None` #: :default: :class:`None` - num_tasks_per_socket = _var('/', int, type(None), value=None) + num_tasks_per_socket = variable(int, type(None), value=None) #: Specify whether this tests needs simultaneous multithreading enabled. #: @@ -500,7 +500,7 @@ def pipeline_hooks(cls): #: #: :type: boolean or :class:`None` #: :default: :class:`None` - use_multithreading = _var('/', bool, type(None), value=None) + use_multithreading = variable(bool, type(None), value=None) #: .. versionadded:: 3.0 #: @@ -510,20 +510,20 @@ def pipeline_hooks(cls): #: #: :type: :class:`str` or :class:`datetime.timedelta` #: :default: :class:`None` - max_pending_time = _var( - '/', type(None), field=fields.TimerField, value=None) + max_pending_time = variable( + type(None), field=fields.TimerField, value=None) #: Specify whether this test needs exclusive access to nodes. #: #: :type: boolean #: :default: :class:`False` - exclusive_access = _var('/', bool, value=False) + exclusive_access = variable(bool, value=False) #: Always execute this test locally. #: #: :type: boolean #: :default: :class:`False` - local = _var('/', bool, value=False) + local = variable(bool, value=False) #: The set of reference values for this test. #: @@ -555,7 +555,7 @@ def pipeline_hooks(cls): #: .. versionchanged:: 3.0 #: The measurement unit is required. The user should explicitly #: specify :class:`None` if no unit is available. - reference = _var('/', typ.Tuple[object, object, object, object], + reference = variable(typ.Tuple[object, object, object, object], field=fields.ScopedDictField, value={}) # FIXME: There is not way currently to express tuples of `float`s or # `None`s, so we just use the very generic `object` @@ -582,7 +582,7 @@ def pipeline_hooks(cls): #: :: #: #: self.sanity_patterns = sn.assert_found(r'.*', self.stdout) - sanity_patterns = _var('/', _DeferredExpression, type(None), value=None) + sanity_patterns = variable(_DeferredExpression, type(None), value=None) #: Patterns for verifying the performance of this test. #: @@ -596,7 +596,7 @@ def pipeline_hooks(cls): #: `) as values. #: :class:`None` is also allowed. #: :default: :class:`None` - perf_patterns = _var('/', typ.Dict[str, _DeferredExpression], + perf_patterns = variable(typ.Dict[str, _DeferredExpression], type(None), value=None) #: List of modules to be loaded before running this test. @@ -605,7 +605,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`List[str]` #: :default: ``[]`` - modules = _var('/', typ.List[str], value=[]) + modules = variable(typ.List[str], value=[]) #: Environment variables to be set before running this test. #: @@ -613,7 +613,7 @@ def pipeline_hooks(cls): #: #: :type: :class:`Dict[str, str]` #: :default: ``{}`` - variables = _var('/', typ.Dict[str, str], value={}) + variables = variable(typ.Dict[str, str], value={}) #: Time limit for this test. #: @@ -637,7 +637,7 @@ def pipeline_hooks(cls): #: - The old syntax using a ``(h, m, s)`` tuple is dropped. #: - Support of `timedelta` objects is dropped. #: - Number values are now accepted. - time_limit = _var('/', type(None), field=fields.TimerField, value='10m') + time_limit = variable(type(None), field=fields.TimerField, value='10m') #: .. versionadded:: 2.8 #: @@ -705,7 +705,7 @@ def pipeline_hooks(cls): #: .. versionchanged:: 2.9 #: A new more powerful syntax was introduced #: that allows also custom job script directive prefixes. - extra_resources = _var('/', typ.Dict[str, typ.Dict[str, object]], value={}) + extra_resources = variable(typ.Dict[str, typ.Dict[str, object]], value={}) #: .. versionadded:: 3.3 #: @@ -720,7 +720,7 @@ def pipeline_hooks(cls): #: appropriate sanity check. #: #: :type: boolean : :default: :class:`True` - build_locally = _var('/', bool, value=True) + build_locally = variable(bool, value=True) def __new__(cls, *args, _rfm_use_params=False, **kwargs): obj = super().__new__(cls) diff --git a/reframe/core/variables.py b/reframe/core/variables.py index 09770ad452..00451978f8 100644 --- a/reframe/core/variables.py +++ b/reframe/core/variables.py @@ -7,6 +7,7 @@ # Functionality to build extensible variable spaces into ReFrame tests. # +from types import MappingProxyType import reframe.core.namespaces as namespaces import reframe.core.fields as fields @@ -19,13 +20,15 @@ class _UndefinedType: _Undefined = _UndefinedType() +class VarDirective: + '''Base class for the variable directives.''' -class _TestVar: +class TestVar(VarDirective): '''Regression test variable. Buffer to store a regression test variable declared through directives. ''' - def __init__(self, name, *args, **kwargs): + def __init__(self, *args, **kwargs): self.field_type = kwargs.pop('field', fields.TypedField) self.default_value = kwargs.pop('value', _Undefined) @@ -35,10 +38,6 @@ def __init__(self, name, *args, **kwargs): f'{fields.Field.__qualname__}' ) - if not isinstance(name, str): - raise ValueError("the argument 'name' must be a string") - - self.name = name self.args = args self.kwargs = kwargs @@ -61,90 +60,16 @@ def __set_name__(self, owner, name): (owner argument) from this variable. ''' self.name = name - owner._rfm_local_var_space[name] = self - delattr(owner, name) -class LocalVarSpace(namespaces.LocalNamespace): - '''Local variable space of a regression test. +class DefineVar(VarDirective): + def __init__(self, default_value): + self.default_value = default_value - Stores the input from the var directives executed in the class body of - the regression test. This local variable space is later used during the - instantiation of the VarSpace class to extend the final variable space. - ''' +class UndefineVar(VarDirective): def __init__(self): - super().__init__() - self.undefined = set() - self.definitions = {} - - def declare(self, name, *args, **kwargs): - '''Declare a new regression test variable. - - This method may only be called in the main class body. Otherwise, its - behavior is undefined. - - .. seealso:: - - :ref:`directives` - - .. versionadded:: 3.5 - ''' - self._is_present(name) - self[name] = _TestVar(name, *args, **kwargs) - - def undefine(self, name): - '''Undefine a variable previously declared in a parent class. - - This method may only be called in the main class body. Otherwise, its - behavior is undefined. - - .. seealso:: - - :ref:`directives` - - .. versionadded:: 3.5 - ''' - self._is_present(name) - self.undefined.add(name) - - def define(self, name, value): - '''Assign a value to a previously declared regression test variable. - - This method may only be called in the main class body. Otherwise, its - behavior is undefined. - - .. seealso:: - - :ref:`directives` - - .. versionadded:: 3.5 - ''' - self._is_present(name) - self.definitions[name] = value - - def _is_present(self, name): - ''' Check if an action has been registered for this variable. - - Calling more than one of the directives above on the same variable - does not make sense. - ''' - if any(name in x for x in [self.vars, - self.undefined, - self.definitions]): - raise ValueError( - f'cannot specify more than one action on variable' - f' {name!r} in the same class' - ) - - @property - def vars(self): - return self._namespace - - def _raise_namespace_clash(self, name): - raise ValueError( - f'{name!r} is already present in the local variable space' - ) + self.default_value = _Undefined class VarSpace(namespaces.Namespace): @@ -168,7 +93,7 @@ def local_namespace_name(self): @property def local_namespace_class(self): - return LocalVarSpace + return namespaces.LocalNamespace @property def namespace_name(self): @@ -206,25 +131,35 @@ def extend(self, cls): ''' local_varspace = getattr(cls, self.local_namespace_name) - # Extend the VarSpace for key, var in local_varspace.items(): - # Disable redeclaring a variable - if key in self.vars: + if isinstance(var, TestVar): + # Disable redeclaring a variable + if key in self.vars: + raise ValueError( + f'cannot redeclare a variable ({key})' + ) + + # Add a new var + self.vars[key] = var + + elif isinstance(var, VarDirective): + # Modify the value of a previously declarated var + self._check_var_is_declared(key) + self.vars[key].define(var.default_value) + + # If any previously declared variable was defined in the class body + # retrieve the value from the class namespace and update it into the + # variable space. + for key, value in cls.__dict__.items(): + if key in local_varspace: raise ValueError( - f'cannot redeclare a variable ({key})' + f'cannot specify more than one action on variable' + f' {key!r} in the same class' ) - self.vars[key] = var - - # Undefine the vars as indicated by the local var space - for key in local_varspace.undefined: - self._check_var_is_declared(key) - self.vars[key].undefine() - - # Define the vars as indicated by the local var space - for key, val in local_varspace.definitions.items(): - self._check_var_is_declared(key) - self.vars[key].define(val) + elif key in self.vars: + self._check_var_is_declared(key) + self.vars[key].define(value) def _check_var_is_declared(self, key): if key not in self.vars: @@ -232,6 +167,7 @@ def _check_var_is_declared(self, key): f'variable {key!r} has not been declared' ) + def inject(self, obj, cls): '''Insert the vars in the regression test. diff --git a/unittests/test_variables.py b/unittests/test_variables.py index 6c5d6a4faa..7168b14f91 100644 --- a/unittests/test_variables.py +++ b/unittests/test_variables.py @@ -22,12 +22,12 @@ class NoVarsTest(rfm.RegressionTest): @pytest.fixture def OneVarTest(NoVarsTest): class OneVarTest(NoVarsTest): - var('foo', int, value=10) + foo = variable(int, value=10) yield OneVarTest -def test_custom_var(OneVarTest): +def test_custom_variable(OneVarTest): assert not hasattr(OneVarTest, 'foo') inst = OneVarTest() assert hasattr(OneVarTest, 'foo') @@ -36,28 +36,28 @@ def test_custom_var(OneVarTest): assert inst.foo == 10 -def test_instantiate_and_inherit(NoVarsTest): - inst = NoVarsTest() - with pytest.raises(ValueError): - class MyTest(NoVarsTest): - '''Error from name clashing''' +##def test_instantiate_and_inherit(NoVarsTest): +## inst = NoVarsTest() +## with pytest.raises(ValueError): +## class MyTest(NoVarsTest): +## '''Error from name clashing''' def test_redeclare_builtin_var_clash(NoVarsTest): with pytest.raises(ValueError): class MyTest(NoVarsTest): - var('name', str) + name = variable(str) def test_redeclare_var_clash(OneVarTest): with pytest.raises(ValueError): class MyTest(OneVarTest): - var('foo', str) + foo = variable(str) def test_inheritance_clash(NoVarsTest): class MyMixin(rfm.RegressionMixin): - var('name', str) + name = variable(str) with pytest.raises(ValueError): class MyTest(NoVarsTest, MyMixin): @@ -66,10 +66,10 @@ class MyTest(NoVarsTest, MyMixin): def test_var_space_clash(): class Spam(rfm.RegressionMixin): - var('v0', int, value=1) + v0 = variable(int, value=1) class Ham(rfm.RegressionMixin): - var('v0', int, value=2) + v0 = variable(int, value=2) with pytest.raises(ValueError): class Eggs(Spam, Ham): @@ -79,26 +79,26 @@ class Eggs(Spam, Ham): def test_double_declare(): with pytest.raises(ValueError): class MyTest(rfm.RegressionTest): - var('v0', int, value=1) - var('v0', float, value=0.5) + v0 = variable(int, value=1) + v0 = variable(float, value=0.5) -def test_double_action_on_var(): +def test_double_action_on_variable(): with pytest.raises(ValueError): class MyTest(rfm.RegressionTest): - set_var('v0', 2) - var('v0', int, value=2) + v0 = 2 + v0 = variable(int, value=2) -def test_namespace_clash(NoVarsTest): - with pytest.raises(ValueError): - class MyTest(NoVarsTest): - var('current_environ', str) +#def test_namespace_clash(NoVarsTest): +# with pytest.raises(ValueError): +# class MyTest(NoVarsTest): +# current_environ = variable(str) def test_set_var(OneVarTest): class MyTest(OneVarTest): - set_var('foo', 4) + foo = 4 inst = MyTest() assert not hasattr(OneVarTest, 'foo') @@ -109,21 +109,21 @@ class MyTest(OneVarTest): def test_var_type(OneVarTest): class MyTest(OneVarTest): - set_var('foo', 'bananas') + foo = 'bananas' with pytest.raises(TypeError): inst = MyTest() -def test_set_undef(NoVarsTest): - with pytest.raises(ValueError): - class MyTest(NoVarsTest): - set_var('foo', 4) +##def test_set_undef(NoVarsTest): +## with pytest.raises(ValueError): +## class MyTest(NoVarsTest): +## foo = 4 def test_require_var(OneVarTest): class MyTest(OneVarTest): - require_var('foo') + foo = required_variable def __init__(self): print(self.foo) @@ -134,7 +134,7 @@ def __init__(self): def test_required_var_not_present(OneVarTest): class MyTest(OneVarTest): - require_var('foo') + foo = required_variable def __init__(self): pass @@ -142,10 +142,10 @@ def __init__(self): mytest = MyTest() -def test_require_undeclared_var(NoVarsTest): +def test_require_undeclared_variable(NoVarsTest): with pytest.raises(ValueError): class MyTest(NoVarsTest): - require_var('foo') + foo = required_variable def test_invalid_field(): @@ -154,4 +154,4 @@ class Foo: with pytest.raises(ValueError): class MyTest(rfm.RegressionTest): - var('a', int, value=4, field=Foo) + a = variable(int, value=4, field=Foo) From 998c9b9e438c50639d455a09e8bd981f32d72617 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Thu, 11 Feb 2021 22:41:15 +0100 Subject: [PATCH 28/49] Port parameters to new syntax --- reframe/core/meta.py | 19 +++++++++-------- reframe/core/namespaces.py | 7 +------ reframe/core/parameters.py | 40 ++++-------------------------------- reframe/core/variables.py | 4 ---- unittests/test_parameters.py | 28 ++++++++++++------------- unittests/test_pipeline.py | 12 ----------- unittests/test_variables.py | 21 ++++--------------- 7 files changed, 34 insertions(+), 97 deletions(-) diff --git a/reframe/core/meta.py b/reframe/core/meta.py index 7b4a340e97..2028c5f919 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -15,28 +15,32 @@ class MetaNamespace(namespaces.LocalNamespace): + '''Regression test's namespace to control the cls attribute assignment.''' def __setitem__(self, key, value): if isinstance(value, variables.VarDirective): + # Insert the attribute in the variable namespace self['_rfm_local_var_space'][key] = value - #elif isinstance(value, parameters._TestParam): - # self['_rfm_local_param_space'][key] = value + elif isinstance(value, parameters.TestParam): + # Insert the attribute in the parameter namespace + self['_rfm_local_param_space'][key] = value else: super().__setitem__(key, value) + class RegressionTestMeta(type): @classmethod def __prepare__(metacls, name, bases, **kwargs): namespace = super().__prepare__(name, bases, **kwargs) # Regression test parameter space defined at the class level - local_param_space = parameters.LocalParamSpace() + local_param_space = namespaces.LocalNamespace() namespace['_rfm_local_param_space'] = local_param_space # Directive to insert a regression test parameter directly in the - # class body as: `parameter('P0', 0,1,2,3)`. - namespace['parameter'] = local_param_space.insert + # class body as: `P0 = parameter([0,1,2,3])`. + namespace['parameter'] = parameters.TestParam # Regression test var space defined at the class level local_var_space = namespaces.LocalNamespace() @@ -45,7 +49,6 @@ def __prepare__(metacls, name, bases, **kwargs): # Directives to add/modify a regression test variable namespace['variable'] = variables.TestVar namespace['required_variable'] = variables.UndefineVar() - #namespace['set_var'] = variables.DefineVar return MetaNamespace(namespace) def __new__(metacls, name, bases, namespace, **kwargs): @@ -55,10 +58,10 @@ def __init__(cls, name, bases, namespace, **kwargs): super().__init__(name, bases, namespace, **kwargs) # Create a set with the attribute names already in use. - used_attribute_names = set(dir(cls)) + used_attribute_names = set(dir(cls)) - set(cls.__dict__) # Build the var space and extend the target namespace - variables.VarSpace(cls, set()) + variables.VarSpace(cls, used_attribute_names) used_attribute_names.update(cls._rfm_var_space.vars) # Build the parameter space diff --git a/reframe/core/namespaces.py b/reframe/core/namespaces.py index d7f2949233..757e028c7c 100644 --- a/reframe/core/namespaces.py +++ b/reframe/core/namespaces.py @@ -101,11 +101,6 @@ def local_namespace_name(self): :class:`reframe.core.pipeline.RegressionTest` class. ''' - @property - @abc.abstractmethod - def local_namespace_class(self): - '''Type of the expected local namespace.''' - @property @abc.abstractmethod def namespace_name(self): @@ -138,7 +133,7 @@ def assert_target_cls(self, cls): assert hasattr(cls, self.local_namespace_name) assert isinstance(getattr(cls, self.local_namespace_name), - self.local_namespace_class) + LocalNamespace) def inherit(self, cls): '''Inherit the Namespaces from the bases.''' diff --git a/reframe/core/parameters.py b/reframe/core/parameters.py index cf1f7be101..dc7d47dd03 100644 --- a/reframe/core/parameters.py +++ b/reframe/core/parameters.py @@ -13,7 +13,7 @@ import reframe.core.namespaces as namespaces -class _TestParameter: +class TestParam: '''Regression test paramter class. Stores the attributes of a regression test parameter as defined directly @@ -23,7 +23,7 @@ class _TestParameter: parameter space is built. ''' - def __init__(self, name, values=None, + def __init__(self, values=None, inherit_params=False, filter_params=None): if values is None: values = [] @@ -40,39 +40,11 @@ def filter_params(x): def filter_params(x): return x - self.name = name self.values = tuple(values) self.filter_params = filter_params - -class LocalParamSpace(namespaces.LocalNamespace): - '''Local parameter space of a regression test. - - Stores all the regression test parameters defined in the test class body. - ''' - - def insert(self, name, values=None, **kwargs): - '''Insert or modify a regression test parameter. - - This method may only be called in the main class body. Otherwise, its - behavior is undefined. - - .. seealso:: - - :ref:`directives` - - .. versionadded:: 3.4 - ''' - self[name] = _TestParameter(name, values, **kwargs) - - @property - def params(self): - return self._namespace - - def _raise_namespace_clash(self, name): - raise ValueError( - f'{name!r} is already present in the local parameter space' - ) + def __set_name__(self, owner, name): + self.name = name class ParamSpace(namespaces.Namespace): @@ -107,10 +79,6 @@ class ParamSpace(namespaces.Namespace): def local_namespace_name(self): return '_rfm_local_param_space' - @property - def local_namespace_class(self): - return LocalParamSpace - @property def namespace_name(self): return '_rfm_param_space' diff --git a/reframe/core/variables.py b/reframe/core/variables.py index 00451978f8..00b1776339 100644 --- a/reframe/core/variables.py +++ b/reframe/core/variables.py @@ -91,10 +91,6 @@ class VarSpace(namespaces.Namespace): def local_namespace_name(self): return '_rfm_local_var_space' - @property - def local_namespace_class(self): - return namespaces.LocalNamespace - @property def namespace_name(self): return '_rfm_var_space' diff --git a/unittests/test_parameters.py b/unittests/test_parameters.py index 0396e0c349..cec3e4df6a 100644 --- a/unittests/test_parameters.py +++ b/unittests/test_parameters.py @@ -16,17 +16,17 @@ class NoParams(rfm.RunOnlyRegressionTest): class TwoParams(NoParams): - parameter('P0', ['a']) - parameter('P1', ['b']) + P0 = parameter(['a']) + P1 = parameter(['b']) class Abstract(TwoParams): - parameter('P0') + P0 = parameter() class ExtendParams(TwoParams): - parameter('P1', ['c', 'd', 'e'], inherit_params=True) - parameter('P2', ['f', 'g']) + P1 = parameter(['c', 'd', 'e'], inherit_params=True) + P2 = parameter(['f', 'g']) def test_param_space_is_empty(): @@ -54,7 +54,7 @@ class MyTest(Abstract): def test_param_override(): class MyTest(TwoParams): - parameter('P1', ['-']) + P1 = parameter(['-']) assert MyTest.param_space['P0'] == ('a',) assert MyTest.param_space['P1'] == ('-',) @@ -62,7 +62,7 @@ class MyTest(TwoParams): def test_param_inheritance(): class MyTest(TwoParams): - parameter('P1', ['c'], inherit_params=True) + P1 = parameter(['c'], inherit_params=True) assert MyTest.param_space['P0'] == ('a',) assert MyTest.param_space['P1'] == ('b', 'c',) @@ -70,7 +70,7 @@ class MyTest(TwoParams): def test_filter_params(): class MyTest(ExtendParams): - parameter('P1', inherit_params=True, filter_params=lambda x: x[2:]) + P1 = parameter(inherit_params=True, filter_params=lambda x: x[2:]) assert MyTest.param_space['P0'] == ('a',) assert MyTest.param_space['P1'] == ('d', 'e',) @@ -173,10 +173,10 @@ def __init__(self, var): def test_param_space_clash(): class Spam(rfm.RegressionMixin): - parameter('P0', [1]) + P0 = parameter([1]) class Ham(rfm.RegressionMixin): - parameter('P0', [2]) + P0 = parameter([2]) with pytest.raises(ValueError): class Eggs(Spam, Ham): @@ -185,15 +185,15 @@ class Eggs(Spam, Ham): def test_namespace_clash(): class Spam(rfm.RegressionTest): - var('foo', int, 1) + foo = variable(int, 1) with pytest.raises(ValueError): class Ham(Spam): - parameter('foo', [1]) + foo = parameter([1]) def test_double_declare(): with pytest.raises(ValueError): class MyTest(rfm.RegressionTest): - parameter('P0', [1, 2, 3]) - parameter('P0') + P0 = parameter([1, 2, 3]) + P0 = parameter() diff --git a/unittests/test_pipeline.py b/unittests/test_pipeline.py index d87fd41a0e..3d4af3cfcc 100644 --- a/unittests/test_pipeline.py +++ b/unittests/test_pipeline.py @@ -615,18 +615,6 @@ class MyTest(rfm.RunOnlyRegressionTest, HelloTest): pass -def test_extend_after_instantiation(HelloTest): - '''Instantiation will inject the vars as class attributes. - - Therefore, inheriting from this class after the instantiation will create - a namespace clash with the vars. - ''' - hellotest = HelloTest() - with pytest.raises(ValueError): - class MyTest(HelloTest): - pass - - def test_inherited_hooks(HelloTest, local_exec_ctx): @fixtures.custom_prefix('unittests/resources/checks') class BaseTest(HelloTest): diff --git a/unittests/test_variables.py b/unittests/test_variables.py index 7168b14f91..9eaca96c99 100644 --- a/unittests/test_variables.py +++ b/unittests/test_variables.py @@ -36,13 +36,6 @@ def test_custom_variable(OneVarTest): assert inst.foo == 10 -##def test_instantiate_and_inherit(NoVarsTest): -## inst = NoVarsTest() -## with pytest.raises(ValueError): -## class MyTest(NoVarsTest): -## '''Error from name clashing''' - - def test_redeclare_builtin_var_clash(NoVarsTest): with pytest.raises(ValueError): class MyTest(NoVarsTest): @@ -90,10 +83,10 @@ class MyTest(rfm.RegressionTest): v0 = variable(int, value=2) -#def test_namespace_clash(NoVarsTest): -# with pytest.raises(ValueError): -# class MyTest(NoVarsTest): -# current_environ = variable(str) +def test_namespace_clash(NoVarsTest): + with pytest.raises(ValueError): + class MyTest(NoVarsTest): + current_environ = variable(str) def test_set_var(OneVarTest): @@ -115,12 +108,6 @@ class MyTest(OneVarTest): inst = MyTest() -##def test_set_undef(NoVarsTest): -## with pytest.raises(ValueError): -## class MyTest(NoVarsTest): -## foo = 4 - - def test_require_var(OneVarTest): class MyTest(OneVarTest): foo = required_variable From 2b23b4bffa3215c7e6a08d9a2cbf5142ad250544 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Thu, 11 Feb 2021 22:56:00 +0100 Subject: [PATCH 29/49] Remove unused import --- reframe/core/variables.py | 1 - 1 file changed, 1 deletion(-) diff --git a/reframe/core/variables.py b/reframe/core/variables.py index 00b1776339..4d3b488357 100644 --- a/reframe/core/variables.py +++ b/reframe/core/variables.py @@ -7,7 +7,6 @@ # Functionality to build extensible variable spaces into ReFrame tests. # -from types import MappingProxyType import reframe.core.namespaces as namespaces import reframe.core.fields as fields From 2891369be51ec50ef40e89e4e1edee796bea30b9 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Fri, 12 Feb 2021 12:03:18 +0100 Subject: [PATCH 30/49] Fix instantiate and inherit --- reframe/core/variables.py | 30 ++++++++++++++++++++++++++++++ unittests/test_variables.py | 23 +++++++++++++++++------ 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/reframe/core/variables.py b/reframe/core/variables.py index 4d3b488357..0842510250 100644 --- a/reframe/core/variables.py +++ b/reframe/core/variables.py @@ -94,6 +94,14 @@ def local_namespace_name(self): def namespace_name(self): return '_rfm_var_space' + def __init__(self, target_cls=None, illegal_names=None): + # Set to register the variables already injected in the class + self._injected_vars = set() + + super().__init__(target_cls, illegal_names) + + + def join(self, other, cls): '''Join an existing VarSpace into the current one. @@ -112,6 +120,9 @@ def join(self, other, cls): self.vars[key] = var + # Carry over the set of injected variables + self._injected_vars.update(other._injected_vars) + def extend(self, cls): '''Extend the VarSpace with the content in the LocalVarSpace. @@ -162,6 +173,22 @@ def _check_var_is_declared(self, key): f'variable {key!r} has not been declared' ) + def sanity(self, cls, illegal_names=None): + '''Sanity checks post-creation of the var namespace. + + By default, we make illegal to have any item in the namespace + that clashes with a member of the target class unless this member + was injected by this namespace. + ''' + if illegal_names is None: + illegal_names = set(dir(cls)) + + for key in self._namespace: + if (key in illegal_names) and (key not in self._injected_vars): + raise ValueError( + f'{key!r} already defined in class ' + f'{cls.__qualname__!r}' + ) def inject(self, obj, cls): '''Insert the vars in the regression test. @@ -178,6 +205,9 @@ def inject(self, obj, cls): if var.is_defined(): setattr(obj, name, var.default_value) + # Track the variables that have been injected. + self._injected_vars.add(name) + @property def vars(self): return self._namespace diff --git a/unittests/test_variables.py b/unittests/test_variables.py index 9eaca96c99..c335a04415 100644 --- a/unittests/test_variables.py +++ b/unittests/test_variables.py @@ -42,6 +42,12 @@ class MyTest(NoVarsTest): name = variable(str) +def test_name_clash_builtin_property(NoVarsTest): + with pytest.raises(ValueError): + class MyTest(NoVarsTest): + current_environ = variable(str) + + def test_redeclare_var_clash(OneVarTest): with pytest.raises(ValueError): class MyTest(OneVarTest): @@ -57,6 +63,17 @@ class MyTest(NoVarsTest, MyMixin): '''Trigger error from inheritance clash.''' +def test_instantiate_and_inherit(OneVarTest): + '''Instantiation will inject the vars as class attributes. + + Ensure that inheriting from this class after the instantiation does not + raise a namespace clash with the vars. + ''' + inst = OneVarTest() + class MyTest(OneVarTest): + pass + + def test_var_space_clash(): class Spam(rfm.RegressionMixin): v0 = variable(int, value=1) @@ -83,12 +100,6 @@ class MyTest(rfm.RegressionTest): v0 = variable(int, value=2) -def test_namespace_clash(NoVarsTest): - with pytest.raises(ValueError): - class MyTest(NoVarsTest): - current_environ = variable(str) - - def test_set_var(OneVarTest): class MyTest(OneVarTest): foo = 4 From 5a09077931c79a85a598014cd636a8b604ce266d Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Fri, 12 Feb 2021 13:23:22 +0100 Subject: [PATCH 31/49] Address PR comments --- docs/regression_test_api.rst | 23 ++--------------------- unittests/test_variables.py | 2 +- 2 files changed, 3 insertions(+), 22 deletions(-) diff --git a/docs/regression_test_api.rst b/docs/regression_test_api.rst index 6ce5d1eaf9..23eba76f95 100644 --- a/docs/regression_test_api.rst +++ b/docs/regression_test_api.rst @@ -87,10 +87,10 @@ For instance, continuing with the example above, one could override the :func:`_ This only has an effect if used with ``inherit_params=True``. -.. py:function:: reframe.core.pipeline.RegressionTest.var(name, *types, value=None, field=None) +.. py:function:: reframe.core.pipeline.RegressionTest.variable(name, *types, value=None, field=None) Inserts a new regression test variable. - If the ``value`` argument is not provided, the variable is considered *declared* but not *defined*. + If argument ``value`` sets the default value for the variable. Thus, a variable may not be declared more than once. However, it is possible to alter a variable's value after it was declared by using the :func:`set_var` and :func:`require_var` directives. Note that a variable must be defined before is referenced in the regression test. Otherwise, an :py:exc:`AttributeError` will be raised. @@ -104,25 +104,6 @@ For instance, continuing with the example above, one could override the :func:`_ :class:`reframe.core.fields.Field`. -.. py:function:: reframe.core.pipeline.RegressionTest.set_var(name, value) - - Assign a value to a regression test variable. The variable must have been defined in a parent class using the :func:`var` directive. - - :param name: the variable name. - :param value: the value assigned to the variable. - - -.. py:function:: reframe.core.pipeline.RegressionTest.require_var(name) - - Undefine a regression test variable. The variable must have been defined in a parent class using the :func:`var` directive. - This method is particularly useful when writing a test library, since it permits to remove any default values that may have been defined for a variable in any of the parent classes. - Effectively, this will force the user of the library to provide the required value for a variable. - However, a variable flagged as *required* which is not referenced in the regression test is implemented as a no-op. - - :param name: the name of the required variable. - - - Environments and Systems ------------------------ diff --git a/unittests/test_variables.py b/unittests/test_variables.py index c335a04415..5022b0934a 100644 --- a/unittests/test_variables.py +++ b/unittests/test_variables.py @@ -96,8 +96,8 @@ class MyTest(rfm.RegressionTest): def test_double_action_on_variable(): with pytest.raises(ValueError): class MyTest(rfm.RegressionTest): - v0 = 2 v0 = variable(int, value=2) + v0 = 2 def test_set_var(OneVarTest): From fa211bae133218e0cbd9078685609b0282f47083 Mon Sep 17 00:00:00 2001 From: Javier Otero <71280927+jjotero@users.noreply.github.com> Date: Fri, 12 Feb 2021 13:48:46 +0100 Subject: [PATCH 32/49] Update docs --- docs/regression_test_api.rst | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/docs/regression_test_api.rst b/docs/regression_test_api.rst index 23eba76f95..7f3a0cb193 100644 --- a/docs/regression_test_api.rst +++ b/docs/regression_test_api.rst @@ -48,7 +48,7 @@ For example, a test can be parameterized using the :func:`parameter` directive a .. code:: python class MyTest(rfm.RegressionTest): - parameter('variant', ['A', 'B']) + variant = parameter(['A', 'B']) def __init__(self): if self.variant == 'A': @@ -71,13 +71,12 @@ For instance, continuing with the example above, one could override the :func:`_ override_other() -.. py:function:: reframe.core.pipeline.RegressionTest.parameter(name, values=None, inherit_params=False, filter_params=None) +.. py:class:: reframe.core.pipeline.RegressionTest.parameter(values=None, inherit_params=False, filter_params=None) Inserts or modifies a regression test parameter. If a parameter with a matching name is already present in the parameter space of a parent class, the existing parameter values will be combined with those provided by this method following the inheritance behaviour set by the arguments ``inherit_params`` and ``filter_params``. Instead, if no parameter with a matching name exists in any of the parent parameter spaces, a new regression test parameter is created. - :param name: The parameter name. :param values: A list containing the parameter values. If no values are passed when creating a new parameter, the parameter is considered as *declared* but not *defined* (i.e. an abstract parameter). Instead, for an existing parameter, this depends on the parameter's inheritance behaviour and on whether any values where provided in any of the parent parameter spaces. @@ -87,14 +86,14 @@ For instance, continuing with the example above, one could override the :func:`_ This only has an effect if used with ``inherit_params=True``. -.. py:function:: reframe.core.pipeline.RegressionTest.variable(name, *types, value=None, field=None) +.. py:class:: reframe.core.pipeline.RegressionTest.variable(*types, value=None, field=None) Inserts a new regression test variable. - If argument ``value`` sets the default value for the variable. - Thus, a variable may not be declared more than once. However, it is possible to alter a variable's value after it was declared by using the :func:`set_var` and :func:`require_var` directives. + The argument ``value`` sets the default value for the variable. + A variable may not be declared more than once. However, it is possible to alter a variable's value after it was declared by simply assigning it a new value directly in the class body. + A variable may be set as required by simply assigning a variable as `required_variable`. Note that a variable must be defined before is referenced in the regression test. Otherwise, an :py:exc:`AttributeError` will be raised. - :param name: the variable name. :param types: the supported types for the variable. :param value: the default value assigned to the variable. :param field: the field validator to be used for this variable. From a04fe23470816464632da6d7a61b08b8ff8fa678 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Fri, 12 Feb 2021 16:04:03 +0100 Subject: [PATCH 33/49] Change required_variable to required --- docs/regression_test_api.rst | 2 +- reframe/core/meta.py | 2 +- unittests/test_variables.py | 7 ++++--- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/docs/regression_test_api.rst b/docs/regression_test_api.rst index 7f3a0cb193..10995aa9af 100644 --- a/docs/regression_test_api.rst +++ b/docs/regression_test_api.rst @@ -91,7 +91,7 @@ For instance, continuing with the example above, one could override the :func:`_ Inserts a new regression test variable. The argument ``value`` sets the default value for the variable. A variable may not be declared more than once. However, it is possible to alter a variable's value after it was declared by simply assigning it a new value directly in the class body. - A variable may be set as required by simply assigning a variable as `required_variable`. + A variable may be set as required by simply assigning a variable as `required`. Note that a variable must be defined before is referenced in the regression test. Otherwise, an :py:exc:`AttributeError` will be raised. :param types: the supported types for the variable. diff --git a/reframe/core/meta.py b/reframe/core/meta.py index 2028c5f919..2b3aaa4ca6 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -48,7 +48,7 @@ def __prepare__(metacls, name, bases, **kwargs): # Directives to add/modify a regression test variable namespace['variable'] = variables.TestVar - namespace['required_variable'] = variables.UndefineVar() + namespace['required'] = variables.UndefineVar() return MetaNamespace(namespace) def __new__(metacls, name, bases, namespace, **kwargs): diff --git a/unittests/test_variables.py b/unittests/test_variables.py index 5022b0934a..c32806f91d 100644 --- a/unittests/test_variables.py +++ b/unittests/test_variables.py @@ -70,6 +70,7 @@ def test_instantiate_and_inherit(OneVarTest): raise a namespace clash with the vars. ''' inst = OneVarTest() + class MyTest(OneVarTest): pass @@ -121,7 +122,7 @@ class MyTest(OneVarTest): def test_require_var(OneVarTest): class MyTest(OneVarTest): - foo = required_variable + foo = required def __init__(self): print(self.foo) @@ -132,7 +133,7 @@ def __init__(self): def test_required_var_not_present(OneVarTest): class MyTest(OneVarTest): - foo = required_variable + foo = required def __init__(self): pass @@ -143,7 +144,7 @@ def __init__(self): def test_require_undeclared_variable(NoVarsTest): with pytest.raises(ValueError): class MyTest(NoVarsTest): - foo = required_variable + foo = required def test_invalid_field(): From eebdb7e024787f99c15cc07fb00667d0fc7cbb1f Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Fri, 12 Feb 2021 16:12:30 +0100 Subject: [PATCH 34/49] PEP8 fixes --- reframe/core/variables.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/reframe/core/variables.py b/reframe/core/variables.py index 0842510250..f542363aed 100644 --- a/reframe/core/variables.py +++ b/reframe/core/variables.py @@ -19,14 +19,17 @@ class _UndefinedType: _Undefined = _UndefinedType() + class VarDirective: '''Base class for the variable directives.''' + class TestVar(VarDirective): '''Regression test variable. Buffer to store a regression test variable declared through directives. ''' + def __init__(self, *args, **kwargs): self.field_type = kwargs.pop('field', fields.TypedField) self.default_value = kwargs.pop('value', _Undefined) @@ -100,8 +103,6 @@ def __init__(self, target_cls=None, illegal_names=None): super().__init__(target_cls, illegal_names) - - def join(self, other, cls): '''Join an existing VarSpace into the current one. From 26f47c9706573b9e2a38ea8fb3d027817524b7d8 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Fri, 12 Feb 2021 16:15:16 +0100 Subject: [PATCH 35/49] Capture error on required variables --- reframe/core/pipeline.py | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 4798f14bf1..d1a4695058 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -346,7 +346,7 @@ def pipeline_hooks(cls): #: :class:`reframe.core.containers.ContainerPlatform`. #: :default: :class:`None`. container_platform = variable(type(None), - field=ContainerPlatformField, value=None) + field=ContainerPlatformField, value=None) #: .. versionadded:: 3.0 #: @@ -556,7 +556,7 @@ def pipeline_hooks(cls): #: The measurement unit is required. The user should explicitly #: specify :class:`None` if no unit is available. reference = variable(typ.Tuple[object, object, object, object], - field=fields.ScopedDictField, value={}) + field=fields.ScopedDictField, value={}) # FIXME: There is not way currently to express tuples of `float`s or # `None`s, so we just use the very generic `object` @@ -597,7 +597,7 @@ def pipeline_hooks(cls): #: :class:`None` is also allowed. #: :default: :class:`None` perf_patterns = variable(typ.Dict[str, _DeferredExpression], - type(None), value=None) + type(None), value=None) #: List of modules to be loaded before running this test. #: @@ -760,6 +760,22 @@ def __new__(cls, *args, _rfm_use_params=False, **kwargs): def __init__(self): pass + def __getattribute__(self, name): + try: + return super().__getattribute__(name) + + except AttributeError as err: + # 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 defined' + ) from None + + else: + super().__getattr__(name) + def _append_parameters_to_name(self): if self._rfm_param_space.params: return '_' + '_'.join([str(self.__dict__[key]) From 704e1eef35314281e5d5f7a362c3926ce0d53916 Mon Sep 17 00:00:00 2001 From: Javier Otero <71280927+jjotero@users.noreply.github.com> Date: Fri, 12 Feb 2021 20:07:33 +0100 Subject: [PATCH 36/49] Add docs on variable and parameter builtins --- docs/regression_test_api.rst | 102 ++++++++++++++++++++++------------- reframe/core/parameters.py | 2 + reframe/core/variables.py | 16 +++--- 3 files changed, 72 insertions(+), 48 deletions(-) diff --git a/docs/regression_test_api.rst b/docs/regression_test_api.rst index 10995aa9af..4564a35d67 100644 --- a/docs/regression_test_api.rst +++ b/docs/regression_test_api.rst @@ -36,33 +36,40 @@ Pipeline Hooks .. autodecorator:: reframe.core.decorators.require_deps -.. _directives: -Directives ----------- +Builtins +-------- -Directives are functions that can be called directly in the body of a ReFrame regression test class. -These functions exert control over the test creation, and they allow adding and/or modifying certain attributes of the regression test. -For example, a test can be parameterized using the :func:`parameter` directive as follows: +ReFrame provides built-in functions that facilitate the creation of extensible tests (i.e. a test library). +These *builtins* are intended to be used directly in the class body of the test, allowing the ReFrame internals to *pre-process* their input before the actual test creation takes place. +This provides the ReFrame internals with further control over the user's input, making the process of writing regression tests less error-prone thanks to a better error checking. +In essence, these builtins exert control over the test creation, and they allow adding and/or modifying certain attributes of the regression test. -.. code:: python +.. py:function:: reframe.core.pipeline.RegressionTest.parameter(values=None, inherit_params=False, filter_params=None) + + Inserts or modifies a regression test parameter. + If a parameter with a matching name is already present in the parameter space of a parent class, the existing parameter values will be combined with those provided by this method following the inheritance behaviour set by the arguments ``inherit_params`` and ``filter_params``. + Instead, if no parameter with a matching name exists in any of the parent parameter spaces, a new regression test parameter is created. + A regression test can be parametrized as follows: - class MyTest(rfm.RegressionTest): + .. code:: python + + class Foo(rfm.RegressionTest): variant = parameter(['A', 'B']) - + def __init__(self): if self.variant == 'A': do_this() else: do_other() -One of the most powerful features about using directives is that they store their input information at the class level. -This means if one were to extend or specialize an existing regression test, the test attribute additions and modifications made through directives in the parent class will be automatically inherited by the child test. -For instance, continuing with the example above, one could override the :func:`__init__` method in the :class:`MyTest` regression test as follows: + One of the most powerful features about these built-in functions is that they store their input information at the class level. + This means if one were to extend or specialize an existing regression test, the test attribute additions and modifications made through built-in functions in the parent class will be automatically inherited by the child test. + For instance, continuing with the example above, one could override the :func:`__init__` method in the :class:`MyTest` regression test as follows: -.. code:: python + .. code:: python - class MyModifiedTest(MyTest): + class Bar(Foo): def __init__(self): if self.variant == 'A': @@ -70,37 +77,56 @@ For instance, continuing with the example above, one could override the :func:`_ else: override_other() + :param values: A list containing the parameter values. + If no values are passed when creating a new parameter, the parameter is considered as *declared* but not *defined* (i.e. an abstract parameter). + Instead, for an existing parameter, this depends on the parameter's inheritance behaviour and on whether any values where provided in any of the parent parameter spaces. + :param inherit_params: If :obj:`False`, no parameter values that may have been defined in any of the parent parameter spaces will be inherited. + :param filter_params: Function to filter/modify the inherited parameter values that may have been provided in any of the parent parameter spaces. + This function must accept a single argument, which will be passed as an iterable containing the inherited parameter values. + This only has an effect if used with ``inherit_params=True``. + -.. py:class:: reframe.core.pipeline.RegressionTest.parameter(values=None, inherit_params=False, filter_params=None) +.. py:function:: reframe.core.pipeline.RegressionTest.variable(*types, value=None, field=None) - Inserts or modifies a regression test parameter. - If a parameter with a matching name is already present in the parameter space of a parent class, the existing parameter values will be combined with those provided by this method following the inheritance behaviour set by the arguments ``inherit_params`` and ``filter_params``. - Instead, if no parameter with a matching name exists in any of the parent parameter spaces, a new regression test parameter is created. + Inserts a new regression test variable. - :param values: A list containing the parameter values. - If no values are passed when creating a new parameter, the parameter is considered as *declared* but not *defined* (i.e. an abstract parameter). - Instead, for an existing parameter, this depends on the parameter's inheritance behaviour and on whether any values where provided in any of the parent parameter spaces. - :param inherit_params: If :obj:`False`, no parameter values that may have been defined in any of the parent parameter spaces will be inherited. - :param filter_params: Function to filter/modify the inherited parameter values that may have been provided in any of the parent parameter spaces. - This function must accept a single argument, which will be passed as an iterable containing the inherited parameter values. - This only has an effect if used with ``inherit_params=True``. + .. code:: python + class Foo(rfm.RegressionTest): + my_var = variable(int, value = 8) + + def __init__(self): + print(self.my_var) # prints 8. -.. py:class:: reframe.core.pipeline.RegressionTest.variable(*types, value=None, field=None) + The argument ``value`` sets the default value for the variable. + A variable may not be declared more than once. However, it is possible to alter a variable's value after it was declared by simply assigning it a new value directly in the class body. - Inserts a new regression test variable. - The argument ``value`` sets the default value for the variable. - A variable may not be declared more than once. However, it is possible to alter a variable's value after it was declared by simply assigning it a new value directly in the class body. - A variable may be set as required by simply assigning a variable as `required`. - Note that a variable must be defined before is referenced in the regression test. Otherwise, an :py:exc:`AttributeError` will be raised. + .. code:: python - :param types: the supported types for the variable. - :param value: the default value assigned to the variable. - :param field: the field validator to be used for this variable. - If no field argument is provided, it defaults to - :class:`reframe.core.fields.TypedField`. - Note that the field validator provided by this argument must derive from - :class:`reframe.core.fields.Field`. + class Bar(Foo): + my_var = 4 + + def __init__(self): + print(self.my_var) # prints 4. + + A variable may be set as required by simply assigning a variable as `required`. + Note that a variable must be defined before is referenced in the regression test. Otherwise, an :py:exc:`AttributeError` will be raised. + + .. code:: python + + class Baz(Bar): + my_var = required + + def __init__(self): + print(self.my_var) # throws an AttributeError. + + :param types: the supported types for the variable. + :param value: the default value assigned to the variable. + :param field: the field validator to be used for this variable. + If no field argument is provided, it defaults to + :class:`reframe.core.fields.TypedField`. + Note that the field validator provided by this argument must derive from + :class:`reframe.core.fields.Field`. Environments and Systems diff --git a/reframe/core/parameters.py b/reframe/core/parameters.py index dc7d47dd03..c590ce45c9 100644 --- a/reframe/core/parameters.py +++ b/reframe/core/parameters.py @@ -21,6 +21,8 @@ class TestParam: values, and inheritance behaviour. This class should be thought of as a temporary storage for these parameter attributes, before the full final parameter space is built. + + :meta private: ''' def __init__(self, values=None, diff --git a/reframe/core/variables.py b/reframe/core/variables.py index f542363aed..5e108dbc8b 100644 --- a/reframe/core/variables.py +++ b/reframe/core/variables.py @@ -25,9 +25,13 @@ class VarDirective: class TestVar(VarDirective): - '''Regression test variable. + '''Regression test variable class. - Buffer to store a regression test variable declared through directives. + Stores the attributes of a variable when defined directly in the class + body. Instances of this class are injected into the regression test + during class instantiation. + + :meta private: ''' def __init__(self, *args, **kwargs): @@ -53,14 +57,6 @@ def define(self, value): self.default_value = value def __set_name__(self, owner, name): - '''Overwrite the dummy name. - - If the variable was created directly by assignment in the test class, - this function assigns the variable the name used in the test class body - and inserts the variable in the test's local variable space. To avoid - any namespace collisions, this function also disowns the test class - (owner argument) from this variable. - ''' self.name = name From b8bcdf7dd556cf3929b88b79c767a4db9bb40638 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Mon, 15 Feb 2021 15:53:50 +0100 Subject: [PATCH 37/49] Add PR comments --- reframe/core/meta.py | 52 +++++++++++++++++++++++++----------- reframe/core/namespaces.py | 6 ++--- reframe/core/parameters.py | 10 +++++++ reframe/core/pipeline.py | 6 ++--- reframe/core/variables.py | 33 +++++++++++------------ unittests/test_parameters.py | 6 +++++ unittests/test_variables.py | 7 +++-- 7 files changed, 78 insertions(+), 42 deletions(-) diff --git a/reframe/core/meta.py b/reframe/core/meta.py index 2b3aaa4ca6..f3d85cb155 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -8,28 +8,28 @@ # +import copy + from reframe.core.exceptions import ReframeSyntaxError import reframe.core.namespaces as namespaces import reframe.core.parameters as parameters import reframe.core.variables as variables -class MetaNamespace(namespaces.LocalNamespace): - '''Regression test's namespace to control the cls attribute assignment.''' - def __setitem__(self, key, value): - if isinstance(value, variables.VarDirective): - # Insert the attribute in the variable namespace - self['_rfm_local_var_space'][key] = value - - elif isinstance(value, parameters.TestParam): - # Insert the attribute in the parameter namespace - self['_rfm_local_param_space'][key] = value - - else: - super().__setitem__(key, value) +class RegressionTestMeta(type): + class MetaNamespace(namespaces.LocalNamespace): + '''Custom namespace to control the cls attribute assignment.''' + def __setitem__(self, key, value): + if isinstance(value, variables.VarDirective): + # Insert the attribute in the variable namespace + self['_rfm_local_var_space'][key] = value + elif isinstance(value, parameters.TestParam): + # Insert the attribute in the parameter namespace + self['_rfm_local_param_space'][key] = value + else: + super().__setitem__(key, value) -class RegressionTestMeta(type): @classmethod def __prepare__(metacls, name, bases, **kwargs): namespace = super().__prepare__(name, bases, **kwargs) @@ -49,7 +49,7 @@ def __prepare__(metacls, name, bases, **kwargs): # Directives to add/modify a regression test variable namespace['variable'] = variables.TestVar namespace['required'] = variables.UndefineVar() - return MetaNamespace(namespace) + return metacls.MetaNamespace(namespace) def __new__(metacls, name, bases, namespace, **kwargs): return super().__new__(metacls, name, bases, dict(namespace), **kwargs) @@ -58,7 +58,12 @@ def __init__(cls, name, bases, namespace, **kwargs): super().__init__(name, bases, namespace, **kwargs) # Create a set with the attribute names already in use. - used_attribute_names = set(dir(cls)) - set(cls.__dict__) + cls._rfm_dir = set() + for base in bases: + if hasattr(base, '_rfm_dir'): + cls._rfm_dir.update(base._rfm_dir) + + used_attribute_names = copy.deepcopy(cls._rfm_dir) # Build the var space and extend the target namespace variables.VarSpace(cls, used_attribute_names) @@ -67,6 +72,9 @@ def __init__(cls, name, bases, namespace, **kwargs): # Build the parameter space parameters.ParamSpace(cls, used_attribute_names) + # Update used names set with the local __dict__ + cls._rfm_dir.update(cls.__dict__) + # Set up the hooks for the pipeline stages based on the _rfm_attach # attribute; all dependencies will be resolved first in the post-setup # phase if not assigned elsewhere @@ -134,6 +142,18 @@ def __call__(cls, *args, **kwargs): obj.__init__(*args, **kwargs) return obj + def __getattribute__(cls, name): + try: + return super().__getattribute__(name) + except AttributeError: + try: + return cls._rfm_local_var_space[name] + except KeyError: + try: + return cls._rfm_local_param_space[name] + except KeyError: + return super().__getattr__(name) + @property def param_space(cls): # Make the parameter space available as read-only diff --git a/reframe/core/namespaces.py b/reframe/core/namespaces.py index 757e028c7c..c67c9f0866 100644 --- a/reframe/core/namespaces.py +++ b/reframe/core/namespaces.py @@ -31,7 +31,7 @@ class B(A): ''' def __init__(self, namespace=None): - self._namespace = namespace if namespace else {} + self._namespace = namespace or {} def __getattr__(self, name): return getattr(self._namespace, name) @@ -55,7 +55,7 @@ def __len__(self): return len(self._namespace) def __repr__(self): - return '{}({!r})'.format(type(self).__name__, self._namespace) + return f'{type(self).__name__}({self._namespace!r})' def _raise_namespace_clash(self, name): '''Raise an error if there is a namespace clash.''' @@ -64,7 +64,7 @@ def _raise_namespace_clash(self, name): ) -class Namespace(metaclass=abc.ABCMeta): +class Namespace: '''Namespace of a regression test. The final namespace may be built by inheriting namespaces from diff --git a/reframe/core/parameters.py b/reframe/core/parameters.py index dc7d47dd03..6f87c55aa4 100644 --- a/reframe/core/parameters.py +++ b/reframe/core/parameters.py @@ -126,6 +126,16 @@ def extend(self, cls): p.filter_params(self.params.get(name, ())) + p.values ) + # If any previously declared parameter was defined in the class body + # by directly assigning it a value, raise an error. Parameters must be + # changed using the `x = parameter([...])` syntax. + for key, values in cls.__dict__.items(): + if key in self.params: + raise ValueError( + f'parameter {key!r} must be modified through the built-in ' + f'parameter type' + ) + def inject(self, obj, cls=None, use_params=False): '''Insert the params in the regression test. diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index d1a4695058..48432fa268 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -763,16 +763,14 @@ def __init__(self): def __getattribute__(self, name): try: return super().__getattribute__(name) - - except AttributeError as err: + 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 defined' + f'required variable {name!r} has not been set' ) from None - else: super().__getattr__(name) diff --git a/reframe/core/variables.py b/reframe/core/variables.py index f542363aed..377e6cedc5 100644 --- a/reframe/core/variables.py +++ b/reframe/core/variables.py @@ -64,11 +64,6 @@ def __set_name__(self, owner, name): self.name = name -class DefineVar(VarDirective): - def __init__(self, default_value): - self.default_value = default_value - - class UndefineVar(VarDirective): def __init__(self): self.default_value = _Undefined @@ -100,7 +95,6 @@ def namespace_name(self): def __init__(self, target_cls=None, illegal_names=None): # Set to register the variables already injected in the class self._injected_vars = set() - super().__init__(target_cls, illegal_names) def join(self, other, cls): @@ -137,36 +131,41 @@ def extend(self, cls): is disallowed. ''' local_varspace = getattr(cls, self.local_namespace_name) - for key, var in local_varspace.items(): if isinstance(var, TestVar): # Disable redeclaring a variable if key in self.vars: raise ValueError( - f'cannot redeclare a variable ({key})' + f'cannot redeclare the variable {key!r}' ) # Add a new var self.vars[key] = var - elif isinstance(var, VarDirective): - # Modify the value of a previously declarated var + # Modify the value of a previously declared var. + # If var is an instance of UndefineVar, we set its default + # value to _Undefined. Alternatively, the value is just updated + # with the user's input. self._check_var_is_declared(key) self.vars[key].define(var.default_value) # If any previously declared variable was defined in the class body - # retrieve the value from the class namespace and update it into the - # variable space. + # 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 local_varspace: raise ValueError( - f'cannot specify more than one action on variable' - f' {key!r} in the same class' + f'cannot specify more than one action on variable ' + f'{key!r} in the same class' ) - elif key in self.vars: - self._check_var_is_declared(key) self.vars[key].define(value) + _assigned_vars.add(key) + + # Delete the vars from the class __dict__. + for key in _assigned_vars: + delattr(cls, key) def _check_var_is_declared(self, key): if key not in self.vars: @@ -185,7 +184,7 @@ def sanity(self, cls, illegal_names=None): illegal_names = set(dir(cls)) for key in self._namespace: - if (key in illegal_names) and (key not in self._injected_vars): + if key in illegal_names and key not in self._injected_vars: raise ValueError( f'{key!r} already defined in class ' f'{cls.__qualname__!r}' diff --git a/unittests/test_parameters.py b/unittests/test_parameters.py index cec3e4df6a..1e265f0ee1 100644 --- a/unittests/test_parameters.py +++ b/unittests/test_parameters.py @@ -197,3 +197,9 @@ def test_double_declare(): class MyTest(rfm.RegressionTest): P0 = parameter([1, 2, 3]) P0 = parameter() + + +def test_overwrite_param(): + with pytest.raises(ValueError): + class MyTest(TwoParams): + P0 = [1,2,3] diff --git a/unittests/test_variables.py b/unittests/test_variables.py index c32806f91d..a4cc2e6a16 100644 --- a/unittests/test_variables.py +++ b/unittests/test_variables.py @@ -28,7 +28,8 @@ class OneVarTest(NoVarsTest): def test_custom_variable(OneVarTest): - assert not hasattr(OneVarTest, 'foo') + assert hasattr(OneVarTest, 'foo') + assert not isinstance(OneVarTest.foo, Field) inst = OneVarTest() assert hasattr(OneVarTest, 'foo') assert isinstance(OneVarTest.foo, Field) @@ -106,8 +107,10 @@ class MyTest(OneVarTest): foo = 4 inst = MyTest() - assert not hasattr(OneVarTest, 'foo') + assert hasattr(OneVarTest, 'foo') + assert not isinstance(OneVarTest.foo, Field) assert hasattr(MyTest, 'foo') + assert isinstance(MyTest.foo, Field) assert hasattr(inst, 'foo') assert inst.foo == 4 From 10b9657ae77074b43485c0391d06f02f4ec3c4a0 Mon Sep 17 00:00:00 2001 From: Javier Otero <71280927+jjotero@users.noreply.github.com> Date: Mon, 15 Feb 2021 17:31:20 +0100 Subject: [PATCH 38/49] PEP8 fixes --- unittests/test_parameters.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unittests/test_parameters.py b/unittests/test_parameters.py index 1e265f0ee1..48d95087a7 100644 --- a/unittests/test_parameters.py +++ b/unittests/test_parameters.py @@ -202,4 +202,4 @@ class MyTest(rfm.RegressionTest): def test_overwrite_param(): with pytest.raises(ValueError): class MyTest(TwoParams): - P0 = [1,2,3] + P0 = [1, 2, 3] From 94a6cbc07b58c311baa27a49f7ac24848659c32f Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Tue, 16 Feb 2021 12:29:11 +0100 Subject: [PATCH 39/49] Expand the comments in the metaclass --- docs/regression_test_api.rst | 2 ++ reframe/core/meta.py | 13 ++++++++++--- unittests/test_variables.py | 4 ++++ 3 files changed, 16 insertions(+), 3 deletions(-) diff --git a/docs/regression_test_api.rst b/docs/regression_test_api.rst index 4564a35d67..dadfae0843 100644 --- a/docs/regression_test_api.rst +++ b/docs/regression_test_api.rst @@ -40,6 +40,8 @@ Pipeline Hooks Builtins -------- +..versionadded:3.5 + ReFrame provides built-in functions that facilitate the creation of extensible tests (i.e. a test library). These *builtins* are intended to be used directly in the class body of the test, allowing the ReFrame internals to *pre-process* their input before the actual test creation takes place. This provides the ReFrame internals with further control over the user's input, making the process of writing regression tests less error-prone thanks to a better error checking. diff --git a/reframe/core/meta.py b/reframe/core/meta.py index f3d85cb155..1b4a71aaca 100644 --- a/reframe/core/meta.py +++ b/reframe/core/meta.py @@ -8,8 +8,6 @@ # -import copy - from reframe.core.exceptions import ReframeSyntaxError import reframe.core.namespaces as namespaces import reframe.core.parameters as parameters @@ -63,7 +61,7 @@ def __init__(cls, name, bases, namespace, **kwargs): if hasattr(base, '_rfm_dir'): cls._rfm_dir.update(base._rfm_dir) - used_attribute_names = copy.deepcopy(cls._rfm_dir) + used_attribute_names = set(cls._rfm_dir) # Build the var space and extend the target namespace variables.VarSpace(cls, used_attribute_names) @@ -143,6 +141,15 @@ def __call__(cls, *args, **kwargs): return obj def __getattribute__(cls, name): + ''' Attribute lookup method for the MetaNamespace. + + This metaclass implements a custom namespace, where built-in `variable` + and `parameter` types are stored in their own sub-namespaces (see + :class:`reframe.core.meta.RegressionTestMeta.MetaNamespace`). + This method will perform an attribute lookup on these sub-namespaces if + a call to the default `__getattribute__` method fails to retrieve the + requested class attribute. + ''' try: return super().__getattribute__(name) except AttributeError: diff --git a/unittests/test_variables.py b/unittests/test_variables.py index a4cc2e6a16..24640ed5f9 100644 --- a/unittests/test_variables.py +++ b/unittests/test_variables.py @@ -13,6 +13,10 @@ @pytest.fixture def NoVarsTest(): + '''Variables are injected as descriptors in the classes. + + Thus, fixtures are needed to provide a fresh class to each test. + ''' class NoVarsTest(rfm.RegressionTest): pass From 3175d11867ad5a1bdc2b09de747de91bf3e6608f Mon Sep 17 00:00:00 2001 From: Javier Otero <71280927+jjotero@users.noreply.github.com> Date: Tue, 16 Feb 2021 17:36:02 +0100 Subject: [PATCH 40/49] Update docs --- docs/regression_test_api.rst | 75 +++++++++++++++++++++++++++++------- 1 file changed, 62 insertions(+), 13 deletions(-) diff --git a/docs/regression_test_api.rst b/docs/regression_test_api.rst index dadfae0843..8d544de552 100644 --- a/docs/regression_test_api.rst +++ b/docs/regression_test_api.rst @@ -40,17 +40,17 @@ Pipeline Hooks Builtins -------- -..versionadded:3.5 - ReFrame provides built-in functions that facilitate the creation of extensible tests (i.e. a test library). These *builtins* are intended to be used directly in the class body of the test, allowing the ReFrame internals to *pre-process* their input before the actual test creation takes place. This provides the ReFrame internals with further control over the user's input, making the process of writing regression tests less error-prone thanks to a better error checking. In essence, these builtins exert control over the test creation, and they allow adding and/or modifying certain attributes of the regression test. +.. versionadded:: 3.5 + .. py:function:: reframe.core.pipeline.RegressionTest.parameter(values=None, inherit_params=False, filter_params=None) Inserts or modifies a regression test parameter. - If a parameter with a matching name is already present in the parameter space of a parent class, the existing parameter values will be combined with those provided by this method following the inheritance behaviour set by the arguments ``inherit_params`` and ``filter_params``. + If a parameter with a matching name is already present in the parameter space of a parent class, the existing parameter values will be combined with those provided by this method following the inheritance behavior set by the arguments ``inherit_params`` and ``filter_params``. Instead, if no parameter with a matching name exists in any of the parent parameter spaces, a new regression test parameter is created. A regression test can be parametrized as follows: @@ -79,6 +79,30 @@ In essence, these builtins exert control over the test creation, and they allow else: override_other() + Note that this built-in parameter function provides an alternative method to parameterize a test to :func:`reframe.core.decorators.parameterized_test`, and the use of both approaches in the same test is currently disallowed. + The two main advantages of the built-in :func:`parameter` over the decorated approach reside in the parameter inheritance across classes and the handling of large parameter sets. + As shown in the example above, the parameters declared with the built-in :func:`parameter` are automatically carried over into derived tests through class inheritance, whereas tests using the decorated approach would have to redefine the parameters on every test. + Similarly, parameters declared through the built-in :func:`parameter` are regarded as fully independent from each other and ReFrame will automatically generate as many tests as available parameter combinations. This is a major advantage over the decorated approach, where one would have to manually expand the parameter combinations. + This is illustrated in the example below, consisting of a case with two parameters, each having two possible values. + + .. code:: python + + # Parameterized test with two parameters (p0 = ['a', 'b'] and p1 = ['x', 'y']) + @rfm.parameterized_test(['a','x'], ['a','y'], ['b','x'], ['b', 'y']) + class Foo(rfm.RegressionTest): + def __init__(self, p0, p1): + do_something(p0, p1) + + # This is easier to write with the parameter built-in. + @rfm.simple_test + class Bar(rfm.RegressionTest): + p0 = parameter(['a', 'b']) + p1 = parameter(['x', 'y']) + + def __init__(self): + do_something(self.p0, self.p1) + + :param values: A list containing the parameter values. If no values are passed when creating a new parameter, the parameter is considered as *declared* but not *defined* (i.e. an abstract parameter). Instead, for an existing parameter, this depends on the parameter's inheritance behaviour and on whether any values where provided in any of the parent parameter spaces. @@ -91,39 +115,64 @@ In essence, these builtins exert control over the test creation, and they allow .. py:function:: reframe.core.pipeline.RegressionTest.variable(*types, value=None, field=None) Inserts a new regression test variable. + Declaring a test variable through the :func:`variable` built-in allows for a more robust test implementation than if the variables were just defined as regular test attributes by direct assignment in the body of the test (e.g. ``self.a = 10``). + Using variables declared through the :func:`variable` built-in guarantees that these regression test variables will not be redeclared by any child class, while also ensuring that any values that may be assigned to such variables comply with its original declaration. + In essence, by using test variables, the user removes any potential test errors that might be caused by accidentally overriding a class attribute. See the example below. + .. code:: python class Foo(rfm.RegressionTest): - my_var = variable(int, value = 8) + my_var = variable(int, value=8) + not_a_var = 4 def __init__(self): print(self.my_var) # prints 8. + # self.my_var = 'override' # Error: my_var must be an int! + self.not_a_var = 'override' # However, this would work. Dangerous! - The argument ``value`` sets the default value for the variable. - A variable may not be declared more than once. However, it is possible to alter a variable's value after it was declared by simply assigning it a new value directly in the class body. + The argument ``value`` in the :func:`variable` built-in sets the default value for the variable. + As mentioned above, a variable may not be declared more than once, but its value can be updated by simply assigning it a new value directly in the class body. .. code:: python class Bar(Foo): my_var = 4 + # my_var = 'override' # Error again! def __init__(self): print(self.my_var) # prints 4. - A variable may be set as required by simply assigning a variable as `required`. - Note that a variable must be defined before is referenced in the regression test. Otherwise, an :py:exc:`AttributeError` will be raised. + Here, the class :class:`Bar` inherits the variables from :class:`Foo` and can see that ``my_var`` has already been declared in the parent class. Therefore, the value of ``my_var`` is updated ensuring that the new value complies to the original variable declaration. + + These examples above assumed that a default value can be provided to the variables in the bases tests, but that might not always be the case. + For example, when writing a test library, one might want to leave some variables undefined and force the user to set these when using the test. + To this end, a variable may be set as ``required``, which forces the derived test to set the variable before it is referenced. + Note that, if a value is not provided when declaring a new variable, the variable is set as required by default. + One possible example that illustrates the use of the ``required`` keyword is a test library with a test that runs a parallel code. + This test is system-agnostic, so the library developer cannot possibly know the number of tasks the user will require. .. code:: python - class Baz(Bar): - my_var = required + # Test as written in the library + class MyBaseTest(rfm.RegressionTest): + # Require the user to provide a value for num_tasks. + num_tasks = required - def __init__(self): - print(self.my_var) # throws an AttributeError. + def __init__(self): + self.executable = './my_parallel_code.x' + + + # Test as written by the user + class MyTest(MyBaseTest): + # Set the number of tasks + num_tasks = 256 + + Here ``num_tasks`` was already declared in the :class:`rfm.RegressionTest` class, so we just need to set it as ``required``. + The value of a required variable may be set either directly in the class body, on the :func:`__init__` method, or in any other hook before it is referenced. Otherwise an error will be raised indicating that a required variable has not been set. :param types: the supported types for the variable. - :param value: the default value assigned to 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 :class:`reframe.core.fields.TypedField`. From be2e0e9690edf40d7136941d0d51d9258c7178b6 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Wed, 17 Feb 2021 11:34:04 +0100 Subject: [PATCH 41/49] Deep-copy vars and params on instantiation --- reframe/core/parameters.py | 6 +++++- reframe/core/variables.py | 15 +++++++++++---- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/reframe/core/parameters.py b/reframe/core/parameters.py index 26a580e96e..3a35dc5f7e 100644 --- a/reframe/core/parameters.py +++ b/reframe/core/parameters.py @@ -7,6 +7,7 @@ # Functionality to build extensible parameterized tests. # +import copy import functools import itertools @@ -180,9 +181,12 @@ def inject(self, obj, cls=None, use_params=False): def __iter__(self): '''Create a generator object to iterate over the parameter space + The parameters must be deep-copied to prevent an instance from + modifying the class parameter space. + :return: generator object to iterate over the parameter space. ''' - yield from itertools.product(*(p for p in self.params.values())) + yield from itertools.product(*(copy.deepcopy(p) for p in self.params.values())) @property def params(self): diff --git a/reframe/core/variables.py b/reframe/core/variables.py index ca1786a56e..ef1fce3603 100644 --- a/reframe/core/variables.py +++ b/reframe/core/variables.py @@ -7,6 +7,7 @@ # Functionality to build extensible variable spaces into ReFrame tests. # +import copy import reframe.core.namespaces as namespaces import reframe.core.fields as fields @@ -36,7 +37,7 @@ class TestVar(VarDirective): def __init__(self, *args, **kwargs): self.field_type = kwargs.pop('field', fields.TypedField) - self.default_value = kwargs.pop('value', _Undefined) + self._default_value = kwargs.pop('value', _Undefined) if not issubclass(self.field_type, fields.Field): raise ValueError( @@ -48,17 +49,23 @@ def __init__(self, *args, **kwargs): self.kwargs = kwargs def is_defined(self): - return self.default_value is not _Undefined + return self._default_value is not _Undefined def undefine(self): - self.default_value = _Undefined + self._default_value = _Undefined def define(self, value): - self.default_value = value + self._default_value = value def __set_name__(self, owner, name): self.name = name + @property + def default_value(self): + # Variables must be returned by-value to prevent an instance from + # modifying the class variable space. + return copy.deepcopy(self._default_value) + class UndefineVar(VarDirective): def __init__(self): From dc1c213230d6a251096bb0703fb6ab35e3a7b035 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Wed, 17 Feb 2021 14:21:08 +0100 Subject: [PATCH 42/49] Add unit tests for var and param spaces copies --- reframe/core/parameters.py | 8 +++++--- unittests/test_parameters.py | 25 +++++++++++++++++++++++++ unittests/test_variables.py | 23 +++++++++++++++++++++++ 3 files changed, 53 insertions(+), 3 deletions(-) diff --git a/reframe/core/parameters.py b/reframe/core/parameters.py index 3a35dc5f7e..5cf5fccf04 100644 --- a/reframe/core/parameters.py +++ b/reframe/core/parameters.py @@ -108,8 +108,8 @@ def join(self, other, cls): # could be doubly defined and lead to repeated # values if (key in self.params and - self.params[key] != () and - other.params[key] != ()): + self.params[key] != () and + other.params[key] != ()): raise ValueError( f'parameter space conflict: ' @@ -186,7 +186,9 @@ def __iter__(self): :return: generator object to iterate over the parameter space. ''' - yield from itertools.product(*(copy.deepcopy(p) for p in self.params.values())) + yield from itertools.product( + *(copy.deepcopy(p) for p in self.params.values()) + ) @property def params(self): diff --git a/unittests/test_parameters.py b/unittests/test_parameters.py index 48d95087a7..4c13386d8f 100644 --- a/unittests/test_parameters.py +++ b/unittests/test_parameters.py @@ -203,3 +203,28 @@ def test_overwrite_param(): with pytest.raises(ValueError): class MyTest(TwoParams): P0 = [1, 2, 3] + + +def test_param_deepcopy(): + '''Test that there is no cross-class pollution. + + Each instance must deal with its own copies of the parameters. + ''' + class MyParam: + def __init__(self, val): + self.val = val + + class Base(rfm.RegressionTest): + p0 = parameter([MyParam(1), MyParam(2)]) + + class Foo(Base): + def __init__(self): + self.p0.val = -20 + + class Bar(Base): + pass + + assert Foo(_rfm_use_params=True).p0.val == -20 + assert Foo(_rfm_use_params=True).p0.val == -20 + assert Bar(_rfm_use_params=True).p0.val == 1 + assert Bar(_rfm_use_params=True).p0.val == 2 diff --git a/unittests/test_variables.py b/unittests/test_variables.py index 24640ed5f9..b095c8142b 100644 --- a/unittests/test_variables.py +++ b/unittests/test_variables.py @@ -161,3 +161,26 @@ class Foo: with pytest.raises(ValueError): class MyTest(rfm.RegressionTest): a = variable(int, value=4, field=Foo) + + +def test_var_deepcopy(): + '''Test that there is no cross-class pollution. + + Each instance must have its own copies of each variable. + ''' + class MyType: + def __init__(self, val): + self.val = val + + class Base(rfm.RegressionTest): + my_var = variable(MyType, value=MyType(3)) + + class Foo(Base): + def __init__(self): + self.my_var.val = -2 + + class Bar(Base): + pass + + assert Foo().my_var.val == -2 + assert Bar().my_var.val == 3 From bd45f65c7914fdaaef4f02c2ee4b71ae442b1389 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Wed, 17 Feb 2021 15:58:55 +0100 Subject: [PATCH 43/49] Update variable docs --- docs/regression_test_api.rst | 43 ++++++++++++++++++++++++------------ 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/docs/regression_test_api.rst b/docs/regression_test_api.rst index 8d544de552..12289f61a1 100644 --- a/docs/regression_test_api.rst +++ b/docs/regression_test_api.rst @@ -147,29 +147,44 @@ In essence, these builtins exert control over the test creation, and they allow These examples above assumed that a default value can be provided to the variables in the bases tests, but that might not always be the case. For example, when writing a test library, one might want to leave some variables undefined and force the user to set these when using the test. - To this end, a variable may be set as ``required``, which forces the derived test to set the variable before it is referenced. - Note that, if a value is not provided when declaring a new variable, the variable is set as required by default. - One possible example that illustrates the use of the ``required`` keyword is a test library with a test that runs a parallel code. - This test is system-agnostic, so the library developer cannot possibly know the number of tasks the user will require. + As shown in the example below, imposing such requirement is as simple as not passing any ``value`` to the :func:`variable` built-in, which marks the given variable as ``required``. .. code:: python # Test as written in the library - class MyBaseTest(rfm.RegressionTest): - # Require the user to provide a value for num_tasks. - num_tasks = required + class EchoBaseTest(rfm.RunOnlyRegressionTest): + what = variable(str) def __init__(self): - self.executable = './my_parallel_code.x' + self.valid_systems = ['*'] + self.valid_prog_environs = ['PrgEnv-gnu'] + self.executable = f'echo {self.what}' + self.sanity_patterns = sn.assert_found(fr'{self.what}') + + # Test as written by the user: + @rfm.simple_test + class HelloTest(EchoBaseTest): + what = 'Hello' + + # A parametrized test with type-checking + @rfm.simple_test + class FoodTest(EchoBaseTest): + param = parameter(['Bacon', 'Eggs']) + + def __init__(self): + self.what = self.param + super().__init__() - # Test as written by the user - class MyTest(MyBaseTest): - # Set the number of tasks - num_tasks = 256 + Similarly to a variable with a value already assigned to it, the value of a required variable may be set either directly in the class body, on the :func:`__init__` method, or in any other hook before it is referenced. + Otherwise an error will be raised indicating that a required variable has not been set. + Conversely, a variable with a default value already assigned to it can be made required by assigning it the ``required`` keyword. + + .. code:: python + class MyRequiredTest(HelloTest): + what = required - Here ``num_tasks`` was already declared in the :class:`rfm.RegressionTest` class, so we just need to set it as ``required``. - The value of a required variable may be set either directly in the class body, on the :func:`__init__` method, or in any other hook before it is referenced. Otherwise an error will be raised indicating that a required variable has not been set. + Running the above test will cause the :func:`__init__` method from :class:`EchoBaseTest` to throw an error indicating that the variable ``what`` has not been set. :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``. From 03f33953b17ae13d825a321b493d67afd7c473ff Mon Sep 17 00:00:00 2001 From: Javier Otero <71280927+jjotero@users.noreply.github.com> Date: Wed, 17 Feb 2021 16:22:05 +0100 Subject: [PATCH 44/49] More updates on the variable docs --- docs/regression_test_api.rst | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/docs/regression_test_api.rst b/docs/regression_test_api.rst index 12289f61a1..7b9b3ab320 100644 --- a/docs/regression_test_api.rst +++ b/docs/regression_test_api.rst @@ -72,7 +72,6 @@ In essence, these builtins exert control over the test creation, and they allow .. code:: python class Bar(Foo): - def __init__(self): if self.variant == 'A': override_this() @@ -112,10 +111,10 @@ In essence, these builtins exert control over the test creation, and they allow This only has an effect if used with ``inherit_params=True``. -.. py:function:: reframe.core.pipeline.RegressionTest.variable(*types, value=None, field=None) +.. py:function:: reframe.core.pipeline.RegressionTest.variable(*types, value=None) Inserts a new regression test variable. - Declaring a test variable through the :func:`variable` built-in allows for a more robust test implementation than if the variables were just defined as regular test attributes by direct assignment in the body of the test (e.g. ``self.a = 10``). + Declaring a test variable through the :func:`variable` built-in allows for a more robust test implementation than if the variables were just defined as regular test attributes (e.g. ``self.a = 10``). Using variables declared through the :func:`variable` built-in guarantees that these regression test variables will not be redeclared by any child class, while also ensuring that any values that may be assigned to such variables comply with its original declaration. In essence, by using test variables, the user removes any potential test errors that might be caused by accidentally overriding a class attribute. See the example below. @@ -161,11 +160,13 @@ In essence, these builtins exert control over the test creation, and they allow self.executable = f'echo {self.what}' self.sanity_patterns = sn.assert_found(fr'{self.what}') - # Test as written by the user: + + # Test as written by the user @rfm.simple_test class HelloTest(EchoBaseTest): what = 'Hello' + # A parametrized test with type-checking @rfm.simple_test class FoodTest(EchoBaseTest): @@ -181,8 +182,10 @@ In essence, these builtins exert control over the test creation, and they allow Conversely, a variable with a default value already assigned to it can be made required by assigning it the ``required`` keyword. .. code:: python + class MyRequiredTest(HelloTest): - what = required + what = required + Running the above test will cause the :func:`__init__` method from :class:`EchoBaseTest` to throw an error indicating that the variable ``what`` has not been set. From 7b55d652edb6af347d769d3a0f1ae609933affd0 Mon Sep 17 00:00:00 2001 From: Javier Otero <71280927+jjotero@users.noreply.github.com> Date: Wed, 17 Feb 2021 16:40:34 +0100 Subject: [PATCH 45/49] Add meta abc.ABCMeta to Namespace class --- reframe/core/namespaces.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/reframe/core/namespaces.py b/reframe/core/namespaces.py index c67c9f0866..13ce9dc62c 100644 --- a/reframe/core/namespaces.py +++ b/reframe/core/namespaces.py @@ -64,7 +64,7 @@ def _raise_namespace_clash(self, name): ) -class Namespace: +class Namespace(metaclass=abc.ABCMeta): '''Namespace of a regression test. The final namespace may be built by inheriting namespaces from From f014c4ea7863939acc481f3d65eccd52db926bcd Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Thu, 18 Feb 2021 11:04:26 +0100 Subject: [PATCH 46/49] Improve docs --- docs/regression_test_api.rst | 8 +++++--- reframe/core/parameters.py | 4 ++-- unittests/test_variables.py | 12 ++++-------- 3 files changed, 11 insertions(+), 13 deletions(-) diff --git a/docs/regression_test_api.rst b/docs/regression_test_api.rst index 7b9b3ab320..d8035d794b 100644 --- a/docs/regression_test_api.rst +++ b/docs/regression_test_api.rst @@ -40,12 +40,13 @@ Pipeline Hooks Builtins -------- +.. versionadded:: 3.5 + ReFrame provides built-in functions that facilitate the creation of extensible tests (i.e. a test library). These *builtins* are intended to be used directly in the class body of the test, allowing the ReFrame internals to *pre-process* their input before the actual test creation takes place. This provides the ReFrame internals with further control over the user's input, making the process of writing regression tests less error-prone thanks to a better error checking. In essence, these builtins exert control over the test creation, and they allow adding and/or modifying certain attributes of the regression test. -.. versionadded:: 3.5 .. py:function:: reframe.core.pipeline.RegressionTest.parameter(values=None, inherit_params=False, filter_params=None) @@ -129,9 +130,10 @@ In essence, these builtins exert control over the test creation, and they allow print(self.my_var) # prints 8. # self.my_var = 'override' # Error: my_var must be an int! self.not_a_var = 'override' # However, this would work. Dangerous! + self.my_var = 10 # tests may also assign values the standard way The argument ``value`` in the :func:`variable` built-in sets the default value for the variable. - As mentioned above, a variable may not be declared more than once, but its value can be updated by simply assigning it a new value directly in the class body. + As mentioned above, a variable may not be declared more than once, but its default value can be updated by simply assigning it a new value directly in the class body. .. code:: python @@ -146,7 +148,7 @@ In essence, these builtins exert control over the test creation, and they allow These examples above assumed that a default value can be provided to the variables in the bases tests, but that might not always be the case. For example, when writing a test library, one might want to leave some variables undefined and force the user to set these when using the test. - As shown in the example below, imposing such requirement is as simple as not passing any ``value`` to the :func:`variable` built-in, which marks the given variable as ``required``. + As shown in the example below, imposing such requirement is as simple as not passing any ``value`` to the :func:`variable` built-in, which marks the given variable as *required*. .. code:: python diff --git a/reframe/core/parameters.py b/reframe/core/parameters.py index 5cf5fccf04..bbb7643fd6 100644 --- a/reframe/core/parameters.py +++ b/reframe/core/parameters.py @@ -108,8 +108,8 @@ def join(self, other, cls): # could be doubly defined and lead to repeated # values if (key in self.params and - self.params[key] != () and - other.params[key] != ()): + self.params[key] != () and + other.params[key] != ()): raise ValueError( f'parameter space conflict: ' diff --git a/unittests/test_variables.py b/unittests/test_variables.py index b095c8142b..891432e9ed 100644 --- a/unittests/test_variables.py +++ b/unittests/test_variables.py @@ -168,19 +168,15 @@ def test_var_deepcopy(): Each instance must have its own copies of each variable. ''' - class MyType: - def __init__(self, val): - self.val = val - class Base(rfm.RegressionTest): - my_var = variable(MyType, value=MyType(3)) + my_var = variable(list, value=[1,2]) class Foo(Base): def __init__(self): - self.my_var.val = -2 + self.my_var += [3] class Bar(Base): pass - assert Foo().my_var.val == -2 - assert Bar().my_var.val == 3 + assert Foo().my_var == [1, 2, 3] + assert Bar().my_var == [1, 2] From 553b15acd98916a5c42f4a113fcc2bf73f8da77c Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Thu, 18 Feb 2021 11:32:59 +0100 Subject: [PATCH 47/49] Update version notes to 3.4.2 --- docs/regression_test_api.rst | 2 +- reframe/core/pipeline.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/regression_test_api.rst b/docs/regression_test_api.rst index d8035d794b..fdd20e5e18 100644 --- a/docs/regression_test_api.rst +++ b/docs/regression_test_api.rst @@ -40,7 +40,7 @@ Pipeline Hooks Builtins -------- -.. versionadded:: 3.5 +.. versionadded:: 3.4.2 ReFrame provides built-in functions that facilitate the creation of extensible tests (i.e. a test library). These *builtins* are intended to be used directly in the class body of the test, allowing the ReFrame internals to *pre-process* their input before the actual test creation takes place. diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index 48432fa268..cf00207f3b 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -136,7 +136,7 @@ class RegressionMixin(metaclass=RegressionTestMeta): :class:`RegressionTestMeta`. Using this metaclass allows mixin classes to use powerful ReFrame features, such as hooks, parameters or variables. - .. versionadded:: 3.5 + .. versionadded:: 3.4.2 ''' @@ -148,7 +148,7 @@ class RegressionTest(jsonext.JSONSerializable, metaclass=RegressionTestMeta): regression test goes through during its lifetime. .. warning:: - .. versionchanged:: 3.5 + .. versionchanged:: 3.4.2 Multiple inheritance with a shared common ancestor is not allowed. .. note:: From 800dbdd7005a1523d3e005fafc70e9771f859840 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Thu, 18 Feb 2021 11:48:03 +0100 Subject: [PATCH 48/49] Add PEP8 fixes --- unittests/test_variables.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unittests/test_variables.py b/unittests/test_variables.py index 891432e9ed..e678824c23 100644 --- a/unittests/test_variables.py +++ b/unittests/test_variables.py @@ -169,7 +169,7 @@ def test_var_deepcopy(): Each instance must have its own copies of each variable. ''' class Base(rfm.RegressionTest): - my_var = variable(list, value=[1,2]) + my_var = variable(list, value=[1, 2]) class Foo(Base): def __init__(self): From 0a64d4cc520ba1b9dbdcee0dd7bc4af1a4a73814 Mon Sep 17 00:00:00 2001 From: Javier Otero Date: Thu, 18 Feb 2021 12:39:38 +0100 Subject: [PATCH 49/49] Fix test name for parameters as lists. --- reframe/core/pipeline.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index cf00207f3b..1b1d644b65 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -776,7 +776,7 @@ def __getattribute__(self, name): def _append_parameters_to_name(self): if self._rfm_param_space.params: - return '_' + '_'.join([str(self.__dict__[key]) + return '_' + '_'.join([util.toalphanum(str(self.__dict__[key])) for key in self._rfm_param_space.params]) else: return ''