mirror of
https://github.com/BerriAI/litellm.git
synced 2026-11-04 07:27:51 +00:00
test(e2e-ui): delete the MCP servers these specs create
MCP servers outlive the test that made them, the MCP page contacts every server it lists, and most of the ones these specs create point at an unreachable host. They accumulate, and each one makes navigateToPage's networkidle wait a little slower to settle. Measured on a local stack: with eleven leaked servers the whole MCP suite failed on a 30s navigation timeout, including specs that leaked nothing. Deleting the leftovers made all five pass. With per-test cleanup added, a run from a clean slate leaves zero behind and takes 20s instead of 1m24s. mcpServers.spec.ts carried a note that no teardown was needed because the runner brings up a fresh database each time. That holds for CI and is why this went unnoticed; it does not hold for a local stack that is reused.
This commit is contained in:
parent
4662d32894
commit
ffc7e6386b
4 changed files with 57 additions and 4 deletions
|
|
@ -1,6 +1,7 @@
|
|||
import { expect, Page as PwPage } from "@playwright/test";
|
||||
import { navigateToPage } from "./navigation";
|
||||
import { Page } from "../fixtures/pages";
|
||||
import { masterKey } from "./traffic";
|
||||
|
||||
/**
|
||||
* Creates an MCP server through the UI's discovery -> custom form flow and
|
||||
|
|
@ -52,6 +53,35 @@ export async function createMcpServer(page: PwPage, url: string): Promise<string
|
|||
return name;
|
||||
}
|
||||
|
||||
/**
|
||||
* Deletes every server carrying `serverName`, through the API.
|
||||
*
|
||||
* Cleaning up is not housekeeping here, it is what keeps the MCP specs
|
||||
* runnable. Servers persist for the life of the database, the MCP page reaches
|
||||
* out to each one it lists, and a server whose URL does not answer holds that
|
||||
* up. Measured on a local stack: with eleven leaked servers, eight of them
|
||||
* pointing at an unreachable host, navigateToPage's networkidle wait stopped
|
||||
* settling inside 30s and every MCP spec failed -- including the ones that
|
||||
* leaked nothing. Deleting the leftovers made all five pass again.
|
||||
*
|
||||
* Failures here are swallowed on purpose: this runs from afterEach, and a
|
||||
* cleanup error reported as a test failure hides whatever the test itself
|
||||
* found.
|
||||
*/
|
||||
export async function deleteMcpServerByName(page: PwPage, serverName: string): Promise<void> {
|
||||
const headers = { Authorization: `Bearer ${masterKey()}` };
|
||||
try {
|
||||
const res = await page.request.get("/v1/mcp/server", { headers });
|
||||
if (!res.ok()) return;
|
||||
const servers = (await res.json()) as { server_id: string; server_name?: string }[];
|
||||
for (const server of servers.filter((candidate) => candidate.server_name === serverName)) {
|
||||
await page.request.delete(`/v1/mcp/server/${server.server_id}`, { headers });
|
||||
}
|
||||
} catch {
|
||||
// best effort -- see above
|
||||
}
|
||||
}
|
||||
|
||||
/** Opens a server from the grid and switches to its MCP Tools tab. */
|
||||
export async function openMcpToolsTab(page: PwPage, serverName: string): Promise<void> {
|
||||
await page.getByTestId("mcp-servers-grid").getByText(serverName).first().click();
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
import { test, expect, type Page as PlaywrightPage } from "@playwright/test";
|
||||
import { ADMIN_STORAGE_PATH } from "../../constants";
|
||||
import { createMcpServer } from "../../helpers/mcp";
|
||||
import { createMcpServer, deleteMcpServerByName } from "../../helpers/mcp";
|
||||
import { captureRequestBody, readBack } from "../../helpers/roundTrip";
|
||||
|
||||
/**
|
||||
|
|
@ -36,6 +36,14 @@ test.describe("MCP Servers - edit and delete", () => {
|
|||
serverName = await createMcpServer(page, UNREACHABLE_URL);
|
||||
});
|
||||
|
||||
// The rename test leaves its server behind, and this spec's servers are the
|
||||
// unreachable ones -- exactly the kind that slow the MCP page down for every
|
||||
// later test. The delete test's server is already gone; deleting by name is
|
||||
// a no-op then.
|
||||
test.afterEach(async ({ page }) => {
|
||||
await deleteMcpServerByName(page, serverName);
|
||||
});
|
||||
|
||||
test("Renaming a server's alias persists", async ({ page }) => {
|
||||
const before = await findServerByName(page, serverName);
|
||||
expect(before, `created server ${serverName} readable from /v1/mcp/server`).toBeTruthy();
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ import { test, expect } from "@playwright/test";
|
|||
import { ADMIN_STORAGE_PATH } from "../../constants";
|
||||
import { navigateToPage } from "../../helpers/navigation";
|
||||
import { Page } from "../../fixtures/pages";
|
||||
import { deleteMcpServerByName } from "../../helpers/mcp";
|
||||
|
||||
// Coverage scope: only the happy-path Streamable HTTP + None auth create flow.
|
||||
// See E2E_COVERAGE.md (#29 row) for the full list of uncovered MCP surfaces
|
||||
|
|
@ -11,6 +12,15 @@ import { Page } from "../../fixtures/pages";
|
|||
test.describe("MCP Servers", () => {
|
||||
test.use({ storageState: ADMIN_STORAGE_PATH });
|
||||
|
||||
let createdServerName = "";
|
||||
|
||||
// The server this test creates is unreachable, and the MCP page contacts
|
||||
// every server it lists, so leaving it behind slows down every later MCP
|
||||
// test. See deleteMcpServerByName for what that actually cost.
|
||||
test.afterEach(async ({ page }) => {
|
||||
if (createdServerName) await deleteMcpServerByName(page, createdServerName);
|
||||
});
|
||||
|
||||
test("Add a custom MCP server via the discovery → custom form", async ({ page }) => {
|
||||
await navigateToPage(page, Page.McpServers);
|
||||
|
||||
|
|
@ -25,6 +35,7 @@ test.describe("MCP Servers", () => {
|
|||
|
||||
// Name — no spaces or hyphens per validateMCPServerName
|
||||
const uniqueName = `e2e_mcp_${Date.now()}`;
|
||||
createdServerName = uniqueName;
|
||||
await formModal.locator('input[id="server_name"]').fill(uniqueName);
|
||||
|
||||
// Transport: Streamable HTTP — the only value the proxy actually accepts is "http"
|
||||
|
|
@ -48,8 +59,6 @@ test.describe("MCP Servers", () => {
|
|||
// Submit
|
||||
await formModal.getByRole("button", { name: /^Add MCP Server$/ }).click();
|
||||
|
||||
// No teardown needed — the e2e runner spins up a fresh DB per invocation.
|
||||
|
||||
// Success toast and the new card in the server grid. Scope the lookup to
|
||||
// the MCP servers grid so the form modal's `server_name` input — which
|
||||
// still holds the timestamped value during its close animation — can't
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
import { test, expect, Locator } from "@playwright/test";
|
||||
import { ADMIN_STORAGE_PATH } from "../../constants";
|
||||
import { createMcpServer, openMcpToolsTab } from "../../helpers/mcp";
|
||||
import { createMcpServer, deleteMcpServerByName, openMcpToolsTab } from "../../helpers/mcp";
|
||||
|
||||
// Covers the two MCP manual-QA items that mcpServers.spec.ts cannot: listing a
|
||||
// server's tools, and actually calling one. Both need an MCP server that really
|
||||
|
|
@ -47,6 +47,12 @@ test.describe("MCP Tools", () => {
|
|||
await openMcpToolsTab(page, serverName);
|
||||
});
|
||||
|
||||
// Servers outlive the test that made them, and the MCP page contacts every
|
||||
// one it lists, so leaked servers make later MCP tests slower run by run.
|
||||
test.afterEach(async ({ page }) => {
|
||||
await deleteMcpServerByName(page, serverName);
|
||||
});
|
||||
|
||||
test("MCP Tools tab lists the tools the upstream server advertises", async ({ page }) => {
|
||||
// The list is fetched through the proxy on tab mount, so allow for a cold
|
||||
// upstream connection rather than the ~250ms a warm direct call takes.
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue