From a9deacc0102efccd3fccc53ea3cf10823143a8f9 Mon Sep 17 00:00:00 2001 From: PulpCattel Date: Tue, 10 Jan 2023 15:19:34 +0000 Subject: [PATCH] Refactor `conftest.py`: * Uses shlex.split() to avoid unnecessary handling of command lists * Removes unnecessary globals * PEP8 fixes * Uses `yield` instead of `addfinalizer` teardown * Add docstrings * Clean up functions --- conftest.py | 196 ++++++++++++++++++++++++++++------------------------ 1 file changed, 104 insertions(+), 92 deletions(-) diff --git a/conftest.py b/conftest.py index 3eba5d27d..1ded3adf3 100644 --- a/conftest.py +++ b/conftest.py @@ -1,51 +1,72 @@ -import pytest -import re import os -import time +import re import subprocess -from typing import Any +from shlex import split +from time import sleep +from typing import Any, Tuple + +import pytest -bitcoin_path = None -bitcoin_conf = None -bitcoin_rpcpassword = None -bitcoin_rpcusername = None -miniircd_procs = [] -def get_bitcoind_version(version_string: bytes) -> tuple: - # this utility function returns the version number - # as a tuple in the form (major, minor) +def get_bitcoind_version(bitcoind_path: str) -> Tuple[int, int]: + """ + This utility function returns the bitcoind version number + as a tuple in the form (major, minor) + """ + version = local_command(f'{bitcoind_path} -version') + if version.returncode != 0: + raise RuntimeError(version.stdout.decode('utf-8')) + version_string = version.stdout.split(b'\n')[0] version_tuple = re.match( - br'.*v(?P\d+)\.(?P\d+)', - version_string).groups() - return tuple(map(lambda x: int(x), version_tuple)) + br'.*v(?P\d+)\.(?P\d+)', version_string).groups() + major, minor = map(lambda x: int(x), version_tuple) + return major, minor + def local_command(command: str, bg: bool = False): + """ + Execute command in a new process. + """ + command = split(command) if bg: - #using subprocess.PIPE seems to cause problems + # using subprocess.PIPE seems to cause problems FNULL = open(os.devnull, 'w') return subprocess.Popen(command, stdout=FNULL, stderr=subprocess.STDOUT, close_fds=True) - else: - #in case of foreground execution, we can use the output; if not - #it doesn't matter - return subprocess.run(command, stdout=subprocess.PIPE, - stderr=subprocess.STDOUT) + # in case of foreground execution, we can use the output; if not + # it doesn't matter + return subprocess.run(command, stdout=subprocess.PIPE, + stderr=subprocess.STDOUT) + def root_path() -> str: - # returns the directory in which this file is contained + """ + Returns the directory in which this file is contained. + """ return os.path.dirname(os.path.realpath(__file__)) + +def btc_conf_test_path() -> str: + """ + Returns default Bitcoin conf test path. + """ + return os.path.join(root_path(), 'test/bitcoin.conf') + + def pytest_addoption(parser: Any) -> None: + """ + Pytest initialization hook to register argparse-style options. + """ parser.addoption("--btcroot", action="store", default='', - help="the fully qualified path to the directory containing "+\ - "the bitcoin binaries, e.g. /home/user/bitcoin/bin/") + help="the fully qualified path to the directory containing " + + "the bitcoin binaries, e.g. /home/user/bitcoin/bin/") parser.addoption("--btcconf", action="store", - default=os.path.join(root_path(), 'test/bitcoin.conf'), - help="the fully qualified path to the location of the "+\ - "bitcoin configuration file you use for testing, e.g. "+\ - "/home/user/.bitcoin/bitcoin.conf") + default=btc_conf_test_path(), + help="the fully qualified path to the location of the " + + "bitcoin configuration file you use for testing, e.g. " + + "/home/user/.bitcoin/bitcoin.conf") parser.addoption("--btcpwd", action="store", help="the RPC password for your test bitcoin instance") @@ -54,82 +75,73 @@ def pytest_addoption(parser: Any) -> None: default='bitcoinrpc', help="the RPC username for your test bitcoin instance (default=bitcoinrpc)") parser.addoption("--nirc", - type="int", - action="store", - default=1, - help="the number of local miniircd instances") - -def teardown() -> None: - #didn't find a stop command in miniircd, so just kill - global miniircd_procs - for m in miniircd_procs: - m.kill() - #shut down bitcoin and remove the regtest dir - local_command([os.path.join(bitcoin_path, "bitcoin-cli"), "-regtest", - "-rpcuser=" + bitcoin_rpcusername, - "-rpcpassword=" + bitcoin_rpcpassword, "stop"]) - #note, it is better to clean out ~/.bitcoin/regtest but too - #dangerous to automate it here perhaps + type="int", + action="store", + default=1, + help="the number of local miniircd instances") @pytest.fixture(scope="session", autouse=True) -def setup(request) -> None: - request.addfinalizer(teardown) - - global bitcoin_conf, bitcoin_path, bitcoin_rpcpassword, bitcoin_rpcusername - bitcoin_path = request.config.getoption("--btcroot") - bitcoin_conf = request.config.getoption("--btcconf") - print("Here is the bitcoin_conf path:") - print(bitcoin_conf) - bitcoin_rpcpassword = request.config.getoption("--btcpwd") - bitcoin_rpcusername = request.config.getoption("--btcuser") - - #start up miniircd - #minor bug in miniircd (seems); need *full* unqualified path for motd file +def setup_miniircd(pytestconfig): + """ + Setup miniircd and handle its clean up. + """ + miniircd_procs = [] cwd = os.getcwd() - n_irc = request.config.getoption("--nirc") - global miniircd_procs + n_irc = pytestconfig.getoption("--nirc") + miniircd_path = os.path.join(root_path(), 'miniircd', 'miniircd') + # minor bug in miniircd (seems); need *full* unqualified path for motd file + motd_path = os.path.join(cwd, 'miniircd', 'testmotd') for i in range(n_irc): - miniircd_proc = local_command( - ["./miniircd/miniircd", "--ports=" + str(16667+i), - "--motd=" + cwd + "/miniircd/testmotd"], - bg=True) + command = f"{miniircd_path} --ports={16667 + i} --motd={motd_path}" + miniircd_proc = local_command(command, bg=True) miniircd_procs.append(miniircd_proc) + yield + # didn't find a stop command in miniircd, so just kill + for m in miniircd_procs: + m.kill() - # determine bitcoind version - bitcoind_version_string = subprocess.check_output([ - os.path.join(bitcoin_path, "bitcoind"), "-version"]).split(b'\n')[0] - bitcoind_version = get_bitcoind_version(bitcoind_version_string) - - #start up regtest blockchain - bitcoin_args = ["-regtest", "-daemon", "-conf=" + bitcoin_conf] - btc_proc = subprocess.call([os.path.join(bitcoin_path, "bitcoind")] + - bitcoin_args) - root_cmd = [os.path.join(bitcoin_path, "bitcoin-cli"), "-regtest", - "-rpcuser=" + bitcoin_rpcusername, - "-rpcpassword=" + bitcoin_rpcpassword] +@pytest.fixture(scope="session", autouse=True) +def setup_regtest_bitcoind(pytestconfig): + """ + Setup regtest bitcoind and handle its clean up. + """ + conf = pytestconfig.getoption("--btcconf") + rpcuser = pytestconfig.getoption("--btcuser") + rpcpassword = pytestconfig.getoption("--btcpwd") + bitcoin_path = pytestconfig.getoption("--btcroot") + bitcoind_path = os.path.join(bitcoin_path, "bitcoind") + bitcoincli_path = os.path.join(bitcoin_path, "bitcoin-cli") + start_cmd = f'{bitcoind_path} -regtest -daemon -conf={conf}' + stop_cmd = f'{bitcoincli_path} -regtest -rpcuser={rpcuser} -rpcpassword={rpcpassword} stop' + local_command(start_cmd, bg=True) + # determine bitcoind version + try: + bitcoind_version = get_bitcoind_version(bitcoind_path) + except RuntimeError as exc: + pytest.exit(f"Cannot setup tests, bitcoind failing.\n{exc}") + root_cmd = f'{bitcoincli_path} -regtest -rpcuser={rpcuser} -rpcpassword={rpcpassword}' + wallet_name = 'jm-test-wallet' # Bitcoin Core v0.21+ does not create default wallet # From Bitcoin Core 0.21.0 there is support for descriptor wallets, which # are default from 23.x+ (including 22.99.0 development versions). # We don't support descriptor wallets yet. if bitcoind_version[0] >= 22: - local_command(root_cmd + ["-rpcwait"] + ["-named"] + - ["createwallet", - "wallet_name=jm-test-wallet", "descriptors=false"]) + create_wallet = f'{root_cmd} -rpcwait -named createwallet wallet_name={wallet_name} descriptors=false' else: - local_command(root_cmd + ["-rpcwait"] + - ["createwallet", "jm-test-wallet"]) - local_command(root_cmd + ["loadwallet", "jm-test-wallet"]) + create_wallet = f'{root_cmd} -rpcwait createwallet {wallet_name}' + local_command(create_wallet) + local_command(f'{root_cmd} loadwallet {wallet_name}') for i in range(2): - cpe = local_command(root_cmd + ["-rpcwallet=jm-test-wallet"] + - ["getnewaddress"]) - if cpe.returncode == 0: - destn_addr = cpe.stdout[:-1].decode('utf-8') - local_command(root_cmd + ["-rpcwallet=jm-test-wallet"] + - ["generatetoaddress", "301", destn_addr]) - else: - pytest.exit("Cannot setup tests, bitcoin-cli failing.\n" + - str(cpe.stdout)) - time.sleep(1) - + cpe = local_command(f'{root_cmd} -rpcwallet={wallet_name} getnewaddress') + if cpe.returncode != 0: + pytest.exit(f"Cannot setup tests, bitcoin-cli failing.\n{cpe.stdout.decode('utf-8')}") + destn_addr = cpe.stdout[:-1].decode('utf-8') + local_command(f'{root_cmd} -rpcwallet={wallet_name} generatetoaddress 301 {destn_addr}') + sleep(1) + yield + # shut down bitcoind + local_command(stop_cmd) + # note, it is better to clean out ~/.bitcoin/regtest but too + # dangerous to automate it here perhaps