Code quality fixes

Author: everoddandeven

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

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

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}"

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)

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())
 

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()