Merge #7971: [qa] Refactor test_framework and pull tester

fad3366 [qa] pull-tester: Adjust comment (MarcoFalke)
fafb33c [qa] Stop other nodes, even when one fails to stop (MarcoFalke)
2222dae [qa] Update README.md (MarcoFalke)
fabbf6b [qa] Refactor test_framework and pull tester (MarcoFalke)

5 files changed, 79 insertions, 57 deletions

diff --git a/qa/README.md b/qa/README.md
index 3e0a526d13..c7e574ff70 100644
--- a/qa/README.md
+++ b/qa/README.md
@@ -19,15 +19,25 @@ sudo apt-get install python3-zmq
 Running tests
 =============
 
-You can run any single test by calling `qa/pull-tester/rpc-tests.py <testname>`.
+You can run any single test by calling
 
-Or you can run any combination of tests by calling `qa/pull-tester/rpc-tests.py <testname1> <testname2> <testname3> ...`
+    qa/pull-tester/rpc-tests.py <testname>
 
-Run the regression test suite with `qa/pull-tester/rpc-tests.py`
+Or you can run any combination of tests by calling
 
-Run all possible tests with `qa/pull-tester/rpc-tests.py -extended`
+    qa/pull-tester/rpc-tests.py <testname1> <testname2> <testname3> ...
 
-Possible options:
+Run the regression test suite with
+
+    qa/pull-tester/rpc-tests.py
+
+Run all possible tests with
+
+    qa/pull-tester/rpc-tests.py -extended
+
+If you want to create a basic coverage report for the rpc test suite, append `--coverage`.
+
+Possible options, which apply to each individual test run:
 
 ```
   -h, --help            show this help message and exit
diff --git a/qa/pull-tester/rpc-tests.py b/qa/pull-tester/rpc-tests.py
index 01c2736e66..10c3799b86 100755
--- a/qa/pull-tester/rpc-tests.py
+++ b/qa/pull-tester/rpc-tests.py
@@ -31,6 +31,14 @@ import re
 
 from tests_config import *
 
+BOLD = ("","")
+if os.name == 'posix':
+    # primitive formatting on supported
+    # terminal via ANSI escape sequences:
+    BOLD = ('\033[0m', '\033[1m')
+
+RPC_TESTS_DIR = BUILDDIR + '/qa/rpc-tests/'
+
 #If imported values are not defined then set to zero (or disabled)
 if 'ENABLE_WALLET' not in vars():
     ENABLE_WALLET=0
@@ -43,29 +51,29 @@ if 'ENABLE_ZMQ' not in vars():
 
 ENABLE_COVERAGE=0
 
-#Create a set to store arguments and create the passOn string
+#Create a set to store arguments and create the passon string
 opts = set()
-passOn = ""
-p = re.compile("^--")
+passon_args = ""
+PASSON_REGEX = re.compile("^--")
 
-bold = ("","")
-if (os.name == 'posix'):
-    bold = ('\033[0m', '\033[1m')
+print_help = False
 
 for arg in sys.argv[1:]:
+    if arg == "--help" or arg == "-h" or arg == "-?":
+        print_help = True
+        break
     if arg == '--coverage':
         ENABLE_COVERAGE = 1
-    elif (p.match(arg) or arg == "-h"):
-        passOn += " " + arg
+    elif PASSON_REGEX.match(arg):
+        passon_args += " " + arg
     else:
         opts.add(arg)
 
 #Set env vars
-buildDir = BUILDDIR
 if "BITCOIND" not in os.environ:
-    os.environ["BITCOIND"] = buildDir + '/src/bitcoind' + EXEEXT
+    os.environ["BITCOIND"] = BUILDDIR + '/src/bitcoind' + EXEEXT
 if "BITCOINCLI" not in os.environ:
-    os.environ["BITCOINCLI"] = buildDir + '/src/bitcoin-cli' + EXEEXT
+    os.environ["BITCOINCLI"] = BUILDDIR + '/src/bitcoin-cli' + EXEEXT
 
 if EXEEXT == ".exe" and "-win" not in opts:
     # https://github.com/bitcoin/bitcoin/commit/d52802551752140cf41f0d9a225a43e84404d3e9
@@ -153,48 +161,34 @@ testScriptsExt = [
 ]
 
 def runtests():
+    test_list = []
+    if '-extended' in opts:
+        test_list = testScripts + testScriptsExt
+    elif len(opts) == 0 or (len(opts) == 1 and "-win" in opts):
+        test_list = testScripts
+    else:
+        for t in testScripts + testScriptsExt:
+            if t in opts or re.sub(".py$", "", t) in opts:
+                test_list.append(t)
+
+    if print_help:
+        # Only print help of the first script and exit
+        subprocess.check_call(RPC_TESTS_DIR + test_list[0] + ' -h', shell=True)
+        sys.exit(0)
+
     coverage = None
 
     if ENABLE_COVERAGE:
         coverage = RPCCoverage()
         print("Initializing coverage directory at %s\n" % coverage.dir)
-
-    rpcTestDir = buildDir + '/qa/rpc-tests/'
-    run_extended = '-extended' in opts
-    cov_flag = coverage.flag if coverage else ''
-    flags = " --srcdir %s/src %s %s" % (buildDir, cov_flag, passOn)
+    flags = " --srcdir %s/src %s %s" % (BUILDDIR, coverage.flag if coverage else '', passon_args)
 
     #Run Tests
-    for i in range(len(testScripts)):
-        if (len(opts) == 0
-                or (len(opts) == 1 and "-win" in opts )
-                or run_extended
-                or testScripts[i] in opts
-                or re.sub(".py$", "", testScripts[i]) in opts ):
-
-            print("Running testscript %s%s%s ..." % (bold[1], testScripts[i], bold[0]))
-            time0 = time.time()
-            subprocess.check_call(
-                rpcTestDir + testScripts[i] + flags, shell=True)
-            print("Duration: %s s\n" % (int(time.time() - time0)))
-
-            # exit if help is called so we print just one set of
-            # instructions
-            p = re.compile(" -h| --help")
-            if p.match(passOn):
-                sys.exit(0)
-
-    # Run Extended Tests
-    for i in range(len(testScriptsExt)):
-        if (run_extended or testScriptsExt[i] in opts
-                or re.sub(".py$", "", testScriptsExt[i]) in opts):
-
-            print(
-                "Running 2nd level testscript "
-                + "%s%s%s ..." % (bold[1], testScriptsExt[i], bold[0]))
+    for t in test_list:
+            print("Running testscript %s%s%s ..." % (BOLD[1], t, BOLD[0]))
             time0 = time.time()
             subprocess.check_call(
-                rpcTestDir + testScriptsExt[i] + flags, shell=True)
+                RPC_TESTS_DIR + t + flags, shell=True)
             print("Duration: %s s\n" % (int(time.time() - time0)))
 
     if coverage:
diff --git a/qa/rpc-tests/test_framework/test_framework.py b/qa/rpc-tests/test_framework/test_framework.py
index 0b6d1b6abc..4e72463096 100755
--- a/qa/rpc-tests/test_framework/test_framework.py
+++ b/qa/rpc-tests/test_framework/test_framework.py
@@ -115,7 +115,7 @@ class BitcoinTestFramework(object):
 
         if self.options.trace_rpc:
             import logging
-            logging.basicConfig(level=logging.DEBUG)
+            logging.basicConfig(level=logging.DEBUG, stream=sys.stdout)
 
         if self.options.coveragedir:
             enable_coverage(self.options.coveragedir)
@@ -148,6 +148,8 @@ class BitcoinTestFramework(object):
         except Exception as e:
             print("Unexpected exception caught during testing: " + repr(e))
             traceback.print_tb(sys.exc_info()[2])
+        except KeyboardInterrupt as e:
+            print("Exiting after " + repr(e))
 
         if not self.options.noshutdown:
             print("Stopping nodes")
diff --git a/qa/rpc-tests/test_framework/util.py b/qa/rpc-tests/test_framework/util.py
index ea3931cef3..6dc685ea1b 100644
--- a/qa/rpc-tests/test_framework/util.py
+++ b/qa/rpc-tests/test_framework/util.py
@@ -16,6 +16,7 @@ from binascii import hexlify, unhexlify
 from base64 import b64encode
 from decimal import Decimal, ROUND_DOWN
 import json
+import http.client
 import random
 import shutil
 import subprocess
@@ -28,6 +29,13 @@ from .authproxy import AuthServiceProxy, JSONRPCException
 
 COVERAGE_DIR = None
 
+# The maximum number of nodes a single test can spawn
+MAX_NODES = 8
+# Don't assign rpc or p2p ports lower than this
+PORT_MIN = 11000
+# The number of ports to "reserve" for p2p and rpc, each
+PORT_RANGE = 5000
+
 #Set Mocktime default to OFF.
 #MOCKTIME is only needed for scripts that use the
 #cached version of the blockchain.  If the cached
@@ -82,9 +90,11 @@ def get_rpc_proxy(url, node_number, timeout=None):
 
 
 def p2p_port(n):
-    return 11000 + n + os.getpid()%999
+    assert(n <= MAX_NODES)
+    return PORT_MIN + n + (MAX_NODES * os.getpid()) % (PORT_RANGE - 1 - MAX_NODES)
+
 def rpc_port(n):
-    return 12000 + n + os.getpid()%999
+    return PORT_MIN + PORT_RANGE + n + (MAX_NODES * os.getpid()) % (PORT_RANGE -1 - MAX_NODES)
 
 def check_json_precision():
     """Make sure json library being used does not lose precision converting BTC values"""
@@ -292,8 +302,8 @@ def start_nodes(num_nodes, dirname, extra_args=None, rpchost=None, binary=None):
     """
     Start multiple bitcoinds, return RPC connections to them
     """
-    if extra_args is None: extra_args = [ None for i in range(num_nodes) ]
-    if binary is None: binary = [ None for i in range(num_nodes) ]
+    if extra_args is None: extra_args = [ None for _ in range(num_nodes) ]
+    if binary is None: binary = [ None for _ in range(num_nodes) ]
     rpcs = []
     try:
         for i in range(num_nodes):
@@ -307,13 +317,19 @@ def log_filename(dirname, n_node, logname):
     return os.path.join(dirname, "node"+str(n_node), "regtest", logname)
 
 def stop_node(node, i):
-    node.stop()
+    try:
+        node.stop()
+    except http.client.CannotSendRequest as e:
+        print("WARN: Unable to stop node: " + repr(e))
     bitcoind_processes[i].wait()
     del bitcoind_processes[i]
 
 def stop_nodes(nodes):
     for node in nodes:
-        node.stop()
+        try:
+            node.stop()
+        except http.client.CannotSendRequest as e:
+            print("WARN: Unable to stop node: " + repr(e))
     del nodes[:] # Emptying array closes connections as a side effect
 
 def set_node_times(nodes, t):
diff --git a/qa/rpc-tests/walletbackup.py b/qa/rpc-tests/walletbackup.py
index 418f3103e3..c3d53669c9 100755
--- a/qa/rpc-tests/walletbackup.py
+++ b/qa/rpc-tests/walletbackup.py
@@ -37,7 +37,7 @@ from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import *
 from random import randint
 import logging
-logging.basicConfig(format='%(levelname)s:%(message)s', level=logging.INFO)
+logging.basicConfig(format='%(levelname)s: %(message)s', level=logging.INFO, stream=sys.stdout)
 
 class WalletBackupTest(BitcoinTestFramework):