From ac4caf3366a85617641394a97aa9f029550d77d4 Mon Sep 17 00:00:00 2001 From: Sebastian Falbesoner Date: Thu, 12 Oct 2023 17:24:16 +0200 Subject: test: fix `assert_debug_log` call-site bugs, add type checks Two recently added tests (PR #28625 / commit 2e31250027ac580a7a72221fe2ff505b30836175 and PR #28634 / commit 3bb51c29df596aab2c1fde184667cee435597715) introduced a bug by wrongly using the `assert_debug_log` helper. Instead of passing the expected debug string in a list as expected, it was passed as bare string, which is then interpretered as a list of characters, very likely leading the debug log assertion pass even if the intended message is not appearing. In order to avoid bugs like this in the future, enforce that the `{un}expected_msgs` parameters are lists. --- test/functional/p2p_segwit.py | 4 ++-- test/functional/p2p_v2_transport.py | 4 ++-- test/functional/test_framework/test_node.py | 3 +++ 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/test/functional/p2p_segwit.py b/test/functional/p2p_segwit.py index b398ef51e1..d316c4b602 100755 --- a/test/functional/p2p_segwit.py +++ b/test/functional/p2p_segwit.py @@ -706,14 +706,14 @@ class SegWitTest(BitcoinTestFramework): # segwit activation. Note that older bitcoind's that are not # segwit-aware would also reject this for failing CLEANSTACK. with self.nodes[0].assert_debug_log( - expected_msgs=(spend_tx.hash, 'was not accepted: mandatory-script-verify-flag-failed (Witness program was passed an empty witness)')): + expected_msgs=[spend_tx.hash, 'was not accepted: mandatory-script-verify-flag-failed (Witness program was passed an empty witness)']): test_transaction_acceptance(self.nodes[0], self.test_node, spend_tx, with_witness=False, accepted=False) # Try to put the witness script in the scriptSig, should also fail. spend_tx.vin[0].scriptSig = CScript([p2wsh_pubkey, b'a']) spend_tx.rehash() with self.nodes[0].assert_debug_log( - expected_msgs=(spend_tx.hash, 'was not accepted: mandatory-script-verify-flag-failed (Script evaluated without error but finished with a false/empty top stack element)')): + expected_msgs=[spend_tx.hash, 'was not accepted: mandatory-script-verify-flag-failed (Script evaluated without error but finished with a false/empty top stack element)']): test_transaction_acceptance(self.nodes[0], self.test_node, spend_tx, with_witness=False, accepted=False) # Now put the witness script in the witness, should succeed after diff --git a/test/functional/p2p_v2_transport.py b/test/functional/p2p_v2_transport.py index 99417a17a9..5ad2194b84 100755 --- a/test/functional/p2p_v2_transport.py +++ b/test/functional/p2p_v2_transport.py @@ -145,7 +145,7 @@ class V2TransportTest(BitcoinTestFramework): wrong_network_magic_prefix = MAGIC_BYTES["signet"] + V1_PREFIX[4:] with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: s.connect(("127.0.0.1", p2p_port(0))) - with self.nodes[0].assert_debug_log("V2 transport error: V1 peer with wrong MessageStart"): + with self.nodes[0].assert_debug_log(["V2 transport error: V1 peer with wrong MessageStart"]): s.sendall(wrong_network_magic_prefix + b"somepayload") # Check detection of missing garbage terminator (hits after fixed amount of data if terminator never matches garbage) @@ -156,7 +156,7 @@ class V2TransportTest(BitcoinTestFramework): self.wait_until(lambda: len(self.nodes[0].getpeerinfo()) == num_peers + 1) s.sendall(b'\x00' * (MAX_KEY_GARB_AND_GARBTERM_LEN - 1)) self.wait_until(lambda: self.nodes[0].getpeerinfo()[-1]["bytesrecv"] == MAX_KEY_GARB_AND_GARBTERM_LEN - 1) - with self.nodes[0].assert_debug_log("V2 transport error: missing garbage terminator"): + with self.nodes[0].assert_debug_log(["V2 transport error: missing garbage terminator"]): s.sendall(b'\x00') # send out last byte # should disconnect immediately self.wait_until(lambda: len(self.nodes[0].getpeerinfo()) == num_peers) diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index 4d7adf322c..c77cfbdd91 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -448,6 +448,9 @@ class TestNode(): def assert_debug_log(self, expected_msgs, unexpected_msgs=None, timeout=2): if unexpected_msgs is None: unexpected_msgs = [] + assert_equal(type(expected_msgs), list) + assert_equal(type(unexpected_msgs), list) + time_end = time.time() + timeout * self.timeout_factor prev_size = self.debug_log_size(encoding="utf-8") # Must use same encoding that is used to read() below -- cgit v1.2.3