From adbfb90d5224c092a9464bfeb34e346d5b4a94bb Mon Sep 17 00:00:00 2001 From: everoddandeven Date: Thu, 9 Apr 2026 21:02:43 +0200 Subject: [PATCH 1/4] Fix lost notification and potential deadlock --- src/cpp/daemon/py_monero_daemon.h | 10 +++++++--- src/cpp/daemon/py_monero_daemon_rpc.cpp | 7 ++++--- tests/test_monero_connection_manager.py | 11 ++++++----- tests/utils/connection_change_collector.py | 8 +++++--- 4 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/cpp/daemon/py_monero_daemon.h b/src/cpp/daemon/py_monero_daemon.h index a52aa77..7325b45 100644 --- a/src/cpp/daemon/py_monero_daemon.h +++ b/src/cpp/daemon/py_monero_daemon.h @@ -21,13 +21,17 @@ class PyMoneroDaemonListener : public monero_daemon_listener { class PyMoneroBlockNotifier : public PyMoneroDaemonListener { public: - boost::mutex* temp; - boost::condition_variable* cv; - PyMoneroBlockNotifier(boost::mutex* temp, boost::condition_variable* cv) { this->temp = temp; this->cv = cv; } + PyMoneroBlockNotifier(boost::mutex* temp, boost::condition_variable* cv, bool* ready) { this->temp = temp; this->cv = cv; this->ready = ready; } void on_block_header(const std::shared_ptr& header) override { + boost::mutex::scoped_lock lock(*temp); m_last_header = header; + *ready = true; cv->notify_one(); } +private: + boost::mutex* temp; + boost::condition_variable* cv; + bool* ready; }; class PyMoneroDaemon { diff --git a/src/cpp/daemon/py_monero_daemon_rpc.cpp b/src/cpp/daemon/py_monero_daemon_rpc.cpp index ce240e1..35569f0 100644 --- a/src/cpp/daemon/py_monero_daemon_rpc.cpp +++ b/src/cpp/daemon/py_monero_daemon_rpc.cpp @@ -616,19 +616,20 @@ void PyMoneroDaemonRpc::stop() { } std::shared_ptr PyMoneroDaemonRpc::wait_for_next_block_header() { - // use mutex and condition variable to wait for block + // use mutex and condition variable with predicate to wait for block boost::mutex temp; boost::condition_variable cv; + bool ready = false; // create listener which notifies condition variable when block is added - auto block_listener = std::make_shared(&temp, &cv); + auto block_listener = std::make_shared(&temp, &cv, &ready); // register the listener add_listener(block_listener); // wait until condition variable is notified boost::mutex::scoped_lock lock(temp); - cv.wait(lock); + cv.wait(lock, [&]() { return ready; }); // unregister the listener remove_listener(block_listener); diff --git a/tests/test_monero_connection_manager.py b/tests/test_monero_connection_manager.py index 529e20b..faedef3 100644 --- a/tests/test_monero_connection_manager.py +++ b/tests/test_monero_connection_manager.py @@ -21,6 +21,7 @@ class TestMoneroConnectionManager: """Proxy used to simulate offline servers""" _cm: MoneroConnectionManager | None = None + """Connection manager test instance.""" #region Fixtures @@ -122,7 +123,7 @@ def test_connection_manager(self, connection_manager: MoneroConnectionManager, c # auto connect to the best available connection connection_manager.start_polling(Utils.SYNC_PERIOD_IN_MS) - listener.wait_for_change(Utils.SYNC_PERIOD_IN_MS, "Waiting for auto connect to best available connection") + listener.wait_for_change(num_expected_changes + 1, Utils.SYNC_PERIOD_IN_MS, f"Waiting for auto connect to best available connection") assert connection_manager.is_connected() connection = connection_manager.get_connection() assert connection is not None @@ -166,7 +167,7 @@ def test_connection_manager(self, connection_manager: MoneroConnectionManager, c continue conn.proxy_uri = self.OFFLINE_PROXY_URI - listener.wait_for_change(Utils.SYNC_PERIOD_IN_MS, "Simulating priotizized servers shut down") + listener.wait_for_change(num_expected_changes + 1, Utils.SYNC_PERIOD_IN_MS, "Simulating priotizized servers shut down") assert connection_manager.is_connected() is False, f"{connection_manager.get_connection().serialize()}" connection = connection_manager.get_connection() @@ -330,7 +331,7 @@ def test_connection_manager(self, connection_manager: MoneroConnectionManager, c poll_type=MoneroConnectionPollType.CURRENT ) - listener.wait_for_change(Utils.SYNC_PERIOD_IN_MS, "Polling current connection") + listener.wait_for_change(num_expected_changes + 1, Utils.SYNC_PERIOD_IN_MS, "Polling current connection") assert connection_manager.is_connected() is True num_expected_changes += 1 assert num_expected_changes == listener.num_changed_connections @@ -340,7 +341,7 @@ def test_connection_manager(self, connection_manager: MoneroConnectionManager, c num_expected_changes += 1 assert num_expected_changes == listener.num_changed_connections connection_manager.start_polling(period_ms=Utils.SYNC_PERIOD_IN_MS, poll_type=MoneroConnectionPollType.ALL) - listener.wait_for_change(Utils.SYNC_PERIOD_IN_MS, "Polling all connections") + listener.wait_for_change(num_expected_changes + 1, Utils.SYNC_PERIOD_IN_MS, "Polling all connections") assert connection_manager.is_connected() is True num_expected_changes += 1 assert num_expected_changes == listener.num_changed_connections @@ -351,7 +352,7 @@ def test_connection_manager(self, connection_manager: MoneroConnectionManager, c for con in ordered_connections: con.proxy_uri = self.OFFLINE_PROXY_URI - listener.wait_for_change(Utils.SYNC_PERIOD_IN_MS, "Simulating total shut down") + listener.wait_for_change(num_expected_changes + 1, Utils.SYNC_PERIOD_IN_MS, "Simulating total shut down") assert connection.is_online() is False, f"Expected offline connection: {connection.serialize()}" num_expected_changes += 1 assert num_expected_changes == listener.num_changed_connections diff --git a/tests/utils/connection_change_collector.py b/tests/utils/connection_change_collector.py index adf410c..51370a8 100644 --- a/tests/utils/connection_change_collector.py +++ b/tests/utils/connection_change_collector.py @@ -34,19 +34,21 @@ def on_connection_changed(self, connection: Optional[MoneroRpcConnection]) -> No logger.debug(f"Collecting connection change: {conn_str}") self.changed_connections.append(connection) - def wait_for_change(self, interval_ms: int = 5000, custom_message: str = "Waiting for connection change") -> Optional[MoneroRpcConnection]: + def wait_for_change(self, expected_num_changes: int, interval_ms: int = 5000, custom_message: str = "Waiting for connection change") -> Optional[MoneroRpcConnection]: """ Wait until a connection change occurs. + :param int expected_num_changes: expected number of connection changes to wait for. :param int interval_ms: custom check interval in milliseconds (default 5000). :param str custom_message: custom message to show in debug during wait. :returns MoneroRpcConnection | None: changed connection. """ last_num_changes: int = self.num_changed_connections - while self.num_changed_connections <= last_num_changes: - logger.debug(f"{custom_message} (connections {last_num_changes})...") + while expected_num_changes > last_num_changes: + logger.debug(f"{custom_message} (changes {last_num_changes}/{expected_num_changes})...") GenUtils.wait_for(interval_ms) + last_num_changes = self.num_changed_connections logger.debug(f"Connection changed (connections {self.num_changed_connections}).") From 50764b6072751aec8e2f0e6293ce49a39f7cd82a Mon Sep 17 00:00:00 2001 From: everoddandeven Date: Thu, 9 Apr 2026 21:15:17 +0200 Subject: [PATCH 2/4] Refactory test fixtures --- tests/test_monero_common.py | 10 ++-- tests/test_monero_connection_manager.py | 33 ++++--------- tests/test_monero_daemon_interface.py | 9 +--- tests/test_monero_daemon_rpc.py | 15 +++--- tests/test_monero_rpc_connection.py | 25 +--------- tests/test_monero_utils.py | 20 +------- tests/test_monero_wallet_common.py | 55 ++++++---------------- tests/test_monero_wallet_interface.py | 19 +------- tests/test_monero_wallet_model.py | 22 +-------- tests/utils/__init__.py | 4 +- tests/utils/base_test_class.py | 54 +++++++++++++++++++++ tests/utils/connection_change_collector.py | 7 ++- tests/utils/to_multiple_tx_sender.py | 2 +- 13 files changed, 105 insertions(+), 170 deletions(-) create mode 100644 tests/utils/base_test_class.py diff --git a/tests/test_monero_common.py b/tests/test_monero_common.py index 838dcb4..fc2ed4b 100644 --- a/tests/test_monero_common.py +++ b/tests/test_monero_common.py @@ -5,19 +5,15 @@ MoneroError, MoneroRpcError, SerializableStruct ) +from utils import BaseTestClass + logger: logging.Logger = logging.getLogger("TestMoneroCommon") @pytest.mark.unit -class TestMoneroCommon: +class TestMoneroCommon(BaseTestClass): """Monero common unit tests""" - @pytest.fixture(autouse=True) - def setup_and_teardown(self, request: pytest.FixtureRequest): - logger.info(f"Before {request.node.name}") # type: ignore - yield - logger.info(f"After {request.node.name}") # type: ignore - # test monero error inheritance def test_monero_error(self) -> None: monero_err: MoneroError = MoneroError("Test monero error") diff --git a/tests/test_monero_connection_manager.py b/tests/test_monero_connection_manager.py index faedef3..74bc056 100644 --- a/tests/test_monero_connection_manager.py +++ b/tests/test_monero_connection_manager.py @@ -1,20 +1,20 @@ import pytest import logging -from typing import Optional +from typing import Optional, override from monero import ( MoneroConnectionManager, MoneroRpcConnection, MoneroConnectionPollType ) from utils import ( ConnectionChangeCollector, TestUtils as Utils, - AssertUtils, RpcConnectionUtils + AssertUtils, RpcConnectionUtils, BaseTestClass ) logger: logging.Logger = logging.getLogger("TestMoneroConnectionManager") @pytest.mark.integration -class TestMoneroConnectionManager: +class TestMoneroConnectionManager(BaseTestClass): """Connection manager integration tests""" OFFLINE_PROXY_URI: str = "127.0.0.1:9050" @@ -25,39 +25,22 @@ class TestMoneroConnectionManager: #region Fixtures - # Setup and teardown of test class - @pytest.fixture(scope="class", autouse=True) - def global_setup_and_teardown(self): - """Executed once before all tests""" - self.before_all() - yield - self.after_all() - # Before all tests + @override def before_all(self) -> None: - """Executed once before all tests""" - logger.info(f"Setup test class {type(self).__name__}") + super().before_all() self._cm = MoneroConnectionManager() # After all tests + @override def after_all(self) -> None: - """Executed once after all tests""" - logger.info(f"Teardown test class {type(self).__name__}") + super().after_all() if self._cm: self._cm.reset() logger.debug("Resetted connection manager") - else: - logger.warning("Test connection manager is not set!") Utils.RPC_WALLET_MANAGER.clear() - # setup and teardown of each test - @pytest.fixture(autouse=True) - def setup_and_teardown(self, request: pytest.FixtureRequest): - logger.info(f"Before {request.node.name}") # type: ignore - yield - logger.info(f"After {request.node.name}") # type: ignore - # test connnections fixture @pytest.fixture(scope="class") def connections(self) -> list[MoneroRpcConnection]: @@ -123,7 +106,7 @@ def test_connection_manager(self, connection_manager: MoneroConnectionManager, c # auto connect to the best available connection connection_manager.start_polling(Utils.SYNC_PERIOD_IN_MS) - listener.wait_for_change(num_expected_changes + 1, Utils.SYNC_PERIOD_IN_MS, f"Waiting for auto connect to best available connection") + listener.wait_for_change(num_expected_changes + 1, Utils.SYNC_PERIOD_IN_MS, "Waiting for auto connect to best available connection") assert connection_manager.is_connected() connection = connection_manager.get_connection() assert connection is not None diff --git a/tests/test_monero_daemon_interface.py b/tests/test_monero_daemon_interface.py index cc05f91..27c60b8 100644 --- a/tests/test_monero_daemon_interface.py +++ b/tests/test_monero_daemon_interface.py @@ -2,21 +2,16 @@ import logging from monero import MoneroDaemon, MoneroBan +from utils import BaseTestClass logger: logging.Logger = logging.getLogger("TestMoneroDaemonInterface") # Test calls to MoneroDaemon interface @pytest.mark.unit -class TestMoneroDaemonInterface: +class TestMoneroDaemonInterface(BaseTestClass): """Daemon interface bindings unit tests""" - @pytest.fixture(autouse=True) - def setup_and_teardown(self, request: pytest.FixtureRequest): - logger.info(f"Before {request.node.name}") # type: ignore - yield - logger.info(f"After {request.node.name}") # type: ignore - @pytest.fixture(scope="class") def daemon(self) -> MoneroDaemon: """Test daemon interface instance""" diff --git a/tests/test_monero_daemon_rpc.py b/tests/test_monero_daemon_rpc.py index 9a5fd0c..38bad95 100644 --- a/tests/test_monero_daemon_rpc.py +++ b/tests/test_monero_daemon_rpc.py @@ -2,6 +2,8 @@ import time import logging +from typing import override + from monero import ( MoneroDaemonRpc, MoneroVersion, MoneroBlockHeader, MoneroBlockTemplate, MoneroBlock, MoneroMiningStatus, MoneroPruneResult, @@ -20,30 +22,25 @@ BlockUtils, GenUtils, DaemonUtils, WalletType, IntegrationTestUtils, - SubmitThenRelayTxTester + SubmitThenRelayTxTester, BaseTestClass ) logger: logging.Logger = logging.getLogger("TestMoneroDaemonRpc") @pytest.mark.integration -class TestMoneroDaemonRpc: +class TestMoneroDaemonRpc(BaseTestClass): """Rpc daemon integration tests""" BINARY_BLOCK_CTX: BinaryBlockContext = BinaryBlockContext() _test_wallet: MoneroWalletRpc | None = None #region Fixtures - @pytest.fixture(scope="class", autouse=True) + @override def before_all(self): + # setup wallet rpc for tests IntegrationTestUtils.setup(WalletType.RPC) - @pytest.fixture(autouse=True) - def setup_and_teardown(self, request: pytest.FixtureRequest): - logger.info(f"Before {request.node.name}") # type: ignore - yield - logger.info(f"After {request.node.name}") # type: ignore - @pytest.fixture(scope="class") def daemon(self) -> MoneroDaemonRpc: """Test rpc daemon instance""" diff --git a/tests/test_monero_rpc_connection.py b/tests/test_monero_rpc_connection.py index 3bbbd25..64aeb7e 100644 --- a/tests/test_monero_rpc_connection.py +++ b/tests/test_monero_rpc_connection.py @@ -5,13 +5,13 @@ MoneroRpcConnection, MoneroConnectionType, MoneroRpcError, MoneroUtils, MoneroConnectionProriotyComparator ) -from utils import TestUtils as Utils, DaemonUtils, StringUtils +from utils import TestUtils as Utils, DaemonUtils, StringUtils, BaseTestClass logger: logging.Logger = logging.getLogger("TestMoneroRpcConnection") @pytest.mark.integration -class TestMoneroRpcConnection: +class TestMoneroRpcConnection(BaseTestClass): """Rpc connection integration tests""" TIMEOUT_MS: int = Utils.AUTO_CONNECT_TIMEOUT_MS * 5 @@ -19,27 +19,6 @@ class TestMoneroRpcConnection: # region Fixtures - # Setup and teardown of test class - @pytest.fixture(scope="class", autouse=True) - def global_setup_and_teardown(self): - """Executed once before all tests""" - logger.info(f"Setup test class {type(self).__name__}") - self.before_all() - yield - logger.info(f"Teardown test class {type(self).__name__}") - - # Before all tests - def before_all(self) -> None: - """Executed once before all tests""" - logger.info(f"Setup test class {type(self).__name__}") - - # Setup and teardown of each tests - @pytest.fixture(autouse=True) - def setup_and_teardown(self, request: pytest.FixtureRequest): - logger.info(f"Before {request.node.name}") # type: ignore - yield - logger.info(f"After {request.node.name}") # type: ignore - # Node rpc connection fixture @pytest.fixture(scope="class") def node_connection(self) -> MoneroRpcConnection: diff --git a/tests/test_monero_utils.py b/tests/test_monero_utils.py index 0418819..f55c5ee 100644 --- a/tests/test_monero_utils.py +++ b/tests/test_monero_utils.py @@ -8,13 +8,13 @@ from monero import ( MoneroNetworkType, MoneroIntegratedAddress, MoneroUtils, MoneroTxConfig ) -from utils import AddressBook, KeysBook, WalletUtils +from utils import AddressBook, KeysBook, WalletUtils, BaseTestClass logger: logging.Logger = logging.getLogger("TestMoneroUtils") @pytest.mark.unit -class TestMoneroUtils: +class TestMoneroUtils(BaseTestClass): """Monero utilities unit tests""" class Config: @@ -59,22 +59,6 @@ def config(self) -> TestMoneroUtils.Config: parser.read('tests/config/test_monero_utils.ini') return TestMoneroUtils.Config.parse(parser) - # Setup and teardown of test class - @pytest.fixture(scope="class", autouse=True) - def global_setup_and_teardown(self): - """Executed once before all tests""" - logger.info(f"Setup test class {type(self).__name__}") - yield - logger.info(f"Teardown test class {type(self).__name__}") - - # Setup and teardown of each tests - @pytest.fixture(autouse=True) - def setup_and_teardown(self, request: pytest.FixtureRequest): - """Executed once before each test""" - logger.info(f"Before {request.node.name}") # type: ignore - yield - logger.info(f"After {request.node.name}") # type: ignore - #endregion #region Tests diff --git a/tests/test_monero_wallet_common.py b/tests/test_monero_wallet_common.py index f4a7bd4..aa67826 100644 --- a/tests/test_monero_wallet_common.py +++ b/tests/test_monero_wallet_common.py @@ -3,10 +3,11 @@ import pytest import logging +from typing import override from time import sleep from random import shuffle from configparser import ConfigParser -from abc import ABC, abstractmethod +from abc import abstractmethod from typing import Optional from datetime import datetime @@ -28,14 +29,14 @@ ViewOnlyAndOfflineWalletTester, WalletNotificationCollector, MiningUtils, SendAndUpdateTxsTester, - SyncWithPoolSubmitTester + SyncWithPoolSubmitTester, BaseTestClass ) logger: logging.Logger = logging.getLogger("TestMoneroWalletCommon") # Base class for common wallet tests -class BaseTestMoneroWallet(ABC): +class BaseTestMoneroWallet(BaseTestClass): """Common wallet tests that every Monero wallet implementation should support""" CREATED_WALLET_KEYS_ERROR: str = "Wallet created from keys is not connected to authenticated daemon" _test_wallet: Optional[MoneroWallet] = None @@ -177,50 +178,27 @@ def wallet(self) -> MoneroWallet: """Test wallet instance""" pytest.skip("No wallet test instance setup") - # Setup and teardown of test class - @pytest.fixture(scope="class", autouse=True) - def global_setup_and_teardown(self): - """Executed once before all tests""" - self.before_all() - yield - self.after_all() - - # Setup and teardown of each test - @pytest.fixture(autouse=True) - def setup_and_teardown(self, request: pytest.FixtureRequest): - self.before_each(request) - yield - self.after_each(request) - # Before all tests + @override def before_all(self) -> None: - """Executed once before all tests""" - logger.info(f"Setup test class {type(self).__name__}") + super().before_all() IntegrationTestUtils.setup(self.get_wallet_type()) # After all tests + @override def after_all(self) -> None: - """Executed once after all tests""" - logger.info(f"Teardown test class {type(self).__name__}") - daemon: MoneroDaemonRpc | None = self._get_test_daemon() - try: - daemon.stop_mining() - except Exception as e: - logger.debug(str(e)) + super().after_all() + daemon: MoneroDaemonRpc = self._get_test_daemon() + MiningUtils.try_stop_mining(daemon) # close wallet wallet = self.get_test_wallet() wallet.close(True) # Before each test + @override def before_each(self, request: pytest.FixtureRequest) -> None: - """ - Executed before each test - - :param pytest.FixtureRequest: Request fixture - """ - logger.info(f"Before {request.node.name}") # type: ignore - + super().before_each(request) daemon = self._get_test_daemon() wallet = self.get_test_wallet() status = daemon.get_mining_status() @@ -229,14 +207,9 @@ def before_each(self, request: pytest.FixtureRequest) -> None: wallet.stop_mining() # After each test + @override def after_each(self, request: pytest.FixtureRequest) -> None: - """ - Executed after each test - - :param pytest.FixtureRequest: Request fixture - """ - logger.info(f"After {request.node.name}") # type: ignore - + super().after_each(request) daemon = self._get_test_daemon() status = daemon.get_mining_status() diff --git a/tests/test_monero_wallet_interface.py b/tests/test_monero_wallet_interface.py index 6f12632..114a275 100644 --- a/tests/test_monero_wallet_interface.py +++ b/tests/test_monero_wallet_interface.py @@ -8,14 +8,14 @@ MoneroTxWallet ) -from utils import WalletUtils, StringUtils +from utils import WalletUtils, StringUtils, BaseTestClass logger: logging.Logger = logging.getLogger("TestMoneroWalletInterface") # Test binding calls to MoneroWallet interface @pytest.mark.unit -class TestMoneroWalletInterface: +class TestMoneroWalletInterface(BaseTestClass): """Wallet interface binding calls unit tests""" @pytest.fixture(scope="class") @@ -23,21 +23,6 @@ def wallet(self) -> MoneroWallet: """Test wallet instance""" return MoneroWallet() - # Setup and teardown of test class - @pytest.fixture(scope="class", autouse=True) - def global_setup_and_teardown(self): - """Executed once before all tests""" - logger.info(f"Setup test class {type(self).__name__}") - yield - logger.info(f"Teardown test class {type(self).__name__}") - - # Setup and teardown of each tests - @pytest.fixture(autouse=True) - def setup_and_teardown(self, request: pytest.FixtureRequest): - logger.info(f"Before {request.node.name}") # type: ignore - yield - logger.info(f"After {request.node.name}") # type: ignore - #region Tests # Test default language static property diff --git a/tests/test_monero_wallet_model.py b/tests/test_monero_wallet_model.py index a39d9b7..2e137ca 100644 --- a/tests/test_monero_wallet_model.py +++ b/tests/test_monero_wallet_model.py @@ -2,33 +2,15 @@ import logging from monero import MoneroTxQuery, MoneroTransferQuery, MoneroOutputQuery +from utils import BaseTestClass logger: logging.Logger = logging.getLogger("TestMoneroWalletModel") @pytest.mark.unit -class TestMoneroWalletModel: +class TestMoneroWalletModel(BaseTestClass): """Test monero wallet data model""" - #region Fixtures - - # Setup and teardown of test class - @pytest.fixture(scope="class", autouse=True) - def global_setup_and_teardown(self): - """Executed once before all tests""" - logger.info(f"Setup test class {type(self).__name__}") - yield - logger.info(f"Teardown test class {type(self).__name__}") - - # setup and teardown of each tests - @pytest.fixture(autouse=True) - def setup_and_teardown(self, request: pytest.FixtureRequest): - logger.info(f"Before {request.node.name}") # type: ignore - yield - logger.info(f"After {request.node.name}") # type: ignore - - #endregion - #region Tests # Test output query expected behaviour diff --git a/tests/utils/__init__.py b/tests/utils/__init__.py index 6d51968..0deefb2 100644 --- a/tests/utils/__init__.py +++ b/tests/utils/__init__.py @@ -35,6 +35,7 @@ from .sync_with_pool_submit_tester import SyncWithPoolSubmitTester from .docker_wallet_rpc_manager import DockerWalletRpcManager from .rpc_connection_utils import RpcConnectionUtils +from .base_test_class import BaseTestClass __all__ = [ 'WalletUtils', @@ -73,5 +74,6 @@ 'SendAndUpdateTxsTester', 'SyncWithPoolSubmitTester', 'DockerWalletRpcManager', - 'RpcConnectionUtils' + 'RpcConnectionUtils', + 'BaseTestClass' ] diff --git a/tests/utils/base_test_class.py b/tests/utils/base_test_class.py new file mode 100644 index 0000000..08e6434 --- /dev/null +++ b/tests/utils/base_test_class.py @@ -0,0 +1,54 @@ +import logging +import pytest + +from abc import ABC + +logger: logging.Logger = logging.getLogger("BaseTestClass") + + +class BaseTestClass(ABC): + """Base test class with common fixtures.""" + + # Setup and teardown of test class + @pytest.fixture(scope="class", autouse=True) + def global_setup_and_teardown(self): + """Executed once before all tests""" + self.before_all() + yield + self.after_all() + + # Setup and teardown of each test + @pytest.fixture(autouse=True) + def setup_and_teardown(self, request: pytest.FixtureRequest): + """Executed before each test.""" + self.before_each(request) + yield + self.after_each(request) + + # Before all tests + def before_all(self) -> None: + """Executed once before all tests""" + logger.info(f"Setup test class {type(self).__name__}") + + # After all tests + def after_all(self) -> None: + """Executed once after all tests""" + logger.info(f"Teardown test class {type(self).__name__}") + + # Before each test + def before_each(self, request: pytest.FixtureRequest) -> None: + """ + Executed before each test + + :param pytest.FixtureRequest: Request fixture + """ + logger.info(f"Before {request.node.name}") # type: ignore + + # After each test + def after_each(self, request: pytest.FixtureRequest) -> None: + """ + Executed after each test + + :param pytest.FixtureRequest: Request fixture + """ + logger.info(f"After {request.node.name}") # type: ignore diff --git a/tests/utils/connection_change_collector.py b/tests/utils/connection_change_collector.py index 51370a8..9614108 100644 --- a/tests/utils/connection_change_collector.py +++ b/tests/utils/connection_change_collector.py @@ -34,7 +34,12 @@ def on_connection_changed(self, connection: Optional[MoneroRpcConnection]) -> No logger.debug(f"Collecting connection change: {conn_str}") self.changed_connections.append(connection) - def wait_for_change(self, expected_num_changes: int, interval_ms: int = 5000, custom_message: str = "Waiting for connection change") -> Optional[MoneroRpcConnection]: + def wait_for_change( + self, + expected_num_changes: int, + interval_ms: int = 5000, + custom_message: str = "Waiting for connection change" + ) -> Optional[MoneroRpcConnection]: """ Wait until a connection change occurs. diff --git a/tests/utils/to_multiple_tx_sender.py b/tests/utils/to_multiple_tx_sender.py index f1307f2..273fb50 100644 --- a/tests/utils/to_multiple_tx_sender.py +++ b/tests/utils/to_multiple_tx_sender.py @@ -106,7 +106,7 @@ def _create_accounts(self) -> int: :returns int: number of accounts created. """ num_accounts: int = len(self._wallet.get_accounts()) - logger.info(f"Wallet has already {num_accounts} accounts") + logger.debug(f"Wallet has already {num_accounts} accounts") num_accounts_to_create: int = self._num_accounts - num_accounts if num_accounts <= self._num_accounts else 0 for i in range(num_accounts_to_create): self._wallet.create_account() From 0dc6784814834740f3b0daa61872539e06d60ee2 Mon Sep 17 00:00:00 2001 From: everoddandeven Date: Thu, 9 Apr 2026 22:11:01 +0200 Subject: [PATCH 3/4] Code quality fixes --- tests/test_monero_wallet_full.py | 18 +++++++++++++++--- tests/test_monero_wallet_keys.py | 21 ++------------------- tests/utils/block_utils.py | 21 +++++++++++++++++++-- tests/utils/sync_with_pool_submit_tester.py | 7 +++++-- tests/utils/tx_utils.py | 17 +++++++++++++++-- tests/utils/wallet_utils.py | 17 +++++++++++++++++ 6 files changed, 73 insertions(+), 28 deletions(-) diff --git a/tests/test_monero_wallet_full.py b/tests/test_monero_wallet_full.py index c07b97c..fba9cab 100644 --- a/tests/test_monero_wallet_full.py +++ b/tests/test_monero_wallet_full.py @@ -694,7 +694,7 @@ def test_get_height_by_date_regtest(self, wallet: MoneroWallet): #region Disabled Tests - @pytest.mark.skip(reason="TODO disabled because importing key images deletes corresponding incoming transfers: https://github.com/monero-project/monero/issues/5812") + @pytest.mark.skip(reason="TODO disabled because importing key images deletes corresponding incoming transfers: #5812") @override def test_import_key_images(self, wallet: MoneroWallet): return super().test_import_key_images(wallet) @@ -719,8 +719,20 @@ def _test_multisig_sample(self, m: int, n: int) -> None: tester: MultisigSampleCodeTester = MultisigSampleCodeTester(m, wallets) tester.test() - def _test_sync_seed(self, daemon: MoneroDaemonRpc, wallet: MoneroWalletFull, start_height: Optional[int], restore_height: Optional[int], skip_gt_comparison: bool = False, test_post_sync_notifications: bool = False) -> None: - tester: SyncSeedTester = SyncSeedTester(daemon, wallet, self._create_wallet, start_height, restore_height, skip_gt_comparison, test_post_sync_notifications) + def _test_sync_seed( + self, + daemon: MoneroDaemonRpc, + wallet: MoneroWalletFull, + start_height: Optional[int], + restore_height: Optional[int], + skip_gt_comparison: bool = False, + test_post_sync_notifications: bool = False + ) -> None: + tester: SyncSeedTester = SyncSeedTester( + daemon, wallet, self._create_wallet, + start_height, restore_height, + skip_gt_comparison, test_post_sync_notifications + ) tester.test() #endregion diff --git a/tests/test_monero_wallet_keys.py b/tests/test_monero_wallet_keys.py index 968e437..aa7dc2b 100644 --- a/tests/test_monero_wallet_keys.py +++ b/tests/test_monero_wallet_keys.py @@ -661,31 +661,14 @@ def test_create_wallet_from_keys(self, daemon: MoneroDaemonRpc, wallet: MoneroWa config.private_view_key = private_view_key config.private_spend_key = private_spend_key w: MoneroWallet = self._create_wallet(config) - - try: - assert primary_address == w.get_primary_address() - assert private_view_key == w.get_private_view_key() - assert private_spend_key == w.get_private_spend_key() - MoneroUtils.validate_mnemonic(w.get_seed()) - assert MoneroWallet.DEFAULT_LANGUAGE == w.get_seed_language() - finally: - self._close_wallet(w) + WalletUtils.test_wallet_keys(primary_address, private_view_key, private_spend_key, w) # recreate test wallet from spend key config = MoneroWalletConfig() config.primary_address = primary_address config.private_spend_key = private_spend_key w = self._create_wallet(config) - - try: - assert primary_address == w.get_primary_address() - assert private_view_key == w.get_private_view_key() - assert private_spend_key == w.get_private_spend_key() - # TODO monero-wallet-rpc: cannot get seed from wallet created from keys? - MoneroUtils.validate_mnemonic(w.get_seed()) - assert MoneroWallet.DEFAULT_LANGUAGE == w.get_seed_language() - finally: - self._close_wallet(w) + WalletUtils.test_wallet_keys(primary_address, private_view_key, private_spend_key, w) @pytest.mark.skipif(Utils.TEST_NON_RELAYS is False, reason="TEST_NON_RELAYS disabled") @override diff --git a/tests/utils/block_utils.py b/tests/utils/block_utils.py index 0908a6a..7df70ea 100644 --- a/tests/utils/block_utils.py +++ b/tests/utils/block_utils.py @@ -17,7 +17,7 @@ class BlockUtils(ABC): """Block utilities""" @classmethod - def test_block_header(cls, header: Optional[MoneroBlockHeader], is_full: Optional[bool]): + def test_block_header(cls, header: Optional[MoneroBlockHeader], is_full: Optional[bool]) -> None: """ Test a block header @@ -46,6 +46,17 @@ def test_block_header(cls, header: Optional[MoneroBlockHeader], is_full: Optiona assert header.nonce > 0 # never seen defined assert header.pow_hash is None + # test full header details + cls.test_full_header(header, is_full) + + @classmethod + def test_full_header(cls, header: MoneroBlockHeader, is_full: Optional[bool]) -> None: + """ + Test full header details. + + :param MoneroBlockHeader header: header to test full details. + :param bool | None is_full: indicates if `header`'s full details should be defined. + """ if is_full: # check full block assert header.size is not None @@ -133,7 +144,13 @@ def test_get_blocks_range( # fetch blocks by range real_start_height: int = 0 if start_height is None else start_height real_end_height: int = chain_height - 1 if end_height is None else end_height - blocks: list[MoneroBlock] = daemon.get_blocks_by_range_chunked(start_height, end_height) if chunked else daemon.get_blocks_by_range(start_height, end_height) + blocks: list[MoneroBlock] + + if chunked: + blocks = daemon.get_blocks_by_range_chunked(start_height, end_height) + else: + blocks = daemon.get_blocks_by_range(start_height, end_height) + num_blocks: int = len(blocks) expected_num_blocks: int = real_end_height - real_start_height + 1 assert expected_num_blocks == num_blocks, f"Expected {expected_num_blocks} block(s), got {num_blocks}" diff --git a/tests/utils/sync_with_pool_submit_tester.py b/tests/utils/sync_with_pool_submit_tester.py index 3c4e883..d739d9e 100644 --- a/tests/utils/sync_with_pool_submit_tester.py +++ b/tests/utils/sync_with_pool_submit_tester.py @@ -14,6 +14,9 @@ class SyncWithPoolSubmitTester: """Test wallet capability to sync transactions in the pool.""" + BALANCE_ERR: str = "Wallet balance should revert to original after flushing tx from pool without relaying" + UNLOCKED_BALANCE_ERR: str = "Wallet unlocked balance should revert to original after flushing tx from pool without relaying" + daemon: MoneroDaemon """Daemon instance.""" wallet: MoneroWallet @@ -56,9 +59,9 @@ def run_failing_core_code(self, config_no_relay: MoneroTxConfig) -> None: return # wallet balances should change - assert self.balance_before != self.wallet.get_balance(), "Wallet balance should revert to original after flushing tx from pool without relaying" + assert self.balance_before != self.wallet.get_balance(), self.BALANCE_ERR # TODO: test exact amounts, maybe in ux test - assert self.unlocked_balance_before != self.wallet.get_unlocked_balance(), "Wallet unlocked balance should revert to original after flushing tx from pool without relaying" + assert self.unlocked_balance_before != self.wallet.get_unlocked_balance(), self.UNLOCKED_BALANCE_ERR # create tx using same config which is no longer double spend tx2: MoneroTxWallet = self.wallet.create_tx(config_no_relay) diff --git a/tests/utils/tx_utils.py b/tests/utils/tx_utils.py index 802265c..aa249e8 100644 --- a/tests/utils/tx_utils.py +++ b/tests/utils/tx_utils.py @@ -867,7 +867,14 @@ def test_signature_header_error(cls, ex: Exception) -> None: assert msg == "Signature header check error", msg @classmethod - def get_and_test_txs(cls, wallet: MoneroWallet, query: Optional[MoneroTxQuery], ctx: Optional[TxContext], is_expected: Optional[bool], regtest: bool) -> list[MoneroTxWallet]: + def get_and_test_txs( + cls, + wallet: MoneroWallet, + query: Optional[MoneroTxQuery], + ctx: Optional[TxContext], + is_expected: Optional[bool], + regtest: bool + ) -> list[MoneroTxWallet]: """Get and test txs from wallet""" copy: Optional[MoneroTxQuery] = query.copy() if query is not None else None txs = wallet.get_txs(query) if query is not None else wallet.get_txs() @@ -888,7 +895,13 @@ def get_and_test_txs(cls, wallet: MoneroWallet, query: Optional[MoneroTxQuery], return txs @classmethod - def get_and_test_transfers(cls, wallet: MoneroWallet, query: Optional[MoneroTransferQuery], ctx: Optional[TxContext], is_expected: Optional[bool]) -> list[MoneroTransfer]: + def get_and_test_transfers( + cls, + wallet: MoneroWallet, + query: Optional[MoneroTransferQuery], + ctx: Optional[TxContext], + is_expected: Optional[bool] + ) -> list[MoneroTransfer]: copy: Optional[MoneroTransferQuery] = query.copy() if query is not None else None transfers = wallet.get_transfers(query) if query is not None else wallet.get_transfers(MoneroTransferQuery()) diff --git a/tests/utils/wallet_utils.py b/tests/utils/wallet_utils.py index 1e0e5aa..b02e134 100644 --- a/tests/utils/wallet_utils.py +++ b/tests/utils/wallet_utils.py @@ -483,3 +483,20 @@ def test_sweep_wallet(cls, wallet: MoneroWallet, sweep_each_subaddress: Optional """ sweeper: WalletSweeper = WalletSweeper(wallet, sweep_each_subaddress) sweeper.sweep() + + @classmethod + def test_wallet_keys(cls, address: str, view_key: str, spend_key: str, w: MoneroWallet) -> None: + """ + Test wallet keys. + + :param str address: expected primary address. + :param str view_key: expected private view key. + :param str spend_key: expected private spend key. + :param MoneroWallet wallet: wallet to test keys. + """ + + assert address == w.get_primary_address() + assert view_key == w.get_private_view_key() + assert spend_key == w.get_private_spend_key() + MoneroUtils.validate_mnemonic(w.get_seed()) + assert MoneroWallet.DEFAULT_LANGUAGE == w.get_seed_language() From 500345e37165e14c7e5b1e42428235b383a2b6c7 Mon Sep 17 00:00:00 2001 From: everoddandeven Date: Fri, 10 Apr 2026 23:29:14 +0200 Subject: [PATCH 4/4] Debug test connection --- .github/workflows/test.yml | 2 +- pytest.ini | 4 ++-- src/cpp/common/py_monero_common.cpp | 25 +++++++++++++++++++++++-- 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 8af8b35..19f87a5 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -96,7 +96,7 @@ jobs: env: IN_CONTAINER: "true" run: | - pytest --cov=tests --cov-report=xml + pytest -s -v --cov=tests --cov-report=xml - name: Cleanup test environment if: always() diff --git a/pytest.ini b/pytest.ini index 679a423..545d1ab 100644 --- a/pytest.ini +++ b/pytest.ini @@ -1,9 +1,9 @@ [pytest] minversion = 6.0 addopts = -v -log_level = INFO +log_level = DEBUG log_cli = True -log_cli_level = INFO +log_cli_level = DEBUG console_output_style = progress testpaths = tests diff --git a/src/cpp/common/py_monero_common.cpp b/src/cpp/common/py_monero_common.cpp index 4039952..e1da7b9 100644 --- a/src/cpp/common/py_monero_common.cpp +++ b/src/cpp/common/py_monero_common.cpp @@ -26,7 +26,10 @@ void PyThreadPoller::init_common(const std::string& name) { } void PyThreadPoller::set_is_polling(bool is_polling) { - if (is_polling == m_is_polling) return; + if (is_polling == m_is_polling) { + std::cout << "Already set " << is_polling << std::endl; + return; + } m_is_polling = is_polling; if (m_is_polling) { @@ -46,12 +49,17 @@ void PyThreadPoller::set_period_in_ms(uint64_t period_ms) { } void PyThreadPoller::run_poll_loop() { - if (m_poll_loop_running) return; // only run one loop at a time + if (m_poll_loop_running) { + std::cout << "PyThreadPoller::run_poll_loop(): already running loop" << std::endl; + return; // only run one loop at a time + } m_poll_loop_running = true; // start pool loop thread // TODO: use global threadpool, background sync wasm wallet in c++ thread + std::cout << "Initializing thread..." << std::endl; m_thread = boost::thread([this]() { + std::cout << "Started thread" << std::endl; // poll while enabled while (m_is_polling) { @@ -67,8 +75,11 @@ void PyThreadPoller::run_poll_loop() { } } + std::cout << "End thread" << std::endl; m_poll_loop_running = false; }); + std::cout << "Initialized thread" << std::endl; + } py::object PyGenUtils::convert_value(const std::string& val) { @@ -753,8 +764,10 @@ void PyMoneroConnectionManager::stop_polling() { } void PyMoneroConnectionManager::start_polling(const boost::optional& period_ms, const boost::optional& auto_switch, const boost::optional& timeout_ms, const boost::optional& poll_type, const boost::optional>> &excluded_connections) { + std::cout << "PyMoneroConnectionManager::start_polling()" << std::endl; // stop polling stop_polling(); + std::cout << "PyMoneroConnectionManager::start_polling(): stopped polling" << std::endl; // apply defaults uint64_t poll_period_ms = period_ms == boost::none ? DEFAULT_POLL_PERIOD : period_ms.get(); @@ -770,7 +783,9 @@ void PyMoneroConnectionManager::start_polling(const boost::optional& p } // start polling + std::cout << "PyMoneroConnectionManager::start_polling(): set polling..." << std::endl; set_is_polling(true); + std::cout << "PyMoneroConnectionManager::start_polling(): set polling: " << m_is_polling << std::endl; } std::shared_ptr PyMoneroConnectionManager::get_best_available_connection(const std::set>& excluded_connections) { @@ -908,6 +923,7 @@ std::vector>> PyMoneroConnect void PyMoneroConnectionManager::poll() { // do polling + std::cout << "PyMoneroConnectionManager::poll()" << std::endl; switch (m_poll_type) { case PyMoneroConnectionPollType::CURRENT: check_connection(); @@ -927,6 +943,7 @@ bool PyMoneroConnectionManager::check_connections(const std::vector> completed; @@ -939,9 +956,11 @@ bool PyMoneroConnectionManager::check_connections(const std::vectorserialize() << std::endl; bool change = connection->check_connection(m_timeout); if (change && connection == get_connection()) { + std::cout << "changed connection: " << connection->serialize() << std::endl; on_connection_changed(connection); } @@ -960,9 +979,11 @@ bool PyMoneroConnectionManager::check_connections(const std::vector lock(result_mutex); + std::cout << "Waiting for connection..." << std::endl; result_cv.wait(lock, [&]() { return completed.size() > received; }); auto connection = completed[received++]; + std::cout << "Got connection: " << connection->serialize() << std::endl; lock.unlock(); if (connection->is_connected().value_or(false) && !has_connection) {