test: Use TestNode *_path properties where possible

MarcoFalke · MarcoFalke · commit aaaa3aefbdfc · 2023-06-21T08:49:18.000+02: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 bitcoind AbortNode's.
 """
-
 from test_framework.test_framework import BitcoinTestFramework
-from test_framework.util import get_datadir_path
-import os
 
 
 class AbortNodeTest(BitcoinTestFramework):
@@ -26,10 +23,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, self.chain, '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
@@ -113,7 +113,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 / "bitcoin_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', 'bitcoin_main.conf')
+        main_conf_file_path = self.nodes[0].datadir_path / "bitcoin_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('acceptnonstdtxn=1\n')
diff --git a/test/functional/feature_filelock.py b/test/functional/feature_filelock.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.
 """Check that it's not possible to start a second bitcoind instance using the same datadir or wallet."""
-import os
 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 bitcoind 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 bitcoind instance using the same wallet")
                 if descriptors:
                     expected_msg = f"Error: SQLiteDatabase: Unable to obtain an exclusive lock on the database, is it being used by another instance of {self.config['environment']['PACKAGE_NAME']}?"
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 BitcoinTestFramework
 
+
 class IncludeConfTest(BitcoinTestFramework):
     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", "bitcoin.conf"), "a", encoding='utf8') as f:
+        with open(self.nodes[0].datadir_path / "bitcoin.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", "bitcoin.conf"), "a", encoding='utf8') as f:
+        with open(self.nodes[0].datadir_path / "bitcoin.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, self.chain, "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 BitcoinTestFramework
 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 BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
@@ -21,12 +18,12 @@ def set_test_params(self):
         self.disable_syscall_sandbox = True
 
     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
@@ -158,7 +158,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
@@ -7,11 +7,9 @@
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
-    get_datadir_path,
     str_to_b64str,
 )
 
-import os
 import http.client
 import urllib.parse
 import subprocess
@@ -38,8 +36,7 @@ def set_test_params(self):
         self.num_nodes = 2
         self.supports_cli = False
 
-    def setup_chain(self):
-        super().setup_chain()
+    def conf_setup(self):
         #Append rpcauth to bitcoin.conf before initialization
         self.rtpassword = "cA773lm788buwYe4g4WT+05pKyNruVKjQ25x3n0DQcM="
         rpcauth = "rpcauth=rt:93648e835a54c573682c2eb19f882535$7681e9c5b74bdd85e78166031d2058e1069b3ed7ed967c93fc63abba06f31144"
@@ -64,13 +61,15 @@ def setup_chain(self):
         rpcauth3 = lines[1]
         self.password = lines[3]
 
-        with open(os.path.join(get_datadir_path(self.options.tmpdir, 0), "bitcoin.conf"), 'a', encoding='utf8') as f:
+        with open(self.nodes[0].datadir_path / "bitcoin.conf", "a", encoding="utf8") as f:
             f.write(rpcauth + "\n")
             f.write(rpcauth2 + "\n")
             f.write(rpcauth3 + "\n")
-        with open(os.path.join(get_datadir_path(self.options.tmpdir, 1), "bitcoin.conf"), 'a', encoding='utf8') as f:
+        with open(self.nodes[1].datadir_path / "bitcoin.conf", "a", encoding="utf8") as f:
             f.write("rpcuser={}\n".format(self.rpcuser))
             f.write("rpcpassword={}\n".format(self.rpcpassword))
+        self.restart_node(0)
+        self.restart_node(1)
 
     def test_auth(self, node, user, password):
         self.log.info('Correct...')
@@ -86,6 +85,7 @@ def test_auth(self, node, user, password):
         assert_equal(401, call_with_auth(node, user + 'wrong', password + 'wrong').status)
 
     def run_test(self):
+        self.conf_setup()
         self.log.info('Check correctness of the rpcauth config option')
         url = urllib.parse.urlparse(self.nodes[0].url)
 
@@ -112,8 +112,7 @@ def run_test(self):
         self.nodes[0].assert_start_raises_init_error(expected_msg=init_error, extra_args=['-rpcauth=foo$bar$baz'])
 
         self.log.info('Check that failure to write cookie file will abort the node gracefully')
-        cookie_file = os.path.join(get_datadir_path(self.options.tmpdir, 0), self.chain, '.cookie.tmp')
-        os.mkdir(cookie_file)
+        (self.nodes[0].chain_path / ".cookie.tmp").mkdir()
         self.nodes[0].assert_start_raises_init_error(expected_msg=init_error)
 
 
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
