#36402 refactor: `wallet_importdescriptors` to separate `run_test()` contents into specific functions

ReviewabilityReviewsAgreementSize
#36402 nebula-21 refactor: `wallet_importdescriptors` to separate `run_test()` contents into specific functionsReady0Positive0 + 543 tests
  • Make the descriptor import functional test easier to read, debug, and extend by splitting a monolithic test
  • Ready to review
  • Concept and approach approval without stated reasons (w0xlt, polespinasa)
  • Author explained why some modified newlines were needed to keep test separation (polespinasa)
  • 0 lines added or modified outside tests
  • 543 lines added or modified in tests
  • 494 lines removed in total
  • 1 files, 1 commits
Wallet#91 of 92
P4 · cleanup
  • P4 because it is an internal test reorganization without changing test coverage or behavior
  • Modifies a single functional test file to improve maintainability for test writers
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

Refactors functional test `wallet_importdescriptors.py` by splitting its monolithic `run_test()` method into modular helper functions. The changes consist of test code movement and reorganization without modifying test coverage or expected behavior.

Problem: The `run_test()` method in `wallet_importdescriptors.py` was over 800 lines long, making test logic difficult to follow, debug, and extend when adding new descriptor import test cases.

Evidence of importance:

  • Author observed during review of another PR that run_test() was 'more than 800 lines of code long, and looked messy and confusing, specially for someone not familiar with that test suite' (PR description)

Scope: Mechanical code movement in a single functional test file.

Risk of a mistake: low (tests). A mistake would cause test flakiness or false failures in the descriptor import functional test.

Changed since the previous assessment: The commit was updated/rebased; the scope and approach remain identical.

Reviewability

Ready

Nothing known that would invalidate a review now.

Waiting on (from the thread read):

Agreement

Positive

support without stated reasons, no open objection (w0xlt, polespinasa)

Thread notes: Nebula-21 announced a rebase on master; no new review claims or support were added.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
polespinasa (member)suggestion, styleRemove unnecessary added and removed newlines that add noise to the color-moved diff filteranswered · author explained newlines were avoided as much as possible but are needed to preserve original test separationsansweredyes2026-10-05: 'There are some unnecessary new lines added and removed that add noise to the color-dimmed filter :)'
Status from: 2026-10-08: nebula-21: 'I've tried to avoid them as much as possible, but some of them are necessary to re-arrange into new functions keeping th'

Support:

  • w0xlt (contributor) (Approach ACK, 2026-10-01): (no reason given) [not substantive]
  • polespinasa (member) (Concept ACK, 2026-10-05): (no reason given) [not substantive]

Participants:

  • w0xlt (contributor, support, 1 statements 2026-10-01 to 2026-10-01): gave an Approach ACK
  • polespinasa (member, support, 1 statements 2026-10-05 to 2026-10-05): gave a Concept ACK and noted unnecessary newlines adding diff noise

Files

1037 lines under test/bench/ci.

  • test/functional/wallet_importdescriptors.py +543/-494

Card

The short text the ranking pass compares across PRs.

Refactors functional test wallet_importdescriptors.py by splitting its 800-line run_test method into modular sub-functions. The PR targets developer friction caused by a monolithic test structure, aiming to make descriptor import tests easier to understand and extend. The only evidence presented is author observation regarding test readability. This change is internal test code movement with no dependencies or behavior changes.

Data

dossier JSON · extract JSON · model ledger, generated 2026-10-10T01:23, confidence high, input hash 43e6eb7062d575b1
thread read through 2026-10-09 (4 statements), head e46592815f
labels: Refactoring

Categories

Wallet

P4 · cleanup

Refactors wallet descriptor functional tests into helper functions without changing behavior or coverage. The author noted the monolithic test method was over 800 lines and 'looked messy and confusing' (PR description).

Membership: Modifies test/functional/wallet_importdescriptors.py, which exercises wallet descriptor import RPCs and is maintained by wallet reviewers.

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