diff --git a/.changes/unreleased/optimization-20260322-103653.yaml b/.changes/unreleased/optimization-20260322-103653.yaml new file mode 100644 index 000000000..0fd6a2c76 --- /dev/null +++ b/.changes/unreleased/optimization-20260322-103653.yaml @@ -0,0 +1,6 @@ +kind: optimization +body: Replace config-based mode setting with runtime detection for interactive/command-line mode +time: 2026-03-22T10:36:53.000000000Z +custom: + Author: ayeshurun + AuthorLink: https://github.com/ayeshurun diff --git a/docs/essentials/modes.md b/docs/essentials/modes.md index b21fcea58..0732cfa7c 100644 --- a/docs/essentials/modes.md +++ b/docs/essentials/modes.md @@ -1,47 +1,40 @@ # CLI Modes -The Fabric CLI supports two primary modes to accommodate a variety of workflows: **command line** and **interactive**. The selected mode is preserved between sessions. If you exit and login to the CLI later, it will resume in the same mode you last used. +The Fabric CLI supports two modes: **command-line** and **REPL** (interactive). The active mode is determined automatically at runtime — no configuration is required. -Use the following command to see the current stored mode setting: +## Command-Line Mode -``` -fab config get mode -``` - -## Command Line Mode +Command-line mode is best suited for scripted tasks, automation, or when you prefer running single commands without a persistent prompt. -Command line mode is best suited for scripted tasks, automation, or when you prefer running single commands without a prompt. - -Typing commands directly in the terminal replicates typical UNIX-style usage. - -Use the following command to switch the CLI into command line mode: +Invoke any command directly from your terminal with the `fab` prefix: ``` -fab config set mode command_line +fab ls / +fab get /myworkspace.Workspace/mynotebook.Notebook ``` -You will be required to log in again after switching modes. - -## Interactive Mode +## REPL Mode -Interactive mode provides a shell-like environment in which you can run Fabric CLI commands directly without the `fab` prefix. +REPL mode provides a shell-like interactive environment. Run `fab` without any arguments to enter REPL mode: -Upon entering interactive mode, you see a `fab:/$` prompt. Commands are executed one by one without needing to type `fab` before each command, giving you a more guided experience. +``` +fab +``` -Use the following command to switch the CLI into interactive mode: +Upon entering REPL mode, you see a `fab:/$` prompt. Commands are executed one by one without needing to type `fab` before each command: ``` -fab config set mode interactive +fab:/$ ls +fab:/$ cd myworkspace.Workspace +fab:/myworkspace.Workspace$ get mynotebook.Notebook +fab:/myworkspace.Workspace$ quit ``` -You will be required to log in again after switching modes. +Type `help` for a list of available commands, and `quit` or `exit` to leave REPL mode. ## Switching Between Modes -To switch from one mode to the other, enter: - -``` -fab config set mode -``` +There is no explicit mode switch command. The mode is determined by how you invoke the CLI: -where `` is either `command_line` or `interactive`. Because the Fabric CLI needs to establish new authentication for each mode, you must re-authenticate after switching. The mode choice then remains in effect until you change it again. \ No newline at end of file +- **Command-line mode** — run `fab ` with one or more arguments. +- **REPL mode** — run `fab` with no arguments. diff --git a/docs/essentials/settings.md b/docs/essentials/settings.md index 3a162825d..56b7dec19 100644 --- a/docs/essentials/settings.md +++ b/docs/essentials/settings.md @@ -1,6 +1,6 @@ # Settings -The Fabric CLI provides a comprehensive set of configuration settings that allow you to customize its behavior, performance, and default values. All settings persist across CLI sessions, except for `mode` and `encryption_fallback_enabled`. +The Fabric CLI provides a comprehensive set of configuration settings that allow you to customize its behavior, performance, and default values. All settings persist across CLI sessions, except for `encryption_fallback_enabled`. ## Available Settings @@ -9,11 +9,10 @@ The Fabric CLI provides a comprehensive set of configuration settings that allow | `cache_enabled` | Toggles caching of CLI HTTP responses | `BOOLEAN` | `true` | | `check_cli_version_updates` | Enables automatic update notifications on login | `BOOLEAN` | `true` | | `debug_enabled` | Toggles additional diagnostic logs for troubleshooting | `BOOLEAN` | `false` | -| `context_persistence_enabled` | Persists CLI navigation context in command line mode across sessions | `BOOLEAN` | `false` | +| `context_persistence_enabled` | Persists CLI navigation context in command-line mode across sessions | `BOOLEAN` | `false` | | `encryption_fallback_enabled` | Permits storing tokens in plain text if secure encryption is unavailable | `BOOLEAN` | `false` | | `job_cancel_ontimeout` | Cancels job runs that exceed the timeout period | `BOOLEAN` | `true` | | `local_definition_labels` | Indicates the local JSON file path for label definitions mapping | `VARCHAR` | | -| `mode` | Determines the CLI mode (`interactive` or `command_line`) | `VARCHAR` | `command_line` | | `output_item_sort_criteria` | Defines items output order (`byname` or `bytype`) | `VARCHAR` | `byname`| | `show_hidden` | Displays all Fabric elements | `BOOLEAN` | `false` | | `default_az_admin` | Defines the default Fabric administrator email for capacities | `VARCHAR` | | diff --git a/src/fabric_cli/commands/config/fab_config_get.py b/src/fabric_cli/commands/config/fab_config_get.py index cf4561cdd..c59774744 100644 --- a/src/fabric_cli/commands/config/fab_config_get.py +++ b/src/fabric_cli/commands/config/fab_config_get.py @@ -11,6 +11,20 @@ def exec_command(args: Namespace) -> None: key = args.key.lower() + + # Backward compatibility: 'mode' is no longer a configurable setting. + # Phase 1: warn but still return the runtime mode so existing scripts don't break. + if key == fab_constant.FAB_MODE: + utils_ui.print_warning( + "The 'mode' setting is deprecated and will be removed in a future release. " + "Run 'fab' without arguments to enter REPL mode, " + "or use 'fab ' for command-line mode." + ) + from fabric_cli.core.fab_context import Context + + utils_ui.print_output_format(args, data=Context().get_runtime_mode()) + return + if key not in fab_constant.FAB_CONFIG_KEYS_TO_VALID_VALUES: raise FabricCLIError( ErrorMessages.Config.unknown_configuration_key(key), diff --git a/src/fabric_cli/commands/config/fab_config_set.py b/src/fabric_cli/commands/config/fab_config_set.py index e1c1d9b26..efd109a62 100644 --- a/src/fabric_cli/commands/config/fab_config_set.py +++ b/src/fabric_cli/commands/config/fab_config_set.py @@ -18,6 +18,20 @@ def exec_command(args: Namespace) -> None: key = args.key.lower() value = args.value.strip().strip("'").strip('"') + # Backward compatibility: 'mode' is no longer a configurable setting. + # Phase 1: warn but still honour the request so existing scripts don't break. + if key == fab_constant.FAB_MODE: + utils_ui.print_warning( + "The 'mode' setting is deprecated and will be removed in a future release. " + "Run 'fab' without arguments to enter REPL mode, " + "or use 'fab ' for command-line mode." + ) + if value == fab_constant.FAB_MODE_INTERACTIVE: + from fabric_cli.core.fab_interactive import start_interactive_mode + + start_interactive_mode() + return + if key not in fab_constant.FAB_CONFIG_KEYS_TO_VALID_VALUES: raise FabricCLIError( ErrorMessages.Config.unknown_configuration_key(key), @@ -62,16 +76,12 @@ def _set_config(args: Namespace, key: str, value: Any, verbose: bool = True) -> fab_constant.ERROR_INVALID_PATH, ) - previous_mode = fab_state_config.get_config(key) fab_state_config.set_config(key, value) if verbose: utils_ui.print_output_format( args, message=f"Configuration '{key}' set to '{value}'" ) - if key == fab_constant.FAB_MODE: - _handle_fab_config_mode(previous_mode, value) - def _set_capacity(args: Namespace, value: str) -> None: value = utils.remove_dot_suffix(value, ".Capacity") @@ -89,30 +99,4 @@ def _set_capacity(args: Namespace, value: str) -> None: raise FabricCLIError( 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: - # Show deprecation warning - utils_ui.print_warning( - "Mode configuration is deprecated. Running 'fab' now automatically enters interactive mode." - ) - utils_ui.print("Starting interactive mode...") - from fabric_cli.core.fab_interactive import start_interactive_mode - start_interactive_mode() - - elif current_mode == fab_constant.FAB_MODE_COMMANDLINE: - # Show deprecation warning with better messaging - utils_ui.print_warning( - "Mode configuration is deprecated. Running 'fab' now automatically enters interactive mode." - ) - utils_ui.print("Configuration saved for backward compatibility.") - - if previous_mode == fab_constant.FAB_MODE_INTERACTIVE: - utils_ui.print("Exiting interactive mode. Goodbye!") - os._exit(0) \ No newline at end of file + ) \ No newline at end of file diff --git a/src/fabric_cli/core/fab_constant.py b/src/fabric_cli/core/fab_constant.py index a5a23470e..1bf37c89e 100644 --- a/src/fabric_cli/core/fab_constant.py +++ b/src/fabric_cli/core/fab_constant.py @@ -106,7 +106,6 @@ FAB_ENCRYPTION_FALLBACK_ENABLED: ["false", "true"], FAB_JOB_CANCEL_ONTIMEOUT: ["false", "true"], FAB_LOCAL_DEFINITION_LABELS: [], - FAB_MODE: [FAB_MODE_INTERACTIVE, FAB_MODE_COMMANDLINE], FAB_OUTPUT_ITEM_SORT_CRITERIA: ["byname", "bytype"], FAB_SHOW_HIDDEN: ["false", "true"], FAB_DEFAULT_AZ_SUBSCRIPTION_ID: [], @@ -123,7 +122,6 @@ } CONFIG_DEFAULT_VALUES = { - FAB_MODE: FAB_MODE_COMMANDLINE, FAB_CACHE_ENABLED: "true", FAB_CONTEXT_PERSISTENCE_ENABLED: "false", FAB_JOB_CANCEL_ONTIMEOUT: "true", @@ -344,3 +342,4 @@ # Invalid query parameters for set command across all fabric resources SET_COMMAND_INVALID_QUERIES = ["id", "type", "workspaceId", "folderId"] + diff --git a/src/fabric_cli/core/fab_context.py b/src/fabric_cli/core/fab_context.py index 4a5760563..10c9da2e1 100644 --- a/src/fabric_cli/core/fab_context.py +++ b/src/fabric_cli/core/fab_context.py @@ -23,12 +23,21 @@ class Context: def __init__(self): self._context: FabricElement = None self._command: str = None + self._runtime_mode: str = fab_constant.FAB_MODE_COMMANDLINE session_id = self._get_context_session_id() self._context_file = os.path.join( fab_state_config.config_location(), f"context-{session_id}.json" ) self._loading_context = False + def set_runtime_mode(self, mode: str) -> None: + """Set the current runtime mode. Called when entering or leaving the REPL.""" + self._runtime_mode = mode + + def get_runtime_mode(self) -> str: + """Return the current runtime mode (FAB_MODE_INTERACTIVE or FAB_MODE_COMMANDLINE).""" + return self._runtime_mode + @property def context(self) -> FabricElement: if self._context is None: @@ -126,12 +135,11 @@ def _load_context(self) -> None: def _should_use_context_file(self) -> bool: """Determine if the context file should be used based on the current mode and persistence settings.""" - mode = fab_state_config.get_config(fab_constant.FAB_MODE) persistence_enabled = fab_state_config.get_config( fab_constant.FAB_CONTEXT_PERSISTENCE_ENABLED ) return ( - mode == fab_constant.FAB_MODE_COMMANDLINE + self.get_runtime_mode() == fab_constant.FAB_MODE_COMMANDLINE and persistence_enabled == "true" and not self._loading_context ) diff --git a/src/fabric_cli/core/fab_interactive.py b/src/fabric_cli/core/fab_interactive.py index ee7d7bc9a..fc2cc6b4a 100644 --- a/src/fabric_cli/core/fab_interactive.py +++ b/src/fabric_cli/core/fab_interactive.py @@ -26,6 +26,7 @@ def __init__(self, parser=None, subparsers=None): self.parser = parser self.parser.set_mode(fab_constant.FAB_MODE_INTERACTIVE) + Context().set_runtime_mode(fab_constant.FAB_MODE_INTERACTIVE) self.subparsers = subparsers self.history = InMemoryHistory() self.session = self.init_session(self.history) @@ -147,6 +148,7 @@ def start_interactive(self): utils_ui.print(fab_constant.INTERACTIVE_EXIT_MESSAGE) finally: self._is_running = False + Context().set_runtime_mode(fab_constant.FAB_MODE_COMMANDLINE) def start_interactive_mode(): diff --git a/src/fabric_cli/core/fab_state_config.py b/src/fabric_cli/core/fab_state_config.py index ef634ae21..15af89b3d 100644 --- a/src/fabric_cli/core/fab_state_config.py +++ b/src/fabric_cli/core/fab_state_config.py @@ -56,6 +56,10 @@ def init_defaults(): current_config = read_config(config_file) changed = False + # Migration: remove the deprecated 'mode' key (mode is now detected at runtime) + if fab_constant.FAB_MODE in current_config: + del current_config[fab_constant.FAB_MODE] + for key in fab_constant.FAB_CONFIG_KEYS_TO_VALID_VALUES: old_key = f"fab_{key}" if old_key in current_config: diff --git a/src/fabric_cli/main.py b/src/fabric_cli/main.py index 3f03ee090..8581e00a0 100644 --- a/src/fabric_cli/main.py +++ b/src/fabric_cli/main.py @@ -31,15 +31,7 @@ def main(): if args.command == "auth" and args.auth_command == "login": from fabric_cli.commands.auth import fab_auth - if fab_auth.init(args): - if ( - fab_state_config.get_config(fab_constant.FAB_MODE) - == fab_constant.FAB_MODE_INTERACTIVE - ): - from fabric_cli.core.fab_interactive import start_interactive_mode - - start_interactive_mode() - return + fab_auth.init(args) if args.command == "auth" and args.auth_command == "logout": from fabric_cli.commands.auth import fab_auth diff --git a/src/fabric_cli/parsers/fab_config_parser.py b/src/fabric_cli/parsers/fab_config_parser.py index 555d0b7fb..271d8e773 100644 --- a/src/fabric_cli/parsers/fab_config_parser.py +++ b/src/fabric_cli/parsers/fab_config_parser.py @@ -31,8 +31,8 @@ def register_parser(subparsers: _SubParsersAction) -> None: # Subcommand for 'set' set_examples = [ - "# switch to command line mode", - "$ config set mode command_line\n", + "# enable debug mode", + "$ config set debug_enabled true\n", "# set default capacity", "$ config set default_capacity Trial-0000", ] @@ -59,8 +59,8 @@ def register_parser(subparsers: _SubParsersAction) -> None: # Subcommand for 'get' get_examples = [ - "# get current CLI mode", - "$ config get mode\n", + "# get current debug setting", + "$ config get debug_enabled\n", "# get default capacity", "$ config get default_capacity", ] diff --git a/tests/test_commands/test_config.py b/tests/test_commands/test_config.py index da9bb014a..036335593 100644 --- a/tests/test_commands/test_config.py +++ b/tests/test_commands/test_config.py @@ -1,8 +1,11 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. +from argparse import Namespace from unittest.mock import patch +import pytest + import fabric_cli.core.fab_constant as constant from fabric_cli.errors import ErrorMessages from tests.test_commands.commands_parser import CLIExecutor @@ -13,15 +16,15 @@ class TestConfig: # region config SET def test_config_set_success(self, mock_print_done, cli_executor: CLIExecutor): # Execute command - cli_executor.exec_command(f"config set mode {constant.FAB_MODE_INTERACTIVE}") + cli_executor.exec_command(f"config set debug_enabled true") # Extract the arguments passed to the mock call_args, _ = mock_print_done.call_args # Assert mock_print_done.assert_called_once() - assert constant.FAB_MODE in call_args[0].lower() - assert constant.FAB_MODE_INTERACTIVE in call_args[0] + assert "debug_enabled" in call_args[0].lower() + assert "true" in call_args[0] def test_config_set_default_capacity_success( self, mock_print_done, cli_executor: CLIExecutor, test_data: StaticTestData @@ -75,11 +78,8 @@ def test_config_set_unknown_key_failure( def test_config_set_invalid_value_failure( self, mock_print_done, assert_fabric_cli_error, cli_executor: CLIExecutor ): - # Setup - value = constant.FAB_MODE_INTERACTIVE + "?" - - # Execute command - cli_executor.exec_command(f"config set {constant.FAB_MODE} {value}") + # Execute command - invalid value for a real config key + cli_executor.exec_command(f"config set {constant.FAB_CACHE_ENABLED} invalid") # Assert mock_print_done.assert_not_called() @@ -172,74 +172,130 @@ def test_config_clear_cache_success( # endregion - # region config MODE SWITCHING - def test_config_set_mode_interactive_success( - self, mock_questionary_print, mock_fab_set_state_config, cli_executor: CLIExecutor + # region config MODE (deprecated) + def test_config_set_mode_interactive_shows_deprecation_warning( + self, mock_questionary_print, cli_executor: CLIExecutor ): - """Test successful transition to interactive mode""" - with patch("fabric_cli.core.fab_interactive.start_interactive_mode") as mock_start_interactive, \ - patch("fabric_cli.utils.fab_ui.print_warning") as mock_print_warning: - - mock_fab_set_state_config(constant.FAB_MODE, constant.FAB_MODE_COMMANDLINE) - - # Execute command + """Test that 'config set mode interactive' shows deprecation warning and launches REPL.""" + with patch("fabric_cli.utils.fab_ui.print_warning") as mock_print_warning, \ + patch("fabric_cli.core.fab_interactive.start_interactive_mode") as mock_repl: cli_executor.exec_command(f"config set mode {constant.FAB_MODE_INTERACTIVE}") - # Assert - mock_print_warning.assert_called_once_with( - "Mode configuration is deprecated. Running 'fab' now automatically enters interactive mode." - ) - mock_questionary_print.assert_called() - mock_start_interactive.assert_called_once_with() - assert mock_questionary_print.call_args[0][0] == 'Starting interactive mode...' + mock_print_warning.assert_any_call(DEPRECATION_WARNING) + mock_repl.assert_called_once() - def test_config_set_mode_interactive_from_interactive_success( - self, mock_questionary_print, mock_fab_set_state_config, cli_executor: CLIExecutor + def test_config_set_mode_command_line_shows_deprecation_warning( + self, mock_questionary_print, 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, \ - patch("fabric_cli.utils.fab_ui.print_warning") as mock_print_warning: - - 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}") + """Test that 'config set mode command_line' shows deprecation warning without launching REPL.""" + with patch("fabric_cli.utils.fab_ui.print_warning") as mock_print_warning, \ + patch("fabric_cli.core.fab_interactive.start_interactive_mode") as mock_repl: + cli_executor.exec_command(f"config set mode {constant.FAB_MODE_COMMANDLINE}") - # Assert - mock_print_warning.assert_called_once_with( - "Mode configuration is deprecated. Running 'fab' now automatically enters interactive mode." - ) - mock_questionary_print.assert_called() - mock_start_interactive.assert_called_once() - assert mock_questionary_print.call_args[0][0] == 'Starting interactive mode...' + mock_print_warning.assert_any_call(DEPRECATION_WARNING) + mock_repl.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 + def test_config_get_mode_shows_deprecation_warning( + self, mock_questionary_print, cli_executor: CLIExecutor ): - """Test transition from interactive to command_line mode""" - with patch("fabric_cli.utils.fab_ui.print_warning") as mock_print_warning, \ - patch("os._exit") as mock_exit: - - mock_fab_set_state_config(constant.FAB_MODE, constant.FAB_MODE_INTERACTIVE) + """Test that 'config get mode' shows deprecation warning and returns runtime mode.""" + with patch("fabric_cli.utils.fab_ui.print_warning") as mock_print_warning: + cli_executor.exec_command("config get mode") - # Execute command - cli_executor.exec_command(f"config set mode {constant.FAB_MODE_COMMANDLINE}") + mock_print_warning.assert_any_call(DEPRECATION_WARNING) + # Should still output the runtime mode value + mock_questionary_print.assert_called() - expected_calls = [ - ("Updating 'mode' value...",), - ("Configuration saved for backward compatibility.",), - ("Exiting interactive mode. Goodbye!",) - ] + # endregion - # Assert - mock_questionary_print.assert_called() - actual_calls = [call.args for call in mock_questionary_print.mock_calls] - assert actual_calls == expected_calls - mock_exit.assert_called_once_with(0) - # Verify proper exit sequence - mock_print_warning.assert_called_once_with( - "Mode configuration is deprecated. Running 'fab' now automatically enters interactive mode." - ) +DEPRECATION_WARNING = ( + "The 'mode' setting is deprecated and will be removed in a future release. " + "Run 'fab' without arguments to enter REPL mode, " + "or use 'fab ' for command-line mode." +) - # endregion + +class TestConfigModeDeprecated: + """Unit tests for mode deprecation in config set/get commands.""" + + def test_config_set_mode_interactive__warns_and_launches_repl(self): + """'config set mode interactive' must warn and launch REPL.""" + from fabric_cli.commands.config import fab_config_set + + args = Namespace(key="mode", value=constant.FAB_MODE_INTERACTIVE, output_format="text") + + with patch("fabric_cli.utils.fab_ui.print_warning") as mock_warn, \ + patch("fabric_cli.core.fab_state_config.set_config") as mock_set, \ + patch("fabric_cli.core.fab_interactive.start_interactive_mode") as mock_repl: + fab_config_set.exec_command(args) + + mock_warn.assert_called_once_with(DEPRECATION_WARNING) + mock_set.assert_not_called() + mock_repl.assert_called_once() + + @pytest.mark.parametrize("mode_value", [ + constant.FAB_MODE_COMMANDLINE, + "bogus_value", + ]) + def test_config_set_mode_non_interactive__warns_without_repl(self, mode_value): + """'config set mode command_line' (or bogus) must warn but not launch REPL.""" + from fabric_cli.commands.config import fab_config_set + + args = Namespace(key="mode", value=mode_value, output_format="text") + + with patch("fabric_cli.utils.fab_ui.print_warning") as mock_warn, \ + patch("fabric_cli.core.fab_state_config.set_config") as mock_set, \ + patch("fabric_cli.core.fab_interactive.start_interactive_mode") as mock_repl: + fab_config_set.exec_command(args) + + mock_warn.assert_called_once_with(DEPRECATION_WARNING) + mock_set.assert_not_called() + mock_repl.assert_not_called() + + def test_config_set_non_mode_key__still_works(self): + """Other config keys should still be writable.""" + from fabric_cli.commands.config import fab_config_set + + args = Namespace( + key=constant.FAB_DEBUG_ENABLED, + value="true", + output_format="text", + ) + + with patch("fabric_cli.core.fab_state_config.set_config") as mock_set, \ + patch("fabric_cli.utils.fab_ui.print_grey"), \ + patch("fabric_cli.utils.fab_ui.print_output_format"): + fab_config_set.exec_command(args) + mock_set.assert_called_once_with(constant.FAB_DEBUG_ENABLED, "true") + + def test_config_get_mode__warns_and_returns_runtime_mode(self): + """'config get mode' must warn and return the runtime mode.""" + from fabric_cli.commands.config import fab_config_get + + args = Namespace(key="mode", output_format="text") + + with patch("fabric_cli.utils.fab_ui.print_warning") as mock_warn, \ + patch("fabric_cli.core.fab_state_config.get_config") as mock_get, \ + patch("fabric_cli.utils.fab_ui.print_output_format") as mock_output, \ + patch("fabric_cli.core.fab_context.Context") as mock_ctx: + mock_ctx.return_value.get_runtime_mode.return_value = constant.FAB_MODE_COMMANDLINE + fab_config_get.exec_command(args) + + mock_warn.assert_called_once_with(DEPRECATION_WARNING) + mock_get.assert_not_called() + mock_output.assert_called_once() + + def test_config_get_non_mode_key__still_works(self): + """Other config keys should still be readable.""" + from fabric_cli.commands.config import fab_config_get + + args = Namespace( + key=constant.FAB_DEBUG_ENABLED, + output_format="text", + ) + + with patch("fabric_cli.core.fab_state_config.get_config", return_value="false") as mock_get, \ + patch("fabric_cli.utils.fab_ui.print_output_format"): + fab_config_get.exec_command(args) + mock_get.assert_called_once_with(constant.FAB_DEBUG_ENABLED) diff --git a/tests/test_core/test_context_persistence.py b/tests/test_core/test_context_persistence.py index 180454c28..c242a21d0 100644 --- a/tests/test_core/test_context_persistence.py +++ b/tests/test_core/test_context_persistence.py @@ -25,14 +25,15 @@ def test_context_persistence_save(monkeypatch): # Mock the state config def mock_get_config(key): - if key == fab_constant.FAB_MODE: - return fab_constant.FAB_MODE_COMMANDLINE - elif key == fab_constant.FAB_CONTEXT_PERSISTENCE_ENABLED: + if key == fab_constant.FAB_CONTEXT_PERSISTENCE_ENABLED: return "true" return None monkeypatch.setattr(fab_state_config, "get_config", mock_get_config) + context = Context() + monkeypatch.setattr(context, "get_runtime_mode", lambda: fab_constant.FAB_MODE_COMMANDLINE) + # Create a secure temporary context file path with tempfile.NamedTemporaryFile(suffix=".json", delete=False) as temp_file: temp_context_file = temp_file.name @@ -67,16 +68,15 @@ def test_context_persistence_load(monkeypatch): # Mock the state config def mock_get_config(key): - if key == fab_constant.FAB_MODE: - return fab_constant.FAB_MODE_COMMANDLINE - elif key == fab_constant.FAB_CONTEXT_PERSISTENCE_ENABLED: + if key == fab_constant.FAB_CONTEXT_PERSISTENCE_ENABLED: return "true" return None monkeypatch.setattr(fab_state_config, "get_config", mock_get_config) - # Create a context instance context = Context() + monkeypatch.setattr(context, "get_runtime_mode", lambda: fab_constant.FAB_MODE_COMMANDLINE) + # Reset the context to force loading context._context = None @@ -90,21 +90,19 @@ def mock_get_config(key): def test_context_persistence_not_used_in_interactive_mode(monkeypatch): - """Test that context persistence is not used in interactive mode.""" + """Test that context persistence is not used in REPL (interactive) mode.""" + + # Set runtime mode to interactive and enable persistence + context = Context() + monkeypatch.setattr(context, "get_runtime_mode", lambda: fab_constant.FAB_MODE_INTERACTIVE) - # Mock the state config to return interactive mode def mock_get_config(key): - if key == fab_constant.FAB_MODE: - return fab_constant.FAB_MODE_INTERACTIVE - elif key == fab_constant.FAB_CONTEXT_PERSISTENCE_ENABLED: - return "true" # Even if enabled, should not be used in interactive mode + if key == fab_constant.FAB_CONTEXT_PERSISTENCE_ENABLED: + return "true" # Even if enabled, should not be used in REPL mode return None monkeypatch.setattr(fab_state_config, "get_config", mock_get_config) - # Create a context instance - context = Context() - # Create a mock tenant and workspace tenant = Tenant("test_tenant", "1234") workspace = Workspace("test_workspace", "5678", tenant, "Workspace") @@ -115,10 +113,10 @@ def mock_get_config(key): patch.object(context, "_load_context_from_file") as mock_load, ): - # Set the context - this should NOT trigger file save in interactive mode + # Set the context - this should NOT trigger file save when persistence is disabled context.context = workspace - # Get the context - this should NOT trigger file load in interactive mode + # Get the context - this should NOT trigger file load when persistence is disabled result = context.context # Verify that file operations were not called @@ -132,18 +130,16 @@ def mock_get_config(key): def test_context_persistence_disabled_by_default(monkeypatch): """Test that context persistence is disabled by default even in command-line mode.""" - # Mock the state config to return command-line mode but default persistence setting + # Mock the state config to return default persistence setting (disabled) def mock_get_config(key): - if key == fab_constant.FAB_MODE: - return fab_constant.FAB_MODE_COMMANDLINE - elif key == fab_constant.FAB_CONTEXT_PERSISTENCE_ENABLED: + if key == fab_constant.FAB_CONTEXT_PERSISTENCE_ENABLED: return "false" # Default value - disabled return None monkeypatch.setattr(fab_state_config, "get_config", mock_get_config) - # Create a context instance context = Context() + monkeypatch.setattr(context, "get_runtime_mode", lambda: fab_constant.FAB_MODE_COMMANDLINE) # Create a mock tenant and workspace tenant = Tenant("test_tenant", "1234") @@ -172,23 +168,22 @@ def mock_get_config(key): def test_context_persistence_enabled_when_configured(monkeypatch): """Test that context persistence works when explicitly enabled in command-line mode.""" - # Mock the state config to return command-line mode with persistence enabled + # Mock the state config with persistence enabled def mock_get_config(key): - if key == fab_constant.FAB_MODE: - return fab_constant.FAB_MODE_COMMANDLINE - elif key == fab_constant.FAB_CONTEXT_PERSISTENCE_ENABLED: + if key == fab_constant.FAB_CONTEXT_PERSISTENCE_ENABLED: return "true" # Explicitly enabled return None monkeypatch.setattr(fab_state_config, "get_config", mock_get_config) + context = Context() + monkeypatch.setattr(context, "get_runtime_mode", lambda: fab_constant.FAB_MODE_COMMANDLINE) + # Create a secure temporary context file path with tempfile.NamedTemporaryFile(suffix=".json", delete=False) as temp_file: temp_context_file = temp_file.name try: - # Create a context instance - context = Context() context._context_file = temp_context_file # Create a mock tenant and workspace @@ -363,18 +358,17 @@ def test_loading_context_re_entrancy_guard(monkeypatch): prevents this circular dependency from causing a stack overflow. """ - # Mock the state config to enable context persistence + # Set runtime mode to command-line and enable context persistence + context = Context() + monkeypatch.setattr(context, "get_runtime_mode", lambda: fab_constant.FAB_MODE_COMMANDLINE) + def mock_get_config(key): - if key == fab_constant.FAB_MODE: - return fab_constant.FAB_MODE_COMMANDLINE - elif key == fab_constant.FAB_CONTEXT_PERSISTENCE_ENABLED: + if key == fab_constant.FAB_CONTEXT_PERSISTENCE_ENABLED: return "true" return None monkeypatch.setattr(fab_state_config, "get_config", mock_get_config) - # Create a context instance - context = Context() context._context = None # Reset to force loading # Test that _should_use_context_file returns False when _loading_context is True diff --git a/tests/test_core/test_fab_config_completers.py b/tests/test_core/test_fab_config_completers.py index 7a3a3a328..a13716ae7 100644 --- a/tests/test_core/test_fab_config_completers.py +++ b/tests/test_core/test_fab_config_completers.py @@ -23,8 +23,8 @@ def test_complete_config_keys_returns_all_keys_when_no_prefix_should_return_all_ def test_complete_config_keys_filters_by_prefix_should_return_matching_keys(): - result = fab_config_completers.complete_config_keys("mode") - assert "mode" in result + result = fab_config_completers.complete_config_keys("debug") + assert "debug_enabled" in result result = fab_config_completers.complete_config_keys("default") assert len(result) > 0 @@ -33,21 +33,21 @@ def test_complete_config_keys_filters_by_prefix_should_return_matching_keys(): def test_complete_config_keys_case_insensitive_should_return_same_results(): - result_lower = fab_config_completers.complete_config_keys("mode") - result_upper = fab_config_completers.complete_config_keys("MODE") + result_lower = fab_config_completers.complete_config_keys("debug") + result_upper = fab_config_completers.complete_config_keys("DEBUG") assert result_lower == result_upper def test_complete_config_values_enum_keys_should_return_expected_values(): args = Namespace() - args.key = "mode" + args.key = "cache_enabled" result = fab_config_completers.complete_config_values("", args) - assert result == ["command_line", "interactive"] + assert result == ["false", "true"] - result = fab_config_completers.complete_config_values("inter", args) - assert result == ["interactive"] + result = fab_config_completers.complete_config_values("tr", args) + assert result == ["true"] def test_complete_config_values_invalid_key_should_return_empty(): diff --git a/tests/test_core/test_fab_context.py b/tests/test_core/test_fab_context.py index f894db180..89bb345cd 100644 --- a/tests/test_core/test_fab_context.py +++ b/tests/test_core/test_fab_context.py @@ -436,3 +436,46 @@ def mock_psutil_process(): def mock_get_command_context(): with patch("fabric_cli.core.fab_handle_context.get_command_context") as mock: yield mock + + +# region Runtime Mode + +class TestRuntimeMode: + """Verify Context.set_runtime_mode / get_runtime_mode behaviour after mode-setting removal.""" + + def setup_method(self): + Context()._runtime_mode = fab_constant.FAB_MODE_COMMANDLINE + + def teardown_method(self): + Context()._runtime_mode = fab_constant.FAB_MODE_COMMANDLINE + + def test_default_runtime_mode__is_command_line(self): + """Default runtime mode must be COMMANDLINE when no REPL is active.""" + assert Context().get_runtime_mode() == fab_constant.FAB_MODE_COMMANDLINE + + def test_set_runtime_mode__to_interactive(self): + """set_runtime_mode(INTERACTIVE) should switch the mode.""" + Context().set_runtime_mode(fab_constant.FAB_MODE_INTERACTIVE) + assert Context().get_runtime_mode() == fab_constant.FAB_MODE_INTERACTIVE + + def test_set_runtime_mode__back_to_command_line(self): + """Switching to INTERACTIVE then back to COMMANDLINE must work.""" + Context().set_runtime_mode(fab_constant.FAB_MODE_INTERACTIVE) + Context().set_runtime_mode(fab_constant.FAB_MODE_COMMANDLINE) + assert Context().get_runtime_mode() == fab_constant.FAB_MODE_COMMANDLINE + + def test_runtime_mode__not_in_config_keys(self): + """'mode' must no longer appear in FAB_CONFIG_KEYS_TO_VALID_VALUES.""" + assert fab_constant.FAB_MODE not in fab_constant.FAB_CONFIG_KEYS_TO_VALID_VALUES + + def test_runtime_mode__not_in_config_defaults(self): + """'mode' must no longer appear in CONFIG_DEFAULT_VALUES.""" + assert fab_constant.FAB_MODE not in fab_constant.CONFIG_DEFAULT_VALUES + + def test_runtime_mode__not_module_level(self): + """Runtime mode must live on Context, not as module-level functions.""" + from fabric_cli.core import fab_context as ctx_module + assert not hasattr(ctx_module, "set_runtime_mode") + assert not hasattr(ctx_module, "get_runtime_mode") + +# endregion diff --git a/tests/test_core/test_fab_state_config.py b/tests/test_core/test_fab_state_config.py index 483347fa2..835d99692 100644 --- a/tests/test_core/test_fab_state_config.py +++ b/tests/test_core/test_fab_state_config.py @@ -1,10 +1,12 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. +import json import os import tempfile import fabric_cli.core.fab_state_config as cfg +from fabric_cli.core import fab_constant class TestStateConfig: @@ -97,3 +99,88 @@ def tracking_write(data): # Should NOT have written since nothing changed assert len(write_calls) == 0, "Should skip write when config unchanged" + + +def test_list_configs(monkeypatch): + with tempfile.TemporaryDirectory() as tmpdir: + tmp_file = os.path.join(tmpdir, "tmp_test.txt") + with open(tmp_file, "w") as file: + file.write('{"key": "value"}') + file.flush() + monkeypatch.setattr(cfg, "config_file", tmp_file) + cfg.set_config("key2", "value2") + assert cfg.list_configs() == {"key": "value", "key2": "value2"} + + +# region init_defaults migration + +def test_init_defaults__removes_mode_key(monkeypatch): + """If an existing config file contains 'mode', init_defaults must delete it.""" + with tempfile.TemporaryDirectory() as tmpdir: + config_file = os.path.join(tmpdir, "config.json") + old_config = { + fab_constant.FAB_MODE: fab_constant.FAB_MODE_INTERACTIVE, + fab_constant.FAB_CACHE_ENABLED: "true", + } + with open(config_file, "w") as f: + json.dump(old_config, f) + + monkeypatch.setattr(cfg, "config_file", config_file) + + cfg.init_defaults() + + result = cfg.read_config(config_file) + assert fab_constant.FAB_MODE not in result + assert result[fab_constant.FAB_CACHE_ENABLED] == "true" + + +def test_init_defaults__no_mode_key_works(monkeypatch): + """Config without 'mode' should initialize cleanly without errors.""" + with tempfile.TemporaryDirectory() as tmpdir: + config_file = os.path.join(tmpdir, "config.json") + with open(config_file, "w") as f: + json.dump({fab_constant.FAB_DEBUG_ENABLED: "true"}, f) + + monkeypatch.setattr(cfg, "config_file", config_file) + + cfg.init_defaults() + + result = cfg.read_config(config_file) + assert fab_constant.FAB_MODE not in result + assert result[fab_constant.FAB_DEBUG_ENABLED] == "true" + + +def test_init_defaults__applies_missing_defaults(monkeypatch): + """init_defaults must fill in missing default values.""" + with tempfile.TemporaryDirectory() as tmpdir: + config_file = os.path.join(tmpdir, "config.json") + with open(config_file, "w") as f: + json.dump({}, f) + + monkeypatch.setattr(cfg, "config_file", config_file) + + cfg.init_defaults() + + result = cfg.read_config(config_file) + for key, default_val in fab_constant.CONFIG_DEFAULT_VALUES.items(): + assert result.get(key) == default_val, ( + f"Expected default for '{key}' = '{default_val}', got '{result.get(key)}'" + ) + + +def test_init_defaults__preserves_user_overrides(monkeypatch): + """User-set values must not be overwritten by defaults.""" + with tempfile.TemporaryDirectory() as tmpdir: + config_file = os.path.join(tmpdir, "config.json") + user_config = {fab_constant.FAB_CACHE_ENABLED: "false"} + with open(config_file, "w") as f: + json.dump(user_config, f) + + monkeypatch.setattr(cfg, "config_file", config_file) + + cfg.init_defaults() + + result = cfg.read_config(config_file) + assert result[fab_constant.FAB_CACHE_ENABLED] == "false" + +# endregion