Merge #19489: test: Fail wait_until early if connection is lost

faa9a74 test: Fail wait_until early if connection is lost (MarcoFalke)

Pull request description:

  Calling `minonode.wait_until` needs a connection to make progress (e.g. waiting for an inv), unless the mininode waits for the initial connection or for a disconnection. So for test development and failure debugging, fail early in all `wait_until`, unless opted out.

ACKs for top commit:
  jnewbery:
    Code review ACK faa9a74.

Tree-SHA512: 4be850b96e23b87bc2ff42c028a5045d6f5cdbc9482ce6a6ba01cc5eb26710dab9e2ed547c363aac4bd5825151ee9996fb797261420b631bceeddbfa698d1dec

Author: MarcoFalke

test/functional/p2p_filter.py

@@ -218,7 +218,11 @@ def run_test(self):
         # Add peer but do not send version yet
         filter_peer_without_nrelay = self.nodes[0].add_p2p_connection(P2PBloomFilter(), send_version=False, wait_for_verack=False)
         # Send version with fRelay=False
-        filter_peer_without_nrelay.wait_until(lambda: filter_peer_without_nrelay.is_connected, timeout=10)
+        filter_peer_without_nrelay.wait_until(
+            lambda: filter_peer_without_nrelay.is_connected,
+            timeout=10,
+            check_connected=False,
+        )
         version_without_fRelay = msg_version()
         version_without_fRelay.nRelay = 0
         filter_peer_without_nrelay.send_message(version_without_fRelay)

test/functional/p2p_ping.py

@@ -7,13 +7,8 @@
 
 import time
 
-from test_framework.messages import (
-    msg_pong,
-)
-from test_framework.mininode import (
-    P2PInterface,
-    wait_until,
-)
+from test_framework.messages import msg_pong
+from test_framework.mininode import P2PInterface
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import assert_equal
 
@@ -78,7 +73,7 @@ def run_test(self):
         with self.nodes[0].assert_debug_log(['pong peer=0: Nonce mismatch']):
             # mock time PING_INTERVAL ahead to trigger node into sending a ping
             self.mock_forward(PING_INTERVAL + 1)
-            wait_until(lambda: 'ping' in no_pong_node.last_message)
+            no_pong_node.wait_until(lambda: 'ping' in no_pong_node.last_message)
             self.mock_forward(9)
             # Send the wrong pong
             no_pong_node.send_and_ping(msg_pong(no_pong_node.last_message.pop('ping').nonce - 1))
@@ -93,27 +88,27 @@ def run_test(self):
         assert 'ping' not in no_pong_node.last_message
         # mock time PING_INTERVAL ahead to trigger node into sending a ping
         self.mock_forward(PING_INTERVAL + 1)
-        wait_until(lambda: 'ping' in no_pong_node.last_message)
+        no_pong_node.wait_until(lambda: 'ping' in no_pong_node.last_message)
         ping_delay = 29
         self.mock_forward(ping_delay)
-        wait_until(lambda: 'ping' in no_pong_node.last_message)
+        no_pong_node.wait_until(lambda: 'ping' in no_pong_node.last_message)
         no_pong_node.send_and_ping(msg_pong(no_pong_node.last_message.pop('ping').nonce))
         self.check_peer_info(pingtime=ping_delay, minping=ping_delay, pingwait=None)
 
         self.log.info('Check that minping is decreased after a fast roundtrip')
         # mock time PING_INTERVAL ahead to trigger node into sending a ping
         self.mock_forward(PING_INTERVAL + 1)
-        wait_until(lambda: 'ping' in no_pong_node.last_message)
+        no_pong_node.wait_until(lambda: 'ping' in no_pong_node.last_message)
         ping_delay = 9
         self.mock_forward(ping_delay)
-        wait_until(lambda: 'ping' in no_pong_node.last_message)
+        no_pong_node.wait_until(lambda: 'ping' in no_pong_node.last_message)
         no_pong_node.send_and_ping(msg_pong(no_pong_node.last_message.pop('ping').nonce))
         self.check_peer_info(pingtime=ping_delay, minping=ping_delay, pingwait=None)
 
         self.log.info('Check that peer is disconnected after ping timeout')
         assert 'ping' not in no_pong_node.last_message
         self.nodes[0].ping()
-        wait_until(lambda: 'ping' in no_pong_node.last_message)
+        no_pong_node.wait_until(lambda: 'ping' in no_pong_node.last_message)
         with self.nodes[0].assert_debug_log(['ping timeout: 1201.000000s']):
             self.mock_forward(20 * 60 + 1)
             time.sleep(4)  # peertimeout + 1

test/functional/test_framework/mininode.py

@@ -388,18 +388,22 @@ def on_version(self, message):
 
     # Connection helper methods
 
-    def wait_until(self, test_function, timeout=60):
+    def wait_until(self, test_function_in, *, timeout=60, check_connected=True):
+        def test_function():
+            if check_connected:
+                assert self.is_connected
+            return test_function_in()
+
         wait_until(test_function, timeout=timeout, lock=mininode_lock, timeout_factor=self.timeout_factor)
 
     def wait_for_disconnect(self, timeout=60):
         test_function = lambda: not self.is_connected
-        self.wait_until(test_function, timeout=timeout)
+        self.wait_until(test_function, timeout=timeout, check_connected=False)
 
     # Message receiving helper methods
 
     def wait_for_tx(self, txid, timeout=60):
         def test_function():
-            assert self.is_connected
             if not self.last_message.get('tx'):
                 return False
             return self.last_message['tx'].tx.rehash() == txid
@@ -408,14 +412,12 @@ def test_function():
 
     def wait_for_block(self, blockhash, timeout=60):
         def test_function():
-            assert self.is_connected
             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 test_function():
-            assert self.is_connected
             last_headers = self.last_message.get('headers')
             if not last_headers:
                 return False
@@ -425,7 +427,6 @@ def test_function():
 
     def wait_for_merkleblock(self, blockhash, timeout=60):
         def test_function():
-            assert self.is_connected
             last_filtered_block = self.last_message.get('merkleblock')
             if not last_filtered_block:
                 return False
@@ -437,9 +438,7 @@ 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."""
-
         def test_function():
-            assert self.is_connected
             last_data = self.last_message.get("getdata")
             if not last_data:
                 return False
@@ -454,9 +453,7 @@ def wait_for_getheaders(self, timeout=60):
         value must be explicitly cleared before calling this method, or this will return
         immediately with success. TODO: change this method to take a hash value and only
         return true if the correct block header has been requested."""
-
         def test_function():
-            assert self.is_connected
             return self.last_message.get("getheaders")
 
         self.wait_until(test_function, timeout=timeout)
@@ -467,7 +464,6 @@ def wait_for_inv(self, expected_inv, timeout=60):
             raise NotImplementedError("wait_for_inv() will only verify the first inv object")
 
         def test_function():
-            assert self.is_connected
             return self.last_message.get("inv") and \
                                 self.last_message["inv"].inv[0].type == expected_inv[0].type and \
                                 self.last_message["inv"].inv[0].hash == expected_inv[0].hash
@@ -478,7 +474,7 @@ def wait_for_verack(self, timeout=60):
         def test_function():
             return "verack" in self.last_message
 
-        self.wait_until(test_function, timeout=timeout)
+        self.wait_until(test_function, timeout=timeout, check_connected=False)
 
     # Message sending helper functions
 
@@ -491,7 +487,6 @@ def sync_with_ping(self, timeout=60):
         self.send_message(msg_ping(nonce=self.ping_counter))
 
         def test_function():
-            assert self.is_connected
             return self.last_message.get("pong") and self.last_message["pong"].nonce == self.ping_counter
 
         self.wait_until(test_function, timeout=timeout)
@@ -609,7 +604,11 @@ def send_blocks_and_test(self, blocks, node, *, success=True, force_send=False,
                     self.send_message(msg_block(block=b))
             else:
                 self.send_message(msg_headers([CBlockHeader(block) for block in blocks]))
-                self.wait_until(lambda: blocks[-1].sha256 in self.getdata_requests, timeout=timeout)
+                self.wait_until(
+                    lambda: blocks[-1].sha256 in self.getdata_requests,
+                    timeout=timeout,
+                    check_connected=success,
+                )
 
             if expect_disconnect:
                 self.wait_for_disconnect(timeout=timeout)
@@ -677,6 +676,6 @@ def wait_for_broadcast(self, txns, timeout=60):
         The mempool should mark unbroadcast=False for these transactions.
         """
         # Wait until invs have been received (and getdatas sent) for each txid.
-        self.wait_until(lambda: set(self.tx_invs_received.keys()) == set([int(tx, 16) for tx in txns]), timeout)
+        self.wait_until(lambda: set(self.tx_invs_received.keys()) == set([int(tx, 16) for tx in txns]), timeout=timeout)
         # Flush messages and wait for the getdatas to be processed
         self.sync_with_ping()