From 1e1381255f34cc82ac8e75a8553b7739944a35f7 Mon Sep 17 00:00:00 2001
From: Vasileios Karakasis <karakasis@cscs.ch>
Date: Sun, 20 Dec 2020 12:23:57 +0100
Subject: [PATCH] Do not evaluate deferred expressions when dumping in JSON.

We instead cache the last evaluated value and returned that one. If the deferred
expression has never been evaluated, then `None` is returned.
---
 reframe/core/deferrable.py | 31 ++++++++++++++++++++++---------
 unittests/test_logging.py  |  5 +++++
 unittests/test_utility.py  | 17 +++++++++++++----
 3 files changed, 40 insertions(+), 13 deletions(-)

diff --git a/reframe/core/deferrable.py b/reframe/core/deferrable.py
index 8d820f8b31..4031b1ccf6 100644
--- a/reframe/core/deferrable.py
+++ b/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):
diff --git a/unittests/test_logging.py b/unittests/test_logging.py
index 964d1fe924..6707c29dbc 100644
--- a/unittests/test_logging.py
+++ 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)
 
 
diff --git a/unittests/test_utility.py b/unittests/test_utility.py
index c1e8ae9f1d..866b0db111 100644
--- a/unittests/test_utility.py
+++ b/unittests/test_utility.py
@@ -1467,6 +1467,12 @@ 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()
 
@@ -1474,14 +1480,17 @@ def test_jsonext_dump(tmp_path):
         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