[refactor] clarify abc* tests by referencing p2p objects directly
ClosedPublic
Actions

Authored by PiRK on Oct 13 2021, 07:19.

Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Commits

rABCa57f68f3faa1: [refactor] clarify abc* tests by referencing p2p objects directly

Summary

Use object returned from add_p2p_connection to refer to p2ps, or use node.p2ps[0] when it needs to be used across many methods. Don't use the p2p property.

For these files, this is best done by removing some unnecessary (and often overcomplicated) helper functions that can be replaced with a single line of code at the callsite.

This is a backport of core#19804 [1b/4]

Test Plan

ninja check-functional-extended

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 16989
Build 33814: Build Diff	build-without-wallet · build-diff · build-debug · build-clang-tidy · build-clang
Build 33813: arc lint + arc unit

Event Timeline

PiRK created this revision.Oct 13 2021, 07:19

Herald added a reviewer: Restricted Project. · View Herald TranscriptOct 13 2021, 07:19

PiRK requested review of this revision.Oct 13 2021, 07:19

teamcity edited the summary of this revision. (Show Details)Oct 13 2021, 07:19

PiRK added a child revision: D10325: [test] remove confusing p2p property.Oct 13 2021, 08:33

fix a few missed p2p references

Harbormaster completed remote builds in B16995: Diff 30486.Oct 13 2021, 10:17

Fabien requested changes to this revision.Oct 13 2021, 13:01

Fabien added a subscriber: Fabien.

Fabien added inline comments.

test/functional/abc-minimaldata.py
56 ↗	(On Diff #30486)	`peer` is not an attribute of `BitcoinTestFramework` but an attribute of a node, so `self.peer` is not appropriated here and this is a case where you should use self.nodes[0].p2ps[0] imo
test/functional/abc-schnorr.py
75 ↗	(On Diff #30486)	dito
test/functional/abc-schnorrmultisig.py
77 ↗	(On Diff #30486)	dito
test/functional/abc-segwit-recovery.py
188 ↗	(On Diff #30486)	which one is that ? std or nonstd ?

This revision now requires changes to proceed.Oct 13 2021, 13:01

PiRK added inline comments.Oct 13 2021, 13:08

test/functional/abc-segwit-recovery.py
188 ↗	(On Diff #30486)	This function is actually unused now. Will remove it.

replace self.peer with self.nodes[0].p2ps[0]

remove a function that is now unused in abc-segwit-recovery.py (this test has more unused functions, but I will address this in a separate revision).

Fabien accepted this revision.Oct 13 2021, 15:52

Fabien added inline comments.

test/functional/abc-schnorrmultisig.py
124 ↗	(On Diff #30493)	That would be a good idea to clean these kind of tests by supplying the peer to the various test methods and using a local variable instead of node.p2ps[0], but it's out of scope here