Add review suggestions

safe_cli/main.py

@@ -123,7 +123,7 @@ def build_safe_cli() -> Optional[SafeCli]:
     parser = argparse.ArgumentParser()
     parser.add_argument(
         "address",
-        help="The address of the Safe, or an owner address if --is-owner is specified.",
+        help="The address of the Safe, or an owner address if --get-safes-from-owner is specified.",
         type=check_ethereum_address,
     )
     parser.add_argument("node_url", help="Ethereum node url")
@@ -151,8 +151,7 @@ def build_safe_cli() -> Optional[SafeCli]:
 
 
 def main(*args, **kwargs):
-    if (safe_cli := build_safe_cli()) is None:
-        return None
+    safe_cli = build_safe_cli()
     safe_cli.print_startup_info()
     safe_cli.loop()
 

safe_cli/operators/safe_operator.py

@@ -63,11 +63,7 @@
     get_safe_contract_address,
     get_safe_l2_contract_address,
 )
-from safe_cli.utils import (
-    choose_option_from_list_question,
-    get_erc_20_list,
-    yes_or_no_question,
-)
+from safe_cli.utils import ask_option_from_list, get_erc_20_list, yes_or_no_question
 
 from ..contracts import safe_to_l2_migration
 
@@ -298,9 +294,7 @@ def load_ledger_cli_owners(
             if len(ledger_accounts) == 0:
                 return None
 
-            option = choose_option_from_list_question(
-                "Select the owner address", ledger_accounts
-            )
+            option = ask_option_from_list("Select the owner address", ledger_accounts)
             if option is None:
                 return None
             _, derivation_path = ledger_accounts[option]

safe_cli/utils.py

@@ -55,7 +55,7 @@ def yes_or_no_question(question: str, default_no: bool = True) -> bool:
         return False if default_no else True
 
 
-def choose_option_from_list_question(
+def ask_option_from_list(
     question: str, options: List, default_option: int = 0
 ) -> Optional[int]:
     if "PYTEST_CURRENT_TEST" in os.environ:
@@ -93,13 +93,10 @@ def get_safe_from_owner(
     safe_tx_service = TransactionServiceApi.from_ethereum_client(ethereum_client)
     safes = safe_tx_service.get_safes_for_owner(owner)
     if safes:
-        option = choose_option_from_list_question(
+        option = ask_option_from_list(
             "Select the Safe to initialize the safe-cli", safes
         )
         if option is not None:
             return safes[option]
-
-    print_formatted_text(
-        HTML(f"<ansired>No safe was found for the specified owner {owner}</ansired>")
-    )
-    return None
+    else:
+        raise ValueError(f"No safe was found for the specified owner {owner}")

tests/test_entrypoint.py

@@ -82,8 +82,8 @@ def test_build_safe_cli_for_owner(
             "1.4.1",
         )
         get_safes_for_owner_mock.return_value = []
-        safe_cli = self.build_test_safe_cli_for_owner()
-        self.assertIsNone(safe_cli)
+        with self.assertRaises(ValueError):
+            self.build_test_safe_cli_for_owner()
         get_safes_for_owner_mock.return_value = [self.random_safe_address]
         safe_cli = self.build_test_safe_cli_for_owner()
         self.assertIsNotNone(safe_cli)

tests/test_utils.py

@@ -5,7 +5,7 @@
 
 from eth_account import Account
 
-from safe_cli.utils import choose_option_from_list_question, yes_or_no_question
+from safe_cli.utils import ask_option_from_list, yes_or_no_question
 
 
 class TestUtils(unittest.TestCase):
@@ -37,20 +37,20 @@ def test_yes_or_no_question(self, input_mock: MagicMock):
         os.environ["PYTEST_CURRENT_TEST"] = pytest_current_test
 
     @mock.patch("safe_cli.utils.get_input")
-    def test_choose_option_question(self, input_mock: MagicMock):
+    def test_ask_option_from_list(self, input_mock: MagicMock):
         pytest_current_test = os.environ.pop("PYTEST_CURRENT_TEST")
         address = Account.create().address
         options = [address for i in range(0, 5)]
         input_mock.return_value = ""
-        self.assertEqual(choose_option_from_list_question("", options), 0)
+        self.assertEqual(ask_option_from_list("", options), 0)
         input_mock.return_value = ""
-        self.assertEqual(choose_option_from_list_question("", options, 4), 4)
+        self.assertEqual(ask_option_from_list("", options, 4), 4)
         input_mock.return_value = "m"
-        self.assertIsNone(choose_option_from_list_question("", options))
+        self.assertIsNone(ask_option_from_list("", options))
         input_mock.return_value = "10"
-        self.assertIsNone(choose_option_from_list_question("", options))
+        self.assertIsNone(ask_option_from_list("", options))
         input_mock.return_value = "1"
-        self.assertEqual(choose_option_from_list_question("", options), 1)
+        self.assertEqual(ask_option_from_list("", options), 1)
 
         os.environ["PYTEST_CURRENT_TEST"] = pytest_current_test