From 5f82ef883ae8ab6d9d6a5168c3e0e460dbf4b431 Mon Sep 17 00:00:00 2001 From: Erwan Leboucher Date: Sat, 25 Jul 2026 11:44:38 +0200 Subject: [PATCH 1/2] test: restructure e2e around fixtures and page objects, run against a build --- oxlint.config.ts | 8 +++++ playwright.config.ts | 21 ++++++++--- tests/e2e/fixtures/test.ts | 17 +++++++++ tests/e2e/leave-room-prompt.spec.ts | 8 +++++ tests/e2e/pages/AppShell.ts | 54 +++++++++++++++++++++++++++++ tests/e2e/shell.spec.ts | 34 ++++++------------ 6 files changed, 114 insertions(+), 28 deletions(-) create mode 100644 tests/e2e/fixtures/test.ts create mode 100644 tests/e2e/leave-room-prompt.spec.ts create mode 100644 tests/e2e/pages/AppShell.ts diff --git a/oxlint.config.ts b/oxlint.config.ts index ec935b4da5..24ab29872e 100644 --- a/oxlint.config.ts +++ b/oxlint.config.ts @@ -81,5 +81,13 @@ export default defineConfig({ 'typescript/no-unsafe-enum-comparison': 'off', }, }, + { + // Playwright fixtures take a `use` callback, which the React hook rule + // mistakes for the `use` hook. + files: ['tests/e2e/**'], + rules: { + 'react/rules-of-hooks': 'off', + }, + }, ], }); diff --git a/playwright.config.ts b/playwright.config.ts index 73c623517b..b52db5abcd 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -4,12 +4,18 @@ export default defineConfig({ testDir: './tests/e2e', testMatch: '**/*.spec.ts', snapshotPathTemplate: 'tests/e2e/__screenshots__/{projectName}/{testFileName}/{arg}{ext}', + // Every context shares one Matrix account and device id from the seeded + // storageState, so concurrent clients contend over sync and crypto state. + // Parallelising would need a worker-scoped account fixture; the suite runs in + // well under a minute serially, so it is not worth the plumbing. fullyParallel: false, workers: 1, - retries: 2, + // Locally a failure should surface at once; CI keeps retries for flakes. + retries: process.env.CI ? 2 : 0, reporter: [['html', { open: 'never' }], ['list']], globalSetup: './tests/e2e/global-setup.ts', - timeout: 240_000, + // Tests run in 3-6s against a built app; this is headroom, not a wait budget. + timeout: 60_000, expect: { toHaveScreenshot: { maxDiffPixelRatio: 0.01 }, }, @@ -29,10 +35,15 @@ export default defineConfig({ }, ], webServer: { - command: 'pnpm dev', + // A real build, not the dev server: no HMR and no dependency optimiser, both + // of which invalidate the module graph mid-run and make the suite lie. + command: 'pnpm run build && pnpm exec vite preview --port 8080 --strictPort', url: 'http://localhost:8080', - reuseExistingServer: !process.env.CI, - timeout: 240_000, + // Never reuse: a leftover server would silently serve a stale build. The + // build is ~30s, which is worth paying to know what is under test. + reuseExistingServer: false, + // ~35s observed (31s build + preview start), with headroom for a cold CI box. + timeout: 180_000, env: { NODE_OPTIONS: '--max-old-space-size=8192' }, }, }); diff --git a/tests/e2e/fixtures/test.ts b/tests/e2e/fixtures/test.ts new file mode 100644 index 0000000000..2cf4ae351c --- /dev/null +++ b/tests/e2e/fixtures/test.ts @@ -0,0 +1,17 @@ +import { test as base } from '@playwright/test'; +import { AppShell } from '../pages/AppShell'; + +type Fixtures = { + /** The signed-in app, already loaded and past the device banner. */ + app: AppShell; +}; + +export const test = base.extend({ + app: async ({ page }, use) => { + const app = new AppShell(page); + await app.open(); + await use(app); + }, +}); + +export { expect } from '@playwright/test'; diff --git a/tests/e2e/leave-room-prompt.spec.ts b/tests/e2e/leave-room-prompt.spec.ts new file mode 100644 index 0000000000..85a771e0ee --- /dev/null +++ b/tests/e2e/leave-room-prompt.spec.ts @@ -0,0 +1,8 @@ +import { test, expect } from './fixtures/test'; + +test('leave room prompt opens from the room options menu', async ({ app }) => { + const menu = await app.openRoomOptions('General'); + await menu.leaveRoom(); + + await expect(app.leaveRoomPrompt).toBeVisible(); +}); diff --git a/tests/e2e/pages/AppShell.ts b/tests/e2e/pages/AppShell.ts new file mode 100644 index 0000000000..071dc358dc --- /dev/null +++ b/tests/e2e/pages/AppShell.ts @@ -0,0 +1,54 @@ +import type { Locator, Page } from '@playwright/test'; +import { expect } from '@playwright/test'; + +/** + * How long the Matrix client may take to sync and paint the room list. One + * definition so a cold start and a warm one are held to the same bar. + */ +export const CLIENT_READY_TIMEOUT = 30_000; + +/** The signed-in app: room list, rail, and the menus reachable from them. */ +export class AppShell { + readonly leaveRoomPrompt: Locator; + + readonly createRoomButton: Locator; + + constructor(readonly page: Page) { + this.leaveRoomPrompt = page.getByText('Are you sure you want to leave this room?'); + this.createRoomButton = page.getByRole('button', { name: 'Create Room' }).first(); + } + + /** Loads the app and waits until the seeded rooms are on screen. */ + async open(): Promise { + await this.page.goto('/'); + await expect(this.room('General')).toBeVisible({ timeout: CLIENT_READY_TIMEOUT }); + await this.dismissDeviceBanner(); + } + + room(name: string): Locator { + return this.page.getByText(name).first(); + } + + /** The unverified-device banner is not always present, so this is best effort. */ + async dismissDeviceBanner(): Promise { + await this.page + .getByRole('button', { name: 'Dismiss' }) + .click({ timeout: 5_000 }) + .catch(() => undefined); + } + + async openRoomOptions(name: string): Promise { + await this.room(name).hover(); + await this.page.getByRole('button', { name: 'More Options' }).first().click(); + return new RoomOptionsMenu(this.page); + } +} + +export class RoomOptionsMenu { + constructor(readonly page: Page) {} + + async leaveRoom(): Promise { + // The mobile sheet can overflow the viewport, where a real pointer click fails. + await this.page.getByRole('button', { name: 'Leave Room' }).dispatchEvent('click'); + } +} diff --git a/tests/e2e/shell.spec.ts b/tests/e2e/shell.spec.ts index 938a551b1d..48c449e554 100644 --- a/tests/e2e/shell.spec.ts +++ b/tests/e2e/shell.spec.ts @@ -1,41 +1,29 @@ -import { test, expect, type Page } from '@playwright/test'; +import type { Page } from '@playwright/test'; +import { test, expect } from './fixtures/test'; const containerised = Boolean(process.env.PW_TEST_CONNECT_WS_ENDPOINT); -async function dismissDeviceBanner(page: Page): Promise { - await page - .getByRole('button', { name: 'Dismiss' }) - .click({ timeout: 5_000 }) - .catch(() => undefined); -} - const maskDynamic = (page: Page) => ({ mask: [page.locator('time'), page.getByText(/^Created by /)], }); test.describe('app shell', () => { - test('shows seeded rooms after login', async ({ page }) => { - await page.goto('/'); - - await expect(page.getByText('General').first()).toBeVisible({ timeout: 180_000 }); - await expect(page.getByText('Random').first()).toBeVisible(); + test('shows seeded rooms after login', async ({ app }) => { + await expect(app.room('General')).toBeVisible(); + await expect(app.room('Random')).toBeVisible(); }); - test('opens a room and renders its timeline', async ({ page }) => { - await page.goto('/'); - await dismissDeviceBanner(page); + test('opens a room and renders its timeline', async ({ app, page }) => { + await app.room('General').click(); - await page.getByText('General').first().click(); - await expect(page.getByText('Welcome to the test room.')).toBeVisible({ timeout: 180_000 }); + await expect(page.getByText('Welcome to the test room.')).toBeVisible(); await expect(page.getByText('Layout baseline seed message.')).toBeVisible(); }); - test('matches the shell layout baseline', async ({ page }) => { + // `app` is requested so the fixture loads the shell before the snapshot. + test('matches the shell layout baseline', async ({ app, page }) => { test.skip(!containerised, 'run via pnpm test:e2e:docker'); - - await page.goto('/'); - await expect(page.getByText('General').first()).toBeVisible({ timeout: 180_000 }); - await dismissDeviceBanner(page); + await expect(app.room('General')).toBeVisible(); await page.evaluate(async () => { await document.fonts.ready; From 0f211ab68ffb19782e26b800ee605edd1de042e3 Mon Sep 17 00:00:00 2001 From: Erwan Leboucher Date: Sat, 25 Jul 2026 17:13:33 +0200 Subject: [PATCH 2/2] fix(room-nav): keep the mobile options sheet mounted while it is open --- .changeset/room-nav-mobile-options-sheet.md | 5 +++++ oxlint.config.ts | 2 -- playwright.config.ts | 11 ----------- src/app/features/room-nav/RoomNavItem.tsx | 2 +- tests/e2e/fixtures/test.ts | 1 - tests/e2e/pages/AppShell.ts | 10 +--------- tests/e2e/shell.spec.ts | 1 - 7 files changed, 7 insertions(+), 25 deletions(-) create mode 100644 .changeset/room-nav-mobile-options-sheet.md diff --git a/.changeset/room-nav-mobile-options-sheet.md b/.changeset/room-nav-mobile-options-sheet.md new file mode 100644 index 0000000000..4d4ae16c0f --- /dev/null +++ b/.changeset/room-nav-mobile-options-sheet.md @@ -0,0 +1,5 @@ +--- +default: patch +--- + +Keep the mobile room options sheet open instead of dismissing it as soon as the pointer leaves the room. diff --git a/oxlint.config.ts b/oxlint.config.ts index 24ab29872e..97f04f35b4 100644 --- a/oxlint.config.ts +++ b/oxlint.config.ts @@ -82,8 +82,6 @@ export default defineConfig({ }, }, { - // Playwright fixtures take a `use` callback, which the React hook rule - // mistakes for the `use` hook. files: ['tests/e2e/**'], rules: { 'react/rules-of-hooks': 'off', diff --git a/playwright.config.ts b/playwright.config.ts index b52db5abcd..f5a9587a89 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -4,17 +4,11 @@ export default defineConfig({ testDir: './tests/e2e', testMatch: '**/*.spec.ts', snapshotPathTemplate: 'tests/e2e/__screenshots__/{projectName}/{testFileName}/{arg}{ext}', - // Every context shares one Matrix account and device id from the seeded - // storageState, so concurrent clients contend over sync and crypto state. - // Parallelising would need a worker-scoped account fixture; the suite runs in - // well under a minute serially, so it is not worth the plumbing. fullyParallel: false, workers: 1, - // Locally a failure should surface at once; CI keeps retries for flakes. retries: process.env.CI ? 2 : 0, reporter: [['html', { open: 'never' }], ['list']], globalSetup: './tests/e2e/global-setup.ts', - // Tests run in 3-6s against a built app; this is headroom, not a wait budget. timeout: 60_000, expect: { toHaveScreenshot: { maxDiffPixelRatio: 0.01 }, @@ -35,14 +29,9 @@ export default defineConfig({ }, ], webServer: { - // A real build, not the dev server: no HMR and no dependency optimiser, both - // of which invalidate the module graph mid-run and make the suite lie. command: 'pnpm run build && pnpm exec vite preview --port 8080 --strictPort', url: 'http://localhost:8080', - // Never reuse: a leftover server would silently serve a stale build. The - // build is ~30s, which is worth paying to know what is under test. reuseExistingServer: false, - // ~35s observed (31s build + preview start), with headroom for a cold CI box. timeout: 180_000, env: { NODE_OPTIONS: '--max-old-space-size=8192' }, }, diff --git a/src/app/features/room-nav/RoomNavItem.tsx b/src/app/features/room-nav/RoomNavItem.tsx index 99c14b70c3..d2c255d749 100644 --- a/src/app/features/room-nav/RoomNavItem.tsx +++ b/src/app/features/room-nav/RoomNavItem.tsx @@ -439,7 +439,7 @@ export function RoomNavItem({ navigate(linkPath); }; - const optionsVisible = hover || !!menuAnchor; + const optionsVisible = hover || !!menuAnchor || isMobileMenuOpen; const isMutedRoom = notificationMode === RoomNotificationMode.Mute; const shouldShowUnreadIndicator = !isMutedRoom && (!!unread || hasRoomUnread); diff --git a/tests/e2e/fixtures/test.ts b/tests/e2e/fixtures/test.ts index 2cf4ae351c..fc016d1204 100644 --- a/tests/e2e/fixtures/test.ts +++ b/tests/e2e/fixtures/test.ts @@ -2,7 +2,6 @@ import { test as base } from '@playwright/test'; import { AppShell } from '../pages/AppShell'; type Fixtures = { - /** The signed-in app, already loaded and past the device banner. */ app: AppShell; }; diff --git a/tests/e2e/pages/AppShell.ts b/tests/e2e/pages/AppShell.ts index 071dc358dc..8626b37943 100644 --- a/tests/e2e/pages/AppShell.ts +++ b/tests/e2e/pages/AppShell.ts @@ -1,13 +1,8 @@ import type { Locator, Page } from '@playwright/test'; import { expect } from '@playwright/test'; -/** - * How long the Matrix client may take to sync and paint the room list. One - * definition so a cold start and a warm one are held to the same bar. - */ export const CLIENT_READY_TIMEOUT = 30_000; -/** The signed-in app: room list, rail, and the menus reachable from them. */ export class AppShell { readonly leaveRoomPrompt: Locator; @@ -18,7 +13,6 @@ export class AppShell { this.createRoomButton = page.getByRole('button', { name: 'Create Room' }).first(); } - /** Loads the app and waits until the seeded rooms are on screen. */ async open(): Promise { await this.page.goto('/'); await expect(this.room('General')).toBeVisible({ timeout: CLIENT_READY_TIMEOUT }); @@ -29,7 +23,6 @@ export class AppShell { return this.page.getByText(name).first(); } - /** The unverified-device banner is not always present, so this is best effort. */ async dismissDeviceBanner(): Promise { await this.page .getByRole('button', { name: 'Dismiss' }) @@ -48,7 +41,6 @@ export class RoomOptionsMenu { constructor(readonly page: Page) {} async leaveRoom(): Promise { - // The mobile sheet can overflow the viewport, where a real pointer click fails. - await this.page.getByRole('button', { name: 'Leave Room' }).dispatchEvent('click'); + await this.page.getByRole('button', { name: 'Leave Room' }).click(); } } diff --git a/tests/e2e/shell.spec.ts b/tests/e2e/shell.spec.ts index 48c449e554..fd4c697bac 100644 --- a/tests/e2e/shell.spec.ts +++ b/tests/e2e/shell.spec.ts @@ -20,7 +20,6 @@ test.describe('app shell', () => { await expect(page.getByText('Layout baseline seed message.')).toBeVisible(); }); - // `app` is requested so the fixture loads the shell before the snapshot. test('matches the shell layout baseline', async ({ app, page }) => { test.skip(!containerised, 'run via pnpm test:e2e:docker'); await expect(app.room('General')).toBeVisible();