Skip to content

Commit

Permalink
Merge #11641: qa: Only allow disconnecting all NodeConns
Browse files Browse the repository at this point in the history
faaa7db qa: Only allow disconnecting all NodeConns (MarcoFalke)

Pull request description:

  Disconnecting the connection with `index=0` makes no sense when there are more than one connections, as the list "rotates around" and populates index 0 after `del`.

  Just disconnect all NodeConns in any case.

Tree-SHA512: e5cf540823fccb31634b5a11501f54222be89862e80ccafc28bc06726480f8d2153b8c1b6f859fa6a6d087876251d48a6c6035bccdaaf16831e300bc17ff613d
  • Loading branch information
laanwj committed Nov 14, 2017
2 parents cbb54e7 + faaa7db commit 7adeea3
Show file tree
Hide file tree
Showing 4 changed files with 13 additions and 14 deletions.
5 changes: 2 additions & 3 deletions test/functional/maxuploadtarget.py
Expand Up @@ -54,7 +54,7 @@ def run_test(self):
# p2p_conns[2] will test resetting the counters
p2p_conns = []

for i in range(3):
for _ in range(3):
p2p_conns.append(self.nodes[0].add_p2p_connection(TestNode()))

NetworkThread().start() # Start up network handling in another thread
Expand Down Expand Up @@ -139,8 +139,7 @@ def run_test(self):

self.log.info("Peer 2 able to download old block")

for i in range(3):
self.nodes[0].disconnect_p2p()
self.nodes[0].disconnect_p2ps()

#stop and start node 0 with 1MB maxuploadtarget, whitelist 127.0.0.1
self.log.info("Restarting nodes with -whitelist=127.0.0.1")
Expand Down
4 changes: 2 additions & 2 deletions test/functional/p2p-acceptblock.py
Expand Up @@ -206,7 +206,7 @@ def run_test(self):
# The node should have requested the blocks at some point, so
# disconnect/reconnect first

self.nodes[0].disconnect_p2p()
self.nodes[0].disconnect_p2ps()
test_node = self.nodes[0].add_p2p_connection(NodeConnCB())

test_node.wait_for_verack()
Expand Down Expand Up @@ -291,7 +291,7 @@ def run_test(self):
except AssertionError:
test_node.wait_for_disconnect()

self.nodes[0].disconnect_p2p()
self.nodes[0].disconnect_p2ps()
test_node = self.nodes[0].add_p2p_connection(NodeConnCB())

NetworkThread().start() # Start up network handling in another thread
Expand Down
3 changes: 1 addition & 2 deletions test/functional/p2p-leaktests.py
Expand Up @@ -125,8 +125,7 @@ def run_test(self):
assert not unsupported_service_bit5_node.connected
assert not unsupported_service_bit7_node.connected

for _ in range(5):
self.nodes[0].disconnect_p2p()
self.nodes[0].disconnect_p2ps()

# Wait until all connections are closed
wait_until(lambda: len(self.nodes[0].getpeerinfo()) == 0)
Expand Down
15 changes: 8 additions & 7 deletions test/functional/test_framework/test_node.py
Expand Up @@ -182,13 +182,14 @@ def p2p(self):
assert self.p2ps, "No p2p connection"
return self.p2ps[0]

def disconnect_p2p(self, index=0):
"""Close the p2p connection to the node."""
# Connection could have already been closed by other end. Calling disconnect_p2p()
# on an already disconnected p2p connection is not an error.
if self.p2ps[index].connection is not None:
self.p2ps[index].connection.disconnect_node()
del self.p2ps[index]
def disconnect_p2ps(self):
"""Close all p2p connections to the node."""
for p in self.p2ps:
# Connection could have already been closed by other end.
if p.connection is not None:
p.connection.disconnect_node()
self.p2ps = []


class TestNodeCLI():
"""Interface to bitcoin-cli for an individual node"""
Expand Down

0 comments on commit 7adeea3

Please sign in to comment.