From 6fd0d8862b80096bd38c1383c8cd19ea86fdcc84 Mon Sep 17 00:00:00 2001 From: mubashir1osmani Date: Mon, 27 Jul 2026 16:18:21 -0700 Subject: [PATCH] test(e2e): let the ui suite run from a read-only cwd The playwright suite never executed on stage. It died in globalSetup before a single test ran, and the reported error was a red herring. /app/e2e/ui is a read-only filesystem in the packaged e2e image (the image runner already redirects playwright's own artifacts to TMPDIR for this reason), but the suite wrote three things relative to cwd: the per-role storageState files, the failure-screenshot directory, and the html report. Reproduced in the pod: storageState raises EROFS, mkdir test-results raises ENOENT. Worse, the catch block that exists to capture a screenshot threw its own ENOENT while handling a failure, so the real login error was replaced by a filesystem error. That is why the run looked like a missing directory rather than whatever actually went wrong. Route every artifact through ARTIFACT_DIR (E2E_UI_ARTIFACT_DIR, default "." to keep run_e2e.sh behavior unchanged), make the diagnostic screenshot best-effort so it can never mask the underlying failure, and point playwright's reporter and outputDir at the same place so a bare `npx playwright test` works there too. fixtures/users.ts had its own copy of the five storageState filenames; it now re-exports the ones from constants so the paths have a single definition. Verified in the read-only pod: both writes fail before, both succeed after. 85 tests enumerate and tsc --noEmit is clean. Refs LIT-4821 --- tests/e2e/ui/constants.ts | 23 ++++++++++++++++++----- tests/e2e/ui/fixtures/users.ts | 20 +++++++++++++++----- tests/e2e/ui/globalSetup.ts | 24 ++++++++++++++++++++---- tests/e2e/ui/playwright.config.ts | 9 +++++++-- 4 files changed, 60 insertions(+), 16 deletions(-) diff --git a/tests/e2e/ui/constants.ts b/tests/e2e/ui/constants.ts index 17adb0f5fce..9d918736262 100644 --- a/tests/e2e/ui/constants.ts +++ b/tests/e2e/ui/constants.ts @@ -1,15 +1,28 @@ +import * as path from "path"; + export const UI_BASE_URL = ( process.env.E2E_UI_BASE_URL || process.env.LITELLM_PROXY_URL || "http://localhost:4000" ).replace(/\/+$/, ""); +// Directory every artifact this suite writes must land in: storage states, +// failure screenshots, playwright output. A local `./run_e2e.sh` run has a +// writable cwd, so it keeps the historical behavior of writing beside the +// suite. In the packaged e2e image the suite ships on a read-only filesystem +// (/app/e2e/ui), so bare relative paths raise EROFS/ENOENT and the run dies in +// globalSetup before a single test executes. Point E2E_UI_ARTIFACT_DIR at a +// writable path (the image runner already exports TMPDIR) to relocate them. +export const ARTIFACT_DIR = process.env.E2E_UI_ARTIFACT_DIR || "."; + +const storagePath = (name: string): string => path.join(ARTIFACT_DIR, name); + // Storage state paths for each role -export const ADMIN_STORAGE_PATH = "admin.storageState.json"; -export const ADMIN_VIEWER_STORAGE_PATH = "adminViewer.storageState.json"; -export const INTERNAL_USER_STORAGE_PATH = "internalUser.storageState.json"; -export const INTERNAL_VIEWER_STORAGE_PATH = "internalViewer.storageState.json"; -export const TEAM_ADMIN_STORAGE_PATH = "teamAdmin.storageState.json"; +export const ADMIN_STORAGE_PATH = storagePath("admin.storageState.json"); +export const ADMIN_VIEWER_STORAGE_PATH = storagePath("adminViewer.storageState.json"); +export const INTERNAL_USER_STORAGE_PATH = storagePath("internalUser.storageState.json"); +export const INTERNAL_VIEWER_STORAGE_PATH = storagePath("internalViewer.storageState.json"); +export const TEAM_ADMIN_STORAGE_PATH = storagePath("teamAdmin.storageState.json"); // Seeded user identities (match seed.sql) export const E2E_PROXY_ADMIN_USER_ID = "e2e-proxy-admin"; diff --git a/tests/e2e/ui/fixtures/users.ts b/tests/e2e/ui/fixtures/users.ts index 7d6d356cefb..731234b5ea9 100644 --- a/tests/e2e/ui/fixtures/users.ts +++ b/tests/e2e/ui/fixtures/users.ts @@ -1,3 +1,11 @@ +import { + ADMIN_STORAGE_PATH, + ADMIN_VIEWER_STORAGE_PATH, + INTERNAL_USER_STORAGE_PATH, + INTERNAL_VIEWER_STORAGE_PATH, + TEAM_ADMIN_STORAGE_PATH, +} from "../constants"; + export enum Role { ProxyAdmin = "proxy_admin", ProxyAdminViewer = "proxy_admin_viewer", @@ -29,10 +37,12 @@ export const users: Record = { }, }; +// Re-exported from constants so the paths have one definition; they must honor +// ARTIFACT_DIR, since the suite runs from a read-only cwd in the e2e image. export const STORAGE_PATHS: Record = { - [Role.ProxyAdmin]: "admin.storageState.json", - [Role.ProxyAdminViewer]: "adminViewer.storageState.json", - [Role.InternalUser]: "internalUser.storageState.json", - [Role.InternalUserViewer]: "internalViewer.storageState.json", - [Role.TeamAdmin]: "teamAdmin.storageState.json", + [Role.ProxyAdmin]: ADMIN_STORAGE_PATH, + [Role.ProxyAdminViewer]: ADMIN_VIEWER_STORAGE_PATH, + [Role.InternalUser]: INTERNAL_USER_STORAGE_PATH, + [Role.InternalUserViewer]: INTERNAL_VIEWER_STORAGE_PATH, + [Role.TeamAdmin]: TEAM_ADMIN_STORAGE_PATH, }; diff --git a/tests/e2e/ui/globalSetup.ts b/tests/e2e/ui/globalSetup.ts index 6dae603b7cf..3f79fac52e6 100644 --- a/tests/e2e/ui/globalSetup.ts +++ b/tests/e2e/ui/globalSetup.ts @@ -1,7 +1,8 @@ import { chromium, expect, request } from "@playwright/test"; import { users, Role, STORAGE_PATHS } from "./fixtures/users"; -import { UI_BASE_URL } from "./constants"; +import { ARTIFACT_DIR, UI_BASE_URL } from "./constants"; import * as fs from "fs"; +import * as path from "path"; async function globalSetup() { const browser = await chromium.launch(); @@ -47,9 +48,24 @@ async function globalSetup() { await page.context().clearCookies({ name: "litellm_return_url" }); await page.context().storageState({ path: storagePath }); } catch (e) { - fs.mkdirSync("test-results", { recursive: true }); - await page.screenshot({ path: `test-results/global-setup-${role}-failure.png`, fullPage: true }); - console.error(`Global setup failed for role ${role}. Screenshot saved. URL: ${page.url()}`); + // Best-effort diagnostics only: this handler must never replace the real + // failure with its own. Writing the screenshot used to throw ENOENT/EROFS + // on the read-only cwd in the e2e image, which masked every underlying + // login error and made the run look like a filesystem bug. + try { + const failureDir = path.join(ARTIFACT_DIR, "test-results"); + fs.mkdirSync(failureDir, { recursive: true }); + await page.screenshot({ + path: path.join(failureDir, `global-setup-${role}-failure.png`), + fullPage: true, + }); + console.error(`Global setup failed for role ${role}. Screenshot saved. URL: ${page.url()}`); + } catch (diagnosticError) { + console.error( + `Global setup failed for role ${role} at URL: ${page.url()}. ` + + `Could not save a screenshot: ${diagnosticError}`, + ); + } throw e; } finally { await page.close(); diff --git a/tests/e2e/ui/playwright.config.ts b/tests/e2e/ui/playwright.config.ts index 8ae8ddad639..a92192f64ae 100644 --- a/tests/e2e/ui/playwright.config.ts +++ b/tests/e2e/ui/playwright.config.ts @@ -1,5 +1,6 @@ import { defineConfig, devices } from "@playwright/test"; -import { UI_BASE_URL } from "./constants"; +import * as path from "path"; +import { ARTIFACT_DIR, UI_BASE_URL } from "./constants"; /** * See https://playwright.dev/docs/test-configuration. @@ -17,7 +18,11 @@ export default defineConfig({ /* Opt out of parallel tests on CI. */ workers: process.env.CI ? 1 : undefined, /* Reporter to use. See https://playwright.dev/docs/test-reporters */ - reporter: "html", + /* The html reporter and the artifact dir both write relative to cwd, which is + read-only in the packaged e2e image; keep them under ARTIFACT_DIR so a plain + `npx playwright test` works there without extra flags. */ + reporter: [["html", { outputFolder: path.join(ARTIFACT_DIR, "playwright-report"), open: "never" }]], + outputDir: path.join(ARTIFACT_DIR, "test-results"), /* Shared settings for all the projects below. See https://playwright.dev/docs/api/class-testoptions. */ use: { /* Base URL to use in actions like `await page.goto('/')`. */