Merge pull request #1665 from vkarak/bugfix/jsonext-dump-deferrable-no-eval

[bugfix] Do not evaluate deferred expressions when dumping in JSON

Author: Vasileios Karakasis

reframe/core/deferrable.py

@@ -42,22 +42,35 @@ def __init__(self, fn, *args, **kwargs):
         self._args = args
         self._kwargs = kwargs
 
+        # We cache the value of the last evaluation inside a tuple.
+        # We don't cache the value directly, because it can be any.
+
+        # NOTE: The cache for the moment is only used by
+        # `__rfm_json_encode__`. Enabling caching in the evaluation is a
+        # reasonable optimization, but might break compatibility, so it needs
+        # to be thought thoroughly and communicated properly in the
+        # documentation.
+        self._cached = ()
+
     def evaluate(self):
         fn_args = []
         for arg in self._args:
             fn_args.append(
-                arg.evaluate() if isinstance(arg, type(self)) else arg)
+                arg.evaluate() if isinstance(arg, type(self)) else arg
+            )
 
         fn_kwargs = {}
         for k, v in self._kwargs.items():
             fn_kwargs[k] = (
-                v.evaluate() if isinstance(v, type(self)) else v)
+                v.evaluate() if isinstance(v, type(self)) else v
+            )
 
         ret = self._fn(*fn_args, **fn_kwargs)
         if isinstance(ret, type(self)):
-            return ret.evaluate()
-        else:
-            return ret
+            ret = ret.evaluate()
+
+        self._cached = (ret,)
+        return ret
 
     def __bool__(self):
         '''The truthy value of a deferred expression.
@@ -76,10 +89,10 @@ def __iter__(self):
         return iter(self.evaluate())
 
     def __rfm_json_encode__(self):
-        try:
-            return self.evaluate()
-        except BaseException:
+        if self._cached == ():
             return None
+        else:
+            return self._cached[0]
 
     # Overload Python operators to be able to defer any expression
     #
@@ -258,7 +271,7 @@ def __ror__(a, b):
     # Augmented operators
     #
     # NOTE: These are usually part of mutable objects, however
-    # _DeferredExpression remains immutable, since it evnentually delegates
+    # _DeferredExpression remains immutable, since it eventually delegates
     # their evaluation to the objects it wraps
     @deferrable
     def __iadd__(a, b):

unittests/test_logging.py

@@ -174,6 +174,11 @@ def test_logger_dynamic_attributes_deferrables(logfile, logger_with_check):
     )
     logger_with_check.logger.handlers[0].setFormatter(formatter)
     logger_with_check.info('xxx')
+    assert _pattern_in_logfile(r'null\|null', logfile)
+
+    # Evaluate the deferrable and log again
+    logger_with_check.check.deferred.evaluate()
+    logger_with_check.info('xxx')
     assert _pattern_in_logfile(r'"hello"\|null', logfile)
 
 

unittests/test_utility.py

@@ -1467,21 +1467,30 @@ def test_jsonext_dump(tmp_path):
     with open(json_dump, 'w') as fp:
         jsonext.dump({'foo': sn.defer(['bar'])}, fp)
 
+    with open(json_dump, 'r') as fp:
+        assert '{"foo": null}' == fp.read()
+
+    with open(json_dump, 'w') as fp:
+        jsonext.dump({'foo': sn.defer(['bar']).evaluate()}, fp)
+
     with open(json_dump, 'r') as fp:
         assert '{"foo": ["bar"]}' == fp.read()
 
     with open(json_dump, 'w') as fp:
         jsonext.dump({'foo': sn.defer(['bar'])}, fp, separators=(',', ':'))
 
     with open(json_dump, 'r') as fp:
-        assert '{"foo":["bar"]}' == fp.read()
+        assert '{"foo":null}' == fp.read()
 
 
 def test_jsonext_dumps():
     assert '"foo"' == jsonext.dumps('foo')
-    assert '{"foo": ["bar"]}' == jsonext.dumps({'foo': sn.defer(['bar'])})
-    assert '{"foo":["bar"]}' == jsonext.dumps({'foo': sn.defer(['bar'])},
-                                              separators=(',', ':'))
+    assert '{"foo": ["bar"]}' == jsonext.dumps(
+        {'foo': sn.defer(['bar']).evaluate()}
+    )
+    assert '{"foo":["bar"]}' == jsonext.dumps(
+        {'foo': sn.defer(['bar']).evaluate()}, separators=(',', ':')
+    )
 
 
 # Classes to test JSON deserialization