From fb1857624453fea3a8fed26e02cf1b1a7aaec5fb Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Fri, 26 Jun 2026 18:21:48 +0200 Subject: [PATCH] refactor(tests): cleanup PlayWright tests reuse fixtures where possible to deduplicate logic. Signed-off-by: Ferdinand Thiessen --- .../e2e/settings/personal-info.spec.ts | 27 ++++------- .../e2e/users/users-disable.spec.ts | 30 ++++-------- .../e2e/users/users-group-admin.spec.ts | 10 +--- .../playwright/e2e/users/users-groups.spec.ts | 48 ++++++------------- .../e2e/users/users-manager.spec.ts | 15 ++---- .../playwright/e2e/users/users-modify.spec.ts | 13 +---- .../support/fixtures/admin-with-user.ts | 14 ++++++ .../support/fixtures/random-user-session.ts | 21 ++++---- .../support/fixtures/random-user.ts | 22 +++++++++ .../support/sections/SettingsUsersPage.ts | 18 +++++++ tests/playwright/support/utils/users.ts | 33 ++++++++++++- 11 files changed, 139 insertions(+), 112 deletions(-) create mode 100644 tests/playwright/support/fixtures/admin-with-user.ts create mode 100644 tests/playwright/support/fixtures/random-user.ts diff --git a/tests/playwright/e2e/settings/personal-info.spec.ts b/tests/playwright/e2e/settings/personal-info.spec.ts index 26d29910b90df..3612dc67ec6ef 100644 --- a/tests/playwright/e2e/settings/personal-info.spec.ts +++ b/tests/playwright/e2e/settings/personal-info.spec.ts @@ -3,12 +3,11 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ -import type { User } from '@nextcloud/e2e-test-server' import type { Page, Response } from '@playwright/test' import { runOcc } from '@nextcloud/e2e-test-server/docker' -import { createRandomUser, login } from '@nextcloud/e2e-test-server/playwright' -import { test as baseTest, expect } from '@playwright/test' +import { expect } from '@playwright/test' +import { test as userSessionTest } from '../../support/fixtures/random-user-session.ts' import { handlePasswordConfirmation } from '../../support/utils/password-confirmation.ts' // ── Visibility scope labels exactly as rendered in the UI ───────────────────── @@ -45,23 +44,13 @@ async function changeVisibility(page: Page, property: string, scope: Visibility, // ── Fixture ─────────────────────────────────────────────────────────────────── -const test = baseTest.extend<{ user: User }>({ - user: async ({}, use) => { - const user = await createRandomUser() - // Ensure English UI language and locale so string assertions are stable - await runOcc(['user:setting', user.userId, 'core', 'lang', 'en']) - await runOcc(['user:setting', user.userId, 'core', 'locale', 'en_US']) - await use(user) - await runOcc(['user:delete', user.userId]) +// Ensure English UI language and locale so string assertions are stable +const test = userSessionTest.extend({ + user: async ({ user: baseUser }, use) => { + await runOcc(['user:setting', baseUser.userId, 'core', 'lang', 'en']) + await runOcc(['user:setting', baseUser.userId, 'core', 'locale', 'en_US']) + await use(baseUser) }, - - page: async ({ browser, user }, use) => { - const page = await browser.newPage() - await login(page.request, user) - await use(page) - await page.close() - }, - }) // ── Spec ────────────────────────────────────────────────────────────────────── diff --git a/tests/playwright/e2e/users/users-disable.spec.ts b/tests/playwright/e2e/users/users-disable.spec.ts index a32e798ef90a5..6c2202b35110c 100644 --- a/tests/playwright/e2e/users/users-disable.spec.ts +++ b/tests/playwright/e2e/users/users-disable.spec.ts @@ -3,36 +3,26 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ -import { type User } from '@nextcloud/e2e-test-server' import { runOcc } from '@nextcloud/e2e-test-server/docker' -import { createRandomUser } from '@nextcloud/e2e-test-server/playwright' import { expect } from '@playwright/test' -import { test as adminTest } from '../../support/fixtures/admin-session.ts' +import { test } from '../../support/fixtures/admin-with-user.ts' import { SettingsUsersPage } from '../../support/sections/SettingsUsersPage.ts' -const test = adminTest.extend<{ testUser: User }>({ - testUser: async ({}, use) => { - const user = await createRandomUser() - await use(user) - await runOcc(['user:delete', user.userId]) - }, -}) - test.describe('Settings: Disable and enable users', () => { - test('can disable a user', async ({ page, testUser }) => { + test('can disable a user', async ({ page, user }) => { // Ensure user is enabled - await runOcc(['user:enable', testUser.userId]) + await runOcc(['user:enable', user.userId]) const settingsPage = new SettingsUsersPage(page) await settingsPage.open() - await expect(settingsPage.userRow(testUser.userId)).toBeVisible() + await expect(settingsPage.userRow(user.userId)).toBeVisible() - await settingsPage.openActionsMenu(testUser.userId) + await settingsPage.openActionsMenu(user.userId) await page.getByRole('menuitem', { name: 'Disable account' }).click() // User should no longer be in the main list - await expect(settingsPage.userRow(testUser.userId)).toHaveCount(0) + await expect(settingsPage.userRow(user.userId)).toHaveCount(0) // Disabled accounts nav link should now appear const disabledLink = settingsPage.navigation().getByRole('link', { name: /Disabled accounts/i }) @@ -44,12 +34,12 @@ test.describe('Settings: Disable and enable users', () => { // The disabled user should be in the list await settingsPage.userList().waitFor({ state: 'visible' }) - await expect(settingsPage.userRow(testUser.userId)).toBeVisible() + await expect(settingsPage.userRow(user.userId)).toBeVisible() }) - test('can enable a user', async ({ page, testUser }) => { + test('can enable a user', async ({ page, user }) => { // Ensure user is disabled - await runOcc(['user:disable', testUser.userId]) + await runOcc(['user:disable', user.userId]) const settingsPage = new SettingsUsersPage(page) await settingsPage.open() @@ -62,7 +52,7 @@ test.describe('Settings: Disable and enable users', () => { await settingsPage.userList().waitFor({ state: 'visible' }) const waitForEnableRequest = page.waitForResponse((r) => r.request().url().match(/\/ocs\/v2\.php\/cloud\/users\/[^/]+\/enable/) !== null) - await settingsPage.openActionsMenu(testUser.userId) + await settingsPage.openActionsMenu(user.userId) await page.getByRole('menuitem', { name: 'Enable account' }).click() await waitForEnableRequest diff --git a/tests/playwright/e2e/users/users-group-admin.spec.ts b/tests/playwright/e2e/users/users-group-admin.spec.ts index f624c61e49ac8..ace1dd4bd9976 100644 --- a/tests/playwright/e2e/users/users-group-admin.spec.ts +++ b/tests/playwright/e2e/users/users-group-admin.spec.ts @@ -10,6 +10,7 @@ import { createRandomUser, login } from '@nextcloud/e2e-test-server/playwright' import { test as baseTest, expect } from '@playwright/test' import { SettingsUsersPage } from '../../support/sections/SettingsUsersPage.ts' import { handlePasswordConfirmation } from '../../support/utils/password-confirmation.ts' +import { makeSubAdmin } from '../../support/utils/users.ts' const test = baseTest.extend<{ subadmin: User, group: string }>({ group: async ({}, use) => { @@ -21,14 +22,7 @@ const test = baseTest.extend<{ subadmin: User, group: string }>({ subadmin: async ({ group, request }, use) => { const user = await createRandomUser() await runOcc(['group:adduser', group, user.userId]) - // Grant subadmin rights via OCS API authenticated as admin - await request.post(`/ocs/v2.php/cloud/users/${user.userId}/subadmins`, { - headers: { - 'OCS-APIRequest': 'true', - Authorization: 'Basic ' + Buffer.from('admin:admin').toString('base64'), - }, - form: { groupid: group }, - }) + await makeSubAdmin(request, user.userId, group) await use(user) await runOcc(['user:delete', user.userId]) }, diff --git a/tests/playwright/e2e/users/users-groups.spec.ts b/tests/playwright/e2e/users/users-groups.spec.ts index b3f5418258bb2..cc40b7bf984b5 100644 --- a/tests/playwright/e2e/users/users-groups.spec.ts +++ b/tests/playwright/e2e/users/users-groups.spec.ts @@ -8,7 +8,7 @@ import type { User } from '@nextcloud/e2e-test-server' import { runOcc } from '@nextcloud/e2e-test-server/docker' import { createRandomUser } from '@nextcloud/e2e-test-server/playwright' import { expect } from '@playwright/test' -import { test } from '../../support/fixtures/admin-session.ts' +import { test } from '../../support/fixtures/admin-with-user.ts' import { SettingsUsersPage } from '../../support/sections/SettingsUsersPage.ts' import { handlePasswordConfirmation } from '../../support/utils/password-confirmation.ts' @@ -37,12 +37,7 @@ test('Account Management: Can create a group', async ({ page }) => { // ── Assign user to group ────────────────────────────────────────────────────── -const userGroupTest = test.extend<{ testUser: User, testGroup: string }>({ - async testUser({}, use) { - const testUser = await createRandomUser() - await use(testUser) - await runOcc(['user:delete', testUser.userId]) - }, +const userGroupTest = test.extend<{ testGroup: string }>({ async testGroup({}, use) { const testGroup = crypto.randomUUID() await runOcc(['group:add', testGroup]) @@ -51,19 +46,19 @@ const userGroupTest = test.extend<{ testUser: User, testGroup: string }>({ }, }) -userGroupTest('Account Management: Assign user to a group', async ({ page, testGroup, testUser }) => { +userGroupTest('Account Management: Assign user to a group', async ({ page, testGroup, user }) => { const settingsPage = new SettingsUsersPage(page) await settingsPage.open() // group is in the list with no members await expect(settingsPage.groupListItem(testGroup)).toBeVisible() // Counter bubble is absent when member count is 0 - await expect(settingsPage.groupListItem(testGroup).locator('.counter-bubble__counter')).toHaveCount(0) + await expect(settingsPage.groupMemberCount(testGroup)).toHaveCount(0) // user is in the list - await expect(settingsPage.userRow(testUser.userId)).toBeVisible() + await expect(settingsPage.userRow(user.userId)).toBeVisible() // can assign the group via the edit dialog - await settingsPage.openEditDialog(testUser.userId) + await settingsPage.openEditDialog(user.userId) const dialog = settingsPage.editUserDialog() const groupsCombobox = dialog.getByRole('combobox', { name: /Member of the following groups/i }) const searchRequest = page.waitForResponse((r) => r.request().url().match(new RegExp('/ocs/v2\\.php/cloud/groups/details\\?(.+&|)search=' + testGroup.slice(0, 5))) !== null) @@ -77,9 +72,9 @@ userGroupTest('Account Management: Assign user to a group', async ({ page, testG await expect(page.getByText(/Account updated/i)).toBeVisible() // user is now group now shows 1 member - await expect(settingsPage.groupListItem(testGroup).locator('.counter-bubble__counter')).toHaveText('1') + await expect(settingsPage.groupMemberCount(testGroup)).toHaveText('1') // backend confirms the user is in the group - const info = JSON.parse(await runOcc(['user:info', '--output=json', testUser.userId])) + const info = JSON.parse(await runOcc(['user:info', '--output=json', user.userId])) expect(info?.groups).toContain(testGroup) }) @@ -100,13 +95,9 @@ test.describe('Settings: Delete an empty group', () => { const settingsPage = new SettingsUsersPage(page) await settingsPage.open() - const groupItem = settingsPage.groupListItem(groupName) - await expect(groupItem).toBeVisible() + await expect(settingsPage.groupListItem(groupName)).toBeVisible() - // Open the group's actions menu - await groupItem.hover() - await expect(groupItem.getByRole('button', { name: /Actions/i })).toBeVisible() - await groupItem.getByRole('button', { name: /Actions/i }).click() + await settingsPage.openGroupActionsMenu(groupName) // and delete the group await page.getByRole('button', { name: 'Delete group' }).click() @@ -143,13 +134,9 @@ test.describe('Settings: Delete a non-empty group', () => { const settingsPage = new SettingsUsersPage(page) await settingsPage.open() - const groupItem = settingsPage.groupListItem(groupName) - await expect(groupItem).toBeVisible() + await expect(settingsPage.groupListItem(groupName)).toBeVisible() - // Open the group's actions menu - await groupItem.hover() - expect(groupItem.getByRole('button', { name: /Actions/i })).toBeVisible() - await groupItem.getByRole('button', { name: /Actions/i }).click() + await settingsPage.openGroupActionsMenu(groupName) // and delete the group await page.getByRole('button', { name: 'Delete group' }).click() @@ -164,24 +151,19 @@ test.describe('Settings: Delete a non-empty group', () => { }) // ── Sort groups ─────────────────────────────────────────────────────────────── -const sortGroupsTest = test.extend<{ testUser: User, testGroups: [string, string] }>({ - async testGroups({ testUser }, use) { +const sortGroupsTest = test.extend<{ testGroups: [string, string] }>({ + async testGroups({ user }, use) { const suffix = crypto.randomUUID().slice(0, 8) const groupA = `A-${suffix}` const groupB = `B-${suffix}` await runOcc(['group:add', groupA]) await runOcc(['group:add', groupB]) - await runOcc(['group:adduser', groupB, testUser.userId]) + await runOcc(['group:adduser', groupB, user.userId]) await use([groupA, groupB]) await runOcc(['group:delete', groupA]).catch(() => {}) await runOcc(['group:delete', groupB]).catch(() => {}) }, - testUser: async ({}, use) => { - const testUser = await createRandomUser() - await use(testUser) - await runOcc(['user:delete', testUser.userId]) - }, }) sortGroupsTest('Settings: Sort groups by member count and then by name', async ({ page, testGroups }) => { diff --git a/tests/playwright/e2e/users/users-manager.spec.ts b/tests/playwright/e2e/users/users-manager.spec.ts index 66dca603fe669..f71484828eabe 100644 --- a/tests/playwright/e2e/users/users-manager.spec.ts +++ b/tests/playwright/e2e/users/users-manager.spec.ts @@ -7,20 +7,15 @@ import { type User } from '@nextcloud/e2e-test-server' import { runOcc } from '@nextcloud/e2e-test-server/docker' import { createRandomUser } from '@nextcloud/e2e-test-server/playwright' import { expect } from '@playwright/test' -import { test as adminTest } from '../../support/fixtures/admin-session.ts' +import { test as adminUserTest } from '../../support/fixtures/admin-with-user.ts' import { SettingsUsersPage } from '../../support/sections/SettingsUsersPage.ts' import { handlePasswordConfirmation } from '../../support/utils/password-confirmation.ts' -const test = adminTest.extend<{ user: User, manager: User }>({ - user: async ({}, use) => { - const u = await createRandomUser() - await use(u) - await runOcc(['user:delete', u.userId]) - }, +const test = adminUserTest.extend<{ manager: User }>({ manager: async ({}, use) => { - const u = await createRandomUser() - await use(u) - await runOcc(['user:delete', u.userId]) + const manager = await createRandomUser() + await use(manager) + await runOcc(['user:delete', manager.userId]).catch(() => {}) }, }) diff --git a/tests/playwright/e2e/users/users-modify.spec.ts b/tests/playwright/e2e/users/users-modify.spec.ts index fb09956245f69..0af595bc05709 100644 --- a/tests/playwright/e2e/users/users-modify.spec.ts +++ b/tests/playwright/e2e/users/users-modify.spec.ts @@ -3,22 +3,13 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ -import { type User } from '@nextcloud/e2e-test-server' import { runOcc } from '@nextcloud/e2e-test-server/docker' -import { createRandomUser, login } from '@nextcloud/e2e-test-server/playwright' +import { login } from '@nextcloud/e2e-test-server/playwright' import { expect } from '@playwright/test' -import { test as adminTest } from '../../support/fixtures/admin-session.ts' +import { test } from '../../support/fixtures/admin-with-user.ts' import { SettingsUsersPage } from '../../support/sections/SettingsUsersPage.ts' import { handlePasswordConfirmation } from '../../support/utils/password-confirmation.ts' -const test = adminTest.extend<{ user: User }>({ - user: async ({}, use) => { - const user = await createRandomUser() - await use(user) - await runOcc(['user:delete', user.userId]) - }, -}) - test.describe('Settings: Change user properties', () => { test('can change the display name', async ({ page, user }) => { const settingsPage = new SettingsUsersPage(page) diff --git a/tests/playwright/support/fixtures/admin-with-user.ts b/tests/playwright/support/fixtures/admin-with-user.ts new file mode 100644 index 0000000000000..ec8421dfe15a5 --- /dev/null +++ b/tests/playwright/support/fixtures/admin-with-user.ts @@ -0,0 +1,14 @@ +/* + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import { mergeTests } from '@playwright/test' +import { test as adminTest } from './admin-session.ts' +import { test as randomUserTest } from './random-user.ts' + +/** + * Admin session combined with a freshly-created random `user` fixture. + * The page is logged in as admin; the user is available via the `user` fixture. + */ +export const test = mergeTests(adminTest, randomUserTest) diff --git a/tests/playwright/support/fixtures/random-user-session.ts b/tests/playwright/support/fixtures/random-user-session.ts index 8b3951a17aabe..dc350a7eb3da5 100644 --- a/tests/playwright/support/fixtures/random-user-session.ts +++ b/tests/playwright/support/fixtures/random-user-session.ts @@ -3,17 +3,18 @@ * SPDX-License-Identifier: AGPL-3.0-or-later */ -import { runOcc } from '@nextcloud/e2e-test-server/docker' -import { createRandomUser, login } from '@nextcloud/e2e-test-server/playwright' -import { test as baseTest } from '@playwright/test' - -export const test = baseTest.extend({ - page: async ({ page, context }, use) => { - const user = await createRandomUser() - await login(context.request, user) +import { login } from '@nextcloud/e2e-test-server/playwright' +import { test as randomUserTest } from './random-user.ts' +/** + * Extends the random-user fixture with a `page` logged in as that user. + * The page runs in an isolated browser context — no admin session leaks in. + */ +export const test = randomUserTest.extend({ + page: async ({ browser, user }, use) => { + const page = await browser.newPage() + await login(page.request, user) await use(page) - - await runOcc(['user:delete', user.userId]) + await page.close() }, }) diff --git a/tests/playwright/support/fixtures/random-user.ts b/tests/playwright/support/fixtures/random-user.ts new file mode 100644 index 0000000000000..ad465e4e2b00e --- /dev/null +++ b/tests/playwright/support/fixtures/random-user.ts @@ -0,0 +1,22 @@ +/* + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import type { User } from '@nextcloud/e2e-test-server' + +import { runOcc } from '@nextcloud/e2e-test-server/docker' +import { createRandomUser } from '@nextcloud/e2e-test-server/playwright' +import { test as baseTest } from '@playwright/test' + +/** + * Extends the base test with a freshly-created random `user`. + * The user is deleted in teardown regardless of test outcome. + */ +export const test = baseTest.extend<{ user: User }>({ + user: async ({}, use) => { + const user = await createRandomUser() + await use(user) + await runOcc(['user:delete', user.userId]).catch(() => {}) + }, +}) diff --git a/tests/playwright/support/sections/SettingsUsersPage.ts b/tests/playwright/support/sections/SettingsUsersPage.ts index 56c0d6fc4d68a..a129287df55e1 100644 --- a/tests/playwright/support/sections/SettingsUsersPage.ts +++ b/tests/playwright/support/sections/SettingsUsersPage.ts @@ -91,6 +91,24 @@ export class SettingsUsersPage { await dialog.waitFor({ state: 'hidden' }) } + /** + * Open the NcActions flyout for a custom group nav item. + * NcActions inside NcAppNavigationItem falls back to the default `t('Actions')` label. + */ + async openGroupActionsMenu(groupName: string): Promise { + const item = this.groupListItem(groupName) + await item.hover() + await item.getByRole('button', { name: /Actions/i }).click() + } + + /** + * Locator for the member count inside a group list item. + * NcCounterBubble has no accessible role — CSS class is a last resort. + */ + groupMemberCount(groupName: string): Locator { + return this.groupListItem(groupName).locator('.counter-bubble__counter') + } + /** Open the actions dropdown for `userId`. */ async openActionsMenu(userId: string): Promise { const button = this.userRow(userId).getByRole('button', { name: 'Toggle account actions menu' }) diff --git a/tests/playwright/support/utils/users.ts b/tests/playwright/support/utils/users.ts index a484145050f7d..5da4707fa9b7f 100644 --- a/tests/playwright/support/utils/users.ts +++ b/tests/playwright/support/utils/users.ts @@ -5,6 +5,34 @@ import type { APIRequestContext } from '@playwright/test' +/** + * Grant subadmin rights to `subadminId` over `groupId` via the OCS Provisioning API. + * Uses admin Basic Auth because this call is typically made in fixture setup before + * any browser session exists. + * + * @param request - A Playwright APIRequestContext (global or browser-level) + * @param subadminId - The user id to promote + * @param groupId - The group id to grant subadmin rights over + */ +export async function makeSubAdmin( + request: APIRequestContext, + subadminId: string, + groupId: string, +): Promise { + const response = await request.post(`/ocs/v2.php/cloud/users/${subadminId}/subadmins`, { + headers: { + Accept: 'application/json', + 'OCS-APIRequest': 'true', + Authorization: 'Basic ' + Buffer.from('admin:admin').toString('base64'), + }, + form: { groupid: groupId }, + }) + const meta = (await response.json()).ocs?.meta + if (meta?.statuscode !== 200 && meta?.statuscode !== 100) { + throw new Error(`makeSubAdmin(${subadminId}, ${groupId}) failed: ${meta?.statuscode} ${meta?.message}`) + } +} + /** * Set a user's display name through the OCS Provisioning API. A user may edit * their own display name, so `request` must be authenticated as `userId`. @@ -19,7 +47,10 @@ export async function setUserDisplayName( displayName: string, ): Promise { const response = await request.put(`/ocs/v2.php/cloud/users/${userId}?format=json`, { - headers: { 'OCS-APIRequest': 'true' }, + headers: { + Accept: 'application/json', + 'OCS-APIRequest': 'true', + }, form: { key: 'display', value: displayName }, }) // OCS returns HTTP 200 even on failure; the real status lives in ocs.meta