-
Notifications
You must be signed in to change notification settings - Fork 37.7k
test: checks that bitcoin-cli autocomplete is in sync #18606
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
Conversation
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.
Concept ACK
a332a11
to
4576b77
Compare
It seems that even when using A decent way to fix this would be to add the bash auto-completion file to If you think this is too intrusive, another more hackish way would be to just look for the file in the parent directories (https://github.com/pierreN/bitcoin/blob/feature-auto-cli-2/test/functional/tool_cli_completion.py#L260)? |
""" Complete the arguments of option via the RPC help command. """ | ||
|
||
# if we can't find an argument list in the help text, then it has no option to add | ||
regexp_res = ARG_REGEXP.match(self.nodes[0].help(option.command)) |
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.
regexp_res = ARG_REGEXP.match(self.nodes[0].help(option.command)) | |
res = self.nodes[0].format(command=option.command, output=args_cli) |
Instead of using regex to parse the pseudo doc-json, a hidden RPC could be added that spits out the format you want.
I think all you need to do is register a formatter in RPCMan::Check
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.
Yes, good idea. I have updated the branch to do that. Please have a look at the new code, thanks.
Note that some non-hidden RPC calls didn't use RPCHelpMan::Check so I added a commit to fix that first (otherwise they wouldn't appear in the bash cli completion file).
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.
4576b77
to
412bb2f
Compare
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, 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. |
- 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
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.
412bb2f
to
a081590
Compare
Updated branch to partially get rid of conflict with #17356 (some conflict might be left if git is not clever enough). Added a comment for #12674 : https://github.com/bitcoin/bitcoin/pull/12674/files#r408113386 |
🐙 This pull request conflicts with the target branch and needs rebase. |
@pierreN is this something you plan on continuing to work on? |
@pierreN ping for rebase. |
Sorry for the few months delay. |
There hasn't been much activity lately and the patch still needs rebase. What is the status here?
|
# If you want to modify this file, modify test/functional/tool_cli_completion.py and re-autogenerate | ||
# this file via the --overwrite test flag. | ||
|
||
# Copyright (c) 2012-2020 The Bitcoin Core developers |
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.
Happy New Year 🥂
# Copyright (c) 2012-2020 The Bitcoin Core developers | |
# Copyright (c) 2012-2022 The Bitcoin Core developers |
ACK, would be great to see this merged. I can see in https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#squashing-commits that the next task is primarily to amend the commit message after squashing, the message that'll make it into the Here's the starting point:
|
#25243 has picked this up. |
Adds a functional test that 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.The first commit syncs the auto-completion file with the current master.
The second commits adds the test. It just changes the header of the autocomplete file
and moves it to.test/functional/data/completion/bitcoin-cli.bash-completion
(plus for ease of use and backward compatibility a symlink fromcontrib/bitcoin-cli.bash-completion
to the moved file is created)This PR aims to fix: #17289