Merge bitcoin/bitcoin#35363: test: Allow --usecli in more tests

fa24693819 test: Allow --usecli in tests that already support it (MarcoFalke)
fa8d4d5c35 test: Catch CalledProcessError to support --usecli in feature_dbcrash.py (MarcoFalke)
faf0f848ef test: use echojson to allow rpc_named_arguments.py --usecli (MarcoFalke)
faf993ee44 test: Stop node before modifying config to support rpc_users.py --usecli (MarcoFalke)
fa4fc8c1d7 test: Set TestNode url field early, so that feature_loadblock.py --usecli works (MarcoFalke)

Pull request description:

  Some tests disallow to be run under `--usecli`. This reduces the coverage and risks that bugs in the bitcoin-cli go unnoticed.

  The commits should be self-explanatory and can be reviewed and tested one-by-one.

ACKs for top commit:
  willcl-ark:
    ACK fa24693819

Tree-SHA512: e34077be98f88ad1e8649600a5f43fc8c77e4ebb03bbccd88c33f2d67882ccdd52b5d18bcfbfc611dff3ebf7455f8e624a88d062aa1863c5eb813bbf4f48e58b

test/functional/feature_coinstatsindex_compatibility.py

@@ -16,7 +16,6 @@ from test_framework.util import assert_equal
 class CoinStatsIndexTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 2
-        self.supports_cli = False
         self.extra_args = [["-coinstatsindex"],["-coinstatsindex"]]
 
     def skip_test_if_missing_module(self):

test/functional/feature_dbcrash.py

@@ -28,6 +28,7 @@
 import errno
 import http.client
 import random
+import subprocess
 import time
 
 from test_framework.blocktools import COINBASE_MATURITY
@@ -49,7 +50,6 @@ class ChainstateWriteCrashTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 4
         self.rpc_timeout = 480
-        self.supports_cli = False
 
         # Set -maxmempool=0 to turn off mempool memory sharing with dbcache
         self.base_args = [
@@ -113,6 +113,9 @@ class ChainstateWriteCrashTest(BitcoinTestFramework):
         except (http.client.CannotSendRequest, http.client.RemoteDisconnected) as e:
             self.log.debug(f"node {node_index} submitblock raised exception: {e}")
             return False
+        except subprocess.CalledProcessError as e:
+            self.log.debug(f"node {node_index} submitblock raised CalledProcessError: {e}")
+            return False
         except OSError as e:
             self.log.debug(f"node {node_index} submitblock raised OSError exception: errno={e.errno}")
             if e.errno in [errno.EPIPE, errno.ECONNREFUSED, errno.ECONNRESET]:

test/functional/feature_loadblock.py

@@ -25,7 +25,6 @@ class LoadblockTest(BitcoinTestFramework):
     def set_test_params(self):
         self.setup_clean_chain = True
         self.num_nodes = 2
-        self.supports_cli = False
 
     def run_test(self):
         self.nodes[1].setnetworkactive(state=False)

test/functional/interface_http.py

@@ -95,7 +95,6 @@ class BitcoinHTTPConnection:
 class HTTPBasicsTest (BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 1
-        self.supports_cli = False
 
     def setup_network(self):
         self.setup_nodes()

test/functional/interface_rest.py

@@ -55,7 +55,6 @@ class RESTTest (BitcoinTestFramework):
         self.extra_args = [["-rest", "-blockfilterindex=1"], []]
         # whitelist peers to speed up tx relay / mempool sync
         self.noban_tx_relay = True
-        self.supports_cli = False
 
     def test_rest_request(
             self,

test/functional/rpc_named_arguments.py

@@ -10,10 +10,10 @@ from test_framework.util import (
     assert_raises_rpc_error,
 )
 
+
 class NamedArgumentTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 1
-        self.supports_cli = False
 
     def run_test(self):
         node = self.nodes[0]
@@ -25,14 +25,14 @@ class NamedArgumentTest(BitcoinTestFramework):
         h = node.getblockhash(height=0)
         node.getblock(blockhash=h)
 
-        assert_equal(node.echo(), [])
-        assert_equal(node.echo(arg0=0,arg9=9), [0] + [None]*8 + [9])
-        assert_equal(node.echo(arg1=1), [None, 1])
-        assert_equal(node.echo(arg9=None), [None]*10)
-        assert_equal(node.echo(arg0=0,arg3=3,arg9=9), [0] + [None]*2 + [3] + [None]*5 + [9])
-        assert_equal(node.echo(0, 1, arg3=3, arg5=5), [0, 1, None, 3, None, 5])
-        assert_raises_rpc_error(-8, "Parameter arg1 specified twice both as positional and named argument", node.echo, 0, 1, arg1=1)
-        assert_raises_rpc_error(-8, "Parameter arg1 specified twice both as positional and named argument", node.echo, 0, None, 2, arg1=1)
+        assert_equal(node.echojson(), [])
+        assert_equal(node.echojson(arg0=0, arg9=9), [0] + [None] * 8 + [9])
+        assert_equal(node.echojson(arg1=1), [None, 1])
+        assert_equal(node.echojson(arg9=None), [] if self.options.usecli else [None] * 10)
+        assert_equal(node.echojson(arg0=0, arg3=3, arg9=9), [0] + [None] * 2 + [3] + [None] * 5 + [9])
+        assert_equal(node.echojson(0, 1, arg3=3, arg5=5), [0, 1, None, 3, None, 5])
+        assert_raises_rpc_error(-8, "Parameter arg1 specified twice both as positional and named argument", node.echojson, 0, 1, arg1=1)
+        assert_raises_rpc_error(-8, "Parameter arg1 specified twice both as positional and named argument", node.echojson, 0, None, 2, arg1=1)
 
 if __name__ == '__main__':
     NamedArgumentTest(__file__).main()

test/functional/rpc_users.py

@@ -36,7 +36,6 @@ def call_with_auth(node, user, password, method="getbestblockhash"):
 class HTTPBasicsTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 2
-        self.supports_cli = False
 
     def conf_setup(self):
         #Append rpcauth to bitcoin.conf before initialization
@@ -61,6 +60,7 @@ class HTTPBasicsTest(BitcoinTestFramework):
         rpcauth3 = lines[1]
         self.password = lines[3]
 
+        self.stop_nodes()
         with open(self.nodes[0].datadir_path / "bitcoin.conf", "a") as f:
             f.write(rpcauth + "\n")
             f.write(rpcauth2 + "\n")
@@ -68,8 +68,7 @@ class HTTPBasicsTest(BitcoinTestFramework):
         with open(self.nodes[1].datadir_path / "bitcoin.conf", "a") as f:
             f.write("rpcuser={}\n".format(self.rpcuser))
             f.write("rpcpassword={}\n".format(self.rpcpassword))
-        self.restart_node(0)
-        self.restart_node(1)
+        self.start_nodes()
 
     def test_auth(self, node, user, password):
         self.log.info('Correct...')
@@ -109,6 +108,7 @@ class HTTPBasicsTest(BitcoinTestFramework):
             assert_equal(expected_perms, actual_perms)
 
         # Remove any leftover rpc{user|password} config options from previous tests
+        self.stop_node(1)
         self.nodes[1].replace_in_config([("rpcuser", "#rpcuser"), ("rpcpassword", "#rpcpassword")])
 
         self.log.info('Check default cookie permission')

test/functional/rpc_whitelist.py

@@ -33,7 +33,6 @@ class RPCWhitelistTest(BitcoinTestFramework):
 
     def set_test_params(self):
         self.num_nodes = 1
-        self.supports_cli = False
 
     def run_test(self):
         # 0 => Username

test/functional/test_framework/test_node.py

@@ -379,11 +379,11 @@ class TestNode():
                 self.log.debug("RPC successfully started")
                 # Set rpc_connected even if we are in use_cli mode so that we know we can call self.stop() if needed.
                 self.rpc_connected = True
+                self.url = rpc.rpc_url
                 self.cli = self.create_new_rpc_connection(mode="CLI")
                 if self.use_cli:
                     return
                 self._rpc = rpc
-                self.url = self._rpc.rpc_url
                 return
             except JSONRPCException as e:
                 # Suppress these as they are expected during initialization.

test/functional/wallet_txn_clone.py

@@ -17,7 +17,6 @@ from test_framework.messages import (
 class TxnMallTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 3
-        self.supports_cli = False
         self.extra_args = [[] for i in range(self.num_nodes)]
 
     def skip_test_if_missing_module(self):