Add basic coverage reporting for RPC tests

Thanks to @MarcoFalke @dexX7 @laanwj for review.

8 files changed, 334 insertions, 55 deletions

diff --git a/.travis.yml b/.travis.yml
index 8e9684826f..d2fbfee6f2 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -69,6 +69,6 @@ script:
     - make $MAKEJOBS $GOAL || ( echo "Build failure. Verbose build follows." && make $GOAL V=1 ; false )
     - export LD_LIBRARY_PATH=$TRAVIS_BUILD_DIR/depends/$HOST/lib
     - if [ "$RUN_TESTS" = "true" ]; then make check; fi
-    - if [ "$RUN_TESTS" = "true" ]; then qa/pull-tester/rpc-tests.py; fi
+    - if [ "$RUN_TESTS" = "true" ]; then qa/pull-tester/rpc-tests.py --coverage; fi
 after_script:
     - if [ "$TRAVIS_PULL_REQUEST" != "false" ]; then (echo "Upload goes here. Something like: scp -r $BASE_OUTDIR server" || echo "upload failed"); fi
diff --git a/qa/pull-tester/rpc-tests.py b/qa/pull-tester/rpc-tests.py
index 3059fee426..891962ac14 100755
--- a/qa/pull-tester/rpc-tests.py
+++ b/qa/pull-tester/rpc-tests.py
@@ -3,16 +3,32 @@
 # Distributed under the MIT software license, see the accompanying
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
-#
-# Run Regression Test Suite
-#
+"""
+Run Regression Test Suite
+
+This module calls down into individual test cases via subprocess. It will
+forward all unrecognized arguments onto the individual test scripts, other
+than:
+
+    - `-extended`: run the "extended" test suite in addition to the basic one.
+    - `-win`: signal that this is running in a Windows environment, and we
+      should run the tests.
+    - `--coverage`: this generates a basic coverage report for the RPC
+      interface.
+
+For a description of arguments recognized by test scripts, see
+`qa/pull-tester/test_framework/test_framework.py:BitcoinTestFramework.main`.
+
+"""
 
 import os
+import shutil
 import sys
 import subprocess
+import tempfile
 import re
+
 from tests_config import *
-from sets import Set
 
 #If imported values are not defined then set to zero (or disabled)
 if not vars().has_key('ENABLE_WALLET'):
@@ -24,15 +40,20 @@ if not vars().has_key('ENABLE_UTILS'):
 if not vars().has_key('ENABLE_ZMQ'):
     ENABLE_ZMQ=0
 
+ENABLE_COVERAGE=0
+
 #Create a set to store arguments and create the passOn string
-opts = Set()
+opts = set()
 passOn = ""
 p = re.compile("^--")
-for i in range(1,len(sys.argv)):
-    if (p.match(sys.argv[i]) or sys.argv[i] == "-h"):
-        passOn += " " + sys.argv[i]
+
+for arg in sys.argv[1:]:
+    if arg == '--coverage':
+        ENABLE_COVERAGE = 1
+    elif (p.match(arg) or arg == "-h"):
+        passOn += " " + arg
     else:
-        opts.add(sys.argv[i])
+        opts.add(arg)
 
 #Set env vars
 buildDir = BUILDDIR
@@ -97,24 +118,125 @@ testScriptsExt = [
 if ENABLE_ZMQ == 1:
     testScripts.append('zmq_test.py')
 
-if(ENABLE_WALLET == 1 and ENABLE_UTILS == 1 and ENABLE_BITCOIND == 1):
-    rpcTestDir = buildDir + '/qa/rpc-tests/'
-    #Run Tests
-    for i in range(len(testScripts)):
-       if (len(opts) == 0 or (len(opts) == 1 and "-win" in opts ) or '-extended' in opts
-           or testScripts[i] in opts or  re.sub(".py$", "", testScripts[i]) in opts ):
-            print  "Running testscript " + testScripts[i] + "..."
-            subprocess.check_call(rpcTestDir + testScripts[i] + " --srcdir " + buildDir + '/src ' + passOn,shell=True)
-	    #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 ('-extended' in opts or testScriptsExt[i] in opts
-           or re.sub(".py$", "", testScriptsExt[i]) in opts):
-            print  "Running 2nd level testscript " + testScriptsExt[i] + "..."
-            subprocess.check_call(rpcTestDir + testScriptsExt[i] + " --srcdir " + buildDir + '/src ' + passOn,shell=True)
-else:
-    print "No rpc tests to run. Wallet, utils, and bitcoind must all be enabled"
+
+def runtests():
+    coverage = None
+
+    if ENABLE_COVERAGE:
+        coverage = RPCCoverage()
+        print("Initializing coverage directory at %s" % coverage.dir)
+
+    if(ENABLE_WALLET == 1 and ENABLE_UTILS == 1 and ENABLE_BITCOIND == 1):
+        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)
+
+        #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 " + testScripts[i] + "...")
+
+                subprocess.check_call(
+                    rpcTestDir + testScripts[i] + flags, shell=True)
+
+                # 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 "
+                    + testScriptsExt[i] + "...")
+
+                subprocess.check_call(
+                    rpcTestDir + testScriptsExt[i] + flags, shell=True)
+
+        if coverage:
+            coverage.report_rpc_coverage()
+
+            print("Cleaning up coverage data")
+            coverage.cleanup()
+
+    else:
+        print "No rpc tests to run. Wallet, utils, and bitcoind must all be enabled"
+
+
+class RPCCoverage(object):
+    """
+    Coverage reporting utilities for pull-tester.
+
+    Coverage calculation works by having each test script subprocess write
+    coverage files into a particular directory. These files contain the RPC
+    commands invoked during testing, as well as a complete listing of RPC
+    commands per `bitcoin-cli help` (`rpc_interface.txt`).
+
+    After all tests complete, the commands run are combined and diff'd against
+    the complete list to calculate uncovered RPC commands.
+
+    See also: qa/rpc-tests/test_framework/coverage.py
+
+    """
+    def __init__(self):
+        self.dir = tempfile.mkdtemp(prefix="coverage")
+        self.flag = '--coveragedir %s' % self.dir
+
+    def report_rpc_coverage(self):
+        """
+        Print out RPC commands that were unexercised by tests.
+
+        """
+        uncovered = self._get_uncovered_rpc_commands()
+
+        if uncovered:
+            print("Uncovered RPC commands:")
+            print("".join(("  - %s\n" % i) for i in sorted(uncovered)))
+        else:
+            print("All RPC commands covered.")
+
+    def cleanup(self):
+        return shutil.rmtree(self.dir)
+
+    def _get_uncovered_rpc_commands(self):
+        """
+        Return a set of currently untested RPC commands.
+
+        """
+        # This is shared from `qa/rpc-tests/test-framework/coverage.py`
+        REFERENCE_FILENAME = 'rpc_interface.txt'
+        COVERAGE_FILE_PREFIX = 'coverage.'
+
+        coverage_ref_filename = os.path.join(self.dir, REFERENCE_FILENAME)
+        coverage_filenames = set()
+        all_cmds = set()
+        covered_cmds = set()
+
+        if not os.path.isfile(coverage_ref_filename):
+            raise RuntimeError("No coverage reference found")
+
+        with open(coverage_ref_filename, 'r') as f:
+            all_cmds.update([i.strip() for i in f.readlines()])
+
+        for root, dirs, files in os.walk(self.dir):
+            for filename in files:
+                if filename.startswith(COVERAGE_FILE_PREFIX):
+                    coverage_filenames.add(os.path.join(root, filename))
+
+        for filename in coverage_filenames:
+            with open(filename, 'r') as f:
+                covered_cmds.update([i.strip() for i in f.readlines()])
+
+        return all_cmds - covered_cmds
+
+
+if __name__ == '__main__':
+    runtests()
diff --git a/qa/rpc-tests/getblocktemplate_longpoll.py b/qa/rpc-tests/getblocktemplate_longpoll.py
index aab4562422..1ddff8a298 100755
--- a/qa/rpc-tests/getblocktemplate_longpoll.py
+++ b/qa/rpc-tests/getblocktemplate_longpoll.py
@@ -38,7 +38,7 @@ class LongpollThread(threading.Thread):
         self.longpollid = templat['longpollid']
         # create a new connection to the node, we can't use the same
         # connection from two threads
-        self.node = AuthServiceProxy(node.url, timeout=600)
+        self.node = get_rpc_proxy(node.url, 1, timeout=600)
 
     def run(self):
         self.node.getblocktemplate({'longpollid':self.longpollid})
diff --git a/qa/rpc-tests/rpcbind_test.py b/qa/rpc-tests/rpcbind_test.py
index 04110c2831..7a9da66787 100755
--- a/qa/rpc-tests/rpcbind_test.py
+++ b/qa/rpc-tests/rpcbind_test.py
@@ -47,7 +47,7 @@ def run_allowip_test(tmpdir, allow_ips, rpchost, rpcport):
     try:
         # connect to node through non-loopback interface
         url = "http://rt:rt@%s:%d" % (rpchost, rpcport,)
-        node = AuthServiceProxy(url)
+        node = get_rpc_proxy(url, 1)
         node.getinfo()
     finally:
         node = None # make sure connection will be garbage collected and closed
diff --git a/qa/rpc-tests/test_framework/authproxy.py b/qa/rpc-tests/test_framework/authproxy.py
index 33014dc139..fba469a0dd 100644
--- a/qa/rpc-tests/test_framework/authproxy.py
+++ b/qa/rpc-tests/test_framework/authproxy.py
@@ -69,7 +69,7 @@ class AuthServiceProxy(object):
 
     def __init__(self, service_url, service_name=None, timeout=HTTP_TIMEOUT, connection=None):
         self.__service_url = service_url
-        self.__service_name = service_name
+        self._service_name = service_name
         self.__url = urlparse.urlparse(service_url)
         if self.__url.port is None:
             port = 80
@@ -102,8 +102,8 @@ class AuthServiceProxy(object):
         if name.startswith('__') and name.endswith('__'):
             # Python internal stuff
             raise AttributeError
-        if self.__service_name is not None:
-            name = "%s.%s" % (self.__service_name, name)
+        if self._service_name is not None:
+            name = "%s.%s" % (self._service_name, name)
         return AuthServiceProxy(self.__service_url, name, connection=self.__conn)
 
     def _request(self, method, path, postdata):
@@ -129,10 +129,10 @@ class AuthServiceProxy(object):
     def __call__(self, *args):
         AuthServiceProxy.__id_count += 1
 
-        log.debug("-%s-> %s %s"%(AuthServiceProxy.__id_count, self.__service_name,
+        log.debug("-%s-> %s %s"%(AuthServiceProxy.__id_count, self._service_name,
                                  json.dumps(args, default=EncodeDecimal)))
         postdata = json.dumps({'version': '1.1',
-                               'method': self.__service_name,
+                               'method': self._service_name,
                                'params': args,
                                'id': AuthServiceProxy.__id_count}, default=EncodeDecimal)
         response = self._request('POST', self.__url.path, postdata)
diff --git a/qa/rpc-tests/test_framework/coverage.py b/qa/rpc-tests/test_framework/coverage.py
new file mode 100644
index 0000000000..50f066a850
--- /dev/null
+++ b/qa/rpc-tests/test_framework/coverage.py
@@ -0,0 +1,101 @@
+"""
+This module contains utilities for doing coverage analysis on the RPC
+interface.
+
+It provides a way to track which RPC commands are exercised during
+testing.
+
+"""
+import os
+
+
+REFERENCE_FILENAME = 'rpc_interface.txt'
+
+
+class AuthServiceProxyWrapper(object):
+    """
+    An object that wraps AuthServiceProxy to record specific RPC calls.
+
+    """
+    def __init__(self, auth_service_proxy_instance, coverage_logfile=None):
+        """
+        Kwargs:
+            auth_service_proxy_instance (AuthServiceProxy): the instance
+                being wrapped.
+            coverage_logfile (str): if specified, write each service_name
+                out to a file when called.
+
+        """
+        self.auth_service_proxy_instance = auth_service_proxy_instance
+        self.coverage_logfile = coverage_logfile
+
+    def __getattr__(self, *args, **kwargs):
+        return_val = self.auth_service_proxy_instance.__getattr__(
+            *args, **kwargs)
+
+        return AuthServiceProxyWrapper(return_val, self.coverage_logfile)
+
+    def __call__(self, *args, **kwargs):
+        """
+        Delegates to AuthServiceProxy, then writes the particular RPC method
+        called to a file.
+
+        """
+        return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
+        rpc_method = self.auth_service_proxy_instance._service_name
+
+        if self.coverage_logfile:
+            with open(self.coverage_logfile, 'a+') as f:
+                f.write("%s\n" % rpc_method)
+
+        return return_val
+
+    @property
+    def url(self):
+        return self.auth_service_proxy_instance.url
+
+
+def get_filename(dirname, n_node):
+    """
+    Get a filename unique to the test process ID and node.
+
+    This file will contain a list of RPC commands covered.
+    """
+    pid = str(os.getpid())
+    return os.path.join(
+        dirname, "coverage.pid%s.node%s.txt" % (pid, str(n_node)))
+
+
+def write_all_rpc_commands(dirname, node):
+    """
+    Write out a list of all RPC functions available in `bitcoin-cli` for
+    coverage comparison. This will only happen once per coverage
+    directory.
+
+    Args:
+        dirname (str): temporary test dir
+        node (AuthServiceProxy): client
+
+    Returns:
+        bool. if the RPC interface file was written.
+
+    """
+    filename = os.path.join(dirname, REFERENCE_FILENAME)
+
+    if os.path.isfile(filename):
+        return False
+
+    help_output = node.help().split('\n')
+    commands = set()
+
+    for line in help_output:
+        line = line.strip()
+
+        # Ignore blanks and headers
+        if line and not line.startswith('='):
+            commands.add("%s\n" % line.split()[0])
+
+    with open(filename, 'w') as f:
+        f.writelines(list(commands))
+
+    return True
diff --git a/qa/rpc-tests/test_framework/test_framework.py b/qa/rpc-tests/test_framework/test_framework.py
index 5671431f6e..ae2d91ab60 100755
--- a/qa/rpc-tests/test_framework/test_framework.py
+++ b/qa/rpc-tests/test_framework/test_framework.py
@@ -13,8 +13,20 @@ import shutil
 import tempfile
 import traceback
 
+from .util import (
+    initialize_chain,
+    assert_equal,
+    start_nodes,
+    connect_nodes_bi,
+    sync_blocks,
+    sync_mempools,
+    stop_nodes,
+    wait_bitcoinds,
+    enable_coverage,
+    check_json_precision,
+    initialize_chain_clean,
+)
 from authproxy import AuthServiceProxy, JSONRPCException
-from util import *
 
 
 class BitcoinTestFramework(object):
@@ -96,6 +108,8 @@ class BitcoinTestFramework(object):
                           help="Root directory for datadirs")
         parser.add_option("--tracerpc", dest="trace_rpc", default=False, action="store_true",
                           help="Print out all RPC calls as they are made")
+        parser.add_option("--coveragedir", dest="coveragedir",
+                          help="Write tested RPC commands into this directory")
         self.add_options(parser)
         (self.options, self.args) = parser.parse_args()
 
@@ -103,6 +117,9 @@ class BitcoinTestFramework(object):
             import logging
             logging.basicConfig(level=logging.DEBUG)
 
+        if self.options.coveragedir:
+            enable_coverage(self.options.coveragedir)
+
         os.environ['PATH'] = self.options.srcdir+":"+os.environ['PATH']
 
         check_json_precision()
@@ -173,7 +190,8 @@ class ComparisonTestFramework(BitcoinTestFramework):
         initialize_chain_clean(self.options.tmpdir, self.num_nodes)
 
     def setup_network(self):
-        self.nodes = start_nodes(self.num_nodes, self.options.tmpdir,
-                                    extra_args=[['-debug', '-whitelist=127.0.0.1']] * self.num_nodes,
-                                    binary=[self.options.testbinary] +
-                                           [self.options.refbinary]*(self.num_nodes-1))
+        self.nodes = start_nodes(
+            self.num_nodes, self.options.tmpdir,
+            extra_args=[['-debug', '-whitelist=127.0.0.1']] * self.num_nodes,
+            binary=[self.options.testbinary] +
+            [self.options.refbinary]*(self.num_nodes-1))
diff --git a/qa/rpc-tests/test_framework/util.py b/qa/rpc-tests/test_framework/util.py
index 3759cc8162..30dd5de585 100644
--- a/qa/rpc-tests/test_framework/util.py
+++ b/qa/rpc-tests/test_framework/util.py
@@ -17,8 +17,43 @@ import subprocess
 import time
 import re
 
-from authproxy import AuthServiceProxy, JSONRPCException
-from util import *
+from . import coverage
+from .authproxy import AuthServiceProxy, JSONRPCException
+
+COVERAGE_DIR = None
+
+
+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):
+    """
+    Args:
+        url (str): URL of the RPC server to call
+        node_number (int): the node number (or id) that this calls to
+
+    Kwargs:
+        timeout (int): HTTP timeout in seconds
+
+    Returns:
+        AuthServiceProxy. convenience object for making RPC calls.
+
+    """
+    proxy_kwargs = {}
+    if timeout is not None:
+        proxy_kwargs['timeout'] = timeout
+
+    proxy = AuthServiceProxy(url, **proxy_kwargs)
+    proxy.url = url  # store URL on proxy for info
+
+    coverage_logfile = coverage.get_filename(
+        COVERAGE_DIR, node_number) if COVERAGE_DIR else None
+
+    return coverage.AuthServiceProxyWrapper(proxy, coverage_logfile)
+
 
 def p2p_port(n):
     return 11000 + n + os.getpid()%999
@@ -79,13 +114,13 @@ def initialize_chain(test_dir):
     """
 
     if (not os.path.isdir(os.path.join("cache","node0"))
-        or not os.path.isdir(os.path.join("cache","node1")) 
-        or not os.path.isdir(os.path.join("cache","node2")) 
+        or not os.path.isdir(os.path.join("cache","node1"))
+        or not os.path.isdir(os.path.join("cache","node2"))
         or not os.path.isdir(os.path.join("cache","node3"))):
 
         #find and delete old cache directories if any exist
         for i in range(4):
-            if os.path.isdir(os.path.join("cache","node"+str(i))): 
+            if os.path.isdir(os.path.join("cache","node"+str(i))):
                 shutil.rmtree(os.path.join("cache","node"+str(i)))
 
         devnull = open(os.devnull, "w")
@@ -103,11 +138,13 @@ def initialize_chain(test_dir):
             if os.getenv("PYTHON_DEBUG", ""):
                 print "initialize_chain: bitcoin-cli -rpcwait getblockcount completed"
         devnull.close()
+
         rpcs = []
+
         for i in range(4):
             try:
-                url = "http://rt:rt@127.0.0.1:%d"%(rpc_port(i),)
-                rpcs.append(AuthServiceProxy(url))
+                url = "http://rt:rt@127.0.0.1:%d" % (rpc_port(i),)
+                rpcs.append(get_rpc_proxy(url, i))
             except:
                 sys.stderr.write("Error connecting to "+url+"\n")
                 sys.exit(1)
@@ -190,11 +227,12 @@ def start_node(i, dirname, extra_args=None, rpchost=None, timewait=None, binary=
         print "start_node: calling bitcoin-cli -rpcwait getblockcount returned"
     devnull.close()
     url = "http://rt:rt@%s:%d" % (rpchost or '127.0.0.1', rpc_port(i))
-    if timewait is not None:
-        proxy = AuthServiceProxy(url, timeout=timewait)
-    else:
-        proxy = AuthServiceProxy(url)
-    proxy.url = url # store URL on proxy for info
+
+    proxy = get_rpc_proxy(url, i, timeout=timewait)
+
+    if COVERAGE_DIR:
+        coverage.write_all_rpc_commands(COVERAGE_DIR, proxy)
+
     return proxy
 
 def start_nodes(num_nodes, dirname, extra_args=None, rpchost=None, binary=None):