#36547 test: return typed P2PInterface from add_p2p_connection helpers

ReviewabilityReviewsAgreementSize
#36547 ViniciusCestarii test: return typed P2PInterface from add_p2p_connection helpersReady1Strong0 + 28 tests
  • Improve autocompletion and static type checking for developers writing functional tests
  • Ready to review
  • Broad support with no outstanding objections
  • Verified by testing editor autocompletion on connection helpers (tumutech)
  • Supportive if others find it helpful (maflcko)
  • Concept approval without stated reasons (fanquake)
  • 0 lines added or modified outside tests
  • 28 lines added or modified in tests
  • 3 lines removed in total
  • 1 files, 1 commits
Test infrastructure#48 of 54
P4 · cleanup
  • P4 because type annotations in test helpers provide modest developer convenience
  • The change offers minor ergonomics without altering test coverage or fixing bugs
Ranking notes
Reviewability: ReadyStalePaused
Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets
Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs.
Size: lines added or modified outside tests, then in tests; greener = smaller.

Goal

Adds type annotations to `add_p2p_connection` and `add_outbound_p2p_connection` in the Python test framework. Uses a generic `TypeVar` bounded by `P2PInterface` for the connection parameter and return value so editors and static analyzers preserve the specific subtype passed in.

Problem: The helper methods in `TestNode` lack type annotations, causing language servers and static analyzers to treat returned connections as `Any` and degrading autocompletion and type checking when writing functional tests.

Evidence of importance:

  • PR description: 'Return type of add_p2p_connection and add_outbound_p2p_connection is not explicitly typed so LSPs treats the return as Any. Add explicit return type to improve DX.'

Risk of a mistake: low (tests). Only touches Python type annotations in test framework helper signatures, which at worst could cause static analysis errors or test startup syntax issues.

Changed since the previous assessment: No functional changes to the patch; rebased or reformatted without altering the type annotations.

Reviewability

Ready

Nothing known that would invalidate a review now.

Waiting on (from the thread read):

  • reviewer: re-review from maflcko following force-push 2026-10-20

Agreement

Strong

substantive support, no open objection (tumutech)

Thread notes: The author addressed maflcko's line-splitting suggestion in a push, and tumutech tested and ACKed the change.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
maflcko (member)suggestion, stylesplit lines in add_p2p_connection helpers as they are getting longfixed · fix pushed · author agreed and split lines in push b08c7da4d19cf259f86a7b91a473fb6014f19eb0answeredyes2026-10-16: 'nit: at some point the lines are getting long enough to split them. Maybe now? (same below)'
Status from: 2026-10-20: ViniciusCestarii: 'Force-pushed b08c7da4d19cf259f86a7b91a473fb6014f19eb0 addressing https://github.com/bitcoin/bitcoin/pull/36547#discussion_r4292253804'

Support:

  • fanquake (member) (Concept ACK, 2026-10-16): (no reason given) [not substantive]
  • maflcko (member) (ACK, 2026-10-16): seems fine if others want them [not substantive]
  • tumutech (none) (ACK, 2026-10-20) [tested: add_p2p_connection, add_outbound_p2p_connection]: in an editor reading python types, method autocompletion works as expected for P2PInterface

Participants:

  • fanquake (member, support, 1 statements 2026-10-16 to 2026-10-16): Concept ACK
  • maflcko (member, support, 2 statements 2026-10-16 to 2026-10-16): approved; suggested splitting long lines
  • tumutech (none, support, 1 statements 2026-10-20 to 2026-10-20): tested ACK verifying LSP method completion in editor

Files

31 lines under test/bench/ci.

  • test/functional/test_framework/test_node.py +28/-3

Card

The short text the ranking pass compares across PRs.

Adds generic type annotations to add_p2p_connection and add_outbound_p2p_connection in the Python test framework using a TypeVar bounded by P2PInterface. Without type annotations, language servers and static analyzers treat returned P2P connections as Any, reducing editor assistance for test developers. This is an internal developer experience improvement with no linked dependencies.

Data

dossier JSON · extract JSON · model ledger, generated 2026-10-22T01:13, confidence high, input hash d1ecd4803d02ebfe
thread read through 2026-10-20 (6 statements), head b08c7da4d1
labels: Tests

Categories

Test infrastructure

P4 · cleanup

Adds type annotations to test framework connection helpers to improve editor autocompletion and static analysis. The PR notes that without explicit return types, 'LSPs treats the return as Any', so typing was added 'to improve DX'. Within test infrastructure, typing and developer-experience improvements represent low-priority cleanliness changes.

Membership: Modifies test framework machinery in test/functional/test_framework/test_node.py and carries the Tests label.

Factors: security/stability 0, bug 0, performance 0, user value 0, leverage 0