[tests] Move stop_node and start_node methods to BitcoinTestFramework

This commit moves functions start_node, start_nodes, stop_node and
stop_nodes functions into the BitcoinTestFramework class. It also moves
the bitcoind_processes dict and coverage variables into BitcoinTestFramework.

2 files changed, 116 insertions, 152 deletions

diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py
index 37e59f79db..557c9db482 100755
--- a/test/functional/test_framework/test_framework.py
+++ b/test/functional/test_framework/test_framework.py
@@ -5,7 +5,9 @@
 """Base class for RPC testing."""
 
 from collections import deque
+import errno
 from enum import Enum
+import http.client
 import logging
 import optparse
 import os
@@ -16,15 +18,16 @@ import tempfile
 import time
 import traceback
 
+from .authproxy import JSONRPCException
+from . import coverage
 from .util import (
-    PortSeed,
     MAX_NODES,
-    bitcoind_processes,
+    PortSeed,
+    assert_equal,
     check_json_precision,
     connect_nodes_bi,
     disable_mocktime,
     disconnect_nodes,
-    enable_coverage,
     enable_mocktime,
     get_mocktime,
     get_rpc_proxy,
@@ -34,15 +37,9 @@ from .util import (
     p2p_port,
     rpc_url,
     set_node_times,
-    _start_node,
-    _start_nodes,
-    _stop_node,
-    _stop_nodes,
     sync_blocks,
     sync_mempools,
-    wait_for_bitcoind_start,
 )
-from .authproxy import JSONRPCException
 
 class TestStatus(Enum):
     PASSED = 1
@@ -53,6 +50,8 @@ TEST_EXIT_PASSED = 0
 TEST_EXIT_FAILED = 1
 TEST_EXIT_SKIPPED = 77
 
+BITCOIND_PROC_WAIT_TIMEOUT = 60
+
 class BitcoinTestFramework(object):
     """Base class for a bitcoin test script.
 
@@ -72,7 +71,8 @@ class BitcoinTestFramework(object):
     def __init__(self):
         self.num_nodes = 4
         self.setup_clean_chain = False
-        self.nodes = None
+        self.nodes = []
+        self.bitcoind_processes = {}
 
     def add_options(self, parser):
         pass
@@ -98,7 +98,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 = self.start_nodes(self.num_nodes, self.options.tmpdir, extra_args)
 
     def run_test(self):
         raise NotImplementedError
@@ -130,9 +130,6 @@ class BitcoinTestFramework(object):
         self.add_options(parser)
         (self.options, self.args) = parser.parse_args()
 
-        if self.options.coveragedir:
-            enable_coverage(self.options.coveragedir)
-
         PortSeed.n = self.options.port_seed
 
         os.environ['PATH'] = self.options.srcdir + ":" + self.options.srcdir + "/qt:" + os.environ['PATH']
@@ -209,16 +206,88 @@ 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)
+        """Start a bitcoind and return RPC connection to it"""
+
+        datadir = os.path.join(dirname, "node" + str(i))
+        if binary is None:
+            binary = os.getenv("BITCOIND", "bitcoind")
+        args = [binary, "-datadir=" + datadir, "-server", "-keypool=1", "-discover=0", "-rest", "-logtimemicros", "-debug", "-debugexclude=libevent", "-debugexclude=leveldb", "-mocktime=" + str(get_mocktime()), "-uacomment=testnode%d" % i]
+        if extra_args is not None:
+            args.extend(extra_args)
+        self.bitcoind_processes[i] = subprocess.Popen(args, stderr=stderr)
+        self.log.debug("initialize_chain: bitcoind started, waiting for RPC to come up")
+        self._wait_for_bitcoind_start(self.bitcoind_processes[i], datadir, i, rpchost)
+        self.log.debug("initialize_chain: RPC successfully started")
+        proxy = get_rpc_proxy(rpc_url(datadir, i, rpchost), i, timeout=timewait)
+
+        if self.options.coveragedir:
+            coverage.write_all_rpc_commands(self.options.coveragedir, proxy)
+
+        return proxy
 
     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)
+        """Start multiple bitcoinds, return RPC connections to them"""
+
+        if extra_args is None:
+            extra_args = [None] * num_nodes
+        if binary is None:
+            binary = [None] * num_nodes
+        assert_equal(len(extra_args), num_nodes)
+        assert_equal(len(binary), num_nodes)
+        rpcs = []
+        try:
+            for i in range(num_nodes):
+                rpcs.append(self.start_node(i, dirname, extra_args[i], rpchost, timewait=timewait, binary=binary[i]))
+        except:
+            # If one node failed to start, stop the others
+            # TODO: abusing self.nodes in this way is a little hacky.
+            # Eventually we should do a better job of tracking nodes
+            self.nodes.extend(rpcs)
+            self.stop_nodes()
+            self.nodes = []
+            raise
+        return rpcs
+
+    def stop_node(self, i):
+        """Stop a bitcoind test node"""
 
-    def stop_node(self, num_node):
-        _stop_node(self.nodes[num_node], num_node)
+        self.log.debug("Stopping node %d" % i)
+        try:
+            self.nodes[i].stop()
+        except http.client.CannotSendRequest as e:
+            self.log.exception("Unable to stop node")
+        return_code = self.bitcoind_processes[i].wait(timeout=BITCOIND_PROC_WAIT_TIMEOUT)
+        assert_equal(return_code, 0)
+        del self.bitcoind_processes[i]
 
     def stop_nodes(self):
-        _stop_nodes(self.nodes)
+        """Stop multiple bitcoind test nodes"""
+
+        for i in range(len(self.nodes)):
+            self.stop_node(i)
+        assert not self.bitcoind_processes.values()  # All connections must be gone now
+
+    def assert_start_raises_init_error(self, i, dirname, extra_args=None, expected_msg=None):
+        with tempfile.SpooledTemporaryFile(max_size=2**16) as log_stderr:
+            try:
+                self.start_node(i, dirname, extra_args, stderr=log_stderr)
+                self.stop_node(i)
+            except Exception as e:
+                assert 'bitcoind exited' in str(e)  # node must have shutdown
+                if expected_msg is not None:
+                    log_stderr.seek(0)
+                    stderr = log_stderr.read().decode('utf-8')
+                    if expected_msg not in stderr:
+                        raise AssertionError("Expected error \"" + expected_msg + "\" not found in:\n" + stderr)
+            else:
+                if expected_msg is None:
+                    assert_msg = "bitcoind should have exited with an error"
+                else:
+                    assert_msg = "bitcoind should have exited with expected error " + expected_msg
+                raise AssertionError(assert_msg)
+
+    def wait_for_node_exit(self, i, timeout):
+        self.bitcoind_processes[i].wait(timeout)
 
     def split_network(self):
         """
@@ -300,9 +369,9 @@ class BitcoinTestFramework(object):
                 args = [os.getenv("BITCOIND", "bitcoind"), "-server", "-keypool=1", "-datadir=" + datadir, "-discover=0"]
                 if i > 0:
                     args.append("-connect=127.0.0.1:" + str(p2p_port(0)))
-                bitcoind_processes[i] = subprocess.Popen(args)
+                self.bitcoind_processes[i] = subprocess.Popen(args)
                 self.log.debug("initialize_chain: bitcoind started, waiting for RPC to come up")
-                wait_for_bitcoind_start(bitcoind_processes[i], datadir, i)
+                self._wait_for_bitcoind_start(self.bitcoind_processes[i], datadir, i)
                 self.log.debug("initialize_chain: RPC successfully started")
 
             self.nodes = []
@@ -355,6 +424,30 @@ class BitcoinTestFramework(object):
         for i in range(num_nodes):
             initialize_datadir(test_dir, i)
 
+    def _wait_for_bitcoind_start(self, process, datadir, i, rpchost=None):
+        """Wait for bitcoind to start.
+
+        This means that RPC is accessible and fully initialized.
+        Raise an exception if bitcoind exits during initialization."""
+        while True:
+            if process.poll() is not None:
+                raise Exception('bitcoind exited with status %i during initialization' % process.returncode)
+            try:
+                # Check if .cookie file to be created
+                rpc = get_rpc_proxy(rpc_url(datadir, i, rpchost), i, coveragedir=self.options.coveragedir)
+                rpc.getblockcount()
+                break  # break out of loop on success
+            except IOError as e:
+                if e.errno != errno.ECONNREFUSED:  # Port not yet open?
+                    raise  # unknown IO error
+            except JSONRPCException as e:  # Initialization phase
+                if e.error['code'] != -28:  # RPC in warmup?
+                    raise  # unknown JSON RPC exception
+            except ValueError as e:  # cookie file not found and no rpcuser or rpcassword. bitcoind still starting
+                if "No RPC credentials" not in str(e):
+                    raise
+            time.sleep(0.25)
+
 class ComparisonTestFramework(BitcoinTestFramework):
     """Test framework for doing p2p comparison testing
 
diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py
index d02a130a1f..fbdb3d7e6d 100644
--- a/test/functional/test_framework/util.py
+++ b/test/functional/test_framework/util.py
@@ -7,22 +7,16 @@
 from base64 import b64encode
 from binascii import hexlify, unhexlify
 from decimal import Decimal, ROUND_DOWN
-import errno
-import http.client
 import json
 import logging
 import os
 import random
 import re
-import subprocess
-import tempfile
 import time
 
 from . import coverage
 from .authproxy import AuthServiceProxy, JSONRPCException
 
-COVERAGE_DIR = None
-
 logger = logging.getLogger("TestFramework.utils")
 
 # The maximum number of nodes a single test can spawn
@@ -32,9 +26,6 @@ PORT_MIN = 11000
 # The number of ports to "reserve" for p2p and rpc, each
 PORT_RANGE = 5000
 
-BITCOIND_PROC_WAIT_TIMEOUT = 60
-
-
 class PortSeed:
     # Must be initialized with a unique integer for each process
     n = None
@@ -60,13 +51,7 @@ def disable_mocktime():
 def get_mocktime():
     return MOCKTIME
 
-def enable_coverage(dirname):
-    """Maintain a log of which RPC calls are made during testing."""
-    global COVERAGE_DIR
-    COVERAGE_DIR = dirname
-
-
-def get_rpc_proxy(url, node_number, timeout=None):
+def get_rpc_proxy(url, node_number, timeout=None, coveragedir=None):
     """
     Args:
         url (str): URL of the RPC server to call
@@ -87,7 +72,7 @@ def get_rpc_proxy(url, node_number, timeout=None):
     proxy.url = url  # store URL on proxy for info
 
     coverage_logfile = coverage.get_filename(
-        COVERAGE_DIR, node_number) if COVERAGE_DIR else None
+        coveragedir, node_number) if coveragedir else None
 
     return coverage.AuthServiceProxyWrapper(proxy, coverage_logfile)
 
@@ -172,8 +157,6 @@ def sync_mempools(rpc_connections, *, wait=1, timeout=60):
         timeout -= wait
     raise AssertionError("Mempool sync failed")
 
-bitcoind_processes = {}
-
 def initialize_datadir(dirname, n):
     datadir = os.path.join(dirname, "node" + str(n))
     if not os.path.isdir(datadir):
@@ -222,121 +205,9 @@ def rpc_url(datadir, i, rpchost=None):
             host = rpchost
     return "http://%s:%s@%s:%d" % (rpc_u, rpc_p, host, int(port))
 
-def wait_for_bitcoind_start(process, datadir, i, rpchost=None):
-    '''
-    Wait for bitcoind to start. This means that RPC is accessible and fully initialized.
-    Raise an exception if bitcoind exits during initialization.
-    '''
-    while True:
-        if process.poll() is not None:
-            raise Exception('bitcoind exited with status %i during initialization' % process.returncode)
-        try:
-            # Check if .cookie file to be created
-            rpc = get_rpc_proxy(rpc_url(datadir, i, rpchost), i)
-            rpc.getblockcount()
-            break  # break out of loop on success
-        except IOError as e:
-            if e.errno != errno.ECONNREFUSED:  # Port not yet open?
-                raise  # unknown IO error
-        except JSONRPCException as e:  # Initialization phase
-            if e.error['code'] != -28:  # RPC in warmup?
-                raise  # unknown JSON RPC exception
-        except ValueError as e:  # cookie file not found and no rpcuser or rpcassword. bitcoind still starting
-            if "No RPC credentials" not in str(e):
-                raise
-        time.sleep(0.25)
-
-def wait_for_node_exit(node_index, timeout):
-    bitcoind_processes[node_index].wait(timeout)
-
-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")
-    args = [binary, "-datadir=" + datadir, "-server", "-keypool=1", "-discover=0", "-rest", "-logtimemicros", "-debug", "-debugexclude=libevent", "-debugexclude=leveldb", "-mocktime=" + str(get_mocktime()), "-uacomment=testnode%d" % i]
-    if extra_args is not None:
-        args.extend(extra_args)
-    bitcoind_processes[i] = subprocess.Popen(args, stderr=stderr)
-    logger.debug("initialize_chain: bitcoind started, waiting for RPC to come up")
-    wait_for_bitcoind_start(bitcoind_processes[i], datadir, i, rpchost)
-    logger.debug("initialize_chain: RPC successfully started")
-    proxy = get_rpc_proxy(rpc_url(datadir, i, rpchost), i, timeout=timewait)
-
-    if COVERAGE_DIR:
-        coverage.write_all_rpc_commands(COVERAGE_DIR, proxy)
-
-    return proxy
-
-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)
-        except Exception as e:
-            assert 'bitcoind exited' in str(e)  # node must have shutdown
-            if expected_msg is not None:
-                log_stderr.seek(0)
-                stderr = log_stderr.read().decode('utf-8')
-                if expected_msg not in stderr:
-                    raise AssertionError("Expected error \"" + expected_msg + "\" not found in:\n" + stderr)
-        else:
-            if expected_msg is None:
-                assert_msg = "bitcoind should have exited with an error"
-            else:
-                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
-
-    This function should only be called from within test_framework, not by individual test scripts."""
-
-    if extra_args is None:
-        extra_args = [None] * num_nodes
-    if binary is None:
-        binary = [None] * num_nodes
-    assert_equal(len(extra_args), num_nodes)
-    assert_equal(len(binary), num_nodes)
-    rpcs = []
-    try:
-        for i in range(num_nodes):
-            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)
-        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):
-    """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()
-    except http.client.CannotSendRequest as e:
-        logger.exception("Unable to stop node")
-    return_code = bitcoind_processes[i].wait(timeout=BITCOIND_PROC_WAIT_TIMEOUT)
-    del bitcoind_processes[i]
-    assert_equal(return_code, 0)
-
-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)
-    assert not bitcoind_processes.values()  # All connections must be gone now
-
 def set_node_times(nodes, t):
     for node in nodes:
         node.setmocktime(t)