test: checks that bitcoin-cli autocomplete is in sync by pierreN · Pull Request #18606 · bitcoin/bitcoin

pierreN · 2020-04-12T14:52:07Z

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 from contrib/bitcoin-cli.bash-completion to the moved file is created).

This PR aims to fix: #17289

maflcko

Concept ACK

contrib/bitcoin-cli.bash-completion

pierreN · 2020-04-12T21:19:17Z

It seems that even when using SRCDIR CI builds fail: https://travis-ci.org/github/pierreN/bitcoin/jobs/674154306#L3417

A decent way to fix this would be to add the bash auto-completion file to Makefile.am's EXTRA_DIST. I updated the PR to reflect that.

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

maflcko · 2020-04-12T22:43:48Z

test/functional/tool_cli_completion.py

+        """ 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))


Suggested change

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

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

I hadn't notice #18531 when writing 412bb2f - I'll rebase this PR after #18531 is merged.

Makefile.am

DrahtBot · 2020-04-14T12:24:58Z

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

rpc: Assert that RPCArg names are equal to CRPCCommand ones (server) #19386 (rpc: Assert that RPCArg names are equal to CRPCCommand ones (server) by MarcoFalke)
rpc: Replace OMITTED_NAMED_ARG with OMITTED #19262 (rpc: Replace OMITTED_NAMED_ARG with OMITTED by MarcoFalke)
tests: Update more tests to work with descriptor wallets #18788 (tests: Update more tests to work with descriptor wallets by achow101)
Log RPC parameters (arguments) if -debug=rpcparams #16365 (Log RPC parameters (arguments) if -debug=rpcparams by LarryRuane)

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.

pierreN · 2020-04-14T12:57:15Z

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

DrahtBot · 2020-07-15T18:58:56Z

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

adamjonas · 2021-01-14T19:52:55Z

@pierreN is this something you plan on continuing to work on?

adamjonas · 2021-03-19T15:02:48Z

@pierreN ping for rebase.

pierreN · 2021-05-06T21:39:59Z

Sorry for the few months delay.
I have a bit more time now and will try to follow through with this PR.

DrahtBot · 2021-12-15T11:22:01Z

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.

niVelion · 2022-01-14T11:49:19Z

contrib/bitcoin-cli.bash-completion

+# 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


Happy New Year 🥂

Suggested change

# Copyright (c) 2012-2020 The Bitcoin Core developers

# Copyright (c) 2012-2022 The Bitcoin Core developers

niVelion · 2022-01-14T12:27:19Z

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 master branch to describe this improvement.

Here's the starting point:

# This is a combination of 4 commits.
# This is the 1st commit message:

rpc: make all unhidden actors use RPCHelpMan::Check

# This is the commit message #2:

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

# This is the commit message #3:

contrib: sync bitcoin-cli bash autocompletion

# This is the commit message #4:

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.

fanquake · 2022-05-30T09:37:12Z

#25243 has picked this up.

pierreN changed the title ~~test: check that bitcoin-cli autocomplete is in sync~~ test: checks that bitcoin-cli autocomplete is in sync Apr 12, 2020

pierreN mentioned this pull request Apr 12, 2020

contrib: Autogenerate bash completion #17289

Open

DrahtBot added Scripts and tools Tests labels Apr 12, 2020

maflcko approved these changes Apr 12, 2020

View reviewed changes

contrib/bitcoin-cli.bash-completion Outdated Show resolved Hide resolved

maflcko removed the Tests label Apr 12, 2020

pierreN force-pushed the feature-auto-cli branch 2 times, most recently from a332a11 to 4576b77 Compare April 12, 2020 21:16

maflcko reviewed Apr 12, 2020

View reviewed changes

Makefile.am Show resolved Hide resolved

rpc: make all unhidden actors use RPCHelpMan::Check

18602f1

pierreN force-pushed the feature-auto-cli branch from 4576b77 to 412bb2f Compare April 14, 2020 07:30

pierrenn added 3 commits April 14, 2020 21:44

contrib: sync bitcoin-cli bash autocompletion

e159c3e

pierreN force-pushed the feature-auto-cli branch from 412bb2f to a081590 Compare April 14, 2020 12:46

pierreN mentioned this pull request Apr 14, 2020

RPC: Support addnode onetry without making the connection priviliged #12674

Closed

This was referenced Apr 14, 2020

RPC: Internal named params #17356

Closed

rpc: remove g_rpc_node & g_rpc_chain #18647

Closed

rpc: remove deprecated CRPCCommand constructor #18531

Merged

DrahtBot mentioned this pull request Apr 27, 2020

tests: Update more tests to work with descriptor wallets #18788

Merged

DrahtBot mentioned this pull request May 5, 2020

rpc: prevent circular deps by extracting into separate include #18871

Closed

DrahtBot mentioned this pull request Jun 13, 2020

rpc: Replace OMITTED_NAMED_ARG with OMITTED #19262

Closed

DrahtBot mentioned this pull request Jun 26, 2020

rpc: Assert that RPCArg names are equal to CRPCCommand ones (server) #19386

Merged

DrahtBot mentioned this pull request Jul 13, 2020

Log RPC parameters (arguments) if -debug=rpcparams #16365

Closed

DrahtBot added the Needs rebase label Jul 15, 2020

niVelion reviewed Jan 14, 2022

View reviewed changes

fanquake added the Up for grabs label Apr 26, 2022

fanquake closed this Apr 26, 2022

fanquake removed the Up for grabs label May 30, 2022

fanquake mentioned this pull request May 30, 2022

test: autogenerate bash completion #25243

Closed

bitcoin locked and limited conversation to collaborators May 30, 2023

	regexp_res = ARG_REGEXP.match(self.nodes[0].help(option.command))
	res = self.nodes[0].format(command=option.command, output=args_cli)

	# Copyright (c) 2012-2020 The Bitcoin Core developers
	# Copyright (c) 2012-2022 The Bitcoin Core developers

Conversation

pierreN commented Apr 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maflcko left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pierreN commented Apr 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maflcko Apr 12, 2020

Choose a reason for hiding this comment

Uh oh!

pierreN Apr 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pierreN Apr 18, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DrahtBot commented Apr 14, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

pierreN commented Apr 14, 2020

Uh oh!

DrahtBot commented Jul 15, 2020

Uh oh!

adamjonas commented Jan 14, 2021

Uh oh!

adamjonas commented Mar 19, 2021

Uh oh!

pierreN commented May 6, 2021

Uh oh!

DrahtBot commented Dec 15, 2021

Uh oh!

niVelion Jan 14, 2022

Choose a reason for hiding this comment

Uh oh!

niVelion commented Jan 14, 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

pierreN commented Apr 12, 2020 •

edited

Loading

pierreN commented Apr 12, 2020 •

edited

Loading

pierreN Apr 14, 2020 •

edited

Loading

DrahtBot commented Apr 14, 2020 •

edited

Loading

niVelion commented Jan 14, 2022 •

edited

Loading