From 91a986b13dd4f03523578471502c15085c66f3bb Mon Sep 17 00:00:00 2001 From: Chirag Madlani <12962843+chirag-madlani@users.noreply.github.com> Date: Fri, 11 Sep 2026 19:34:09 +0530 Subject: [PATCH 1/2] refactor(playwright): one place that builds a signed-in page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `e2e/fixtures/pages.ts` and `support/fixtures/userPages.ts` were near-identical copies of the same role-page fixtures, and had already drifted — one had `viewOnlyPage`, the other did not. `userPages.ts` now owns every role page and all the construction logic; `pages.ts` adds only the `page` -> `adminPage` alias, which is the one behaviour that genuinely differs between them. Adds the missing sanctioned path for a test that needs its *own* account, which is why specs kept hand-rolling one. `support/fixtures/isolatedUser.ts` exposes `isolatedUserPage` (one account per worker, fresh page per test) and `freshUserPage` (a new account per test). Both create and delete the account in fixture teardown, so the beforeAll + array + afterAll shape that produces the merge queue's number-one flake cannot be written by accident. Adds `UserClass.signIn()`, which establishes the same session as `login()` with one POST instead of nine UI interactions. `login()` drives the form: navigate to /signin, wait, fill, Tab, fill, click, await the response, await the redirect, dismiss the getting-started dialog, collapse the sidebar. The suite does that ~290 times and none of it is what the tests are testing. Both paths funnel through the same `completeSignIn()` — conditional reads off, dialog dismissed, sidebar collapsed — so they differ in how the session was established and nothing else. That matters: the sidebar collapse changes page geometry, so a sign-in path that skipped it would move every locator in the spec. The mechanism is assembled from pieces already load-bearing here: the login request is the one `createAdminApiContext` makes (base64 password, matching the UI's `btoa`), and `setToken` is the exact inverse of the `getToken` that `auth.setup.ts` reads back after a real sign-in. `signInViaApi` asserts the app shell rendered, so a session the app refuses fails there, naming the user, instead of timing out somewhere unrelated. `auth.setup.ts` and `performUserLogin` now take that path. Setup drops from 50.4s to 25.6s and still writes all eight storage states. `RoleFixtureSessions.spec.ts` asserts each of those eight yields a signed-in page — the contract every lane depends on, which previously failed far from its cause when broken. Co-Authored-By: Claude Opus 5 (1M context) --- .claude/rules/frontend-playwright.md | 10 + .github/playwright/impact-map.generated.json | 4 + .../PLAYWRIGHT_DEVELOPER_HANDBOOK.md | 83 ++++++++ .../e2e/Features/RoleFixtureSessions.spec.ts | 82 ++++++++ .../resources/ui/playwright/e2e/auth.setup.ts | 29 ++- .../ui/playwright/e2e/fixtures/pages.ts | 120 ++++-------- .../ui/playwright/support/fixtures/base.ts | 5 +- .../support/fixtures/isolatedUser.ts | 179 ++++++++++++++++++ .../playwright/support/fixtures/userPages.ts | 154 ++++++++------- .../ui/playwright/support/user/AdminClass.ts | 9 + .../ui/playwright/support/user/UserClass.ts | 47 +++++ .../ui/playwright/utils/apiSignIn.ts | 112 +++++++++++ .../ui/playwright/utils/initialSetup.ts | 2 +- .../resources/ui/playwright/utils/user.ts | 14 +- 14 files changed, 673 insertions(+), 177 deletions(-) create mode 100644 openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/RoleFixtureSessions.spec.ts create mode 100644 openmetadata-ui/src/main/resources/ui/playwright/support/fixtures/isolatedUser.ts create mode 100644 openmetadata-ui/src/main/resources/ui/playwright/utils/apiSignIn.ts diff --git a/.claude/rules/frontend-playwright.md b/.claude/rules/frontend-playwright.md index 993709e1675b..c0943b2ba172 100644 --- a/.claude/rules/frontend-playwright.md +++ b/.claude/rules/frontend-playwright.md @@ -27,6 +27,16 @@ Highest-value constraints, all machine-enforced: - No positional locators (`.first()`, `.last()`, `.nth()`) — narrow the locator, or use `getRowByName()` from `playwright/utils/scopedLocators.ts`. +- **Sign in with a role page fixture, not a bespoke user.** `support/fixtures/userPages.ts` owns + every signed-in page (`adminPage`, `dataConsumerPage`, `dataStewardPage`, `ownerPage`, + `editDescriptionPage`, `editTagsPage`, `editGlossaryTermPage`, `viewOnlyPage`); + `e2e/fixtures/pages.ts` re-exports them and aliases `page` to `adminPage`. When the test needs its + own account, `support/fixtures/isolatedUser.ts` has `isolatedUserPage` (one per worker) and + `freshUserPage` (one per test) — both create *and delete* the account, so there is no + `beforeAll`/`afterAll` bookkeeping to get wrong. Never call `UserClass.login()`: it drives the + sign-in form (nine UI interactions). `UserClass.signIn()` establishes the same session with one + POST and runs the identical post-sign-in steps; only a spec testing the form itself should drive + `login()`. Creating a user as *test data* is unrelated and unaffected. - **`beforeAll` is not a per-worker hook.** Under `fullyParallel` it runs once per *group* of the file's tests dispatched to a worker, with `afterAll` in between — so it can run twice in one worker. Rebuild describe-scope state at the top of the hook; never `.push()` into it. diff --git a/.github/playwright/impact-map.generated.json b/.github/playwright/impact-map.generated.json index 8583632b5f51..7f62ef29bd56 100644 --- a/.github/playwright/impact-map.generated.json +++ b/.github/playwright/impact-map.generated.json @@ -2149,6 +2149,7 @@ "playwright/e2e/Features/PersonaAIContext.spec.ts", "playwright/e2e/Features/PersonaAIContextPermissions.spec.ts", "playwright/e2e/Features/PersonaAIContextRuleCardAndStates.spec.ts", + "playwright/e2e/Features/RoleFixtureSessions.spec.ts", "playwright/e2e/Flow/PersonaFlow.spec.ts", "playwright/e2e/Pages/DomainDataProductsRightPanel.spec.ts", "playwright/e2e/Pages/ExplorePageRightPanel.spec.ts", @@ -6984,6 +6985,7 @@ "playwright/e2e/Features/AppMode/AppModeResolver.spec.ts", "playwright/e2e/Features/CustomizeNavigationNewItems.spec.ts", "playwright/e2e/Features/DataQuality/IncidentManagerLocaleLayout.spec.ts", + "playwright/e2e/Features/RoleFixtureSessions.spec.ts", "playwright/e2e/Features/SettingsNavigationPage.spec.ts", "playwright/e2e/Features/Workflows/WorkflowOssRestrictions.spec.ts" ] @@ -7761,6 +7763,7 @@ "playwright/e2e/Features/CustomizeNavigationNewItems.spec.ts", "playwright/e2e/Features/LanguageOverride.spec.ts", "playwright/e2e/Features/PersonaSessionPersistence.spec.ts", + "playwright/e2e/Features/RoleFixtureSessions.spec.ts", "playwright/e2e/Features/SettingsNavigationPage.spec.ts", "playwright/e2e/Flow/IngestionBot.spec.ts", "playwright/e2e/Flow/PersonaDeletionUserProfile.spec.ts", @@ -10006,6 +10009,7 @@ "playwright/e2e/Pages/DataInsightReportApplication.spec.ts", "playwright/e2e/Pages/DataInsightSettings.spec.ts", "playwright/e2e/Pages/DataMarketplace.spec.ts", + "playwright/e2e/Pages/DataMarketplaceAnnouncements.spec.ts", "playwright/e2e/Pages/DataMarketplacePermissions.spec.ts", "playwright/e2e/Pages/DataProductAndSubdomains.spec.ts", "playwright/e2e/Pages/DataProductCertificationFilter.spec.ts", diff --git a/openmetadata-ui/src/main/resources/ui/playwright/PLAYWRIGHT_DEVELOPER_HANDBOOK.md b/openmetadata-ui/src/main/resources/ui/playwright/PLAYWRIGHT_DEVELOPER_HANDBOOK.md index cdbe86114ebf..505e83b7ca23 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/PLAYWRIGHT_DEVELOPER_HANDBOOK.md +++ b/openmetadata-ui/src/main/resources/ui/playwright/PLAYWRIGHT_DEVELOPER_HANDBOOK.md @@ -244,6 +244,89 @@ export const test = base.extend({ 4. **Only test UI flows once** — if a UI flow is already tested, use API for setup in other tests that depend on that state. +### Signing in: use the role page fixtures, not a bespoke user + +`support/fixtures/userPages.ts` is the **single** place a signed-in page is built. It exposes one +fixture per pre-seeded role — `adminPage`, `dataConsumerPage`, `dataStewardPage`, `ownerPage`, +`editDescriptionPage`, `editTagsPage`, `editGlossaryTermPage`, `viewOnlyPage` — whose storage +states `e2e/auth.setup.ts` creates once and every worker reuses. + +`e2e/fixtures/pages.ts` re-exports those and additionally aliases the built-in `page` to +`adminPage`. It contains no page-construction logic. Import it when the spec is admin-first and +wants `{ page }` to be a signed-in admin; import `support/fixtures/userPages` when the spec drives +named roles and wants `page` left as Playwright's own, so a file-level `test.use({ storageState })` +still applies. + +```typescript +// yes — one context, no login on the critical path +import { test } from '../fixtures/pages'; + +test('a consumer cannot edit the description', async ({ page, dataConsumerPage }) => { … }); + +// no — a signup, a UI login, and a user teardown, per test +const user = new UserClass(); +await user.create(apiContext); +const { page } = await performUserLogin(browser, user); +``` + +### When a seeded role will not do: `isolatedUser` + +Some tests genuinely need their own account — their own team, their own policy, or they rename or +delete the account they are signed in as. That case has a fixture too, so there is never a reason +to hand-roll the lifecycle: + +```typescript +import { test } from '../../support/fixtures/isolatedUser'; + +test.use({ isolatedUserOptions: { isAdmin: true } }); + +test('…', async ({ isolatedUserPage, isolatedUser }) => { … }); +``` + +| Fixture | Scope | Cost | Use when | +|---|---|---|---| +| `isolatedUserPage` + `isolatedUser` | one account per **worker**, fresh page per test | one signup + one sign-in per worker | the test needs an account that is not a seeded role, but does not modify the account itself | +| `freshUserPage` | a new account per **test** | a signup + a sign-in per test | the test mutates the account it is signed in as (rename, role change, deactivate, delete) | + +Both create the account and delete it in teardown. That is the whole point: the lifecycle is +Playwright's, not yours, so the `beforeAll` + array + `afterAll` shape that produces the merge +queue's number-one flake cannot be written by accident. `isolatedUserPage` also restores a +captured storage state — including IndexedDB, where the app keeps its token — instead of signing +in for every test. + +### Signing in without the form: `signInViaApi` + +Both fixtures authenticate through `utils/apiSignIn.ts` rather than `UserClass.login()`: + +```typescript +await signInUserViaApi(page, user); // one POST + two navigations +``` + +`UserClass.login()` performs nine UI interactions — navigate to /signin, wait for the form, fill, +Tab, fill, click, await the response, await the redirect, dismiss the getting-started modal, +collapse the sidebar. The suite does that ~290 times and none of it is what the tests are testing. +`signInViaApi` posts to `/api/v1/auth/login` (the same request `createAdminApiContext` makes, with +the base64 password the UI's `btoa()` produces) and writes the returned token with `setToken`, +which is the exact inverse of the `getToken` that `auth.setup.ts` reads back after a real sign-in. + +`performUserLogin` already routes through it, so its call sites got the speed-up without changing. +Drive `UserClass.login()` directly only in a spec that is testing the sign-in **form**. + +Two things to know if you touch this path: + +- The navigation to the app origin before writing the token is required — IndexedDB is + origin-scoped, so there is nowhere to write until the page has loaded the origin. +- `tokenStorage` only uses IndexedDB when `'serviceWorker' in navigator`, which is false on a + non-secure origin; it silently falls back to localStorage there. Any test of this path must run + against `localhost` or it exercises the fallback and proves nothing. + `e2e/Features/TokenStorage.spec.ts` guards both, and the third case in it exists specifically to + fail if the fallback is what is under test. + +Use `signIn()` in new specs. `login()` remains for the specs that are testing the sign-in form +itself — `Pages/Login.spec.ts`, `Features/OnlineUsers.spec.ts`, `Flow/Tour.spec.ts` and the +`Features/AppMode` specs. Creating a user as test data is unrelated and unaffected: `new +UserClass()` for an owner, reviewer or assignee stays exactly as it is. + --- ## Locator Priority Order diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/RoleFixtureSessions.spec.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/RoleFixtureSessions.spec.ts new file mode 100644 index 000000000000..9b26c3073354 --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/RoleFixtureSessions.spec.ts @@ -0,0 +1,82 @@ +/* + * Copyright 2026 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { Page } from '@playwright/test'; +import { expect, test } from '../../support/fixtures/userPages'; + +/** + * Every lane in the suite reuses the storage states `e2e/auth.setup.ts` writes. + * If one of them is captured without a usable session, the specs that take that + * role fail far away from the cause — with a permission error, an empty page, or + * a timeout on something that was never going to render — and the setup itself + * still reports green. + * + * These assert the contract directly: each role fixture yields a page that is + * signed in. They are cheap (a navigation each) and they are the regression + * guard for the sign-in mechanism `auth.setup.ts` uses — which is now the API + * (`UserClass.signIn`) rather than the sign-in form, so a change in how the app + * persists its token would otherwise surface as a suite-wide mystery. + */ +const expectSignedIn = async (page: Page, role: string) => { + await page.goto('/my-data', { waitUntil: 'domcontentloaded' }); + + expect( + new URL(page.url()).pathname, + `${role} was redirected to the sign-in page — its storage state carries no usable session` + ).not.toContain('signin'); + + await expect( + page.getByTestId('left-sidebar'), + `${role} did not render the app shell` + ).toBeAttached({ timeout: 30_000 }); + + await expect( + page.getByTestId('dropdown-profile'), + `${role} rendered the shell but no signed-in user` + ).toBeVisible({ timeout: 30_000 }); +}; + +test.describe('role fixture sessions', () => { + test('adminPage is signed in', async ({ adminPage }) => { + await expectSignedIn(adminPage, 'adminPage'); + }); + + test('dataConsumerPage is signed in', async ({ dataConsumerPage }) => { + await expectSignedIn(dataConsumerPage, 'dataConsumerPage'); + }); + + test('dataStewardPage is signed in', async ({ dataStewardPage }) => { + await expectSignedIn(dataStewardPage, 'dataStewardPage'); + }); + + test('ownerPage is signed in', async ({ ownerPage }) => { + await expectSignedIn(ownerPage, 'ownerPage'); + }); + + test('editDescriptionPage is signed in', async ({ editDescriptionPage }) => { + await expectSignedIn(editDescriptionPage, 'editDescriptionPage'); + }); + + test('editTagsPage is signed in', async ({ editTagsPage }) => { + await expectSignedIn(editTagsPage, 'editTagsPage'); + }); + + test('editGlossaryTermPage is signed in', async ({ + editGlossaryTermPage, + }) => { + await expectSignedIn(editGlossaryTermPage, 'editGlossaryTermPage'); + }); + + test('viewOnlyPage is signed in', async ({ viewOnlyPage }) => { + await expectSignedIn(viewOnlyPage, 'viewOnlyPage'); + }); +}); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/auth.setup.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/auth.setup.ts index 1b084a867553..300be7f334ec 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/auth.setup.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/auth.setup.ts @@ -122,10 +122,19 @@ setup('authenticate all users', async ({ browser }) => { await loginAsAdmin(adminPage, admin); - // Create a new page to login with admin user after token expiry is set to 4 hours - // This is done to avoid logging out the user to get the new token + // Create a new page to sign the admin in after token expiry is set to 4 + // hours. This is done to avoid logging out the user to get the new token. + // + // Every sign-in here goes through `UserClass.signIn()` — one POST to + // /api/v1/auth/login, then the token written where the app reads it — + // rather than driving the sign-in form eight times. The storage state this + // captures is what every worker in every lane reuses, so the only thing + // that matters is that the session is real; how it was established is not + // part of the fixture's contract. `loginAsAdmin` above already took this + // path. A spec that is testing the sign-in *form* drives + // `UserClass.login()` directly instead. const newAdminPage = await browser.newPage(); - await admin.login(newAdminPage); + await admin.signIn(newAdminPage); await newAdminPage.waitForURL( (url) => url.pathname === '/' || url.pathname === '/my-data' @@ -239,43 +248,43 @@ setup('authenticate all users', async ({ browser }) => { .storageState({ path: adminFile, indexedDB: true }); // Save states for each user sequentially to avoid file operation conflicts - await dataConsumer.login(dataConsumerPage); + await dataConsumer.signIn(dataConsumerPage); await disableEtagConditionalReads(dataConsumerPage); await dataConsumerPage .context() .storageState({ path: dataConsumerFile, indexedDB: true }); - await dataSteward.login(dataStewardPage); + await dataSteward.signIn(dataStewardPage); await disableEtagConditionalReads(dataStewardPage); await dataStewardPage .context() .storageState({ path: dataStewardFile, indexedDB: true }); - await editDescriptionUser.login(editDescriptionPage); + await editDescriptionUser.signIn(editDescriptionPage); await disableEtagConditionalReads(editDescriptionPage); await editDescriptionPage .context() .storageState({ path: editDescriptionFile, indexedDB: true }); - await editTagsUser.login(editTagsPage); + await editTagsUser.signIn(editTagsPage); await disableEtagConditionalReads(editTagsPage); await editTagsPage .context() .storageState({ path: editTagsFile, indexedDB: true }); - await editGlossaryTermUser.login(editGlossaryTermPage); + await editGlossaryTermUser.signIn(editGlossaryTermPage); await disableEtagConditionalReads(editGlossaryTermPage); await editGlossaryTermPage .context() .storageState({ path: editGlossaryTermFile, indexedDB: true }); - await viewOnlyUser.login(viewOnlyPage); + await viewOnlyUser.signIn(viewOnlyPage); await disableEtagConditionalReads(viewOnlyPage); await viewOnlyPage .context() .storageState({ path: viewOnlyFile, indexedDB: true }); - await ownerUser.login(ownerPage); + await ownerUser.signIn(ownerPage); await disableEtagConditionalReads(ownerPage); await ownerPage .context() diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/fixtures/pages.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/fixtures/pages.ts index 760e2b45bb5b..95a46158ec5c 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/fixtures/pages.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/fixtures/pages.ts @@ -10,97 +10,41 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { Browser, Page } from '@playwright/test'; -import { test as base } from '../../support/fixtures/base'; -import { installServerLoadReducers } from '../../support/fixtures/serverLoad'; -import { disableEtagConditionalReads } from '../../utils/common'; - -// Define the type for our custom fixtures -export type CustomFixtures = { +import { Page } from '@playwright/test'; +import { + test as userPagesTest, + UserPages, +} from '../../support/fixtures/userPages'; + +/** + * The role-page fixtures with the built-in `page` aliased to `adminPage`. + * + * This is the only difference from `support/fixtures/userPages.ts`, which owns + * every role page and all the construction logic. The two used to be + * near-identical copies that had already drifted — one had `viewOnlyPage`, the + * other did not — so the pages themselves now live in exactly one place and this + * module adds only the alias. + * + * Which one to import: + * + * - **this module** when the spec is admin-first and wants `{ page }` to be a + * signed-in admin, optionally taking a role page alongside it; + * - **`support/fixtures/userPages`** when the spec drives named roles explicitly + * and wants `page` left as Playwright's own (so a file-level + * `test.use({ storageState })` still applies). + * + * `page` here ignores `test.use({ storageState })` — it is always the admin + * storage state. That is deliberate and long-standing; use the other module if + * you need `storageState` to win. + */ +export type CustomFixtures = UserPages & { page: Page; - dataConsumerPage: Page; - dataStewardPage: Page; - editDescriptionPage: Page; - editTagsPage: Page; - editGlossaryTermPage: Page; - viewOnlyPage: Page; - ownerPage: Page; -}; - -// Open a role page with conditional reads disabled so fixture-based specs -// always receive fresh entity state. -const openRolePage = async (browser: Browser, storageState: string) => { - const page = await browser.newPage({ storageState }); - await installServerLoadReducers(page.context()); - await disableEtagConditionalReads(page); - - return page; }; -// Create a new test object with our custom fixtures -export const test = base.extend({ - // Admin page as default page value - page: async ({ browser }, use) => { - const adminPage = await openRolePage( - browser, - 'playwright/.auth/admin.json' - ); - +export const test = userPagesTest.extend<{ page: Page }>({ + page: async ({ adminPage }, use) => { await use(adminPage); - await adminPage.close(); - }, - dataConsumerPage: async ({ browser }, use) => { - const page = await openRolePage( - browser, - 'playwright/.auth/dataConsumer.json' - ); - - await use(page); - await page.close(); - }, - dataStewardPage: async ({ browser }, use) => { - const page = await openRolePage( - browser, - 'playwright/.auth/dataSteward.json' - ); - - await use(page); - await page.close(); - }, - ownerPage: async ({ browser }, use) => { - const page = await openRolePage(browser, 'playwright/.auth/owner.json'); - - await use(page); - await page.close(); - }, - editDescriptionPage: async ({ browser }, use) => { - const page = await openRolePage( - browser, - 'playwright/.auth/editDescription.json' - ); - - await use(page); - await page.close(); - }, - editTagsPage: async ({ browser }, use) => { - const page = await openRolePage(browser, 'playwright/.auth/editTags.json'); - - await use(page); - await page.close(); - }, - editGlossaryTermPage: async ({ browser }, use) => { - const page = await openRolePage( - browser, - 'playwright/.auth/editGlossaryTerm.json' - ); - - await use(page); - await page.close(); - }, - viewOnlyPage: async ({ browser }, use) => { - const page = await openRolePage(browser, 'playwright/.auth/viewOnly.json'); - - await use(page); - await page.close(); }, }); + +export { expect } from '@playwright/test'; diff --git a/openmetadata-ui/src/main/resources/ui/playwright/support/fixtures/base.ts b/openmetadata-ui/src/main/resources/ui/playwright/support/fixtures/base.ts index 89bf4a9d23d4..20377ee1d03f 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/support/fixtures/base.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/support/fixtures/base.ts @@ -23,8 +23,9 @@ import { installServerLoadReducers } from './serverLoad'; * * Specs that build their own pages via `browser.newContext()` or * `browser.newPage()` bypass this fixture entirely and call - * `installServerLoadReducers` themselves — see `e2e/fixtures/pages.ts` and - * `support/fixtures/userPages.ts`. + * `installServerLoadReducers` themselves — that is `support/fixtures/userPages.ts`, + * which owns every signed-in role page. (`e2e/fixtures/pages.ts` re-exports those + * and aliases `page` to `adminPage`; it builds no pages of its own.) */ export const test = playwrightTest.extend({ context: async ({ context }, use) => { diff --git a/openmetadata-ui/src/main/resources/ui/playwright/support/fixtures/isolatedUser.ts b/openmetadata-ui/src/main/resources/ui/playwright/support/fixtures/isolatedUser.ts new file mode 100644 index 000000000000..92629684ab22 --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/playwright/support/fixtures/isolatedUser.ts @@ -0,0 +1,179 @@ +/* + * Copyright 2026 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { BrowserContext, Page } from '@playwright/test'; +import { UserClass } from '../../support/user/UserClass'; +import { + disableEtagConditionalReads, + getWorkerAdminAPIContext, +} from '../../utils/common'; +import { test as base } from './base'; +import { installServerLoadReducers } from './serverLoad'; + +/** + * The sanctioned way to run a test as a user that is *not* one of the seeded + * roles. + * + * The seeded role pages in `userPages.ts` cover most specs. When they do not — + * the test needs its own team, its own policy, or it renames or deletes the + * account it is signed in as — the answer is one of these fixtures, **not** a + * hand-rolled `new UserClass()` in `beforeAll` plus `performUserLogin`. + * + * That hand-rolled shape is what these exist to remove, because it fails the + * same three ways every time: + * + * 1. **Lifecycle.** The user is created in `beforeAll` and deleted in + * `afterAll`, usually via an array the hook pushes into. `beforeAll` runs + * once per *group* of the file's tests dispatched to a worker, not once per + * worker, so it can run twice with an `afterAll` in between — and the array + * then holds accounts that no longer exist. That is the merge queue's + * number-one flake. A fixture cannot be got wrong this way: Playwright owns + * the teardown and runs it exactly once per instantiation. + * 2. **Cost.** `UserClass.login()` drives the sign-in form: navigate, fill, + * tab, fill, click, await the response, await the redirect, dismiss the + * getting-started modal, collapse the sidebar. Nine UI interactions on the + * critical path of a test that is not about signing in. These fixtures sign + * in through the API instead (`UserClass.signIn`) — one POST and two + * navigations — so even the per-test variant is cheap. + * 3. **Leaks.** A `beforeAll` that creates and an `afterAll` that misses one + * branch leaves the account behind for the rest of the run. Fixture teardown + * runs even when the test throws. + * + * Which one to take: + * + * - **`isolatedUserPage`** — one account per worker, a fresh page per test. + * Correct whenever the test needs *an* account that is not a seeded role but + * does not modify the account itself. The account is created and signed in + * once per worker; each test gets a page restored from the captured storage + * state. + * - **`freshUserPage`** — a brand-new account per test. Use it whenever the + * test mutates the account it is signed in as (rename, role change, + * deactivate, delete), or whenever you simply want per-test isolation. + * Because the sign-in is an API call rather than a form, this costs a signup + * plus one POST — cheap enough to be a default rather than a last resort. + * + * ```ts + * import { test } from '../../support/fixtures/isolatedUser'; + * + * test.use({ isolatedUserOptions: { isAdmin: true } }); + * + * test('an admin that is not the shared admin can do X', async ({ + * isolatedUserPage, + * isolatedUser, + * }) => { + * await isolatedUserPage.goto(`/users/${isolatedUser.getUserName()}`); + * }); + * ``` + */ +export type IsolatedUserOptions = { + /** Grant the account admin. Defaults to a plain DataConsumer. */ + isAdmin?: boolean; +}; + +type StorageState = Awaited>; + +type IsolatedUserSession = { + user: UserClass; + storageState: StorageState; +}; + +export type IsolatedUserWorkerFixtures = { + isolatedUserOptions: IsolatedUserOptions; + isolatedUserSession: IsolatedUserSession; +}; + +export type IsolatedUserTestFixtures = { + /** The account `isolatedUserPage` is signed in as. */ + isolatedUser: UserClass; + /** A page signed in as the worker's isolated account. */ + isolatedUserPage: Page; + /** A brand-new account and page, for tests that mutate the account itself. */ + freshUserPage: { page: Page; user: UserClass }; +}; + +const preparePage = async (page: Page) => { + await installServerLoadReducers(page.context()); + await disableEtagConditionalReads(page); + + return page; +}; + +export const test = base.extend< + IsolatedUserTestFixtures, + IsolatedUserWorkerFixtures +>({ + isolatedUserOptions: [{}, { option: true, scope: 'worker' }], + + /** + * Sign in once per worker and keep the storage state, rather than driving the + * sign-in form again for every test. The capture must include IndexedDB: the + * app keeps its token under `AppDataStore/keyValueStore/app_state`, not in + * localStorage, so a state captured without it restores a signed-out page. + */ + isolatedUserSession: [ + async ({ browser, isolatedUserOptions }, use) => { + const apiContext = await getWorkerAdminAPIContext(); + const user = new UserClass(undefined, isolatedUserOptions.isAdmin); + await user.create(apiContext); + + const loginPage = await browser.newPage(); + + try { + await installServerLoadReducers(loginPage.context()); + await user.signIn(loginPage); + const storageState = await loginPage + .context() + .storageState({ indexedDB: true }); + + await use({ user, storageState }); + } finally { + await loginPage.close(); + await user.delete(apiContext); + } + }, + { scope: 'worker' }, + ], + + isolatedUser: async ({ isolatedUserSession }, use) => { + await use(isolatedUserSession.user); + }, + + isolatedUserPage: async ({ browser, isolatedUserSession }, use) => { + const page = await preparePage( + await browser.newPage({ storageState: isolatedUserSession.storageState }) + ); + + await use(page); + await page.close(); + }, + + freshUserPage: async ({ browser }, use) => { + const apiContext = await getWorkerAdminAPIContext(); + const user = new UserClass(); + await user.create(apiContext); + + const page = await browser.newPage(); + + try { + await installServerLoadReducers(page.context()); + await user.signIn(page); + await disableEtagConditionalReads(page); + + await use({ page, user }); + } finally { + await page.close(); + await user.delete(apiContext); + } + }, +}); + +export { expect } from '@playwright/test'; diff --git a/openmetadata-ui/src/main/resources/ui/playwright/support/fixtures/userPages.ts b/openmetadata-ui/src/main/resources/ui/playwright/support/fixtures/userPages.ts index dc94a97f394e..b7ef7b9bd33c 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/support/fixtures/userPages.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/support/fixtures/userPages.ts @@ -15,8 +15,38 @@ import { disableEtagConditionalReads } from '../../utils/common'; import { test as base } from './base'; import { installServerLoadReducers } from './serverLoad'; -// Declare the types of your fixtures -type UserPages = { +/** + * The suite's role-page fixtures — the single place a signed-in page for one of + * the pre-seeded roles is built. + * + * Prefer these over creating a user in `beforeAll` and logging it in. The role + * accounts are created once by `e2e/auth.setup.ts` and their storage states are + * reused by every worker, so a spec that takes `dataConsumerPage` costs one + * browser context; a spec that mints its own user costs a signup, a login, a + * page, and a teardown that has to delete the user again — per run, and often + * per test. Reach for a bespoke user only when the test needs something the + * seeded roles cannot express (a specific team, a custom policy, a rename), and + * say so in a comment. + * + * `openmetadata-playwright/prefer-role-page-fixture` (warn) enforces this. + * + * Available roles and what `auth.setup.ts` grants them: + * + * | Fixture | Role | + * |-------------------------|---------------------------------------------| + * | `adminPage` | admin | + * | `dataConsumerPage` | Data Consumer | + * | `dataStewardPage` | Data Steward | + * | `ownerPage` | owner of the seeded entities | + * | `editDescriptionPage` | EditDescription only | + * | `editTagsPage` | EditTags only | + * | `editGlossaryTermPage` | EditGlossaryTerms only | + * | `viewOnlyPage` | view permissions, no edit | + * + * `e2e/fixtures/pages.ts` re-exports these and additionally aliases the built-in + * `page` to `adminPage`; it holds no page-construction logic of its own. + */ +export type UserPages = { adminPage: Page; dataConsumerPage: Page; dataStewardPage: Page; @@ -24,80 +54,58 @@ type UserPages = { editDescriptionPage: Page; editTagsPage: Page; editGlossaryTermPage: Page; + viewOnlyPage: Page; }; -// Extend the base test type with your fixtures -export const test = base.extend({ - adminPage: async ({ browser }: { browser: Browser }, use) => { - const context = await browser.newContext({ - storageState: 'playwright/.auth/admin.json', - }); - await installServerLoadReducers(context); - const page = await context.newPage(); - await disableEtagConditionalReads(page); - await use(page); - await context.close(); - }, - dataConsumerPage: async ({ browser }: { browser: Browser }, use) => { - const context = await browser.newContext({ - storageState: 'playwright/.auth/dataConsumer.json', - }); - await installServerLoadReducers(context); - const page = await context.newPage(); - await disableEtagConditionalReads(page); - await use(page); - await context.close(); - }, - dataStewardPage: async ({ browser }: { browser: Browser }, use) => { - const context = await browser.newContext({ - storageState: 'playwright/.auth/dataSteward.json', - }); - await installServerLoadReducers(context); - const page = await context.newPage(); - await disableEtagConditionalReads(page); - await use(page); - await context.close(); - }, - ownerPage: async ({ browser }: { browser: Browser }, use) => { - const context = await browser.newContext({ - storageState: 'playwright/.auth/owner.json', - }); - await installServerLoadReducers(context); - const page = await context.newPage(); - await disableEtagConditionalReads(page); - await use(page); - await context.close(); - }, - editDescriptionPage: async ({ browser }: { browser: Browser }, use) => { - const context = await browser.newContext({ - storageState: 'playwright/.auth/editDescription.json', - }); - await installServerLoadReducers(context); - const page = await context.newPage(); - await disableEtagConditionalReads(page); - await use(page); - await context.close(); - }, - editTagsPage: async ({ browser }: { browser: Browser }, use) => { - const context = await browser.newContext({ - storageState: 'playwright/.auth/editTags.json', - }); - await installServerLoadReducers(context); - const page = await context.newPage(); - await disableEtagConditionalReads(page); - await use(page); - await context.close(); - }, - editGlossaryTermPage: async ({ browser }: { browser: Browser }, use) => { - const context = await browser.newContext({ - storageState: 'playwright/.auth/editGlossaryTerm.json', - }); - await installServerLoadReducers(context); - const page = await context.newPage(); - await disableEtagConditionalReads(page); +export const ROLE_STORAGE_STATE = { + adminPage: 'playwright/.auth/admin.json', + dataConsumerPage: 'playwright/.auth/dataConsumer.json', + dataStewardPage: 'playwright/.auth/dataSteward.json', + ownerPage: 'playwright/.auth/owner.json', + editDescriptionPage: 'playwright/.auth/editDescription.json', + editTagsPage: 'playwright/.auth/editTags.json', + editGlossaryTermPage: 'playwright/.auth/editGlossaryTerm.json', + viewOnlyPage: 'playwright/.auth/viewOnly.json', +} as const satisfies Record; + +/** + * These pages are built from `browser`, not from the `context` fixture, so they + * bypass the `base.ts` override that installs the server-load reducers — hence + * the explicit call here. `browser.newPage()` owns its context and closes it + * with the page, so no separate context teardown is needed. + * + * Conditional reads are disabled so a fixture-based spec always receives fresh + * entity state rather than a 304 against a stale ETag. + */ +const openRolePage = async (browser: Browser, storageState: string) => { + const page = await browser.newPage({ storageState }); + await installServerLoadReducers(page.context()); + await disableEtagConditionalReads(page); + + return page; +}; + +const roleFixture = + (role: keyof UserPages) => + async ( + { browser }: { browser: Browser }, + use: (page: Page) => Promise + ) => { + const page = await openRolePage(browser, ROLE_STORAGE_STATE[role]); + await use(page); - await context.close(); - }, + await page.close(); + }; + +export const test = base.extend({ + adminPage: roleFixture('adminPage'), + dataConsumerPage: roleFixture('dataConsumerPage'), + dataStewardPage: roleFixture('dataStewardPage'), + ownerPage: roleFixture('ownerPage'), + editDescriptionPage: roleFixture('editDescriptionPage'), + editTagsPage: roleFixture('editTagsPage'), + editGlossaryTermPage: roleFixture('editGlossaryTermPage'), + viewOnlyPage: roleFixture('viewOnlyPage'), }); export { expect } from '@playwright/test'; diff --git a/openmetadata-ui/src/main/resources/ui/playwright/support/user/AdminClass.ts b/openmetadata-ui/src/main/resources/ui/playwright/support/user/AdminClass.ts index 9df2acaf5338..9ad2c787d38c 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/support/user/AdminClass.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/support/user/AdminClass.ts @@ -27,4 +27,13 @@ export class AdminClass extends UserClass { ) { await super.login(page, userName, password, options); } + + async signIn( + page: Page, + userName = DEFAULT_ADMIN_USER.userName, + password = DEFAULT_ADMIN_USER.password, + options: { suppressWelcomeScreen?: boolean } = {} + ): Promise { + return super.signIn(page, userName, password, options); + } } diff --git a/openmetadata-ui/src/main/resources/ui/playwright/support/user/UserClass.ts b/openmetadata-ui/src/main/resources/ui/playwright/support/user/UserClass.ts index c8b780476725..a79b2041972a 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/support/user/UserClass.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/support/user/UserClass.ts @@ -18,6 +18,7 @@ import { SYSTEM_POLICY_NAMES, } from '../../constant/permission'; import { okJson, withNotFoundRetry } from '../../utils/apiResponse'; +import { signInViaApi } from '../../utils/apiSignIn'; import { disableEtagConditionalReads, generateRandomUsername, @@ -297,6 +298,21 @@ export class UserClass { }) .catch(() => undefined); await page.waitForLoadState('domcontentloaded').catch(() => undefined); + + await this.completeSignIn(page); + } + + /** + * Everything a signed-in page owes its caller once the session exists. + * + * Shared by {@link login} and {@link signIn} so the two differ only in how + * the session is established. Call sites depend on all three: conditional + * reads off (so a test sees fresh entity state rather than a 304), the + * getting-started dialog dismissed, and the sidebar collapsed — the last one + * changes page geometry, so a sign-in path that skipped it would move every + * locator in the spec. + */ + protected async completeSignIn(page: Page) { await disableEtagConditionalReads(page); const modal = await page @@ -324,6 +340,37 @@ export class UserClass { } } + /** + * The same signed-in page as {@link login}, established through the API + * instead of the sign-in form. + * + * Prefer this everywhere except a spec that is testing the form itself. + * `login()` performs nine UI interactions before the test has done anything; + * this performs one POST. Everything after the session exists is identical — + * both funnel through {@link completeSignIn} — so swapping a call site over + * changes how the page got signed in and nothing else. + * + * Returns the access token, so callers that also need an API context can + * build one without a second round trip. + */ + async signIn( + page: Page, + userName = this.data.email, + password = this.data.password, + options: { suppressWelcomeScreen?: boolean } = {} + ): Promise { + const token = await signInViaApi(page, { + email: userName, + password, + userName: this.responseData?.name ?? userName, + suppressWelcome: options.suppressWelcomeScreen ?? true, + }); + + await this.completeSignIn(page); + + return token; + } + async logout(page: Page) { await page.getByRole('menuitem', { name: 'Logout' }).click(); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/utils/apiSignIn.ts b/openmetadata-ui/src/main/resources/ui/playwright/utils/apiSignIn.ts new file mode 100644 index 000000000000..d7bb379999f9 --- /dev/null +++ b/openmetadata-ui/src/main/resources/ui/playwright/utils/apiSignIn.ts @@ -0,0 +1,112 @@ +/* + * Copyright 2026 Collate. + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +import { expect, Page, request } from '@playwright/test'; +import { suppressWelcomeScreen } from './common'; +import { setToken } from './tokenStorage'; + +/** + * Sign a page in by asking the API for a token and writing it where the app + * looks, instead of driving the sign-in form. + * + * `UserClass.login()` performs nine UI interactions — navigate to /signin, wait + * for the form, fill the email, Tab, fill the password, click, await the login + * response, await the redirect, dismiss the getting-started modal, collapse the + * sidebar. Every one of them is a step that can time out, and the suite does it + * ~290 times. None of it is what the tests are testing. + * + * This does the same job with one POST and two navigations, out of pieces that + * are already load-bearing elsewhere in the suite: + * + * - the request shape is the one `createAdminApiContext` uses (the password is + * base64, matching the UI's `btoa(password)`); + * - `setToken` is the exact inverse of the `getToken` that `auth.setup.ts` + * reads back after a real sign-in — both operate on `app_state.primary` in + * the `AppDataStore` IndexedDB store, which is where the app's + * `setOidcToken()` puts the access token after a basic-auth login; + * - `suppressWelcomeScreen` is the same call `UserClass.login()` makes. + * + * The navigation to the app origin before writing the token is required, not + * incidental: IndexedDB is origin-scoped, so there is nowhere to write until + * the page has loaded the origin. Unauthenticated it lands on /signin, which is + * the same origin and serves fine as a place to seed from. + * + * The trailing assertion is deliberate. If the injected session is not accepted + * — a changed token key, a refresh token the app has started to require, an + * auth provider that is not basic — this fails here, naming the user, rather + * than letting the test walk on to a signed-out page and time out somewhere + * unrelated 60 seconds later. + */ +export const signInViaApi = async ( + page: Page, + credentials: { + email: string; + password: string; + userName?: string; + /** Seed `loggedInUsers` so the landing-page welcome banner never renders. */ + suppressWelcome?: boolean; + } +): Promise => { + const isH2Mode = process.env.PW_PROTOCOL === 'h2'; + const loginContext = await request.newContext({ + baseURL: + process.env.PLAYWRIGHT_TEST_BASE_URL ?? + (isH2Mode ? 'https://localhost:8585' : 'http://localhost:8585'), + ignoreHTTPSErrors: isH2Mode, + timeout: 90000, + }); + + try { + const response = await loginContext.post('/api/v1/auth/login', { + data: { + email: credentials.email, + password: Buffer.from(credentials.password).toString('base64'), + }, + }); + + if (!response.ok()) { + throw new Error( + `API sign-in failed for "${ + credentials.email + }" (${response.status()}): ${await response.text()}` + ); + } + + const { accessToken } = (await response.json()) as { accessToken: string }; + + if (!accessToken) { + throw new Error( + `API sign-in for "${credentials.email}" returned no accessToken.` + ); + } + + await page.goto('/', { waitUntil: 'domcontentloaded' }); + + if (credentials.suppressWelcome ?? true) { + await suppressWelcomeScreen( + page, + credentials.userName ?? credentials.email + ); + } + await setToken(page, accessToken); + await page.goto('/my-data', { waitUntil: 'domcontentloaded' }); + + await expect( + page.getByTestId('left-sidebar'), + `API sign-in as "${credentials.email}" did not produce a signed-in session — the app shell never rendered. The token was accepted by /api/v1/auth/login but the app did not pick it up from app_state.primary; check utils/tokenStorage.ts against the app's SwTokenStorageUtils.` + ).toBeAttached({ timeout: 30_000 }); + + return accessToken; + } finally { + await loginContext.dispose(); + } +}; diff --git a/openmetadata-ui/src/main/resources/ui/playwright/utils/initialSetup.ts b/openmetadata-ui/src/main/resources/ui/playwright/utils/initialSetup.ts index 26fe54b04a3d..a26bc8defd26 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/utils/initialSetup.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/utils/initialSetup.ts @@ -47,7 +47,7 @@ const initialSetup = async (page: Page) => { }; export const loginAsAdmin = async (page: Page, admin: AdminClass) => { - await admin.login(page); + await admin.signIn(page); await page.waitForURL( (url) => url.pathname === '/' || url.pathname === '/my-data' ); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/utils/user.ts b/openmetadata-ui/src/main/resources/ui/playwright/utils/user.ts index 8ffa5bfb6f0c..6551c2b53247 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/utils/user.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/utils/user.ts @@ -30,7 +30,6 @@ import { descriptionBoxReadOnly, fillDescriptionBox, getAuthContext, - getToken, redirectToHomePage, toastNotification, visitOwnProfilePage, @@ -64,6 +63,16 @@ export const searchUserByEmail = async ( await expect(page.getByTestId(userName)).toBeVisible(); }; +/** + * A signed-in page for `user`, plus an API context authenticated as them. + * + * Signs in through the API rather than the form. The nine UI interactions + * `UserClass.login()` performs are not what any caller of this helper is + * testing, and every one of them is a step that can time out — swapping the + * mechanism here speeds up and de-flakes every call site without any of them + * changing. A spec that is genuinely testing the sign-in *form* should drive + * `UserClass.login()` directly instead of coming through here. + */ export const performUserLogin = async (browser: Browser, user: UserClass) => { const context = await browser.newContext({ storageState: { @@ -73,8 +82,7 @@ export const performUserLogin = async (browser: Browser, user: UserClass) => { }); await installServerLoadReducers(context); const page = await context.newPage(); - await user.login(page); - const token = await getToken(page); + const token = await user.signIn(page); const apiContext = await getAuthContext(token); const afterAction = async () => { await apiContext.dispose(); From 877dac4ee437922effeb0224fbf92019a54a20b8 Mon Sep 17 00:00:00 2001 From: Chirag Madlani <12962843+chirag-madlani@users.noreply.github.com> Date: Tue, 15 Sep 2026 16:19:48 +0530 Subject: [PATCH 2/2] fix(playwright): keep the OM_SESSION cookie when signing in via the API MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `signInViaApi` posted the login from a standalone `request.newContext()`. The server sets `OM_SESSION` on that response, so the cookie landed on a throwaway context and never reached the browser — every storage state `auth.setup.ts` wrote carried `i18next`, `__session` and `versionFetchTime`, but no session cookie. The app does not notice, because it authenticates from the token in IndexedDB. That is exactly what made this invisible locally: every spec passed, including the eight-role fixture check. CI does notice — `PW_PRESEEDED_STATE` rotates the cached auth state through `.github/scripts/rotate_playwright_auth_state.py`, which replaces that cookie and raises ValueError: Playwright auth state has no OM_SESSION cookie before a single test runs. It took out 34 of 37 shards in "Setup Openmetadata Test Environment". Post through `page.context().request` instead, which shares its cookie jar with the browser, so `Set-Cookie` is kept. Route every capture in `auth.setup.ts` through `saveStorageState`, which refuses to write a state without the cookie. A sign-in path that drops it now fails at the point that produced it, naming the cause, instead of surfacing as a shard that dies in environment setup. Verified from a wiped `playwright/.auth`: all eight states carry OM_SESSION, setup runs in 26.5s, and RoleFixtureSessions passes 8/8. Co-Authored-By: Claude Opus 5 (1M context) --- .../resources/ui/playwright/e2e/auth.setup.ts | 58 ++++++------ .../ui/playwright/utils/apiSignIn.ts | 90 +++++++++---------- 2 files changed, 77 insertions(+), 71 deletions(-) diff --git a/openmetadata-ui/src/main/resources/ui/playwright/e2e/auth.setup.ts b/openmetadata-ui/src/main/resources/ui/playwright/e2e/auth.setup.ts index 300be7f334ec..dccb57f8ffb6 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/e2e/auth.setup.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/e2e/auth.setup.ts @@ -10,7 +10,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { test as setup } from '@playwright/test'; +import { Page, test as setup } from '@playwright/test'; import { mkdir, writeFile } from 'fs/promises'; import { EDIT_DESCRIPTION_RULE, @@ -93,6 +93,30 @@ const ownerUser = new UserClass({ password: 'User@OMD123', }); +/** + * Capture a signed-in context's storage state, and fail loudly if it is missing + * the session cookie. + * + * The app authenticates from the token in IndexedDB, so a state without + * `OM_SESSION` still drives every test locally — which is exactly what makes its + * absence easy to ship. CI's `.github/scripts/rotate_playwright_auth_state.py` + * rotates the cached preseeded state by replacing that cookie, and without it + * every shard dies in "Setup Openmetadata Test Environment" with + * "Playwright auth state has no OM_SESSION cookie" — a failure that names + * neither the test nor the sign-in path that dropped it. + */ +const saveStorageState = async (page: Page, path: string) => { + const state = await page.context().storageState({ path, indexedDB: true }); + + if (!state.cookies.some((cookie) => cookie.name === 'OM_SESSION')) { + throw new Error( + `Refusing to write ${path}: the signed-in context has no OM_SESSION cookie, so CI's auth-state rotation would fail. The login request must go through the browser context (page.context().request), not a standalone request context, or the Set-Cookie is discarded.` + ); + } + + return state; +}; + setup('authenticate all users', async ({ browser }) => { setup.setTimeout(120 * 1000); // Create separate pages for each user @@ -243,52 +267,36 @@ setup('authenticate all users', async ({ browser }) => { // Save admin state await disableEtagConditionalReads(newAdminPage); - await newAdminPage - .context() - .storageState({ path: adminFile, indexedDB: true }); + await saveStorageState(newAdminPage, adminFile); // Save states for each user sequentially to avoid file operation conflicts await dataConsumer.signIn(dataConsumerPage); await disableEtagConditionalReads(dataConsumerPage); - await dataConsumerPage - .context() - .storageState({ path: dataConsumerFile, indexedDB: true }); + await saveStorageState(dataConsumerPage, dataConsumerFile); await dataSteward.signIn(dataStewardPage); await disableEtagConditionalReads(dataStewardPage); - await dataStewardPage - .context() - .storageState({ path: dataStewardFile, indexedDB: true }); + await saveStorageState(dataStewardPage, dataStewardFile); await editDescriptionUser.signIn(editDescriptionPage); await disableEtagConditionalReads(editDescriptionPage); - await editDescriptionPage - .context() - .storageState({ path: editDescriptionFile, indexedDB: true }); + await saveStorageState(editDescriptionPage, editDescriptionFile); await editTagsUser.signIn(editTagsPage); await disableEtagConditionalReads(editTagsPage); - await editTagsPage - .context() - .storageState({ path: editTagsFile, indexedDB: true }); + await saveStorageState(editTagsPage, editTagsFile); await editGlossaryTermUser.signIn(editGlossaryTermPage); await disableEtagConditionalReads(editGlossaryTermPage); - await editGlossaryTermPage - .context() - .storageState({ path: editGlossaryTermFile, indexedDB: true }); + await saveStorageState(editGlossaryTermPage, editGlossaryTermFile); await viewOnlyUser.signIn(viewOnlyPage); await disableEtagConditionalReads(viewOnlyPage); - await viewOnlyPage - .context() - .storageState({ path: viewOnlyFile, indexedDB: true }); + await saveStorageState(viewOnlyPage, viewOnlyFile); await ownerUser.signIn(ownerPage); await disableEtagConditionalReads(ownerPage); - await ownerPage - .context() - .storageState({ path: ownerFile, indexedDB: true }); + await saveStorageState(ownerPage, ownerFile); await afterAction(); diff --git a/openmetadata-ui/src/main/resources/ui/playwright/utils/apiSignIn.ts b/openmetadata-ui/src/main/resources/ui/playwright/utils/apiSignIn.ts index d7bb379999f9..21d27e8b33cb 100644 --- a/openmetadata-ui/src/main/resources/ui/playwright/utils/apiSignIn.ts +++ b/openmetadata-ui/src/main/resources/ui/playwright/utils/apiSignIn.ts @@ -10,7 +10,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { expect, Page, request } from '@playwright/test'; +import { expect, Page } from '@playwright/test'; import { suppressWelcomeScreen } from './common'; import { setToken } from './tokenStorage'; @@ -56,57 +56,55 @@ export const signInViaApi = async ( suppressWelcome?: boolean; } ): Promise => { - const isH2Mode = process.env.PW_PROTOCOL === 'h2'; - const loginContext = await request.newContext({ - baseURL: - process.env.PLAYWRIGHT_TEST_BASE_URL ?? - (isH2Mode ? 'https://localhost:8585' : 'http://localhost:8585'), - ignoreHTTPSErrors: isH2Mode, - timeout: 90000, - }); + // Post through the page's own context, not a standalone `request.newContext()`. + // The server sets `OM_SESSION` on the login response, and `context.request` + // shares its cookie jar with the browser — a throwaway context would swallow + // that cookie, leaving a storage state that authenticates through the token in + // IndexedDB but carries no session cookie. The app itself does not mind, which + // is what makes the omission easy to miss locally, but + // `.github/scripts/rotate_playwright_auth_state.py` rotates the cached + // preseeded state by replacing that cookie and fails the whole CI job with + // "Playwright auth state has no OM_SESSION cookie" when it is absent. + const loginContext = page.context().request; - try { - const response = await loginContext.post('/api/v1/auth/login', { - data: { - email: credentials.email, - password: Buffer.from(credentials.password).toString('base64'), - }, - }); + const response = await loginContext.post('/api/v1/auth/login', { + data: { + email: credentials.email, + password: Buffer.from(credentials.password).toString('base64'), + }, + }); - if (!response.ok()) { - throw new Error( - `API sign-in failed for "${ - credentials.email - }" (${response.status()}): ${await response.text()}` - ); - } + if (!response.ok()) { + throw new Error( + `API sign-in failed for "${ + credentials.email + }" (${response.status()}): ${await response.text()}` + ); + } - const { accessToken } = (await response.json()) as { accessToken: string }; + const { accessToken } = (await response.json()) as { accessToken: string }; - if (!accessToken) { - throw new Error( - `API sign-in for "${credentials.email}" returned no accessToken.` - ); - } + if (!accessToken) { + throw new Error( + `API sign-in for "${credentials.email}" returned no accessToken.` + ); + } - await page.goto('/', { waitUntil: 'domcontentloaded' }); + await page.goto('/', { waitUntil: 'domcontentloaded' }); - if (credentials.suppressWelcome ?? true) { - await suppressWelcomeScreen( - page, - credentials.userName ?? credentials.email - ); - } - await setToken(page, accessToken); - await page.goto('/my-data', { waitUntil: 'domcontentloaded' }); + if (credentials.suppressWelcome ?? true) { + await suppressWelcomeScreen( + page, + credentials.userName ?? credentials.email + ); + } + await setToken(page, accessToken); + await page.goto('/my-data', { waitUntil: 'domcontentloaded' }); - await expect( - page.getByTestId('left-sidebar'), - `API sign-in as "${credentials.email}" did not produce a signed-in session — the app shell never rendered. The token was accepted by /api/v1/auth/login but the app did not pick it up from app_state.primary; check utils/tokenStorage.ts against the app's SwTokenStorageUtils.` - ).toBeAttached({ timeout: 30_000 }); + await expect( + page.getByTestId('left-sidebar'), + `API sign-in as "${credentials.email}" did not produce a signed-in session — the app shell never rendered. The token was accepted by /api/v1/auth/login but the app did not pick it up from app_state.primary; check utils/tokenStorage.ts against the app's SwTokenStorageUtils.` + ).toBeAttached({ timeout: 30_000 }); - return accessToken; - } finally { - await loginContext.dispose(); - } + return accessToken; };