[test] clarify rpc_net & p2p_disconnect_ban functional tests
ClosedPublic
Actions

Authored by roqqit on Wed, Jan 15, 00:01.

Details

Reviewers

Fabien
PiRK

Group Reviewers

Restricted Project

Commits

rABC7878d95a0db2: [test] clarify rpc_net & p2p_disconnect_ban functional tests

Summary

Pull request description:

small improvements to clarify logic in the functional tests
1. have test logic in `rpc_net.py` match run order of the test
2. remove `connect_nodes` calls that are redundant with the automatic test setup executed by the test framework

Noticed when I was trying to debug a test for #19725. Small changes but imo very helpful, because they initially confused me.

Backport of core#19877

Test Plan

./test/functional/test_runner.py rpc_net p2p_disconnect_ban

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

roqqit created this revision.Wed, Jan 15, 00:01

Herald added a reviewer: Restricted Project. · View Herald TranscriptWed, Jan 15, 00:01

roqqit requested review of this revision.Wed, Jan 15, 00:01

teamcity edited the summary of this revision. (Show Details)Wed, Jan 15, 00:01

roqqit added a child revision: D17530: test: Fail if connect_nodes fails.Wed, Jan 15, 00:07

Harbormaster completed remote builds in B32089: Diff 52196.Wed, Jan 15, 00:16

Not really related to the changes but since you are touching the code it's best to fix it

test/functional/rpc_net.py
91 ↗	(On Diff #52196)	this is implicit with self.generate, remove
114 ↗	(On Diff #52196)	This would be more robust with a mock time

This revision now requires changes to proceed.Wed, Jan 15, 09:15

PiRK accepted this revision.Wed, Jan 15, 09:16

roqqit added a child revision: D17538: [test] Make test_getpeerinfo more robust by using mocktime.Wed, Jan 15, 17:19

roqqit requested review of this revision.Wed, Jan 15, 17:20

roqqit added inline comments.

test/functional/rpc_net.py
91 ↗	(On Diff #52196)	There is a backport that cleans this up. I will do this in a separate diff.
114 ↗	(On Diff #52196)	It is best to keep this block move-only to match the backport. I made this improvement in a separate diff: https://reviews.bitcoinabc.org/D17538