diff --git a/reframe/core/pipeline.py b/reframe/core/pipeline.py index d873797ba8..43b362de71 100644 --- a/reframe/core/pipeline.py +++ b/reframe/core/pipeline.py @@ -169,7 +169,7 @@ def disable_hook(cls, hook_name): #: Support for wildcards is dropped. #: valid_prog_environs = fields.TypedField('valid_prog_environs', - typ.List[str]) + typ.List[str], type(None)) #: List of systems supported by this test. #: The general syntax for systems is ``[:]``. @@ -178,7 +178,8 @@ def disable_hook(cls, hook_name): #: #: :type: :class:`List[str]` #: :default: ``[]`` - valid_systems = fields.TypedField('valid_systems', typ.List[str]) + valid_systems = fields.TypedField('valid_systems', + typ.List[str], type(None)) #: A detailed description of the test. #: @@ -744,8 +745,8 @@ def _rfm_init(self, name=None, prefix=None): self.name = name self.descr = self.name - self.valid_prog_environs = [] - self.valid_systems = [] + self.valid_prog_environs = None + self.valid_systems = None self.sourcepath = '' self.prebuild_cmds = [] self.postbuild_cmds = [] diff --git a/reframe/frontend/loader.py b/reframe/frontend/loader.py index 956ed5f5f6..e1f1b56b2f 100644 --- a/reframe/frontend/loader.py +++ b/reframe/frontend/loader.py @@ -8,7 +8,9 @@ # import ast -import collections +import collections.abc +import inspect +import io import os import reframe.utility as util @@ -75,6 +77,33 @@ def _validate_source(self, filename): getlogger().debug(msg) return validator.valid + def _validate_check(self, check): + import reframe.utility as util + + name = type(check).__name__ + checkfile = os.path.relpath(inspect.getfile(type(check))) + required_attrs = ['valid_systems', 'valid_prog_environs'] + for attr in required_attrs: + if getattr(check, attr) is None: + getlogger().warning( + f'{checkfile}: {attr!r} not defined for test {name!r}; ' + f'skipping...' + ) + return False + + is_copyable = util.attr_validator(lambda obj: util.is_copyable(obj)) + valid, attr = is_copyable(check) + if not valid: + getlogger().warning( + f'{checkfile}: {attr!r} is not copyable; ' + f'not copyable attributes are not ' + f'allowed inside the __init__() method; ' + f'consider setting them in a pipeline hook instead' + ) + return False + + return True + @property def load_path(self): return self._load_path @@ -118,6 +147,9 @@ def load_from_module(self, module): if not isinstance(c, RegressionTest): continue + if not self._validate_check(c): + continue + testfile = module.__file__ try: conflicted = self._loaded[c.name] @@ -125,11 +157,11 @@ def load_from_module(self, module): self._loaded[c.name] = testfile ret.append(c) else: - msg = ("%s: test `%s' already defined in `%s'" % - (testfile, c.name, conflicted)) + msg = (f'{testfile}: test {c.name!r} ' + f'already defined in {conflicted!r}') if self._ignore_conflicts: - getlogger().warning(msg + '; ignoring...') + getlogger().warning(f'{msg}; skipping...') else: raise NameConflictError(msg) diff --git a/reframe/utility/__init__.py b/reframe/utility/__init__.py index c56e0915a4..25ab5241e4 100644 --- a/reframe/utility/__init__.py +++ b/reframe/utility/__init__.py @@ -14,6 +14,7 @@ import re import sys import types +import weakref from collections import UserDict from . import typecheck as typ @@ -270,6 +271,199 @@ def repr(obj, htchar=' ', lfchar='\n', indent=4, basic_offset=0): return f'{type(obj).__name__}({r})@{hex(id(obj))}' +def _is_builtin_type(cls): + # NOTE: The set of types is copied from the copy.deepcopy() implementation + builtin_types = (type(None), int, float, bool, complex, str, tuple, + bytes, frozenset, type, range, slice, property, + type(Ellipsis), type(NotImplemented), weakref.ref, + types.BuiltinFunctionType, types.FunctionType) + + if not isinstance(cls, type): + return False + + return any(t == cls for t in builtin_types) + + +def _is_function_type(cls): + return (isinstance(cls, types.BuiltinFunctionType) or + isinstance(cls, types.FunctionType)) + + +def attr_validator(validate_fn): + '''Validate object attributes recursively. + + This returns a function which you can call with the object to check. It + will return :class:`True` if the :func:`validate_fn` returns :class:`True` + for all object attributes recursively. If the object to be validated is an + iterable, its elements will be validated individually. + + :arg validate_fn: A callable that validates an object. It takes a single + argument, which is the object to validate. + + :returns: A validation function that will perform the actual validation. + It accepts a single argument, which is the object to validate. It + returns a two-element tuple, containing the result of the validation + as a boolean and a formatted string indicating the faulty attribute. + + .. note:: + Objects defining :attr:`__slots__` are passed directly to the + ``validate_fn`` function. + + .. versionadded:: 3.3 + + ''' + + # Already visited objects + visited = set() + depth = 0 + + def _do_validate(obj, path=None): + def _fmt(path): + ret = '' + for p in path: + t, name = p + if t == 'A': + ret += f'.{name}' + elif t == 'I': + ret += f'[{name}]' + elif t == 'K': + ret += f'[{name!r}]' + + # Remove leading '.' + return ret[1:] if ret[0] == '.' else ret + + nonlocal depth + + def _clean_cache(): + nonlocal depth + + depth -= 1 + if depth == 0: + # We are exiting the top-level call + visited.clear() + + depth += 1 + visited.add(id(obj)) + if path is None: + path = [('A', type(obj).__name__)] + + if isinstance(obj, dict): + for k, v in obj.items(): + if id(v) in visited: + continue + + path.append(('K', k)) + valid, _ = _do_validate(v, path) + if not valid: + _clean_cache() + return False, _fmt(path) + + path.pop() + + _clean_cache() + return True, _fmt(path) + + if (isinstance(obj, list) or + isinstance(obj, tuple) or + isinstance(obj, set)): + for i, x in enumerate(obj): + if id(x) in visited: + continue + + path.append(('I', i)) + valid, _ = _do_validate(x, path) + if not valid: + _clean_cache() + return False, _fmt(path) + + path.pop() + + _clean_cache() + return True, _fmt(path) + + valid = validate_fn(obj) + if not valid: + _clean_cache() + return False, _fmt(path) + + # Stop here if obj is a built-in type + if isinstance(obj, type) and _is_builtin_type(obj): + return True, _fmt(path) + + if hasattr(obj, '__dict__'): + for k, v in obj.__dict__.items(): + if id(v) in visited: + continue + + path.append(('A', k)) + valid, _ = _do_validate(v, path) + if not valid: + _clean_cache() + return False, _fmt(path) + + path.pop() + + _clean_cache() + return True, _fmt(path) + + return _do_validate + + +def is_copyable(obj): + '''Check if an object can be copied with :py:func:`copy.deepcopy`, without + performing the copy. + + This is a superset of :func:`is_picklable`. It returns :class:`True` also + in the following cases: + + - The object defines a :func:`__copy__` method. + - The object defines a :func:`__deepcopy__` method. + - The object is a function. + - The object is a builtin type. + + .. versionadded:: 3.3 + + ''' + + if hasattr(obj, '__copy__') or hasattr(obj, '__deepcopy__'): + return True + + if _is_function_type(obj): + return True + + if _is_builtin_type(obj): + return True + + return is_picklable(obj) + + +def is_picklable(obj): + '''Check if an object can be pickled. + + .. versionadded:: 3.3 + + ''' + + if isinstance(obj, type): + return False + + if hasattr(obj, '__reduce_ex__'): + try: + obj.__reduce_ex__(4) + return True + except TypeError: + return False + + if hasattr(obj, '__reduce__'): + try: + obj.__reduce__() + return True + except TypeError: + return False + + return False + + def shortest(*iterables): '''Return the shortest sequence. diff --git a/unittests/resources/checks/bad/invalid_check.py b/unittests/resources/checks/bad/invalid_check.py index cc99bc14cc..a8a4be52dd 100644 --- a/unittests/resources/checks/bad/invalid_check.py +++ b/unittests/resources/checks/bad/invalid_check.py @@ -8,7 +8,9 @@ @rfm.simple_test class SomeTest(rfm.RegressionTest): - pass + def __init__(self): + self.valid_systems = [] + self.valid_prog_environs = [] class NotATest: diff --git a/unittests/resources/checks/bad/noentry.py b/unittests/resources/checks/bad/noentry.py deleted file mode 100644 index 7fed3f34ca..0000000000 --- a/unittests/resources/checks/bad/noentry.py +++ /dev/null @@ -1,13 +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 - -import os - -from reframe.core.pipeline import RegressionTest - - -class EmptyTest(RegressionTest): - def __init__(self, **kwargs): - super().__init__('emptycheck', os.path.dirname(__file__), **kwargs) diff --git a/unittests/resources/checks/emptycheck.py b/unittests/resources/checks/emptycheck.py index a6a177aa73..0fe02437f9 100644 --- a/unittests/resources/checks/emptycheck.py +++ b/unittests/resources/checks/emptycheck.py @@ -8,4 +8,6 @@ @rfm.simple_test class EmptyTest(rfm.RegressionTest): - pass + def __init__(self): + self.valid_systems = [] + self.valid_prog_environs = [] diff --git a/unittests/resources/checks/frontend_checks.py b/unittests/resources/checks/frontend_checks.py index 85c3a86e03..b2923f4058 100644 --- a/unittests/resources/checks/frontend_checks.py +++ b/unittests/resources/checks/frontend_checks.py @@ -54,6 +54,7 @@ def raise_error_early(self): class NoSystemCheck(BaseFrontendCheck): def __init__(self): super().__init__() + self.valid_systems = [] self.valid_prog_environs = ['*'] @@ -62,6 +63,7 @@ class NoPrgEnvCheck(BaseFrontendCheck): def __init__(self): super().__init__() self.valid_systems = ['*'] + self.valid_prog_environs = [] @rfm.simple_test @@ -169,7 +171,7 @@ def __init__(self, sleep_time): print_timestamp = ( "python3 -c \"from datetime import datetime; " "print(datetime.today().strftime('%s.%f'), flush=True)\"") - self.prerun_cmds = [print_timestamp] + self.prerun_cmds = [print_timestamp] self.postrun_cmds = [print_timestamp] self.sanity_patterns = sn.assert_found(r'.*', self.stdout) self.valid_systems = ['*'] @@ -230,3 +232,37 @@ def __init__(self): self.sourcesdir = None self.sourcepath = 'x.c' self.prebuild_cmds = ['echo foo > x.c'] + + +# The following tests do not validate and should not be loaded + +@rfm.simple_test +class TestWithGenerator(rfm.RunOnlyRegressionTest): + '''This test is invalid in ReFrame and the loader must not load it''' + + def __init__(self): + self.valid_systems = ['*'] + self.valid_prog_environs = ['*'] + + def foo(): + yield True + + self.sanity_patterns = sn.defer(foo()) + + +@rfm.simple_test +class TestWithFileObject(rfm.RunOnlyRegressionTest): + '''This test is invalid in ReFrame and the loader must not load it''' + + def __init__(self): + self.valid_systems = ['*'] + self.valid_prog_environs = ['*'] + with open(__file__) as fp: + pass + + self.x = fp + + +@rfm.simple_test +class EmptyInvalidTest(rfm.RegressionTest): + pass diff --git a/unittests/resources/checks_unlisted/good.py b/unittests/resources/checks_unlisted/good.py index 3dfe249e79..fd771436aa 100644 --- a/unittests/resources/checks_unlisted/good.py +++ b/unittests/resources/checks_unlisted/good.py @@ -13,9 +13,16 @@ from reframe.core.pipeline import RegressionTest +class _Base(RegressionTest): + def __init__(self): + self.valid_systems = ['*'] + self.valid_prog_environs = ['*'] + + @rfm.parameterized_test(*((x, y) for x in range(3) for y in range(2))) -class MyBaseTest(RegressionTest): +class MyBaseTest(_Base): def __init__(self, a, b): + super().__init__() self.a = a self.b = b @@ -33,8 +40,9 @@ def __repr__(self): @rfm.parameterized_test( *({'a': x, 'b': y} for x in range(3) for y in range(2)) ) -class AnotherBaseTest(RegressionTest): +class AnotherBaseTest(_Base): def __init__(self, a, b): + super().__init__() self.a = a self.b = b diff --git a/unittests/test_utility.py b/unittests/test_utility.py index fb04be4851..1975049b9e 100644 --- a/unittests/test_utility.py +++ b/unittests/test_utility.py @@ -1483,3 +1483,111 @@ def test_jsonext_dumps(): assert '{"foo": ["bar"]}' == jsonext.dumps({'foo': sn.defer(['bar'])}) assert '{"foo":["bar"]}' == jsonext.dumps({'foo': sn.defer(['bar'])}, separators=(',', ':')) + + +def test_attr_validator(): + class C: + def __init__(self): + self.x = 3 + self.y = [1, 2, 3] + self.z = {'a': 1, 'b': 2} + + class D: + def __init__(self): + self.x = 1 + self.y = C() + + has_no_str = util.attr_validator(lambda x: not isinstance(x, str)) + + d = D() + assert has_no_str(d)[0] + + # Check when a list element does not validate + d.y.y[1] = 'foo' + assert has_no_str(d) == (False, 'D.y.y[1]') + d.y.y[1] = 2 + + # Check when a dict element does not validate + d.y.z['a'] = 'b' + assert has_no_str(d) == (False, "D.y.z['a']") + d.y.z['a'] = 1 + + # Check when an attribute does not validate + d.x = 'foo' + assert has_no_str(d) == (False, 'D.x') + d.x = 1 + + # Check when an attribute does not validate + d.y.x = 'foo' + assert has_no_str(d) == (False, 'D.y.x') + d.y.x = 3 + + # Check when an attribute does not validate against a custom type + has_no_c = util.attr_validator(lambda x: not isinstance(x, C)) + assert has_no_c(d) == (False, 'D.y') + + +def test_is_picklable(): + class X: + pass + + x = X() + assert util.is_picklable(x) + assert not util.is_picklable(X) + + assert util.is_picklable(1) + assert util.is_picklable([1, 2]) + assert util.is_picklable((1, 2)) + assert util.is_picklable({1, 2}) + assert util.is_picklable({'a': 1, 'b': 2}) + + class Y: + def __reduce_ex__(self, proto): + raise TypeError + + y = Y() + assert not util.is_picklable(y) + + class Z: + def __reduce__(self): + return TypeError + + # This is still picklable, because __reduce_ex__() is preferred + z = Z() + assert util.is_picklable(z) + + def foo(): + yield + + assert not util.is_picklable(foo) + assert not util.is_picklable(foo()) + + +def test_is_copyable(): + class X: + pass + + x = X() + assert util.is_copyable(x) + + class Y: + def __copy__(self): + pass + + y = Y() + assert util.is_copyable(y) + + class Z: + def __deepcopy__(self, memo): + pass + + z = Z() + assert util.is_copyable(z) + + def foo(): + yield + + assert util.is_copyable(foo) + assert util.is_copyable(len) + assert util.is_copyable(int) + assert not util.is_copyable(foo())