-
Notifications
You must be signed in to change notification settings - Fork 1.2k
test: RPC coverage linter + related fixes for listaddressbalances #7279
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
afa9b91
f6c66c8
4b1a335
2b3cdfb
ff4302f
53cd792
c27ba87
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -69,7 +69,7 @@ class TestNode(): | |
| To make things easier for the test writer, any unrecognised messages will | ||
| be dispatched to the RPC connection.""" | ||
|
|
||
| def __init__(self, i, datadir, extra_args_from_options, *, chain, rpchost, timewait, timeout_factor, bitcoind, bitcoin_cli, mocktime, coverage_dir, cwd, extra_conf=None, extra_args=None, use_cli=False, start_perf=False, use_valgrind=False, version=None, descriptors=False, v2transport=False): | ||
| def __init__(self, i, datadir, extra_args_from_options, *, chain, rpchost, timewait, timeout_factor, bitcoind, bitcoin_cli, mocktime, coverage_dir, cwd, extra_conf=None, extra_args=None, use_cli=False, start_perf=False, use_valgrind=False, version=None, descriptors=False, v2transport=False, uses_wallet=False): | ||
| """ | ||
| Kwargs: | ||
| start_perf (bool): If True, begin profiling the node with `perf` as soon as | ||
|
|
@@ -114,7 +114,7 @@ def __init__(self, i, datadir, extra_args_from_options, *, chain, rpchost, timew | |
| if self.mocktime != 0: | ||
| self.args.append(f"-mocktime={mocktime}") | ||
|
|
||
| if self.descriptors is None: | ||
| if uses_wallet is not None and not uses_wallet and self.descriptors is None: | ||
| self.args.append("-disablewallet") | ||
|
Comment on lines
72
to
118
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💬 Nitpick: Partial backport of bitcoin#28710 bitcoin#33064 relies on the source: ['claude'] |
||
|
|
||
| # Use valgrind, expect for previous release binaries | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Suggestion: Two overlapping wallet flags:
_requires_walletanduses_walletBitcoinTestFramework.__init__now owns two separately-purposed wallet flags:_requires_wallet(set byskip_if_no_wallet()at line 1071, gatesimport_deterministic_coinbase_privkeys()at lines 466 and 1820) anduses_wallet(controls whether-disablewalletis appended byTestNodeat test_node.py:117). The distinction is undocumented and easy to get wrong — e.g. the missed propagation inadd_dynamically_node()at line 619 is partly enabled by the fact that tests must now remember both flags. Consider either deriving one from the other (self.uses_wallet = self.uses_wallet or self._requires_wallet) or adding an explanatory comment at the definition site so future test authors don't set only one and wonder why the wallet is still disabled.source: unknown
🤖 Fix this with AI agents