Merge #12718: [Tests] Require exact match in assert_start_raises_init_eror (jnewbery)

fae1374 qa: Allow for partial_match when checking init error (MarcoFalke)
5812273 [Tests] Require exact match in assert_start_raises_init_eror() (John Newbery)
0ec08a6 [Tests] Move assert_start_raises_init_error method to TestNode (John Newbery)

Pull request description:

  Extracted from #12379, because the changes are important on their own.

  This allows for exact testing, since the match can be specified with a strict regex. Internal details (such as exact formatting of the error message) can still be fuzzed away by regex wildcards.

Tree-SHA512: 605d2c9c42362a32d42321b066637577a026d0bb8cfc1c9f5737a4ca6503ffe85457a5122cea6e1101053ccc6c8aa1bbae3602e1fa7d2988bf7d5c275f412f66

7 files changed, 66 insertions, 43 deletions

diff --git a/test/functional/feature_config_args.py b/test/functional/feature_config_args.py
index c6cec0596b..0c36547054 100755
--- a/test/functional/feature_config_args.py
+++ b/test/functional/feature_config_args.py
@@ -5,6 +5,7 @@
 """Test various command line arguments and configuration file parameters."""
 
 import os
+import re
 
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import get_datadir_path
@@ -25,13 +26,13 @@ class ConfArgsTest(BitcoinTestFramework):
 
         # Check that using -datadir argument on non-existent directory fails
         self.nodes[0].datadir = new_data_dir
-        self.assert_start_raises_init_error(0, ['-datadir='+new_data_dir], 'Error: Specified data directory "' + new_data_dir + '" does not exist.')
+        self.nodes[0].assert_start_raises_init_error(['-datadir=' + new_data_dir], 'Error: Specified data directory "' + re.escape(new_data_dir) + '" does not exist.')
 
         # Check that using non-existent datadir in conf file fails
         conf_file = os.path.join(default_data_dir, "bitcoin.conf")
         with open(conf_file, 'a', encoding='utf8') as f:
             f.write("datadir=" + new_data_dir + "\n")
-        self.assert_start_raises_init_error(0, ['-conf='+conf_file], 'Error reading configuration file: specified data directory "' + new_data_dir + '" does not exist.')
+        self.nodes[0].assert_start_raises_init_error(['-conf=' + conf_file], 'Error reading configuration file: specified data directory "' + re.escape(new_data_dir) + '" does not exist.')
 
         # Create the directory and ensure the config file now works
         os.mkdir(new_data_dir)
diff --git a/test/functional/feature_logging.py b/test/functional/feature_logging.py
index da4e7b0398..a4ebc7cca3 100755
--- a/test/functional/feature_logging.py
+++ b/test/functional/feature_logging.py
@@ -30,8 +30,8 @@ class LoggingTest(BitcoinTestFramework):
         invdir = os.path.join(self.nodes[0].datadir, "regtest", "foo")
         invalidname = os.path.join("foo", "foo.log")
         self.stop_node(0)
-        self.assert_start_raises_init_error(0, ["-debuglogfile=%s" % (invalidname)],
-                                                "Error: Could not open debug log file")
+        exp_stderr = "Error: Could not open debug log file \S+$"
+        self.nodes[0].assert_start_raises_init_error(["-debuglogfile=%s" % (invalidname)], exp_stderr)
         assert not os.path.isfile(os.path.join(invdir, "foo.log"))
 
         # check that invalid log (relative) works after path exists
@@ -44,8 +44,7 @@ class LoggingTest(BitcoinTestFramework):
         self.stop_node(0)
         invdir = os.path.join(self.options.tmpdir, "foo")
         invalidname = os.path.join(invdir, "foo.log")
-        self.assert_start_raises_init_error(0, ["-debuglogfile=%s" % invalidname],
-                                               "Error: Could not open debug log file")
+        self.nodes[0].assert_start_raises_init_error(["-debuglogfile=%s" % invalidname], exp_stderr)
         assert not os.path.isfile(os.path.join(invdir, "foo.log"))
 
         # check that invalid log (absolute) works after path exists
diff --git a/test/functional/feature_uacomment.py b/test/functional/feature_uacomment.py
index bc3791508a..c73bdcfbb8 100755
--- a/test/functional/feature_uacomment.py
+++ b/test/functional/feature_uacomment.py
@@ -4,9 +4,12 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Test the -uacomment option."""
 
+import re
+
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import assert_equal
 
+
 class UacommentTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 1
@@ -23,13 +26,14 @@ class UacommentTest(BitcoinTestFramework):
 
         self.log.info("test -uacomment max length")
         self.stop_node(0)
-        expected = "exceeds maximum length (256). Reduce the number or size of uacomments."
-        self.assert_start_raises_init_error(0, ["-uacomment=" + 'a' * 256], expected)
+        expected = "Error: Total length of network version string \([0-9]+\) exceeds maximum length \(256\). Reduce the number or size of uacomments."
+        self.nodes[0].assert_start_raises_init_error(["-uacomment=" + 'a' * 256], expected)
 
         self.log.info("test -uacomment unsafe characters")
         for unsafe_char in ['/', ':', '(', ')']:
-            expected = "User Agent comment (" + unsafe_char + ") contains unsafe characters"
-            self.assert_start_raises_init_error(0, ["-uacomment=" + unsafe_char], expected)
+            expected = "Error: User Agent comment \(" + re.escape(unsafe_char) + "\) contains unsafe characters."
+            self.nodes[0].assert_start_raises_init_error(["-uacomment=" + unsafe_char], expected)
+
 
 if __name__ == '__main__':
     UacommentTest().main()
diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py
index 8233087faa..1b5e1a594b 100755
--- a/test/functional/test_framework/test_framework.py
+++ b/test/functional/test_framework/test_framework.py
@@ -281,27 +281,6 @@ class BitcoinTestFramework():
         self.stop_node(i)
         self.start_node(i, extra_args)
 
-    def assert_start_raises_init_error(self, i, extra_args=None, expected_msg=None, *args, **kwargs):
-        with tempfile.SpooledTemporaryFile(max_size=2**16) as log_stderr:
-            try:
-                self.start_node(i, extra_args, stderr=log_stderr, *args, **kwargs)
-                self.stop_node(i)
-            except Exception as e:
-                assert 'bitcoind exited' in str(e)  # node must have shutdown
-                self.nodes[i].running = False
-                self.nodes[i].process = None
-                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.nodes[i].process.wait(timeout)
 
diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py
index 13f5d224c7..8257219ffd 100755
--- a/test/functional/test_framework/test_node.py
+++ b/test/functional/test_framework/test_node.py
@@ -12,6 +12,7 @@ import logging
 import os
 import re
 import subprocess
+import tempfile
 import time
 
 from .authproxy import JSONRPCException
@@ -165,6 +166,41 @@ class TestNode():
     def wait_until_stopped(self, timeout=BITCOIND_PROC_WAIT_TIMEOUT):
         wait_until(self.is_node_stopped, timeout=timeout)
 
+    def assert_start_raises_init_error(self, extra_args=None, expected_msg=None, partial_match=False, *args, **kwargs):
+        """Attempt to start the node and expect it to raise an error.
+
+        extra_args: extra arguments to pass through to bitcoind
+        expected_msg: regex that stderr should match when bitcoind fails
+
+        Will throw if bitcoind starts without an error.
+        Will throw if an expected_msg is provided and it does not match bitcoind's stdout."""
+        with tempfile.SpooledTemporaryFile(max_size=2**16) as log_stderr:
+            try:
+                self.start(extra_args, stderr=log_stderr, *args, **kwargs)
+                self.wait_for_rpc_connection()
+                self.stop_node()
+                self.wait_util_stopped()
+            except Exception as e:
+                assert 'bitcoind exited' in str(e)  # node must have shutdown
+                self.running = False
+                self.process = None
+                # Check stderr for expected message
+                if expected_msg is not None:
+                    log_stderr.seek(0)
+                    stderr = log_stderr.read().decode('utf-8').strip()
+                    if partial_match:
+                        if re.search(expected_msg, stderr, flags=re.MULTILINE) is None:
+                            raise AssertionError('Expected message "{}" does not partially match stderr:\n"{}"'.format(expected_msg, stderr))
+                    else:
+                        if re.fullmatch(expected_msg, stderr) is None:
+                            raise AssertionError('Expected message "{}" does not fully match stderr:\n"{}"'.format(expected_msg, 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 node_encrypt_wallet(self, passphrase):
         """"Encrypts the wallet.
 
diff --git a/test/functional/wallet_hd.py b/test/functional/wallet_hd.py
index 91f77dd5ba..2f6e2852ac 100755
--- a/test/functional/wallet_hd.py
+++ b/test/functional/wallet_hd.py
@@ -23,7 +23,7 @@ class WalletHDTest(BitcoinTestFramework):
 
         # Make sure can't switch off usehd after wallet creation
         self.stop_node(1)
-        self.assert_start_raises_init_error(1, ['-usehd=0'], 'already existing HD wallet')
+        self.nodes[1].assert_start_raises_init_error(['-usehd=0'], "Error: Error loading : You can't disable HD on an already existing HD wallet")
         self.start_node(1)
         connect_nodes_bi(self.nodes, 0, 1)
 
diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py
index 378c06ee59..cf6d1274b5 100755
--- a/test/functional/wallet_multiwallet.py
+++ b/test/functional/wallet_multiwallet.py
@@ -7,6 +7,7 @@
 Verify that a bitcoind node can load multiple wallet files
 """
 import os
+import re
 import shutil
 
 from test_framework.test_framework import BitcoinTestFramework
@@ -60,29 +61,31 @@ class MultiWalletTest(BitcoinTestFramework):
                 assert_equal(os.path.isfile(wallet_dir(wallet_name)), True)
 
         # should not initialize if wallet path can't be created
-        self.assert_start_raises_init_error(0, ['-wallet=wallet.dat/bad'], 'Not a directory')
+        exp_stderr = "boost::filesystem::create_directory: (The system cannot find the path specified|Not a directory):"
+        self.nodes[0].assert_start_raises_init_error(['-wallet=wallet.dat/bad'], exp_stderr, partial_match=True)
 
-        self.assert_start_raises_init_error(0, ['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" does not exist')
-        self.assert_start_raises_init_error(0, ['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" is a relative path', cwd=data_dir())
-        self.assert_start_raises_init_error(0, ['-walletdir=debug.log'], 'Error: Specified -walletdir "debug.log" is not a directory', cwd=data_dir())
+        self.nodes[0].assert_start_raises_init_error(['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" does not exist')
+        self.nodes[0].assert_start_raises_init_error(['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" is a relative path', cwd=data_dir())
+        self.nodes[0].assert_start_raises_init_error(['-walletdir=debug.log'], 'Error: Specified -walletdir "debug.log" is not a directory', cwd=data_dir())
 
         # should not initialize if there are duplicate wallets
-        self.assert_start_raises_init_error(0, ['-wallet=w1', '-wallet=w1'], 'Error loading wallet w1. Duplicate -wallet filename specified.')
+        self.nodes[0].assert_start_raises_init_error(['-wallet=w1', '-wallet=w1'], 'Error: Error loading wallet w1. Duplicate -wallet filename specified.')
 
         # should not initialize if one wallet is a copy of another
         shutil.copyfile(wallet_dir('w8'), wallet_dir('w8_copy'))
-        self.assert_start_raises_init_error(0, ['-wallet=w8', '-wallet=w8_copy'], 'duplicates fileid')
+        exp_stderr = "CDB: Can't open database w8_copy \(duplicates fileid \w+ from w8\)"
+        self.nodes[0].assert_start_raises_init_error(['-wallet=w8', '-wallet=w8_copy'], exp_stderr, partial_match=True)
 
         # should not initialize if wallet file is a symlink
         os.symlink('w8', wallet_dir('w8_symlink'))
-        self.assert_start_raises_init_error(0, ['-wallet=w8_symlink'], 'Invalid -wallet path')
+        self.nodes[0].assert_start_raises_init_error(['-wallet=w8_symlink'], 'Error: Invalid -wallet path \'w8_symlink\'\. .*')
 
         # should not initialize if the specified walletdir does not exist
-        self.assert_start_raises_init_error(0, ['-walletdir=bad'], 'Error: Specified -walletdir "bad" does not exist')
+        self.nodes[0].assert_start_raises_init_error(['-walletdir=bad'], 'Error: Specified -walletdir "bad" does not exist')
         # should not initialize if the specified walletdir is not a directory
         not_a_dir = wallet_dir('notadir')
         open(not_a_dir, 'a').close()
-        self.assert_start_raises_init_error(0, ['-walletdir=' + not_a_dir], 'Error: Specified -walletdir "' + not_a_dir + '" is not a directory')
+        self.nodes[0].assert_start_raises_init_error(['-walletdir=' + not_a_dir], 'Error: Specified -walletdir "' + re.escape(not_a_dir) + '" is not a directory')
 
         # if wallets/ doesn't exist, datadir should be the default wallet dir
         wallet_dir2 = data_dir('walletdir')
@@ -102,8 +105,9 @@ class MultiWalletTest(BitcoinTestFramework):
 
         competing_wallet_dir = os.path.join(self.options.tmpdir, 'competing_walletdir')
         os.mkdir(competing_wallet_dir)
-        self.restart_node(0, ['-walletdir='+competing_wallet_dir])
-        self.assert_start_raises_init_error(1, ['-walletdir='+competing_wallet_dir], 'Error initializing wallet database environment')
+        self.restart_node(0, ['-walletdir=' + competing_wallet_dir])
+        exp_stderr = "Error: Error initializing wallet database environment \"\S+competing_walletdir\"!"
+        self.nodes[1].assert_start_raises_init_error(['-walletdir=' + competing_wallet_dir], exp_stderr, partial_match=True)
 
         self.restart_node(0, extra_args)