test: autogenerate bash completion #25243

suhailsaqan · 2022-05-30T09:33:05Z

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

fanquake · 2022-05-30T09:47:27Z

Thanks for picking this up. Did you re-use any code from #18606? A lot of the changes are very similar to the changes there, except for different whitespace, class names, copyright dates etc. If so, cherry-picking so that the original author is preserved is appropriate.

laanwj · 2022-05-30T12:59:08Z

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.

suhailsaqan · 2022-05-30T13:48:36Z

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 (comment)

maflcko · 2022-05-30T14:01:37Z

Yeah, I wrote that comment before bitcoin.conf moved over to auto-generation. A functional test is one way to achieve this, but this can also be achieved by an auto-generation script at release time.

laanwj · 2022-05-30T15:59:54Z

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)

brunoerg · 2022-05-30T17:40:09Z

From linter:

File "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"

suhailsaqan · 2022-05-31T00:25:08Z

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.

suhailsaqan · 2022-06-03T05:03:19Z

Squashed commits.

laanwj · 2022-06-07T13:06:23Z

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.

luke-jr · 2022-06-18T00:56:48Z

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?).

DrahtBot · 2022-06-21T01:39:20Z

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

Reviews

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.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#27788 (rpc: Be able to access RPC parameters by name by achow101)
#27432 (contrib: add tool to convert compact-serialized UTXO set to SQLite database by theStack)
#26485 (RPC: Accept options as named-only parameters by ryanofsky)

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.

hernanmarino

Hi everyone, I have a questions / suggestion:
I believe the object of this issue was to keep the bash files up to date, but this PR only adds a test to check for the file, or optionally update it. Is there something that can be done to make sure that the file is updated or generated on every compile, perhaps by modifying the Makefile to run this test everytime? In that case, I believe that the "--overwrite" option should be the default.

src/rpc/net.cpp

achow101 · 2022-10-12T19:24:39Z

Closed and reopened to hopefully trigger CI

suhailsaqan · 2022-10-12T21:41:20Z

@achow101 Any idea why CI is not running on this PR?

fanquake · 2022-10-13T02:58:52Z

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.

maflcko · 2022-10-13T08:43:45Z

Triggered CI

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

DrahtBot · 2023-06-01T20:10:40Z

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

DrahtBot · 2023-08-30T01:45:10Z

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.

achow101 · 2023-09-23T16:56:03Z

Are you still working on this?

DrahtBot · 2023-12-22T00:57:32Z

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.

fanquake mentioned this pull request May 30, 2022

test: checks that bitcoin-cli autocomplete is in sync #18606

Closed

fanquake linked an issue May 30, 2022 that may be closed by this pull request

contrib: Autogenerate bash completion #17289

Open

suhailsaqan force-pushed the bash-completion branch from 1e67d08 to 2c10b6d Compare May 30, 2022 10:20

DrahtBot added RPC/REST/ZMQ Scripts and tools labels May 30, 2022

suhailsaqan force-pushed the bash-completion branch from 380c7f4 to d0c2140 Compare May 30, 2022 12:54

suhailsaqan force-pushed the bash-completion branch 2 times, most recently from d79fe4a to 784c4e5 Compare May 30, 2022 13:38

suhailsaqan force-pushed the bash-completion branch 3 times, most recently from c97d573 to a8e6ab5 Compare June 3, 2022 05:01

suhailsaqan force-pushed the bash-completion branch 2 times, most recently from f9d7f57 to 5f3ba0e Compare June 3, 2022 07:00

hernanmarino reviewed Aug 12, 2022

View reviewed changes

pablomartin4btc reviewed Aug 12, 2022

View reviewed changes

src/rpc/net.cpp Outdated Show resolved Hide resolved

achow101 closed this Oct 12, 2022

achow101 reopened this Oct 12, 2022

suhailsaqan force-pushed the bash-completion branch from 54d06dd to 0cf4fd4 Compare October 13, 2022 12:45

DrahtBot mentioned this pull request Nov 12, 2022

RPC: Accept options as named-only parameters #26485

Merged

DrahtBot added the Needs rebase label Dec 8, 2022

fanquake marked this pull request as draft February 15, 2023 10:37

pierrenn and others added 3 commits February 18, 2023 11:18

added format to RPC_COMMANDS_NOT_SAFE_FOR_FUZZING

f8858f7

suhailsaqan force-pushed the bash-completion branch from 5d7224d to f8858f7 Compare February 18, 2023 19:21

remove unnecessary changes

3f06340

DrahtBot removed the Needs rebase label Feb 18, 2023

DrahtBot mentioned this pull request Apr 6, 2023

contrib: add tool to convert compact-serialized UTXO set to SQLite database #27432

Merged

DrahtBot mentioned this pull request May 31, 2023

rpc: Be able to access RPC parameters by name #27788

Closed

DrahtBot added the Needs rebase label Jun 1, 2023

maflcko closed this Dec 22, 2023

bitcoin locked and limited conversation to collaborators Dec 21, 2024

test: autogenerate bash completion #25243

test: autogenerate bash completion #25243

Uh oh!

Conversation

suhailsaqan commented May 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fanquake commented May 30, 2022

Uh oh!

laanwj commented May 30, 2022

Uh oh!

suhailsaqan commented May 30, 2022

Uh oh!

maflcko commented May 30, 2022

Uh oh!

laanwj commented May 30, 2022

Uh oh!

brunoerg commented May 30, 2022

Uh oh!

suhailsaqan commented May 31, 2022

Uh oh!

suhailsaqan commented Jun 3, 2022

Uh oh!

laanwj commented Jun 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

luke-jr commented Jun 18, 2022

Uh oh!

DrahtBot commented Jun 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews

Conflicts

Uh oh!

hernanmarino left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

achow101 commented Oct 12, 2022

Uh oh!

suhailsaqan commented Oct 12, 2022

Uh oh!

fanquake commented Oct 13, 2022

Uh oh!

maflcko commented Oct 13, 2022

Uh oh!

DrahtBot commented Jun 1, 2023

Uh oh!

DrahtBot commented Aug 30, 2023

Uh oh!

achow101 commented Sep 23, 2023

Uh oh!

DrahtBot commented Dec 22, 2023

Uh oh!

Uh oh!

suhailsaqan commented May 30, 2022 •

edited

Loading

laanwj commented Jun 7, 2022 •

edited

Loading

DrahtBot commented Jun 21, 2022 •

edited

Loading