From 7874e7852eb116bedbcccac2389c0d95137cd0c7 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Thu, 5 Sep 2019 13:28:39 +0200 Subject: [PATCH] Merge #16804: test: Remove unused try-block in assert_debug_log fae91a09c453a9a95c382df765bd71e54698d5b2 test: Remove incorrect and unused try-block in assert_debug_log (MarcoFalke) Pull request description: This try block has accidentally been added by me in fa3e9f7627784ee00980590e5bf044a0e1249999. It was unused all the time, but commit 6011c9d72d1df5c2cd09de6f85c21eb4f7eb1ba8 added a `return` in the finally block, muting all exceptions. This can be tested by adding an `assert False` after any `with ...assert_debug_log...:` line. ACKs for top commit: laanwj: ACK fae91a09c453a9a95c382df765bd71e54698d5b2 ryanofsky: utACK fae91a09c453a9a95c382df765bd71e54698d5b2. I didn't know returning inside a `finally` block would cancel pending exceptions or return values, but I guess this makes sense and is a good thing to be aware of. Tree-SHA512: 47ed0165062060e9af055a3e92f1a529cd41d00476bfad64e3cd141ae084d22f926a343bb1257717e164e15459a59ab66aed198c95d18bf780d8cb0b76aa3298 --- test/functional/rpc_setban.py | 2 +- test/functional/test_framework/test_node.py | 36 ++++++++++----------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/test/functional/rpc_setban.py b/test/functional/rpc_setban.py index 0c6d3fc86c..bb0e73f7d7 100755 --- a/test/functional/rpc_setban.py +++ b/test/functional/rpc_setban.py @@ -26,7 +26,7 @@ class SetBanTests(BitcoinTestFramework): self.nodes[1].setban("127.0.0.1", "add") # Node 0 should not be able to reconnect - with self.nodes[1].assert_debug_log(expected_msgs=['dropped (banned)\n'],timeout=5): + with self.nodes[1].assert_debug_log(expected_msgs=['dropped (banned)\n'], timeout=5): self.restart_node(1, []) self.nodes[0].addnode("127.0.0.1:" + str(p2p_port(1)), "onetry") diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 22916b8ce3..77772f84d1 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -316,24 +316,24 @@ class TestNode(): with open(debug_log, encoding='utf-8') as dl: dl.seek(0, 2) prev_size = dl.tell() - try: - yield - finally: - while True: - found = True - with open(debug_log, encoding='utf-8') as dl: - dl.seek(prev_size) - log = dl.read() - print_log = " - " + "\n - ".join(log.splitlines()) - for expected_msg in expected_msgs: - if re.search(re.escape(expected_msg), log, flags=re.MULTILINE) is None: - found = False - if found: - return - if time.time() >= time_end: - break - time.sleep(0.05) - self._raise_assertion_error('Expected messages "{}" does not partially match log:\n\n{}\n\n'.format(str(expected_msgs), print_log)) + + yield + + while True: + found = True + with open(debug_log, encoding='utf-8') as dl: + dl.seek(prev_size) + log = dl.read() + print_log = " - " + "\n - ".join(log.splitlines()) + for expected_msg in expected_msgs: + if re.search(re.escape(expected_msg), log, flags=re.MULTILINE) is None: + found = False + if found: + return + if time.time() >= time_end: + break + time.sleep(0.05) + self._raise_assertion_error('Expected messages "{}" does not partially match log:\n\n{}\n\n'.format(str(expected_msgs), print_log)) @contextlib.contextmanager def assert_memory_usage_stable(self, *, increase_allowed=0.03):