Skip to content

Conversation

@guggero
Copy link
Contributor

@guggero guggero commented Sep 26, 2020

This is a follow-up patch to #19804 as suggested by MarcoFalke (#19804 (comment)).

To make the intent of the tests easier to understand, we reference the
p2p connection objects by their explicit names instead of the p2ps array.

To make the intent of the tests easier to understand, we reference the
p2p connection objects by their explicit names instead of the p2ps array.
@DrahtBot DrahtBot added the Tests label Sep 26, 2020
@practicalswift
Copy link
Contributor

Concept ACK

Welcome (again) as a contributor @guggero! :)

Copy link
Contributor

@theStack theStack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 0fcaf73
Identified the remaining p2ps[] accesses via git grep "\.p2ps\[" and verified that the tests p2p_getdata.py and wallet_resendwallettransactions.py have explicit p2p objects that can be used instead. The affected tests passed locally. 👌

@maflcko maflcko merged commit 8aa3a4a into bitcoin:master Sep 26, 2020
@guggero guggero deleted the p2p-conn-rename branch September 26, 2020 10:56
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Oct 14, 2021
Summary:
To make the intent of the tests easier to understand, we reference the
p2p connection objects by their explicit names instead of the p2ps array.

This is a backport of [[bitcoin/bitcoin#20022 | core#20022]]

Test Plan: `test/functional/test_runner.py p2p_getdata wallet_resendwallettransactions`

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D10343
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants