Merge #18890: test: disconnect_nodes should warn if nodes were already disconnected

34e641a564531853342b03db2d9f0bf52b6e439e test: Remove unnecessary disconnect_nodes call in rpc_psbt.py (Danny Lee)
e6e7abd51a9a6027acac7a9964e36357f25e242c test: remove redundant two-way disconnect_nodes calls (Danny Lee)
a9bd1f9adf869a95f70b3a40615a2f8e8e52db1d test: warn if nodes not connected before disconnect_nodes (Danny Lee)

Pull request description:

  There's no harm in calling `disconnect_nodes` for nodes that weren't connected (in this case it's a no-op).  However, detecting this case and logging a warning can help ensure that tests are behaving as expected.

  In addition, since `disconnect_nodes` works bidirectionally, I removed all instances of this pattern:

  ```
  disconnect_nodes(self.nodes[0], 1)
  disconnect_nodes(self.nodes[1], 0)
  ```

ACKs for top commit:
  MarcoFalke:
    review ACK 34e641a564531853342b03db2d9f0bf52b6e439e 👔
  amitiuttarwar:
    ACK 34e641a564. Thanks for this test improvement!

Tree-SHA512: 344855ceb46c012d43c13d7c09f44d32dcb7645706d10ae1e4645d9edca54c6c6c13fee26b79480755cdfcdf39b4b5770b36bb03ce71ba002d5be8a27fe008af

1 files changed, 16 insertions, 2 deletions

diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py
index 6dfea7efd2..52306c8c3d 100644
--- a/test/functional/test_framework/util.py
+++ b/test/functional/test_framework/util.py
@@ -381,7 +381,21 @@ def set_node_times(nodes, t):
         node.setmocktime(t)
 
 def disconnect_nodes(from_connection, node_num):
-    for peer_id in [peer['id'] for peer in from_connection.getpeerinfo() if "testnode%d" % node_num in peer['subver']]:
+    def get_peer_ids():
+        result = []
+        for peer in from_connection.getpeerinfo():
+            if "testnode{}".format(node_num) in peer['subver']:
+                result.append(peer['id'])
+        return result
+
+    peer_ids = get_peer_ids()
+    if not peer_ids:
+        logger.warning("disconnect_nodes: {} and {} were not connected".format(
+            from_connection.index,
+            node_num
+        ))
+        return
+    for peer_id in peer_ids:
         try:
             from_connection.disconnectnode(nodeid=peer_id)
         except JSONRPCException as e:
@@ -392,7 +406,7 @@ def disconnect_nodes(from_connection, node_num):
                 raise
 
     # wait to disconnect
-    wait_until(lambda: [peer['id'] for peer in from_connection.getpeerinfo() if "testnode%d" % node_num in peer['subver']] == [], timeout=5)
+    wait_until(lambda: not get_peer_ids(), timeout=5)
 
 def connect_nodes(from_connection, node_num):
     ip_port = "127.0.0.1:" + str(p2p_port(node_num))