[Feature] UI E2E Tests: Proxy Admin Team and Key Management - #25365
Conversation
…ntegration Add Playwright E2E tests covering proxy admin team and key management workflows, with a self-contained test runner and CircleCI integration. Tests cover: create team, invite user, edit/delete team members, create key in team, regenerate key, update TPM/RPM limits, delete key, and verify internal user keys are visible. Infrastructure: run_e2e.sh builds the UI from source before starting the proxy, ensuring tests always run against the latest UI changes. Added data-testid attributes to key UI components for reliable selectors.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR adds a self-contained Playwright E2E test suite for proxy admin team and key management workflows, replacing the previous machine-based CircleCI job with a docker-based one. It introduces a seed SQL file, a mock LLM server, a consolidated test runner script, and Confidence Score: 5/5Safe to merge — all previously blocking issues are resolved; remaining findings are minor P2 style suggestions. All P0/P1 findings from prior review rounds (missing constants, broken globalSetup imports, absent seed.sql) are addressed. The three remaining comments are P2: redundant popup-dismiss calls, a fragile SVG selector for delete, and an unused TeamAdmin session. None of these block correctness or CI reliability. No files require special attention beyond the P2 style suggestions in teams.spec.ts, navigation.ts, and fixtures/users.ts.
|
| Filename | Overview |
|---|---|
| ui/litellm-dashboard/e2e_tests/constants.ts | Replaced hardcoded key-ID prefixes with stable key/team aliases that match seed.sql; all exports now present and consistent with spec imports. |
| ui/litellm-dashboard/e2e_tests/fixtures/users.ts | Inlines Role enum and adds STORAGE_PATHS; TeamAdmin role is set up in globalSetup but has no consuming test yet (unused overhead). |
| ui/litellm-dashboard/e2e_tests/globalSetup.ts | Now loops over all roles using STORAGE_PATHS correctly; includes screenshot on failure and proper page cleanup. |
| ui/litellm-dashboard/e2e_tests/fixtures/seed.sql | New idempotent seed file; deletes and re-inserts deterministic e2e test data for users, teams, memberships, and API keys. |
| ui/litellm-dashboard/e2e_tests/run_e2e.sh | Self-contained runner: starts Postgres (local) or uses CI service, builds UI from source, seeds DB, starts mock + proxy, then runs Playwright. |
| ui/litellm-dashboard/e2e_tests/tests/proxy-admin/keys.spec.ts | Five key-management tests using seeded data; all constants now correctly imported from updated constants.ts. |
| ui/litellm-dashboard/e2e_tests/tests/proxy-admin/teams.spec.ts | Five team-management tests; delete-team uses a fragile svg/img locator instead of a dedicated testid for the delete button. |
| ui/litellm-dashboard/e2e_tests/helpers/navigation.ts | navigateToPage now internally calls dismissFeedbackPopup, but every test also calls it explicitly — redundant ~1.5 s per navigation. |
| .circleci/config.yml | Replaces the old machine-based e2e job with a docker-based one; workflow entry omits the context key needed to inject DockerHub credentials. |
| ui/litellm-dashboard/src/components/OldTeams.tsx | Adds data-testid attributes to team-id cells, create-team button, team-name input, and create-team submit button. |
| ui/litellm-dashboard/src/components/organisms/create_key_button.tsx | Adds data-testid=create-key-button to the create-key trigger button. |
Reviews (5): Last reviewed commit: "[Fix] Remove old broken key tests supers..." | Re-trigger Greptile
| import { test, expect } from "@playwright/test"; | ||
| import { | ||
| ADMIN_STORAGE_PATH, | ||
| E2E_DELETE_KEY_ALIAS, | ||
| E2E_REGENERATE_KEY_ALIAS, | ||
| E2E_UPDATE_LIMITS_KEY_ALIAS, | ||
| E2E_INTERNAL_USER_KEY_ALIAS, | ||
| E2E_TEAM_CRUD_ALIAS, | ||
| } from "../../constants"; |
There was a problem hiding this comment.
Imported constants are missing from
constants.ts
All five named imports here — E2E_DELETE_KEY_ALIAS, E2E_REGENERATE_KEY_ALIAS, E2E_UPDATE_LIMITS_KEY_ALIAS, E2E_INTERNAL_USER_KEY_ALIAS, and E2E_TEAM_CRUD_ALIAS — are not exported from e2e_tests/constants.ts. That file only exports ADMIN_STORAGE_PATH, E2E_UPDATE_LIMITS_KEY_ID_PREFIX, E2E_DELETE_KEY_ID_PREFIX, E2E_DELETE_KEY_NAME, and E2E_REGENERATE_KEY_ID_PREFIX. Every one of these imports resolves to undefined at runtime, so selectors like page.locator("tr", { hasText: E2E_DELETE_KEY_ALIAS }) match everything (or nothing), making the tests useless. The same gap exists in teams.spec.ts for E2E_TEAM_CRUD_ID, E2E_TEAM_DELETE_ALIAS, E2E_TEAM_NO_ADMIN_ID, and E2E_TEAM_ORG_ID.
| import { chromium, expect } from "@playwright/test"; | ||
| import { users, Role, STORAGE_PATHS } from "./fixtures/users"; | ||
| import * as fs from "fs"; |
There was a problem hiding this comment.
STORAGE_PATHS and Role are not exported from fixtures/users
globalSetup.ts now imports { users, Role, STORAGE_PATHS } from ./fixtures/users, but users.ts only exports users. Role is defined in ./fixtures/roles.ts and is merely imported (not re-exported) by users.ts. STORAGE_PATHS does not exist anywhere in the visible codebase. As a result, STORAGE_PATHS is undefined and storagePath on line 11 is always undefined, making page.context().storageState({ path: undefined }) throw at runtime and abort global setup entirely before any test runs.
| for (const role of Object.values(Role)) { | ||
| const { email, password } = users[role]; | ||
| const storagePath = STORAGE_PATHS[role]; |
There was a problem hiding this comment.
Loop over all roles but
users only defines ProxyAdmin
Object.values(Role) yields four values (proxy_admin, proxy_admin_viewer, internal_user, internal_user_viewer), but users.ts only contains an entry for Role.ProxyAdmin. For every other role the destructure on line 9 (const { email, password } = users[role]) hits undefined and throws TypeError: Cannot destructure property 'email' of undefined, crashing global setup before any test has a chance to run.
| PGPASSWORD="$DB_PASS" psql -h "$DB_HOST" -p "$DB_PORT" -U "$DB_USER" -d "$DB_NAME" \ | ||
| -f "$SCRIPT_DIR/fixtures/seed.sql" |
There was a problem hiding this comment.
seed.sql is referenced but does not exist
run_e2e.sh and the CircleCI ui_e2e_tests job both attempt to run ui/litellm-dashboard/e2e_tests/fixtures/seed.sql, but no such file is present in the repository. The seeding step will fail with a "file not found" error, leaving the database empty. All tests that rely on pre-seeded teams, keys, or users (i.e. the majority of the 10 test cases) will then fail against the missing data.
| async function clickTeamId(page: import("@playwright/test").Page, teamId: string) { | ||
| const idPrefix = teamId.slice(0, 7); |
There was a problem hiding this comment.
idPrefix is computed but never referenced in the function body — only teamId itself is used in the locator filter.
| async function clickTeamId(page: import("@playwright/test").Page, teamId: string) { | |
| const idPrefix = teamId.slice(0, 7); | |
| async function clickTeamId(page: import("@playwright/test").Page, teamId: string) { |
- Add constants.ts with all required exports (key aliases, team IDs) - Add fixtures/users.ts with all role definitions and storage paths - Add fixtures/seed.sql for deterministic test database seeding - Remove Firefox project from playwright config (only Chromium installed) - Remove unused variable in teams.spec.ts - Rename CircleCI job to e2e_ui_testing
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
| only: | ||
| - main | ||
| - /litellm_.*/ | ||
| # - e2e_ui_testing: | ||
| # name: e2e_ui_testing_chromium | ||
| # browser: chromium | ||
| # context: e2e_ui_tests | ||
| # requires: | ||
| # - ui_build | ||
| # - build_docker_database_image | ||
| # - prisma_schema_sync | ||
| # filters: | ||
| # branches: | ||
| # only: | ||
| # - main | ||
| # - /litellm_.*/ | ||
| # - e2e_ui_testing: | ||
| # name: e2e_ui_testing_firefox | ||
| # browser: firefox | ||
| # context: e2e_ui_tests | ||
| # requires: | ||
| # - ui_build | ||
| # - build_docker_database_image | ||
| # - prisma_schema_sync | ||
| # filters: | ||
| # branches: | ||
| # only: | ||
| # - main | ||
| # - /litellm_.*/ | ||
| - e2e_ui_testing: | ||
| filters: | ||
| branches: | ||
| only: | ||
| - main | ||
| - /litellm_.*/ |
There was a problem hiding this comment.
Missing
context causes DockerHub auth to fail
The new e2e_ui_testing workflow entry omits the context: key that the old (commented-out) entries used (context: e2e_ui_tests). The job definition's auth block references ${DOCKERHUB_USERNAME} and ${DOCKERHUB_PASSWORD}, but without a context those variables are never injected, so the credentials resolve to empty strings. CircleCI will then attempt an unauthenticated pull and hit Docker Hub's rate limit or an auth error, causing every run on main/litellm_* to fail before any test executes.
| only: | |
| - main | |
| - /litellm_.*/ | |
| # - e2e_ui_testing: | |
| # name: e2e_ui_testing_chromium | |
| # browser: chromium | |
| # context: e2e_ui_tests | |
| # requires: | |
| # - ui_build | |
| # - build_docker_database_image | |
| # - prisma_schema_sync | |
| # filters: | |
| # branches: | |
| # only: | |
| # - main | |
| # - /litellm_.*/ | |
| # - e2e_ui_testing: | |
| # name: e2e_ui_testing_firefox | |
| # browser: firefox | |
| # context: e2e_ui_tests | |
| # requires: | |
| # - ui_build | |
| # - build_docker_database_image | |
| # - prisma_schema_sync | |
| # filters: | |
| # branches: | |
| # only: | |
| # - main | |
| # - /litellm_.*/ | |
| - e2e_ui_testing: | |
| filters: | |
| branches: | |
| only: | |
| - main | |
| - /litellm_.*/ | |
| - e2e_ui_testing: | |
| context: e2e_ui_tests | |
| filters: | |
| branches: | |
| only: | |
| - main | |
| - /litellm_.*/ |
Resolved conflicts: - streaming_handler.py: combined role check (PR #24354, Azure streaming) with reasoning_items check (new in main) — both are independent OR conditions in is_chunk_non_empty() - CI/CD: accepted main's versions throughout - Redis tests migrated to CircleCI (PR #25354): removed enable-redis from GH Actions workflows - E2E UI tests restructured (PR #25365): simplified CircleCI job - Coverage via Codecov added to all GH Actions unit test workflows - Deleted test-litellm-matrix.yml and test-proxy-e2e-azure-batches.yml (removed in main)
[Feature] UI E2E Tests: Proxy Admin Team and Key Management
Resolved conflicts: - streaming_handler.py: combined role check (PR BerriAI#24354, Azure streaming) with reasoning_items check (new in main) — both are independent OR conditions in is_chunk_non_empty() - CI/CD: accepted main's versions throughout - Redis tests migrated to CircleCI (PR BerriAI#25354): removed enable-redis from GH Actions workflows - E2E UI tests restructured (PR BerriAI#25365): simplified CircleCI job - Coverage via Codecov added to all GH Actions unit test workflows - Deleted test-litellm-matrix.yml and test-proxy-e2e-azure-batches.yml (removed in main)
Summary
Add Playwright E2E tests covering proxy admin team and key management workflows, with a self-contained test runner and CircleCI integration.
run_e2e.sh) builds the UI from source before starting the proxy, so tests always run against the latest UI changesdata-testidattributes to key UI components for reliable Playwright selectorsui_e2e_testsjob to CircleCI workflowTesting
10 tests covering the stable release QA checklist items:
Teams (5 tests): create team, invite user, edit member (non-member team), delete team, edit member (org team)
Keys (5 tests): create key in team, regenerate key, update TPM/RPM limits, delete key, see internal user keys
Run locally:
Type
🆕 New Feature
✅ Test
🚄 Infrastructure