Add tests for RPC help

promag commented at 0:41 am on August 22, 2018: member

At the moment the new test checks for:

invalid usages
expected output for unknown command
current RPC command titles (derived from command categories) — this prevents adding wrong RPC categories and new categories must be added to the test

promag commented at 0:41 am on August 22, 2018: member

Follow up of #13945 (comment)

fanquake added the label Tests on Aug 22, 2018

isghe commented at 1:01 am on August 22, 2018: contributor

missing hidden category

isghe commented at 1:04 am on August 22, 2018: contributor

missing test category (in Qt)

promag commented at 1:07 am on August 22, 2018: member

missing hidden category

It’s hidden to the RPC client, and has special treatment in https://github.com/bitcoin/bitcoin/blob/2df11322faae555bd963a7610102b61049168333/src/rpc/server.cpp#L172

isghe commented at 1:16 am on August 22, 2018: contributor

How can be sure, external scripts analysing C/C++ Bitcoin source code, are immune to injection?

isghe commented at 1:18 am on August 22, 2018: contributor

I would prefer a C/C++ compile time security check, not an external script security check.

promag commented at 1:23 am on August 22, 2018: member

missing test category (in Qt)

I’m not following, in Qt?

How can be sure, external scripts analysing C/C++ Bitcoin source code, are immune to injection?

What is subject to injection here?

I would prefer a C/C++ compile time security check, not an external script security check.

There is still room to that, this test file can check for other stuff, like output is sorted, formatting.

isghe commented at 1:24 am on August 22, 2018: contributor

worst, I understood only now: it looks it is working on ./bitcoin-cli help output!

isghe commented at 1:35 am on August 22, 2018: contributor

I’m not following, in Qt?

I found this (as I wrote here #13945 in the p.s: anyway I was able to catch the QT command rpcNestedTest.)

in file src/qt/test/rpcnestedtests.cpp line 32:

0static const CRPCCommand vRPCCommands[] =
1{
2    { "test", "rpcNestedTest", &rpcNestedTest_rpc, {} },
3};

Anyway take care, I always configure --without-gui, I usually (read it: ALWAYS) don’t take care about QT.

promag commented at 1:37 am on August 22, 2018: member

Oh, that doesn’t matter IMO.

l2a5b1 commented at 12:04 pm on August 22, 2018: contributor

Thanks @promag!

Would it be an idea to assert that the actual titles are valid (i.e. not map the titles back to categories)?

I think this is more in spirit of a functional test and the test can then also assert that there are no true negatives (for example: "== Hidden ==", "== hidden ==", "== blockchain ==","== ==").

promag commented at 12:09 pm on August 22, 2018: member

@251Labs I’ll update.

test: Add tests for RPC help 6af6d9b23d

promag force-pushed on Aug 23, 2018

promag renamed this:
~~Assert RPC command categories~~
Add tests for RPC help
on Aug 23, 2018

promag commented at 0:42 am on August 23, 2018: member

@251Labs updated.

l2a5b1 commented at 2:27 pm on August 24, 2018: contributor

utACK 6af6d9b

Thanks @promag! Maybe a small nit would be to assert against the full category title to catch potentially missing title suffixes (==). Other than this nit LGTM.

isghe commented at 1:04 am on August 25, 2018: contributor

I am not an expert about functional test, and their implementations; but I like to learn new things, that’s why I am asking (maybe stupid) question.

In this kind of functional and deterministic test, wouldn’t be easier simply to check the sha256 output?

0$ ./src/bitcoin-cli help | sha256
12ae3a94e05bbb98323f72f19272bde01f075db16c7e4357bc46815a1393cd44a

thanks :-)

in test/functional/test_runner.py:156 in 6af6d9b23d

151@@ -152,6 +152,7 @@
152     'p2p_node_network_limited.py',
153     'feature_blocksdir.py',
154     'feature_config_args.py',
155+    'rpc_help.py',
156     'feature_help.py',

MarcoFalke commented at 1:55 pm on August 25, 2018:

Imo this could be combined into the feature_help test. Just adding a new function there def rpc_help(self): ... and moving the other code into def cli_help(self): ....

laanwj commented at 12:24 pm on August 29, 2018:

hmm not sure about this, my first intuition was to agree with you but now I think RPC help and options help are sufficiently different concepts (and in code) to have as separate tests But I’ll leave it to @promag

promag commented at 12:49 pm on August 29, 2018: member

I tend to prefer like this because

follows current convention
more little test files is more “readable” than less bigger test files
more files allows parallelization

MarcoFalke merged this on Aug 29, 2018

MarcoFalke closed this on Aug 29, 2018

MarcoFalke referenced this in commit 1361f8babc on Aug 29, 2018

in test/functional/rpc_help.py:28 in 6af6d9b23d

23+        # help of unknown command
24+        assert_equal(node.help('foo'), 'help: unknown command: foo')
25+
26+        # command titles
27+        titles = [line[3:-3] for line in node.help().splitlines() if line.startswith('==')]
28+        assert_equal(titles, ['Blockchain', 'Control', 'Generating', 'Mining', 'Network', 'Rawtransactions', 'Util', 'Wallet', 'Zmq'])

MarcoFalke commented at 11:58 am on August 30, 2018:

zmq is only available when installed. Should check for that like the other zmq tests?

promag deleted the branch on Sep 2, 2018

MarcoFalke referenced this in commit 1d7a2d4d15 on Oct 4, 2018

MarcoFalke referenced this in commit 24d796a6cc on Oct 25, 2018

toxeus referenced this in commit e07bb9fa85 on Nov 28, 2018

PastaPastaPasta referenced this in commit 4bf2569853 on Jun 27, 2021

PastaPastaPasta referenced this in commit 442bd4260c on Jun 28, 2021

PastaPastaPasta referenced this in commit c86620ffbc on Jun 29, 2021

PastaPastaPasta referenced this in commit eeacad713e on Jun 29, 2021

PastaPastaPasta referenced this in commit c41913bcb3 on Jun 29, 2021

PastaPastaPasta referenced this in commit 22e1c47ac9 on Jun 29, 2021

PastaPastaPasta referenced this in commit 844c360b2c on Jun 29, 2021

Munkybooty referenced this in commit f3685d32d7 on Jun 30, 2021

PastaPastaPasta referenced this in commit 12adb05b12 on Jul 1, 2021

MarcoFalke locked this on Sep 8, 2021

Add tests for RPC help #14020