Merge bitcoin#28392: test: Use pathlib over os path

bfa0bd6  test: Use pathlib over os.path bitcoin#28362 (ns-xvrn)

Pull request description:

  In reference to issue  bitcoin#28362  refactoring of functional tests to use pathlib over os.path to reduce verbosity and increase the intuitiveness of managing file access.

ACKs for top commit:
  maflcko:
    re-ACK  bfa0bd6  🐨
  willcl-ark:
    ACK bfa0bd6

Tree-SHA512: fb0833c4039d09758796514e47567a93ac831cb0776ff1a7ed8299792ad132e83282ef80bea098a88ae1551d906f2a56093d3e8de240412f9080735d00d496d9

Author: fanquake

test/functional/feature_blocksdir.py

@@ -5,8 +5,8 @@
 """Test the blocksdir option.
 """
 
-import os
 import shutil
+from pathlib import Path
 
 from test_framework.test_framework import BitcoinTestFramework, initialize_datadir
 
@@ -18,20 +18,20 @@ def set_test_params(self):
 
     def run_test(self):
         self.stop_node(0)
-        assert os.path.isdir(os.path.join(self.nodes[0].blocks_path))
-        assert not os.path.isdir(os.path.join(self.nodes[0].datadir, "blocks"))
-        shutil.rmtree(self.nodes[0].datadir)
+        assert self.nodes[0].blocks_path.is_dir()
+        assert not (self.nodes[0].datadir_path / "blocks").is_dir()
+        shutil.rmtree(self.nodes[0].datadir_path)
         initialize_datadir(self.options.tmpdir, 0, self.chain)
         self.log.info("Starting with nonexistent blocksdir ...")
-        blocksdir_path = os.path.join(self.options.tmpdir, 'blocksdir')
+        blocksdir_path = Path(self.options.tmpdir) / 'blocksdir'
         self.nodes[0].assert_start_raises_init_error([f"-blocksdir={blocksdir_path}"], f'Error: Specified blocks directory "{blocksdir_path}" does not exist.')
-        os.mkdir(blocksdir_path)
+        blocksdir_path.mkdir()
         self.log.info("Starting with existing blocksdir ...")
         self.start_node(0, [f"-blocksdir={blocksdir_path}"])
         self.log.info("mining blocks..")
         self.generatetoaddress(self.nodes[0], 10, self.nodes[0].get_deterministic_priv_key().address)
-        assert os.path.isfile(os.path.join(blocksdir_path, self.chain, "blocks", "blk00000.dat"))
-        assert os.path.isdir(os.path.join(self.nodes[0].blocks_path, "index"))
+        assert (blocksdir_path / self.chain / "blocks" / "blk00000.dat").is_file()
+        assert (self.nodes[0].blocks_path / "index").is_dir()
 
 
 if __name__ == '__main__':

test/functional/feature_config_args.py

@@ -5,7 +5,7 @@
 """Test various command line arguments and configuration file parameters."""
 
 import os
-import pathlib
+from pathlib import Path
 import re
 import sys
 import tempfile
@@ -39,8 +39,8 @@ def test_config_file_parser(self):
             extra_args=[f'-conf={bad_conf_file_path}'],
             expected_msg=conf_in_config_file_err,
         )
-        inc_conf_file_path = os.path.join(self.nodes[0].datadir, 'include.conf')
-        with open(os.path.join(self.nodes[0].datadir, 'bitcoin.conf'), 'a', encoding='utf-8') as conf:
+        inc_conf_file_path = self.nodes[0].datadir_path / 'include.conf'
+        with open(self.nodes[0].datadir_path / 'bitcoin.conf', 'a', encoding='utf-8') as conf:
             conf.write(f'includeconf={inc_conf_file_path}\n')
         with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
             conf.write('conf=some.conf\n')
@@ -97,8 +97,8 @@ def test_config_file_parser(self):
             conf.write('server=1\nrpcuser=someuser\n[main]\nrpcpassword=some#pass')
         self.nodes[0].assert_start_raises_init_error(expected_msg='Error: Error reading configuration file: parse error on line 4, using # in rpcpassword can be ambiguous and should be avoided')
 
-        inc_conf_file2_path = os.path.join(self.nodes[0].datadir, 'include2.conf')
-        with open(os.path.join(self.nodes[0].datadir, 'bitcoin.conf'), 'a', encoding='utf-8') as conf:
+        inc_conf_file2_path = self.nodes[0].datadir_path / 'include2.conf'
+        with open(self.nodes[0].datadir_path / 'bitcoin.conf', 'a', encoding='utf-8') as conf:
             conf.write(f'includeconf={inc_conf_file2_path}\n')
 
         with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
@@ -123,15 +123,15 @@ def test_config_file_log(self):
 
         # Create a temporary directory that will be treated as the default data
         # directory by bitcoind.
-        env, default_datadir = util.get_temp_default_datadir(pathlib.Path(self.options.tmpdir, "test_config_file_log"))
+        env, default_datadir = util.get_temp_default_datadir(Path(self.options.tmpdir, "test_config_file_log"))
         default_datadir.mkdir(parents=True)
 
         # 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_text = node.bitcoinconf.read_text()
         conf_path = default_datadir / "bitcoin.conf"
-        conf_path.write_text(f"datadir={node.datadir}\n{conf_text}")
+        conf_path.write_text(f"datadir={node.datadir_path}\n{conf_text}")
 
         # Drop the node -datadir= argument during this test, because if it is
         # specified it would take precedence over the datadir setting in the
@@ -218,7 +218,8 @@ def test_networkactive(self):
 
     def test_seed_peers(self):
         self.log.info('Test seed peers')
-        default_data_dir = self.nodes[0].datadir
+        default_data_dir = self.nodes[0].datadir_path
+        peer_dat = default_data_dir / 'peers.dat'
         # Only regtest has no fixed seeds. To avoid connections to random
         # nodes, regtest is the only network where it is safe to enable
         # -fixedseeds in tests
@@ -229,7 +230,7 @@ def test_seed_peers(self):
         # We expect the node will use DNS Seeds, but Regtest mode does not have
         # any valid DNS seeds. So after 60 seconds, the node should fallback to
         # fixed seeds
-        assert not os.path.exists(os.path.join(default_data_dir, "peers.dat"))
+        assert not peer_dat.exists()
         start = int(time.time())
         with self.nodes[0].assert_debug_log(
                 expected_msgs=[
@@ -248,7 +249,7 @@ def test_seed_peers(self):
 
         # No peers.dat exists and -dnsseed=0
         # We expect the node will fallback immediately to fixed seeds
-        assert not os.path.exists(os.path.join(default_data_dir, "peers.dat"))
+        assert not peer_dat.exists()
         with self.nodes[0].assert_debug_log(expected_msgs=[
                 "Loaded 0 addresses from peers.dat",
                 "DNS seeding disabled",
@@ -260,7 +261,7 @@ def test_seed_peers(self):
 
         # No peers.dat exists and dns seeds are disabled.
         # We expect the node will not add fixed seeds when explicitly disabled.
-        assert not os.path.exists(os.path.join(default_data_dir, "peers.dat"))
+        assert not peer_dat.exists()
         with self.nodes[0].assert_debug_log(expected_msgs=[
                 "Loaded 0 addresses from peers.dat",
                 "DNS seeding disabled",
@@ -271,7 +272,7 @@ def test_seed_peers(self):
 
         # No peers.dat exists and -dnsseed=0, but a -addnode is provided
         # We expect the node will allow 60 seconds prior to using fixed seeds
-        assert not os.path.exists(os.path.join(default_data_dir, "peers.dat"))
+        assert not peer_dat.exists()
         start = int(time.time())
         with self.nodes[0].assert_debug_log(
                 expected_msgs=[
@@ -323,16 +324,16 @@ def test_ignored_conf(self):
                       '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")
+            temp_conf.write(f"datadir={node.datadir_path}\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_path}" contains a "bitcoin.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_path}/bitcoin.conf'])
         self.stop_node(0)
 
     def test_ignored_default_conf(self):
@@ -346,23 +347,23 @@ def test_ignored_default_conf(self):
 
         # Create a temporary directory that will be treated as the default data
         # directory by bitcoind.
-        env, default_datadir = util.get_temp_default_datadir(pathlib.Path(self.options.tmpdir, "home"))
+        env, default_datadir = util.get_temp_default_datadir(Path(self.options.tmpdir, "home"))
         default_datadir.mkdir(parents=True)
 
         # Write a bitcoin.conf file in the default data directory containing a
         # datadir= line pointing at the node datadir. This will trigger a
         # 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 / "bitcoin.conf").write_text(f"datadir={node.datadir_path}\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'Error: Data directory "{node.datadir_path}" contains a "bitcoin.conf" file which is ignored, because a '
             f'different configuration file "{default_datadir}/bitcoin.conf" from data directory "{default_datadir}" '
             f'is being used instead.') + r"[\s\S]*", env=env, match=ErrorMatch.FULL_REGEX)
         node.args = node_args
@@ -392,16 +393,16 @@ def run_test(self):
         # Remove the -datadir argument so it doesn't override the config file
         self.nodes[0].args = [arg for arg in self.nodes[0].args if not arg.startswith("-datadir")]
 
-        default_data_dir = self.nodes[0].datadir
-        new_data_dir = os.path.join(default_data_dir, 'newdatadir')
-        new_data_dir_2 = os.path.join(default_data_dir, 'newdatadir2')
+        default_data_dir = self.nodes[0].datadir_path
+        new_data_dir = default_data_dir / 'newdatadir'
+        new_data_dir_2 = default_data_dir / 'newdatadir2'
 
         # Check that using -datadir argument on non-existent directory fails
-        self.nodes[0].datadir = new_data_dir
+        self.nodes[0].datadir_path = new_data_dir
         self.nodes[0].assert_start_raises_init_error([f'-datadir={new_data_dir}'], f'Error: Specified data directory "{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")
+        conf_file = default_data_dir / "bitcoin.conf"
 
         # datadir needs to be set before [chain] section
         with open(conf_file, encoding='utf8') as f:
@@ -413,20 +414,20 @@ def run_test(self):
         self.nodes[0].assert_start_raises_init_error([f'-conf={conf_file}'], f'Error: Error reading configuration file: specified data directory "{new_data_dir}" does not exist.')
 
         # Check that an explicitly specified config file that cannot be opened fails
-        none_existent_conf_file = os.path.join(default_data_dir, "none_existent_bitcoin.conf")
-        self.nodes[0].assert_start_raises_init_error(['-conf=' + none_existent_conf_file], 'Error: Error reading configuration file: specified config file "' + none_existent_conf_file + '" could not be opened.')
+        none_existent_conf_file = default_data_dir / "none_existent_bitcoin.conf"
+        self.nodes[0].assert_start_raises_init_error(['-conf=' + f'{none_existent_conf_file}'], 'Error: Error reading configuration file: specified config file "' + f'{none_existent_conf_file}' + '" could not be opened.')
 
         # Create the directory and ensure the config file now works
-        os.mkdir(new_data_dir)
+        new_data_dir.mkdir()
         self.start_node(0, [f'-conf={conf_file}'])
         self.stop_node(0)
-        assert os.path.exists(os.path.join(new_data_dir, self.chain, 'blocks'))
+        assert (new_data_dir / self.chain / 'blocks').exists()
 
         # Ensure command line argument overrides datadir in conf
-        os.mkdir(new_data_dir_2)
-        self.nodes[0].datadir = new_data_dir_2
+        new_data_dir_2.mkdir()
+        self.nodes[0].datadir_path = new_data_dir_2
         self.start_node(0, [f'-datadir={new_data_dir_2}', f'-conf={conf_file}'])
-        assert os.path.exists(os.path.join(new_data_dir_2, self.chain, 'blocks'))
+        assert (new_data_dir_2 / self.chain / 'blocks').exists()
 
 
 if __name__ == '__main__':

test/functional/feature_filelock.py

@@ -28,7 +28,7 @@ def run_test(self):
 
         self.log.info("Check that we can't start a second bitcoind instance using the same datadir")
         expected_msg = f"Error: Cannot obtain a lock on data directory {datadir}. {self.config['environment']['PACKAGE_NAME']} is probably already running."
-        self.nodes[1].assert_start_raises_init_error(extra_args=[f'-datadir={self.nodes[0].datadir}', '-noserver'], expected_msg=expected_msg)
+        self.nodes[1].assert_start_raises_init_error(extra_args=[f'-datadir={self.nodes[0].datadir_path}', '-noserver'], expected_msg=expected_msg)
 
         if self.is_wallet_compiled():
             def check_wallet_filelock(descriptors):

test/functional/feature_loadblock.py

@@ -10,7 +10,7 @@
 in contrib/linearize.
 """
 
-import os
+from pathlib import Path
 import subprocess
 import sys
 import tempfile
@@ -32,10 +32,10 @@ def run_test(self):
         self.generate(self.nodes[0], COINBASE_MATURITY, sync_fun=self.no_op)
 
         # Parsing the url of our node to get settings for config file
-        data_dir = self.nodes[0].datadir
+        data_dir = self.nodes[0].datadir_path
         node_url = urllib.parse.urlparse(self.nodes[0].url)
-        cfg_file = os.path.join(data_dir, "linearize.cfg")
-        bootstrap_file = os.path.join(self.options.tmpdir, "bootstrap.dat")
+        cfg_file = data_dir / "linearize.cfg"
+        bootstrap_file = Path(self.options.tmpdir) / "bootstrap.dat"
         genesis_block = self.nodes[0].getblockhash(0)
         blocks_dir = self.nodes[0].blocks_path
         hash_list = tempfile.NamedTemporaryFile(dir=data_dir,
@@ -58,16 +58,16 @@ def run_test(self):
             cfg.write(f"hashlist={hash_list.name}\n")
 
         base_dir = self.config["environment"]["SRCDIR"]
-        linearize_dir = os.path.join(base_dir, "contrib", "linearize")
+        linearize_dir = Path(base_dir) / "contrib" / "linearize"
 
         self.log.info("Run linearization of block hashes")
-        linearize_hashes_file = os.path.join(linearize_dir, "linearize-hashes.py")
+        linearize_hashes_file = linearize_dir / "linearize-hashes.py"
         subprocess.run([sys.executable, linearize_hashes_file, cfg_file],
                        stdout=hash_list,
                        check=True)
 
         self.log.info("Run linearization of block data")
-        linearize_data_file = os.path.join(linearize_dir, "linearize-data.py")
+        linearize_data_file = linearize_dir / "linearize-data.py"
         subprocess.run([sys.executable, linearize_data_file, cfg_file],
                        check=True)
 

test/functional/feature_settings.py

@@ -6,7 +6,6 @@
 
 import json
 
-from pathlib import Path
 
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.test_node import ErrorMatch
@@ -21,8 +20,8 @@ def set_test_params(self):
 
     def run_test(self):
         node, = self.nodes
-        settings = Path(node.chain_path, "settings.json")
-        conf = Path(node.datadir, "bitcoin.conf")
+        settings = node.chain_path / "settings.json"
+        conf = node.datadir_path / "bitcoin.conf"
 
         # Assert empty settings file was created
         self.stop_node(0)
@@ -79,7 +78,7 @@ def run_test(self):
             self.stop_node(0)
 
         # Test alternate settings path
-        altsettings = Path(node.datadir, "altsettings.json")
+        altsettings = node.datadir_path / "altsettings.json"
         with altsettings.open("w") as fp:
             fp.write('{"key": "value"}')
         with node.assert_debug_log(expected_msgs=['Setting file arg: key = "value"']):

test/functional/interface_bitcoin_cli.py

@@ -94,7 +94,7 @@ def run_test(self):
         assert_equal(self.nodes[0].cli("-named", "echo", "arg0=0", "arg1=1", "arg2=2", "arg1=3").send_cli(), ['0', '3', '2'])
         assert_raises_rpc_error(-8, "Parameter args specified multiple times", self.nodes[0].cli("-named", "echo", "args=[0,1,2,3]", "4", "5", "6", ).send_cli)
 
-        user, password = get_auth_cookie(self.nodes[0].datadir, self.chain)
+        user, password = get_auth_cookie(self.nodes[0].datadir_path, self.chain)
 
         self.log.info("Test -stdinrpcpass option")
         assert_equal(BLOCKS, self.nodes[0].cli(f'-rpcuser={user}', '-stdinrpcpass', input=password).getblockcount())

test/functional/rpc_bind.py

@@ -56,7 +56,7 @@ def run_allowip_test(self, allow_ips, rpchost, rpcport):
         self.nodes[0].rpchost = None
         self.start_nodes([node_args])
         # connect to node through non-loopback interface
-        node = get_rpc_proxy(rpc_url(self.nodes[0].datadir, 0, self.chain, "%s:%d" % (rpchost, rpcport)), 0, coveragedir=self.options.coveragedir)
+        node = get_rpc_proxy(rpc_url(self.nodes[0].datadir_path, 0, self.chain, "%s:%d" % (rpchost, rpcport)), 0, coveragedir=self.options.coveragedir)
         node.getnetworkinfo()
         self.stop_nodes()
 

test/functional/rpc_dumptxoutset.py

@@ -4,7 +4,6 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Test the generation of UTXO snapshots using `dumptxoutset`.
 """
-from pathlib import Path
 
 from test_framework.blocktools import COINBASE_MATURITY
 from test_framework.test_framework import BitcoinTestFramework
@@ -29,7 +28,7 @@ def run_test(self):
 
         FILENAME = 'txoutset.dat'
         out = node.dumptxoutset(FILENAME)
-        expected_path = Path(node.datadir) / self.chain / FILENAME
+        expected_path = node.datadir_path / self.chain / FILENAME
 
         assert expected_path.is_file()
 

test/functional/test_framework/test_framework.py

@@ -1006,5 +1006,5 @@ def is_bdb_compiled(self):
         return self.config["components"].getboolean("USE_BDB")
 
     def has_blockfile(self, node, filenum: str):
-        blocksdir = os.path.join(node.datadir, self.chain, 'blocks', '')
-        return os.path.isfile(os.path.join(blocksdir, f"blk{filenum}.dat"))
+        blocksdir = node.datadir_path / self.chain / 'blocks'
+        return (blocksdir / f"blk{filenum}.dat").is_file()