Merge #10359: [tests] functional tests should call BitcoinTestFramework start/stop node methods

53f6775 fixup: fix nits (John Newbery)
a433d8a [tests] Update start/stop node functions to be private module functions (John Newbery)
d8c218f [tests] Functional tests call self.start_node(s) and self.stop_node(s) (John Newbery)

Tree-SHA512: 9cc01584a5e57686b7e7cb1c4c5186ad8cc7eb650d6d4f27b06bdb5e249a10966705814bdfb22d9ff2d5d3326911e489bf3d22257d751a299c0b24b7f40bffb5

test/functional/test_framework/test_framework.py

@@ -32,10 +32,10 @@ from .util import (
     p2p_port,
     rpc_url,
     set_node_times,
-    start_node,
-    start_nodes,
-    stop_node,
-    stop_nodes,
+    _start_node,
+    _start_nodes,
+    _stop_node,
+    _stop_nodes,
     sync_blocks,
     sync_mempools,
     wait_for_bitcoind_start,
@@ -96,7 +96,7 @@ class BitcoinTestFramework(object):
         extra_args = None
         if hasattr(self, "extra_args"):
             extra_args = self.extra_args
-        self.nodes = start_nodes(self.num_nodes, self.options.tmpdir, extra_args)
+        self.nodes = _start_nodes(self.num_nodes, self.options.tmpdir, extra_args)
 
     def run_test(self):
         raise NotImplementedError
@@ -207,16 +207,16 @@ class BitcoinTestFramework(object):
     # Public helper methods. These can be accessed by the subclass test scripts.
 
     def start_node(self, i, dirname, extra_args=None, rpchost=None, timewait=None, binary=None, stderr=None):
-        return start_node(i, dirname, extra_args, rpchost, timewait, binary, stderr)
+        return _start_node(i, dirname, extra_args, rpchost, timewait, binary, stderr)
 
     def start_nodes(self, num_nodes, dirname, extra_args=None, rpchost=None, timewait=None, binary=None):
-        return start_nodes(num_nodes, dirname, extra_args, rpchost, timewait, binary)
+        return _start_nodes(num_nodes, dirname, extra_args, rpchost, timewait, binary)
 
     def stop_node(self, num_node):
-        stop_node(self.nodes[num_node], num_node)
+        _stop_node(self.nodes[num_node], num_node)
 
     def stop_nodes(self):
-        stop_nodes(self.nodes)
+        _stop_nodes(self.nodes)
 
     def split_network(self):
         """

test/functional/test_framework/util.py

@@ -227,10 +227,11 @@ def wait_for_bitcoind_start(process, url, i):
         time.sleep(0.25)
 
 
-def start_node(i, dirname, extra_args=None, rpchost=None, timewait=None, binary=None, stderr=None):
-    """
-    Start a bitcoind and return RPC connection to it
-    """
+def _start_node(i, dirname, extra_args=None, rpchost=None, timewait=None, binary=None, stderr=None):
+    """Start a bitcoind and return RPC connection to it
+
+    This function should only be called from within test_framework, not by individual test scripts."""
+
     datadir = os.path.join(dirname, "node"+str(i))
     if binary is None:
         binary = os.getenv("BITCOIND", "bitcoind")
@@ -251,8 +252,8 @@ def start_node(i, dirname, extra_args=None, rpchost=None, timewait=None, binary=
 def assert_start_raises_init_error(i, dirname, extra_args=None, expected_msg=None):
     with tempfile.SpooledTemporaryFile(max_size=2**16) as log_stderr:
         try:
-            node = start_node(i, dirname, extra_args, stderr=log_stderr)
-            stop_node(node, i)
+            node = _start_node(i, dirname, extra_args, stderr=log_stderr)
+            _stop_node(node, i)
         except Exception as e:
             assert 'bitcoind exited' in str(e) #node must have shutdown
             if expected_msg is not None:
@@ -267,10 +268,11 @@ def assert_start_raises_init_error(i, dirname, extra_args=None, expected_msg=Non
                 assert_msg = "bitcoind should have exited with expected error " + expected_msg
             raise AssertionError(assert_msg)
 
-def start_nodes(num_nodes, dirname, extra_args=None, rpchost=None, timewait=None, binary=None):
-    """
-    Start multiple bitcoinds, return RPC connections to them
-    """
+def _start_nodes(num_nodes, dirname, extra_args=None, rpchost=None, timewait=None, binary=None):
+    """Start multiple bitcoinds, return RPC connections to them
+    
+    This function should only be called from within test_framework, not by individual test scripts."""
+
     if extra_args is None: extra_args = [ None for _ in range(num_nodes) ]
     if binary is None: binary = [ None for _ in range(num_nodes) ]
     assert_equal(len(extra_args), num_nodes)
@@ -278,16 +280,20 @@ def start_nodes(num_nodes, dirname, extra_args=None, rpchost=None, timewait=None
     rpcs = []
     try:
         for i in range(num_nodes):
-            rpcs.append(start_node(i, dirname, extra_args[i], rpchost, timewait=timewait, binary=binary[i]))
+            rpcs.append(_start_node(i, dirname, extra_args[i], rpchost, timewait=timewait, binary=binary[i]))
     except: # If one node failed to start, stop the others
-        stop_nodes(rpcs)
+        _stop_nodes(rpcs)
         raise
     return rpcs
 
 def log_filename(dirname, n_node, logname):
     return os.path.join(dirname, "node"+str(n_node), "regtest", logname)
 
-def stop_node(node, i):
+def _stop_node(node, i):
+    """Stop a bitcoind test node
+
+    This function should only be called from within test_framework, not by individual test scripts."""
+
     logger.debug("Stopping node %d" % i)
     try:
         node.stop()
@@ -297,9 +303,13 @@ def stop_node(node, i):
     assert_equal(return_code, 0)
     del bitcoind_processes[i]
 
-def stop_nodes(nodes):
+def _stop_nodes(nodes):
+    """Stop multiple bitcoind test nodes
+
+    This function should only be called from within test_framework, not by individual test scripts."""
+
     for i, node in enumerate(nodes):
-        stop_node(node, i)
+        _stop_node(node, i)
     assert not bitcoind_processes.values() # All connections must be gone now
 
 def set_node_times(nodes, t):