From 609f9e4260b505255ca3839cc0cc9309ed9a2526 Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Fri, 13 Nov 2020 16:12:31 +0100 Subject: [PATCH 1/7] WIP: Validate tests after init --- reframe/core/pipeline.py | 9 +- reframe/frontend/loader.py | 128 +++++++++++++++++- .../resources/checks/bad/invalid_check.py | 4 +- unittests/resources/checks/bad/noentry.py | 13 -- unittests/resources/checks/emptycheck.py | 7 + unittests/resources/checks/frontend_checks.py | 19 ++- unittests/resources/checks_unlisted/good.py | 12 +- 7 files changed, 167 insertions(+), 25 deletions(-) delete mode 100644 unittests/resources/checks/bad/noentry.py 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..ed20f24cfb 100644 --- a/reframe/frontend/loader.py +++ b/reframe/frontend/loader.py @@ -8,7 +8,9 @@ # import ast -import collections +import collections.abc +import functools +import inspect import os import reframe.utility as util @@ -17,6 +19,97 @@ from reframe.core.logging import getlogger +def _cache_object(fn): + cache = {} + + @functools.wraps(fn) + def _func(obj, *args, **kwargs): + addr = id(obj) + try: + return cache[addr] + except KeyError: + cache[addr] = fn(obj, *args, **kwargs) + return cache[addr] + + return _func + + +def _validator(validate_fn): + '''Validate an object recursively, making sure that validate_fn is True + for each attribute.''' + + # Already visited objects + visited = set() + + 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}]' + + return ret + +# print(util.repr(obj)) + + visited.add(id(obj)) + if path is None: + path = [('A', type(obj).__name__)] + + print(path) + if isinstance(obj, dict): + for k, v in obj.items(): + if id(v) in visited: + continue + + valid, _ = _do_validate(v, path) + if not valid: + path.append(('I', k)) + return False, _fmt(path) + + path.pop() + + 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 + + valid, _ = _do_validate(x, path) + if not valid: + path.append(('I', i)) + return False, _fmt(path) + + path.pop() + + return True, _fmt(path) + + if not hasattr(obj, '__dict__'): + valid = validate_fn(obj) + return valid, _fmt(path) + + 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: + return False, _fmt(path) + + path.pop() + + return True, _fmt(path) + + return _do_validate + + class RegressionCheckValidator(ast.NodeVisitor): def __init__(self): self._has_import = False @@ -75,6 +168,30 @@ 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 = 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 + + # Check that the test has no generator + has_generator = _validator( + lambda obj: not inspect.isgenerator(obj) + ) + valid, attr = has_generator(check) + if not valid: + getlogger().warning(f'{attr} is a generator') + + return valid + @property def load_path(self): return self._load_path @@ -118,6 +235,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 +245,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/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..e952f34fd2 100644 --- a/unittests/resources/checks/emptycheck.py +++ b/unittests/resources/checks/emptycheck.py @@ -8,4 +8,11 @@ @rfm.simple_test class EmptyTest(rfm.RegressionTest): + def __init__(self): + self.valid_systems = [] + self.valid_prog_environs = [] + + +@rfm.simple_test +class EmptyInvalidTest(rfm.RegressionTest): pass diff --git a/unittests/resources/checks/frontend_checks.py b/unittests/resources/checks/frontend_checks.py index 85c3a86e03..f032e46f61 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 @@ -154,6 +156,21 @@ def fail(self): raise Exception +@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.x = foo() + self.sanity_patterns = sn.defer(foo()) + + class SleepCheck(BaseFrontendCheck): _next_id = 0 @@ -169,7 +186,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 = ['*'] 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 From a4199b51da9974fb50b77903b5427b0655a60cc0 Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Fri, 13 Nov 2020 18:54:18 +0100 Subject: [PATCH 2/7] Validate tests after loading - Tests must provide `valid_systems` and `valid_prog_environs` - Tests must not use generator objects or file objects inside their constructors. --- reframe/frontend/loader.py | 121 ++++-------------- reframe/utility/__init__.py | 114 +++++++++++++++++ unittests/resources/checks/frontend_checks.py | 44 ++++--- unittests/test_utility.py | 42 ++++++ 4 files changed, 210 insertions(+), 111 deletions(-) diff --git a/reframe/frontend/loader.py b/reframe/frontend/loader.py index ed20f24cfb..292f53817b 100644 --- a/reframe/frontend/loader.py +++ b/reframe/frontend/loader.py @@ -9,8 +9,8 @@ import ast import collections.abc -import functools import inspect +import io import os import reframe.utility as util @@ -19,97 +19,6 @@ from reframe.core.logging import getlogger -def _cache_object(fn): - cache = {} - - @functools.wraps(fn) - def _func(obj, *args, **kwargs): - addr = id(obj) - try: - return cache[addr] - except KeyError: - cache[addr] = fn(obj, *args, **kwargs) - return cache[addr] - - return _func - - -def _validator(validate_fn): - '''Validate an object recursively, making sure that validate_fn is True - for each attribute.''' - - # Already visited objects - visited = set() - - 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}]' - - return ret - -# print(util.repr(obj)) - - visited.add(id(obj)) - if path is None: - path = [('A', type(obj).__name__)] - - print(path) - if isinstance(obj, dict): - for k, v in obj.items(): - if id(v) in visited: - continue - - valid, _ = _do_validate(v, path) - if not valid: - path.append(('I', k)) - return False, _fmt(path) - - path.pop() - - 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 - - valid, _ = _do_validate(x, path) - if not valid: - path.append(('I', i)) - return False, _fmt(path) - - path.pop() - - return True, _fmt(path) - - if not hasattr(obj, '__dict__'): - valid = validate_fn(obj) - return valid, _fmt(path) - - 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: - return False, _fmt(path) - - path.pop() - - return True, _fmt(path) - - return _do_validate - - class RegressionCheckValidator(ast.NodeVisitor): def __init__(self): self._has_import = False @@ -183,14 +92,34 @@ def _validate_check(self, check): return False # Check that the test has no generator - has_generator = _validator( + has_no_generator = util.attr_validator( lambda obj: not inspect.isgenerator(obj) ) - valid, attr = has_generator(check) + valid, attr = has_no_generator(check) + if not valid: + getlogger().warning( + f'{attr!r} is a generator object; ' + f'generator objects are not allowed inside ' + f'the __init__() method of a test; ' + f'consider using a pipeline hook instead' + ) + return False + + # Check that the test has IO object + has_no_io = util.attr_validator( + lambda obj: not isinstance(obj, io.IOBase) + ) + valid, attr = has_no_io(check) if not valid: - getlogger().warning(f'{attr} is a generator') + getlogger().warning( + f'{attr!r} is a file object; ' + f'file objects are not allowed inside ' + f'the __init__() method of a test; ' + f'consider using a pipeline hook instead' + ) + return False - return valid + return True @property def load_path(self): diff --git a/reframe/utility/__init__.py b/reframe/utility/__init__.py index c56e0915a4..24c283cd16 100644 --- a/reframe/utility/__init__.py +++ b/reframe/utility/__init__.py @@ -270,6 +270,120 @@ def repr(obj, htchar=' ', lfchar='\n', indent=4, basic_offset=0): return f'{type(obj).__name__}({r})@{hex(id(obj))}' +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 validate 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. + + ''' + + # 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) + + 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 shortest(*iterables): '''Return the shortest sequence. diff --git a/unittests/resources/checks/frontend_checks.py b/unittests/resources/checks/frontend_checks.py index f032e46f61..a9e0b0988a 100644 --- a/unittests/resources/checks/frontend_checks.py +++ b/unittests/resources/checks/frontend_checks.py @@ -156,21 +156,6 @@ def fail(self): raise Exception -@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.x = foo() - self.sanity_patterns = sn.defer(foo()) - - class SleepCheck(BaseFrontendCheck): _next_id = 0 @@ -247,3 +232,32 @@ 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 diff --git a/unittests/test_utility.py b/unittests/test_utility.py index fb04be4851..6aafcd81a1 100644 --- a/unittests/test_utility.py +++ b/unittests/test_utility.py @@ -1483,3 +1483,45 @@ 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] == True + + # 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') From 18b1d648acd71b3fbca68be9a0f753c68742abb6 Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Fri, 13 Nov 2020 23:35:42 +0100 Subject: [PATCH 3/7] Generalize check about whether a test can be copied --- reframe/frontend/loader.py | 28 +++---------- reframe/utility/__init__.py | 80 +++++++++++++++++++++++++++++++++++++ unittests/test_utility.py | 66 ++++++++++++++++++++++++++++++ 3 files changed, 152 insertions(+), 22 deletions(-) diff --git a/reframe/frontend/loader.py b/reframe/frontend/loader.py index 292f53817b..0af9fa350d 100644 --- a/reframe/frontend/loader.py +++ b/reframe/frontend/loader.py @@ -91,31 +91,15 @@ def _validate_check(self, check): ) return False - # Check that the test has no generator - has_no_generator = util.attr_validator( - lambda obj: not inspect.isgenerator(obj) + is_copyable = util.attr_validator( + lambda obj: util.is_copyable(obj) ) - valid, attr = has_no_generator(check) + valid, attr = is_copyable(check) if not valid: getlogger().warning( - f'{attr!r} is a generator object; ' - f'generator objects are not allowed inside ' - f'the __init__() method of a test; ' - f'consider using a pipeline hook instead' - ) - return False - - # Check that the test has IO object - has_no_io = util.attr_validator( - lambda obj: not isinstance(obj, io.IOBase) - ) - valid, attr = has_no_io(check) - if not valid: - getlogger().warning( - f'{attr!r} is a file object; ' - f'file objects are not allowed inside ' - f'the __init__() method of a test; ' - f'consider using a pipeline hook instead' + f'{attr!r} is not copyable; not copyable attributes are not ' + f'allowed inside the __init__() method; ' + f'consider setting them in a pipeline hook instead' ) return False diff --git a/reframe/utility/__init__.py b/reframe/utility/__init__.py index 24c283cd16..3778b61e8b 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,24 @@ 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. @@ -290,6 +309,8 @@ def attr_validator(validate_fn): Objects defining :attr:`__slots__` are passed directly to the ``validate_fn`` function. + .. versionadded:: 3.3 + ''' # Already visited objects @@ -365,6 +386,10 @@ def _clean_cache(): _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: @@ -384,6 +409,61 @@ def _clean_cache(): 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/test_utility.py b/unittests/test_utility.py index 6aafcd81a1..b0db44806a 100644 --- a/unittests/test_utility.py +++ b/unittests/test_utility.py @@ -1525,3 +1525,69 @@ def __init__(self): # 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()) From 1c3ca661969e14d8b100cbb1ff31e2313c15c62e Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Fri, 13 Nov 2020 23:43:33 +0100 Subject: [PATCH 4/7] Better error messages --- reframe/frontend/loader.py | 5 +++-- unittests/resources/checks/emptycheck.py | 5 ----- unittests/resources/checks/frontend_checks.py | 5 +++++ 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/reframe/frontend/loader.py b/reframe/frontend/loader.py index 0af9fa350d..d2005af5e1 100644 --- a/reframe/frontend/loader.py +++ b/reframe/frontend/loader.py @@ -81,7 +81,7 @@ def _validate_check(self, check): import reframe.utility as util name = type(check).__name__ - checkfile = inspect.getfile(type(check)) + 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: @@ -97,7 +97,8 @@ def _validate_check(self, check): valid, attr = is_copyable(check) if not valid: getlogger().warning( - f'{attr!r} is not copyable; not copyable attributes are not ' + 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' ) diff --git a/unittests/resources/checks/emptycheck.py b/unittests/resources/checks/emptycheck.py index e952f34fd2..0fe02437f9 100644 --- a/unittests/resources/checks/emptycheck.py +++ b/unittests/resources/checks/emptycheck.py @@ -11,8 +11,3 @@ class EmptyTest(rfm.RegressionTest): def __init__(self): self.valid_systems = [] self.valid_prog_environs = [] - - -@rfm.simple_test -class EmptyInvalidTest(rfm.RegressionTest): - pass diff --git a/unittests/resources/checks/frontend_checks.py b/unittests/resources/checks/frontend_checks.py index a9e0b0988a..b2923f4058 100644 --- a/unittests/resources/checks/frontend_checks.py +++ b/unittests/resources/checks/frontend_checks.py @@ -261,3 +261,8 @@ def __init__(self): pass self.x = fp + + +@rfm.simple_test +class EmptyInvalidTest(rfm.RegressionTest): + pass From c828afddae844a4ac73270a0abb8206a3ebf0beb Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Fri, 13 Nov 2020 23:55:58 +0100 Subject: [PATCH 5/7] Fix PEP8 issues --- unittests/test_utility.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unittests/test_utility.py b/unittests/test_utility.py index b0db44806a..1975049b9e 100644 --- a/unittests/test_utility.py +++ b/unittests/test_utility.py @@ -1500,7 +1500,7 @@ def __init__(self): has_no_str = util.attr_validator(lambda x: not isinstance(x, str)) d = D() - assert has_no_str(d)[0] == True + assert has_no_str(d)[0] # Check when a list element does not validate d.y.y[1] = 'foo' From 74887b29fcfab4be04fc2c14f33a995c25affc24 Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Fri, 13 Nov 2020 23:57:34 +0100 Subject: [PATCH 6/7] Fix coding style issue --- reframe/frontend/loader.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/reframe/frontend/loader.py b/reframe/frontend/loader.py index d2005af5e1..e1f1b56b2f 100644 --- a/reframe/frontend/loader.py +++ b/reframe/frontend/loader.py @@ -91,9 +91,7 @@ def _validate_check(self, check): ) return False - is_copyable = util.attr_validator( - lambda obj: util.is_copyable(obj) - ) + is_copyable = util.attr_validator(lambda obj: util.is_copyable(obj)) valid, attr = is_copyable(check) if not valid: getlogger().warning( From cdbf5b90bd8e6b2345d2767e7f1d28d598261a04 Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Mon, 16 Nov 2020 16:11:41 +0100 Subject: [PATCH 7/7] Fix typo in documentation --- reframe/utility/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/reframe/utility/__init__.py b/reframe/utility/__init__.py index 3778b61e8b..25ab5241e4 100644 --- a/reframe/utility/__init__.py +++ b/reframe/utility/__init__.py @@ -294,7 +294,7 @@ def attr_validator(validate_fn): 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 validate is an + 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