The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #29335 (test: Handle functional test disk-full error by BrandonOdiwuor)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

Also, it doesn't seem to work, because the CI should fail, because the abortrescan rpc is uncovered

@maflcko this is because self.options.descriptors == None in TestNode(... , descriptors=self.options.descriptors, ...) hence the disable_wallet flag is added when the TestNode is instantiated disabling the wallet RPCs just before the first start_node() is invoked

@jo-elimu that's the error the fix was to catch. Check the issue

In any case, a pull request with a failing CI can not be merged. Someone will need to add coverage for the RPC, here, or in a different pull.

Concept ACK 7dc0442ff5013827c051c8ff50c8fd8aa9440d7c.

@maflcko I have added a basic test for the abortrescan RPC and TODO for adding a test when rescanblockchain is running

I cannot add the second test since I'm unable to get abortrescan to execute when a rescan is ongoing

@fanquake fixed by disabling --descriptors flag when running create_cache.py if --legacy-wallet flag is provided

Can you test what happens when the wallet is completely disabled in configure?

@maflcko When wallet is disabled these RPS are reported as uncovered;

Why would they? If the RPCs are not compiled into the binary, how would the test know they exist?

@maflcko I was responding to this request

Can you test what happens when the wallet is completely disabled in configure?
#29387 (comment)

I just tried this myself with --disable-wallet, then call help, and the wallet RPCs were not printed. write_all_rpc_commands also uses help(), so I wonder how you achieved this outcome.

@maflcko @hernanmarino sorry I misunderstood #29387 (comment) earlier

When the wallet is disabled in the configure, the wallet RPCs are not included in the rpc_interface.txt list, hence not part of the coverage check

(example below from my test)
rpc_interface.txt

🐙 This pull request conflicts with the target branch and needs rebase.

⌛ There hasn't been much activity lately and the patch still needs rebase. What is the status here?

• Is it still relevant? ➡️ Please solve the conflicts to make it ready for review and to ensure the CI passes.
• Is it no longer relevant? ➡️ Please close.
• Did the author lose interest or time to work on this? ➡️ Please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.

Picked up in #33064.