fix: address review comments - use mock_repl fixture and naming conventions

- Use mock_repl fixture in TestConfig instead of inline with-patch
- Add _success suffix to all TestRuntimeMode test names
- Fix test_init_defaults_no_mode_key_succeeds -> _success

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: Alon Yeshurun

tests/test_commands/test_config.py

@@ -12,6 +12,11 @@
 
 
 class TestConfig:
+    @pytest.fixture
+    def mock_repl(self):
+        with patch("fabric_cli.core.fab_interactive.start_interactive_mode") as mock:
+            yield mock
+
     # region config SET
     def test_config_set_success(self, mock_print_done, cli_executor: CLIExecutor):
         # Execute command
@@ -173,24 +178,22 @@ def test_config_clear_cache_success(
 
     # region config MODE (deprecated)
     def test_config_set_mode_interactive_shows_deprecation_warning(
-        self, mock_questionary_print, mock_print_warning, cli_executor: CLIExecutor
+        self, mock_questionary_print, mock_print_warning, mock_repl, cli_executor: CLIExecutor
     ):
         """Test that 'config set mode interactive' shows deprecation warning and launches REPL."""
-        with patch("fabric_cli.core.fab_interactive.start_interactive_mode") as mock_repl:
-            cli_executor.exec_command(f"config set mode {constant.FAB_MODE_INTERACTIVE}")
+        cli_executor.exec_command(f"config set mode {constant.FAB_MODE_INTERACTIVE}")
 
-            mock_print_warning.assert_any_call(DEPRECATION_WARNING)
-            mock_repl.assert_called_once()
+        mock_print_warning.assert_any_call(DEPRECATION_WARNING)
+        mock_repl.assert_called_once()
 
     def test_config_set_mode_command_line_shows_deprecation_warning(
-        self, mock_questionary_print, mock_print_warning, cli_executor: CLIExecutor
+        self, mock_questionary_print, mock_print_warning, mock_repl, cli_executor: CLIExecutor
     ):
         """Test that 'config set mode command_line' shows deprecation warning without launching REPL."""
-        with patch("fabric_cli.core.fab_interactive.start_interactive_mode") as mock_repl:
-            cli_executor.exec_command(f"config set mode {constant.FAB_MODE_COMMANDLINE}")
+        cli_executor.exec_command(f"config set mode {constant.FAB_MODE_COMMANDLINE}")
 
-            mock_print_warning.assert_any_call(DEPRECATION_WARNING)
-            mock_repl.assert_not_called()
+        mock_print_warning.assert_any_call(DEPRECATION_WARNING)
+        mock_repl.assert_not_called()
 
     def test_config_get_mode_shows_deprecation_warning(
         self, mock_questionary_print, mock_print_warning, cli_executor: CLIExecutor

tests/test_core/test_fab_context.py

@@ -446,30 +446,30 @@ class TestRuntimeMode:
     def teardown_method(self):
         Context()._runtime_mode = fab_constant.FAB_MODE_COMMANDLINE
 
-    def test_default_runtime_mode_is_command_line(self):
+    def test_default_runtime_mode_is_command_line_success(self):
         """Default runtime mode must be COMMANDLINE when no REPL is active."""
         assert Context().get_runtime_mode() == fab_constant.FAB_MODE_COMMANDLINE
 
-    def test_set_runtime_mode_to_interactive(self):
+    def test_set_runtime_mode_to_interactive_success(self):
         """set_runtime_mode(INTERACTIVE) should switch the mode."""
         Context().set_runtime_mode(fab_constant.FAB_MODE_INTERACTIVE)
         assert Context().get_runtime_mode() == fab_constant.FAB_MODE_INTERACTIVE
 
-    def test_set_runtime_mode_back_to_command_line(self):
+    def test_set_runtime_mode_back_to_command_line_success(self):
         """Switching to INTERACTIVE then back to COMMANDLINE must work."""
         Context().set_runtime_mode(fab_constant.FAB_MODE_INTERACTIVE)
         Context().set_runtime_mode(fab_constant.FAB_MODE_COMMANDLINE)
         assert Context().get_runtime_mode() == fab_constant.FAB_MODE_COMMANDLINE
 
-    def test_runtime_mode_not_in_config_keys(self):
+    def test_runtime_mode_not_in_config_keys_success(self):
         """'mode' must no longer appear in FAB_CONFIG_KEYS_TO_VALID_VALUES."""
         assert fab_constant.FAB_MODE not in fab_constant.FAB_CONFIG_KEYS_TO_VALID_VALUES
 
-    def test_runtime_mode_not_in_config_defaults(self):
+    def test_runtime_mode_not_in_config_defaults_success(self):
         """'mode' must no longer appear in CONFIG_DEFAULT_VALUES."""
         assert fab_constant.FAB_MODE not in fab_constant.CONFIG_DEFAULT_VALUES
 
-    def test_runtime_mode_not_module_level(self):
+    def test_runtime_mode_not_module_level_success(self):
         """Runtime mode must live on Context, not as module-level functions."""
         from fabric_cli.core import fab_context as ctx_module
         assert not hasattr(ctx_module, "set_runtime_mode")

tests/test_core/test_fab_state_config.py

@@ -128,7 +128,7 @@ def test_init_defaults_removes_mode_key(monkeypatch):
     assert result[fab_constant.FAB_CACHE_ENABLED] == "true"
 
 
-def test_init_defaults_no_mode_key_succeeds(monkeypatch):
+def test_init_defaults_no_mode_key_success(monkeypatch):
     """Config without 'mode' must initialize cleanly (distinct from removes_mode_key: verifies no error on absence)."""
     config_file = _create_temp_config(monkeypatch, {
         fab_constant.FAB_DEBUG_ENABLED: "true",