Added a functional test which parses all the RPC help commands then automatically generates the bitcoin-cli bash-completion file and makes sure that the original file matches the newly generated one.
In order to get the RPC help commands in the correct format needed for the test, I added the “format” RPC command.
Test using python3 test/functional/tool_cli_completion.py --overwrite
Concept ACK on auto-generating what can be auto-generated.
I’m not entirely sure about the approach of making this a functional test. It’s clever, but also different from how other auto-generated files (the manual pages, the example bitcoin.conf introduced in #22235) are handled in the release process.
On the other hand by adding the check this forces the file on the branch to always be up-to-date so it doesn’t need to be in the release process. That’s also something.
Concept ACK on auto-generating what can be auto-generated.
I’m not entirely sure about the approach of making this a functional test. It’s clever, but also different from how other auto-generated files (the manual pages, the example bitcoin.conf introduced in #22235) are handled in the release process.
On the other hand by adding the check this forces the file on the branch to always be up-to-date so it doesn’t need to be in the release process. That’s also something.
Look at #17289#issue-513520983
From linter:
0File "test/functional/tool_cli_completion.py" contains a shebang line, but has the file permission 644 instead of the expected executable permission 755. Do "chmod 755 test/functional/tool_cli_completion.py"
I mean you can argue this is a better solution, and that we should move to this for all three. (but for the manual pages it’s difficult as they contain a version number as well as the current month)
Thanks for the review @laanwj! Just wondering, do the three of them have to be done the same way? I don’t know how I would do it the same way it was done in #22235.
do the three of them have to be done the same way?
Not necessarily, but some structure/standardization around release processes makes it harder to forget any steps, than if everything needs to be done in a different way.
On the other hand by adding the check this forces the file on the branch to always be up-to-date so it doesn’t need to be in the release process. That’s also something.
If we’re not careful, it could also increase the rate of diff conflicts (and/or silent merge bugs?).
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| Concept ACK | laanwj |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
Reviewers, this pull request conflicts with the following ones:
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.
289@@ -290,8 +290,7 @@ static RPCHelpMan addnode()
290 if (!request.params[1].isNull())
291 strCommand = request.params[1].get_str();
292 if (strCommand != "onetry" && strCommand != "add" && strCommand != "remove") {
293- throw std::runtime_error(
294- self.ToString());
295+ throw std::runtime_error(self.ToString());
achow101 Any idea why CI is not running on this PR?
Rebasing may help. While you are at it, you can also squash your commits.
Add a functional test which parses available RPC commands, generates
the associated bitcoin-cli autocomplete file and checks that the current
autocomplete file matches.
An outdated autocomplete file can be updated via the --overwrite test
parameter.
test: keeps bitcoin-cli autocomplete in sync
Add a functional test which parses available RPC commands, generates
the associated bitcoin-cli autocomplete file and checks that the current
autocomplete file matches.
An outdated autocomplete file can be updated via the --overwrite test
parameter.
rpc: add format command with support for args_cli
- add format command to get infos about commands via a particular format
- add output format args_cli to get arguments type info of shown commands
- refactor RPCArg::ToTypeString to be used accross multiple output formats
- overload CRPCTable::execute to call != methods than in their request
test: keeps bitcoin-cli autocomplete in sync
fixed linter error
added bash-completion to makefile
fixed typo
autogenerate bash completion
autogenerate bash completion
added tool_bash_completion.py to test_runner.py
added format RPC
fixed typo
added python functional test
test: keeps bitcoin-cli autocomplete in sync
Add a functional test which parses available RPC commands, generates
the associated bitcoin-cli autocomplete file and checks that the current
autocomplete file matches.
An outdated autocomplete file can be updated via the --overwrite test
parameter.
rpc: add format command with support for args_cli
- add format command to get infos about commands via a particular format
- add output format args_cli to get arguments type info of shown commands
- refactor RPCArg::ToTypeString to be used accross multiple output formats
- overload CRPCTable::execute to call != methods than in their request
removed duplicates
removed unnecessary change
🐙 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?
There hasn’t been much activity lately and the patch still needs rebase. What is the status here?