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..fb245957 100644 --- a/src/fabric_cli/core/fab_decorators.py +++ b/src/fabric_cli/core/fab_decorators.py @@ -9,11 +9,22 @@ 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_] + + return getinstance + + def handle_exceptions(): """ Decorator that catches FabricCLIError, logs the error, and returns @@ -51,6 +62,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..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) diff --git a/tests/test_commands/test_config.py b/tests/test_commands/test_config.py index 619393b0..3cedf1b0 100644 --- a/tests/test_commands/test_config.py +++ b/tests/test_commands/test_config.py @@ -222,58 +222,46 @@ 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 - # 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 - 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() + instance1 = InteractiveCLI() + instance2 = InteractiveCLI() # 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 c4f8a12a..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.get_instance() + + # 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()