refactor(playwright): sign in through a fixture, not the form

.claude/rules/frontend-playwright.md

@@ -35,8 +35,9 @@ Highest-value constraints, all machine-enforced:
   `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.
+  POST and runs the identical post-sign-in steps. The rule is at **error** with no suppressions —
+  only the nine specs that are testing the sign-in flow itself carry a justified disable. Creating a
+  user as *test data* is fine; signing one in through the form is what the rule flags.
 - **`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.

.github/playwright/impact-map.generated.json

@@ -2160,6 +2160,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",
@@ -4434,6 +4435,7 @@
         "openmetadata-ui/src/main/resources/ui/playwright/utils/tokenStorage.ts"
       ],
       "specs": [
+        "playwright/e2e/Features/TokenStorage.spec.ts",
         "playwright/e2e/Flow/IngestionBot.spec.ts"
       ]
     },
@@ -7003,6 +7005,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"
       ]
@@ -7768,6 +7771,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",
@@ -9884,6 +9888,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",

openmetadata-ui/src/main/resources/ui/eslint-rules/openmetadata-playwright.mjs

@@ -107,8 +107,108 @@ const requireAggregationWaitHelper = {
   },
 };
 
+/**
+ * `UserClass.login()` drives the sign-in form: navigate to /signin, wait for it,
+ * fill, Tab, fill, click, await the response, await the redirect, dismiss the
+ * getting-started dialog, collapse the sidebar. Nine UI interactions on the
+ * critical path of a test that is not about signing in, each of them a step that
+ * can time out.
+ *
+ * `UserClass.signIn()` establishes the same session with one POST, and both
+ * funnel through the same `completeSignIn`, so the two differ in how the session
+ * was established and nothing else. Every sanctioned path — the seeded role
+ * pages, the `isolatedUser` fixtures, and `performUserLogin` — goes through it.
+ *
+ * So `login()` in a spec means one of two things: the spec is testing the form
+ * itself, which is legitimate and wants a justified disable, or it is a call
+ * that has not been migrated yet.
+ *
+ * Scope, stated plainly: this flags *authenticating as* a bespoke user, not
+ * *creating* one. `new UserClass()` for an owner, reviewer or assignee is
+ * ordinary test data and is untouched — flagging it would bury the signal.
+ * The implementation modules that must log in (auth.setup, the fixtures, the
+ * login helpers) are exempt by path.
+ *
+ * It runs at `warn`: there are ~290 existing call sites, and a rule whose
+ * baseline is most of the corpus teaches nothing. Fix them as you touch them.
+ */
+const ROLE_FIXTURES = [
+  'adminPage',
+  'dataConsumerPage',
+  'dataStewardPage',
+  'ownerPage',
+  'editDescriptionPage',
+  'editTagsPage',
+  'editGlossaryTermPage',
+  'viewOnlyPage',
+];
+
+/** Modules that build the storage states, or are the login path itself. */
+const LOGIN_IMPLEMENTATION_PATHS = [
+  'e2e/auth.setup.ts',
+  'support/fixtures/userPages.ts',
+  'support/fixtures/isolatedUser.ts',
+  'e2e/fixtures/pages.ts',
+  'utils/user.ts',
+  'utils/apiSignIn.ts',
+  'utils/admin.ts',
+  'support/user/',
+];
+
+const preferRolePageFixture = {
+  meta: {
+    docs: {
+      description:
+        'Prefer the shared role page fixtures over creating and logging in a bespoke user',
+    },
+    messages: {
+      preferRolePageFixture:
+        '`login()` drives the sign-in form — nine UI interactions before this test has done anything. Use `signIn()` instead: same session, same post-sign-in steps, one POST. Better still, take a fixture and let it own the account: one of {{fixtures}} from support/fixtures/userPages (or e2e/fixtures/pages) for a seeded role, or `isolatedUserPage` / `freshUserPage` from support/fixtures/isolatedUser when the test needs its own account — those create and delete it for you, so there is no beforeAll/afterAll bookkeeping to get wrong. If this spec is testing the sign-in form itself, keep `login()` and disable this rule with a reason.',
+    },
+    schema: [],
+    type: 'suggestion',
+  },
+  create(context) {
+    const filename = context.filename.replace(/\\/g, '/');
+
+    if (LOGIN_IMPLEMENTATION_PATHS.some((path) => filename.includes(path))) {
+      return {};
+    }
+
+    const report = (node) =>
+      context.report({
+        node,
+        messageId: 'preferRolePageFixture',
+        data: { fixtures: ROLE_FIXTURES.join(', ') },
+      });
+
+    return {
+      CallExpression(node) {
+        const { callee } = node;
+
+        // `<user>.login(<page>, ...)` — the UserClass login, which takes the
+        // page first and may carry trailing options. Requiring a first argument
+        // that is not an object literal keeps an unrelated `login({ ... })` on
+        // some other client, which takes options rather than a page, out of it.
+        const isUserLogin =
+          callee.type === 'MemberExpression' &&
+          !callee.computed &&
+          callee.property.type === 'Identifier' &&
+          callee.property.name === 'login' &&
+          node.arguments.length > 0 &&
+          node.arguments[0].type !== 'ObjectExpression';
+
+        if (isUserLogin) {
+          report(node);
+        }
+      },
+    };
+  },
+};
+
 export default {
   rules: {
     'require-aggregation-wait-helper': requireAggregationWaitHelper,
+    'prefer-role-page-fixture': preferRolePageFixture,
   },
 };

openmetadata-ui/src/main/resources/ui/eslint-rules/openmetadata-playwright.test.mjs

@@ -10,9 +10,9 @@
  *  See the License for the specific language governing permissions and
  *  limitations under the License.
  */
+import { RuleTester } from 'eslint';
 import assert from 'node:assert/strict';
 import test, { describe, it } from 'node:test';
-import { RuleTester } from 'eslint';
 import tseslint from 'typescript-eslint';
 
 RuleTester.describe = describe;
@@ -25,6 +25,10 @@ test('exports the aggregation wait helper rule', () => {
   assert.ok(playwrightPlugin.rules['require-aggregation-wait-helper']);
 });
 
+test('exports the role page fixture rule', () => {
+  assert.ok(playwrightPlugin.rules['prefer-role-page-fixture']);
+});
+
 const ruleTester = new RuleTester({
   languageOptions: {
     ecmaVersion: 'latest',
@@ -131,3 +135,79 @@ ruleTester.run(
     ],
   }
 );
+
+ruleTester.run(
+  'prefer-role-page-fixture',
+  playwrightPlugin.rules['prefer-role-page-fixture'],
+  {
+    valid: [
+      {
+        // Taking a role page is the point of the rule.
+        code: "test('x', async ({ dataConsumerPage }) => { await dataConsumerPage.goto('/'); });",
+        filename: 'playwright/e2e/Flow/Example.spec.ts',
+      },
+      {
+        // Creating a user as *test data* — an owner, a reviewer, an assignee —
+        // has nothing to do with authenticating as one.
+        code: 'const owner = new UserClass(); await owner.create(apiContext);',
+        filename: 'playwright/e2e/Flow/Example.spec.ts',
+      },
+      {
+        // auth.setup.ts is what mints the storage states the fixtures reuse.
+        code: 'await dataConsumer.login(dataConsumerPage);',
+        filename: 'playwright/e2e/auth.setup.ts',
+      },
+      {
+        // The fixture modules and the login helper itself are the implementation.
+        code: 'await user.login(page);',
+        filename: 'playwright/utils/user.ts',
+      },
+      {
+        // performUserLogin signs in through the API and owns the page, the
+        // context and their teardown — it is a sanctioned path, not a
+        // hand-rolled login.
+        code: 'const { page, afterAction } = await performUserLogin(browser, user);',
+        filename: 'playwright/e2e/Flow/Example.spec.ts',
+      },
+      {
+        // The migrated shape.
+        code: 'await user.signIn(page);',
+        filename: 'playwright/e2e/Flow/Example.spec.ts',
+      },
+      {
+        // The isolated-user fixtures are the sanctioned bespoke-account path,
+        // so they are the one place that legitimately drives the sign-in form.
+        code: 'await user.login(loginPage);',
+        filename: 'playwright/support/fixtures/isolatedUser.ts',
+      },
+      {
+        // Taking the sanctioned bespoke-account fixture.
+        code: "test('x', async ({ isolatedUserPage }) => { await isolatedUserPage.goto('/'); });",
+        filename: 'playwright/e2e/Flow/Example.spec.ts',
+      },
+      {
+        // A method that merely shares the name on an unrelated object.
+        code: "await ssoProvider.login({ user: 'x' });",
+        filename: 'playwright/e2e/Flow/Example.spec.ts',
+      },
+    ],
+    invalid: [
+      {
+        code: 'await regularUser.login(page);',
+        errors: [{ messageId: 'preferRolePageFixture' }],
+        filename: 'playwright/e2e/Pages/Example.spec.ts',
+      },
+      {
+        code: 'await user.login(await browser.newPage());',
+        errors: [{ messageId: 'preferRolePageFixture' }],
+        filename: 'playwright/e2e/Pages/Example.spec.ts',
+      },
+      {
+        // Trailing options do not make it any less a bespoke login.
+        code: 'await user.login(page, undefined, undefined, { skipTour: true });',
+        errors: [{ messageId: 'preferRolePageFixture' }],
+        filename: 'playwright/e2e/Flow/Example.spec.ts',
+      },
+    ],
+  }
+);

openmetadata-ui/src/main/resources/ui/eslint.config.mjs

@@ -635,6 +635,7 @@ export default [
       // rather than error while the remaining 27 call sites are migrated to
       // playwright/utils/searchAggregation.ts.
       'openmetadata-playwright/require-aggregation-wait-helper': 'warn',
+      'openmetadata-playwright/prefer-role-page-fixture': 'error',
 
       // Playwright rules — promoted to error behind the suppressions ratchet
       // (see eslint-suppressions.json): existing violations are snapshotted,

openmetadata-ui/src/main/resources/ui/playwright/PLAYWRIGHT_DEVELOPER_HANDBOOK.md

@@ -322,10 +322,18 @@ Two things to know if you touch this path:
   `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.
+`openmetadata-playwright/prefer-role-page-fixture` runs at **error** with no suppressions: the
+whole corpus was migrated, so a `login()` in a spec is either new code that should be using
+`signIn()` or a fixture, or a spec that is genuinely testing the sign-in form. It flags
+*authenticating as* a bespoke user, not *creating* one — `new UserClass()` for an owner, reviewer
+or assignee is ordinary test data and is untouched, as is `performUserLogin`, which signs in
+through the API and owns its page, context and teardown.
+
+Nine files legitimately keep `login()` because the sign-in flow is what they are testing —
+`Pages/Login.spec.ts`, `Features/OnlineUsers.spec.ts`, `Flow/Tour.spec.ts` and the `Features/AppMode`
+specs. Each call there carries a justified disable
+(`// eslint-disable-next-line openmetadata-playwright/prefer-role-page-fixture -- <why>`) naming the
+reason. If you find yourself adding a tenth, check first that `signIn()` really cannot do the job.
 
 ---
 
@@ -907,6 +915,7 @@ not hand-edit it, run `yarn generate:playwright-rules` instead.
 | `om-playwright/no-blanket-test-slow` | error | Disallow test.slow() at file or describe scope |
 | `om-playwright/no-positional-locator` | error | Disallow positional locators (.first(), .last(), .nth()) |
 | `om-playwright/require-assertion-per-test` | error | Flag tests that only perform page interactions and verify nothing |
+| `openmetadata-playwright/prefer-role-page-fixture` | error | Prefer the shared role page fixtures over creating and logging in a bespoke user |
 | `openmetadata-playwright/require-aggregation-wait-helper` | warn | Require waitForAggregation instead of waiting on search/aggregate directly |
 | `playwright/missing-playwright-await` | error | Identify false positives when async Playwright APIs are not properly awaited. |
 | `playwright/no-element-handle` | error | The use of ElementHandle is discouraged, use Locator instead |

openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/ActivityFeed.spec.ts

@@ -157,7 +157,7 @@ test.describe('FeedWidget on landing page', () => {
 
         // Set up widget in a separate page context
         const adminPage = await browser.newPage({ storageState: undefined });
-        await adminUser.login(adminPage);
+        await adminUser.signIn(adminPage);
 
         try {
           // Set persona as default
@@ -214,7 +214,7 @@ test.describe('FeedWidget on landing page', () => {
   });
 
   test.beforeEach(async ({ page }) => {
-    await adminUser.login(page);
+    await adminUser.signIn(page);
     await redirectToHomePage(page);
     await waitForAllLoadersToDisappear(page);
   });
@@ -502,13 +502,13 @@ test.describe('Mention notifications in Notification Box', () => {
   }>({
     adminPage: async ({ browser }, use) => {
       const page = await browser.newPage({ storageState: undefined });
-      await adminUser.login(page);
+      await adminUser.signIn(page);
       await use(page);
       await page.close();
     },
     user1Page: async ({ browser }, use) => {
       const page = await browser.newPage({ storageState: undefined });
-      await user1.login(page);
+      await user1.signIn(page);
       await use(page);
       await page.close();
     },
@@ -830,7 +830,7 @@ test.describe('Mentions: Chinese character encoding in activity feed', () => {
   });
 
   test.beforeEach(async ({ page }) => {
-    await adminUser.login(page);
+    await adminUser.signIn(page);
     await redirectToHomePage(page);
   });
 
@@ -967,7 +967,7 @@ test.describe('ActivityFeed: activity + conversation merge (regression #25894)',
   const test = base.extend<{ adminPage: Page }>({
     adminPage: async ({ browser }, use) => {
       const page = await browser.newPage({ storageState: undefined });
-      await adminUser.login(page);
+      await adminUser.signIn(page);
       await use(page);
       await page.close();
     },

openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/ActivityStream.spec.ts

@@ -64,7 +64,7 @@ test.describe('Activity Stream on Entity Pages', () => {
   });
 
   test.beforeEach(async ({ page }) => {
-    await adminUser.login(page);
+    await adminUser.signIn(page);
   });
 
   test('activity feed tab shows activity events for entity', async ({

openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/AppMode/AppModeAiPersonaLandsAtRoot.spec.ts

@@ -163,6 +163,7 @@ test.describe('AppMode — landing URL', { tag: ['@Platform'] }, () => {
           }
         });
 
+        // eslint-disable-next-line openmetadata-playwright/prefer-role-page-fixture -- app-mode precedence is asserted across real sign-in sessions, so the session must be established the way a user establishes it
         await user.login(page);
         await waitForAllLoadersToDisappear(page);
 

openmetadata-ui/src/main/resources/ui/playwright/e2e/Features/AppMode/AppModeAuthGating.spec.ts

@@ -47,6 +47,7 @@ test.beforeAll(
 
     const contextA = await browser.newContext();
     const pageA = await contextA.newPage();
+    // eslint-disable-next-line openmetadata-playwright/prefer-role-page-fixture -- app-mode precedence is asserted across real sign-in sessions, so the session must be established the way a user establishes it
     await userA.login(pageA);
     const resultA = await getApiContext(pageA);
     userAApiContext = resultA.apiContext;
@@ -57,6 +58,7 @@ test.beforeAll(
 
     const contextB = await browser.newContext();
     const pageB = await contextB.newPage();
+    // eslint-disable-next-line openmetadata-playwright/prefer-role-page-fixture -- app-mode precedence is asserted across real sign-in sessions, so the session must be established the way a user establishes it
     await userB.login(pageB);
     const resultB = await getApiContext(pageB);
     userBApiContext = resultB.apiContext;