From 00a8b3f963f86da8f9825e507f60c76bfe1bb7f8 Mon Sep 17 00:00:00 2001
From: PiRK <pierrekn@gmail.com>
Date: Thu, 20 Jul 2023 08:52:33 +0200
Subject: [PATCH] [electrum] restart the Electrum ABC daemon with a fresh
 datadir for every test

Summary:
It is better if the various tests share as little state as possible, so make sure they all have their own Electrum ABC daemon with a fresh wallet and datadir.
Restrict the scope `fulcrum_service` fixture to `function`, create and delete the datadir for every test function.

This removes a global variable.

Also note that the `bitcoind_rpc_connection` already assigns the `_bitcoind` global variable the first time it is called, so there is no need to redo it in `fulcrum_service`.

It would be good to also restart/reset the bitcoind instance and the fulcrum instance between tests, but I was not able to find any `pytest-docker` documentation about how to do that. The indivudual tests can mine blocks and send funds from the `bitcoind` wallet, so there is still some shared state between tests.

Test Plan: `pytest electrum/electrumabc/tests/regtest`

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Differential Revision: https://reviews.bitcoinabc.org/D14293
---
 electrumabc/tests/regtest/util.py | 49 +++++++++++++------------------
 1 file changed, 20 insertions(+), 29 deletions(-)

diff --git a/electrumabc/tests/regtest/util.py b/electrumabc/tests/regtest/util.py
index 55a48d2fd625..8682a36e12e2 100644
--- a/electrumabc/tests/regtest/util.py
+++ b/electrumabc/tests/regtest/util.py
@@ -15,7 +15,6 @@
 from jsonrpcclient import parse as rpc_parse
 from jsonrpcclient import request
 
-_datadir = None
 _bitcoind = None
 
 SUPPORTED_PLATFORM = (
@@ -88,19 +87,18 @@ def bitcoind_rpc_connection() -> AuthServiceProxy:
 
 # Creates a temp directory on disk for wallet storage
 # Starts a daemon, creates and loads a wallet
-def start_ec_daemon() -> None:
+def start_ec_daemon(datadir: str) -> None:
     """
     Creates a temp directory on disk for wallet storage
     Starts a daemon, creates and loads a wallet
     """
-    assert _datadir is not None
-    default_wallet = os.path.join(_datadir, "default_wallet")
+    default_wallet = os.path.join(datadir, "default_wallet")
     subprocess.run(
         [
             ELECTRUMABC_COMMAND,
             "--regtest",
             "-D",
-            _datadir,
+            datadir,
             "-w",
             default_wallet,
             "daemon",
@@ -130,10 +128,10 @@ def start_ec_daemon() -> None:
     )
 
 
-def stop_ec_daemon() -> None:
+def stop_ec_daemon(datadir) -> None:
     """Stops the daemon"""
     subprocess.run(
-        [ELECTRUMABC_COMMAND, "--regtest", "-D", _datadir, "daemon", "stop"], check=True
+        [ELECTRUMABC_COMMAND, "--regtest", "-D", datadir, "daemon", "stop"], check=True
     )
 
 
@@ -159,13 +157,12 @@ def docker_compose_command() -> str:
     return "docker-compose"
 
 
-def make_electrum_data_dir():
+def make_tmp_electrum_data_dir() -> str:
     """Create a temporary directory with a regtest subdirectory, and copy the Electrum
     config file into it.
     The caller is responsible for deleting the temporary directory."""
-    global _datadir
-    _datadir = tempfile.mkdtemp()
-    os.mkdir(os.path.join(_datadir, "regtest"))
+    datadir = tempfile.mkdtemp()
+    os.mkdir(os.path.join(datadir, "regtest"))
     shutil.copyfile(
         os.path.join(
             ELECTRUM_ROOT,
@@ -175,30 +172,24 @@ def make_electrum_data_dir():
             "configs",
             "electrum-abc-config",
         ),
-        os.path.join(_datadir, "regtest", "config"),
+        os.path.join(datadir, "regtest", "config"),
     )
+    return datadir
 
 
-@pytest.fixture(scope="session")
+@pytest.fixture(scope="function")
 def fulcrum_service(docker_services: Any) -> Generator[None, None, None]:
     """Makes sure all services (bitcoind, fulcrum and the electrum daemon) are up and
     running, make a temporary data dir for Electrum ABC, delete it at the end of the
     test session.
     """
-    global _datadir
-    global _bitcoind
-    if _datadir is not None:
-        # After the first time the fixture is created nothing needs to be done, so this
-        # is a no-op for subsequent calls.
-        yield
-    else:
-        make_electrum_data_dir()
-        _bitcoind = bitcoind_rpc_connection()
-        poll_for_answer(FULCRUM_STATS_URL, expected_answer=("Controller.TxNum", 102))
+    electrum_datadir = make_tmp_electrum_data_dir()
+    bitcoind_rpc_connection()
+    poll_for_answer(FULCRUM_STATS_URL, expected_answer=("Controller.TxNum", 102))
 
-        try:
-            start_ec_daemon()
-            yield
-        finally:
-            stop_ec_daemon()
-            shutil.rmtree(_datadir)
+    try:
+        start_ec_daemon(electrum_datadir)
+        yield
+        stop_ec_daemon(electrum_datadir)
+    finally:
+        shutil.rmtree(electrum_datadir)