Merge #11125: Add bitcoin-cli -stdin and -stdinrpcpass functional tests

29e1dfb [test] Add bitcoin-cli -stdin and -stdinrpcpass functional tests (João Barbosa)
ce379b4 [test] Replace check_output with low level version (João Barbosa)
232e3e8 [test] Add assert_raises_process_error to assert process errors (João Barbosa)
5c18a84 [test] Add support for custom arguments to TestNodeCLI (João Barbosa)
e127494 [test] Improve assert_raises_jsonrpc docstring (João Barbosa)
7696841 Fix style in -stdin and -stdinrpcpass handling (João Barbosa)

Pull request description:

  This patch adds tests for `bitcoin-cli` options `-stdin` (#7550) and `-stdinrpcpass` #10997.

Tree-SHA512: fd8133f44876f2b5b41dfd3762b1988598f6b7bf13fb2385ad95876825d9c0b2b896ce4ea6eeb21012158e1f276907f155d37bb967198b609d2d3dddbfa334c1

Author: laanwj

src/bitcoin-cli.cpp

@@ -296,19 +296,22 @@ int CommandLineRPC(int argc, char *argv[])
         }
         std::string rpcPass;
         if (gArgs.GetBoolArg("-stdinrpcpass", false)) {
-            if(!std::getline(std::cin,rpcPass))
+            if (!std::getline(std::cin, rpcPass)) {
                 throw std::runtime_error("-stdinrpcpass specified but failed to read from standard input");
+            }
             gArgs.ForceSetArg("-rpcpassword", rpcPass);
         }
         std::vector<std::string> args = std::vector<std::string>(&argv[1], &argv[argc]);
         if (gArgs.GetBoolArg("-stdin", false)) {
             // Read one arg per line from stdin and append
             std::string line;
-            while (std::getline(std::cin,line))
+            while (std::getline(std::cin, line)) {
                 args.push_back(line);
+            }
         }
-        if (args.size() < 1)
+        if (args.size() < 1) {
             throw std::runtime_error("too few parameters (need at least command)");
+        }
         std::string strMethod = args[0];
         args.erase(args.begin()); // Remove trailing method name from arguments vector
 

test/functional/bitcoin_cli.py

@@ -4,7 +4,7 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Test bitcoin-cli"""
 from test_framework.test_framework import BitcoinTestFramework
-from test_framework.util import assert_equal
+from test_framework.util import assert_equal, assert_raises_process_error, get_auth_cookie
 
 class TestBitcoinCli(BitcoinTestFramework):
 
@@ -16,16 +16,24 @@ def run_test(self):
         """Main test logic"""
 
         self.log.info("Compare responses from gewalletinfo RPC and `bitcoin-cli getwalletinfo`")
-        cli_get_info = self.nodes[0].cli.getwalletinfo()
-        rpc_get_info = self.nodes[0].getwalletinfo()
-
-        assert_equal(cli_get_info, rpc_get_info)
+        cli_response = self.nodes[0].cli.getwalletinfo()
+        rpc_response = self.nodes[0].getwalletinfo()
+        assert_equal(cli_response, rpc_response)
 
         self.log.info("Compare responses from getblockchaininfo RPC and `bitcoin-cli getblockchaininfo`")
-        cli_get_info = self.nodes[0].cli.getblockchaininfo()
-        rpc_get_info = self.nodes[0].getblockchaininfo()
+        cli_response = self.nodes[0].cli.getblockchaininfo()
+        rpc_response = self.nodes[0].getblockchaininfo()
+        assert_equal(cli_response, rpc_response)
+
+        user, password = get_auth_cookie(self.nodes[0].datadir)
+
+        self.log.info("Test -stdinrpcpass option")
+        assert_equal(0, self.nodes[0].cli('-rpcuser=%s' % user, '-stdinrpcpass', input=password).getblockcount())
+        assert_raises_process_error(1, "incorrect rpcuser or rpcpassword", self.nodes[0].cli('-rpcuser=%s' % user, '-stdinrpcpass', input="foo").echo)
 
-        assert_equal(cli_get_info, rpc_get_info)
+        self.log.info("Test -stdin and -stdinrpcpass")
+        assert_equal(["foo", "bar"], self.nodes[0].cli('-rpcuser=%s' % user, '-stdin', '-stdinrpcpass', input=password + "\nfoo\nbar").echo())
+        assert_raises_process_error(1, "incorrect rpcuser or rpcpassword", self.nodes[0].cli('-rpcuser=%s' % user, '-stdin', '-stdinrpcpass', input="foo").echo)
 
 if __name__ == '__main__':
     TestBitcoinCli().main()

test/functional/test_framework/test_node.py

@@ -155,8 +155,16 @@ class TestNodeCLI():
     """Interface to bitcoin-cli for an individual node"""
 
     def __init__(self, binary, datadir):
+        self.args = []
         self.binary = binary
         self.datadir = datadir
+        self.input = None
+
+    def __call__(self, *args, input=None):
+        # TestNodeCLI is callable with bitcoin-cli command-line args
+        self.args = [str(arg) for arg in args]
+        self.input = input
+        return self
 
     def __getattr__(self, command):
         def dispatcher(*args, **kwargs):
@@ -169,9 +177,14 @@ def send_cli(self, command, *args, **kwargs):
         pos_args = [str(arg) for arg in args]
         named_args = [str(key) + "=" + str(value) for (key, value) in kwargs.items()]
         assert not (pos_args and named_args), "Cannot use positional arguments and named arguments in the same bitcoin-cli call"
-        p_args = [self.binary, "-datadir=" + self.datadir]
+        p_args = [self.binary, "-datadir=" + self.datadir] + self.args
         if named_args:
             p_args += ["-named"]
         p_args += [command] + pos_args + named_args
-        cli_output = subprocess.check_output(p_args, universal_newlines=True)
-        return json.loads(cli_output, parse_float=decimal.Decimal)
+        process = subprocess.Popen(p_args, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True)
+        cli_stdout, cli_stderr = process.communicate(input=self.input)
+        returncode = process.poll()
+        if returncode:
+            # Ignore cli_stdout, raise with cli_stderr
+            raise subprocess.CalledProcessError(returncode, self.binary, output=cli_stderr)
+        return json.loads(cli_stdout, parse_float=decimal.Decimal)

test/functional/test_framework/util.py

@@ -12,6 +12,7 @@
 import os
 import random
 import re
+from subprocess import CalledProcessError
 import time
 
 from . import coverage
@@ -57,18 +58,42 @@ def assert_raises_message(exc, message, fun, *args, **kwds):
     else:
         raise AssertionError("No exception raised")
 
+def assert_raises_process_error(returncode, output, fun, *args, **kwds):
+    """Execute a process and asserts the process return code and output.
+
+    Calls function `fun` with arguments `args` and `kwds`. Catches a CalledProcessError
+    and verifies that the return code and output are as expected. Throws AssertionError if
+    no CalledProcessError was raised or if the return code and output are not as expected.
+
+    Args:
+        returncode (int): the process return code.
+        output (string): [a substring of] the process output.
+        fun (function): the function to call. This should execute a process.
+        args*: positional arguments for the function.
+        kwds**: named arguments for the function.
+    """
+    try:
+        fun(*args, **kwds)
+    except CalledProcessError as e:
+        if returncode != e.returncode:
+            raise AssertionError("Unexpected returncode %i" % e.returncode)
+        if output not in e.output:
+            raise AssertionError("Expected substring not found:" + e.output)
+    else:
+        raise AssertionError("No exception raised")
+
 def assert_raises_jsonrpc(code, message, fun, *args, **kwds):
     """Run an RPC and verify that a specific JSONRPC exception code and message is raised.
 
     Calls function `fun` with arguments `args` and `kwds`. Catches a JSONRPCException
     and verifies that the error code and message are as expected. Throws AssertionError if
-    no JSONRPCException was returned or if the error code/message are not as expected.
+    no JSONRPCException was raised or if the error code/message are not as expected.
 
     Args:
         code (int), optional: the error code returned by the RPC call (defined
             in src/rpc/protocol.h). Set to None if checking the error code is not required.
         message (string), optional: [a substring of] the error string returned by the
-            RPC call. Set to None if checking the error string is not required
+            RPC call. Set to None if checking the error string is not required.
         fun (function): the function to call. This should be the name of an RPC.
         args*: positional arguments for the function.
         kwds**: named arguments for the function.