From df1d92a7a2f8ce98a7cc06152fa8fcadaedb19d2 Mon Sep 17 00:00:00 2001
From: lisa neigut <niftynei@gmail.com>
Date: Wed, 4 Sep 2019 12:11:51 -0500
Subject: [PATCH] test-fixtures: return error set, don't throw exception

Throwing an exception while killing all nodes meant that
we aren't cleaning up all the nodes properly. Instead,
collect the errors, and return them back to the upper level,
where we report them and terminate as expected.
---
 tests/fixtures.py | 6 +++---
 tests/utils.py    | 6 ++++--
 2 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/tests/fixtures.py b/tests/fixtures.py
index 37ad7ed2d..b875e8f1a 100644
--- a/tests/fixtures.py
+++ b/tests/fixtures.py
@@ -109,7 +109,7 @@ def node_factory(request, directory, test_name, bitcoind, executor):
     nf = NodeFactory(test_name, bitcoind, executor, directory=directory)
     yield nf
     err_count = 0
-    ok = nf.killall([not n.may_fail for n in nf.nodes])
+    ok, errs = nf.killall([not n.may_fail for n in nf.nodes])
 
     def check_errors(request, err_count, msg):
         """Just a simple helper to format a message, set flags on request and then raise
@@ -147,7 +147,7 @@ def node_factory(request, directory, test_name, bitcoind, executor):
     for node in nf.nodes:
         err_count += checkMemleak(node)
     if err_count:
-        raise ValueError("{} nodes had memleak messages".format(err_count))
+        raise ValueError("{} nodes had memleak messages \n{}".format(err_count, '\n'.join(errs)))
 
     for node in [n for n in nf.nodes if not n.allow_broken_log]:
         err_count += checkBroken(node)
@@ -155,7 +155,7 @@ def node_factory(request, directory, test_name, bitcoind, executor):
 
     if not ok:
         request.node.has_errors = True
-        raise Exception("At least one lightning exited with unexpected non-zero return code")
+        raise Exception("At least one lightning node exited with unexpected non-zero return code\n Recorded errors: {}".format('\n'.join(errs)))
 
 
 def getValgrindErrors(node):
diff --git a/tests/utils.py b/tests/utils.py
index ad40f0d42..dfe56ace2 100644
--- a/tests/utils.py
+++ b/tests/utils.py
@@ -967,6 +967,7 @@ class NodeFactory(object):
     def killall(self, expected_successes):
         """Returns true if every node we expected to succeed actually succeeded"""
         unexpected_fail = False
+        err_msgs = []
         for i in range(len(self.nodes)):
             leaks = None
             # leak detection upsets VALGRIND by reading uninitialized mem.
@@ -985,9 +986,10 @@ class NodeFactory(object):
                     unexpected_fail = True
 
             if leaks is not None and len(leaks) != 0:
-                raise Exception("Node {} has memory leaks: {}".format(
+                unexpected_fail = True
+                err_msgs.append("Node {} has memory leaks: {}".format(
                     self.nodes[i].daemon.lightning_dir,
                     json.dumps(leaks, sort_keys=True, indent=4)
                 ))
 
-        return not unexpected_fail
+        return not unexpected_fail, err_msgs