From 461b3fcd239388016e537eb3a5ffa0bbd7cf3f84 Mon Sep 17 00:00:00 2001 From: aviat cohen Date: Wed, 3 Dec 2025 15:47:47 +0000 Subject: [PATCH 01/13] avoid re-login when set mode to interactive --- .../commands/config/fab_config_set.py | 65 ++++++- src/fabric_cli/main.py | 9 +- tests/test_commands/test_config.py | 184 +++++++++++++++++- 3 files changed, 249 insertions(+), 9 deletions(-) diff --git a/src/fabric_cli/commands/config/fab_config_set.py b/src/fabric_cli/commands/config/fab_config_set.py index c56e3979..40e6e329 100644 --- a/src/fabric_cli/commands/config/fab_config_set.py +++ b/src/fabric_cli/commands/config/fab_config_set.py @@ -76,13 +76,64 @@ def _set_config(args: Namespace, key: str, value: Any, verbose: bool = True) -> Context().cleanup_context_files(cleanup_all_stale=True, cleanup_current=True) - if ( - key == fab_constant.FAB_MODE - and current_mode == fab_constant.FAB_MODE_COMMANDLINE - and previous_mode == fab_constant.FAB_MODE_INTERACTIVE - ): - utils_ui.print("Exiting interactive mode. Goodbye!") - os._exit(0) + # Enhanced mode transition handling + if key == fab_constant.FAB_MODE: + if (current_mode == fab_constant.FAB_MODE_INTERACTIVE + and previous_mode == fab_constant.FAB_MODE_COMMANDLINE): + # Handle command_line → interactive transition + if _is_user_authenticated(): + utils_ui.print("Switching to interactive mode...") + _start_interactive_mode(args) + else: + utils_ui.print("Please login first to use interactive mode") + + elif (current_mode == fab_constant.FAB_MODE_COMMANDLINE + and previous_mode == fab_constant.FAB_MODE_INTERACTIVE): + # Handle interactive → command_line transition + utils_ui.print("Exiting interactive mode. Goodbye!") + os._exit(0) + + +def _is_user_authenticated() -> bool: + """Check if user has valid authentication tokens""" + try: + from fabric_cli.core.fab_auth import FabAuth + auth = FabAuth() + # Try to get a token without interactive renewal + token = auth.get_access_token( + fab_constant.SCOPE_FABRIC_DEFAULT, + interactive_renew=False + ) + return token is not None + except FabricCLIError as e: + # Handle specific authentication errors + if e.status_code in [ + fab_constant.ERROR_UNAUTHORIZED, + fab_constant.ERROR_AUTHENTICATION_FAILED, + ]: + return False + raise e + except Exception: + return False + + +def _start_interactive_mode(args: Namespace) -> None: + """Launch interactive mode with current parser context""" + try: + # Import parser setup from main module + from fabric_cli.main import _create_parser_and_subparsers + + parser, subparsers = _create_parser_and_subparsers() + + from fabric_cli.core.fab_interactive import InteractiveCLI + interactive_cli = InteractiveCLI(parser, subparsers) + interactive_cli.start_interactive() + + except (KeyboardInterrupt, EOFError): + utils_ui.print("Interactive mode cancelled.") + except Exception as e: + utils_ui.print(f"Failed to start interactive mode: {str(e)}") + utils_ui.print("Please restart the CLI to use interactive mode.") def _set_capacity(args: Namespace, value: str) -> None: diff --git a/src/fabric_cli/main.py b/src/fabric_cli/main.py index d9d95d41..92590b33 100644 --- a/src/fabric_cli/main.py +++ b/src/fabric_cli/main.py @@ -168,7 +168,8 @@ def error(self, message): sys.exit(2) -def main(): +def _create_parser_and_subparsers(): + """Create parser and subparsers for reuse in interactive mode transition""" parser = CustomArgumentParser(description="Fabric CLI") # -c option for command line execution @@ -224,6 +225,12 @@ def main(): ) version_parser.set_defaults(func=fab_ui.print_version) + return parser, subparsers + + +def main(): + parser, subparsers = _create_parser_and_subparsers() + argcomplete.autocomplete(parser, default_completer=None) args = parser.parse_args() diff --git a/tests/test_commands/test_config.py b/tests/test_commands/test_config.py index 7f670d90..c50a7ae5 100644 --- a/tests/test_commands/test_config.py +++ b/tests/test_commands/test_config.py @@ -1,9 +1,11 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. -from unittest.mock import patch +from unittest.mock import patch, MagicMock +import pytest import fabric_cli.core.fab_constant as constant +from fabric_cli.core.fab_exceptions import FabricCLIError from fabric_cli.errors import ErrorMessages from tests.test_commands.commands_parser import CLIExecutor from tests.test_commands.data.static_test_data import StaticTestData @@ -171,3 +173,183 @@ def test_config_clear_cache_success( mock_print_done.assert_called_once() # endregion + + # region config MODE SWITCHING + def test_success_config_set_mode_interactive_authenticated_success( + self, mock_questionary_print, mock_fab_set_state_config, cli_executor: CLIExecutor + ): + """Test successful transition from command_line to interactive mode when authenticated""" + with patch("fabric_cli.commands.config.fab_config_set._is_user_authenticated", return_value=True), \ + patch("fabric_cli.commands.config.fab_config_set._start_interactive_mode") as mock_start_interactive: + + mock_fab_set_state_config(constant.FAB_MODE, constant.FAB_MODE_COMMANDLINE) + + # Execute command + cli_executor.exec_command(f"config set mode {constant.FAB_MODE_INTERACTIVE}") + + # Assert + mock_questionary_print.assert_called() + mock_start_interactive.assert_called_once() + assert mock_questionary_print.call_args[0][0] == 'Switching to interactive mode...' + + + def test_config_set_mode_interactive_user_not_authenticated_failure( + self, mock_fab_set_state_config, mock_questionary_print, cli_executor: CLIExecutor + ): + """Test transition from command_line to interactive mode when not authenticated""" + with patch("fabric_cli.commands.config.fab_config_set._is_user_authenticated", return_value=False), \ + patch("fabric_cli.commands.config.fab_config_set._start_interactive_mode") as mock_start_interactive: + + mock_fab_set_state_config(constant.FAB_MODE, constant.FAB_MODE_COMMANDLINE) + + # Execute command + cli_executor.exec_command(f"config set mode {constant.FAB_MODE_INTERACTIVE}") + + # Assert + mock_questionary_print.assert_called() + assert mock_questionary_print.call_args[0][0] == "Please login first to use interactive mode" + mock_start_interactive.assert_not_called() + + def test_config_set_mode_command_line_from_interactive_success( + self, mock_fab_set_state_config, mock_questionary_print, cli_executor: CLIExecutor + ): + """Test transition from interactive to command_line mode""" + with patch("os._exit") as mock_exit: + + mock_fab_set_state_config(constant.FAB_MODE, constant.FAB_MODE_INTERACTIVE) + # Execute command + cli_executor.exec_command(f"config set mode {constant.FAB_MODE_COMMANDLINE}") + + # Assert + mock_questionary_print.assert_called() + assert mock_questionary_print.call_args[0][0] == "Exiting interactive mode. Goodbye!" + mock_exit.assert_called_once_with(0) + + def test_is_user_authenticated_with_valid_token_success(self): + """Test _is_user_authenticated returns True when user has valid token""" + from fabric_cli.commands.config.fab_config_set import _is_user_authenticated + + with patch("fabric_cli.core.fab_auth.FabAuth") as mock_fab_auth: + mock_auth_instance = MagicMock() + mock_auth_instance.get_access_token.return_value = "valid_token" + mock_fab_auth.return_value = mock_auth_instance + + result = _is_user_authenticated() + + assert result is True + mock_auth_instance.get_access_token.assert_called_once_with( + constant.SCOPE_FABRIC_DEFAULT, interactive_renew=False + ) + + def test_is_user_authenticated_with_no_token_failure(self): + """Test _is_user_authenticated returns False when user has no token""" + from fabric_cli.commands.config.fab_config_set import _is_user_authenticated + + with patch("fabric_cli.core.fab_auth.FabAuth") as mock_fab_auth: + mock_auth_instance = MagicMock() + mock_auth_instance.get_access_token.return_value = None + mock_fab_auth.return_value = mock_auth_instance + + result = _is_user_authenticated() + + assert result is False + + def test_is_user_authenticated_with_authentication_error_failure(self): + """Test _is_user_authenticated returns False when authentication fails""" + from fabric_cli.commands.config.fab_config_set import _is_user_authenticated + + with patch("fabric_cli.core.fab_auth.FabAuth") as mock_fab_auth: + mock_auth_instance = MagicMock() + mock_auth_instance.get_access_token.side_effect = FabricCLIError( + "Authentication failed", constant.ERROR_AUTHENTICATION_FAILED + ) + mock_fab_auth.return_value = mock_auth_instance + + result = _is_user_authenticated() + + assert result is False + + def test_is_user_authenticated_with_unexpected_error_failure(self): + """Test _is_user_authenticated returns False on unexpected error""" + from fabric_cli.commands.config.fab_config_set import _is_user_authenticated + + with patch("fabric_cli.core.fab_auth.FabAuth") as mock_fab_auth: + mock_auth_instance = MagicMock() + mock_auth_instance.get_access_token.side_effect = Exception("Unexpected error") + mock_fab_auth.return_value = mock_auth_instance + + result = _is_user_authenticated() + + assert result is False + + def test_start_interactive_mode_success(self): + """Test _start_interactive_mode successfully launches interactive CLI""" + from fabric_cli.commands.config.fab_config_set import _start_interactive_mode + from argparse import Namespace + + args = Namespace() + + with patch("fabric_cli.main._create_parser_and_subparsers") as mock_create_parser, \ + patch("fabric_cli.core.fab_interactive.InteractiveCLI") as mock_interactive_cli: + + mock_parser = MagicMock() + mock_subparsers = MagicMock() + mock_create_parser.return_value = (mock_parser, mock_subparsers) + + mock_cli_instance = MagicMock() + mock_interactive_cli.return_value = mock_cli_instance + + _start_interactive_mode(args) + + # Assert + mock_create_parser.assert_called_once() + mock_interactive_cli.assert_called_once_with(mock_parser, mock_subparsers) + mock_cli_instance.start_interactive.assert_called_once() + + def test_start_interactive_mode_keyboard_interrupt_success(self, mock_questionary_print): + """Test _start_interactive_mode handles KeyboardInterrupt gracefully""" + from fabric_cli.commands.config.fab_config_set import _start_interactive_mode + from argparse import Namespace + + args = Namespace() + + with patch("fabric_cli.main._create_parser_and_subparsers") as mock_create_parser, \ + patch("fabric_cli.core.fab_interactive.InteractiveCLI") as mock_interactive_cli: + + mock_parser = MagicMock() + mock_subparsers = MagicMock() + mock_create_parser.return_value = (mock_parser, mock_subparsers) + + mock_cli_instance = MagicMock() + mock_cli_instance.start_interactive.side_effect = KeyboardInterrupt() + mock_interactive_cli.return_value = mock_cli_instance + + _start_interactive_mode(args) + + # Assert + mock_questionary_print.call_args[0][0] == "Interactive mode cancelled." + + def test_start_interactive_mode_exception_handling_failure(self, mock_questionary_print): + """Test _start_interactive_mode handles general exceptions""" + from fabric_cli.commands.config.fab_config_set import _start_interactive_mode + from argparse import Namespace + + args = Namespace() + + with patch("fabric_cli.main._create_parser_and_subparsers") as mock_create_parser, \ + patch("fabric_cli.core.fab_interactive.InteractiveCLI") as mock_interactive_cli: + + mock_parser = MagicMock() + mock_subparsers = MagicMock() + mock_create_parser.return_value = (mock_parser, mock_subparsers) + + mock_cli_instance = MagicMock() + mock_cli_instance.start_interactive.side_effect = Exception("Test error") + mock_interactive_cli.return_value = mock_cli_instance + + _start_interactive_mode(args) + + # Assert + mock_questionary_print.call_args[0][0] == "Please restart the CLI to use interactive mode." + + # endregion From 1cbc5e0c447cc1d3d57fd5cd934f9425cf1da897 Mon Sep 17 00:00:00 2001 From: aviat cohen Date: Wed, 3 Dec 2025 15:54:32 +0000 Subject: [PATCH 02/13] add changie --- .changes/unreleased/fixed-20251203-155412.yaml | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 .changes/unreleased/fixed-20251203-155412.yaml diff --git a/.changes/unreleased/fixed-20251203-155412.yaml b/.changes/unreleased/fixed-20251203-155412.yaml new file mode 100644 index 00000000..13c394ca --- /dev/null +++ b/.changes/unreleased/fixed-20251203-155412.yaml @@ -0,0 +1,3 @@ +kind: fixed +body: Avoid re‑authentication when switching from command‑line to interactive mode. +time: 2025-12-03T15:54:12.961104889Z From 42021e35f0917602da976a1b942251298c9a7c8e Mon Sep 17 00:00:00 2001 From: aviat cohen Date: Sun, 7 Dec 2025 12:38:54 +0000 Subject: [PATCH 03/13] remove authentication request when changing to interactive --- .../commands/config/fab_config_set.py | 82 ++---- src/fabric_cli/core/fab_parser_setup.py | 243 ++++++++++++++++++ src/fabric_cli/main.py | 217 +--------------- src/fabric_cli/utils/fab_cmd_config_utils.py | 23 ++ tests/test_commands/test_config.py | 142 +--------- 5 files changed, 294 insertions(+), 413 deletions(-) create mode 100644 src/fabric_cli/core/fab_parser_setup.py create mode 100644 src/fabric_cli/utils/fab_cmd_config_utils.py diff --git a/src/fabric_cli/commands/config/fab_config_set.py b/src/fabric_cli/commands/config/fab_config_set.py index 40e6e329..d8f03df4 100644 --- a/src/fabric_cli/commands/config/fab_config_set.py +++ b/src/fabric_cli/commands/config/fab_config_set.py @@ -68,72 +68,9 @@ def _set_config(args: Namespace, key: str, value: Any, verbose: bool = True) -> utils_ui.print_output_format( args, message=f"Configuration '{key}' set to '{value}'" ) - current_mode = fab_state_config.get_config(fab_constant.FAB_MODE) - # Clean up context files when changing mode if key == fab_constant.FAB_MODE: - from fabric_cli.core.fab_context import Context - - Context().cleanup_context_files(cleanup_all_stale=True, cleanup_current=True) - - # Enhanced mode transition handling - if key == fab_constant.FAB_MODE: - if (current_mode == fab_constant.FAB_MODE_INTERACTIVE - and previous_mode == fab_constant.FAB_MODE_COMMANDLINE): - # Handle command_line → interactive transition - if _is_user_authenticated(): - utils_ui.print("Switching to interactive mode...") - _start_interactive_mode(args) - else: - utils_ui.print("Please login first to use interactive mode") - - elif (current_mode == fab_constant.FAB_MODE_COMMANDLINE - and previous_mode == fab_constant.FAB_MODE_INTERACTIVE): - # Handle interactive → command_line transition - utils_ui.print("Exiting interactive mode. Goodbye!") - os._exit(0) - - -def _is_user_authenticated() -> bool: - """Check if user has valid authentication tokens""" - try: - from fabric_cli.core.fab_auth import FabAuth - auth = FabAuth() - # Try to get a token without interactive renewal - token = auth.get_access_token( - fab_constant.SCOPE_FABRIC_DEFAULT, - interactive_renew=False - ) - return token is not None - except FabricCLIError as e: - # Handle specific authentication errors - if e.status_code in [ - fab_constant.ERROR_UNAUTHORIZED, - fab_constant.ERROR_AUTHENTICATION_FAILED, - ]: - return False - raise e - except Exception: - return False - - -def _start_interactive_mode(args: Namespace) -> None: - """Launch interactive mode with current parser context""" - try: - # Import parser setup from main module - from fabric_cli.main import _create_parser_and_subparsers - - parser, subparsers = _create_parser_and_subparsers() - - from fabric_cli.core.fab_interactive import InteractiveCLI - interactive_cli = InteractiveCLI(parser, subparsers) - interactive_cli.start_interactive() - - except (KeyboardInterrupt, EOFError): - utils_ui.print("Interactive mode cancelled.") - except Exception as e: - utils_ui.print(f"Failed to start interactive mode: {str(e)}") - utils_ui.print("Please restart the CLI to use interactive mode.") + _handle_fab_config_mode(previous_mode, value) def _set_capacity(args: Namespace, value: str) -> None: @@ -153,3 +90,20 @@ def _set_capacity(args: Namespace, value: str) -> None: ErrorMessages.Config.invalid_capacity(value), fab_constant.ERROR_INVALID_INPUT, ) + + +def _handle_fab_config_mode(previous_mode: str, current_mode: str) -> None: + from fabric_cli.core.fab_context import Context + # Clean up context files when changing mode + Context().cleanup_context_files(cleanup_all_stale=True, cleanup_current=True) + + if (current_mode == fab_constant.FAB_MODE_INTERACTIVE + and previous_mode == fab_constant.FAB_MODE_COMMANDLINE): + utils_ui.print("Switching to interactive mode...") + from fabric_cli.utils.fab_cmd_config_utils import start_interactive_mode + start_interactive_mode() + + elif (current_mode == fab_constant.FAB_MODE_COMMANDLINE + and previous_mode == fab_constant.FAB_MODE_INTERACTIVE): + utils_ui.print("Exiting interactive mode. Goodbye!") + os._exit(0) \ No newline at end of file diff --git a/src/fabric_cli/core/fab_parser_setup.py b/src/fabric_cli/core/fab_parser_setup.py new file mode 100644 index 00000000..67898cbc --- /dev/null +++ b/src/fabric_cli/core/fab_parser_setup.py @@ -0,0 +1,243 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +import argparse +import re +import sys + +import argcomplete + +from fabric_cli.core import fab_constant, fab_logger +from fabric_cli.parsers import fab_acls_parser as acls_parser +from fabric_cli.parsers import fab_api_parser as api_parser +from fabric_cli.parsers import fab_auth_parser as auth_parser +from fabric_cli.parsers import fab_config_parser as config_parser +from fabric_cli.parsers import fab_describe_parser as describe_parser +from fabric_cli.parsers import fab_extension_parser as extension_parser +from fabric_cli.parsers import fab_fs_parser as fs_parser +from fabric_cli.parsers import fab_global_params +from fabric_cli.parsers import fab_jobs_parser as jobs_parser +from fabric_cli.parsers import fab_labels_parser as labels_parser +from fabric_cli.parsers import fab_tables_parser as tables_parser +from fabric_cli.utils import fab_error_parser as utils_error_parser +from fabric_cli.utils import fab_ui +from fabric_cli.utils.fab_commands import COMMANDS + + +class CustomHelpFormatter(argparse.HelpFormatter): + + def __init__( + self, + prog, + fab_examples=None, + fab_aliases=None, + fab_learnmore=None, + *args, + **kwargs, + ): + super().__init__(prog, *args, **kwargs) + self.fab_examples = fab_examples or [] + self.fab_aliases = fab_aliases or [] + self.fab_learnmore = fab_learnmore or [] + + def _format_args(self, action, default_metavar): + if action.nargs in ("*", "+"): + if action.option_strings: + return "" + else: + # Ensure metavar is lowercase for positional arguments + return f"<{action.dest}>" + return super()._format_args(action, default_metavar) + + def _format_action_invocation(self, action): + if not action.metavar and action.nargs in (None, "?"): + # For no metavar and simple arguments + return ", ".join(action.option_strings) + elif action.nargs in ("*", "+"): + metavar = self._format_args(action, action.dest) + return ", ".join(action.option_strings) + metavar + else: + return super()._format_action_invocation(action) + + def format_help(self): + help_message = super().format_help() + + # Custom output + help_message = help_message.replace("usage:", "Usage:") + help_message = help_message.replace("positional arguments:", "Arg(s):") + help_message = help_message.replace("options:", "Flags:") + + help_message = re.sub( + r"\s*-h, --help\s*(Show help for command|show this help message and exit)?", + "", + help_message, + ) + help_message = help_message.replace(" -help\n", "") + help_message = help_message.replace("[-h] ", "") + help_message = help_message.replace("[-help] ", "") + help_message = help_message.replace("[-help]", "") + + if "Flags:" in help_message: + flags_section = help_message.split("Flags:")[1].strip() + if not flags_section: # If no flags follow the "Flags:" line, remove it + help_message = help_message.replace("\nFlags:\n", "") + + # Add aliases + if self.fab_aliases: + help_message += "\nAliases:\n" + for alias in self.fab_aliases: + help_message += f" {alias}\n" + + # Add examples + if self.fab_examples: + help_message += "\nExamples:\n" + for example in self.fab_examples: + if "#" in example: + # Grey color + help_message += f" \033[38;5;243m{example}\033[0m\n" + else: + help_message += f" {example}\n" + + # Add learn more + if self.fab_learnmore: + help_message += "\nLearn more:\n" + if self.fab_learnmore != ["_"]: + for learn_more in self.fab_learnmore: + help_message += f" {learn_more}\n" + help_message += " For more usage examples, see https://aka.ms/fabric-cli\n" + + return help_message + "\n" + + +class CustomArgumentParser(argparse.ArgumentParser): + def __init__( + self, *args, fab_examples=None, fab_aliases=None, fab_learnmore=None, **kwargs + ): + kwargs["formatter_class"] = lambda prog: CustomHelpFormatter( + prog, + fab_examples=fab_examples, + fab_aliases=fab_aliases, + fab_learnmore=fab_learnmore, + ) + super().__init__(*args, **kwargs) + # Add custom help and format flags + fab_global_params.add_global_flags(self) + self.fab_mode = fab_constant.FAB_MODE_COMMANDLINE + self.fab_examples = fab_examples or [] + self.fab_aliases = fab_aliases or [] + + def print_help(self, file=None): + command_name = self.prog.split()[-1] + + help_functions = { + "acl": lambda: acls_parser.show_help(None), + "job": lambda: jobs_parser.show_help(None), + "label": lambda: labels_parser.show_help(None), + "table": lambda: tables_parser.show_help(None), + "auth": lambda: auth_parser.show_help(None), + "config": lambda: config_parser.show_help(None), + "fab": lambda: fab_ui.display_help(COMMANDS), + } + + if command_name in help_functions: + help_functions[command_name]() + else: + super().print_help(file) + + def set_mode(self, mode): + self.fab_mode = mode + + def get_mode(self): + return self.fab_mode + + def error(self, message): + if "invalid choice" in message: + utils_error_parser.invalid_choice(self, message) + elif "unrecognized arguments" in message: + utils_error_parser.unrecognized_arguments(message) + elif "the following arguments are required" in message: + utils_error_parser.missing_required_arguments(message) + else: + # Add more custom error parsers here + fab_logger.log_warning(message) + + if self.fab_mode == fab_constant.FAB_MODE_COMMANDLINE: + sys.exit(2) + + +def create_parser_and_subparsers(): + """Create parser and subparsers for reuse across CLI modes""" + parser = CustomArgumentParser(description="Fabric CLI") + + # -c option for command line execution + parser.add_argument( + "-c", + "--command", + action="append", # Allow multiple -c options + help="Run commands in non-interactive mode", + ) + + # -version and --version + parser.add_argument("-v", "--version", action="store_true") + + subparsers = parser.add_subparsers(dest="command", required=False) + + # Custom parsers + config_parser.register_parser(subparsers) + + # Single parsers + fs_parser.register_ls_parser(subparsers) # ls + fs_parser.register_mkdir_parser(subparsers) # mkdir + fs_parser.register_cd_parser(subparsers) # cd + fs_parser.register_rm_parser(subparsers) # rm + fs_parser.register_mv_parser(subparsers) # mv + fs_parser.register_cp_parser(subparsers) # cp + fs_parser.register_exists_parser(subparsers) # exists + fs_parser.register_pwd_parser(subparsers) # pwd + fs_parser.register_open_parser(subparsers) # open + fs_parser.register_export_parser(subparsers) # export + fs_parser.register_import_parser(subparsers) # import + fs_parser.register_set_parser(subparsers) # set + fs_parser.register_get_parser(subparsers) # get + fs_parser.register_clear_parser(subparsers) # clear + fs_parser.register_ln_parser(subparsers) # ln + fs_parser.register_start_parser(subparsers) # start + fs_parser.register_stop_parser(subparsers) # stop + fs_parser.register_assign_parser(subparsers) # assign + fs_parser.register_unassign_parser(subparsers) # unassign + + jobs_parser.register_parser(subparsers) # jobs + tables_parser.register_parser(subparsers) # tables + acls_parser.register_parser(subparsers) # acls + labels_parser.register_parser(subparsers) # labels + + api_parser.register_parser(subparsers) # api + auth_parser.register_parser(subparsers) # auth + describe_parser.register_parser(subparsers) # desc + extension_parser.register_parser(subparsers) # extension + + # version + version_parser = subparsers.add_parser( + "version", help=fab_constant.COMMAND_VERSION_DESCRIPTION + ) + version_parser.set_defaults(func=fab_ui.print_version) + + return parser, subparsers + + +def start_interactive_mode(parser=None, subparsers=None): + """Launch interactive mode with shared parser context""" + try: + # Create parsers if not provided + if parser is None or subparsers is None: + parser, subparsers = create_parser_and_subparsers() + + from fabric_cli.core.fab_interactive import InteractiveCLI + interactive_cli = InteractiveCLI(parser, subparsers) + interactive_cli.start_interactive() + + except (KeyboardInterrupt, EOFError): + fab_ui.print("Interactive mode cancelled.") + except Exception as e: + fab_ui.print(f"Failed to start interactive mode: {str(e)}") + fab_ui.print("Please restart the CLI to use interactive mode.") \ No newline at end of file diff --git a/src/fabric_cli/main.py b/src/fabric_cli/main.py index 92590b33..52e9cfa2 100644 --- a/src/fabric_cli/main.py +++ b/src/fabric_cli/main.py @@ -25,211 +25,12 @@ from fabric_cli.utils import fab_error_parser as utils_error_parser from fabric_cli.utils import fab_ui from fabric_cli.utils.fab_commands import COMMANDS - - -class CustomHelpFormatter(argparse.HelpFormatter): - - def __init__( - self, - prog, - fab_examples=None, - fab_aliases=None, - fab_learnmore=None, - *args, - **kwargs, - ): - super().__init__(prog, *args, **kwargs) - self.fab_examples = fab_examples or [] - self.fab_aliases = fab_aliases or [] - self.fab_learnmore = fab_learnmore or [] - - def _format_args(self, action, default_metavar): - if action.nargs in ("*", "+"): - if action.option_strings: - return "" - else: - # Ensure metavar is lowercase for positional arguments - return f"<{action.dest}>" - return super()._format_args(action, default_metavar) - - def _format_action_invocation(self, action): - if not action.metavar and action.nargs in (None, "?"): - # For no metavar and simple arguments - return ", ".join(action.option_strings) - elif action.nargs in ("*", "+"): - metavar = self._format_args(action, action.dest) - return ", ".join(action.option_strings) + metavar - else: - return super()._format_action_invocation(action) - - def format_help(self): - help_message = super().format_help() - - # Custom output - help_message = help_message.replace("usage:", "Usage:") - help_message = help_message.replace("positional arguments:", "Arg(s):") - help_message = help_message.replace("options:", "Flags:") - - help_message = re.sub( - r"\s*-h, --help\s*(Show help for command|show this help message and exit)?", - "", - help_message, - ) - help_message = help_message.replace(" -help\n", "") - help_message = help_message.replace("[-h] ", "") - help_message = help_message.replace("[-help] ", "") - help_message = help_message.replace("[-help]", "") - - if "Flags:" in help_message: - flags_section = help_message.split("Flags:")[1].strip() - if not flags_section: # If no flags follow the "Flags:" line, remove it - help_message = help_message.replace("\nFlags:\n", "") - - # Add aliases - if self.fab_aliases: - help_message += "\nAliases:\n" - for alias in self.fab_aliases: - help_message += f" {alias}\n" - - # Add examples - if self.fab_examples: - help_message += "\nExamples:\n" - for example in self.fab_examples: - if "#" in example: - # Grey color - help_message += f" \033[38;5;243m{example}\033[0m\n" - else: - help_message += f" {example}\n" - - # Add learn more - if self.fab_learnmore: - help_message += "\nLearn more:\n" - if self.fab_learnmore != ["_"]: - for learn_more in self.fab_learnmore: - help_message += f" {learn_more}\n" - help_message += " For more usage examples, see https://aka.ms/fabric-cli\n" - - return help_message + "\n" - - -class CustomArgumentParser(argparse.ArgumentParser): - def __init__( - self, *args, fab_examples=None, fab_aliases=None, fab_learnmore=None, **kwargs - ): - kwargs["formatter_class"] = lambda prog: CustomHelpFormatter( - prog, - fab_examples=fab_examples, - fab_aliases=fab_aliases, - fab_learnmore=fab_learnmore, - ) - super().__init__(*args, **kwargs) - # Add custom help and format flags - fab_global_params.add_global_flags(self) - self.fab_mode = fab_constant.FAB_MODE_COMMANDLINE - self.fab_examples = fab_examples or [] - self.fab_aliases = fab_aliases or [] - - def print_help(self, file=None): - command_name = self.prog.split()[-1] - - help_functions = { - "acl": lambda: acls_parser.show_help(None), - "job": lambda: jobs_parser.show_help(None), - "label": lambda: labels_parser.show_help(None), - "table": lambda: tables_parser.show_help(None), - "auth": lambda: auth_parser.show_help(None), - "config": lambda: config_parser.show_help(None), - "fab": lambda: fab_ui.display_help(COMMANDS), - } - - if command_name in help_functions: - help_functions[command_name]() - else: - super().print_help(file) - - def set_mode(self, mode): - self.fab_mode = mode - - def get_mode(self): - return self.fab_mode - - def error(self, message): - if "invalid choice" in message: - utils_error_parser.invalid_choice(self, message) - elif "unrecognized arguments" in message: - utils_error_parser.unrecognized_arguments(message) - elif "the following arguments are required" in message: - utils_error_parser.missing_required_arguments(message) - else: - # Add more custom error parsers here - fab_logger.log_warning(message) - - if self.fab_mode == fab_constant.FAB_MODE_COMMANDLINE: - sys.exit(2) - - -def _create_parser_and_subparsers(): - """Create parser and subparsers for reuse in interactive mode transition""" - parser = CustomArgumentParser(description="Fabric CLI") - - # -c option for command line execution - parser.add_argument( - "-c", - "--command", - action="append", # Allow multiple -c options - help="Run commands in non-interactive mode", - ) - - # -version and --version - parser.add_argument("-v", "--version", action="store_true") - - subparsers = parser.add_subparsers(dest="command", required=False) - - # Custom parsers - config_parser.register_parser(subparsers) - - # Single parsers - fs_parser.register_ls_parser(subparsers) # ls - fs_parser.register_mkdir_parser(subparsers) # mkdir - fs_parser.register_cd_parser(subparsers) # cd - fs_parser.register_rm_parser(subparsers) # rm - fs_parser.register_mv_parser(subparsers) # mv - fs_parser.register_cp_parser(subparsers) # cp - fs_parser.register_exists_parser(subparsers) # exists - fs_parser.register_pwd_parser(subparsers) # pwd - fs_parser.register_open_parser(subparsers) # open - fs_parser.register_export_parser(subparsers) # export - fs_parser.register_import_parser(subparsers) # import - fs_parser.register_set_parser(subparsers) # set - fs_parser.register_get_parser(subparsers) # get - fs_parser.register_clear_parser(subparsers) # clear - fs_parser.register_ln_parser(subparsers) # ln - fs_parser.register_start_parser(subparsers) # start - fs_parser.register_stop_parser(subparsers) # stop - fs_parser.register_assign_parser(subparsers) # assign - fs_parser.register_unassign_parser(subparsers) # unassign - - jobs_parser.register_parser(subparsers) # jobs - tables_parser.register_parser(subparsers) # tables - acls_parser.register_parser(subparsers) # acls - labels_parser.register_parser(subparsers) # labels - - api_parser.register_parser(subparsers) # api - auth_parser.register_parser(subparsers) # auth - describe_parser.register_parser(subparsers) # desc - extension_parser.register_parser(subparsers) # extension - - # version - version_parser = subparsers.add_parser( - "version", help=fab_constant.COMMAND_VERSION_DESCRIPTION - ) - version_parser.set_defaults(func=fab_ui.print_version) - - return parser, subparsers +from fabric_cli.utils.fab_cmd_config_utils import start_interactive_mode +from fabric_cli.core.fab_parser_setup import create_parser_and_subparsers def main(): - parser, subparsers = _create_parser_and_subparsers() + parser, subparsers = create_parser_and_subparsers() argcomplete.autocomplete(parser, default_completer=None) @@ -247,16 +48,8 @@ def main(): fab_state_config.get_config(fab_constant.FAB_MODE) == fab_constant.FAB_MODE_INTERACTIVE ): - # Initialize InteractiveCLI - from fabric_cli.core.fab_interactive import InteractiveCLI - - try: - interactive_cli = InteractiveCLI(parser, subparsers) - interactive_cli.start_interactive() - except (KeyboardInterrupt, EOFError): - fab_ui.print( - "\nInteractive mode cancelled. Returning to previous menu." - ) + # Use shared interactive mode startup + start_interactive_mode(parser, subparsers) if args.command == "auth" and args.auth_command == "logout": login.logout(args) diff --git a/src/fabric_cli/utils/fab_cmd_config_utils.py b/src/fabric_cli/utils/fab_cmd_config_utils.py new file mode 100644 index 00000000..aeb45cf5 --- /dev/null +++ b/src/fabric_cli/utils/fab_cmd_config_utils.py @@ -0,0 +1,23 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +from fabric_cli.utils import fab_ui + + +def start_interactive_mode(parser=None, subparsers=None): + """Launch interactive mode with shared parser context""" + try: + # Create parsers if not provided + if parser is None or subparsers is None: + from fabric_cli.core.fab_parser_setup import create_parser_and_subparsers + parser, subparsers = create_parser_and_subparsers() + + from fabric_cli.core.fab_interactive import InteractiveCLI + interactive_cli = InteractiveCLI(parser, subparsers) + interactive_cli.start_interactive() + + except (KeyboardInterrupt, EOFError): + fab_ui.print("Interactive mode cancelled.") + except Exception as e: + fab_ui.print(f"Failed to start interactive mode: {str(e)}") + fab_ui.print("Please restart the CLI to use interactive mode.") \ No newline at end of file diff --git a/tests/test_commands/test_config.py b/tests/test_commands/test_config.py index c50a7ae5..6e403eab 100644 --- a/tests/test_commands/test_config.py +++ b/tests/test_commands/test_config.py @@ -175,12 +175,11 @@ def test_config_clear_cache_success( # endregion # region config MODE SWITCHING - def test_success_config_set_mode_interactive_authenticated_success( + def test_config_set_mode_interactive_success( self, mock_questionary_print, mock_fab_set_state_config, cli_executor: CLIExecutor ): - """Test successful transition from command_line to interactive mode when authenticated""" - with patch("fabric_cli.commands.config.fab_config_set._is_user_authenticated", return_value=True), \ - patch("fabric_cli.commands.config.fab_config_set._start_interactive_mode") as mock_start_interactive: + """Test successful transition from command_line to interactive mode""" + with patch("fabric_cli.commands.config.fab_config_set._start_interactive_mode") as mock_start_interactive: mock_fab_set_state_config(constant.FAB_MODE, constant.FAB_MODE_COMMANDLINE) @@ -192,24 +191,6 @@ def test_success_config_set_mode_interactive_authenticated_success( mock_start_interactive.assert_called_once() assert mock_questionary_print.call_args[0][0] == 'Switching to interactive mode...' - - def test_config_set_mode_interactive_user_not_authenticated_failure( - self, mock_fab_set_state_config, mock_questionary_print, cli_executor: CLIExecutor - ): - """Test transition from command_line to interactive mode when not authenticated""" - with patch("fabric_cli.commands.config.fab_config_set._is_user_authenticated", return_value=False), \ - patch("fabric_cli.commands.config.fab_config_set._start_interactive_mode") as mock_start_interactive: - - mock_fab_set_state_config(constant.FAB_MODE, constant.FAB_MODE_COMMANDLINE) - - # Execute command - cli_executor.exec_command(f"config set mode {constant.FAB_MODE_INTERACTIVE}") - - # Assert - mock_questionary_print.assert_called() - assert mock_questionary_print.call_args[0][0] == "Please login first to use interactive mode" - mock_start_interactive.assert_not_called() - def test_config_set_mode_command_line_from_interactive_success( self, mock_fab_set_state_config, mock_questionary_print, cli_executor: CLIExecutor ): @@ -225,63 +206,6 @@ def test_config_set_mode_command_line_from_interactive_success( assert mock_questionary_print.call_args[0][0] == "Exiting interactive mode. Goodbye!" mock_exit.assert_called_once_with(0) - def test_is_user_authenticated_with_valid_token_success(self): - """Test _is_user_authenticated returns True when user has valid token""" - from fabric_cli.commands.config.fab_config_set import _is_user_authenticated - - with patch("fabric_cli.core.fab_auth.FabAuth") as mock_fab_auth: - mock_auth_instance = MagicMock() - mock_auth_instance.get_access_token.return_value = "valid_token" - mock_fab_auth.return_value = mock_auth_instance - - result = _is_user_authenticated() - - assert result is True - mock_auth_instance.get_access_token.assert_called_once_with( - constant.SCOPE_FABRIC_DEFAULT, interactive_renew=False - ) - - def test_is_user_authenticated_with_no_token_failure(self): - """Test _is_user_authenticated returns False when user has no token""" - from fabric_cli.commands.config.fab_config_set import _is_user_authenticated - - with patch("fabric_cli.core.fab_auth.FabAuth") as mock_fab_auth: - mock_auth_instance = MagicMock() - mock_auth_instance.get_access_token.return_value = None - mock_fab_auth.return_value = mock_auth_instance - - result = _is_user_authenticated() - - assert result is False - - def test_is_user_authenticated_with_authentication_error_failure(self): - """Test _is_user_authenticated returns False when authentication fails""" - from fabric_cli.commands.config.fab_config_set import _is_user_authenticated - - with patch("fabric_cli.core.fab_auth.FabAuth") as mock_fab_auth: - mock_auth_instance = MagicMock() - mock_auth_instance.get_access_token.side_effect = FabricCLIError( - "Authentication failed", constant.ERROR_AUTHENTICATION_FAILED - ) - mock_fab_auth.return_value = mock_auth_instance - - result = _is_user_authenticated() - - assert result is False - - def test_is_user_authenticated_with_unexpected_error_failure(self): - """Test _is_user_authenticated returns False on unexpected error""" - from fabric_cli.commands.config.fab_config_set import _is_user_authenticated - - with patch("fabric_cli.core.fab_auth.FabAuth") as mock_fab_auth: - mock_auth_instance = MagicMock() - mock_auth_instance.get_access_token.side_effect = Exception("Unexpected error") - mock_fab_auth.return_value = mock_auth_instance - - result = _is_user_authenticated() - - assert result is False - def test_start_interactive_mode_success(self): """Test _start_interactive_mode successfully launches interactive CLI""" from fabric_cli.commands.config.fab_config_set import _start_interactive_mode @@ -289,67 +213,11 @@ def test_start_interactive_mode_success(self): args = Namespace() - with patch("fabric_cli.main._create_parser_and_subparsers") as mock_create_parser, \ - patch("fabric_cli.core.fab_interactive.InteractiveCLI") as mock_interactive_cli: - - mock_parser = MagicMock() - mock_subparsers = MagicMock() - mock_create_parser.return_value = (mock_parser, mock_subparsers) - - mock_cli_instance = MagicMock() - mock_interactive_cli.return_value = mock_cli_instance - - _start_interactive_mode(args) - - # Assert - mock_create_parser.assert_called_once() - mock_interactive_cli.assert_called_once_with(mock_parser, mock_subparsers) - mock_cli_instance.start_interactive.assert_called_once() - - def test_start_interactive_mode_keyboard_interrupt_success(self, mock_questionary_print): - """Test _start_interactive_mode handles KeyboardInterrupt gracefully""" - from fabric_cli.commands.config.fab_config_set import _start_interactive_mode - from argparse import Namespace - - args = Namespace() - - with patch("fabric_cli.main._create_parser_and_subparsers") as mock_create_parser, \ - patch("fabric_cli.core.fab_interactive.InteractiveCLI") as mock_interactive_cli: - - mock_parser = MagicMock() - mock_subparsers = MagicMock() - mock_create_parser.return_value = (mock_parser, mock_subparsers) - - mock_cli_instance = MagicMock() - mock_cli_instance.start_interactive.side_effect = KeyboardInterrupt() - mock_interactive_cli.return_value = mock_cli_instance - - _start_interactive_mode(args) - - # Assert - mock_questionary_print.call_args[0][0] == "Interactive mode cancelled." - - def test_start_interactive_mode_exception_handling_failure(self, mock_questionary_print): - """Test _start_interactive_mode handles general exceptions""" - from fabric_cli.commands.config.fab_config_set import _start_interactive_mode - from argparse import Namespace - - args = Namespace() - - with patch("fabric_cli.main._create_parser_and_subparsers") as mock_create_parser, \ - patch("fabric_cli.core.fab_interactive.InteractiveCLI") as mock_interactive_cli: - - mock_parser = MagicMock() - mock_subparsers = MagicMock() - mock_create_parser.return_value = (mock_parser, mock_subparsers) - - mock_cli_instance = MagicMock() - mock_cli_instance.start_interactive.side_effect = Exception("Test error") - mock_interactive_cli.return_value = mock_cli_instance + with patch("fabric_cli.utils.fab_cmd_config_utils.start_interactive_mode") as mock_start_interactive: _start_interactive_mode(args) # Assert - mock_questionary_print.call_args[0][0] == "Please restart the CLI to use interactive mode." + mock_start_interactive.assert_called_once() # endregion From 29bf53a5e38d43f4a4c2df756d60e80834518e0d Mon Sep 17 00:00:00 2001 From: aviat cohen Date: Sun, 7 Dec 2025 13:07:02 +0000 Subject: [PATCH 04/13] fix type check; remove unused commants --- src/fabric_cli/main.py | 13 ------------- tests/test_commands/commands_parser.py | 2 +- 2 files changed, 1 insertion(+), 14 deletions(-) diff --git a/src/fabric_cli/main.py b/src/fabric_cli/main.py index 52e9cfa2..5fb34bbc 100644 --- a/src/fabric_cli/main.py +++ b/src/fabric_cli/main.py @@ -1,8 +1,6 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. -import argparse -import re import sys import argcomplete @@ -11,18 +9,7 @@ from fabric_cli.core import fab_constant, fab_logger, fab_state_config from fabric_cli.core.fab_commands import Command from fabric_cli.core.fab_exceptions import FabricCLIError -from fabric_cli.parsers import fab_acls_parser as acls_parser -from fabric_cli.parsers import fab_api_parser as api_parser from fabric_cli.parsers import fab_auth_parser as auth_parser -from fabric_cli.parsers import fab_config_parser as config_parser -from fabric_cli.parsers import fab_describe_parser as describe_parser -from fabric_cli.parsers import fab_extension_parser as extension_parser -from fabric_cli.parsers import fab_fs_parser as fs_parser -from fabric_cli.parsers import fab_global_params -from fabric_cli.parsers import fab_jobs_parser as jobs_parser -from fabric_cli.parsers import fab_labels_parser as labels_parser -from fabric_cli.parsers import fab_tables_parser as tables_parser -from fabric_cli.utils import fab_error_parser as utils_error_parser from fabric_cli.utils import fab_ui from fabric_cli.utils.fab_commands import COMMANDS from fabric_cli.utils.fab_cmd_config_utils import start_interactive_mode diff --git a/tests/test_commands/commands_parser.py b/tests/test_commands/commands_parser.py index e4a81360..a6f91033 100644 --- a/tests/test_commands/commands_parser.py +++ b/tests/test_commands/commands_parser.py @@ -8,7 +8,7 @@ from prompt_toolkit.history import InMemoryHistory from fabric_cli.core.fab_interactive import InteractiveCLI -from fabric_cli.main import CustomArgumentParser +from fabric_cli.core.fab_parser_setup import CustomArgumentParser from fabric_cli.parsers.fab_acls_parser import register_parser as register_acls_parser from fabric_cli.parsers.fab_api_parser import register_parser as register_api_parser from fabric_cli.parsers.fab_config_parser import ( From ff880e6cec163ea3c635ab34a1a157c62db0c36f Mon Sep 17 00:00:00 2001 From: aviat cohen Date: Mon, 8 Dec 2025 11:02:57 +0000 Subject: [PATCH 05/13] move create parsers to globle param for reuse --- src/fabric_cli/core/fab_parser_setup.py | 28 ++++++++--------- src/fabric_cli/main.py | 6 ++-- src/fabric_cli/utils/fab_cmd_config_utils.py | 12 +++----- tests/test_commands/test_config.py | 32 +++++++++++--------- 4 files changed, 37 insertions(+), 41 deletions(-) diff --git a/src/fabric_cli/core/fab_parser_setup.py b/src/fabric_cli/core/fab_parser_setup.py index 67898cbc..ae91d37a 100644 --- a/src/fabric_cli/core/fab_parser_setup.py +++ b/src/fabric_cli/core/fab_parser_setup.py @@ -165,6 +165,10 @@ def error(self, message): sys.exit(2) +# Global parser instances +_global_parser = None +_global_subparsers = None + def create_parser_and_subparsers(): """Create parser and subparsers for reuse across CLI modes""" parser = CustomArgumentParser(description="Fabric CLI") @@ -224,20 +228,12 @@ def create_parser_and_subparsers(): return parser, subparsers +def get_global_parser_and_subparsers(): + """Get singleton parser and subparsers instances""" + global _global_parser, _global_subparsers + + if _global_parser is None: + _global_parser, _global_subparsers = create_parser_and_subparsers() + + return _global_parser, _global_subparsers -def start_interactive_mode(parser=None, subparsers=None): - """Launch interactive mode with shared parser context""" - try: - # Create parsers if not provided - if parser is None or subparsers is None: - parser, subparsers = create_parser_and_subparsers() - - from fabric_cli.core.fab_interactive import InteractiveCLI - interactive_cli = InteractiveCLI(parser, subparsers) - interactive_cli.start_interactive() - - except (KeyboardInterrupt, EOFError): - fab_ui.print("Interactive mode cancelled.") - except Exception as e: - fab_ui.print(f"Failed to start interactive mode: {str(e)}") - fab_ui.print("Please restart the CLI to use interactive mode.") \ No newline at end of file diff --git a/src/fabric_cli/main.py b/src/fabric_cli/main.py index 5fb34bbc..1ed78401 100644 --- a/src/fabric_cli/main.py +++ b/src/fabric_cli/main.py @@ -13,11 +13,11 @@ from fabric_cli.utils import fab_ui from fabric_cli.utils.fab_commands import COMMANDS from fabric_cli.utils.fab_cmd_config_utils import start_interactive_mode -from fabric_cli.core.fab_parser_setup import create_parser_and_subparsers +from fabric_cli.core.fab_parser_setup import get_global_parser_and_subparsers def main(): - parser, subparsers = create_parser_and_subparsers() + parser, subparsers = get_global_parser_and_subparsers() argcomplete.autocomplete(parser, default_completer=None) @@ -36,7 +36,7 @@ def main(): == fab_constant.FAB_MODE_INTERACTIVE ): # Use shared interactive mode startup - start_interactive_mode(parser, subparsers) + start_interactive_mode() if args.command == "auth" and args.auth_command == "logout": login.logout(args) diff --git a/src/fabric_cli/utils/fab_cmd_config_utils.py b/src/fabric_cli/utils/fab_cmd_config_utils.py index aeb45cf5..04f886a1 100644 --- a/src/fabric_cli/utils/fab_cmd_config_utils.py +++ b/src/fabric_cli/utils/fab_cmd_config_utils.py @@ -4,15 +4,13 @@ from fabric_cli.utils import fab_ui -def start_interactive_mode(parser=None, subparsers=None): - """Launch interactive mode with shared parser context""" +def start_interactive_mode(): + """Launch interactive mode using global parser instances""" try: - # Create parsers if not provided - if parser is None or subparsers is None: - from fabric_cli.core.fab_parser_setup import create_parser_and_subparsers - parser, subparsers = create_parser_and_subparsers() - + from fabric_cli.core.fab_parser_setup import get_global_parser_and_subparsers from fabric_cli.core.fab_interactive import InteractiveCLI + + parser, subparsers = get_global_parser_and_subparsers() interactive_cli = InteractiveCLI(parser, subparsers) interactive_cli.start_interactive() diff --git a/tests/test_commands/test_config.py b/tests/test_commands/test_config.py index 6e403eab..f6bcbb21 100644 --- a/tests/test_commands/test_config.py +++ b/tests/test_commands/test_config.py @@ -1,11 +1,9 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. -from unittest.mock import patch, MagicMock -import pytest +from unittest.mock import patch import fabric_cli.core.fab_constant as constant -from fabric_cli.core.fab_exceptions import FabricCLIError from fabric_cli.errors import ErrorMessages from tests.test_commands.commands_parser import CLIExecutor from tests.test_commands.data.static_test_data import StaticTestData @@ -179,7 +177,7 @@ def test_config_set_mode_interactive_success( self, mock_questionary_print, mock_fab_set_state_config, cli_executor: CLIExecutor ): """Test successful transition from command_line to interactive mode""" - with patch("fabric_cli.commands.config.fab_config_set._start_interactive_mode") as mock_start_interactive: + with patch("fabric_cli.utils.fab_cmd_config_utils.start_interactive_mode") as mock_start_interactive: mock_fab_set_state_config(constant.FAB_MODE, constant.FAB_MODE_COMMANDLINE) @@ -188,7 +186,7 @@ def test_config_set_mode_interactive_success( # Assert mock_questionary_print.assert_called() - mock_start_interactive.assert_called_once() + mock_start_interactive.assert_called_once_with() assert mock_questionary_print.call_args[0][0] == 'Switching to interactive mode...' def test_config_set_mode_command_line_from_interactive_success( @@ -207,17 +205,21 @@ def test_config_set_mode_command_line_from_interactive_success( mock_exit.assert_called_once_with(0) def test_start_interactive_mode_success(self): - """Test _start_interactive_mode successfully launches interactive CLI""" - from fabric_cli.commands.config.fab_config_set import _start_interactive_mode - from argparse import Namespace - - args = Namespace() - - with patch("fabric_cli.utils.fab_cmd_config_utils.start_interactive_mode") as mock_start_interactive: + """Test mode switching creates parsers and launches interactive CLI""" + with patch("fabric_cli.core.fab_parser_setup.get_global_parser_and_subparsers") as mock_get_parsers, \ + patch("fabric_cli.core.fab_interactive.InteractiveCLI") as mock_interactive_cli: - _start_interactive_mode(args) + mock_parser = object() + mock_subparsers = object() + mock_get_parsers.return_value = (mock_parser, mock_subparsers) - # Assert - mock_start_interactive.assert_called_once() + mock_cli_instance = mock_interactive_cli.return_value + + from fabric_cli.utils.fab_cmd_config_utils import start_interactive_mode + start_interactive_mode() + + mock_get_parsers.assert_called_once() + mock_interactive_cli.assert_called_once_with(mock_parser, mock_subparsers) + mock_cli_instance.start_interactive.assert_called_once() # endregion From 2b4a01f144ff8a2509865f5740dde7ec21feb6d7 Mon Sep 17 00:00:00 2001 From: aviat cohen Date: Thu, 11 Dec 2025 11:39:47 +0000 Subject: [PATCH 06/13] move start_interactive_mode to fab_interactive --- .../commands/config/fab_config_set.py | 2 +- src/fabric_cli/core/fab_interactive.py | 16 ++++++++++++++ src/fabric_cli/main.py | 2 +- src/fabric_cli/utils/fab_cmd_config_utils.py | 21 ------------------- tests/test_commands/test_config.py | 4 ++-- 5 files changed, 20 insertions(+), 25 deletions(-) delete mode 100644 src/fabric_cli/utils/fab_cmd_config_utils.py diff --git a/src/fabric_cli/commands/config/fab_config_set.py b/src/fabric_cli/commands/config/fab_config_set.py index d8f03df4..3527af40 100644 --- a/src/fabric_cli/commands/config/fab_config_set.py +++ b/src/fabric_cli/commands/config/fab_config_set.py @@ -100,7 +100,7 @@ def _handle_fab_config_mode(previous_mode: str, current_mode: str) -> None: if (current_mode == fab_constant.FAB_MODE_INTERACTIVE and previous_mode == fab_constant.FAB_MODE_COMMANDLINE): utils_ui.print("Switching to interactive mode...") - from fabric_cli.utils.fab_cmd_config_utils import start_interactive_mode + from fabric_cli.core.fab_interactive import start_interactive_mode start_interactive_mode() elif (current_mode == fab_constant.FAB_MODE_COMMANDLINE diff --git a/src/fabric_cli/core/fab_interactive.py b/src/fabric_cli/core/fab_interactive.py index 7216f72b..03dc5fce 100644 --- a/src/fabric_cli/core/fab_interactive.py +++ b/src/fabric_cli/core/fab_interactive.py @@ -114,3 +114,19 @@ def start_interactive(self): except (EOFError, KeyboardInterrupt): utils_ui.print(f"\n{fab_constant.INTERACTIVE_EXIT_MESSAGE}") break + + +def start_interactive_mode(): + """Launch interactive mode using global parser instances""" + try: + from fabric_cli.core.fab_parser_setup import get_global_parser_and_subparsers + + parser, subparsers = get_global_parser_and_subparsers() + interactive_cli = InteractiveCLI(parser, subparsers) + interactive_cli.start_interactive() + + except (KeyboardInterrupt, EOFError): + utils_ui.print("Interactive mode cancelled.") + except Exception as e: + utils_ui.print(f"Failed to start interactive mode: {str(e)}") + utils_ui.print("Please restart the CLI to use interactive mode.") diff --git a/src/fabric_cli/main.py b/src/fabric_cli/main.py index 1ed78401..6d576695 100644 --- a/src/fabric_cli/main.py +++ b/src/fabric_cli/main.py @@ -12,7 +12,7 @@ from fabric_cli.parsers import fab_auth_parser as auth_parser from fabric_cli.utils import fab_ui from fabric_cli.utils.fab_commands import COMMANDS -from fabric_cli.utils.fab_cmd_config_utils import start_interactive_mode +from fabric_cli.core.fab_interactive import start_interactive_mode from fabric_cli.core.fab_parser_setup import get_global_parser_and_subparsers diff --git a/src/fabric_cli/utils/fab_cmd_config_utils.py b/src/fabric_cli/utils/fab_cmd_config_utils.py deleted file mode 100644 index 04f886a1..00000000 --- a/src/fabric_cli/utils/fab_cmd_config_utils.py +++ /dev/null @@ -1,21 +0,0 @@ -# Copyright (c) Microsoft Corporation. -# Licensed under the MIT License. - -from fabric_cli.utils import fab_ui - - -def start_interactive_mode(): - """Launch interactive mode using global parser instances""" - try: - from fabric_cli.core.fab_parser_setup import get_global_parser_and_subparsers - from fabric_cli.core.fab_interactive import InteractiveCLI - - parser, subparsers = get_global_parser_and_subparsers() - interactive_cli = InteractiveCLI(parser, subparsers) - interactive_cli.start_interactive() - - except (KeyboardInterrupt, EOFError): - fab_ui.print("Interactive mode cancelled.") - except Exception as e: - fab_ui.print(f"Failed to start interactive mode: {str(e)}") - fab_ui.print("Please restart the CLI to use interactive mode.") \ No newline at end of file diff --git a/tests/test_commands/test_config.py b/tests/test_commands/test_config.py index f6bcbb21..5a5c2e32 100644 --- a/tests/test_commands/test_config.py +++ b/tests/test_commands/test_config.py @@ -177,7 +177,7 @@ def test_config_set_mode_interactive_success( self, mock_questionary_print, mock_fab_set_state_config, cli_executor: CLIExecutor ): """Test successful transition from command_line to interactive mode""" - with patch("fabric_cli.utils.fab_cmd_config_utils.start_interactive_mode") as mock_start_interactive: + with patch("fabric_cli.core.fab_interactive.start_interactive_mode") as mock_start_interactive: mock_fab_set_state_config(constant.FAB_MODE, constant.FAB_MODE_COMMANDLINE) @@ -215,7 +215,7 @@ def test_start_interactive_mode_success(self): mock_cli_instance = mock_interactive_cli.return_value - from fabric_cli.utils.fab_cmd_config_utils import start_interactive_mode + from fabric_cli.core.fab_interactive import start_interactive_mode start_interactive_mode() mock_get_parsers.assert_called_once() From 3318916f7df11a0c8e7286aee887a78ee100f9ba Mon Sep 17 00:00:00 2001 From: aviat cohen Date: Mon, 15 Dec 2025 14:18:31 +0000 Subject: [PATCH 07/13] convert InteractiveCLI to sigleton; remove previous mode check in _handle_fab_config_mode --- .../commands/config/fab_config_set.py | 3 +- src/fabric_cli/core/fab_interactive.py | 94 +++++++++++++------ tests/test_commands/test_config.py | 76 ++++++++++++--- 3 files changed, 130 insertions(+), 43 deletions(-) diff --git a/src/fabric_cli/commands/config/fab_config_set.py b/src/fabric_cli/commands/config/fab_config_set.py index 3527af40..becad7d3 100644 --- a/src/fabric_cli/commands/config/fab_config_set.py +++ b/src/fabric_cli/commands/config/fab_config_set.py @@ -97,8 +97,7 @@ def _handle_fab_config_mode(previous_mode: str, current_mode: str) -> None: # Clean up context files when changing mode Context().cleanup_context_files(cleanup_all_stale=True, cleanup_current=True) - if (current_mode == fab_constant.FAB_MODE_INTERACTIVE - and previous_mode == fab_constant.FAB_MODE_COMMANDLINE): + if current_mode == fab_constant.FAB_MODE_INTERACTIVE: utils_ui.print("Switching to interactive mode...") from fabric_cli.core.fab_interactive import start_interactive_mode start_interactive_mode() diff --git a/src/fabric_cli/core/fab_interactive.py b/src/fabric_cli/core/fab_interactive.py index 03dc5fce..0acb4598 100644 --- a/src/fabric_cli/core/fab_interactive.py +++ b/src/fabric_cli/core/fab_interactive.py @@ -17,7 +17,21 @@ class InteractiveCLI: - def __init__(self, parser, subparsers): + _instance = None + + def __new__(cls, parser=None, subparsers=None): + if cls._instance is None: + cls._instance = super(InteractiveCLI, cls).__new__(cls) + # Initialize the instance immediately after creation + cls._instance._init_instance(parser, subparsers) + return cls._instance + + def _init_instance(self, parser=None, subparsers=None): + """Initialize the singleton instance""" + if parser is None or subparsers is None: + from fabric_cli.core.fab_parser_setup import get_global_parser_and_subparsers + parser, subparsers = get_global_parser_and_subparsers() + self.parser = parser self.parser.set_mode(fab_constant.FAB_MODE_INTERACTIVE) self.subparsers = subparsers @@ -31,6 +45,21 @@ def __init__(self, parser, subparsers): ("input", "fg:white"), # Input color ] ) + self._is_running = False + + def __init__(self, parser=None, subparsers=None): + # __init__ is called after __new__, but we've already initialized in __new__ + pass + + @classmethod + def get_instance(cls, parser=None, subparsers=None): + """Get or create the singleton instance""" + return cls(parser, subparsers) + + @classmethod + def reset_instance(cls): + """Reset the singleton instance (mainly for testing)""" + cls._instance = None def init_session(self, session_history: InMemoryHistory) -> PromptSession: return PromptSession(history=session_history) @@ -89,40 +118,45 @@ def handle_command(self, command): def start_interactive(self): """Start the interactive mode using prompt_toolkit for input.""" - utils_ui.print("\nWelcome to the Fabric CLI ⚡") - utils_ui.print("Type 'help' for help. \n") - - while True: - try: - context = Context().context - pwd_context = f"/{context.path.strip('/')}" - - prompt_text = HTML( - f"fab:{html.escape(pwd_context)}$ " - ) - - user_input = self.session.prompt( - prompt_text, - style=self.custom_style, - cursor=CursorShape.BLINKING_BEAM, - enable_history_search=True, - ) - should_exit = self.handle_command(user_input) - if should_exit: # Check if the command was to exit - break + if self._is_running: + utils_ui.print("Interactive mode is already running.") + return - except (EOFError, KeyboardInterrupt): - utils_ui.print(f"\n{fab_constant.INTERACTIVE_EXIT_MESSAGE}") - break + self._is_running = True + try: + utils_ui.print("\nWelcome to the Fabric CLI ⚡") + utils_ui.print("Type 'help' for help. \n") + + while True: + try: + context = Context().context + pwd_context = f"/{context.path.strip('/')}" + + prompt_text = HTML( + f"fab:{html.escape(pwd_context)}$ " + ) + + user_input = self.session.prompt( + prompt_text, + style=self.custom_style, + cursor=CursorShape.BLINKING_BEAM, + enable_history_search=True, + ) + should_exit = self.handle_command(user_input) + if should_exit: # Check if the command was to exit + break + + except (EOFError, KeyboardInterrupt): + utils_ui.print(f"\n{fab_constant.INTERACTIVE_EXIT_MESSAGE}") + break + finally: + self._is_running = False def start_interactive_mode(): - """Launch interactive mode using global parser instances""" + """Launch interactive mode using singleton pattern""" try: - from fabric_cli.core.fab_parser_setup import get_global_parser_and_subparsers - - parser, subparsers = get_global_parser_and_subparsers() - interactive_cli = InteractiveCLI(parser, subparsers) + interactive_cli = InteractiveCLI.get_instance() interactive_cli.start_interactive() except (KeyboardInterrupt, EOFError): diff --git a/tests/test_commands/test_config.py b/tests/test_commands/test_config.py index 5a5c2e32..619393b0 100644 --- a/tests/test_commands/test_config.py +++ b/tests/test_commands/test_config.py @@ -176,7 +176,7 @@ def test_config_clear_cache_success( def test_config_set_mode_interactive_success( self, mock_questionary_print, mock_fab_set_state_config, cli_executor: CLIExecutor ): - """Test successful transition from command_line to interactive mode""" + """Test successful transition to interactive mode""" with patch("fabric_cli.core.fab_interactive.start_interactive_mode") as mock_start_interactive: mock_fab_set_state_config(constant.FAB_MODE, constant.FAB_MODE_COMMANDLINE) @@ -189,6 +189,22 @@ def test_config_set_mode_interactive_success( mock_start_interactive.assert_called_once_with() assert mock_questionary_print.call_args[0][0] == 'Switching to interactive mode...' + def test_config_set_mode_interactive_from_interactive_success( + self, mock_questionary_print, mock_fab_set_state_config, cli_executor: CLIExecutor + ): + """Test setting interactive mode while already in interactive mode""" + with patch("fabric_cli.core.fab_interactive.start_interactive_mode") as mock_start_interactive: + + mock_fab_set_state_config(constant.FAB_MODE, constant.FAB_MODE_INTERACTIVE) + + # Execute command + cli_executor.exec_command(f"config set mode {constant.FAB_MODE_INTERACTIVE}") + + # Assert + mock_questionary_print.assert_called() + mock_start_interactive.assert_called_once_with() + assert mock_questionary_print.call_args[0][0] == 'Switching to interactive mode...' + def test_config_set_mode_command_line_from_interactive_success( self, mock_fab_set_state_config, mock_questionary_print, cli_executor: CLIExecutor ): @@ -205,21 +221,59 @@ def test_config_set_mode_command_line_from_interactive_success( mock_exit.assert_called_once_with(0) def test_start_interactive_mode_success(self): - """Test mode switching creates parsers and launches interactive CLI""" - with patch("fabric_cli.core.fab_parser_setup.get_global_parser_and_subparsers") as mock_get_parsers, \ - patch("fabric_cli.core.fab_interactive.InteractiveCLI") as mock_interactive_cli: - - mock_parser = object() - mock_subparsers = object() - mock_get_parsers.return_value = (mock_parser, mock_subparsers) + """Test mode switching creates singleton and launches interactive CLI""" + with patch("fabric_cli.core.fab_interactive.InteractiveCLI.get_instance") as mock_get_instance: - mock_cli_instance = mock_interactive_cli.return_value + mock_cli_instance = mock_get_instance.return_value from fabric_cli.core.fab_interactive import start_interactive_mode start_interactive_mode() - mock_get_parsers.assert_called_once() - mock_interactive_cli.assert_called_once_with(mock_parser, mock_subparsers) + mock_get_instance.assert_called_once() mock_cli_instance.start_interactive.assert_called_once() + def test_start_interactive_mode_already_running(self): + """Test that calling start_interactive_mode when already running prints message""" + with patch("fabric_cli.core.fab_interactive.InteractiveCLI.get_instance") as mock_get_instance, \ + patch("fabric_cli.core.fab_interactive.utils_ui") as mock_utils_ui: + + from fabric_cli.core import fab_interactive + + # Reset singleton state first + fab_interactive.InteractiveCLI.reset_instance() + + mock_cli_instance = mock_get_instance.return_value + mock_cli_instance._is_running = True + + fab_interactive.start_interactive_mode() + + # Should call get_instance and then start_interactive should print message + mock_get_instance.assert_called_once() + mock_cli_instance.start_interactive.assert_called_once() + + # Reset singleton state + fab_interactive.InteractiveCLI.reset_instance() + + def test_interactive_cli_singleton_pattern(self): + """Test that InteractiveCLI follows singleton pattern""" + from fabric_cli.core.fab_interactive import InteractiveCLI + + # Reset singleton state + InteractiveCLI.reset_instance() + + with patch("fabric_cli.core.fab_parser_setup.get_global_parser_and_subparsers") as mock_get_parsers: + mock_parser = type('MockParser', (), {'set_mode': lambda self, mode: None})() + mock_subparsers = object() + mock_get_parsers.return_value = (mock_parser, mock_subparsers) + + # Create two instances + instance1 = InteractiveCLI.get_instance() + instance2 = InteractiveCLI.get_instance() + + # Should be the same instance + assert instance1 is instance2 + + # Reset singleton state + InteractiveCLI.reset_instance() + # endregion From f4112cc621e5bc2269580b05af6305e315ae4bc5 Mon Sep 17 00:00:00 2001 From: aviat cohen Date: Mon, 15 Dec 2025 14:54:00 +0000 Subject: [PATCH 08/13] fix tests --- tests/test_core/test_fab_interactive.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/test_core/test_fab_interactive.py b/tests/test_core/test_fab_interactive.py index 57b3e1dc..c4f8a12a 100644 --- a/tests/test_core/test_fab_interactive.py +++ b/tests/test_core/test_fab_interactive.py @@ -414,5 +414,13 @@ def interactive_cli( mock_parser, mock_subparsers, mock_in_memory_history, mock_prompt_session ): """Create InteractiveCLI instance with mocked dependencies.""" - cli = InteractiveCLI(mock_parser, mock_subparsers) - return cli + # Reset singleton state before each test + InteractiveCLI.reset_instance() + + # Mock the get_global_parser_and_subparsers function to return our mocks + with patch("fabric_cli.core.fab_parser_setup.get_global_parser_and_subparsers") as mock_get_parsers: + mock_get_parsers.return_value = (mock_parser, mock_subparsers) + cli = InteractiveCLI.get_instance() + yield cli + # Reset after test + InteractiveCLI.reset_instance() From 51abddd2063c683594abc0609864c6534f3e0db9 Mon Sep 17 00:00:00 2001 From: aviat cohen Date: Tue, 23 Dec 2025 11:11:50 +0000 Subject: [PATCH 09/13] move singleton to fab-decorators and reuse it. fix pr comments --- src/fabric_cli/core/fab_interactive.py | 50 +++++++++++--------------- 1 file changed, 21 insertions(+), 29 deletions(-) diff --git a/src/fabric_cli/core/fab_interactive.py b/src/fabric_cli/core/fab_interactive.py index 0acb4598..c68291c4 100644 --- a/src/fabric_cli/core/fab_interactive.py +++ b/src/fabric_cli/core/fab_interactive.py @@ -123,44 +123,36 @@ def start_interactive(self): return self._is_running = True + try: utils_ui.print("\nWelcome to the Fabric CLI ⚡") utils_ui.print("Type 'help' for help. \n") while True: - try: - context = Context().context - pwd_context = f"/{context.path.strip('/')}" - - prompt_text = HTML( - f"fab:{html.escape(pwd_context)}$ " - ) - - user_input = self.session.prompt( - prompt_text, - style=self.custom_style, - cursor=CursorShape.BLINKING_BEAM, - enable_history_search=True, - ) - should_exit = self.handle_command(user_input) - if should_exit: # Check if the command was to exit - break - - except (EOFError, KeyboardInterrupt): - utils_ui.print(f"\n{fab_constant.INTERACTIVE_EXIT_MESSAGE}") + context = Context().context + pwd_context = f"/{context.path.strip('/')}" + + prompt_text = HTML( + f"fab:{html.escape(pwd_context)}$ " + ) + + user_input = self.session.prompt( + prompt_text, + style=self.custom_style, + cursor=CursorShape.BLINKING_BEAM, + enable_history_search=True, + ) + should_exit = self.handle_command(user_input) + if should_exit: # Check if the command was to exit break + + except (EOFError, KeyboardInterrupt): + utils_ui.print(f"\n{fab_constant.INTERACTIVE_EXIT_MESSAGE}") finally: self._is_running = False def start_interactive_mode(): """Launch interactive mode using singleton pattern""" - try: - interactive_cli = InteractiveCLI.get_instance() - interactive_cli.start_interactive() - - except (KeyboardInterrupt, EOFError): - utils_ui.print("Interactive mode cancelled.") - except Exception as e: - utils_ui.print(f"Failed to start interactive mode: {str(e)}") - utils_ui.print("Please restart the CLI to use interactive mode.") + interactive_cli = InteractiveCLI.get_instance() + interactive_cli.start_interactive() From 2d28babff77392fae1310b45b39e38d9818c21d3 Mon Sep 17 00:00:00 2001 From: aviat cohen Date: Wed, 24 Dec 2025 08:54:18 +0000 Subject: [PATCH 10/13] move singelton to fab_decorators --- src/fabric_cli/core/fab_context.py | 12 +--------- src/fabric_cli/core/fab_decorators.py | 18 +++++++++++++- src/fabric_cli/core/fab_interactive.py | 31 ++++--------------------- tests/test_commands/commands_parser.py | 30 +++++++++++++----------- tests/test_commands/test_config.py | 18 +++++++------- tests/test_core/test_fab_interactive.py | 2 +- 6 files changed, 50 insertions(+), 61 deletions(-) diff --git a/src/fabric_cli/core/fab_context.py b/src/fabric_cli/core/fab_context.py index a6cc171b..4a576056 100644 --- a/src/fabric_cli/core/fab_context.py +++ b/src/fabric_cli/core/fab_context.py @@ -10,6 +10,7 @@ import psutil from fabric_cli.core import fab_constant, fab_logger, fab_state_config +from fabric_cli.core.fab_decorators import singleton from fabric_cli.core.fab_exceptions import FabricCLIError from fabric_cli.core.hiearchy.fab_element import FabricElement from fabric_cli.core.hiearchy.fab_tenant import Tenant @@ -17,17 +18,6 @@ from fabric_cli.utils import fab_ui as utils_ui -def singleton(class_): - instances = {} - - def getinstance(*args, **kwargs): - if class_ not in instances: - instances[class_] = class_(*args, **kwargs) - return instances[class_] - - return getinstance - - @singleton class Context: def __init__(self): diff --git a/src/fabric_cli/core/fab_decorators.py b/src/fabric_cli/core/fab_decorators.py index d59b2fb0..3e050625 100644 --- a/src/fabric_cli/core/fab_decorators.py +++ b/src/fabric_cli/core/fab_decorators.py @@ -9,11 +9,25 @@ EXIT_CODE_AUTHORIZATION_REQUIRED, EXIT_CODE_ERROR, ) -from fabric_cli.core.fab_context import Context from fabric_cli.core.fab_exceptions import FabricCLIError from fabric_cli.utils import fab_ui +def singleton(class_): + """Singleton decorator that ensures only one instance of a class exists.""" + instances = {} + + def getinstance(*args, **kwargs): + if class_ not in instances: + instances[class_] = class_(*args, **kwargs) + return instances[class_] + + # Add a reset method to the decorator function for testing purposes + getinstance.reset_instance = lambda: instances.pop(class_, None) + + return getinstance + + def handle_exceptions(): """ Decorator that catches FabricCLIError, logs the error, and returns @@ -51,6 +65,8 @@ def set_command_context(): def decorator(func): @wraps(func) def wrapper(*args, **kwargs): + # Import Context locally to avoid circular import + from fabric_cli.core.fab_context import Context Context().command = args[0].command_path return func(*args, **kwargs) diff --git a/src/fabric_cli/core/fab_interactive.py b/src/fabric_cli/core/fab_interactive.py index c68291c4..d5fa3a32 100644 --- a/src/fabric_cli/core/fab_interactive.py +++ b/src/fabric_cli/core/fab_interactive.py @@ -12,22 +12,15 @@ from fabric_cli.core import fab_constant, fab_logger from fabric_cli.core.fab_commands import Command from fabric_cli.core.fab_context import Context +from fabric_cli.core.fab_decorators import singleton from fabric_cli.utils import fab_commands from fabric_cli.utils import fab_ui as utils_ui +@singleton class InteractiveCLI: - _instance = None - - def __new__(cls, parser=None, subparsers=None): - if cls._instance is None: - cls._instance = super(InteractiveCLI, cls).__new__(cls) - # Initialize the instance immediately after creation - cls._instance._init_instance(parser, subparsers) - return cls._instance - - def _init_instance(self, parser=None, subparsers=None): - """Initialize the singleton instance""" + def __init__(self, parser=None, subparsers=None): + """Initialize the interactive CLI.""" if parser is None or subparsers is None: from fabric_cli.core.fab_parser_setup import get_global_parser_and_subparsers parser, subparsers = get_global_parser_and_subparsers() @@ -47,20 +40,6 @@ def _init_instance(self, parser=None, subparsers=None): ) self._is_running = False - def __init__(self, parser=None, subparsers=None): - # __init__ is called after __new__, but we've already initialized in __new__ - pass - - @classmethod - def get_instance(cls, parser=None, subparsers=None): - """Get or create the singleton instance""" - return cls(parser, subparsers) - - @classmethod - def reset_instance(cls): - """Reset the singleton instance (mainly for testing)""" - cls._instance = None - def init_session(self, session_history: InMemoryHistory) -> PromptSession: return PromptSession(history=session_history) @@ -154,5 +133,5 @@ def start_interactive(self): def start_interactive_mode(): """Launch interactive mode using singleton pattern""" - interactive_cli = InteractiveCLI.get_instance() + interactive_cli = InteractiveCLI() interactive_cli.start_interactive() diff --git a/tests/test_commands/commands_parser.py b/tests/test_commands/commands_parser.py index a6f91033..15f0125a 100644 --- a/tests/test_commands/commands_parser.py +++ b/tests/test_commands/commands_parser.py @@ -64,25 +64,29 @@ ] -class TestInteractiveCLI(InteractiveCLI): - def init_session(self, session_history: InMemoryHistory) -> PromptSession: - if platform.system() == "Windows": - # DummyInput and DummyOutput are test classes of prompt_toolkit to - # solve the NoConsoleScreenBufferError issue - return PromptSession( - history=session_history, input=DummyInput(), output=DummyOutput() - ) - - return super().init_session(session_history) - - class CLIExecutor: def __init__(self): customArgumentParser = CustomArgumentParser() self._parser = customArgumentParser.add_subparsers() for register_parser_handler in parserHandlers: register_parser_handler(self._parser) - self._interactiveCLI = TestInteractiveCLI(customArgumentParser, self._parser) + + # Reset singleton state to ensure clean test environment + InteractiveCLI.reset_instance() + self._interactiveCLI = InteractiveCLI(customArgumentParser, self._parser) + + # Override init_session for Windows compatibility + if platform.system() == "Windows": + original_init_session = self._interactiveCLI.init_session + def test_init_session(session_history: InMemoryHistory) -> PromptSession: + # DummyInput and DummyOutput are test classes of prompt_toolkit to + # solve the NoConsoleScreenBufferError issue + return PromptSession( + history=session_history, input=DummyInput(), output=DummyOutput() + ) + self._interactiveCLI.init_session = test_init_session + # Reinitialize the session with test-friendly settings + self._interactiveCLI.session = self._interactiveCLI.init_session(self._interactiveCLI.history) def exec_command(self, command: str) -> None: self._interactiveCLI.handle_command(command) diff --git a/tests/test_commands/test_config.py b/tests/test_commands/test_config.py index 619393b0..9e761a8f 100644 --- a/tests/test_commands/test_config.py +++ b/tests/test_commands/test_config.py @@ -222,19 +222,19 @@ def test_config_set_mode_command_line_from_interactive_success( def test_start_interactive_mode_success(self): """Test mode switching creates singleton and launches interactive CLI""" - with patch("fabric_cli.core.fab_interactive.InteractiveCLI.get_instance") as mock_get_instance: + with patch("fabric_cli.core.fab_interactive.InteractiveCLI") as mock_interactive_cli: - mock_cli_instance = mock_get_instance.return_value + mock_cli_instance = mock_interactive_cli.return_value from fabric_cli.core.fab_interactive import start_interactive_mode start_interactive_mode() - mock_get_instance.assert_called_once() + mock_interactive_cli.assert_called_once() mock_cli_instance.start_interactive.assert_called_once() def test_start_interactive_mode_already_running(self): """Test that calling start_interactive_mode when already running prints message""" - with patch("fabric_cli.core.fab_interactive.InteractiveCLI.get_instance") as mock_get_instance, \ + with patch("fabric_cli.core.fab_interactive.InteractiveCLI") as mock_interactive_cli, \ patch("fabric_cli.core.fab_interactive.utils_ui") as mock_utils_ui: from fabric_cli.core import fab_interactive @@ -242,13 +242,13 @@ def test_start_interactive_mode_already_running(self): # Reset singleton state first fab_interactive.InteractiveCLI.reset_instance() - mock_cli_instance = mock_get_instance.return_value + mock_cli_instance = mock_interactive_cli.return_value mock_cli_instance._is_running = True fab_interactive.start_interactive_mode() - # Should call get_instance and then start_interactive should print message - mock_get_instance.assert_called_once() + # Should call InteractiveCLI() and then start_interactive should print message + mock_interactive_cli.assert_called_once() mock_cli_instance.start_interactive.assert_called_once() # Reset singleton state @@ -267,8 +267,8 @@ def test_interactive_cli_singleton_pattern(self): mock_get_parsers.return_value = (mock_parser, mock_subparsers) # Create two instances - instance1 = InteractiveCLI.get_instance() - instance2 = InteractiveCLI.get_instance() + instance1 = InteractiveCLI() + instance2 = InteractiveCLI() # Should be the same instance assert instance1 is instance2 diff --git a/tests/test_core/test_fab_interactive.py b/tests/test_core/test_fab_interactive.py index c4f8a12a..6e15c339 100644 --- a/tests/test_core/test_fab_interactive.py +++ b/tests/test_core/test_fab_interactive.py @@ -420,7 +420,7 @@ def interactive_cli( # Mock the get_global_parser_and_subparsers function to return our mocks with patch("fabric_cli.core.fab_parser_setup.get_global_parser_and_subparsers") as mock_get_parsers: mock_get_parsers.return_value = (mock_parser, mock_subparsers) - cli = InteractiveCLI.get_instance() + cli = InteractiveCLI() yield cli # Reset after test InteractiveCLI.reset_instance() From 73ef8785cb5a16c26559bc4941b686f91cf1d342 Mon Sep 17 00:00:00 2001 From: aviat cohen Date: Wed, 24 Dec 2025 10:21:22 +0000 Subject: [PATCH 11/13] revert test commands_parser code --- tests/test_commands/commands_parser.py | 30 +++++++++++--------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/tests/test_commands/commands_parser.py b/tests/test_commands/commands_parser.py index 15f0125a..a6f91033 100644 --- a/tests/test_commands/commands_parser.py +++ b/tests/test_commands/commands_parser.py @@ -64,29 +64,25 @@ ] +class TestInteractiveCLI(InteractiveCLI): + def init_session(self, session_history: InMemoryHistory) -> PromptSession: + if platform.system() == "Windows": + # DummyInput and DummyOutput are test classes of prompt_toolkit to + # solve the NoConsoleScreenBufferError issue + return PromptSession( + history=session_history, input=DummyInput(), output=DummyOutput() + ) + + return super().init_session(session_history) + + class CLIExecutor: def __init__(self): customArgumentParser = CustomArgumentParser() self._parser = customArgumentParser.add_subparsers() for register_parser_handler in parserHandlers: register_parser_handler(self._parser) - - # Reset singleton state to ensure clean test environment - InteractiveCLI.reset_instance() - self._interactiveCLI = InteractiveCLI(customArgumentParser, self._parser) - - # Override init_session for Windows compatibility - if platform.system() == "Windows": - original_init_session = self._interactiveCLI.init_session - def test_init_session(session_history: InMemoryHistory) -> PromptSession: - # DummyInput and DummyOutput are test classes of prompt_toolkit to - # solve the NoConsoleScreenBufferError issue - return PromptSession( - history=session_history, input=DummyInput(), output=DummyOutput() - ) - self._interactiveCLI.init_session = test_init_session - # Reinitialize the session with test-friendly settings - self._interactiveCLI.session = self._interactiveCLI.init_session(self._interactiveCLI.history) + self._interactiveCLI = TestInteractiveCLI(customArgumentParser, self._parser) def exec_command(self, command: str) -> None: self._interactiveCLI.handle_command(command) From c03a198c9fb7b1841895c8b94f969708785a02f8 Mon Sep 17 00:00:00 2001 From: aviat cohen Date: Wed, 24 Dec 2025 10:33:19 +0000 Subject: [PATCH 12/13] remove inherit class since InteractiveCLI is decorated with @singleton --- tests/test_commands/commands_parser.py | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/tests/test_commands/commands_parser.py b/tests/test_commands/commands_parser.py index a6f91033..6035ad3c 100644 --- a/tests/test_commands/commands_parser.py +++ b/tests/test_commands/commands_parser.py @@ -64,25 +64,25 @@ ] -class TestInteractiveCLI(InteractiveCLI): - def init_session(self, session_history: InMemoryHistory) -> PromptSession: - if platform.system() == "Windows": - # DummyInput and DummyOutput are test classes of prompt_toolkit to - # solve the NoConsoleScreenBufferError issue - return PromptSession( - history=session_history, input=DummyInput(), output=DummyOutput() - ) - - return super().init_session(session_history) - - class CLIExecutor: def __init__(self): customArgumentParser = CustomArgumentParser() self._parser = customArgumentParser.add_subparsers() for register_parser_handler in parserHandlers: register_parser_handler(self._parser) - self._interactiveCLI = TestInteractiveCLI(customArgumentParser, self._parser) + self._interactiveCLI = InteractiveCLI(customArgumentParser, self._parser) + + # Override init_session for Windows compatibility + if platform.system() == "Windows": + def test_init_session(session_history: InMemoryHistory) -> PromptSession: + # DummyInput and DummyOutput are test classes of prompt_toolkit to + # solve the NoConsoleScreenBufferError issue + return PromptSession( + history=session_history, input=DummyInput(), output=DummyOutput() + ) + self._interactiveCLI.init_session = test_init_session + # Reinitialize the session with test-friendly settings + self._interactiveCLI.session = self._interactiveCLI.init_session(self._interactiveCLI.history) def exec_command(self, command: str) -> None: self._interactiveCLI.handle_command(command) From f52e108deae1f2fb1a587ef5e97f2c86ec0bd842 Mon Sep 17 00:00:00 2001 From: aviat cohen Date: Wed, 24 Dec 2025 12:08:44 +0000 Subject: [PATCH 13/13] remove reset_instance used only for tests --- src/fabric_cli/core/fab_decorators.py | 3 --- tests/test_commands/test_config.py | 12 ------------ tests/test_core/test_fab_interactive.py | 15 ++++++++++----- 3 files changed, 10 insertions(+), 20 deletions(-) diff --git a/src/fabric_cli/core/fab_decorators.py b/src/fabric_cli/core/fab_decorators.py index 3e050625..fb245957 100644 --- a/src/fabric_cli/core/fab_decorators.py +++ b/src/fabric_cli/core/fab_decorators.py @@ -21,9 +21,6 @@ def getinstance(*args, **kwargs): if class_ not in instances: instances[class_] = class_(*args, **kwargs) return instances[class_] - - # Add a reset method to the decorator function for testing purposes - getinstance.reset_instance = lambda: instances.pop(class_, None) return getinstance diff --git a/tests/test_commands/test_config.py b/tests/test_commands/test_config.py index 9e761a8f..3cedf1b0 100644 --- a/tests/test_commands/test_config.py +++ b/tests/test_commands/test_config.py @@ -239,9 +239,6 @@ def test_start_interactive_mode_already_running(self): from fabric_cli.core import fab_interactive - # Reset singleton state first - fab_interactive.InteractiveCLI.reset_instance() - mock_cli_instance = mock_interactive_cli.return_value mock_cli_instance._is_running = True @@ -250,17 +247,11 @@ def test_start_interactive_mode_already_running(self): # Should call InteractiveCLI() and then start_interactive should print message mock_interactive_cli.assert_called_once() mock_cli_instance.start_interactive.assert_called_once() - - # Reset singleton state - fab_interactive.InteractiveCLI.reset_instance() def test_interactive_cli_singleton_pattern(self): """Test that InteractiveCLI follows singleton pattern""" from fabric_cli.core.fab_interactive import InteractiveCLI - # Reset singleton state - InteractiveCLI.reset_instance() - with patch("fabric_cli.core.fab_parser_setup.get_global_parser_and_subparsers") as mock_get_parsers: mock_parser = type('MockParser', (), {'set_mode': lambda self, mode: None})() mock_subparsers = object() @@ -272,8 +263,5 @@ def test_interactive_cli_singleton_pattern(self): # Should be the same instance assert instance1 is instance2 - - # Reset singleton state - InteractiveCLI.reset_instance() # endregion diff --git a/tests/test_core/test_fab_interactive.py b/tests/test_core/test_fab_interactive.py index 6e15c339..b6a72631 100644 --- a/tests/test_core/test_fab_interactive.py +++ b/tests/test_core/test_fab_interactive.py @@ -414,13 +414,18 @@ def interactive_cli( mock_parser, mock_subparsers, mock_in_memory_history, mock_prompt_session ): """Create InteractiveCLI instance with mocked dependencies.""" - # Reset singleton state before each test - InteractiveCLI.reset_instance() + from fabric_cli.core.fab_interactive import InteractiveCLI # Mock the get_global_parser_and_subparsers function to return our mocks with patch("fabric_cli.core.fab_parser_setup.get_global_parser_and_subparsers") as mock_get_parsers: mock_get_parsers.return_value = (mock_parser, mock_subparsers) - cli = InteractiveCLI() + + # Create a fresh InteractiveCLI instance for each test by directly creating an instance + # and assigning the mock objects to ensure tests use the same mocks + cli = InteractiveCLI(mock_parser, mock_subparsers) + + # Ensure the instance uses our mock objects + cli.parser = mock_parser + cli.subparsers = mock_subparsers + yield cli - # Reset after test - InteractiveCLI.reset_instance()