From ea5880b20c774f2f095e547d215ee56525d6c041 Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Sat, 11 Jul 2020 01:44:16 +0200 Subject: [PATCH 1/5] Add a powerful repr() function for debugging This function replaces the older `debug.repr()` and uses `util.ppretty()` for nicely printing the object attributes. `util.ppretty()` is also extended in order to recursively call the new `repr()` function. Objects are printed only once by this new `repr()` avoiding infinite recursions. Finally, we use that function to print the whole ReFrame runtime state in case issue #1369 appears. This function does not take into account the `__slots__` for the moment. In cases, where an object does not have a `__dict__` attribute, the builtin `repr()` will be used. --- reframe/core/config.py | 1 - reframe/core/debug.py | 62 ----------------------- reframe/core/logging.py | 7 --- reframe/frontend/executors/__init__.py | 7 --- reframe/frontend/loader.py | 4 -- reframe/frontend/statistics.py | 4 -- reframe/utility/__init__.py | 65 ++++++++++++++++++------ unittests/test_policies.py | 3 ++ unittests/test_utility.py | 69 +++++++++++++------------- 9 files changed, 88 insertions(+), 134 deletions(-) delete mode 100644 reframe/core/debug.py diff --git a/reframe/core/config.py b/reframe/core/config.py index afdbd77d98..a7c4da6917 100644 --- a/reframe/core/config.py +++ b/reframe/core/config.py @@ -14,7 +14,6 @@ import tempfile import reframe -import reframe.core.debug as debug import reframe.core.fields as fields import reframe.core.settings as settings import reframe.utility as util diff --git a/reframe/core/debug.py b/reframe/core/debug.py deleted file mode 100644 index ac0a7b2c74..0000000000 --- a/reframe/core/debug.py +++ /dev/null @@ -1,62 +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 - -# -# Internal debug utilities for the framework -# - -import builtins -import threading - -# Current indentation levels per thread -_depth = {} - - -def _gettid(): - tid = threading.get_ident() - _depth.setdefault(tid, 0) - return tid - - -def _increase_indent(): - tid = _gettid() - _depth[tid] += 1 - return _depth[tid] - - -def _decrease_indent(): - tid = _gettid() - _depth[tid] -= 1 - return _depth[tid] - - -def repr(obj, indent=4, max_depth=2): - '''Return a generic representation string for object `obj`. - - Keyword arguments: - indent -- indentation width - max_depth -- maximum depth for expanding nested objects - ''' - if not hasattr(obj, '__dict__'): - # Delegate to the builtin repr() for builtin types - return builtins.repr(obj) - - tid = _gettid() - _increase_indent() - - # Attribute representation - if _depth[tid] == max_depth: - attr_list = ['...'] - else: - attr_list = ['{0}={1}'.format(attr, val) - for attr, val in sorted(obj.__dict__.items())] - - repr_fmt = '{module_name}.{class_name}({attr_repr})@0x{addr:x}' - ret = repr_fmt.format(module_name=obj.__module__, - class_name=type(obj).__name__, - attr_repr=', '.join(attr_list), - addr=id(obj)) - _decrease_indent() - return ret diff --git a/reframe/core/logging.py b/reframe/core/logging.py index b03d8006ed..8fa4b3c79b 100644 --- a/reframe/core/logging.py +++ b/reframe/core/logging.py @@ -18,7 +18,6 @@ import reframe import reframe.utility.color as color -import reframe.core.debug as debug import reframe.utility.os_ext as os_ext from reframe.core.exceptions import ConfigError, LoggingError @@ -333,9 +332,6 @@ def __init__(self, name, level=logging.NOTSET): super().__init__(name, logging.NOTSET) self.level = _check_level(level) - def __repr__(self): - return debug.repr(self) - def setLevel(self, level): self.level = _check_level(level) @@ -405,9 +401,6 @@ def __init__(self, logger=None, check=None): self.check = check self.colorize = False - def __repr__(self): - return debug.repr(self) - def setLevel(self, level): if self.logger: super().setLevel(level) diff --git a/reframe/frontend/executors/__init__.py b/reframe/frontend/executors/__init__.py index 022bfd54f0..8e79929384 100644 --- a/reframe/frontend/executors/__init__.py +++ b/reframe/frontend/executors/__init__.py @@ -10,7 +10,6 @@ import time import weakref -import reframe.core.debug as debug import reframe.core.environments as env import reframe.core.logging as logging import reframe.core.runtime as runtime @@ -350,9 +349,6 @@ def __init__(self, policy, printer=None, max_retries=0): self._policy.printer = self._printer signal.signal(signal.SIGTERM, _handle_sigterm) - def __repr__(self): - return debug.repr(self) - @property def max_retries(self): return self._max_retries @@ -465,9 +461,6 @@ def __init__(self): self.task_listeners = [] self.stats = None - def __repr__(self): - return debug.repr(self) - def enter(self): pass diff --git a/reframe/frontend/loader.py b/reframe/frontend/loader.py index ccdff1474a..546343b880 100644 --- a/reframe/frontend/loader.py +++ b/reframe/frontend/loader.py @@ -11,7 +11,6 @@ import collections import os -import reframe.core.debug as debug import reframe.utility as util import reframe.utility.os_ext as os_ext from reframe.core.exceptions import NameConflictError, RegressionTestLoadError @@ -48,9 +47,6 @@ def __init__(self, load_path, recurse=False, ignore_conflicts=False): # Loaded tests by name; maps test names to the file that were defined self._loaded = {} - def __repr__(self): - return debug.repr(self) - def _module_name(self, filename): '''Figure out a module name from filename. diff --git a/reframe/frontend/statistics.py b/reframe/frontend/statistics.py index 37110de4d5..b2760041fe 100644 --- a/reframe/frontend/statistics.py +++ b/reframe/frontend/statistics.py @@ -3,7 +3,6 @@ # # SPDX-License-Identifier: BSD-3-Clause -import reframe.core.debug as debug import reframe.core.runtime as rt from reframe.core.exceptions import StatisticsError @@ -15,9 +14,6 @@ def __init__(self): # Tasks per run stored as follows: [[run0_tasks], [run1_tasks], ...] self._tasks = [[]] - def __repr__(self): - return debug.repr(self) - def add_task(self, task): current_run = rt.runtime().current_run if current_run == len(self._tasks): diff --git a/reframe/utility/__init__.py b/reframe/utility/__init__.py index b16a0632ca..a462babf97 100644 --- a/reframe/utility/__init__.py +++ b/reframe/utility/__init__.py @@ -3,6 +3,7 @@ # # SPDX-License-Identifier: BSD-3-Clause +import builtins import collections import functools import importlib @@ -126,7 +127,8 @@ def toalphanum(s): return re.sub(r'\W', '_', s) -def ppretty(value, htchar=' ', lfchar='\n', indent=4, basic_offset=0): +def ppretty(value, htchar=' ', lfchar='\n', indent=4, basic_offset=0, + repr=builtins.repr): '''Format string of dictionaries, lists and tuples :arg value: The value to be formatted. @@ -135,18 +137,22 @@ def ppretty(value, htchar=' ', lfchar='\n', indent=4, basic_offset=0): :arg indent: Number of htchar characters for every indentation level. :arg basic_offset: Basic offset for the representation, any additional indentation space is added to the ``basic_offset``. + :arg repr: The :func:`repr` to use for printing values. This function may + accept also a ``basic_offset`` argument + :returns: a formatted string of the ``value``. ''' + ppretty2 = functools.partial( + ppretty, htchar=htchar, lfchar=lfchar, indent=indent, + basic_offset=basic_offset+1, repr=repr + ) nlch = lfchar + htchar * indent * (basic_offset + 1) if isinstance(value, tuple): if value == (): return '()' - items = [ - nlch + ppretty(item, htchar, lfchar, indent, basic_offset + 1) - for item in value - ] + items = [nlch + ppretty2(item) for item in value] return '(%s)' % (','.join(items) + lfchar + htchar * indent * basic_offset) elif isinstance(value, list): @@ -154,7 +160,7 @@ def ppretty(value, htchar=' ', lfchar='\n', indent=4, basic_offset=0): return '[]' items = [ - nlch + ppretty(item, htchar, lfchar, indent, basic_offset + 1) + nlch + ppretty2(item) for item in value ] return '[%s]' % (','.join(items) + lfchar + @@ -164,9 +170,7 @@ def ppretty(value, htchar=' ', lfchar='\n', indent=4, basic_offset=0): return '{}' items = [ - nlch + repr(key) + ': ' + - ppretty(value[key], htchar, lfchar, indent, basic_offset + 1) - for key in value + nlch + repr(key) + ': ' + ppretty2(value[key]) for key in value ] return '{%s}' % (','.join(items) + lfchar + htchar * indent * basic_offset) @@ -174,14 +178,47 @@ def ppretty(value, htchar=' ', lfchar='\n', indent=4, basic_offset=0): if value == set(): return 'set()' - items = [ - nlch + ppretty(item, htchar, lfchar, indent, basic_offset + 1) - for item in value - ] + items = [nlch + ppretty2(item) for item in value] return '{%s}' % (','.join(items) + lfchar + htchar * indent * basic_offset) else: - return repr(value) + try: + return repr(value, basic_offset=basic_offset) + except TypeError: + # Not our custom repr() + return repr(value) + + +def _tracked_repr(func): + objects = set() + + @functools.wraps(func) + def _repr(obj, **kwargs): + addr = id(obj) + if addr in objects: + return f'{type(obj).__name__}(...)@{hex(addr)}' + + # Do not track builtin objects + if hasattr(obj, '__dict__'): + objects.add(addr) + + return func(obj, **kwargs) + + return _repr + + +@_tracked_repr +def repr(obj, *, htchar=' ', lfchar='\n', indent=4, basic_offset=0): + '''A debug repr() function printing all object attributes recursively''' + if (isinstance(obj, list) or isinstance(obj, tuple) or + isinstance(obj, set) or isinstance(obj, dict)): + return ppretty(obj, basic_offset=basic_offset, repr=repr) + + if not hasattr(obj, '__dict__'): + return builtins.repr(obj) + + r = ppretty(obj.__dict__, basic_offset=basic_offset, repr=repr) + return f'{type(obj).__name__}({r})@{hex(id(obj))}' class ScopedDict(UserDict): diff --git a/unittests/test_policies.py b/unittests/test_policies.py index 770ca21016..25283e0381 100644 --- a/unittests/test_policies.py +++ b/unittests/test_policies.py @@ -10,6 +10,7 @@ import reframe.frontend.dependency as dependency import reframe.frontend.executors as executors import reframe.frontend.executors.policies as policies +import reframe.utility as util import reframe.utility.os_ext as os_ext from reframe.core.exceptions import (JobNotStartedError, ReframeForceExitError, @@ -524,6 +525,8 @@ def test_kbd_interrupt_in_wait_with_limited_concurrency( KeyboardInterruptCheck(), SleepCheck(10), SleepCheck(10), SleepCheck(10) ])) + # FIXME: Dump everything in case Github #1369 appears + print(util.repr(runner)) assert_interrupted_run(runner) diff --git a/unittests/test_utility.py b/unittests/test_utility.py index 8c73ec8bf0..bb813ca342 100644 --- a/unittests/test_utility.py +++ b/unittests/test_utility.py @@ -12,7 +12,6 @@ import unittest import reframe -import reframe.core.debug as debug import reframe.core.fields as fields import reframe.utility as util import reframe.utility.os_ext as os_ext @@ -420,40 +419,6 @@ def test_load_namespace_package(self): assert 'unittests.resources' in sys.modules -class TestDebugRepr(unittest.TestCase): - def test_builtin_types(self): - # builtin types must use the default repr() - assert repr(1) == debug.repr(1) - assert repr(1.2) == debug.repr(1.2) - assert repr([1, 2, 3]) == debug.repr([1, 2, 3]) - assert repr({1, 2, 3}) == debug.repr({1, 2, 3}) - assert repr({1, 2, 3}) == debug.repr({1, 2, 3}) - assert repr({'a': 1, 'b': {2, 3}}) == debug.repr({'a': 1, 'b': {2, 3}}) - - def test_obj_repr(self): - class C: - def __repr__(self): - return debug.repr(self) - - class D: - def __repr__(self): - return debug.repr(self) - - c = C() - c._a = -1 - c.a = 1 - c.b = {1, 2, 3} - c.d = D() - c.d.a = 2 - c.d.b = 3 - - rep = repr(c) - assert 'unittests.test_utility' in rep - assert '_a=%r' % c._a in rep - assert 'b=%r' % c.b in rep - assert 'D(...)' in rep - - class TestPpretty: def test_simple_types(self): assert util.ppretty(1) == repr(1) @@ -542,6 +507,40 @@ def __repr__(self): "]") +class _X: + def __init__(self): + self._a = False + + +class _Y: + def __init__(self, x, a=None): + self.x = x + self.y = 'foo' + self.z = self + self.a = a + + +def test_repr_default(): + c0, c1 = _Y(1), _Y(2, _X()) + s = util.repr([c0, c1]) + assert s == f'''[ + _Y({{ + 'x': 1, + 'y': 'foo', + 'z': _Y(...)@{hex(id(c0))}, + 'a': None + }})@{hex(id(c0))}, + _Y({{ + 'x': 2, + 'y': 'foo', + 'z': _Y(...)@{hex(id(c1))}, + 'a': _X({{ + '_a': False + }})@{hex(id(c1.a))} + }})@{hex(id(c1))} +]''' + + class TestChangeDirCtxManager(unittest.TestCase): def setUp(self): self.temp_dir = tempfile.mkdtemp() From 3d9c8430f3f747c22588ca19721a5a8ef0f38f39 Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Sat, 11 Jul 2020 18:26:32 +0200 Subject: [PATCH 2/5] More debugging code for #1369 --- unittests/test_policies.py | 1 + 1 file changed, 1 insertion(+) diff --git a/unittests/test_policies.py b/unittests/test_policies.py index 25283e0381..27b0adc605 100644 --- a/unittests/test_policies.py +++ b/unittests/test_policies.py @@ -527,6 +527,7 @@ def test_kbd_interrupt_in_wait_with_limited_concurrency( ])) # FIXME: Dump everything in case Github #1369 appears print(util.repr(runner)) + print(runner.stats.failure_report()) assert_interrupted_run(runner) From 1e3b1a7f06ce2a25336eb11d6ec9a1ead47f707c Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Sat, 11 Jul 2020 19:06:10 +0200 Subject: [PATCH 3/5] More debugging code for #1369 --- unittests/test_policies.py | 1 + 1 file changed, 1 insertion(+) diff --git a/unittests/test_policies.py b/unittests/test_policies.py index 27b0adc605..2ea2309870 100644 --- a/unittests/test_policies.py +++ b/unittests/test_policies.py @@ -528,6 +528,7 @@ def test_kbd_interrupt_in_wait_with_limited_concurrency( # FIXME: Dump everything in case Github #1369 appears print(util.repr(runner)) print(runner.stats.failure_report()) + print(util.repr(rt.runtime().site_config)) assert_interrupted_run(runner) From 65aa02cce26bf5cee5031fb0edd5a6522e6bea45 Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Mon, 13 Jul 2020 13:44:15 +0200 Subject: [PATCH 4/5] Homogenize arguments for repr() and ppretty() functions --- reframe/utility/__init__.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/reframe/utility/__init__.py b/reframe/utility/__init__.py index a462babf97..07e58c012b 100644 --- a/reframe/utility/__init__.py +++ b/reframe/utility/__init__.py @@ -138,11 +138,12 @@ def ppretty(value, htchar=' ', lfchar='\n', indent=4, basic_offset=0, :arg basic_offset: Basic offset for the representation, any additional indentation space is added to the ``basic_offset``. :arg repr: The :func:`repr` to use for printing values. This function may - accept also a ``basic_offset`` argument + accept also all the arguments of :func:`ppretty` except the ``repr``. :returns: a formatted string of the ``value``. ''' + print(repr) ppretty2 = functools.partial( ppretty, htchar=htchar, lfchar=lfchar, indent=indent, basic_offset=basic_offset+1, repr=repr @@ -183,7 +184,7 @@ def ppretty(value, htchar=' ', lfchar='\n', indent=4, basic_offset=0, htchar * indent * basic_offset) else: try: - return repr(value, basic_offset=basic_offset) + return repr(value, htchar, lfchar, indent, basic_offset) except TypeError: # Not our custom repr() return repr(value) @@ -193,7 +194,7 @@ def _tracked_repr(func): objects = set() @functools.wraps(func) - def _repr(obj, **kwargs): + def _repr(obj, *args, **kwargs): addr = id(obj) if addr in objects: return f'{type(obj).__name__}(...)@{hex(addr)}' @@ -202,13 +203,13 @@ def _repr(obj, **kwargs): if hasattr(obj, '__dict__'): objects.add(addr) - return func(obj, **kwargs) + return func(obj, *args, **kwargs) return _repr @_tracked_repr -def repr(obj, *, htchar=' ', lfchar='\n', indent=4, basic_offset=0): +def repr(obj, htchar=' ', lfchar='\n', indent=4, basic_offset=0): '''A debug repr() function printing all object attributes recursively''' if (isinstance(obj, list) or isinstance(obj, tuple) or isinstance(obj, set) or isinstance(obj, dict)): @@ -217,7 +218,7 @@ def repr(obj, *, htchar=' ', lfchar='\n', indent=4, basic_offset=0): if not hasattr(obj, '__dict__'): return builtins.repr(obj) - r = ppretty(obj.__dict__, basic_offset=basic_offset, repr=repr) + r = ppretty(obj.__dict__, htchar, lfchar, indent, basic_offset, repr) return f'{type(obj).__name__}({r})@{hex(id(obj))}' From fb367bf21b548227ab7dbe6f9497274822c351eb Mon Sep 17 00:00:00 2001 From: Vasileios Karakasis Date: Mon, 13 Jul 2020 14:05:28 +0200 Subject: [PATCH 5/5] Remove stale print --- reframe/utility/__init__.py | 1 - 1 file changed, 1 deletion(-) diff --git a/reframe/utility/__init__.py b/reframe/utility/__init__.py index 07e58c012b..b00ec6956b 100644 --- a/reframe/utility/__init__.py +++ b/reframe/utility/__init__.py @@ -143,7 +143,6 @@ def ppretty(value, htchar=' ', lfchar='\n', indent=4, basic_offset=0, :returns: a formatted string of the ``value``. ''' - print(repr) ppretty2 = functools.partial( ppretty, htchar=htchar, lfchar=lfchar, indent=indent, basic_offset=basic_offset+1, repr=repr