diff --git a/.changeset/0008-debug-observability.md b/.changeset/0008-debug-observability.md new file mode 100644 index 0000000..b44d49f --- /dev/null +++ b/.changeset/0008-debug-observability.md @@ -0,0 +1,5 @@ +--- +"pan-scm-cli": minor +--- + +Added a global `--debug` flag that enables DEBUG logging (including SDK OAuth/auth traffic, previously impossible to surface) and full tracebacks on unexpected errors. Logging is now configured once at CLI startup with a quiet WARNING default (precedence: `--debug` > `SCM_LOG_LEVEL` > context/settings `log_level` > WARNING); invalid log levels warn instead of crashing. Identity profile upserts no longer swallow update failures — real API errors now surface instead of a misleading create-path error. diff --git a/docs-site/docs/guide/configuration.md b/docs-site/docs/guide/configuration.md index 587ec40..7f06c28 100644 --- a/docs-site/docs/guide/configuration.md +++ b/docs-site/docs/guide/configuration.md @@ -313,7 +313,7 @@ The `settings.yaml` file contains default application settings: ```yaml --- default: - log_level: "INFO" + log_level: "WARNING" production: log_level: "WARNING" @@ -331,7 +331,7 @@ The following legacy configuration methods are no longer supported as of version | Setting | Default | Environment Variable | Description | | --- | --- | --- | --- | -| `log_level` | `"INFO"` | `SCM_LOG_LEVEL` | Controls SDK client logging verbosity | +| `log_level` | `"WARNING"` | `SCM_LOG_LEVEL` | Controls logging verbosity (CRITICAL, ERROR, WARNING, INFO, DEBUG) | | `client_id` | None | `SCM_CLIENT_ID` | SCM API authentication | | `client_secret` | None | `SCM_CLIENT_SECRET` | SCM API authentication | | `tsg_id` | None | `SCM_TSG_ID` | SCM API tenant identification | @@ -362,7 +362,33 @@ scm context test --mock scm context test staging --mock ``` -## Debugging Configuration +## Debugging + +### The --debug Flag + +Pass the global `--debug` flag to any command for full diagnostics: + +```bash +scm --debug show object address --folder Texas +``` + +`--debug` enables DEBUG-level logging (including the SDK's OAuth/auth loggers, +which are silenced at every other level) and prints full tracebacks on +unexpected errors instead of one-line messages. + +### Log Levels + +Logging is configured once at CLI startup with this precedence: + +1. `--debug` flag (forces DEBUG) +2. `SCM_LOG_LEVEL` environment variable +3. `log_level` in the active context or `settings.yaml` +4. Default: `WARNING` + +Valid levels: `CRITICAL`, `ERROR`, `WARNING`, `INFO`, `DEBUG`. An invalid value +produces a warning and falls back to `WARNING`. + +### Debugging Configuration To see which configuration values are being loaded: diff --git a/settings.yaml b/settings.yaml index 5c3b9a7..db512cd 100644 --- a/settings.yaml +++ b/settings.yaml @@ -1,7 +1,7 @@ --- default: # Logging configuration - log_level: "INFO" + log_level: "WARNING" production: # Production-specific settings diff --git a/src/scm_cli/commands/context.py b/src/scm_cli/commands/context.py index 757fd92..7e49aa8 100644 --- a/src/scm_cli/commands/context.py +++ b/src/scm_cli/commands/context.py @@ -105,7 +105,7 @@ def show_command( else: console.print(" Client Secret: [red]Not set[/red]") - console.print(f" Log Level: {config.get('log_level', 'INFO')}") + console.print(f" Log Level: {config.get('log_level', 'WARNING')}") console.print(f" API Base URL: {config.get('api_base_url') or '[dim]SDK default[/dim]'}") console.print(f" Token URL: {config.get('token_url') or '[dim]SDK default[/dim]'}") @@ -142,7 +142,7 @@ def create_command( help="Bearer access token for direct authentication (alternative to OAuth2)", ), log_level: str = typer.Option( - "INFO", + "WARNING", "--log-level", "-l", help="Logging level", @@ -382,7 +382,7 @@ def test_command( "client_id": config.get("client_id"), "client_secret": config.get("client_secret"), "tsg_id": config.get("tsg_id"), - "log_level": config.get("log_level", "INFO"), + "log_level": config.get("log_level", "WARNING"), } # Endpoint overrides — omit when unset so SDK defaults apply if config.get("api_base_url"): diff --git a/src/scm_cli/commands/insights.py b/src/scm_cli/commands/insights.py index ff03a1e..e102d98 100644 --- a/src/scm_cli/commands/insights.py +++ b/src/scm_cli/commands/insights.py @@ -13,8 +13,6 @@ import typer -from ..utils.config import settings -from ..utils.context import get_current_context from ..utils.decorators import handle_command_errors from ..utils.output import OutputFormat, emit, info, success from ..utils.sdk_client import scm_client @@ -24,20 +22,6 @@ # ============================================================================================================================================================================================= -def show_context_info() -> None: - """Display current context information if log level is INFO.""" - log_level = settings.get("log_level", "INFO").upper() - if log_level == "INFO": - current_context = get_current_context() - if current_context: - typer.echo(f"[INFO] Using authentication context: {current_context}", err=True) - else: - typer.echo( - "[INFO] No context set, using environment variables or default settings", - err=True, - ) - - def export_data(data: list[dict[str, Any]], export_format: str, output_file: str) -> None: """Export data to the specified format. @@ -155,8 +139,6 @@ def show_alerts( scm insights alerts --real-time """ - show_context_info() - # Note: scm_client automatically uses mock mode when no credentials are available if alert_id: @@ -257,8 +239,6 @@ def show_mobile_users( scm insights mobile-users --list --export json --output users.json """ - show_context_info() - # Note: scm_client automatically uses mock mode when no credentials are available if user_id: @@ -337,8 +317,6 @@ def show_locations( scm insights locations --list --export csv --output locations.csv """ - show_context_info() - # Note: scm_client automatically uses mock mode when no credentials are available if location_id: @@ -420,8 +398,6 @@ def show_remote_networks( scm insights remote-networks --list --export json --output networks.json """ - show_context_info() - # Note: scm_client automatically uses mock mode when no credentials are available if network_id: @@ -501,8 +477,6 @@ def show_service_connections( scm insights service-connections --list --metrics --export csv --output connections.csv """ - show_context_info() - # Note: scm_client automatically uses mock mode when no credentials are available if connection_id: @@ -599,8 +573,6 @@ def show_tunnels( scm insights tunnels --list --stats --export json --output tunnels.json """ - show_context_info() - # Note: scm_client automatically uses mock mode when no credentials are available if tunnel_id: diff --git a/src/scm_cli/commands/mobile_agent.py b/src/scm_cli/commands/mobile_agent.py index 810b37f..776eb73 100644 --- a/src/scm_cli/commands/mobile_agent.py +++ b/src/scm_cli/commands/mobile_agent.py @@ -12,8 +12,7 @@ from pydantic import ValidationError from ..utils import validate_location_params -from ..utils.config import load_from_yaml, settings -from ..utils.context import get_current_context +from ..utils.config import load_from_yaml from ..utils.decorators import handle_command_errors from ..utils.output import OUTPUT_OPTION, OutputFormat, emit, error, info, success from ..utils.sdk_client import scm_client @@ -35,20 +34,6 @@ # ============================================================================================================================================================================================= -def show_context_info() -> None: - """Display current context information if log level is INFO.""" - log_level = settings.get("log_level", "INFO").upper() - if log_level == "INFO": - current_context = get_current_context() - if current_context: - typer.echo(f"[INFO] Using authentication context: {current_context}", err=True) - else: - typer.echo( - "[INFO] No context set, using environment variables or default settings", - err=True, - ) - - def get_default_backup_filename(object_type: str, location_type: str, location_value: str) -> str: """Generate default backup filename based on object type and location.""" safe_location = location_value.lower().replace("/", "-").replace(" ", "-") @@ -177,8 +162,6 @@ def show_agent_version( scm show mobile-agent agent-version --folder "Mobile Users" --name "5.2.0" """ - show_context_info() - if name: # Get a specific agent version by name version = scm_client.get_agent_version(folder=folder, name=name) @@ -440,8 +423,6 @@ def show_auth_setting( scm show mobile-agent auth-setting --folder "Mobile Users" --name "saml-auth" """ - show_context_info() - if name: # Get a specific auth setting by name setting = scm_client.get_auth_setting(folder=folder, name=name) @@ -710,8 +691,6 @@ def show_forwarding_profile( scm show mobile-agent forwarding-profile --id "123e4567-e89b-12d3-a456-426655440000" """ - show_context_info() - if profile_id or name: profile = scm_client.get_forwarding_profile(folder=folder, name=name, profile_id=profile_id) emit(profile, output, title=f"Forwarding Profile: {profile.get('name', 'N/A')}") @@ -949,8 +928,6 @@ def show_forwarding_profile_destination( scm show mobile-agent forwarding-profile-destination --id "123e4567-e89b-12d3-a456-426655440000" """ - show_context_info() - if destination_id or name: destination = scm_client.get_forwarding_profile_destination(folder=folder, name=name, destination_id=destination_id) emit(destination, output, title=f"Forwarding Profile Destination: {destination.get('name', 'N/A')}") @@ -1228,8 +1205,6 @@ def show_agent_profile( scm show mobile-agent agent-profile --folder "Mobile Users" --name "corp-app-settings" """ - show_context_info() - if name: profile = scm_client.get_agent_profile(folder=folder, name=name) emit(profile, output, title=f"Agent Profile: {profile.get('name', 'N/A')}") @@ -1476,8 +1451,6 @@ def show_tunnel_profile( scm show mobile-agent tunnel-profile --folder "Mobile Users" --name "corp-tunnel" """ - show_context_info() - if name: profile = scm_client.get_tunnel_profile(folder=folder, name=name) emit(profile, output, title=f"Tunnel Profile: {profile.get('name', 'N/A')}") @@ -1592,8 +1565,6 @@ def show_infrastructure_setting( scm show mobile-agent infrastructure-setting --name "gp-infra" """ - show_context_info() - setting = scm_client.get_infrastructure_setting(folder=folder, name=name) emit(setting, output, title=f"Infrastructure Setting: {setting.get('name', 'N/A')}") return setting @@ -1747,8 +1718,6 @@ def show_global_setting( scm show mobile-agent global-setting """ - show_context_info() - setting = scm_client.get_global_settings() if not setting: @@ -2022,8 +1991,6 @@ def show_forwarding_profile_source_application( scm show mobile-agent forwarding-profile-source-application --folder "Mobile Users" --name "office-apps" """ - show_context_info() - if name: # Get a specific source application by name app = scm_client.get_forwarding_profile_source_application(folder=folder, name=name) @@ -2299,8 +2266,6 @@ def show_forwarding_profile_user_location( scm show mobile-agent forwarding-profile-user-location --folder "Mobile Users" --name "branch-network" """ - show_context_info() - if name: # Get a specific user location by name location = scm_client.get_forwarding_profile_user_location(folder=folder, name=name) @@ -2623,8 +2588,6 @@ def show_forwarding_profile_regional_and_custom_proxy( scm show mobile-agent forwarding-profile-regional-and-custom-proxy --folder "Mobile Users" --name "emea-proxy" """ - show_context_info() - if name: # Get a specific regional and custom proxy by name proxy = scm_client.get_forwarding_profile_regional_and_custom_proxy(folder=folder, name=name) diff --git a/src/scm_cli/commands/objects.py b/src/scm_cli/commands/objects.py index 934b302..3aa60bd 100644 --- a/src/scm_cli/commands/objects.py +++ b/src/scm_cli/commands/objects.py @@ -12,8 +12,6 @@ # Removed unused import: from the `..utils.config` import load_from_yaml from ..utils import parse_comma_separated_list, validate_location_params -from ..utils.config import settings -from ..utils.context import get_current_context from ..utils.decorators import handle_command_errors from ..utils.output import OUTPUT_OPTION, OutputFormat, emit, error, info, redact, success, warning from ..utils.sdk_client import scm_client @@ -44,20 +42,6 @@ # ============================================================================================================================================================================================= -def show_context_info() -> None: - """Display current context information if log level is INFO.""" - log_level = settings.get("log_level", "INFO").upper() - if log_level == "INFO": - current_context = get_current_context() - if current_context: - typer.echo(f"[INFO] Using authentication context: {current_context}", err=True) - else: - typer.echo( - "[INFO] No context set, using environment variables or default settings", - err=True, - ) - - # ============================================================================================================================================================================================= # TYPER APP CONFIGURATION # ============================================================================================================================================================================================= @@ -1021,8 +1005,6 @@ def show_address( """ # Show context info if log level is INFO - show_context_info() - if name: # Get a specific address by name address = scm_client.get_address(folder=folder, name=name) @@ -4037,8 +4019,6 @@ def delete_quarantined_device( scm delete object quarantined-device 01abcdef-2345-6789-abcd-ef0123456789 """ - show_context_info() - if not force: confirm = typer.confirm(f"Delete quarantined device '{host_id}'?") if not confirm: @@ -4214,8 +4194,6 @@ def set_quarantined_device( serial_number: str = typer.Option(None, "--serial-number", help="Serial number of the device"), ) -> None: """Create a quarantined device entry.""" - show_context_info() - # Build device data device_data: dict[str, Any] = { "host_id": host_id, @@ -4254,8 +4232,6 @@ def show_quarantined_device( scm show object quarantined-device --host-id abc123 """ - show_context_info() - devices = scm_client.list_quarantined_devices( host_id=host_id, serial_number=serial_number, diff --git a/src/scm_cli/main.py b/src/scm_cli/main.py index 8dcd466..bcccf7c 100644 --- a/src/scm_cli/main.py +++ b/src/scm_cli/main.py @@ -294,12 +294,43 @@ _region_override: str | None = None +_VALID_LOG_LEVELS = ("CRITICAL", "ERROR", "WARNING", "INFO", "DEBUG") + def get_region_override() -> str | None: """Get the global --region override value, if set.""" return _region_override +def _configure_logging(debug: bool) -> None: + """Configure logging once at CLI entry. + + Precedence: --debug > SCM_LOG_LEVEL env > `log_level` setting > WARNING. + The SDK auth loggers stay pinned to CRITICAL unless the effective level + is DEBUG, so OAuth/token traffic is only visible when debugging. + """ + import logging + import os + + if debug: + level = logging.DEBUG + else: + from .utils.config import settings + from .utils.output import warning + + level_name = (os.environ.get("SCM_LOG_LEVEL") or settings.get("log_level", "WARNING") or "WARNING").upper() + if level_name not in _VALID_LOG_LEVELS: + warning(f"Invalid SCM_LOG_LEVEL '{level_name}' (valid: {', '.join(_VALID_LOG_LEVELS)}); using WARNING") + level_name = "WARNING" + level = getattr(logging, level_name) + + logging.basicConfig(level=level, force=True) + + auth_logger_level = logging.NOTSET if level == logging.DEBUG else logging.CRITICAL + logging.getLogger("scm.auth").setLevel(auth_logger_level) + logging.getLogger("oauthlib").setLevel(auth_logger_level) + + def _version_callback(value: bool) -> None: if value: from scm_cli import __version__ @@ -315,6 +346,11 @@ def callback( "--region", help="Override SCM API region for this invocation", ), + debug: bool = typer.Option( + False, + "--debug", + help="Enable debug logging (including SDK auth/HTTP) and full tracebacks", + ), version: bool | None = typer.Option( None, "--version", @@ -341,6 +377,11 @@ def callback( global _region_override # noqa: PLW0603 _region_override = region + from .utils.decorators import set_debug + + set_debug(debug) + _configure_logging(debug) + # ============================================================================================================================================================================================= # MAIN ENTRY POINT diff --git a/src/scm_cli/utils/decorators.py b/src/scm_cli/utils/decorators.py index 258bd0e..ecada39 100644 --- a/src/scm_cli/utils/decorators.py +++ b/src/scm_cli/utils/decorators.py @@ -1,15 +1,30 @@ """Shared decorators for scm-cli command modules.""" import functools +import traceback import typer +_debug_enabled = False + + +def set_debug(enabled: bool) -> None: + """Record whether --debug was passed (set once by the main callback).""" + global _debug_enabled # noqa: PLW0603 + _debug_enabled = enabled + + +def is_debug() -> bool: + """Return True when --debug is active.""" + return _debug_enabled + def handle_command_errors(operation: str): """Wrap a command function with standardized error handling. Catches all exceptions (except typer.Exit and typer.Abort) and prints - a formatted error message to stderr before exiting with code 1. + a formatted error message to stderr before exiting with code 1. With + --debug active, the full traceback is printed first. Args: ---- @@ -26,6 +41,8 @@ def wrapper(*args, **kwargs): except (typer.Exit, typer.Abort, SystemExit): raise except Exception as e: + if is_debug(): + typer.echo(traceback.format_exc(), err=True) typer.echo(f"Error {operation}: {str(e)}", err=True) raise typer.Exit(code=1) from e diff --git a/src/scm_cli/utils/sdk_client.py b/src/scm_cli/utils/sdk_client.py index 3731aee..84f43e1 100644 --- a/src/scm_cli/utils/sdk_client.py +++ b/src/scm_cli/utils/sdk_client.py @@ -18,7 +18,7 @@ from oauthlib.oauth2.rfc6749.errors import InvalidClientError from pydantic import ValidationError from scm.client import Scm -from scm.exceptions import APIError, AuthenticationError, ClientError, GatewayTimeoutError, NotFoundError +from scm.exceptions import APIError, AuthenticationError, ClientError, GatewayTimeoutError, NotFoundError, ObjectNotPresentError from .config import get_credentials, settings from .context import get_current_context @@ -72,15 +72,10 @@ class SCMClient: """ def __init__(self): - """Initialize the SCM client with logger and credentials.""" - # Configure logging based on settings - logging_level = getattr(logging, settings.get("log_level", "INFO")) - logging.basicConfig(level=logging_level, force=True) - - # Suppress SDK auth logging for cleaner output - logging.getLogger("scm.auth").setLevel(logging.CRITICAL) - logging.getLogger("oauthlib").setLevel(logging.CRITICAL) + """Initialize the SCM client with logger and credentials. + Logging is configured once at CLI entry (main callback), not here. + """ self.logger = logger self.logger.info("Initializing SCM client") self.client = None @@ -112,8 +107,8 @@ def __init__(self): try: ctx_config = get_context_config(current_context) access_token = ctx_config.get("access_token") - except Exception: - pass + except Exception as e: + self.logger.warning(f"Could not read context config for '{current_context}': {e}") if access_token: # Bearer token authentication mode @@ -16228,8 +16223,8 @@ def create_authentication_profile(self, name: str, folder: str | None = None, sn result = json.loads(updated.model_dump_json(exclude_unset=True)) result["__action__"] = "updated" return result - except Exception: - pass + except (NotFoundError, ObjectNotPresentError): + pass # object does not exist yet; fall through to create create_data = {"name": name, **container_kwargs, **{k: v for k, v in kwargs.items() if v is not None}} created = self.client.authentication_profile.create(create_data) @@ -16341,8 +16336,8 @@ def create_kerberos_server_profile(self, name: str, folder: str | None = None, s result = json.loads(updated.model_dump_json(exclude_unset=True)) result["__action__"] = "updated" return result - except Exception: - pass + except (NotFoundError, ObjectNotPresentError): + pass # object does not exist yet; fall through to create create_data = {"name": name, **container_kwargs, **{k: v for k, v in kwargs.items() if v is not None}} created = self.client.kerberos_server_profile.create(create_data) @@ -16453,8 +16448,8 @@ def create_ldap_server_profile(self, name: str, folder: str | None = None, snipp result = json.loads(updated.model_dump_json(exclude_unset=True)) result["__action__"] = "updated" return result - except Exception: - pass + except (NotFoundError, ObjectNotPresentError): + pass # object does not exist yet; fall through to create create_data = {"name": name, **container_kwargs, **{k: v for k, v in kwargs.items() if v is not None}} created = self.client.ldap_server_profile.create(create_data) @@ -16578,8 +16573,8 @@ def create_radius_server_profile(self, name: str, folder: str | None = None, sni result = json.loads(updated.model_dump_json(exclude_unset=True)) result["__action__"] = "updated" return result - except Exception: - pass + except (NotFoundError, ObjectNotPresentError): + pass # object does not exist yet; fall through to create create_data = {"name": name, **container_kwargs, **{k: v for k, v in kwargs.items() if v is not None}} created = self.client.radius_server_profile.create(create_data) @@ -16698,8 +16693,8 @@ def create_saml_server_profile(self, name: str, folder: str | None = None, snipp result = json.loads(updated.model_dump_json(exclude_unset=True)) result["__action__"] = "updated" return result - except Exception: - pass + except (NotFoundError, ObjectNotPresentError): + pass # object does not exist yet; fall through to create create_data = {"name": name, **container_kwargs, **{k: v for k, v in kwargs.items() if v is not None}} created = self.client.saml_server_profile.create(create_data) @@ -16826,8 +16821,8 @@ def create_tacacs_server_profile(self, name: str, folder: str | None = None, sni result = json.loads(updated.model_dump_json(exclude_unset=True)) result["__action__"] = "updated" return result - except Exception: - pass + except (NotFoundError, ObjectNotPresentError): + pass # object does not exist yet; fall through to create create_data = {"name": name, **container_kwargs, **{k: v for k, v in kwargs.items() if v is not None}} created = self.client.tacacs_server_profile.create(create_data) diff --git a/tests/test_debug_and_logging.py b/tests/test_debug_and_logging.py new file mode 100644 index 0000000..e81e3b8 --- /dev/null +++ b/tests/test_debug_and_logging.py @@ -0,0 +1,129 @@ +"""Tests for the --debug flag, centralized logging config, and error propagation. + +Logging is configured once in the main callback (default WARNING). --debug +turns on DEBUG logging (including the SDK auth loggers) and full tracebacks +on unexpected errors. Identity upserts must not swallow update failures. +""" + +import logging +from types import SimpleNamespace + +import pytest +from scm.exceptions import APIError, NotFoundError + +from src.scm_cli.main import app + + +@pytest.fixture(autouse=True) +def _mock_mode(monkeypatch): + monkeypatch.setenv("SCM_MOCK", "1") + + +class TestDebugFlag: + def test_debug_sets_root_logger_to_debug(self, runner): + result = runner.invoke(app, ["--debug", "show", "object", "address", "--folder", "Texas"]) + + assert result.exit_code == 0 + assert logging.getLogger().level == logging.DEBUG + + def test_debug_unpins_sdk_auth_loggers(self, runner): + result = runner.invoke(app, ["--debug", "show", "object", "address", "--folder", "Texas"]) + + assert result.exit_code == 0 + assert logging.getLogger("scm.auth").level != logging.CRITICAL + assert logging.getLogger("oauthlib").level != logging.CRITICAL + + def test_default_pins_sdk_auth_loggers(self, runner, monkeypatch): + monkeypatch.setenv("SCM_LOG_LEVEL", "WARNING") + result = runner.invoke(app, ["show", "object", "address", "--folder", "Texas"]) + + assert result.exit_code == 0 + assert logging.getLogger("scm.auth").level == logging.CRITICAL + + def test_debug_shows_traceback_on_error(self, runner, monkeypatch): + from src.scm_cli.utils.sdk_client import scm_client + + def boom(**kwargs): + raise RuntimeError("kaboom") + + monkeypatch.setattr(scm_client, "list_addresses", boom) + result = runner.invoke(app, ["--debug", "show", "object", "address", "--folder", "Texas"]) + + assert result.exit_code == 1 + assert "kaboom" in result.output + assert "Traceback" in result.output or "RuntimeError" in result.output + + def test_no_debug_hides_traceback(self, runner, monkeypatch): + from src.scm_cli.utils.sdk_client import scm_client + + def boom(**kwargs): + raise RuntimeError("kaboom") + + monkeypatch.setattr(scm_client, "list_addresses", boom) + result = runner.invoke(app, ["show", "object", "address", "--folder", "Texas"]) + + assert result.exit_code == 1 + assert "kaboom" in result.output + assert "Traceback" not in result.output + + +class TestLogLevelValidation: + def test_invalid_scm_log_level_warns_but_works(self, runner, monkeypatch): + monkeypatch.setenv("SCM_LOG_LEVEL", "VERBOSE") + result = runner.invoke(app, ["show", "object", "address", "--folder", "Texas"]) + + assert result.exit_code == 0 + assert "VERBOSE" in result.output # warning names the bad value + + def test_valid_scm_log_level_applies(self, runner, monkeypatch): + monkeypatch.setenv("SCM_LOG_LEVEL", "ERROR") + result = runner.invoke(app, ["show", "object", "address", "--folder", "Texas"]) + + assert result.exit_code == 0 + assert logging.getLogger().level == logging.ERROR + + +class TestUpsertErrorPropagation: + """Update failures in identity upserts must propagate, not fall through to create.""" + + def _client_with_fake_sdk(self, monkeypatch, fake_profile): + monkeypatch.delenv("SCM_MOCK", raising=False) + import scm_cli.utils.sdk_client as sdk_module + + client = object.__new__(sdk_module.SCMClient) # skip __init__ + client.logger = logging.getLogger("test") + client.client = SimpleNamespace(authentication_profile=fake_profile) + return client + + def test_update_failure_propagates(self, monkeypatch): + calls = {"create": 0} + + class FakeProfile: + def fetch(self, **kwargs): + return SimpleNamespace(id="p1") + + def update(self, existing): + raise APIError("update failed") + + def create(self, data): + calls["create"] += 1 + raise AssertionError("create must not run after a failed update") + + client = self._client_with_fake_sdk(monkeypatch, FakeProfile()) + + with pytest.raises(APIError): + client.create_authentication_profile(name="p1", folder="Texas") + assert calls["create"] == 0 + + def test_not_found_falls_through_to_create(self, monkeypatch): + class FakeProfile: + def fetch(self, **kwargs): + raise NotFoundError("nope") + + def create(self, data): + return SimpleNamespace(model_dump_json=lambda **kw: '{"id": "p1", "name": "p1"}') + + client = self._client_with_fake_sdk(monkeypatch, FakeProfile()) + + result = client.create_authentication_profile(name="p1", folder="Texas") + assert result["__action__"] == "created" diff --git a/tests/test_incidents_commands.py b/tests/test_incidents_commands.py index e6383d0..5557678 100644 --- a/tests/test_incidents_commands.py +++ b/tests/test_incidents_commands.py @@ -74,10 +74,12 @@ def test_list_incidents_filter_status(self, runner, mock_incidents_env): assert result.exit_code == 0 assert "medium" in result.output - def test_list_incidents_json(self, runner, mock_incidents_env): - result = runner.invoke(app, ["incidents", "list", "--json"]) + def test_list_incidents_json(self, mock_incidents_env): + from typer.testing import CliRunner + + result = CliRunner(mix_stderr=False).invoke(app, ["incidents", "list", "--json"]) assert result.exit_code == 0 - data = json.loads(result.output) + data = json.loads(result.stdout) assert isinstance(data, list) assert len(data) >= 2 @@ -101,10 +103,12 @@ def test_show_incident_detail(self, runner, mock_incidents_env): assert "Alerts" in result.output assert "Remediation" in result.output or "remediations" in result.output.lower() - def test_show_incident_json(self, runner, mock_incidents_env): - result = runner.invoke(app, ["incidents", "show", "INC-2026-04-001", "--json"]) + def test_show_incident_json(self, mock_incidents_env): + from typer.testing import CliRunner + + result = CliRunner(mix_stderr=False).invoke(app, ["incidents", "show", "INC-2026-04-001", "--json"]) assert result.exit_code == 0 - data = json.loads(result.output) + data = json.loads(result.stdout) assert data["incident_id"] == "INC-2026-04-001" assert "alerts" in data assert "remediations" in data