Skip to content

Commit b7450cd

Browse files
author
MarcoFalke
committed
Merge #11970: Add test coverage for bitcoin-cli multiwallet calls
a14dbff Allow multiwallet.py to be used with --usecli (Russell Yanofsky) f6ade9c [tests] allow tests to be run with --usecli (John Newbery) ff9a363 TestNodeCLI batch emulation (Russell Yanofsky) ca9085a Prevent TestNodeCLI.args mixups (Russell Yanofsky) fcfb952 Improve TestNodeCLI output parsing (Russell Yanofsky) Pull request description: Lack of test coverage was pointed out by @jnewbery in bitcoin/bitcoin#11687 (comment) Tree-SHA512: 5f10e31abad11a5edab0da4e2515e39547adb6ab9e55e50427ab2eb7ec9a43d6b896b579b15863e5edc9beee7d8bf1c84d9dabd247be0760a1b9ae39e1e8ee02
2 parents 0910cbe + a14dbff commit b7450cd

File tree

5 files changed

+88
-37
lines changed

5 files changed

+88
-37
lines changed

test/functional/create_cache.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ class CreateCache(BitcoinTestFramework):
1616

1717
def set_test_params(self):
1818
self.num_nodes = 0
19+
self.supports_cli = True
1920

2021
def setup_network(self):
2122
pass

test/functional/multiwallet.py

Lines changed: 27 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -17,69 +17,75 @@ def set_test_params(self):
1717
self.setup_clean_chain = True
1818
self.num_nodes = 1
1919
self.extra_args = [['-wallet=w1', '-wallet=w2', '-wallet=w3', '-wallet=w']]
20+
self.supports_cli = True
2021

2122
def run_test(self):
22-
assert_equal(set(self.nodes[0].listwallets()), {"w1", "w2", "w3", "w"})
23+
node = self.nodes[0]
24+
25+
data_dir = lambda *p: os.path.join(node.datadir, 'regtest', *p)
26+
wallet_dir = lambda *p: data_dir('wallets', *p)
27+
wallet = lambda name: node.get_wallet_rpc(name)
28+
29+
assert_equal(set(node.listwallets()), {"w1", "w2", "w3", "w"})
2330

2431
self.stop_node(0)
2532

2633
# should not initialize if there are duplicate wallets
2734
self.assert_start_raises_init_error(0, ['-wallet=w1', '-wallet=w1'], 'Error loading wallet w1. Duplicate -wallet filename specified.')
2835

2936
# should not initialize if wallet file is a directory
30-
wallet_dir = os.path.join(self.options.tmpdir, 'node0', 'regtest', 'wallets')
31-
os.mkdir(os.path.join(wallet_dir, 'w11'))
37+
os.mkdir(wallet_dir('w11'))
3238
self.assert_start_raises_init_error(0, ['-wallet=w11'], 'Error loading wallet w11. -wallet filename must be a regular file.')
3339

3440
# should not initialize if one wallet is a copy of another
35-
shutil.copyfile(os.path.join(wallet_dir, 'w2'), os.path.join(wallet_dir, 'w22'))
41+
shutil.copyfile(wallet_dir('w2'), wallet_dir('w22'))
3642
self.assert_start_raises_init_error(0, ['-wallet=w2', '-wallet=w22'], 'duplicates fileid')
3743

3844
# should not initialize if wallet file is a symlink
39-
os.symlink(os.path.join(wallet_dir, 'w1'), os.path.join(wallet_dir, 'w12'))
45+
os.symlink(wallet_dir('w1'), wallet_dir('w12'))
4046
self.assert_start_raises_init_error(0, ['-wallet=w12'], 'Error loading wallet w12. -wallet filename must be a regular file.')
4147

4248
# should not initialize if the specified walletdir does not exist
4349
self.assert_start_raises_init_error(0, ['-walletdir=bad'], 'Error: Specified -walletdir "bad" does not exist')
4450
# should not initialize if the specified walletdir is not a directory
45-
not_a_dir = os.path.join(wallet_dir, 'notadir')
51+
not_a_dir = wallet_dir('notadir')
4652
open(not_a_dir, 'a').close()
47-
self.assert_start_raises_init_error(0, ['-walletdir='+not_a_dir], 'Error: Specified -walletdir "' + not_a_dir + '" is not a directory')
53+
self.assert_start_raises_init_error(0, ['-walletdir=' + not_a_dir], 'Error: Specified -walletdir "' + not_a_dir + '" is not a directory')
4854

4955
# if wallets/ doesn't exist, datadir should be the default wallet dir
50-
wallet_dir2 = os.path.join(self.options.tmpdir, 'node0', 'regtest', 'walletdir')
51-
os.rename(wallet_dir, wallet_dir2)
56+
wallet_dir2 = data_dir('walletdir')
57+
os.rename(wallet_dir(), wallet_dir2)
5258
self.start_node(0, ['-wallet=w4', '-wallet=w5'])
53-
assert_equal(set(self.nodes[0].listwallets()), {"w4", "w5"})
54-
w5 = self.nodes[0].get_wallet_rpc("w5")
59+
assert_equal(set(node.listwallets()), {"w4", "w5"})
60+
w5 = wallet("w5")
5561
w5.generate(1)
5662
self.stop_node(0)
5763

5864
# now if wallets/ exists again, but the rootdir is specified as the walletdir, w4 and w5 should still be loaded
59-
os.rename(wallet_dir2, wallet_dir)
60-
self.start_node(0, ['-wallet=w4', '-wallet=w5', '-walletdir=' + os.path.join(self.options.tmpdir, 'node0', 'regtest')])
61-
assert_equal(set(self.nodes[0].listwallets()), {"w4", "w5"})
62-
w5 = self.nodes[0].get_wallet_rpc("w5")
65+
os.rename(wallet_dir2, wallet_dir())
66+
self.start_node(0, ['-wallet=w4', '-wallet=w5', '-walletdir=' + data_dir()])
67+
assert_equal(set(node.listwallets()), {"w4", "w5"})
68+
w5 = wallet("w5")
6369
w5_info = w5.getwalletinfo()
6470
assert_equal(w5_info['immature_balance'], 50)
6571

6672
self.stop_node(0)
6773

6874
self.start_node(0, self.extra_args[0])
6975

70-
w1 = self.nodes[0].get_wallet_rpc("w1")
71-
w2 = self.nodes[0].get_wallet_rpc("w2")
72-
w3 = self.nodes[0].get_wallet_rpc("w3")
73-
w4 = self.nodes[0].get_wallet_rpc("w")
74-
wallet_bad = self.nodes[0].get_wallet_rpc("bad")
76+
w1 = wallet("w1")
77+
w2 = wallet("w2")
78+
w3 = wallet("w3")
79+
w4 = wallet("w")
80+
wallet_bad = wallet("bad")
7581

7682
w1.generate(1)
7783

7884
# accessing invalid wallet fails
7985
assert_raises_rpc_error(-18, "Requested wallet does not exist or is not loaded", wallet_bad.getwalletinfo)
8086

8187
# accessing wallet RPC without using wallet endpoint fails
82-
assert_raises_rpc_error(-19, "Wallet file not specified", self.nodes[0].getwalletinfo)
88+
assert_raises_rpc_error(-19, "Wallet file not specified", node.getwalletinfo)
8389

8490
# check w1 wallet balance
8591
w1_info = w1.getwalletinfo()

test/functional/test_framework/test_framework.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ def __init__(self):
6262
self.setup_clean_chain = False
6363
self.nodes = []
6464
self.mocktime = 0
65+
self.supports_cli = False
6566
self.set_test_params()
6667

6768
assert hasattr(self, "num_nodes"), "Test must set self.num_nodes in set_test_params()"
@@ -91,6 +92,8 @@ def main(self):
9192
help="Location of the test framework config file")
9293
parser.add_option("--pdbonfailure", dest="pdbonfailure", default=False, action="store_true",
9394
help="Attach a python debugger if test fails")
95+
parser.add_option("--usecli", dest="usecli", default=False, action="store_true",
96+
help="use bitcoin-cli instead of RPC for all commands")
9497
self.add_options(parser)
9598
(self.options, self.args) = parser.parse_args()
9699

@@ -113,6 +116,8 @@ def main(self):
113116
success = TestStatus.FAILED
114117

115118
try:
119+
if self.options.usecli and not self.supports_cli:
120+
raise SkipTest("--usecli specified but test does not support using CLI")
116121
self.setup_chain()
117122
self.setup_network()
118123
self.run_test()
@@ -213,7 +218,7 @@ def add_nodes(self, num_nodes, extra_args=None, rpchost=None, timewait=None, bin
213218
assert_equal(len(extra_args), num_nodes)
214219
assert_equal(len(binary), num_nodes)
215220
for i in range(num_nodes):
216-
self.nodes.append(TestNode(i, self.options.tmpdir, extra_args[i], rpchost, timewait=timewait, binary=binary[i], stderr=None, mocktime=self.mocktime, coverage_dir=self.options.coveragedir))
221+
self.nodes.append(TestNode(i, self.options.tmpdir, extra_args[i], rpchost, timewait=timewait, binary=binary[i], stderr=None, mocktime=self.mocktime, coverage_dir=self.options.coveragedir, use_cli=self.options.usecli))
217222

218223
def start_node(self, i, extra_args=None, stderr=None):
219224
"""Start a bitcoind"""

test/functional/test_framework/test_node.py

Lines changed: 53 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import json
1111
import logging
1212
import os
13+
import re
1314
import subprocess
1415
import time
1516

@@ -22,6 +23,9 @@
2223
p2p_port,
2324
)
2425

26+
# For Python 3.4 compatibility
27+
JSONDecodeError = getattr(json, "JSONDecodeError", ValueError)
28+
2529
BITCOIND_PROC_WAIT_TIMEOUT = 60
2630

2731
class TestNode():
@@ -38,7 +42,7 @@ class TestNode():
3842
To make things easier for the test writer, any unrecognised messages will
3943
be dispatched to the RPC connection."""
4044

41-
def __init__(self, i, dirname, extra_args, rpchost, timewait, binary, stderr, mocktime, coverage_dir):
45+
def __init__(self, i, dirname, extra_args, rpchost, timewait, binary, stderr, mocktime, coverage_dir, use_cli=False):
4246
self.index = i
4347
self.datadir = os.path.join(dirname, "node" + str(i))
4448
self.rpchost = rpchost
@@ -58,6 +62,7 @@ def __init__(self, i, dirname, extra_args, rpchost, timewait, binary, stderr, mo
5862
self.args = [self.binary, "-datadir=" + self.datadir, "-server", "-keypool=1", "-discover=0", "-rest", "-logtimemicros", "-debug", "-debugexclude=libevent", "-debugexclude=leveldb", "-mocktime=" + str(mocktime), "-uacomment=testnode%d" % i]
5963

6064
self.cli = TestNodeCLI(os.getenv("BITCOINCLI", "bitcoin-cli"), self.datadir)
65+
self.use_cli = use_cli
6166

6267
self.running = False
6368
self.process = None
@@ -69,9 +74,12 @@ def __init__(self, i, dirname, extra_args, rpchost, timewait, binary, stderr, mo
6974
self.p2ps = []
7075

7176
def __getattr__(self, name):
72-
"""Dispatches any unrecognised messages to the RPC connection."""
73-
assert self.rpc_connected and self.rpc is not None, "Error: no RPC connection"
74-
return getattr(self.rpc, name)
77+
"""Dispatches any unrecognised messages to the RPC connection or a CLI instance."""
78+
if self.use_cli:
79+
return getattr(self.cli, name)
80+
else:
81+
assert self.rpc_connected and self.rpc is not None, "Error: no RPC connection"
82+
return getattr(self.rpc, name)
7583

7684
def start(self, extra_args=None, stderr=None):
7785
"""Start the node."""
@@ -110,10 +118,13 @@ def wait_for_rpc_connection(self):
110118
raise AssertionError("Unable to connect to bitcoind")
111119

112120
def get_wallet_rpc(self, wallet_name):
113-
assert self.rpc_connected
114-
assert self.rpc
115-
wallet_path = "wallet/%s" % wallet_name
116-
return self.rpc / wallet_path
121+
if self.use_cli:
122+
return self.cli("-rpcwallet={}".format(wallet_name))
123+
else:
124+
assert self.rpc_connected
125+
assert self.rpc
126+
wallet_path = "wallet/%s" % wallet_name
127+
return self.rpc / wallet_path
117128

118129
def stop_node(self):
119130
"""Stop the node."""
@@ -187,6 +198,16 @@ def disconnect_p2ps(self):
187198
p.peer_disconnect()
188199
del self.p2ps[:]
189200

201+
class TestNodeCLIAttr:
202+
def __init__(self, cli, command):
203+
self.cli = cli
204+
self.command = command
205+
206+
def __call__(self, *args, **kwargs):
207+
return self.cli.send_cli(self.command, *args, **kwargs)
208+
209+
def get_request(self, *args, **kwargs):
210+
return lambda: self(*args, **kwargs)
190211

191212
class TestNodeCLI():
192213
"""Interface to bitcoin-cli for an individual node"""
@@ -196,17 +217,26 @@ def __init__(self, binary, datadir):
196217
self.binary = binary
197218
self.datadir = datadir
198219
self.input = None
220+
self.log = logging.getLogger('TestFramework.bitcoincli')
199221

200222
def __call__(self, *args, input=None):
201223
# TestNodeCLI is callable with bitcoin-cli command-line args
202-
self.args = [str(arg) for arg in args]
203-
self.input = input
204-
return self
224+
cli = TestNodeCLI(self.binary, self.datadir)
225+
cli.args = [str(arg) for arg in args]
226+
cli.input = input
227+
return cli
205228

206229
def __getattr__(self, command):
207-
def dispatcher(*args, **kwargs):
208-
return self.send_cli(command, *args, **kwargs)
209-
return dispatcher
230+
return TestNodeCLIAttr(self, command)
231+
232+
def batch(self, requests):
233+
results = []
234+
for request in requests:
235+
try:
236+
results.append(dict(result=request()))
237+
except JSONRPCException as e:
238+
results.append(dict(error=e))
239+
return results
210240

211241
def send_cli(self, command, *args, **kwargs):
212242
"""Run bitcoin-cli command. Deserializes returned string as python object."""
@@ -218,10 +248,18 @@ def send_cli(self, command, *args, **kwargs):
218248
if named_args:
219249
p_args += ["-named"]
220250
p_args += [command] + pos_args + named_args
251+
self.log.debug("Running bitcoin-cli command: %s" % command)
221252
process = subprocess.Popen(p_args, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True)
222253
cli_stdout, cli_stderr = process.communicate(input=self.input)
223254
returncode = process.poll()
224255
if returncode:
256+
match = re.match(r'error code: ([-0-9]+)\nerror message:\n(.*)', cli_stderr)
257+
if match:
258+
code, message = match.groups()
259+
raise JSONRPCException(dict(code=int(code), message=message))
225260
# Ignore cli_stdout, raise with cli_stderr
226261
raise subprocess.CalledProcessError(returncode, self.binary, output=cli_stderr)
227-
return json.loads(cli_stdout, parse_float=decimal.Decimal)
262+
try:
263+
return json.loads(cli_stdout, parse_float=decimal.Decimal)
264+
except JSONDecodeError:
265+
return cli_stdout.rstrip("\n")

test/functional/test_runner.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@
9494
'mempool_reorg.py',
9595
'mempool_persist.py',
9696
'multiwallet.py',
97+
'multiwallet.py --usecli',
9798
'httpbasics.py',
9899
'multi_rpc.py',
99100
'proxy_test.py',

0 commit comments

Comments
 (0)