qa: Only allow disconnecting all NodeConns #11641

pull MarcoFalke wants to merge 1 commits into bitcoin:master from MarcoFalke:Mf1711-qaNodeConnDisconnectAll changing 4 files +13 −14

MarcoFalke commented at 10:21 PM on November 8, 2017: member

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.
fanquake added the label Tests on Nov 8, 2017
in test/functional/maxuploadtarget.py:57 in faed80343e outdated
```
  53 | @@ -54,7 +54,7 @@ def run_test(self):
  54 |          # p2p_conns[2] will test resetting the counters
  55 |          p2p_conns = []
  56 |  
  57 | -        for i in range(3):
  58 | +        for _ in range(3):
```
JohnVonNeumann commented at 4:34 AM on November 9, 2017:

Interested in the use of the underscore as the variable name here, did some reading and from what I've read use of the underscore only works as something other than a variable name when used in an interactive python shell? Other than that people seem to believe it's a bad naming convention.

Context: https://stackoverflow.com/questions/26895362/what-does-in-python-do

laanwj commented at 9:05 AM on November 9, 2017:

_ as a variable name in python is a convention to indicate that the result won't be used.

JohnVonNeumann commented at 6:10 PM on November 9, 2017:

Thanks for the explanation, makes sense.

in test/functional/test_framework/test_node.py:188 in faed80343e outdated

 191 | -        del self.p2ps[index]
 192 | +    def disconnect_p2ps(self):
 193 | +        """Close all p2p connections to the node."""
 194 | +        for _ in range(len(self.p2ps)):
 195 | +            index = 0
 196 | +            # Connection could have already been closed by other end.

laanwj commented at 7:04 PM on November 13, 2017:

Is the order of deletion relevant? If not, something like this is both easier to read and likely more efficient:

for x in self.p2ps:
    if x.connection is not None:
        x.connection.disconnect_node()
self.p2ps = []

MarcoFalke commented at 7:06 PM on November 13, 2017:

No, the order is not relevant. Will switch to your suggestion. (I tried to minimize the --ignore-all-space diff)

MarcoFalke force-pushed on Nov 13, 2017
MarcoFalke force-pushed on Nov 13, 2017
qa: Only allow disconnecting all NodeConns faaa7db25e
MarcoFalke force-pushed on Nov 13, 2017
laanwj commented at 7:56 AM on November 14, 2017: member

utACK faaa7db
laanwj merged this on Nov 14, 2017
laanwj closed this on Nov 14, 2017
laanwj referenced this in commit 7adeea3b0f on Nov 14, 2017
MarcoFalke deleted the branch on Nov 14, 2017
codablock referenced this in commit d8e6ffc42f on Sep 30, 2019
codablock referenced this in commit 0e36f31382 on Sep 30, 2019
codablock referenced this in commit ed93248c70 on Oct 2, 2019
codablock referenced this in commit dafdfb5e47 on Oct 2, 2019
UdjinM6 referenced this in commit 781a54cabc on Oct 3, 2019
codablock referenced this in commit 6e6e950fca on Oct 3, 2019
barrystyle referenced this in commit fd423ce7a6 on Jan 22, 2020
DrahtBot locked this on Sep 8, 2021

Contributors

Labels