Merge #13867: qa: Make extended tests pass on native Windows

fafe73a qa: Raise feature_help timeout to 5s (MarcoFalke)
faabd7b qa: Use files for stdout/stderr to support Windows (MarcoFalke)
facb56f qa: Run gen_rpcauth with sys.executable (MarcoFalke)
fada896 qa: Close stdout and stderr file when node stops (MarcoFalke)

Pull request description:

  ### qa: Close stdout and stderr file when node stops

  Since these files are potentially deleted by the test framework for cleanup, they should be closed first. Otherwise this will lead to errors on Windows when the tests finish successfully.

  Side note: After the patch, it is no longer possible to reopen the file on Windows (see https://docs.python.org/3/library/tempfile.html#tempfile.NamedTemporaryFile)

  ### qa: Run gen_rpcauth with sys.executable

  Similar to `test_runner.py`, the `sys.executable` needs to be passed down into subprocesses to pass on native Windows. (Should have no effect on Linux)

  ###   qa: Use files for stdout/stderr to support Windows

  It seems that using PIPE is not supported on Windows. Also, it is easier to just use the files that capture the stdout and stderr within the test node class.

Tree-SHA512: ec675012b10705978606b7fcbdb287c39a8e6e3732aae2fa4041d963a3c6993c6eac6a9a3cbd5479514e7d8017fe74c12235d1ed6fed2e8af8f3c71981e91864

Author: MarcoFalke

test/functional/feature_help.py

@@ -3,7 +3,6 @@
 # Distributed under the MIT software license, see the accompanying
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Verify that starting bitcoin with -h works as expected."""
-import subprocess
 
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import assert_equal
@@ -17,41 +16,47 @@ def setup_network(self):
         self.add_nodes(self.num_nodes)
         # Don't start the node
 
+    def get_node_output(self, *, ret_code_expected):
+        ret_code = self.nodes[0].process.wait(timeout=5)
+        assert_equal(ret_code, ret_code_expected)
+        self.nodes[0].stdout.seek(0)
+        self.nodes[0].stderr.seek(0)
+        out = self.nodes[0].stdout.read()
+        err = self.nodes[0].stderr.read()
+        self.nodes[0].stdout.close()
+        self.nodes[0].stderr.close()
+
+        # Clean up TestNode state
+        self.nodes[0].running = False
+        self.nodes[0].process = None
+        self.nodes[0].rpc_connected = False
+        self.nodes[0].rpc = None
+
+        return out, err
+
     def run_test(self):
         self.log.info("Start bitcoin with -h for help text")
-        self.nodes[0].start(extra_args=['-h'], stderr=subprocess.PIPE, stdout=subprocess.PIPE)
+        self.nodes[0].start(extra_args=['-h'])
         # Node should exit immediately and output help to stdout.
-        ret_code = self.nodes[0].process.wait(timeout=1)
-        assert_equal(ret_code, 0)
-        output = self.nodes[0].process.stdout.read()
+        output, _ = self.get_node_output(ret_code_expected=0)
         assert b'Options' in output
         self.log.info("Help text received: {} (...)".format(output[0:60]))
-        self.nodes[0].running = False
 
         self.log.info("Start bitcoin with -version for version information")
-        self.nodes[0].start(extra_args=['-version'], stderr=subprocess.PIPE, stdout=subprocess.PIPE)
+        self.nodes[0].start(extra_args=['-version'])
         # Node should exit immediately and output version to stdout.
-        ret_code = self.nodes[0].process.wait(timeout=1)
-        assert_equal(ret_code, 0)
-        output = self.nodes[0].process.stdout.read()
+        output, _ = self.get_node_output(ret_code_expected=0)
         assert b'version' in output
         self.log.info("Version text received: {} (...)".format(output[0:60]))
 
         # Test that arguments not in the help results in an error
         self.log.info("Start bitcoind with -fakearg to make sure it does not start")
-        self.nodes[0].start(extra_args=['-fakearg'], stderr=subprocess.PIPE, stdout=subprocess.PIPE)
+        self.nodes[0].start(extra_args=['-fakearg'])
         # Node should exit immediately and output an error to stderr
-        ret_code = self.nodes[0].process.wait(timeout=1)
-        assert_equal(ret_code, 1)
-        output = self.nodes[0].process.stderr.read()
+        _, output = self.get_node_output(ret_code_expected=1)
         assert b'Error parsing command line arguments' in output
         self.log.info("Error message received: {} (...)".format(output[0:60]))
 
-        # Clean up TestNode state
-        self.nodes[0].running = False
-        self.nodes[0].process = None
-        self.nodes[0].rpc_connected = False
-        self.nodes[0].rpc = None
 
 if __name__ == '__main__':
     HelpTest().main()

test/functional/rpc_users.py

@@ -18,6 +18,7 @@
 from random import SystemRandom
 import string
 import configparser
+import sys
 
 
 class HTTPBasicsTest(BitcoinTestFramework):
@@ -36,7 +37,7 @@ def setup_chain(self):
         config = configparser.ConfigParser()
         config.read_file(open(self.options.configfile))
         gen_rpcauth = config['environment']['RPCAUTH']
-        p = subprocess.Popen([gen_rpcauth, self.user], stdout=subprocess.PIPE, universal_newlines=True)
+        p = subprocess.Popen([sys.executable, gen_rpcauth, self.user], stdout=subprocess.PIPE, universal_newlines=True)
         lines = p.stdout.read().splitlines()
         rpcauth3 = lines[1]
         self.password = lines[3]

test/functional/test_framework/test_node.py

@@ -122,7 +122,7 @@ def __getattr__(self, name):
             assert self.rpc_connected and self.rpc is not None, self._node_msg("Error: no RPC connection")
             return getattr(self.rpc, name)
 
-    def start(self, extra_args=None, stdout=None, stderr=None, *args, **kwargs):
+    def start(self, extra_args=None, *, stdout=None, stderr=None, **kwargs):
         """Start the node."""
         if extra_args is None:
             extra_args = self.extra_args
@@ -143,7 +143,7 @@ def start(self, extra_args=None, stdout=None, stderr=None, *args, **kwargs):
         # add environment variable LIBC_FATAL_STDERR_=1 so that libc errors are written to stderr and not the terminal
         subp_env = dict(os.environ, LIBC_FATAL_STDERR_="1")
 
-        self.process = subprocess.Popen(self.args + extra_args, env=subp_env, stdout=stdout, stderr=stderr, *args, **kwargs)
+        self.process = subprocess.Popen(self.args + extra_args, env=subp_env, stdout=stdout, stderr=stderr, **kwargs)
 
         self.running = True
         self.log.debug("bitcoind started, waiting for RPC to come up")
@@ -200,6 +200,9 @@ def stop_node(self, expected_stderr=''):
         if stderr != expected_stderr:
             raise AssertionError("Unexpected stderr {} != {}".format(stderr, expected_stderr))
 
+        self.stdout.close()
+        self.stderr.close()
+
         del self.p2ps[:]
 
     def is_node_stopped(self):