qa: Add getdescriptorinfo functional test #15443

pull promag wants to merge 1 commits into bitcoin:master from promag:2019-02-qa-feature-descriptor changing 2 files +66 −0

promag commented at 3:21 PM on February 19, 2019: member

The getdescriptorinfo RPC was added in #15368, this PR adds some tests.
promag commented at 3:23 PM on February 19, 2019: member

@sipa not sure if it makes sense to test these examples. Initially I was only testing the interface.
in test/functional/test_runner.py:193 in 11778c8fd5 outdated
```
 189 | @@ -190,6 +190,7 @@
 190 |      'rpc_help.py',
 191 |      'feature_help.py',
 192 |      'feature_shutdown.py',
 193 | +    'feature_descriptor.py',
```
MarcoFalke commented at 3:25 PM on February 19, 2019:

According to the naming guide, this should be rpc_descriptor?

promag commented at 3:30 PM on February 19, 2019:

Descriptor is not a RPC thing is it?

Sjors commented at 4:22 PM on February 19, 2019:

Or rpc_getdescriptorinfo

promag commented at 8:46 PM on July 1, 2019:

Ok, will rename to @Sjors suggestion.

MarcoFalke commented at 3:10 AM on March 10, 2020:

@promag Are you going to rename this or not. If not I will merge as is.

promag commented at 8:14 AM on March 10, 2020:

Renamed to rpc_getdescriptorinfo.py.
Sjors approved
Sjors commented at 4:33 PM on February 19, 2019: member
tACK 11778c on macOS 10.14.3.

We should distinguish two reasons for having test descriptors:
1. To test the command output (make sure no fields are missing, all error RPC error conditions are covered)
2. To test descsum_create i.e. the Python implementation of descriptor checksums.
(2) probably deserves a seperate test that is called by test_runner but otherwise doesn't need any of the framework. On the other hand, just adding it this test file seems fine too.

It could be useful to put a bunch of test descriptors in a single JSON file to reuse between tests and to make it easier for other projects to borrow.
fanquake added the label Tests on Feb 19, 2019
jonasschnelli commented at 5:55 AM on February 20, 2019: contributor

More tests are always good. Thanks for adding. Concept AcK
practicalswift commented at 7:18 PM on February 20, 2019: contributor

Concept ACK

Thanks for adding!
DrahtBot commented at 1:16 AM on June 9, 2019: member



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



Conflicts

No conflicts as of last run.
jachiang commented at 11:39 AM on July 1, 2019: contributor
@promag I looked into this PR in preparation for the PR Review Club IRC session later this week hosted by John and Harding and had a couple questions or thoughts:
1. I believe the RPC test does not cover a case where no checksum is appended to the output descriptor? Currently, test_desc will append a checksum to every descriptor literal. Could we extend test_desc parameters to allow for such a test: e.g. def test_desc(self, desc, *appendchecksum*, isrange, issolvable, hasprivatekeys): EDIT: I realize the test covers descriptors with and without checksum.
2. It would seem that we could cover a case with hasprivatekeys=True e.g. wpkh(WIF)#checksum or wsh(multi(1, XPRV/1/0/*, XPUB/0/0/*))#checksum
3. ~~How would a issolvable=False return differ from an Invalid descriptor error? I am sure there is a distinction which eludes me. In any case, would it make sense to cover an unsolvable descriptor?~~ EDIT: Perhaps add addr(ADDR)/raw(HEX) inputs for issolvable=False coverage?
Thanks for adding this test.
promag commented at 8:47 PM on July 1, 2019: member

@jachiang FYI these were taken from doc/descriptors.md.
meshcollider commented at 1:33 AM on January 17, 2020: contributor

utACK 11778c8fd5cc13fcbcbc746ea1ca7ddef13a7de3

As mentioned above, it would be good to include some tests where hasprivatekeys=True
DrahtBot closed this on Mar 9, 2020
DrahtBot commented at 8:34 PM on March 9, 2020: member

The last travis run for this pull request was 384 days ago and is thus outdated. To trigger a fresh travis build, this pull request should be closed and re-opened.
DrahtBot reopened this on Mar 9, 2020
promag force-pushed on Mar 10, 2020
qa: Add getdescriptorinfo functional test cbf2d75d8f
promag force-pushed on Mar 10, 2020
MarcoFalke merged this on Mar 10, 2020
MarcoFalke closed this on Mar 10, 2020
promag deleted the branch on Mar 10, 2020
Fabcien referenced this in commit a34d598eb4 on Jan 6, 2021
DrahtBot locked this on Feb 15, 2022