From a9c16217f56e42a9a10ec34d695efe9d77a56fda Mon Sep 17 00:00:00 2001 From: Alon Yeshurun <98805507+ayeshurun@users.noreply.github.com> Date: Tue, 17 Mar 2026 12:14:50 +0200 Subject: [PATCH 01/11] Add benchmark script for CLI startup performance This script benchmarks the startup performance of the CLI by measuring module import times, CLI invocation times, and heavy dependency loading. It allows comparisons against a baseline branch or tag. --- scripts/benchmark_startup.py | 270 +++++++++++++++++++++++++++++++++++ 1 file changed, 270 insertions(+) create mode 100644 scripts/benchmark_startup.py diff --git a/scripts/benchmark_startup.py b/scripts/benchmark_startup.py new file mode 100644 index 000000000..0d502613c --- /dev/null +++ b/scripts/benchmark_startup.py @@ -0,0 +1,270 @@ +#!/usr/bin/env python3 +""" +Benchmark CLI startup performance. + +Compares the current branch against 'main' (or any other branch) by measuring: + 1. Module import time (fabric_cli.main) + 2. CLI invocation time (fab --version) + 3. Heavy dependency loading (msal, jwt, cryptography, requests, prompt_toolkit) + +Usage: + # Compare current branch against main + python scripts/benchmark_startup.py + + # Compare current branch against a specific branch/tag/commit + python scripts/benchmark_startup.py --baseline v1.3.0 + + # Run only on the current branch (no git checkout) + python scripts/benchmark_startup.py --current-only + + # Change number of iterations (default: 10) + python scripts/benchmark_startup.py --iterations 20 +""" + +import argparse +import importlib +import json +import os +import shutil +import statistics +import subprocess +import sys +import time + + +HEAVY_MODULES = ["msal", "jwt", "cryptography", "requests", "prompt_toolkit", "psutil"] + + +def measure_import_time(iterations: int) -> dict: + """Measure fabric_cli.main import time across multiple iterations.""" + times = [] + for _ in range(iterations): + # Clear all fabric_cli modules from cache + mods = [k for k in sys.modules if k.startswith("fabric_cli")] + for m in mods: + del sys.modules[m] + + start = time.perf_counter() + importlib.import_module("fabric_cli.main") + elapsed_ms = (time.perf_counter() - start) * 1000 + times.append(elapsed_ms) + + return { + "median_ms": round(statistics.median(times), 1), + "min_ms": round(min(times), 1), + "max_ms": round(max(times), 1), + "mean_ms": round(statistics.mean(times), 1), + "stdev_ms": round(statistics.stdev(times), 1) if len(times) > 1 else 0, + "samples": times, + } + + +def check_heavy_modules() -> dict: + """Check which heavy modules are loaded after importing fabric_cli.main.""" + # Clear all fabric_cli modules + mods = [k for k in sys.modules if k.startswith("fabric_cli")] + for m in mods: + del sys.modules[m] + + # Also clear heavy modules + for mod in HEAVY_MODULES: + keys = [k for k in sys.modules if k.startswith(mod)] + for k in keys: + del sys.modules[k] + + importlib.import_module("fabric_cli.main") + + return {mod: mod in sys.modules for mod in HEAVY_MODULES} + + +def measure_cli_time(iterations: int) -> dict: + """Measure 'fab --version' wall-clock time.""" + fab_path = shutil.which("fab") + if not fab_path: + return {"error": "'fab' not found in PATH. Run 'pip install -e .' first."} + + times = [] + for _ in range(iterations): + start = time.perf_counter() + subprocess.run( + [fab_path, "--version"], + capture_output=True, + text=True, + ) + elapsed_ms = (time.perf_counter() - start) * 1000 + times.append(elapsed_ms) + + return { + "median_ms": round(statistics.median(times), 1), + "min_ms": round(min(times), 1), + "max_ms": round(max(times), 1), + "mean_ms": round(statistics.mean(times), 1), + "stdev_ms": round(statistics.stdev(times), 1) if len(times) > 1 else 0, + } + + +def run_benchmark(label: str, iterations: int) -> dict: + """Run all benchmarks and return results.""" + print(f"\n{'=' * 60}") + print(f" Benchmarking: {label}") + print(f"{'=' * 60}") + + # 1. Import time + print(f" Measuring import time ({iterations} iterations)...", end="", flush=True) + import_results = measure_import_time(iterations) + print(f" {import_results['median_ms']:.0f}ms median") + + # 2. Heavy modules + print(" Checking heavy module loading...", end="", flush=True) + heavy_results = check_heavy_modules() + loaded = [m for m, v in heavy_results.items() if v] + print(f" {len(loaded)} loaded: {', '.join(loaded) if loaded else 'none'}") + + # 3. CLI time + print(f" Measuring 'fab --version' ({iterations} iterations)...", end="", flush=True) + cli_results = measure_cli_time(iterations) + if "error" in cli_results: + print(f" {cli_results['error']}") + else: + print(f" {cli_results['median_ms']:.0f}ms median") + + return { + "label": label, + "import_time": import_results, + "heavy_modules": heavy_results, + "cli_time": cli_results, + } + + +def print_comparison(baseline: dict, current: dict): + """Print a formatted comparison table.""" + print(f"\n{'=' * 60}") + print(" COMPARISON") + print(f"{'=' * 60}\n") + + bl = baseline["import_time"]["median_ms"] + cu = current["import_time"]["median_ms"] + diff = bl - cu + pct = (diff / bl * 100) if bl > 0 else 0 + + print(f" {'Metric':<30} {'Baseline':>10} {'Current':>10} {'Change':>10}") + print(f" {'-' * 62}") + print(f" {'Import time (median):':<30} {bl:>9.0f}ms {cu:>9.0f}ms {diff:>+8.0f}ms") + print(f" {'Import improvement:':<30} {'':>10} {'':>10} {pct:>+8.0f}%") + + if "error" not in baseline["cli_time"] and "error" not in current["cli_time"]: + bl_cli = baseline["cli_time"]["median_ms"] + cu_cli = current["cli_time"]["median_ms"] + cli_diff = bl_cli - cu_cli + cli_pct = (cli_diff / bl_cli * 100) if bl_cli > 0 else 0 + print(f" {'CLI time (median):':<30} {bl_cli:>9.0f}ms {cu_cli:>9.0f}ms {cli_diff:>+8.0f}ms") + print(f" {'CLI improvement:':<30} {'':>10} {'':>10} {cli_pct:>+8.0f}%") + + print(f"\n {'Heavy modules at startup:':<30}") + for mod in HEAVY_MODULES: + bl_loaded = "LOADED" if baseline["heavy_modules"].get(mod) else "deferred" + cu_loaded = "LOADED" if current["heavy_modules"].get(mod) else "deferred" + marker = " ✓" if cu_loaded == "deferred" and bl_loaded == "LOADED" else "" + print(f" {mod:<25} {bl_loaded:>10} {cu_loaded:>10}{marker}") + + print() + + +def git_checkout_and_install(ref: str): + """Checkout a git ref and reinstall the package.""" + print(f"\n Switching to '{ref}'...") + subprocess.run(["git", "checkout", ref], capture_output=True, check=True) + subprocess.run( + [sys.executable, "-m", "pip", "install", "-e", ".", "-q"], + capture_output=True, + check=True, + ) + # Clear all cached fabric_cli modules after reinstall + mods = [k for k in sys.modules if k.startswith("fabric_cli")] + for m in mods: + del sys.modules[m] + + +def main(): + parser = argparse.ArgumentParser( + description="Benchmark CLI startup performance between branches." + ) + parser.add_argument( + "--baseline", + default="main", + help="Git ref to compare against (default: main)", + ) + parser.add_argument( + "--iterations", "-n", + type=int, + default=10, + help="Number of iterations per measurement (default: 10)", + ) + parser.add_argument( + "--current-only", + action="store_true", + help="Only benchmark the current branch (skip baseline)", + ) + parser.add_argument( + "--json", + action="store_true", + help="Output results as JSON", + ) + args = parser.parse_args() + + repo_root = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) + os.chdir(repo_root) + + # Get current branch name + result = subprocess.run( + ["git", "rev-parse", "--abbrev-ref", "HEAD"], + capture_output=True, text=True, + ) + current_branch = result.stdout.strip() + + print(f" Repo: {repo_root}") + print(f" Current branch: {current_branch}") + print(f" Baseline: {args.baseline}") + print(f" Iterations: {args.iterations}") + + results = {} + + if not args.current_only: + # Benchmark baseline + try: + git_checkout_and_install(args.baseline) + results["baseline"] = run_benchmark(f"Baseline ({args.baseline})", args.iterations) + except subprocess.CalledProcessError: + print(f"\n ERROR: Could not checkout '{args.baseline}'. Does it exist?") + print(f" Try: python scripts/benchmark_startup.py --current-only") + sys.exit(1) + finally: + # Always return to original branch + subprocess.run(["git", "checkout", current_branch], capture_output=True) + subprocess.run( + [sys.executable, "-m", "pip", "install", "-e", ".", "-q"], + capture_output=True, + ) + # Clear modules again + mods = [k for k in sys.modules if k.startswith("fabric_cli")] + for m in mods: + del sys.modules[m] + + # Benchmark current + results["current"] = run_benchmark(f"Current ({current_branch})", args.iterations) + + # Print comparison + if "baseline" in results: + print_comparison(results["baseline"], results["current"]) + + # JSON output + if args.json: + # Remove raw samples for cleaner JSON + for key in results: + if "samples" in results[key].get("import_time", {}): + del results[key]["import_time"]["samples"] + print(json.dumps(results, indent=2)) + + +if __name__ == "__main__": + main() From 0ee8239bd3cb430af1f48ce941f547ac3e1b97bb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Mar 2026 18:32:22 +0000 Subject: [PATCH 02/11] Initial plan From 884e72488490eec1ec3ce07736223b605e556123 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Mar 2026 18:55:37 +0000 Subject: [PATCH 03/11] Remove mode settings: detect REPL mode at runtime instead of config Co-authored-by: ayeshurun <98805507+ayeshurun@users.noreply.github.com> --- docs/essentials/modes.md | 47 ++++---- docs/essentials/settings.md | 5 +- .../commands/config/fab_config_get.py | 10 ++ .../commands/config/fab_config_set.py | 41 ++----- src/fabric_cli/core/fab_constant.py | 17 ++- src/fabric_cli/core/fab_context.py | 3 +- src/fabric_cli/core/fab_interactive.py | 2 + src/fabric_cli/core/fab_state_config.py | 4 + src/fabric_cli/main.py | 8 +- src/fabric_cli/parsers/fab_config_parser.py | 8 +- tests/test_commands/test_config.py | 102 +++++++----------- tests/test_core/test_context_persistence.py | 44 ++++---- tests/test_core/test_fab_config_completers.py | 16 +-- 13 files changed, 133 insertions(+), 174 deletions(-) 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..fa2b2d26e 100644 --- a/src/fabric_cli/commands/config/fab_config_get.py +++ b/src/fabric_cli/commands/config/fab_config_get.py @@ -11,6 +11,16 @@ def exec_command(args: Namespace) -> None: key = args.key.lower() + + # Backward compatibility: 'mode' is no longer a configurable setting. + if key == fab_constant.FAB_MODE: + utils_ui.print_warning( + "The 'mode' setting has been removed. " + "Run 'fab' without arguments to enter REPL mode, " + "or use 'fab ' for command-line 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..a264f9544 100644 --- a/src/fabric_cli/commands/config/fab_config_set.py +++ b/src/fabric_cli/commands/config/fab_config_set.py @@ -18,6 +18,15 @@ 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. + if key == fab_constant.FAB_MODE: + utils_ui.print_warning( + "The 'mode' setting has been removed. " + "Run 'fab' without arguments to enter REPL mode, " + "or use 'fab ' for command-line mode." + ) + return + if key not in fab_constant.FAB_CONFIG_KEYS_TO_VALID_VALUES: raise FabricCLIError( ErrorMessages.Config.unknown_configuration_key(key), @@ -62,16 +71,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 +94,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..d267dd16b 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,18 @@ # Invalid query parameters for set command across all fabric resources SET_COMMAND_INVALID_QUERIES = ["id", "type", "workspaceId", "folderId"] + +# Runtime mode tracking — detected from context, not persisted in config. +# FAB_MODE_COMMANDLINE is the default; InteractiveCLI sets FAB_MODE_INTERACTIVE on start. +_runtime_mode: str = FAB_MODE_COMMANDLINE + + +def set_runtime_mode(mode: str) -> None: + """Set the current runtime mode. Called when entering or leaving the REPL.""" + global _runtime_mode + _runtime_mode = mode + + +def get_runtime_mode() -> str: + """Return the current runtime mode (FAB_MODE_INTERACTIVE or FAB_MODE_COMMANDLINE).""" + return _runtime_mode diff --git a/src/fabric_cli/core/fab_context.py b/src/fabric_cli/core/fab_context.py index 4a5760563..683ff5bcb 100644 --- a/src/fabric_cli/core/fab_context.py +++ b/src/fabric_cli/core/fab_context.py @@ -126,12 +126,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 + fab_constant.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..9a8d52900 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) + fab_constant.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 + fab_constant.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 5f052e9c6..7ba199436 100644 --- a/src/fabric_cli/core/fab_state_config.py +++ b/src/fabric_cli/core/fab_state_config.py @@ -55,6 +55,10 @@ def init_defaults(): """ current_config = read_config(config_file) + # 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 2b4be323a..767f50770 100644 --- a/src/fabric_cli/main.py +++ b/src/fabric_cli/main.py @@ -31,13 +31,7 @@ def main(): return if args.command == "auth" and args.auth_command == "login": - if login.init(args): - if ( - fab_state_config.get_config(fab_constant.FAB_MODE) - == fab_constant.FAB_MODE_INTERACTIVE - ): - start_interactive_mode() - return + login.init(args) if args.command == "auth" and args.auth_command == "logout": login.logout(args) diff --git a/src/fabric_cli/parsers/fab_config_parser.py b/src/fabric_cli/parsers/fab_config_parser.py index f63957c16..1a1e921d1 100644 --- a/src/fabric_cli/parsers/fab_config_parser.py +++ b/src/fabric_cli/parsers/fab_config_parser.py @@ -29,8 +29,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", ] @@ -57,8 +57,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..00fa00243 100644 --- a/tests/test_commands/test_config.py +++ b/tests/test_commands/test_config.py @@ -13,15 +13,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 +75,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 +169,47 @@ 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_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' shows a deprecation warning instead of switching modes.""" + with patch("fabric_cli.utils.fab_ui.print_warning") as mock_print_warning: 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." + deprecation_warning = ( + "The 'mode' setting has been removed. " + "Run 'fab' without arguments to enter REPL mode, " + "or use 'fab ' for command-line 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) - 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' also shows a deprecation warning.""" + with patch("fabric_cli.utils.fab_ui.print_warning") as mock_print_warning: + 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." + deprecation_warning = ( + "The 'mode' setting has been removed. " + "Run 'fab' without arguments to enter REPL mode, " + "or use 'fab ' for command-line 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) - 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) - - # Execute command - cli_executor.exec_command(f"config set mode {constant.FAB_MODE_COMMANDLINE}") - - expected_calls = [ - ("Updating 'mode' value...",), - ("Configuration saved for backward compatibility.",), - ("Exiting interactive mode. Goodbye!",) - ] - - # 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." + """Test that 'config get mode' shows a deprecation warning.""" + with patch("fabric_cli.utils.fab_ui.print_warning") as mock_print_warning: + cli_executor.exec_command("config get mode") + + deprecation_warning = ( + "The 'mode' setting has been removed. " + "Run 'fab' without arguments to enter REPL mode, " + "or use 'fab ' for command-line mode." ) + mock_print_warning.assert_any_call(deprecation_warning) # endregion diff --git a/tests/test_core/test_context_persistence.py b/tests/test_core/test_context_persistence.py index 180454c28..174f9f027 100644 --- a/tests/test_core/test_context_persistence.py +++ b/tests/test_core/test_context_persistence.py @@ -25,13 +25,12 @@ 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) + monkeypatch.setattr(fab_constant, "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: @@ -67,13 +66,12 @@ 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) + monkeypatch.setattr(fab_constant, "get_runtime_mode", lambda: fab_constant.FAB_MODE_COMMANDLINE) # Create a context instance context = Context() @@ -90,14 +88,14 @@ 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 + monkeypatch.setattr(fab_constant, "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) @@ -132,15 +130,14 @@ 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) + monkeypatch.setattr(fab_constant, "get_runtime_mode", lambda: fab_constant.FAB_MODE_COMMANDLINE) # Create a context instance context = Context() @@ -172,15 +169,14 @@ 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) + monkeypatch.setattr(fab_constant, "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: @@ -363,11 +359,11 @@ 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 + monkeypatch.setattr(fab_constant, "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 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(): From 58408d682a61bcfadd209ed32916211692f5c26b Mon Sep 17 00:00:00 2001 From: Alon Yeshurun Date: Thu, 19 Mar 2026 16:16:49 +0000 Subject: [PATCH 04/11] update --- src/fabric_cli/core/fab_constant.py | 14 -------------- src/fabric_cli/core/fab_context.py | 11 ++++++++++- src/fabric_cli/core/fab_interactive.py | 4 ++-- 3 files changed, 12 insertions(+), 17 deletions(-) diff --git a/src/fabric_cli/core/fab_constant.py b/src/fabric_cli/core/fab_constant.py index d267dd16b..1bf37c89e 100644 --- a/src/fabric_cli/core/fab_constant.py +++ b/src/fabric_cli/core/fab_constant.py @@ -343,17 +343,3 @@ # Invalid query parameters for set command across all fabric resources SET_COMMAND_INVALID_QUERIES = ["id", "type", "workspaceId", "folderId"] -# Runtime mode tracking — detected from context, not persisted in config. -# FAB_MODE_COMMANDLINE is the default; InteractiveCLI sets FAB_MODE_INTERACTIVE on start. -_runtime_mode: str = FAB_MODE_COMMANDLINE - - -def set_runtime_mode(mode: str) -> None: - """Set the current runtime mode. Called when entering or leaving the REPL.""" - global _runtime_mode - _runtime_mode = mode - - -def get_runtime_mode() -> str: - """Return the current runtime mode (FAB_MODE_INTERACTIVE or FAB_MODE_COMMANDLINE).""" - return _runtime_mode diff --git a/src/fabric_cli/core/fab_context.py b/src/fabric_cli/core/fab_context.py index 683ff5bcb..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: @@ -130,7 +139,7 @@ def _should_use_context_file(self) -> bool: fab_constant.FAB_CONTEXT_PERSISTENCE_ENABLED ) return ( - fab_constant.get_runtime_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 9a8d52900..fc2cc6b4a 100644 --- a/src/fabric_cli/core/fab_interactive.py +++ b/src/fabric_cli/core/fab_interactive.py @@ -26,7 +26,7 @@ def __init__(self, parser=None, subparsers=None): self.parser = parser self.parser.set_mode(fab_constant.FAB_MODE_INTERACTIVE) - fab_constant.set_runtime_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) @@ -148,7 +148,7 @@ def start_interactive(self): utils_ui.print(fab_constant.INTERACTIVE_EXIT_MESSAGE) finally: self._is_running = False - fab_constant.set_runtime_mode(fab_constant.FAB_MODE_COMMANDLINE) + Context().set_runtime_mode(fab_constant.FAB_MODE_COMMANDLINE) def start_interactive_mode(): From 3bcad54c5b7cd5b9b9399f4bb516420748c07014 Mon Sep 17 00:00:00 2001 From: Alon Yeshurun Date: Sun, 22 Mar 2026 09:27:47 +0200 Subject: [PATCH 05/11] Regression tests and graduated deprecation for mode removal - Move runtime mode into Context class (singleton instance methods) - Graduated deprecation: 'config set mode interactive' still launches REPL with warning - Graduated deprecation: 'config get mode' returns runtime mode with warning - Add regression tests: runtime mode (6), config deprecation (7), init defaults (4), MAP removal (6), session-per-request (3), context persistence mock updates (10) - Consolidate test files into test_fab_context.py, test_config.py, test_fab_state_config.py Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../commands/config/fab_config_get.py | 6 +- .../commands/config/fab_config_set.py | 7 +- tests/test_commands/test_config.py | 134 ++++++++++++++---- tests/test_core/test_context_persistence.py | 32 ++--- tests/test_core/test_fab_context.py | 43 ++++++ tests/test_core/test_fab_state_config.py | 76 ++++++++++ tests/test_core/test_map_item_removal.py | 31 ++++ tests/test_utils/test_session_per_request.py | 60 ++++++++ 8 files changed, 346 insertions(+), 43 deletions(-) create mode 100644 tests/test_core/test_map_item_removal.py create mode 100644 tests/test_utils/test_session_per_request.py diff --git a/src/fabric_cli/commands/config/fab_config_get.py b/src/fabric_cli/commands/config/fab_config_get.py index fa2b2d26e..c59774744 100644 --- a/src/fabric_cli/commands/config/fab_config_get.py +++ b/src/fabric_cli/commands/config/fab_config_get.py @@ -13,12 +13,16 @@ 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 has been removed. " + "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: diff --git a/src/fabric_cli/commands/config/fab_config_set.py b/src/fabric_cli/commands/config/fab_config_set.py index a264f9544..efd109a62 100644 --- a/src/fabric_cli/commands/config/fab_config_set.py +++ b/src/fabric_cli/commands/config/fab_config_set.py @@ -19,12 +19,17 @@ def exec_command(args: Namespace) -> None: 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 has been removed. " + "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: diff --git a/tests/test_commands/test_config.py b/tests/test_commands/test_config.py index 00fa00243..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 @@ -170,46 +173,129 @@ def test_config_clear_cache_success( # endregion # region config MODE (deprecated) - def test_config_set_mode_shows_deprecation_warning( + def test_config_set_mode_interactive_shows_deprecation_warning( self, mock_questionary_print, cli_executor: CLIExecutor ): - """Test that 'config set mode' shows a deprecation warning instead of switching modes.""" - with patch("fabric_cli.utils.fab_ui.print_warning") as mock_print_warning: + """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}") - deprecation_warning = ( - "The 'mode' setting has been removed. " - "Run 'fab' without arguments to enter REPL mode, " - "or use 'fab ' for command-line mode." - ) - mock_print_warning.assert_any_call(deprecation_warning) + mock_print_warning.assert_any_call(DEPRECATION_WARNING) + mock_repl.assert_called_once() def test_config_set_mode_command_line_shows_deprecation_warning( self, mock_questionary_print, cli_executor: CLIExecutor ): - """Test that 'config set mode command_line' also shows a deprecation warning.""" - with patch("fabric_cli.utils.fab_ui.print_warning") as mock_print_warning: + """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}") - deprecation_warning = ( - "The 'mode' setting has been removed. " - "Run 'fab' without arguments to enter REPL mode, " - "or use 'fab ' for command-line mode." - ) - mock_print_warning.assert_any_call(deprecation_warning) + mock_print_warning.assert_any_call(DEPRECATION_WARNING) + mock_repl.assert_not_called() def test_config_get_mode_shows_deprecation_warning( self, mock_questionary_print, cli_executor: CLIExecutor ): - """Test that 'config get mode' shows a deprecation warning.""" + """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") - deprecation_warning = ( - "The 'mode' setting has been removed. " - "Run 'fab' without arguments to enter REPL mode, " - "or use 'fab ' for command-line mode." - ) - mock_print_warning.assert_any_call(deprecation_warning) + mock_print_warning.assert_any_call(DEPRECATION_WARNING) + # Should still output the runtime mode value + mock_questionary_print.assert_called() # endregion + + +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." +) + + +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 174f9f027..c242a21d0 100644 --- a/tests/test_core/test_context_persistence.py +++ b/tests/test_core/test_context_persistence.py @@ -30,7 +30,9 @@ def mock_get_config(key): return None monkeypatch.setattr(fab_state_config, "get_config", mock_get_config) - monkeypatch.setattr(fab_constant, "get_runtime_mode", lambda: fab_constant.FAB_MODE_COMMANDLINE) + + 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: @@ -71,10 +73,10 @@ def mock_get_config(key): return None monkeypatch.setattr(fab_state_config, "get_config", mock_get_config) - monkeypatch.setattr(fab_constant, "get_runtime_mode", lambda: fab_constant.FAB_MODE_COMMANDLINE) - # 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 @@ -91,7 +93,8 @@ def test_context_persistence_not_used_in_interactive_mode(monkeypatch): """Test that context persistence is not used in REPL (interactive) mode.""" # Set runtime mode to interactive and enable persistence - monkeypatch.setattr(fab_constant, "get_runtime_mode", lambda: fab_constant.FAB_MODE_INTERACTIVE) + context = Context() + monkeypatch.setattr(context, "get_runtime_mode", lambda: fab_constant.FAB_MODE_INTERACTIVE) def mock_get_config(key): if key == fab_constant.FAB_CONTEXT_PERSISTENCE_ENABLED: @@ -100,9 +103,6 @@ def mock_get_config(key): 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") @@ -113,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 @@ -137,10 +137,9 @@ def mock_get_config(key): return None monkeypatch.setattr(fab_state_config, "get_config", mock_get_config) - monkeypatch.setattr(fab_constant, "get_runtime_mode", lambda: fab_constant.FAB_MODE_COMMANDLINE) - # 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") @@ -176,15 +175,15 @@ def mock_get_config(key): return None monkeypatch.setattr(fab_state_config, "get_config", mock_get_config) - monkeypatch.setattr(fab_constant, "get_runtime_mode", lambda: fab_constant.FAB_MODE_COMMANDLINE) + + 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 @@ -360,7 +359,8 @@ def test_loading_context_re_entrancy_guard(monkeypatch): """ # Set runtime mode to command-line and enable context persistence - monkeypatch.setattr(fab_constant, "get_runtime_mode", lambda: fab_constant.FAB_MODE_COMMANDLINE) + context = Context() + monkeypatch.setattr(context, "get_runtime_mode", lambda: fab_constant.FAB_MODE_COMMANDLINE) def mock_get_config(key): if key == fab_constant.FAB_CONTEXT_PERSISTENCE_ENABLED: @@ -369,8 +369,6 @@ def mock_get_config(key): 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_context.py b/tests/test_core/test_fab_context.py index ce1ab7131..f3d5d26cf 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 2ec4f1711..08f084d5f 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 def test_read_config(): @@ -67,3 +69,77 @@ def test_list_configs(monkeypatch): 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 diff --git a/tests/test_core/test_map_item_removal.py b/tests/test_core/test_map_item_removal.py new file mode 100644 index 000000000..ae9117c2f --- /dev/null +++ b/tests/test_core/test_map_item_removal.py @@ -0,0 +1,31 @@ +"""Regression tests — ItemType.MAP removal.""" + +import pytest +from fabric_cli.core.fab_types import ItemType + + +class TestMapItemRemoval: + """Verify MAP enum member and its mapping entries are removed.""" + + def test_map_not_in_item_type_enum(self): + assert not hasattr(ItemType, "MAP"), "ItemType.MAP should be removed" + + def test_map_not_in_enum_values(self): + assert "Map" not in [m.value for m in ItemType] + + def test_map_not_in_format_mapping(self): + from fabric_cli.core.fab_types import format_mapping + assert "Map" not in format_mapping + + def test_map_not_in_uri_mapping(self): + from fabric_cli.core.fab_types import uri_mapping + assert "Map" not in uri_mapping + + def test_map_not_in_definition_format_mapping(self): + from fabric_cli.core.fab_types import definition_format_mapping + assert "Map" not in definition_format_mapping + + def test_other_item_types_still_exist(self): + """Sanity check — common item types are still present.""" + for name in ("Notebook", "Lakehouse", "SemanticModel", "Report"): + assert hasattr(ItemType, name.upper()) or name in [m.value for m in ItemType] diff --git a/tests/test_utils/test_session_per_request.py b/tests/test_utils/test_session_per_request.py new file mode 100644 index 000000000..6e30a9cfe --- /dev/null +++ b/tests/test_utils/test_session_per_request.py @@ -0,0 +1,60 @@ +"""Regression tests — per-request session creation (no shared session pool).""" + +import pytest +from unittest.mock import patch, MagicMock +from argparse import Namespace +from fabric_cli.client import fab_api_client + + +class TestSessionPerRequest: + """Verify each do_request creates a fresh Session (no shared pool).""" + + def test_no_shared_session_attribute(self): + assert not hasattr(fab_api_client, "_shared_session"), \ + "Shared session singleton should be removed" + + def test_no_get_session_function(self): + assert not hasattr(fab_api_client, "_get_session"), \ + "_get_session helper should be removed" + + def test_do_request_creates_new_session_each_call(self): + fake_args = Namespace( + json_file=None, + audience=None, + method="GET", + wait=False, + raw_response=False, + request_params=None, + uri="workspaces/00000000-0000-0000-0000-000000000001/items", + headers=None, + ) + mock_response = MagicMock() + mock_response.status_code = 200 + mock_response.headers = {"x-ms-request-id": "test"} + mock_response.json.return_value = {} + + sessions = [] + + def track_session(*a, **kw): + s = MagicMock() + s.request.return_value = mock_response + sessions.append(s) + return s + + with patch("fabric_cli.client.fab_api_client.requests.Session", side_effect=track_session), \ + patch("fabric_cli.core.fab_auth.FabAuth") as mock_auth_cls, \ + patch("fabric_cli.core.fab_context.Context") as mock_ctx_cls, \ + patch("fabric_cli.core.fab_state_config.get_config", return_value=None): + mock_auth_cls.return_value.get_access_token.return_value = "tok" + mock_ctx_cls.return_value.command = "test" + try: + fab_api_client.do_request(fake_args) + except Exception: + pass + try: + fab_api_client.do_request(fake_args) + except Exception: + pass + + assert len(sessions) >= 2, "Each do_request should create its own Session" + assert sessions[0] is not sessions[1] From 7a563ff783facb8acfd74aa8ca629a88e064dad2 Mon Sep 17 00:00:00 2001 From: Alon Yeshurun <98805507+ayeshurun@users.noreply.github.com> Date: Sun, 22 Mar 2026 12:05:29 +0200 Subject: [PATCH 06/11] Delete scripts/benchmark_startup.py --- scripts/benchmark_startup.py | 270 ----------------------------------- 1 file changed, 270 deletions(-) delete mode 100644 scripts/benchmark_startup.py diff --git a/scripts/benchmark_startup.py b/scripts/benchmark_startup.py deleted file mode 100644 index 0d502613c..000000000 --- a/scripts/benchmark_startup.py +++ /dev/null @@ -1,270 +0,0 @@ -#!/usr/bin/env python3 -""" -Benchmark CLI startup performance. - -Compares the current branch against 'main' (or any other branch) by measuring: - 1. Module import time (fabric_cli.main) - 2. CLI invocation time (fab --version) - 3. Heavy dependency loading (msal, jwt, cryptography, requests, prompt_toolkit) - -Usage: - # Compare current branch against main - python scripts/benchmark_startup.py - - # Compare current branch against a specific branch/tag/commit - python scripts/benchmark_startup.py --baseline v1.3.0 - - # Run only on the current branch (no git checkout) - python scripts/benchmark_startup.py --current-only - - # Change number of iterations (default: 10) - python scripts/benchmark_startup.py --iterations 20 -""" - -import argparse -import importlib -import json -import os -import shutil -import statistics -import subprocess -import sys -import time - - -HEAVY_MODULES = ["msal", "jwt", "cryptography", "requests", "prompt_toolkit", "psutil"] - - -def measure_import_time(iterations: int) -> dict: - """Measure fabric_cli.main import time across multiple iterations.""" - times = [] - for _ in range(iterations): - # Clear all fabric_cli modules from cache - mods = [k for k in sys.modules if k.startswith("fabric_cli")] - for m in mods: - del sys.modules[m] - - start = time.perf_counter() - importlib.import_module("fabric_cli.main") - elapsed_ms = (time.perf_counter() - start) * 1000 - times.append(elapsed_ms) - - return { - "median_ms": round(statistics.median(times), 1), - "min_ms": round(min(times), 1), - "max_ms": round(max(times), 1), - "mean_ms": round(statistics.mean(times), 1), - "stdev_ms": round(statistics.stdev(times), 1) if len(times) > 1 else 0, - "samples": times, - } - - -def check_heavy_modules() -> dict: - """Check which heavy modules are loaded after importing fabric_cli.main.""" - # Clear all fabric_cli modules - mods = [k for k in sys.modules if k.startswith("fabric_cli")] - for m in mods: - del sys.modules[m] - - # Also clear heavy modules - for mod in HEAVY_MODULES: - keys = [k for k in sys.modules if k.startswith(mod)] - for k in keys: - del sys.modules[k] - - importlib.import_module("fabric_cli.main") - - return {mod: mod in sys.modules for mod in HEAVY_MODULES} - - -def measure_cli_time(iterations: int) -> dict: - """Measure 'fab --version' wall-clock time.""" - fab_path = shutil.which("fab") - if not fab_path: - return {"error": "'fab' not found in PATH. Run 'pip install -e .' first."} - - times = [] - for _ in range(iterations): - start = time.perf_counter() - subprocess.run( - [fab_path, "--version"], - capture_output=True, - text=True, - ) - elapsed_ms = (time.perf_counter() - start) * 1000 - times.append(elapsed_ms) - - return { - "median_ms": round(statistics.median(times), 1), - "min_ms": round(min(times), 1), - "max_ms": round(max(times), 1), - "mean_ms": round(statistics.mean(times), 1), - "stdev_ms": round(statistics.stdev(times), 1) if len(times) > 1 else 0, - } - - -def run_benchmark(label: str, iterations: int) -> dict: - """Run all benchmarks and return results.""" - print(f"\n{'=' * 60}") - print(f" Benchmarking: {label}") - print(f"{'=' * 60}") - - # 1. Import time - print(f" Measuring import time ({iterations} iterations)...", end="", flush=True) - import_results = measure_import_time(iterations) - print(f" {import_results['median_ms']:.0f}ms median") - - # 2. Heavy modules - print(" Checking heavy module loading...", end="", flush=True) - heavy_results = check_heavy_modules() - loaded = [m for m, v in heavy_results.items() if v] - print(f" {len(loaded)} loaded: {', '.join(loaded) if loaded else 'none'}") - - # 3. CLI time - print(f" Measuring 'fab --version' ({iterations} iterations)...", end="", flush=True) - cli_results = measure_cli_time(iterations) - if "error" in cli_results: - print(f" {cli_results['error']}") - else: - print(f" {cli_results['median_ms']:.0f}ms median") - - return { - "label": label, - "import_time": import_results, - "heavy_modules": heavy_results, - "cli_time": cli_results, - } - - -def print_comparison(baseline: dict, current: dict): - """Print a formatted comparison table.""" - print(f"\n{'=' * 60}") - print(" COMPARISON") - print(f"{'=' * 60}\n") - - bl = baseline["import_time"]["median_ms"] - cu = current["import_time"]["median_ms"] - diff = bl - cu - pct = (diff / bl * 100) if bl > 0 else 0 - - print(f" {'Metric':<30} {'Baseline':>10} {'Current':>10} {'Change':>10}") - print(f" {'-' * 62}") - print(f" {'Import time (median):':<30} {bl:>9.0f}ms {cu:>9.0f}ms {diff:>+8.0f}ms") - print(f" {'Import improvement:':<30} {'':>10} {'':>10} {pct:>+8.0f}%") - - if "error" not in baseline["cli_time"] and "error" not in current["cli_time"]: - bl_cli = baseline["cli_time"]["median_ms"] - cu_cli = current["cli_time"]["median_ms"] - cli_diff = bl_cli - cu_cli - cli_pct = (cli_diff / bl_cli * 100) if bl_cli > 0 else 0 - print(f" {'CLI time (median):':<30} {bl_cli:>9.0f}ms {cu_cli:>9.0f}ms {cli_diff:>+8.0f}ms") - print(f" {'CLI improvement:':<30} {'':>10} {'':>10} {cli_pct:>+8.0f}%") - - print(f"\n {'Heavy modules at startup:':<30}") - for mod in HEAVY_MODULES: - bl_loaded = "LOADED" if baseline["heavy_modules"].get(mod) else "deferred" - cu_loaded = "LOADED" if current["heavy_modules"].get(mod) else "deferred" - marker = " ✓" if cu_loaded == "deferred" and bl_loaded == "LOADED" else "" - print(f" {mod:<25} {bl_loaded:>10} {cu_loaded:>10}{marker}") - - print() - - -def git_checkout_and_install(ref: str): - """Checkout a git ref and reinstall the package.""" - print(f"\n Switching to '{ref}'...") - subprocess.run(["git", "checkout", ref], capture_output=True, check=True) - subprocess.run( - [sys.executable, "-m", "pip", "install", "-e", ".", "-q"], - capture_output=True, - check=True, - ) - # Clear all cached fabric_cli modules after reinstall - mods = [k for k in sys.modules if k.startswith("fabric_cli")] - for m in mods: - del sys.modules[m] - - -def main(): - parser = argparse.ArgumentParser( - description="Benchmark CLI startup performance between branches." - ) - parser.add_argument( - "--baseline", - default="main", - help="Git ref to compare against (default: main)", - ) - parser.add_argument( - "--iterations", "-n", - type=int, - default=10, - help="Number of iterations per measurement (default: 10)", - ) - parser.add_argument( - "--current-only", - action="store_true", - help="Only benchmark the current branch (skip baseline)", - ) - parser.add_argument( - "--json", - action="store_true", - help="Output results as JSON", - ) - args = parser.parse_args() - - repo_root = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) - os.chdir(repo_root) - - # Get current branch name - result = subprocess.run( - ["git", "rev-parse", "--abbrev-ref", "HEAD"], - capture_output=True, text=True, - ) - current_branch = result.stdout.strip() - - print(f" Repo: {repo_root}") - print(f" Current branch: {current_branch}") - print(f" Baseline: {args.baseline}") - print(f" Iterations: {args.iterations}") - - results = {} - - if not args.current_only: - # Benchmark baseline - try: - git_checkout_and_install(args.baseline) - results["baseline"] = run_benchmark(f"Baseline ({args.baseline})", args.iterations) - except subprocess.CalledProcessError: - print(f"\n ERROR: Could not checkout '{args.baseline}'. Does it exist?") - print(f" Try: python scripts/benchmark_startup.py --current-only") - sys.exit(1) - finally: - # Always return to original branch - subprocess.run(["git", "checkout", current_branch], capture_output=True) - subprocess.run( - [sys.executable, "-m", "pip", "install", "-e", ".", "-q"], - capture_output=True, - ) - # Clear modules again - mods = [k for k in sys.modules if k.startswith("fabric_cli")] - for m in mods: - del sys.modules[m] - - # Benchmark current - results["current"] = run_benchmark(f"Current ({current_branch})", args.iterations) - - # Print comparison - if "baseline" in results: - print_comparison(results["baseline"], results["current"]) - - # JSON output - if args.json: - # Remove raw samples for cleaner JSON - for key in results: - if "samples" in results[key].get("import_time", {}): - del results[key]["import_time"]["samples"] - print(json.dumps(results, indent=2)) - - -if __name__ == "__main__": - main() From dcdec925b9af3557664c8a41ba74cf2b53f0a6fd Mon Sep 17 00:00:00 2001 From: Alon Yeshurun Date: Sun, 22 Mar 2026 10:11:20 +0000 Subject: [PATCH 07/11] Delete file --- scripts/benchmark_startup.py | 270 ----------------------------------- 1 file changed, 270 deletions(-) delete mode 100644 scripts/benchmark_startup.py diff --git a/scripts/benchmark_startup.py b/scripts/benchmark_startup.py deleted file mode 100644 index 0d502613c..000000000 --- a/scripts/benchmark_startup.py +++ /dev/null @@ -1,270 +0,0 @@ -#!/usr/bin/env python3 -""" -Benchmark CLI startup performance. - -Compares the current branch against 'main' (or any other branch) by measuring: - 1. Module import time (fabric_cli.main) - 2. CLI invocation time (fab --version) - 3. Heavy dependency loading (msal, jwt, cryptography, requests, prompt_toolkit) - -Usage: - # Compare current branch against main - python scripts/benchmark_startup.py - - # Compare current branch against a specific branch/tag/commit - python scripts/benchmark_startup.py --baseline v1.3.0 - - # Run only on the current branch (no git checkout) - python scripts/benchmark_startup.py --current-only - - # Change number of iterations (default: 10) - python scripts/benchmark_startup.py --iterations 20 -""" - -import argparse -import importlib -import json -import os -import shutil -import statistics -import subprocess -import sys -import time - - -HEAVY_MODULES = ["msal", "jwt", "cryptography", "requests", "prompt_toolkit", "psutil"] - - -def measure_import_time(iterations: int) -> dict: - """Measure fabric_cli.main import time across multiple iterations.""" - times = [] - for _ in range(iterations): - # Clear all fabric_cli modules from cache - mods = [k for k in sys.modules if k.startswith("fabric_cli")] - for m in mods: - del sys.modules[m] - - start = time.perf_counter() - importlib.import_module("fabric_cli.main") - elapsed_ms = (time.perf_counter() - start) * 1000 - times.append(elapsed_ms) - - return { - "median_ms": round(statistics.median(times), 1), - "min_ms": round(min(times), 1), - "max_ms": round(max(times), 1), - "mean_ms": round(statistics.mean(times), 1), - "stdev_ms": round(statistics.stdev(times), 1) if len(times) > 1 else 0, - "samples": times, - } - - -def check_heavy_modules() -> dict: - """Check which heavy modules are loaded after importing fabric_cli.main.""" - # Clear all fabric_cli modules - mods = [k for k in sys.modules if k.startswith("fabric_cli")] - for m in mods: - del sys.modules[m] - - # Also clear heavy modules - for mod in HEAVY_MODULES: - keys = [k for k in sys.modules if k.startswith(mod)] - for k in keys: - del sys.modules[k] - - importlib.import_module("fabric_cli.main") - - return {mod: mod in sys.modules for mod in HEAVY_MODULES} - - -def measure_cli_time(iterations: int) -> dict: - """Measure 'fab --version' wall-clock time.""" - fab_path = shutil.which("fab") - if not fab_path: - return {"error": "'fab' not found in PATH. Run 'pip install -e .' first."} - - times = [] - for _ in range(iterations): - start = time.perf_counter() - subprocess.run( - [fab_path, "--version"], - capture_output=True, - text=True, - ) - elapsed_ms = (time.perf_counter() - start) * 1000 - times.append(elapsed_ms) - - return { - "median_ms": round(statistics.median(times), 1), - "min_ms": round(min(times), 1), - "max_ms": round(max(times), 1), - "mean_ms": round(statistics.mean(times), 1), - "stdev_ms": round(statistics.stdev(times), 1) if len(times) > 1 else 0, - } - - -def run_benchmark(label: str, iterations: int) -> dict: - """Run all benchmarks and return results.""" - print(f"\n{'=' * 60}") - print(f" Benchmarking: {label}") - print(f"{'=' * 60}") - - # 1. Import time - print(f" Measuring import time ({iterations} iterations)...", end="", flush=True) - import_results = measure_import_time(iterations) - print(f" {import_results['median_ms']:.0f}ms median") - - # 2. Heavy modules - print(" Checking heavy module loading...", end="", flush=True) - heavy_results = check_heavy_modules() - loaded = [m for m, v in heavy_results.items() if v] - print(f" {len(loaded)} loaded: {', '.join(loaded) if loaded else 'none'}") - - # 3. CLI time - print(f" Measuring 'fab --version' ({iterations} iterations)...", end="", flush=True) - cli_results = measure_cli_time(iterations) - if "error" in cli_results: - print(f" {cli_results['error']}") - else: - print(f" {cli_results['median_ms']:.0f}ms median") - - return { - "label": label, - "import_time": import_results, - "heavy_modules": heavy_results, - "cli_time": cli_results, - } - - -def print_comparison(baseline: dict, current: dict): - """Print a formatted comparison table.""" - print(f"\n{'=' * 60}") - print(" COMPARISON") - print(f"{'=' * 60}\n") - - bl = baseline["import_time"]["median_ms"] - cu = current["import_time"]["median_ms"] - diff = bl - cu - pct = (diff / bl * 100) if bl > 0 else 0 - - print(f" {'Metric':<30} {'Baseline':>10} {'Current':>10} {'Change':>10}") - print(f" {'-' * 62}") - print(f" {'Import time (median):':<30} {bl:>9.0f}ms {cu:>9.0f}ms {diff:>+8.0f}ms") - print(f" {'Import improvement:':<30} {'':>10} {'':>10} {pct:>+8.0f}%") - - if "error" not in baseline["cli_time"] and "error" not in current["cli_time"]: - bl_cli = baseline["cli_time"]["median_ms"] - cu_cli = current["cli_time"]["median_ms"] - cli_diff = bl_cli - cu_cli - cli_pct = (cli_diff / bl_cli * 100) if bl_cli > 0 else 0 - print(f" {'CLI time (median):':<30} {bl_cli:>9.0f}ms {cu_cli:>9.0f}ms {cli_diff:>+8.0f}ms") - print(f" {'CLI improvement:':<30} {'':>10} {'':>10} {cli_pct:>+8.0f}%") - - print(f"\n {'Heavy modules at startup:':<30}") - for mod in HEAVY_MODULES: - bl_loaded = "LOADED" if baseline["heavy_modules"].get(mod) else "deferred" - cu_loaded = "LOADED" if current["heavy_modules"].get(mod) else "deferred" - marker = " ✓" if cu_loaded == "deferred" and bl_loaded == "LOADED" else "" - print(f" {mod:<25} {bl_loaded:>10} {cu_loaded:>10}{marker}") - - print() - - -def git_checkout_and_install(ref: str): - """Checkout a git ref and reinstall the package.""" - print(f"\n Switching to '{ref}'...") - subprocess.run(["git", "checkout", ref], capture_output=True, check=True) - subprocess.run( - [sys.executable, "-m", "pip", "install", "-e", ".", "-q"], - capture_output=True, - check=True, - ) - # Clear all cached fabric_cli modules after reinstall - mods = [k for k in sys.modules if k.startswith("fabric_cli")] - for m in mods: - del sys.modules[m] - - -def main(): - parser = argparse.ArgumentParser( - description="Benchmark CLI startup performance between branches." - ) - parser.add_argument( - "--baseline", - default="main", - help="Git ref to compare against (default: main)", - ) - parser.add_argument( - "--iterations", "-n", - type=int, - default=10, - help="Number of iterations per measurement (default: 10)", - ) - parser.add_argument( - "--current-only", - action="store_true", - help="Only benchmark the current branch (skip baseline)", - ) - parser.add_argument( - "--json", - action="store_true", - help="Output results as JSON", - ) - args = parser.parse_args() - - repo_root = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) - os.chdir(repo_root) - - # Get current branch name - result = subprocess.run( - ["git", "rev-parse", "--abbrev-ref", "HEAD"], - capture_output=True, text=True, - ) - current_branch = result.stdout.strip() - - print(f" Repo: {repo_root}") - print(f" Current branch: {current_branch}") - print(f" Baseline: {args.baseline}") - print(f" Iterations: {args.iterations}") - - results = {} - - if not args.current_only: - # Benchmark baseline - try: - git_checkout_and_install(args.baseline) - results["baseline"] = run_benchmark(f"Baseline ({args.baseline})", args.iterations) - except subprocess.CalledProcessError: - print(f"\n ERROR: Could not checkout '{args.baseline}'. Does it exist?") - print(f" Try: python scripts/benchmark_startup.py --current-only") - sys.exit(1) - finally: - # Always return to original branch - subprocess.run(["git", "checkout", current_branch], capture_output=True) - subprocess.run( - [sys.executable, "-m", "pip", "install", "-e", ".", "-q"], - capture_output=True, - ) - # Clear modules again - mods = [k for k in sys.modules if k.startswith("fabric_cli")] - for m in mods: - del sys.modules[m] - - # Benchmark current - results["current"] = run_benchmark(f"Current ({current_branch})", args.iterations) - - # Print comparison - if "baseline" in results: - print_comparison(results["baseline"], results["current"]) - - # JSON output - if args.json: - # Remove raw samples for cleaner JSON - for key in results: - if "samples" in results[key].get("import_time", {}): - del results[key]["import_time"]["samples"] - print(json.dumps(results, indent=2)) - - -if __name__ == "__main__": - main() From 20aa53fd26b8a61886b6cd539a9a28bbb480837a Mon Sep 17 00:00:00 2001 From: Alon Yeshurun Date: Sun, 22 Mar 2026 10:16:06 +0000 Subject: [PATCH 08/11] revert --- src/fabric_cli/client/fab_api_client.py | 24 ++++++-- .../core/fab_config/command_support.yaml | 4 ++ src/fabric_cli/core/fab_types.py | 4 ++ src/fabric_cli/core/hiearchy/fab_item.py | 1 + tests/test_commands/conftest.py | 10 +++- tests/test_core/test_map_item_removal.py | 31 ---------- tests/test_utils/test_fab_api_client.py | 25 ++++---- tests/test_utils/test_session_per_request.py | 60 ------------------- 8 files changed, 47 insertions(+), 112 deletions(-) delete mode 100644 tests/test_core/test_map_item_removal.py delete mode 100644 tests/test_utils/test_session_per_request.py diff --git a/src/fabric_cli/client/fab_api_client.py b/src/fabric_cli/client/fab_api_client.py index b1de77845..f396a4b8b 100644 --- a/src/fabric_cli/client/fab_api_client.py +++ b/src/fabric_cli/client/fab_api_client.py @@ -29,6 +29,23 @@ GUID_PATTERN = r"([a-f0-9\-]{36})" FABRIC_WORKSPACE_URI_PATTERN = rf"workspaces/{GUID_PATTERN}" +# Module-level reusable session for connection pooling +_shared_session = None + + +def _get_session() -> requests.Session: + """Return a shared requests session with retry and connection pooling.""" + global _shared_session + if _shared_session is None: + _shared_session = requests.Session() + retries = Retry( + total=3, backoff_factor=1, status_forcelist=[502, 503, 504] + ) + adapter = HTTPAdapter(max_retries=retries) + _shared_session.mount("https://", adapter) + _shared_session.headers.update({"Accept-Encoding": "gzip, deflate"}) + return _shared_session + def do_request( args, @@ -114,13 +131,8 @@ def do_request( ) try: - session = requests.Session() + session = _get_session() retries_count = 3 - retries = Retry( - total=retries_count, backoff_factor=1, status_forcelist=[502, 503, 504] - ) - adapter = HTTPAdapter(max_retries=retries) - session.mount("https://", adapter) request_params = { "headers": headers, diff --git a/src/fabric_cli/core/fab_config/command_support.yaml b/src/fabric_cli/core/fab_config/command_support.yaml index 822aa9808..da19862df 100644 --- a/src/fabric_cli/core/fab_config/command_support.yaml +++ b/src/fabric_cli/core/fab_config/command_support.yaml @@ -171,6 +171,7 @@ commands: - dataflow - sql_database - user_data_function + - map cp: supported_elements: - workspace @@ -199,6 +200,7 @@ commands: - dataflow - sql_database - user_data_function + - map ln: supported_elements: - onelake @@ -253,6 +255,7 @@ commands: - cosmos_db_database - user_data_function - graph_query_set + - map import: supported_items: - report @@ -276,6 +279,7 @@ commands: - sql_database - cosmos_db_database - user_data_function + - map unsupported_items: - graph_query_set get: diff --git a/src/fabric_cli/core/fab_types.py b/src/fabric_cli/core/fab_types.py index e5f11f3f9..e200296ec 100644 --- a/src/fabric_cli/core/fab_types.py +++ b/src/fabric_cli/core/fab_types.py @@ -270,6 +270,7 @@ class ItemType(_BaseItemType): KQL_DATABASE = "KQLDatabase" KQL_QUERYSET = "KQLQueryset" LAKEHOUSE = "Lakehouse" + MAP = "Map" MIRRORED_WAREHOUSE = "MirroredWarehouse" MIRRORED_DATABASE = "MirroredDatabase" ML_EXPERIMENT = "MLExperiment" @@ -509,6 +510,7 @@ class MirroredDatabaseFolders(Enum): ItemType.KQL_DATABASE: "kqlDatabases", ItemType.KQL_QUERYSET: "kqlQuerysets", ItemType.LAKEHOUSE: "lakehouses", + ItemType.MAP: "maps", ItemType.ML_EXPERIMENT: "mlExperiments", ItemType.ML_MODEL: "mlModels", ItemType.MIRRORED_WAREHOUSE: "mirroredWarehouses", @@ -556,6 +558,7 @@ class MirroredDatabaseFolders(Enum): ItemType.KQL_DATABASE: "databases", ItemType.KQL_QUERYSET: "queryworkbenches", ItemType.LAKEHOUSE: "lakehouses", + ItemType.MAP: "maps", ItemType.MIRRORED_DATABASE: "mirroreddatabases", ItemType.ML_EXPERIMENT: "mlexperiments", ItemType.ML_MODEL: "mlmodels", @@ -597,4 +600,5 @@ class MirroredDatabaseFolders(Enum): ItemType.COSMOS_DB_DATABASE: {"default": ""}, ItemType.USER_DATA_FUNCTION: {"default": ""}, ItemType.GRAPH_QUERY_SET: {"default": ""}, + ItemType.MAP: {"default": ""}, } diff --git a/src/fabric_cli/core/hiearchy/fab_item.py b/src/fabric_cli/core/hiearchy/fab_item.py index 37f3c72cd..0707cb187 100644 --- a/src/fabric_cli/core/hiearchy/fab_item.py +++ b/src/fabric_cli/core/hiearchy/fab_item.py @@ -141,6 +141,7 @@ def get_payload(self, definition, input_format=None) -> dict: | ItemType.COSMOS_DB_DATABASE | ItemType.GRAPH_QUERY_SET | ItemType.USER_DATA_FUNCTION + | ItemType.MAP ): return { "type": str(self.item_type), diff --git a/tests/test_commands/conftest.py b/tests/test_commands/conftest.py index 4980aea95..b389acfd2 100644 --- a/tests/test_commands/conftest.py +++ b/tests/test_commands/conftest.py @@ -53,6 +53,7 @@ ItemType.SPARK_JOB_DEFINITION, ItemType.WAREHOUSE, ItemType.COPYJOB, ItemType.GRAPHQLAPI, ItemType.DATAFLOW, ItemType.COSMOS_DB_DATABASE, ItemType.USER_DATA_FUNCTION, ItemType.DIGITAL_TWIN_BUILDER, ItemType.GRAPH_QUERY_SET, + ItemType.MAP, ] item_type_paramerter = pytest.mark.parametrize("item_type", ALL_ITEM_TYPES) @@ -63,6 +64,7 @@ ItemType.ML_MODEL, ItemType.MIRRORED_DATABASE, ItemType.NOTEBOOK, ItemType.REFLEX, ItemType.SPARK_JOB_DEFINITION, ItemType.COSMOS_DB_DATABASE, ItemType.USER_DATA_FUNCTION, ItemType.DIGITAL_TWIN_BUILDER, ItemType.GRAPH_QUERY_SET, + ItemType.MAP, ]) rm_item_without_force_cancel_operation_success_params = pytest.mark.parametrize("item_type", [ @@ -89,7 +91,7 @@ ItemType.DATA_PIPELINE, ItemType.KQL_DASHBOARD, ItemType.KQL_QUERYSET, ItemType.MIRRORED_DATABASE, ItemType.NOTEBOOK, ItemType.REFLEX, ItemType.SPARK_JOB_DEFINITION, - ItemType.COSMOS_DB_DATABASE, ItemType.USER_DATA_FUNCTION + ItemType.COSMOS_DB_DATABASE, ItemType.USER_DATA_FUNCTION, ItemType.MAP ]) mv_item_to_item_unsupported_failure_params = pytest.mark.parametrize("unsupported_item_type", [ @@ -108,7 +110,7 @@ ItemType.DATA_PIPELINE, ItemType.KQL_DASHBOARD, ItemType.KQL_QUERYSET, ItemType.MIRRORED_DATABASE, ItemType.NOTEBOOK, ItemType.REFLEX, ItemType.SPARK_JOB_DEFINITION, - ItemType.COSMOS_DB_DATABASE, ItemType.USER_DATA_FUNCTION + ItemType.COSMOS_DB_DATABASE, ItemType.USER_DATA_FUNCTION, ItemType.MAP ]) get_item_with_properties_success_params = pytest.mark.parametrize("item_type,expected_properties", [ @@ -154,7 +156,7 @@ ItemType.DATA_PIPELINE, ItemType.ENVIRONMENT, ItemType.EVENTSTREAM, ItemType.KQL_DASHBOARD, ItemType.KQL_QUERYSET, ItemType.ML_EXPERIMENT, ItemType.NOTEBOOK, ItemType.REFLEX, ItemType.SPARK_JOB_DEFINITION, - ItemType.USER_DATA_FUNCTION, ItemType.DIGITAL_TWIN_BUILDER + ItemType.USER_DATA_FUNCTION, ItemType.DIGITAL_TWIN_BUILDER, ItemType.MAP ]) set_item_metadata_success_params = pytest.mark.parametrize( @@ -211,6 +213,7 @@ (ItemType.COSMOS_DB_DATABASE, ".json"), (ItemType.USER_DATA_FUNCTION, ".json"), (ItemType.GRAPH_QUERY_SET, ".json"), + (ItemType.MAP, ".json") ]) export_item_types_parameters = pytest.mark.parametrize("item_type", [ @@ -224,6 +227,7 @@ ItemType.COSMOS_DB_DATABASE, ItemType.USER_DATA_FUNCTION, ItemType.GRAPH_QUERY_SET, + ItemType.MAP ]) export_item_format_parameters = pytest.mark.parametrize( diff --git a/tests/test_core/test_map_item_removal.py b/tests/test_core/test_map_item_removal.py deleted file mode 100644 index ae9117c2f..000000000 --- a/tests/test_core/test_map_item_removal.py +++ /dev/null @@ -1,31 +0,0 @@ -"""Regression tests — ItemType.MAP removal.""" - -import pytest -from fabric_cli.core.fab_types import ItemType - - -class TestMapItemRemoval: - """Verify MAP enum member and its mapping entries are removed.""" - - def test_map_not_in_item_type_enum(self): - assert not hasattr(ItemType, "MAP"), "ItemType.MAP should be removed" - - def test_map_not_in_enum_values(self): - assert "Map" not in [m.value for m in ItemType] - - def test_map_not_in_format_mapping(self): - from fabric_cli.core.fab_types import format_mapping - assert "Map" not in format_mapping - - def test_map_not_in_uri_mapping(self): - from fabric_cli.core.fab_types import uri_mapping - assert "Map" not in uri_mapping - - def test_map_not_in_definition_format_mapping(self): - from fabric_cli.core.fab_types import definition_format_mapping - assert "Map" not in definition_format_mapping - - def test_other_item_types_still_exist(self): - """Sanity check — common item types are still present.""" - for name in ("Notebook", "Lakehouse", "SemanticModel", "Report"): - assert hasattr(ItemType, name.upper()) or name in [m.value for m in ItemType] diff --git a/tests/test_utils/test_fab_api_client.py b/tests/test_utils/test_fab_api_client.py index c83445052..64acfa859 100644 --- a/tests/test_utils/test_fab_api_client.py +++ b/tests/test_utils/test_fab_api_client.py @@ -1,22 +1,23 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. -"""Tests for HTTP session handling in fab_api_client.""" +"""Tests for HTTP session reuse in fab_api_client.""" -class TestSessionPerRequestUpstream: - """Verify per-request session creation (shared session pool removed).""" +class TestSessionReuse: + """Test suite for HTTP session reuse.""" - def test_no_shared_session_module_attribute(self): - """_shared_session module attribute must not exist.""" + def test_shared_session__returns_same_instance(self): + """Test that _get_session returns the same session instance.""" from fabric_cli.client import fab_api_client - assert not hasattr(fab_api_client, "_shared_session"), \ - "Shared session singleton should be removed" + # Reset the shared session + fab_api_client._shared_session = None - def test_no_get_session_function(self): - """_get_session helper must not exist.""" - from fabric_cli.client import fab_api_client + session1 = fab_api_client._get_session() + session2 = fab_api_client._get_session() + + assert session1 is session2, "Session should be reused" - assert not hasattr(fab_api_client, "_get_session"), \ - "_get_session helper should be removed" + # Clean up + fab_api_client._shared_session = None diff --git a/tests/test_utils/test_session_per_request.py b/tests/test_utils/test_session_per_request.py deleted file mode 100644 index 6e30a9cfe..000000000 --- a/tests/test_utils/test_session_per_request.py +++ /dev/null @@ -1,60 +0,0 @@ -"""Regression tests — per-request session creation (no shared session pool).""" - -import pytest -from unittest.mock import patch, MagicMock -from argparse import Namespace -from fabric_cli.client import fab_api_client - - -class TestSessionPerRequest: - """Verify each do_request creates a fresh Session (no shared pool).""" - - def test_no_shared_session_attribute(self): - assert not hasattr(fab_api_client, "_shared_session"), \ - "Shared session singleton should be removed" - - def test_no_get_session_function(self): - assert not hasattr(fab_api_client, "_get_session"), \ - "_get_session helper should be removed" - - def test_do_request_creates_new_session_each_call(self): - fake_args = Namespace( - json_file=None, - audience=None, - method="GET", - wait=False, - raw_response=False, - request_params=None, - uri="workspaces/00000000-0000-0000-0000-000000000001/items", - headers=None, - ) - mock_response = MagicMock() - mock_response.status_code = 200 - mock_response.headers = {"x-ms-request-id": "test"} - mock_response.json.return_value = {} - - sessions = [] - - def track_session(*a, **kw): - s = MagicMock() - s.request.return_value = mock_response - sessions.append(s) - return s - - with patch("fabric_cli.client.fab_api_client.requests.Session", side_effect=track_session), \ - patch("fabric_cli.core.fab_auth.FabAuth") as mock_auth_cls, \ - patch("fabric_cli.core.fab_context.Context") as mock_ctx_cls, \ - patch("fabric_cli.core.fab_state_config.get_config", return_value=None): - mock_auth_cls.return_value.get_access_token.return_value = "tok" - mock_ctx_cls.return_value.command = "test" - try: - fab_api_client.do_request(fake_args) - except Exception: - pass - try: - fab_api_client.do_request(fake_args) - except Exception: - pass - - assert len(sessions) >= 2, "Each do_request should create its own Session" - assert sessions[0] is not sessions[1] From daa49eb7168495993322c3911ca11916501a0504 Mon Sep 17 00:00:00 2001 From: Alon Yeshurun Date: Sun, 22 Mar 2026 10:26:13 +0000 Subject: [PATCH 09/11] revert --- src/fabric_cli/main.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/fabric_cli/main.py b/src/fabric_cli/main.py index ee96863cc..8581e00a0 100644 --- a/src/fabric_cli/main.py +++ b/src/fabric_cli/main.py @@ -29,7 +29,9 @@ def main(): return if args.command == "auth" and args.auth_command == "login": - login.init(args) + from fabric_cli.commands.auth import fab_auth + + fab_auth.init(args) if args.command == "auth" and args.auth_command == "logout": from fabric_cli.commands.auth import fab_auth From ef889b3170550192a913e788f753d68327232d17 Mon Sep 17 00:00:00 2001 From: Alon Yeshurun Date: Sun, 22 Mar 2026 10:39:15 +0000 Subject: [PATCH 10/11] Add changelog entry --- .changes/unreleased/optimization-20260322-103653.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changes/unreleased/optimization-20260322-103653.yaml 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 From 17e174c5c2b5d7132c497921919e6acf678aaf0e Mon Sep 17 00:00:00 2001 From: Alon Yeshurun Date: Wed, 25 Mar 2026 11:24:36 +0000 Subject: [PATCH 11/11] Address comments --- .github/instructions/test.instructions.md | 12 +- src/fabric_cli/main.py | 11 +- tests/conftest.py | 2 + tests/test_commands/test_config.py | 117 +++++++------------- tests/test_core/test_context_persistence.py | 7 -- tests/test_core/test_fab_context.py | 15 +-- tests/test_core/test_fab_state_config.py | 106 ++++++++---------- 7 files changed, 103 insertions(+), 167 deletions(-) diff --git a/.github/instructions/test.instructions.md b/.github/instructions/test.instructions.md index c0d532e77..5cedaa22c 100644 --- a/.github/instructions/test.instructions.md +++ b/.github/instructions/test.instructions.md @@ -38,7 +38,7 @@ This guide defines how **every test** in Fabric CLI should be designed, implemen ## 2) Structure & naming - **File names**: `test__.py` (e.g., `test_items_ls.py`, `test_gateways_get.py`) -- **Test names**: `test___` (double underscore between behavior and condition) +- **Test names**: `test__` (single underscore between behavior and condition) - **Parametrization**: prefer `@pytest.mark.parametrize` for path variations (absolute, relative, nested, hidden) --- @@ -88,7 +88,7 @@ from fabric_cli.__main__ import main # or an entrypoint that dispatches argv from fabric_cli import constants as fab_const @responses.activate -def test_ls_semantic_models_under_workspace__json_output(capsys, tmp_home, auth_stub): +def test_ls_semantic_models_under_workspace_json_output(capsys, tmp_home, auth_stub): # Mock list items under workspace responses.add( responses.GET, @@ -123,7 +123,7 @@ vcr_recorder = vcr.VCR( ) @pytest.mark.playback -def test_ls_capacities_hidden_collection__table_output(capsys, tmp_home, auth_stub): +def test_ls_capacities_hidden_collection_table_output(capsys, tmp_home, auth_stub): with vcr_recorder.use_cassette("ls_capacities_hidden.yaml"): argv = ["ls", "-a", ".capacities", "-o", "table"] rc = main(argv) or 0 @@ -179,7 +179,7 @@ pytest -q tests/test_core tests/test_utils pytest -q tests/test_commands --playback # Optional: run a single test -pytest -q tests/test_commands/test_items_ls.py::test_ls_semantic_models_under_workspace__json_output +pytest -q tests/test_commands/test_items_ls.py::test_ls_semantic_models_under_workspace_json_output ``` @@ -193,7 +193,7 @@ import pytest from fabric_cli.commands.items.list import attach_items_parsers from argparse import ArgumentParser -def test_items_ls_parser__has_all_flag_and_output_modes(): +def test_items_ls_parser_has_all_flag_and_output_modes(): parser = ArgumentParser() subs = parser.add_subparsers() attach_items_parsers(subs) @@ -210,7 +210,7 @@ import pytest, responses from fabric_cli.__main__ import main @responses.activate -def test_get_item__404_maps_to_fabric_api_error(capsys): +def test_get_item_404_maps_to_fabric_api_error(capsys): responses.add( responses.GET, "https://api.fabric.microsoft.com/v1/items/does-not-exist", diff --git a/src/fabric_cli/main.py b/src/fabric_cli/main.py index 8581e00a0..9e3aa6d57 100644 --- a/src/fabric_cli/main.py +++ b/src/fabric_cli/main.py @@ -11,12 +11,11 @@ from fabric_cli.core.fab_parser_setup import get_global_parser_and_subparsers 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 def main(): parser, subparsers = get_global_parser_and_subparsers() - + argcomplete.autocomplete(parser, default_completer=None) args = parser.parse_args() @@ -32,6 +31,7 @@ def main(): from fabric_cli.commands.auth import fab_auth fab_auth.init(args) + return if args.command == "auth" and args.auth_command == "logout": from fabric_cli.commands.auth import fab_auth @@ -111,11 +111,11 @@ def _handle_unexpected_error(err, args): error_message = str(err.args[0]) if err.args else str(err) except: error_message = "An unexpected error occurred" - + fab_ui.print_output_error( - FabricCLIError(error_message, fab_constant.ERROR_UNEXPECTED_ERROR), + FabricCLIError(error_message, fab_constant.ERROR_UNEXPECTED_ERROR), output_format_type=args.output_format, - ) + ) sys.exit(fab_constant.EXIT_CODE_ERROR) @@ -137,4 +137,3 @@ def _execute_command(args, subparsers, parser): if __name__ == "__main__": main() - diff --git a/tests/conftest.py b/tests/conftest.py index 4ca534551..b5a1c26db 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -71,11 +71,13 @@ def _set_config(key: str, value: str): @pytest.fixture def reset_context(): """Reset the Context singleton before test to prevent state leakage.""" + from fabric_cli.core import fab_constant from fabric_cli.core.fab_context import Context context_instance = Context() context_instance._context = None context_instance._command = None context_instance._loading_context = False + context_instance._runtime_mode = fab_constant.FAB_MODE_COMMANDLINE yield context_instance diff --git a/tests/test_commands/test_config.py b/tests/test_commands/test_config.py index 036335593..2ef97b37e 100644 --- a/tests/test_commands/test_config.py +++ b/tests/test_commands/test_config.py @@ -1,7 +1,6 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. -from argparse import Namespace from unittest.mock import patch import pytest @@ -174,37 +173,34 @@ def test_config_clear_cache_success( # region config MODE (deprecated) def test_config_set_mode_interactive_shows_deprecation_warning( - self, mock_questionary_print, cli_executor: CLIExecutor + self, mock_questionary_print, mock_print_warning, cli_executor: CLIExecutor ): """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: + with patch("fabric_cli.core.fab_interactive.start_interactive_mode") as mock_repl: cli_executor.exec_command(f"config set mode {constant.FAB_MODE_INTERACTIVE}") mock_print_warning.assert_any_call(DEPRECATION_WARNING) mock_repl.assert_called_once() def test_config_set_mode_command_line_shows_deprecation_warning( - self, mock_questionary_print, cli_executor: CLIExecutor + self, mock_questionary_print, mock_print_warning, cli_executor: CLIExecutor ): """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: + with patch("fabric_cli.core.fab_interactive.start_interactive_mode") as mock_repl: cli_executor.exec_command(f"config set mode {constant.FAB_MODE_COMMANDLINE}") mock_print_warning.assert_any_call(DEPRECATION_WARNING) mock_repl.assert_not_called() def test_config_get_mode_shows_deprecation_warning( - self, mock_questionary_print, cli_executor: CLIExecutor + self, mock_questionary_print, mock_print_warning, cli_executor: CLIExecutor ): """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") + cli_executor.exec_command("config get mode") - mock_print_warning.assert_any_call(DEPRECATION_WARNING) - # Should still output the runtime mode value - mock_questionary_print.assert_called() + mock_print_warning.assert_any_call(DEPRECATION_WARNING) + # Should still output the runtime mode value + mock_questionary_print.assert_called() # endregion @@ -219,83 +215,50 @@ def test_config_get_mode_shows_deprecation_warning( 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") + @pytest.fixture + def mock_repl(self): + with patch("fabric_cli.core.fab_interactive.start_interactive_mode") as mock: + yield mock - 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) + def test_config_set_mode_interactive_warns_and_launches_repl_success( + self, mock_questionary_print, mock_print_warning, mock_repl, cli_executor: CLIExecutor + ): + """'config set mode interactive' must warn and launch REPL.""" + cli_executor.exec_command(f"config set mode {constant.FAB_MODE_INTERACTIVE}") - mock_warn.assert_called_once_with(DEPRECATION_WARNING) - mock_set.assert_not_called() - mock_repl.assert_called_once() + mock_print_warning.assert_called_once_with(DEPRECATION_WARNING) + 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): + def test_config_set_mode_non_interactive_warns_without_repl_success( + self, mode_value, mock_questionary_print, mock_print_warning, mock_repl, cli_executor: CLIExecutor + ): """'config set mode command_line' (or bogus) must warn but not launch REPL.""" - from fabric_cli.commands.config import fab_config_set + cli_executor.exec_command(f"config set mode {mode_value}") - 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() + mock_print_warning.assert_called_once_with(DEPRECATION_WARNING) + mock_repl.assert_not_called() - def test_config_set_non_mode_key__still_works(self): + def test_config_set_non_mode_key_still_works_success( + self, mock_questionary_print, cli_executor: CLIExecutor + ): """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", - ) + cli_executor.exec_command(f"config set {constant.FAB_DEBUG_ENABLED} true") - 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): + def test_config_get_mode_warns_and_returns_runtime_mode_success( + self, mock_questionary_print, mock_print_warning, cli_executor: CLIExecutor + ): """'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") + cli_executor.exec_command("config get mode") - 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() + mock_print_warning.assert_any_call(DEPRECATION_WARNING) + mock_questionary_print.assert_called() - def test_config_get_non_mode_key__still_works(self): + def test_config_get_non_mode_key_still_works_success( + self, mock_questionary_print, cli_executor: CLIExecutor + ): """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) + cli_executor.exec_command(f"config get {constant.FAB_DEBUG_ENABLED}") diff --git a/tests/test_core/test_context_persistence.py b/tests/test_core/test_context_persistence.py index c242a21d0..c5225221f 100644 --- a/tests/test_core/test_context_persistence.py +++ b/tests/test_core/test_context_persistence.py @@ -31,9 +31,6 @@ def mock_get_config(key): 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 @@ -75,7 +72,6 @@ def mock_get_config(key): monkeypatch.setattr(fab_state_config, "get_config", mock_get_config) context = Context() - monkeypatch.setattr(context, "get_runtime_mode", lambda: fab_constant.FAB_MODE_COMMANDLINE) # Reset the context to force loading context._context = None @@ -139,7 +135,6 @@ def mock_get_config(key): 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 mock tenant and workspace tenant = Tenant("test_tenant", "1234") @@ -177,7 +172,6 @@ def mock_get_config(key): 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: @@ -360,7 +354,6 @@ def test_loading_context_re_entrancy_guard(monkeypatch): # 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_CONTEXT_PERSISTENCE_ENABLED: diff --git a/tests/test_core/test_fab_context.py b/tests/test_core/test_fab_context.py index 89bb345cd..196132492 100644 --- a/tests/test_core/test_fab_context.py +++ b/tests/test_core/test_fab_context.py @@ -443,36 +443,33 @@ def mock_get_command_context(): 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): + 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): + 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): + 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): + 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): + 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): + 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") diff --git a/tests/test_core/test_fab_state_config.py b/tests/test_core/test_fab_state_config.py index 835d99692..5c5973b2b 100644 --- a/tests/test_core/test_fab_state_config.py +++ b/tests/test_core/test_fab_state_config.py @@ -72,7 +72,7 @@ def test_list_configs(self, monkeypatch): class TestInitDefaults: """Test suite for config initialization optimization.""" - def test_init_defaults__no_write_when_unchanged(self, tmp_path, monkeypatch): + def test_init_defaults_no_write_when_unchanged(self, tmp_path, monkeypatch): """Test that init_defaults skips writing when config already has all defaults.""" import json @@ -101,86 +101,68 @@ def tracking_write(data): 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) +def _create_temp_config(monkeypatch, config_data): + """Create a temp config file with the given data and monkeypatch cfg.config_file to point to it.""" + tmpdir = tempfile.mkdtemp() + config_file = os.path.join(tmpdir, "config.json") + with open(config_file, "w") as f: + json.dump(config_data, f) + monkeypatch.setattr(cfg, "config_file", config_file) + return 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_removes_mode_key(monkeypatch): + """If an existing config file contains 'mode', init_defaults must delete it.""" + config_file = _create_temp_config(monkeypatch, { + fab_constant.FAB_MODE: fab_constant.FAB_MODE_INTERACTIVE, + fab_constant.FAB_CACHE_ENABLED: "true", + }) + cfg.init_defaults() -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) + result = cfg.read_config(config_file) + assert fab_constant.FAB_MODE not in result + assert result[fab_constant.FAB_CACHE_ENABLED] == "true" - monkeypatch.setattr(cfg, "config_file", config_file) - cfg.init_defaults() +def test_init_defaults_no_mode_key_succeeds(monkeypatch): + """Config without 'mode' must initialize cleanly (distinct from removes_mode_key: verifies no error on absence).""" + config_file = _create_temp_config(monkeypatch, { + fab_constant.FAB_DEBUG_ENABLED: "true", + }) - result = cfg.read_config(config_file) - assert fab_constant.FAB_MODE not in result - assert result[fab_constant.FAB_DEBUG_ENABLED] == "true" + 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) +def test_init_defaults_applies_missing_defaults(monkeypatch): + """init_defaults must fill in missing default values.""" + config_file = _create_temp_config(monkeypatch, {}) - cfg.init_defaults() + 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)}'" - ) + 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): +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) + config_file = _create_temp_config(monkeypatch, { + fab_constant.FAB_CACHE_ENABLED: "false", + }) - monkeypatch.setattr(cfg, "config_file", config_file) - - cfg.init_defaults() + cfg.init_defaults() - result = cfg.read_config(config_file) - assert result[fab_constant.FAB_CACHE_ENABLED] == "false" + result = cfg.read_config(config_file) + assert result[fab_constant.FAB_CACHE_ENABLED] == "false" # endregion