test: add coverage for bitcoin-cli -rpcwait #18653

pull jonatack wants to merge 4 commits into bitcoin:master from jonatack:rpcwait-test-coverage changing 1 files +25 −15

jonatack commented at 3:07 PM on April 15, 2020: member

and other coverage improvements in interface-bitcoin_cli.py
test: add coverage for bitcoin-cli -rpcwait
in interface_bitcoin_cli.py
727b67e7b8

test: verify bitcoin-cli -version with node stopped

in interface_bitcoin_cli.py and improve/harmonize the test logging.

becc8b9747

test: verify cli.getwalletinfo in wallet section bb13f46fb1
test: add -getinfo "unlocked_until" and "headers" coverage c28c7b882b
in test/functional/interface_bitcoin_cli.py:89 in c28c7b882b
```
  90 | +
  91 | +        self.log.info("Test -rpcwait option waits for RPC connection instead of failing")
  92 | +        # Start node without RPC connection.
  93 | +        self.nodes[0].start()
  94 | +        # Verify failure without -rpcwait.
  95 | +        assert_raises_process_error(1, "Could not connect to the server", self.nodes[0].cli('getblockcount').echo)
```
MarcoFalke commented at 3:11 PM on April 15, 2020:

Isn't this racy?

jonatack commented at 3:20 PM on April 15, 2020:

Running (for i in {1..1000}; do test/functional/interface_bitcoin_cli.py ; done) to see.

MarcoFalke commented at 3:38 PM on April 15, 2020:

We generally only see races in travis or valgrind

Logically, I don't see how the rpc connection is guaranteed to be down when the cli is called here.

jonatack commented at 4:06 PM on April 15, 2020:

Makes sense: it ran 1000 times without fail for me in macOS and Debian, but yeah, travis and valgrind. I'll wait to see the travis result for the fun of it and then probably remove that line; we'll still have the assertion with -rpcwait

jonatack commented at 7:06 PM on April 15, 2020:

Travis all green. Maybe leave it in and remove if we see issues later?
DrahtBot added the label Tests on Apr 15, 2020
in test/functional/interface_bitcoin_cli.py:36 in c28c7b882b
```
  31 | -        self.log.info("Compare responses from getwalletinfo RPC and `bitcoin-cli getwalletinfo`")
  32 | -        if self.is_wallet_compiled():
  33 | -            cli_response = self.nodes[0].cli.getwalletinfo()
  34 | -            rpc_response = self.nodes[0].getwalletinfo()
  35 | -            assert_equal(cli_response, rpc_response)
  36 | -
```
kristapsk commented at 3:48 PM on April 15, 2020:

Why this test was removed?

jonatack commented at 3:56 PM on April 15, 2020:

Not removed; moved to line 75 assert_equal(self.nodes[0].cli.getwalletinfo(), wallet_info)

jonatack commented at 3:59 PM on April 15, 2020:

which avoids calling rpc getwalletinfo twice, a conditional, and a few lines of code.

kristapsk commented at 4:06 PM on April 15, 2020:

Ok, yes, I didn't read the whole block correctly, assumed it checks -version, which is also checked below, just that it outputs, not what it outputs. And it's bitcoin-cli version anyways, can differ from version returned by -getinfo, nothing to compare against there.

jonatack commented at 9:56 AM on April 16, 2020:

@kristapsk thanks for reviewing! Can you ACK?
DrahtBot commented at 11:58 PM on April 15, 2020: member


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



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #18594 (cli: display multiwallet balances in -getinfo by jonatack)
- #18453 (rpc, cli: add multiwallet balances rpc and use it in -getinfo by jonatack)
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.
kristapsk approved
kristapsk commented at 12:00 PM on April 16, 2020: contributor

ACK c28c7b882b40cc75989dd102414de3b3dad69706
MarcoFalke merged this on Apr 16, 2020
MarcoFalke closed this on Apr 16, 2020
jonatack deleted the branch on Apr 16, 2020
sidhujag referenced this in commit 3b91de813b on Apr 16, 2020
luke-jr referenced this in commit ddd0278647 on Jun 14, 2020
luke-jr referenced this in commit 525c6d205b on Jun 14, 2020
luke-jr referenced this in commit e9a3e32b9d on Jun 14, 2020
luke-jr referenced this in commit 0b57c13d33 on Jun 14, 2020
Fabcien referenced this in commit b010f23ba6 on Jan 15, 2021
DrahtBot locked this on Feb 15, 2022

Contributors

Labels

test: add coverage for bitcoin-cli -rpcwait #18653

Conflicts