mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-24 00:52:24 +00:00
test(mcp): cover stable server ordering and sort priorities
This commit is contained in:
Joshua Valluru
parent
f5d511b3f9
commit
68074da1d1
2 changed files with 112 additions and 43 deletions
|
|
@ -6,7 +6,7 @@ import logging
|
|||
from contextlib import ExitStack
|
||||
from datetime import datetime, timedelta
|
||||
from types import SimpleNamespace
|
||||
from typing import List, Optional
|
||||
from typing import Final, List, Optional
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
|
@ -1539,54 +1539,57 @@ class TestTeamScopedMCPServerAccess:
|
|||
|
||||
|
||||
class TestFetchAllMCPServersOrdering:
|
||||
def test_display_order_is_case_insensitive_name_then_id(self):
|
||||
servers = [
|
||||
generate_mock_mcp_server_db_record(server_id="s-2", alias="github"),
|
||||
generate_mock_mcp_server_db_record(server_id="s-1", alias="github"),
|
||||
generate_mock_mcp_server_db_record(server_id="s-0", alias="Slack"),
|
||||
generate_mock_mcp_server_db_record(server_id="s-3", alias="confluence"),
|
||||
]
|
||||
def test_display_order_is_case_insensitive_name_then_id(self) -> None:
|
||||
servers: Final = (
|
||||
LiteLLM_MCPServerTable(server_id="s-2", server_name="GitHub", alias="aaa", transport=MCPTransport.http),
|
||||
LiteLLM_MCPServerTable(server_id="s-1", alias="github", transport=MCPTransport.http),
|
||||
LiteLLM_MCPServerTable(server_id="s-0", server_name="Slack", alias="zzz", transport=MCPTransport.http),
|
||||
LiteLLM_MCPServerTable(server_id="confluence", server_name="", alias="", transport=MCPTransport.http),
|
||||
)
|
||||
|
||||
ordered = sorted(servers, key=mgmt_endpoints._mcp_server_display_order)
|
||||
assert [s.server_id for s in ordered] == ["s-3", "s-1", "s-2", "s-0"]
|
||||
ordered: Final = sorted(servers, key=mgmt_endpoints._mcp_server_display_order)
|
||||
assert [s.server_id for s in ordered] == ["confluence", "s-1", "s-2", "s-0"]
|
||||
|
||||
@pytest.mark.parametrize("team_id", [None, "team-1"])
|
||||
@pytest.mark.parametrize("reverse", [False, True])
|
||||
@pytest.mark.asyncio
|
||||
async def test_list_is_sorted_by_display_name_regardless_of_resolution_order(self):
|
||||
"""The registry resolves ids through a set, so the response must impose its own order."""
|
||||
mock_user_auth = generate_mock_user_api_key_auth(
|
||||
async def test_list_is_sorted_by_display_name_regardless_of_resolution_order(
|
||||
self, team_id: str | None, reverse: bool
|
||||
) -> None:
|
||||
mock_user_auth: Final = generate_mock_user_api_key_auth(
|
||||
user_role=LitellmUserRoles.PROXY_ADMIN,
|
||||
user_id="admin_user",
|
||||
)
|
||||
first_order = [
|
||||
servers: Final = (
|
||||
generate_mock_mcp_server_db_record(server_id="s-zeta", alias="zeta"),
|
||||
generate_mock_mcp_server_db_record(server_id="s-alpha", alias="Alpha"),
|
||||
generate_mock_mcp_server_db_record(server_id="s-mid", alias="mid"),
|
||||
]
|
||||
second_order = list(reversed(first_order))
|
||||
|
||||
for resolved in (first_order, second_order):
|
||||
mock_manager = MagicMock()
|
||||
mock_manager.get_all_allowed_mcp_servers = AsyncMock(return_value=resolved)
|
||||
with (
|
||||
patch( # test-quality-ok: the route reads a module-global manager with no injection seam
|
||||
"litellm.proxy.management_endpoints.mcp_management_endpoints.global_mcp_server_manager",
|
||||
mock_manager,
|
||||
),
|
||||
patch( # test-quality-ok: admin view is derived from module-global proxy settings
|
||||
"litellm.proxy.management_endpoints.mcp_management_endpoints._user_has_admin_view",
|
||||
return_value=True,
|
||||
),
|
||||
patch( # test-quality-ok: auth contexts need a live prisma client
|
||||
"litellm.proxy.management_endpoints.mcp_management_endpoints.build_effective_auth_contexts",
|
||||
AsyncMock(return_value=[mock_user_auth]),
|
||||
),
|
||||
):
|
||||
from litellm.proxy.management_endpoints.mcp_management_endpoints import (
|
||||
fetch_all_mcp_servers,
|
||||
)
|
||||
|
||||
result = await fetch_all_mcp_servers(user_api_key_dict=mock_user_auth)
|
||||
assert [s.server_id for s in result] == ["s-alpha", "s-mid", "s-zeta"]
|
||||
)
|
||||
resolved: Final = list(reversed(servers) if reverse else servers)
|
||||
mock_manager: Final = MagicMock()
|
||||
mock_manager.get_all_allowed_mcp_servers = AsyncMock(return_value=resolved)
|
||||
with (
|
||||
patch( # test-quality-ok: the route reads a module-global manager with no injection seam
|
||||
"litellm.proxy.management_endpoints.mcp_management_endpoints.global_mcp_server_manager",
|
||||
mock_manager,
|
||||
),
|
||||
patch( # test-quality-ok: admin view is derived from module-global proxy settings
|
||||
"litellm.proxy.management_endpoints.mcp_management_endpoints._user_has_admin_view",
|
||||
return_value=True,
|
||||
),
|
||||
patch( # test-quality-ok: auth contexts need a live prisma client
|
||||
"litellm.proxy.management_endpoints.mcp_management_endpoints.build_effective_auth_contexts",
|
||||
AsyncMock(return_value=[mock_user_auth]),
|
||||
),
|
||||
patch( # test-quality-ok: isolate the route's ordering from team database resolution
|
||||
"litellm.proxy.management_endpoints.mcp_management_endpoints._get_team_scoped_mcp_server_list",
|
||||
AsyncMock(return_value=resolved),
|
||||
),
|
||||
):
|
||||
result: Final = await mgmt_endpoints.fetch_all_mcp_servers(
|
||||
user_api_key_dict=mock_user_auth, team_id=team_id
|
||||
)
|
||||
assert [s.server_id for s in result] == ["s-alpha", "s-mid", "s-zeta"]
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_restricted_virtual_key_cannot_use_team_id_filter(self):
|
||||
|
|
|
|||
|
|
@ -3,7 +3,7 @@ import { render, waitFor, screen, act, within } from "@testing-library/react";
|
|||
import userEvent from "@testing-library/user-event";
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
|
||||
import MCPServers, { compareServers } from "./mcp_servers";
|
||||
import MCPServers, { compareServers, type SortKey } from "./mcp_servers";
|
||||
import type { MCPServer } from "@/components/mcp_tools/types";
|
||||
import * as networking from "@/components/networking";
|
||||
|
||||
|
|
@ -33,8 +33,14 @@ const createQueryClient = () =>
|
|||
});
|
||||
|
||||
describe("compareServers", () => {
|
||||
const server = (server_id: string, name: string, created_at = ""): MCPServer =>
|
||||
({ server_id, server_name: name, created_at, updated_at: created_at }) as MCPServer;
|
||||
const server = (server_id: string, name: string, created_at = ""): MCPServer => ({
|
||||
server_id,
|
||||
server_name: name,
|
||||
created_at,
|
||||
updated_at: created_at,
|
||||
created_by: "user",
|
||||
updated_by: "user",
|
||||
});
|
||||
|
||||
const shuffled = [server("c", "github"), server("a", "slack"), server("b", "Jira")];
|
||||
|
||||
|
|
@ -56,6 +62,66 @@ describe("compareServers", () => {
|
|||
"old",
|
||||
]);
|
||||
});
|
||||
|
||||
it.each<SortKey>(["created_desc", "updated_desc", "name_asc", "health"])(
|
||||
"breaks equal timestamps and names by ID for %s regardless of input order",
|
||||
(sort) => {
|
||||
const servers = [
|
||||
server("b", "GitHub", "2026-01-01T00:00:00Z"),
|
||||
server("c", "Slack", "2026-01-01T00:00:00Z"),
|
||||
server("a", "github", "2026-01-01T00:00:00Z"),
|
||||
];
|
||||
for (const input of [servers, [...servers].reverse()]) {
|
||||
expect([...input].sort((a, b) => compareServers(a, b, sort)).map((s) => s.server_id)).toEqual(["a", "b", "c"]);
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it("uses the display name before alias, then falls back to alias and ID", () => {
|
||||
const servers: MCPServer[] = [
|
||||
{ ...server("s-slack", "Slack"), alias: "aaa" },
|
||||
{ ...server("s-github", ""), server_name: null, alias: "GitHub" },
|
||||
{ ...server("confluence", ""), alias: "" },
|
||||
];
|
||||
for (const input of [servers, [...servers].reverse()]) {
|
||||
expect([...input].sort((a, b) => compareServers(a, b, "name_asc")).map((s) => s.server_id)).toEqual([
|
||||
"confluence",
|
||||
"s-github",
|
||||
"s-slack",
|
||||
]);
|
||||
}
|
||||
});
|
||||
|
||||
it.each<SortKey>(["created_desc", "updated_desc", "health"])(
|
||||
"keeps timestamped servers before missing timestamps for %s",
|
||||
(sort) => {
|
||||
const servers = [
|
||||
server("config", "aaa"),
|
||||
server("older", "bbb", "2026-01-01T00:00:00Z"),
|
||||
server("newer", "zzz", "2026-02-01T00:00:00Z"),
|
||||
];
|
||||
for (const input of [servers, [...servers].reverse()]) {
|
||||
expect([...input].sort((a, b) => compareServers(a, b, sort)).map((s) => s.server_id)).toEqual([
|
||||
"newer",
|
||||
"older",
|
||||
"config",
|
||||
]);
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it("sorts health before recency and display name", () => {
|
||||
const servers: MCPServer[] = [
|
||||
{ ...server("healthy", "aaa", "2026-03-01T00:00:00Z"), status: "healthy" },
|
||||
{ ...server("unknown", "bbb", "2026-02-01T00:00:00Z"), status: "unknown" },
|
||||
{ ...server("unhealthy", "zzz", "2026-01-01T00:00:00Z"), status: "unhealthy" },
|
||||
];
|
||||
expect(servers.sort((a, b) => compareServers(a, b, "health")).map((s) => s.server_id)).toEqual([
|
||||
"unhealthy",
|
||||
"unknown",
|
||||
"healthy",
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("MCPServers", () => {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue