Page MenuHomePhabricator

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

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

Details

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 Not Applicable
Unit
Tests Not Applicable

Event Timeline

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

fix a few missed p2p references

Fabien requested changes to this revision.Wed, Oct 13, 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.Wed, Oct 13, 13:01
test/functional/abc-segwit-recovery.py
188 ↗(On Diff #30486)

This function is actually unused now. Will remove it.

PiRK edited the summary of this revision. (Show Details)

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 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

This revision is now accepted and ready to land.Wed, Oct 13, 15:52
This revision was landed with ongoing or failed builds.Thu, Oct 14, 11:47
This revision was automatically updated to reflect the committed changes.