diff --git a/tests/e2e/coverage_registry/mgmt.yaml b/tests/e2e/coverage_registry/mgmt.yaml index e1a840b1239..39a1b462dd1 100644 --- a/tests/e2e/coverage_registry/mgmt.yaml +++ b/tests/e2e/coverage_registry/mgmt.yaml @@ -70,6 +70,7 @@ - {id: mgmt.budget.list_v1.happy_path, module: mgmt, tier: P1, surface: api, assertions: [happy_path], source: "management_v1/budgets.py:129", rationale: "Budget enumeration the Budgets page can page, sort and filter"} - {id: mgmt.budget.list_v1.admin_only, module: mgmt, tier: P1, surface: api, assertions: [admin_only], source: "management_v1/budgets.py:129", rationale: "A caller without admin view is refused, not served an empty page"} - {id: mgmt.callback.list.happy_path, module: mgmt, tier: P2, surface: api, assertions: [happy_path], source: "callback_management_endpoints.py", rationale: "Callback config (smoke)"} +- {id: mgmt.callback.delete.removes_configured_callback, module: mgmt, tier: P2, surface: api, assertions: [removes_configured_callback], source: "proxy_server.py delete_callback", fail_before_fix: proven, rationale: "A callback listed by GET /get/config/callbacks from success_callback, failure_callback or callbacks must be removable via POST /config/callback/delete; the delete only searched success_callback, so the other two returned 404 (LIT-8444)"} - {id: mgmt.cost_tracking.estimate.happy_path, module: mgmt, tier: P2, surface: api, assertions: [happy_path], source: "cost_tracking_settings.py", rationale: "Cost estimate (smoke)"} - {id: mgmt.router_settings.update.happy_path, module: mgmt, tier: P2, surface: api, assertions: [happy_path], source: "router_settings_endpoints.py", rationale: "Router config (smoke)"} - {id: mgmt.jwt_key_mapping.new.happy_path, module: mgmt, tier: P2, surface: api, assertions: [happy_path], source: "jwt_key_mapping_endpoints.py", rationale: "JWT->key mapping (smoke)"} diff --git a/tests/e2e/logging/logging_client.py b/tests/e2e/logging/logging_client.py index 9ec62a175c2..63f48def8f7 100644 --- a/tests/e2e/logging/logging_client.py +++ b/tests/e2e/logging/logging_client.py @@ -495,11 +495,7 @@ class LoggingClient: def delete_model(self, model_id: str) -> None: self.proxy.delete_model(model_id) - # ---- global callback configuration (Admin UI Logging & Alerts surface) ---- - def set_litellm_callbacks(self, settings: LitellmCallbackSettings) -> ConfigUpdateResponse: - """POST /config/update, the route the Admin UI's Logging page uses to add - a callback.""" return unwrap( self.proxy.transport.post( "/config/update", @@ -510,10 +506,6 @@ class LoggingClient: ) def clear_litellm_callbacks(self, *keys: Literal["callbacks", "failure_callback"]) -> None: - """Empty the given litellm_settings callback lists so teardown leaves the - stored row as it was. success_callback is absent on purpose: /config/update - unions that key instead of replacing it, so entries there can only leave - through /config/callback/delete.""" _ = self.set_litellm_callbacks( LitellmCallbackSettings( callbacks=[] if "callbacks" in keys else None, @@ -522,7 +514,6 @@ class LoggingClient: ) def config_callback_names(self) -> set[str]: - """Names GET /get/config/callbacks lists as configured (runtime-only read_only rows excluded).""" return { entry.name for entry in unwrap( @@ -537,8 +528,6 @@ class LoggingClient: } def delete_config_callback(self, callback_name: str) -> Result[CallbackDeleteResponse]: - """POST /config/callback/delete. Returns the raw Result so the caller can - unwrap for the success contract or ignore it for best-effort cleanup.""" return self.proxy.transport.post( "/config/callback/delete", headers=self.proxy.transport.master, diff --git a/tests/e2e/logging/test_callback_delete_e2e.py b/tests/e2e/logging/test_callback_delete_e2e.py index b704a6ce807..9462edd43f2 100644 --- a/tests/e2e/logging/test_callback_delete_e2e.py +++ b/tests/e2e/logging/test_callback_delete_e2e.py @@ -13,61 +13,64 @@ pytestmark = pytest.mark.e2e CALLBACK_NAME = "langsmith" -def _assert_listed_then_deleted( - client: LoggingClient, - resources: ResourceManager, - settings: LitellmCallbackSettings, - cleanup: Callable[[], object], -) -> None: - response = client.set_litellm_callbacks(settings) - assert "success" in response.message.lower(), ( - f"/config/update reported {response.message!r}, expected a success message" - ) - resources.defer(cleanup) - - names = client.config_callback_names() - assert CALLBACK_NAME in names, ( - f"GET /get/config/callbacks does not list {CALLBACK_NAME!r} after it was " - f"configured via /config/update; listed {sorted(names)}" - ) - - deleted = unwrap(client.delete_config_callback(CALLBACK_NAME)) - assert deleted.removed_callback == CALLBACK_NAME, ( - f"/config/callback/delete removed {deleted.removed_callback!r}, expected {CALLBACK_NAME!r}" - ) - - names_after = client.config_callback_names() - assert CALLBACK_NAME not in names_after, ( - f"GET /get/config/callbacks still lists {CALLBACK_NAME!r} after a successful " - f"delete; listed {sorted(names_after)}" - ) - - class TestCallbackDelete: + def _assert_listed_then_deleted( + self, + client: LoggingClient, + resources: ResourceManager, + settings: LitellmCallbackSettings, + cleanup: Callable[[], object], + ) -> None: + response = client.set_litellm_callbacks(settings) + assert "success" in response.message.lower(), ( + f"/config/update reported {response.message!r}, expected a success message" + ) + resources.defer(cleanup) + + names = client.config_callback_names() + assert CALLBACK_NAME in names, ( + f"GET /get/config/callbacks does not list {CALLBACK_NAME!r} after it was " + f"configured via /config/update; listed {sorted(names)}" + ) + + deleted = unwrap(client.delete_config_callback(CALLBACK_NAME)) + assert deleted.removed_callback == CALLBACK_NAME, ( + f"/config/callback/delete removed {deleted.removed_callback!r}, expected {CALLBACK_NAME!r}" + ) + + names_after = client.config_callback_names() + assert CALLBACK_NAME not in names_after, ( + f"GET /get/config/callbacks still lists {CALLBACK_NAME!r} after a successful " + f"delete; listed {sorted(names_after)}" + ) + + @pytest.mark.covers("mgmt.callback.delete.removes_configured_callback", exercised_on=[]) def test_callback_under_callbacks_key_is_listed_and_deletable( self, client: LoggingClient, resources: ResourceManager ) -> None: - _assert_listed_then_deleted( + self._assert_listed_then_deleted( client, resources, LitellmCallbackSettings(callbacks=[CALLBACK_NAME]), lambda: client.clear_litellm_callbacks("callbacks"), ) + @pytest.mark.covers("mgmt.callback.delete.removes_configured_callback", exercised_on=[]) def test_callback_under_failure_callback_key_is_listed_and_deletable( self, client: LoggingClient, resources: ResourceManager ) -> None: - _assert_listed_then_deleted( + self._assert_listed_then_deleted( client, resources, LitellmCallbackSettings(failure_callback=[CALLBACK_NAME]), lambda: client.clear_litellm_callbacks("failure_callback"), ) + @pytest.mark.covers("mgmt.callback.delete.removes_configured_callback", exercised_on=[]) def test_callback_under_success_callback_key_is_deletable( self, client: LoggingClient, resources: ResourceManager ) -> None: - _assert_listed_then_deleted( + self._assert_listed_then_deleted( client, resources, LitellmCallbackSettings(success_callback=[CALLBACK_NAME]), diff --git a/tests/test_litellm/proxy/proxy_server/test_routes_config.py b/tests/test_litellm/proxy/proxy_server/test_routes_config.py index 86b07440780..bd143de99ce 100644 --- a/tests/test_litellm/proxy/proxy_server/test_routes_config.py +++ b/tests/test_litellm/proxy/proxy_server/test_routes_config.py @@ -14,10 +14,14 @@ Routes covered: from __future__ import annotations import asyncio +import contextlib import json +from collections.abc import Callable from unittest.mock import AsyncMock, MagicMock +import httpx import pytest +from fastapi.testclient import TestClient from .conftest import VOLATILE_KEYS, normalize @@ -944,7 +948,14 @@ def test_config_callback_delete_not_found(client, auth_as, mock_prisma, monkeypa assert "langfuse" in str(response.json()).lower() or "not found" in str(response.json()).lower() -def _delete_callback_roundtrip(client, auth_as, mock_prisma, monkeypatch, litellm_settings, callback_name): +def _delete_callback_roundtrip( + client: TestClient, + auth_as: Callable[..., contextlib.AbstractContextManager[None]], + mock_prisma: MagicMock, + monkeypatch: pytest.MonkeyPatch, + litellm_settings: dict[str, object], + callback_name: str, +) -> tuple[httpx.Response, dict[str, object]]: """POST /config/callback/delete as admin against a stubbed config, and return (response, saved litellm_settings) so each case pins the full body plus the exact config persisted."""