test: move check_node_connections to util #23392

pull brunoerg wants to merge 1 commits into bitcoin:master from brunoerg:2021-10-check-node-connections changing 3 files +9 −14

brunoerg commented at 12:55 AM on October 30, 2021: member

This function (check_node_connections) exists in feature_anchors.py and p2p_add_connections.py and does the same thing in both, this PR moves this function to util to avoid repetition and facilitate maintenance.
fanquake added the label Tests on Oct 30, 2021
brunoerg force-pushed on Oct 30, 2021
lsilva01 approved
lsilva01 commented at 3:11 AM on October 30, 2021: contributor

tACK https://github.com/bitcoin/bitcoin/pull/23392/commits/5ce601c32791a4e800b633407b4c5f44f52c6c8f on Ubuntu 20.04
brunoerg force-pushed on Nov 17, 2021
stratospher commented at 11:39 AM on November 18, 2021: contributor

ACK ab9bfc8.
in test/functional/test_framework/util.py:454 in ab9bfc8ad6 outdated
```
 445 | @@ -445,6 +446,14 @@ def set_node_times(nodes, t):
 446 |          node.setmocktime(t)
 447 |  
 448 |  
 449 | +def check_node_connections(*, node, num_in, num_out):
 450 | +    info = node.getnetworkinfo()
 451 | +    if info["connections_in"] != num_in:
 452 | +        raise AssertionError("connections_in: %s != %s" % (str(info["connections_in"]), str(num_in)))
 453 | +    if info["connections_out"] != num_out:
 454 | +        raise AssertionError("connections_out: %s != %s" % (str(info["connections_out"]), str(num_out)))
```
MarcoFalke commented at 2:51 PM on November 18, 2021:

why is this changed?

brunoerg commented at 2:59 PM on November 18, 2021:

To get more friendly error messages. It makes easier to indentify if the error happens because in or out connections.

MarcoFalke commented at 3:05 PM on November 18, 2021:

assert_equal will already print all needed info, no?

brunoerg commented at 4:04 PM on November 18, 2021:

The difference is minimal, example: AssertionError: connections_in: 5 != 3, with assert_equal: AssertionError: not(5 == 4). I can change back to assert_equal if you think its better, np.

MarcoFalke commented at 4:31 PM on November 18, 2021:

python will show the stack trace on uncaught exceptions, which shows that the argument was connections_in. So I think there is no difference in practise?

brunoerg commented at 4:39 PM on November 18, 2021:

Yes, considering the stack trace, there is no difference. Going to change back to assert_equal.

brunoerg commented at 4:41 PM on November 18, 2021:

Done.
MarcoFalke commented at 2:51 PM on November 18, 2021: member

I think all commits can be squashed into one to allow for easy --color-moved=dimmed-zebra.
brunoerg force-pushed on Nov 18, 2021
brunoerg commented at 2:57 PM on November 18, 2021: member

I think all commits can be squashed into one to allow for easy --color-moved=dimmed-zebra.

Done.
test: add check_node_connections in util 988024fe37
brunoerg force-pushed on Nov 18, 2021
laanwj commented at 12:50 PM on November 29, 2021: member

Code review ACK 988024fe37fe00c2deff28e44593e330aa6191f4
laanwj merged this on Nov 29, 2021
laanwj closed this on Nov 29, 2021
sidhujag referenced this in commit c4a2d39463 on Nov 29, 2021
RandyMcMillan referenced this in commit 382f9f146e on Dec 23, 2021
Fabcien referenced this in commit 00657787e1 on Feb 3, 2022
DrahtBot locked this on Nov 29, 2022

Contributors

Labels