test: Use TestNode *_path properties where possible

MarcoFalke · janus · commit 5c4d177a1c94 · 2023-09-06T09:48:58.000+01:00
Seems odd to place the burden on test writers to hardcode the chain or
datadir path for the nodes under test.
diff --git a/test/functional/feature_abortnode.py b/test/functional/feature_abortnode.py
@@ -9,10 +9,7 @@
 - Mine a fork that requires disconnecting the tip.
 - Verify that BGLd AbortNode's.
 """
-
 from test_framework.test_framework import BGLTestFramework
-from test_framework.util import get_datadir_path
-import os
 
 class AbortNodeTest(BGLTestFramework):
     def set_test_params(self):
@@ -25,10 +22,9 @@ def setup_network(self):
 
     def run_test(self):
         self.generate(self.nodes[0], 3, sync_fun=self.no_op)
-        datadir = get_datadir_path(self.options.tmpdir, 0)
 
         # Deleting the undo file will result in reorg failure
-        os.unlink(os.path.join(datadir, 'regtest', 'blocks', 'rev00000.dat'))
+        (self.nodes[0].chain_path / "blocks" / "rev00000.dat").unlink()
 
         # Connecting to a node with a more work chain will trigger a reorg
         # attempt.
diff --git a/test/functional/feature_anchors.py b/test/functional/feature_anchors.py
@@ -20,9 +20,7 @@ def set_test_params(self):
         self.disable_autoconnect = False
 
     def run_test(self):
-        node_anchors_path = os.path.join(
-            self.nodes[0].datadir, "regtest", "anchors.dat"
-        )
+        node_anchors_path = self.nodes[0].chain_path / "anchors.dat"
 
         self.log.info("When node starts, check if anchors.dat doesn't exist")
         assert not os.path.exists(node_anchors_path)
diff --git a/test/functional/feature_asmap.py b/test/functional/feature_asmap.py
@@ -114,7 +114,7 @@ def test_empty_asmap(self):
 
     def run_test(self):
         self.node = self.nodes[0]
-        self.datadir = os.path.join(self.node.datadir, self.chain)
+        self.datadir = self.node.chain_path
         self.default_asmap = os.path.join(self.datadir, DEFAULT_ASMAP_FILENAME)
         self.asmap_raw = os.path.join(os.path.dirname(os.path.realpath(__file__)), ASMAP)
 
diff --git a/test/functional/feature_config_args.py b/test/functional/feature_config_args.py
@@ -32,7 +32,7 @@ def test_config_file_parser(self):
         self.stop_node(0)
 
         # Check that startup fails if conf= is set in bitcoin.conf or in an included conf file
-        bad_conf_file_path = os.path.join(self.options.tmpdir, 'node0', 'bitcoin_bad.conf')
+        bad_conf_file_path = self.nodes[0].datadir_path / "BGL_bad.conf"
         util.write_config(bad_conf_file_path, n=0, chain='', extra_config=f'conf=some.conf\n')
         conf_in_config_file_err = 'Error: Error reading configuration file: conf cannot be set in the configuration file; use includeconf= if you want to include additional config files'
         self.nodes[0].assert_start_raises_init_error(
@@ -75,7 +75,7 @@ def test_config_file_parser(self):
                 conf.write("wallet=foo\n")
             self.nodes[0].assert_start_raises_init_error(expected_msg=f'Error: Config setting for -wallet only applied on {self.chain} network when in [{self.chain}] section.')
 
-        main_conf_file_path = os.path.join(self.options.tmpdir, 'node0', 'BGL_main.conf')
+        main_conf_file_path = self.nodes[0].datadir_path / "BGL_main.conf"
         util.write_config(main_conf_file_path, n=0, chain='', extra_config=f'includeconf={inc_conf_file_path}\n')
         with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
             conf.write('regtest=0\n') # mainnet
@@ -130,8 +130,8 @@ def test_config_file_log(self):
         # Write a bitcoin.conf file in the default data directory containing a
         # datadir= line pointing at the node datadir.
         node = self.nodes[0]
-        conf_text = pathlib.Path(node.bitcoinconf).read_text()
-        conf_path = default_datadir / "bitcoin.conf"
+        conf_text = pathlib.Path(node.BGLconf).read_text()
+        conf_path = default_datadir / "BGL.conf"
         conf_path.write_text(f"datadir={node.datadir}\n{conf_text}")
 
         # Drop the node -datadir= argument during this test, because if it is
@@ -324,20 +324,20 @@ def test_connect_with_seednode(self):
                 self.restart_node(0, extra_args=[connect_arg, '-seednode=fakeaddress2'])
 
     def test_ignored_conf(self):
-        self.log.info('Test error is triggered when the datadir in use contains a bitcoin.conf file that would be ignored '
+        self.log.info('Test error is triggered when the datadir in use contains a BGL.conf file that would be ignored '
                       'because a conflicting -conf file argument is passed.')
         node = self.nodes[0]
         with tempfile.NamedTemporaryFile(dir=self.options.tmpdir, mode="wt", delete=False) as temp_conf:
             temp_conf.write(f"datadir={node.datadir}\n")
         node.assert_start_raises_init_error([f"-conf={temp_conf.name}"], re.escape(
-            f'Error: Data directory "{node.datadir}" contains a "bitcoin.conf" file which is ignored, because a '
+            f'Error: Data directory "{node.datadir}" contains a "BGL.conf" file which is ignored, because a '
             f'different configuration file "{temp_conf.name}" from command line argument "-conf={temp_conf.name}" '
             f'is being used instead.') + r"[\s\S]*", match=ErrorMatch.FULL_REGEX)
 
         # Test that passing a redundant -conf command line argument pointing to
         # the same bitcoin.conf that would be loaded anyway does not trigger an
         # error.
-        self.start_node(0, [f'-conf={node.datadir}/bitcoin.conf'])
+        self.start_node(0, [f'-conf={node.datadir}/BGL.conf'])
         self.stop_node(0)
 
     def test_ignored_default_conf(self):
@@ -346,8 +346,8 @@ def test_ignored_default_conf(self):
         if sys.platform == "win32":
             return
 
-        self.log.info('Test error is triggered when bitcoin.conf in the default data directory sets another datadir '
-                      'and it contains a different bitcoin.conf file that would be ignored')
+        self.log.info('Test error is triggered when BGL.conf in the default data directory sets another datadir '
+                      'and it contains a different BGL.conf file that would be ignored')
 
         # Create a temporary directory that will be treated as the default data
         # directory by bitcoind.
@@ -359,16 +359,16 @@ def test_ignored_default_conf(self):
         # startup error because the node datadir contains a different
         # bitcoin.conf that would be ignored.
         node = self.nodes[0]
-        (default_datadir / "bitcoin.conf").write_text(f"datadir={node.datadir}\n")
+        (default_datadir / "BGL.conf").write_text(f"datadir={node.datadir}\n")
 
         # Drop the node -datadir= argument during this test, because if it is
         # specified it would take precedence over the datadir setting in the
         # config file.
         node_args = node.args
         node.args = [arg for arg in node.args if not arg.startswith("-datadir=")]
         node.assert_start_raises_init_error([], re.escape(
-            f'Error: Data directory "{node.datadir}" contains a "bitcoin.conf" file which is ignored, because a '
-            f'different configuration file "{default_datadir}/bitcoin.conf" from data directory "{default_datadir}" '
+            f'Error: Data directory "{node.datadir}" contains a "BGL.conf" file which is ignored, because a '
+            f'different configuration file "{default_datadir}/BGL.conf" from data directory "{default_datadir}" '
             f'is being used instead.') + r"[\s\S]*", env=env, match=ErrorMatch.FULL_REGEX)
         node.args = node_args
 
diff --git a/test/functional/feature_filelock.py b/test/functional/feature_filelock.py
@@ -2,8 +2,7 @@
 # Copyright (c) 2018-2022 The Bitcoin Core developers
 # Distributed under the MIT software license, see the accompanying
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
-"""Check that it's not possible to start a second BGLd instance using the same datadir or wallet."""
-import os
+"""Check that it's not possible to start a second bitcoind instance using the same datadir or wallet."""
 import random
 import string
 
@@ -24,7 +23,7 @@ def setup_network(self):
         self.nodes[0].wait_for_rpc_connection()
 
     def run_test(self):
-        datadir = os.path.join(self.nodes[0].datadir, self.chain)
+        datadir = self.nodes[0].chain_path
         self.log.info(f"Using datadir {datadir}")
 
         self.log.info("Check that we can't start a second BGLd instance using the same datadir")
@@ -35,7 +34,7 @@ def run_test(self):
             def check_wallet_filelock(descriptors):
                 wallet_name = ''.join([random.choice(string.ascii_lowercase) for _ in range(6)])
                 self.nodes[0].createwallet(wallet_name=wallet_name, descriptors=descriptors)
-                wallet_dir = os.path.join(datadir, 'wallets')
+                wallet_dir = self.nodes[0].wallets_path
                 self.log.info("Check that we can't start a second BGLd instance using the same wallet")
                 if descriptors:
                     expected_msg = "Error: SQLiteDatabase: Unable to obtain an exclusive lock on the database, is it being used by another instance of Bitgesell Core?"
diff --git a/test/functional/feature_includeconf.py b/test/functional/feature_includeconf.py
@@ -14,27 +14,25 @@
 4. multiple includeconf arguments can be specified in the main config
    file.
 """
-import os
-
 from test_framework.test_framework import BGLTestFramework
 
+
 class IncludeConfTest(BGLTestFramework):
     def set_test_params(self):
         self.num_nodes = 1
 
-    def setup_chain(self):
-        super().setup_chain()
+    def run_test(self):
         # Create additional config files
         # - tmpdir/node0/relative.conf
-        with open(os.path.join(self.options.tmpdir, "node0", "relative.conf"), "w", encoding="utf8") as f:
+        with open(self.nodes[0].datadir_path / "relative.conf", "w", encoding="utf8") as f:
             f.write("uacomment=relative\n")
         # - tmpdir/node0/relative2.conf
-        with open(os.path.join(self.options.tmpdir, "node0", "relative2.conf"), "w", encoding="utf8") as f:
+        with open(self.nodes[0].datadir_path / "relative2.conf", "w", encoding="utf8") as f:
             f.write("uacomment=relative2\n")
-        with open(os.path.join(self.options.tmpdir, "node0", "BGL.conf"), "a", encoding='utf8') as f:
+        with open(self.nodes[0].datadir_path / "BGL.conf", "a", encoding="utf8") as f:
             f.write("uacomment=main\nincludeconf=relative.conf\n")
+        self.restart_node(0)
 
-    def run_test(self):
         self.log.info("-includeconf works from config file. subversion should end with 'main; relative)/'")
 
         subversion = self.nodes[0].getnetworkinfo()["subversion"]
@@ -52,7 +50,7 @@ def run_test(self):
         )
 
         self.log.info("-includeconf cannot be used recursively. subversion should end with 'main; relative)/'")
-        with open(os.path.join(self.options.tmpdir, "node0", "relative.conf"), "a", encoding="utf8") as f:
+        with open(self.nodes[0].datadir_path / "relative.conf", "a", encoding="utf8") as f:
             f.write("includeconf=relative2.conf\n")
         self.start_node(0)
 
@@ -63,20 +61,20 @@ def run_test(self):
         self.log.info("-includeconf cannot contain invalid arg")
 
         # Commented out as long as we ignore invalid arguments in configuration files
-        #with open(os.path.join(self.options.tmpdir, "node0", "relative.conf"), "w", encoding="utf8") as f:
+        #with open(self.nodes[0].datadir_path / "relative.conf", "w", encoding="utf8") as f:
         #    f.write("foo=bar\n")
         #self.nodes[0].assert_start_raises_init_error(expected_msg="Error: Error reading configuration file: Invalid configuration value foo")
 
         self.log.info("-includeconf cannot be invalid path")
-        os.remove(os.path.join(self.options.tmpdir, "node0", "relative.conf"))
+        (self.nodes[0].datadir_path / "relative.conf").unlink()
         self.nodes[0].assert_start_raises_init_error(expected_msg="Error: Error reading configuration file: Failed to include configuration file relative.conf")
 
         self.log.info("multiple -includeconf args can be used from the base config file. subversion should end with 'main; relative; relative2)/'")
-        with open(os.path.join(self.options.tmpdir, "node0", "relative.conf"), "w", encoding="utf8") as f:
+        with open(self.nodes[0].datadir_path / "relative.conf", "w", encoding="utf8") as f:
             # Restore initial file contents
             f.write("uacomment=relative\n")
 
-        with open(os.path.join(self.options.tmpdir, "node0", "BGL.conf"), "a", encoding='utf8') as f:
+        with open(self.nodes[0].datadir_path / "BGL.conf", "a", encoding="utf8") as f:
             f.write("includeconf=relative2.conf\n")
 
         self.start_node(0)
diff --git a/test/functional/feature_loadblock.py b/test/functional/feature_loadblock.py
@@ -37,7 +37,7 @@ def run_test(self):
         cfg_file = os.path.join(data_dir, "linearize.cfg")
         bootstrap_file = os.path.join(self.options.tmpdir, "bootstrap.dat")
         genesis_block = self.nodes[0].getblockhash(0)
-        blocks_dir = os.path.join(data_dir, "regtest", "blocks")
+        blocks_dir = self.nodes[0].chain_path / "blocks"
         hash_list = tempfile.NamedTemporaryFile(dir=data_dir,
                                                 mode='w',
                                                 delete=False,
diff --git a/test/functional/feature_posix_fs_permissions.py b/test/functional/feature_posix_fs_permissions.py
@@ -31,11 +31,11 @@ def check_file_permissions(self, file):
 
     def run_test(self):
         self.stop_node(0)
-        datadir = os.path.join(self.nodes[0].datadir, self.chain)
+        datadir = self.nodes[0].chain_path
         self.check_directory_permissions(datadir)
-        walletsdir = os.path.join(datadir, "wallets")
+        walletsdir = self.nodes[0].wallets_path
         self.check_directory_permissions(walletsdir)
-        debuglog = os.path.join(datadir, "debug.log")
+        debuglog = self.nodes[0].debug_log_path
         self.check_file_permissions(debuglog)
 
 
diff --git a/test/functional/feature_reindex.py b/test/functional/feature_reindex.py
@@ -10,7 +10,6 @@
 - Verify that out-of-order blocks are correctly processed, see LoadExternalBlockFile()
 """
 
-import os
 from test_framework.test_framework import BGLTestFramework
 from test_framework.p2p import MAGIC_BYTES
 from test_framework.util import assert_equal
@@ -39,7 +38,7 @@ def out_of_order(self):
         # In this test environment, blocks will always be in order (since
         # we're generating them rather than getting them from peers), so to
         # test out-of-order handling, swap blocks 1 and 2 on disk.
-        blk0 = os.path.join(self.nodes[0].datadir, self.nodes[0].chain, 'blocks', 'blk00000.dat')
+        blk0 = self.nodes[0].chain_path / "blocks" / "blk00000.dat"
         with open(blk0, 'r+b') as bf:
             # Read at least the first few blocks (including genesis)
             b = bf.read(2000)
diff --git a/test/functional/feature_remove_pruned_files_on_startup.py b/test/functional/feature_remove_pruned_files_on_startup.py
@@ -20,10 +20,10 @@ def mine_batches(self, blocks):
         self.sync_blocks()
 
     def run_test(self):
-        blk0 = os.path.join(self.nodes[0].datadir, self.nodes[0].chain, 'blocks', 'blk00000.dat')
-        rev0 = os.path.join(self.nodes[0].datadir, self.nodes[0].chain, 'blocks', 'rev00000.dat')
-        blk1 = os.path.join(self.nodes[0].datadir, self.nodes[0].chain, 'blocks', 'blk00001.dat')
-        rev1 = os.path.join(self.nodes[0].datadir, self.nodes[0].chain, 'blocks', 'rev00001.dat')
+        blk0 = self.nodes[0].chain_path / "blocks" / "blk00000.dat"
+        rev0 = self.nodes[0].chain_path / "blocks" / "rev00000.dat"
+        blk1 = self.nodes[0].chain_path / "blocks" / "blk00001.dat"
+        rev1 = self.nodes[0].chain_path / "blocks" / "rev00001.dat"
         self.mine_batches(800)
         fo1 = os.open(blk0, os.O_RDONLY)
         fo2 = os.open(rev1, os.O_RDONLY)
diff --git a/test/functional/feature_startupnotify.py b/test/functional/feature_startupnotify.py
@@ -3,9 +3,6 @@
 # Distributed under the MIT software license, see the accompanying
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Test -startupnotify."""
-
-import os
-
 from test_framework.test_framework import BGLTestFramework
 from test_framework.util import (
     assert_equal,
@@ -20,12 +17,12 @@ def set_test_params(self):
         self.num_nodes = 1
 
     def run_test(self):
-        tmpdir_file = os.path.join(self.options.tmpdir, NODE_DIR, FILE_NAME)
-        assert not os.path.exists(tmpdir_file)
+        tmpdir_file = self.nodes[0].datadir_path / FILE_NAME
+        assert not tmpdir_file.exists()
 
         self.log.info("Test -startupnotify command is run when node starts")
         self.restart_node(0, extra_args=[f"-startupnotify=echo '{FILE_NAME}' >> {NODE_DIR}/{FILE_NAME}"])
-        self.wait_until(lambda: os.path.exists(tmpdir_file))
+        self.wait_until(lambda: tmpdir_file.exists())
 
         self.log.info("Test -startupnotify is executed once")
 
diff --git a/test/functional/feature_txindex_compatibility.py b/test/functional/feature_txindex_compatibility.py
@@ -50,10 +50,10 @@ def run_test(self):
         self.nodes[0].getrawtransaction(txid=spend_utxo["txid"])  # Requires -txindex
 
         self.stop_nodes()
-        legacy_chain_dir = os.path.join(self.nodes[0].datadir, self.chain)
+        legacy_chain_dir = self.nodes[0].chain_path
 
         self.log.info("Migrate legacy txindex")
-        migrate_chain_dir = os.path.join(self.nodes[2].datadir, self.chain)
+        migrate_chain_dir = self.nodes[2].chain_path
         shutil.rmtree(migrate_chain_dir)
         shutil.copytree(legacy_chain_dir, migrate_chain_dir)
         with self.nodes[2].assert_debug_log([
@@ -64,7 +64,7 @@ def run_test(self):
         self.nodes[2].getrawtransaction(txid=spend_utxo["txid"])  # Requires -txindex
 
         self.log.info("Drop legacy txindex")
-        drop_index_chain_dir = os.path.join(self.nodes[1].datadir, self.chain)
+        drop_index_chain_dir = self.nodes[1].chain_path
         shutil.rmtree(drop_index_chain_dir)
         shutil.copytree(legacy_chain_dir, drop_index_chain_dir)
         self.nodes[1].assert_start_raises_init_error(
diff --git a/test/functional/p2p_message_capture.py b/test/functional/p2p_message_capture.py
@@ -58,7 +58,7 @@ def set_test_params(self):
         self.setup_clean_chain = True
 
     def run_test(self):
-        capturedir = os.path.join(self.nodes[0].datadir, "regtest/message_capture")
+        capturedir = self.nodes[0].chain_path / "message_capture"
         # Connect a node so that the handshake occurs
         self.nodes[0].add_p2p_connection(P2PDataStore())
         self.nodes[0].disconnect_p2ps()
diff --git a/test/functional/rpc_blockchain.py b/test/functional/rpc_blockchain.py
@@ -49,7 +49,6 @@
     assert_raises_rpc_error,
     assert_is_hex_string,
     assert_is_hash_string,
-    get_datadir_path,
 )
 from test_framework.wallet import MiniWallet
 
@@ -572,16 +571,15 @@ def assert_vin_does_not_contain_prevout(verbosity):
         self.log.info("Test that getblock with verbosity 3 includes prevout")
         assert_vin_contains_prevout(3)
 
-        self.log.info("Test that getblock with verbosity 2 and 3 still works with pruned Undo data")
-        datadir = get_datadir_path(self.options.tmpdir, 0)
-
         self.log.info("Test getblock with invalid verbosity type returns proper error message")
         assert_raises_rpc_error(-3, "JSON value of type string is not of expected type number", node.getblock, blockhash, "2")
 
+        self.log.info("Test that getblock with verbosity 2 and 3 still works with pruned Undo data")
+
         def move_block_file(old, new):
-            old_path = os.path.join(datadir, self.chain, 'blocks', old)
-            new_path = os.path.join(datadir, self.chain, 'blocks', new)
-            os.rename(old_path, new_path)
+            old_path = self.nodes[0].chain_path / "blocks" / old
+            new_path = self.nodes[0].chain_path / "blocks" / new
+            old_path.rename(new_path)
 
         # Move instead of deleting so we can restore chain state afterwards
         move_block_file('rev00000.dat', 'rev_wrong')
diff --git a/test/functional/rpc_createmultisig.py b/test/functional/rpc_createmultisig.py
@@ -157,7 +157,7 @@ def do_multisig(self):
                 try:
                     node1.loadwallet('wmulti')
                 except JSONRPCException as e:
-                    path = os.path.join(self.options.tmpdir, "node1", "regtest", "wallets", "wmulti")
+                    path = self.nodes[1].wallets_path / "wmulti"
                     if e.error['code'] == -18 and "Wallet file verification failed. Failed to load database path '{}'. Path does not exist.".format(path) in e.error['message']:
                         node1.createwallet(wallet_name='wmulti', disable_private_keys=True)
                     else:
diff --git a/test/functional/rpc_users.py b/test/functional/rpc_users.py
diff --git a/test/functional/rpc_whitelist.py b/test/functional/rpc_whitelist.py
diff --git a/test/functional/tool_wallet.py b/test/functional/tool_wallet.py
diff --git a/test/functional/wallet_backwards_compatibility.py b/test/functional/wallet_backwards_compatibility.py
diff --git a/test/functional/wallet_inactive_hdchains.py b/test/functional/wallet_inactive_hdchains.py
diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py
diff --git a/test/functional/wallet_upgradewallet.py b/test/functional/wallet_upgradewallet.py
