Merge bitcoin/bitcoin#29750: test: makes timeout a forced named argument in tests methods that use it

61560d5 test: makes timeout a forced named argument in tests methods that use it (Sergi Delgado Segura)

Pull request description:

  This makes calls to such methods more explicit and less error-prone.

  Motivated by bitcoin/bitcoin#29736 (comment)

ACKs for top commit:
  maflcko:
    lgtm ACK 61560d5
  brunoerg:
    ACK 61560d5
  BrandonOdiwuor:
    crACK 61560d5
  AngusP:
    ACK 61560d5
  stratospher:
    tested ACK 61560d5.

Tree-SHA512: 8d6ec3fe1076c868ddbd3050f3c242dbd83cc123f560db3d3b0ed74968e6050dc9ebf4e7c716af9cc1b290c97d736c2fc2ac936b0b69ebdbceed934dae7d55d9

Author: fanquake

test/functional/feature_assumevalid.py

@@ -159,7 +159,7 @@ def run_test(self):
         for i in range(2202):
             p2p1.send_message(msg_block(self.blocks[i]))
         # Syncing 2200 blocks can take a while on slow systems. Give it plenty of time to sync.
-        p2p1.sync_with_ping(960)
+        p2p1.sync_with_ping(timeout=960)
         assert_equal(self.nodes[1].getblock(self.nodes[1].getbestblockhash())['height'], 2202)
 
         p2p2 = self.nodes[2].add_p2p_connection(BaseNode())

test/functional/p2p_compactblocks.py

@@ -139,7 +139,7 @@ def send_await_disconnect(self, message, timeout=30):
         This is used when we want to send a message into the node that we expect
         will get us disconnected, eg an invalid block."""
         self.send_message(message)
-        self.wait_for_disconnect(timeout)
+        self.wait_for_disconnect(timeout=timeout)
 
 class CompactBlocksTest(BitcoinTestFramework):
     def set_test_params(self):

test/functional/p2p_node_network_limited.py

@@ -138,7 +138,7 @@ def run_test(self):
 
         self.log.info("Requesting block at height 2 (tip-289) must fail (ignored).")
         node.send_getdata_for_block(blocks[0])  # first block outside of the 288+2 limit
-        node.wait_for_disconnect(5)
+        node.wait_for_disconnect(timeout=5)
         self.nodes[0].disconnect_p2ps()
 
         # connect unsynced node 2 with pruned NODE_NETWORK_LIMITED peer

test/functional/p2p_segwit.py

@@ -198,15 +198,15 @@ def announce_block_and_wait_for_getdata(self, block, use_header, timeout=60):
             self.send_message(msg)
         else:
             self.send_message(msg_inv(inv=[CInv(MSG_BLOCK, block.sha256)]))
-            self.wait_for_getheaders()
+            self.wait_for_getheaders(timeout=timeout)
             self.send_message(msg)
-        self.wait_for_getdata([block.sha256])
+        self.wait_for_getdata([block.sha256], timeout=timeout)
 
     def request_block(self, blockhash, inv_type, timeout=60):
         with p2p_lock:
             self.last_message.pop("block", None)
         self.send_message(msg_getdata(inv=[CInv(inv_type, blockhash)]))
-        self.wait_for_block(blockhash, timeout)
+        self.wait_for_block(blockhash, timeout=timeout)
         return self.last_message["block"].block
 
 class SegWitTest(BitcoinTestFramework):
@@ -2056,7 +2056,7 @@ def received_wtxidrelay():
         test_transaction_acceptance(self.nodes[0], self.wtx_node, tx2, with_witness=True, accepted=False)
 
         # Expect a request for parent (tx) by txid despite use of WTX peer
-        self.wtx_node.wait_for_getdata([tx.sha256], 60)
+        self.wtx_node.wait_for_getdata([tx.sha256], timeout=60)
         with p2p_lock:
             lgd = self.wtx_node.lastgetdata[:]
         assert_equal(lgd, [CInv(MSG_WITNESS_TX, tx.sha256)])

test/functional/test_framework/p2p.py

@@ -585,36 +585,36 @@ def test_function():
 
         wait_until_helper_internal(test_function, timeout=timeout, lock=p2p_lock, timeout_factor=self.timeout_factor)
 
-    def wait_for_connect(self, timeout=60):
+    def wait_for_connect(self, *, timeout=60):
         test_function = lambda: self.is_connected
         self.wait_until(test_function, timeout=timeout, check_connected=False)
 
-    def wait_for_disconnect(self, timeout=60):
+    def wait_for_disconnect(self, *, timeout=60):
         test_function = lambda: not self.is_connected
         self.wait_until(test_function, timeout=timeout, check_connected=False)
 
-    def wait_for_reconnect(self, timeout=60):
+    def wait_for_reconnect(self, *, timeout=60):
         def test_function():
             return self.is_connected and self.last_message.get('version') and not self.supports_v2_p2p
         self.wait_until(test_function, timeout=timeout, check_connected=False)
 
     # Message receiving helper methods
 
-    def wait_for_tx(self, txid, timeout=60):
+    def wait_for_tx(self, txid, *, timeout=60):
         def test_function():
             if not self.last_message.get('tx'):
                 return False
             return self.last_message['tx'].tx.rehash() == txid
 
         self.wait_until(test_function, timeout=timeout)
 
-    def wait_for_block(self, blockhash, timeout=60):
+    def wait_for_block(self, blockhash, *, timeout=60):
         def test_function():
             return self.last_message.get("block") and self.last_message["block"].block.rehash() == blockhash
 
         self.wait_until(test_function, timeout=timeout)
 
-    def wait_for_header(self, blockhash, timeout=60):
+    def wait_for_header(self, blockhash, *, timeout=60):
         def test_function():
             last_headers = self.last_message.get('headers')
             if not last_headers:
@@ -623,7 +623,7 @@ def test_function():
 
         self.wait_until(test_function, timeout=timeout)
 
-    def wait_for_merkleblock(self, blockhash, timeout=60):
+    def wait_for_merkleblock(self, blockhash, *, timeout=60):
         def test_function():
             last_filtered_block = self.last_message.get('merkleblock')
             if not last_filtered_block:
@@ -632,7 +632,7 @@ def test_function():
 
         self.wait_until(test_function, timeout=timeout)
 
-    def wait_for_getdata(self, hash_list, timeout=60):
+    def wait_for_getdata(self, hash_list, *, timeout=60):
         """Waits for a getdata message.
 
         The object hashes in the inventory vector must match the provided hash_list."""
@@ -644,7 +644,7 @@ def test_function():
 
         self.wait_until(test_function, timeout=timeout)
 
-    def wait_for_getheaders(self, timeout=60):
+    def wait_for_getheaders(self, *, timeout=60):
         """Waits for a getheaders message.
 
         Receiving any getheaders message will satisfy the predicate. the last_message["getheaders"]
@@ -656,7 +656,7 @@ def test_function():
 
         self.wait_until(test_function, timeout=timeout)
 
-    def wait_for_inv(self, expected_inv, timeout=60):
+    def wait_for_inv(self, expected_inv, *, timeout=60):
         """Waits for an INV message and checks that the first inv object in the message was as expected."""
         if len(expected_inv) > 1:
             raise NotImplementedError("wait_for_inv() will only verify the first inv object")
@@ -668,7 +668,7 @@ def test_function():
 
         self.wait_until(test_function, timeout=timeout)
 
-    def wait_for_verack(self, timeout=60):
+    def wait_for_verack(self, *, timeout=60):
         def test_function():
             return "verack" in self.last_message
 
@@ -681,11 +681,11 @@ def send_version(self):
             self.send_message(self.on_connection_send_msg)
             self.on_connection_send_msg = None  # Never used again
 
-    def send_and_ping(self, message, timeout=60):
+    def send_and_ping(self, message, *, timeout=60):
         self.send_message(message)
         self.sync_with_ping(timeout=timeout)
 
-    def sync_with_ping(self, timeout=60):
+    def sync_with_ping(self, *, timeout=60):
         """Ensure ProcessMessages and SendMessages is called on this connection"""
         # Sending two pings back-to-back, requires that the node calls
         # `ProcessMessage` twice, and thus ensures `SendMessages` must have
@@ -726,7 +726,7 @@ def run(self):
         """Start the network thread."""
         self.network_event_loop.run_forever()
 
-    def close(self, timeout=10):
+    def close(self, *, timeout=10):
         """Close the connections and network event loop."""
         self.network_event_loop.call_soon_threadsafe(self.network_event_loop.stop)
         wait_until_helper_internal(lambda: not self.network_event_loop.is_running(), timeout=timeout)
@@ -933,7 +933,7 @@ def get_invs(self):
         with p2p_lock:
             return list(self.tx_invs_received.keys())
 
-    def wait_for_broadcast(self, txns, timeout=60):
+    def wait_for_broadcast(self, txns, *, timeout=60):
         """Waits for the txns (list of txids) to complete initial broadcast.
         The mempool should mark unbroadcast=False for these transactions.
         """