diff options
author | MarcoFalke <falke.marco@gmail.com> | 2020-06-08 09:13:17 -0400 |
---|---|---|
committer | MarcoFalke <falke.marco@gmail.com> | 2020-06-08 09:13:22 -0400 |
commit | b3ec1fe8114a790adf0f0987d55952de65fd2242 (patch) | |
tree | 5656ce382563737388fdbf8475028c8e53302130 /test/functional/test_framework | |
parent | 9573d2b55b45b24b9920cf5983d1681205ff41df (diff) | |
parent | 34e641a564531853342b03db2d9f0bf52b6e439e (diff) | |
download | bitcoin-b3ec1fe8114a790adf0f0987d55952de65fd2242.tar.xz |
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
Diffstat (limited to 'test/functional/test_framework')
-rwxr-xr-x | test/functional/test_framework/test_framework.py | 1 | ||||
-rw-r--r-- | test/functional/test_framework/util.py | 18 |
2 files changed, 16 insertions, 3 deletions
diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 8e0da5295d..9f5e9e5f0d 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -531,7 +531,6 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): Split the network of four nodes into nodes 0/1 and 2/3. """ disconnect_nodes(self.nodes[1], 2) - disconnect_nodes(self.nodes[2], 1) self.sync_all(self.nodes[:2]) self.sync_all(self.nodes[2:]) 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)) |