#36022 test: add coverage for bitcoin wrapper argument handling

ReviewabilityReviewsAgreementSize
#36022 cyb3ralbert test: add coverage for bitcoin wrapper argument handlingAuthor silent 69d1Positive0 + 51 tests
  • Verify CLI argument handling and error behavior for the bitcoin wrapper tool
  • Ensure wrapper paths that exit before executing child processes are tested
  • Ready to review
  • Tested approval without stated reasons (mercie-ux)
  • Resolved earlier concern that tests bypassed Valgrind instrumentation (mercie-ux)
  • 0 lines added or modified outside tests
  • 51 lines added or modified in tests
  • 0 lines removed in total
  • 1 files, 2 commits
Test infrastructure#41 of 54
P4 · test coverage
  • P4 because it covers low-risk CLI argument paths that return before executing a child process
  • Adds narrow functional coverage without touching test framework machinery or fixing flaky tests
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 test coverage for the `bitcoin` CLI wrapper argument parsing in `test/functional/tool_bitcoin.py`. It tests `--version`, `--help`, execution without arguments, and unrecognized commands/options, which previously returned without calling `exec` and were not covered by tests.

Problem: The `bitcoin` wrapper executable had zero test coverage for paths that return before executing a child process, leaving CLI argument parsing and error handling unverified.

Reviewability

Stale · Author silent 69d

No author activity for 69 days; a review may wait a long time for a response.

Waiting on (from the thread read):

  • nothing: nothing pending; reviewer tested and ACKed 2026-09-25

Agreement

Positive

support without stated reasons, no open objection (mercie-ux)

Thread notes: mercie-ux tested and ACKed commit b244610a (c:5828659288), changing their stance to support and accepting the earlier valgrind instrumentation fix.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
mercie-ux (contributor)objection, correctnessnew wrapper tests bypass Valgrind instrumentation when running test_runner.py with --valgrindaccepted · fix pushed · reviewer tested and ACKed the updated PR, accepting the fix · v1, not yet re-checkedansweredyes2026-08-20: 'run_wrapper calls bitcoin_bin directly, bypassing node.binaries.valgrind_cmd that set_cmd_args already uses. So test_version/test_help/test_no_command/test_unrecognized never run under valgrind'
Status from: 2026-09-25: mercie-ux: 'tACK b244610a672fc7591297d3db5c63b15d1c3afb1f'

Support:

  • mercie-ux (contributor) (Tested ACK, 2026-09-25): (no reason given) [not substantive]

Participants:

  • mercie-ux (contributor, support, 2 statements 2026-08-20 to 2026-09-25): tested ACK b244610a; earlier noted wrapper tests bypass valgrind_cmd (fixed)

Files

51 lines under test/bench/ci.

  • test/functional/tool_bitcoin.py +51/-0

Card

The short text the ranking pass compares across PRs.

Adds functional tests for the bitcoin wrapper binary's argument handling, covering --version, --help, invocation without commands, and invalid options. Previously, src/bitcoin.cpp had no coverage in the functional test suite for non-exec exit paths. mercie-ux reviewed and suggested valgrind integration, which the author implemented. The PR is ready for review with no blockers.

Data

dossier JSON · extract JSON · model ledger, generated 2026-09-26T01:21, confidence high, input hash 0271b1d34a3c3de7
thread read through 2026-09-25 (3 statements), head b244610a67
Discussion read under schema 1 rules: suggestions and questions may be missing, and an objection may combine several harms (schema history).
labels: Tests

Categories

Test infrastructure

P4 · test coverage

Adds functional test coverage for CLI argument parsing in the `bitcoin` wrapper executable. The PR covers `--version`, `--help`, and invalid arguments that return before calling `exec`, addressing low-risk paths without affecting test framework machinery or addressing flaky test signal.

Membership: Labeled 'Tests' and adds test cases to test/functional/tool_bitcoin.py covering the bitcoin wrapper tool.

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