mirror of
https://github.com/BerriAI/litellm.git
synced 2026-11-11 08:38:24 +00:00
test(e2e-ui): verify key mutations round-trip instead of trusting the toast
The recurring customer report is a form that says "Saved!" and then either
no-ops or clobbers an unrelated field. A toast-only assertion passes in both
cases, and outside three specs that is all this suite checks.
Adds helpers/roundTrip.ts, factoring out the idiom clearCustomPricing,
credentials and routerSettings already use: capture the outgoing request body,
then read the resource back through the management API.
Applies it to the keys spec:
- create: the key is readable from /key/list and owns a team_id, rather than
trusting a table row rendered from the create response the UI already held
- update limits: TPM/RPM are on the wire AND persisted, and the key's models
and team are unchanged -- bumping one field wiping another is the reported
failure mode (PR #34452), not a hypothetical
- delete: the key is gone from /key/list, not merely toasted as deleted
- regenerate: the stored token actually changed
/key/list shape is per KeyListResponseObject in litellm/proxy/_types.py.
Not yet executed: ports 4000/8090 are held by a parallel run.
This commit is contained in:
parent
5e189e9b1a
commit
0ae340be04
2 changed files with 145 additions and 2 deletions
57
tests/e2e/ui/helpers/roundTrip.ts
Normal file
57
tests/e2e/ui/helpers/roundTrip.ts
Normal file
|
|
@ -0,0 +1,57 @@
|
|||
import { expect, Page } from "@playwright/test";
|
||||
import { masterKey } from "./traffic";
|
||||
|
||||
/**
|
||||
* Assert-the-round-trip helpers.
|
||||
*
|
||||
* WHY THESE EXIST: most mutation specs in this suite stop at a success toast,
|
||||
* and a success toast is exactly the thing that lies. The recurring customer
|
||||
* report is a form that says "Saved!" and then either no-ops or clobbers an
|
||||
* unrelated field -- a toast-only assertion passes in both cases.
|
||||
*
|
||||
* A mutation is verified when two things hold:
|
||||
* 1. the UI put the intended change on the wire, and
|
||||
* 2. the change is still there when read back from the API.
|
||||
*
|
||||
* (2) alone misses "the UI sent the right thing but the backend dropped it";
|
||||
* (1) alone misses "the request was right but nothing persisted". Assert both.
|
||||
*
|
||||
* The idiom is lifted from the three specs that already do this correctly --
|
||||
* modelsPage/clearCustomPricing.spec.ts, modelsPage/credentials.spec.ts and
|
||||
* settings/routerSettings.spec.ts -- and is only factored out here so the rest
|
||||
* of the suite stops reinventing it.
|
||||
*/
|
||||
|
||||
/**
|
||||
* Runs `action` and returns the parsed body of the first matching request it
|
||||
* causes.
|
||||
*
|
||||
* `action` is a callback rather than something you await beforehand on purpose:
|
||||
* the listener has to be armed BEFORE the click, and doing that by hand is the
|
||||
* easy mistake -- await the click first and the request has already gone by the
|
||||
* time you start waiting, so the test hangs until timeout for a reason that
|
||||
* looks nothing like the real cause.
|
||||
*/
|
||||
export async function captureRequestBody(
|
||||
page: Page,
|
||||
match: { method: string; urlIncludes: string },
|
||||
action: () => Promise<void>,
|
||||
): Promise<Record<string, any>> {
|
||||
const pending = page.waitForRequest((req) => req.method() === match.method && req.url().includes(match.urlIncludes));
|
||||
await action();
|
||||
const request = await pending;
|
||||
return JSON.parse(request.postData() ?? "{}") as Record<string, any>;
|
||||
}
|
||||
|
||||
/**
|
||||
* Reads an endpoint back through the management API, authenticated as the
|
||||
* master key rather than the browser session -- so a read-back failure is
|
||||
* unambiguously "the data is wrong", never "the UI's token expired".
|
||||
*/
|
||||
export async function readBack<T = any>(page: Page, endpoint: string): Promise<T> {
|
||||
const res = await page.request.get(endpoint, {
|
||||
headers: { Authorization: `Bearer ${masterKey()}` },
|
||||
});
|
||||
expect(res.ok(), `GET ${endpoint}`).toBe(true);
|
||||
return (await res.json()) as T;
|
||||
}
|
||||
|
|
@ -1,4 +1,4 @@
|
|||
import { test, expect } from "@playwright/test";
|
||||
import { test, expect, type Page as PlaywrightPage } from "@playwright/test";
|
||||
import {
|
||||
ADMIN_STORAGE_PATH,
|
||||
E2E_DELETE_KEY_ALIAS,
|
||||
|
|
@ -9,6 +9,31 @@ import {
|
|||
} from "../../constants";
|
||||
import { Page } from "../../fixtures/pages";
|
||||
import { navigateToPage, dismissFeedbackPopup } from "../../helpers/navigation";
|
||||
import { captureRequestBody, readBack } from "../../helpers/roundTrip";
|
||||
|
||||
/**
|
||||
* Looks a key up through the management API by alias.
|
||||
*
|
||||
* `return_full_object=true` is what makes the row carry the fields these tests
|
||||
* assert on (token, models, tpm_limit); without it the response is aliases
|
||||
* only. `/key/list` matches exactly unless `substring_matching` is set, which
|
||||
* is what we want here -- the dashboard opts into substring matching for its
|
||||
* search box, we do not.
|
||||
*
|
||||
* Returns undefined when no key carries the alias, which is how the delete
|
||||
* test proves the row is actually gone.
|
||||
*
|
||||
* Response shape is KeyListResponseObject (litellm/proxy/_types.py): `keys` is
|
||||
* a list of UserAPIKeyAuth once return_full_object is set, so token / models /
|
||||
* team_id sit at the top level of each row.
|
||||
*/
|
||||
async function findKeyByAlias(page: PlaywrightPage, alias: string): Promise<Record<string, any> | undefined> {
|
||||
const body = await readBack<{ keys: Record<string, any>[] }>(
|
||||
page,
|
||||
`/key/list?key_alias=${encodeURIComponent(alias)}&return_full_object=true&size=100`,
|
||||
);
|
||||
return body.keys.find((row) => row.key_alias === alias);
|
||||
}
|
||||
|
||||
test.describe("Proxy Admin - Keys", () => {
|
||||
test.use({ storageState: ADMIN_STORAGE_PATH });
|
||||
|
|
@ -47,12 +72,25 @@ test.describe("Proxy Admin - Keys", () => {
|
|||
|
||||
// Verify the new key appears in the table
|
||||
await expect(page.getByText(keyName)).toBeVisible({ timeout: 10_000 });
|
||||
|
||||
// ...and that it was really created against the team. The row rendering
|
||||
// above comes from the create response the UI already holds, so it would
|
||||
// still show a key whose team assignment the backend dropped.
|
||||
const persisted = await findKeyByAlias(page, keyName);
|
||||
expect(persisted, `key ${keyName} readable from /key/list`).toBeTruthy();
|
||||
expect(typeof persisted?.team_id, "created key is owned by a team, not orphaned").toBe("string");
|
||||
});
|
||||
|
||||
test("Regenerate key", async ({ page }) => {
|
||||
await navigateToPage(page, Page.ApiKeys);
|
||||
await dismissFeedbackPopup(page);
|
||||
|
||||
// The point of regeneration is that the stored token changes. Capture the
|
||||
// old one first -- a modal offering a Copy button proves only that the UI
|
||||
// rendered a success view.
|
||||
const before = await findKeyByAlias(page, E2E_REGENERATE_KEY_ALIAS);
|
||||
expect(before?.token, `seeded key ${E2E_REGENERATE_KEY_ALIAS} has a token`).toBeTruthy();
|
||||
|
||||
// Key IDs are rendered as buttons in the table
|
||||
const keyRow = page.locator("tr", { hasText: E2E_REGENERATE_KEY_ALIAS });
|
||||
await expect(keyRow).toBeVisible({ timeout: 10_000 });
|
||||
|
|
@ -70,12 +108,28 @@ test.describe("Proxy Admin - Keys", () => {
|
|||
|
||||
// Success view shows a Copy button in the footer (text varies between modal versions)
|
||||
await expect(modal.getByRole("button", { name: /Copy.*Key/ })).toBeVisible({ timeout: 20_000 });
|
||||
|
||||
// The stored token must actually have been replaced, and the key must keep
|
||||
// its identity while doing so -- a regenerate that silently orphans the
|
||||
// alias looks identical from the modal.
|
||||
await expect
|
||||
.poll(async () => (await findKeyByAlias(page, E2E_REGENERATE_KEY_ALIAS))?.token, {
|
||||
message: `token for ${E2E_REGENERATE_KEY_ALIAS} did not change after regenerate`,
|
||||
timeout: 15_000,
|
||||
})
|
||||
.not.toBe(before?.token);
|
||||
});
|
||||
|
||||
test("Update key TPM and RPM limits", async ({ page }) => {
|
||||
await navigateToPage(page, Page.ApiKeys);
|
||||
await dismissFeedbackPopup(page);
|
||||
|
||||
// Snapshot the key before touching it, so the assertions at the end can
|
||||
// tell "the limits changed" apart from "the limits changed and something
|
||||
// else changed with them".
|
||||
const before = await findKeyByAlias(page, E2E_UPDATE_LIMITS_KEY_ALIAS);
|
||||
expect(before, `seeded key ${E2E_UPDATE_LIMITS_KEY_ALIAS} exists`).toBeTruthy();
|
||||
|
||||
const keyRow = page.locator("tr", { hasText: E2E_UPDATE_LIMITS_KEY_ALIAS });
|
||||
await expect(keyRow).toBeVisible({ timeout: 10_000 });
|
||||
await keyRow.locator("button").first().click();
|
||||
|
|
@ -87,10 +141,33 @@ test.describe("Proxy Admin - Keys", () => {
|
|||
|
||||
await page.getByRole("spinbutton", { name: "TPM Limit" }).fill("123");
|
||||
await page.getByRole("spinbutton", { name: "RPM Limit" }).fill("456");
|
||||
await page.getByRole("button", { name: "Save Changes" }).click();
|
||||
|
||||
const update = await captureRequestBody(page, { method: "POST", urlIncludes: "/key/update" }, async () => {
|
||||
await page.getByRole("button", { name: "Save Changes" }).click();
|
||||
});
|
||||
|
||||
// The form posts limits at the top level (key_info_view.tsx sets
|
||||
// formValues.key from the token and hands the whole object to
|
||||
// keyUpdateCall). Compare numerically -- the field arrives as a string or a
|
||||
// number depending on how the spinbutton was last touched.
|
||||
expect(Number(update.tpm_limit), "TPM limit on the wire").toBe(123);
|
||||
expect(Number(update.rpm_limit), "RPM limit on the wire").toBe(456);
|
||||
|
||||
await expect(page.getByRole("paragraph").filter({ hasText: "TPM: 123" })).toBeVisible({ timeout: 10_000 });
|
||||
await expect(page.getByRole("paragraph").filter({ hasText: "RPM: 456" })).toBeVisible({ timeout: 10_000 });
|
||||
|
||||
// The rendering above is driven by the response the UI already holds, so it
|
||||
// proves nothing about what is stored. Read the key back.
|
||||
const after = await findKeyByAlias(page, E2E_UPDATE_LIMITS_KEY_ALIAS);
|
||||
expect(after, "key still readable after update").toBeTruthy();
|
||||
expect(Number(after?.tpm_limit), "TPM limit persisted").toBe(123);
|
||||
expect(Number(after?.rpm_limit), "RPM limit persisted").toBe(456);
|
||||
|
||||
// And that editing the limits did not take anything else with it. This is
|
||||
// the reported failure mode, not a hypothetical: bumping a key's budget
|
||||
// wiped its MCP toolset (PR #34452), and the toast said success both times.
|
||||
expect(after?.models, "editing limits left the key's models untouched").toEqual(before?.models);
|
||||
expect(after?.team_id, "editing limits left the key's team untouched").toEqual(before?.team_id);
|
||||
});
|
||||
|
||||
test("Delete key", async ({ page }) => {
|
||||
|
|
@ -115,6 +192,15 @@ test.describe("Proxy Admin - Keys", () => {
|
|||
await deleteButton.click();
|
||||
|
||||
await expect(page.getByText(/Key deleted/i).first()).toBeVisible({ timeout: 10_000 });
|
||||
|
||||
// "Key deleted" is a claim, not evidence. The key is gone when the
|
||||
// management API stops returning it.
|
||||
await expect
|
||||
.poll(async () => await findKeyByAlias(page, E2E_DELETE_KEY_ALIAS), {
|
||||
message: `key ${E2E_DELETE_KEY_ALIAS} still readable from /key/list after delete`,
|
||||
timeout: 15_000,
|
||||
})
|
||||
.toBeUndefined();
|
||||
});
|
||||
|
||||
test("See internal user keys in team", async ({ page }) => {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue