From 180ea374a0a30c80368cc6452d7ffd3df12b992e Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Thu, 29 Oct 2026 23:44:24 +0100 Subject: [PATCH 01/11] test(systemtags): only delete the tags a test created System tags are server-global, but the systemtags specs cleaned up by deleting every tag on the instance (`clearTags()` in `afterAll`, and a mass delete in `beforeEach` of the admin settings spec, which also used the fixed names `foo` / `bar`). A spec running in parallel lost its tags mid-test, and the non-forced delete threw when a tag vanished underneath it. Generate tag names through a `tagName` fixture that deletes exactly the tags created under those names after each test. Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Ferdinand Thiessen --- .../admin-settings-restrictions.spec.ts | 7 ++-- .../e2e/systemtags/admin-settings.spec.ts | 36 ++++++++--------- .../e2e/systemtags/files-bulk-action.spec.ts | 40 +++++++++---------- .../systemtags/files-inline-action.spec.ts | 21 +++++----- .../e2e/systemtags/files-sidebar.spec.ts | 7 +--- .../e2e/systemtags/files-view.spec.ts | 7 +--- .../support/fixtures/systemtags-files-page.ts | 6 ++- tests/playwright/support/utils/systemtags.ts | 23 +++++++++-- 8 files changed, 77 insertions(+), 70 deletions(-) diff --git a/tests/playwright/e2e/systemtags/admin-settings-restrictions.spec.ts b/tests/playwright/e2e/systemtags/admin-settings-restrictions.spec.ts index 0cc1eea013b3b..6f396df965430 100644 --- a/tests/playwright/e2e/systemtags/admin-settings-restrictions.spec.ts +++ b/tests/playwright/e2e/systemtags/admin-settings-restrictions.spec.ts @@ -7,11 +7,10 @@ import { runOcc } from '@nextcloud/e2e-test-server' import { expect } from '@playwright/test' import { test } from '../../support/fixtures/systemtags-files-page.ts' import { uploadContent } from '../../support/utils/dav.ts' -import { clearTags, createTag } from '../../support/utils/systemtags.ts' +import { createTag } from '../../support/utils/systemtags.ts' test.beforeAll(async () => await runOcc(['config:app:set', 'systemtags', 'restrict_creation_to_admin', '--value', '1'])) test.afterAll(async () => await runOcc(['config:app:delete', 'systemtags', 'restrict_creation_to_admin'])) -test.afterAll(async () => await clearTags()) test.beforeEach(async ({ filesListPage, page, user }) => { await uploadContent(page.request, user, Buffer.alloc(0), 'text/plain', '/file1.txt') @@ -19,8 +18,8 @@ test.beforeEach(async ({ filesListPage, page, user }) => { await filesListPage.open() }) -test('Cannot create tag if restriction is in place', async ({ filesListPage }) => { - const tag = crypto.randomUUID() +test('Cannot create tag if restriction is in place', async ({ filesListPage, tagName }) => { + const tag = tagName() await createTag(tag, 'public') await filesListPage.expectInlineTagsForFile('file1.txt', []) diff --git a/tests/playwright/e2e/systemtags/admin-settings.spec.ts b/tests/playwright/e2e/systemtags/admin-settings.spec.ts index cf796fe127002..29d4382f781aa 100644 --- a/tests/playwright/e2e/systemtags/admin-settings.spec.ts +++ b/tests/playwright/e2e/systemtags/admin-settings.spec.ts @@ -4,21 +4,19 @@ */ import { expect } from '@playwright/test' -import { test } from '../../support/fixtures/admin-session.ts' -import { createTag, deleteTag, listTags } from '../../support/utils/systemtags.ts' +import { test as adminTest } from '../../support/fixtures/admin-session.ts' +import { createTag, tagNameFixture } from '../../support/utils/systemtags.ts' -const tagName = 'foo' -const updatedTagName = 'bar' +const test = adminTest.extend<{ tagName: () => string }>({ + tagName: tagNameFixture, +}) -test.describe('System tags admin settings', () => { - test.beforeEach(async () => { - const tags = await listTags() - for (const tag of tags) { - await deleteTag(tag.id) - } - }) +// NcEllipsisedOption splits names of 10+ chars across two spans, which breaks the +// option's accessible name, so options are matched on their text content instead. - test('Can create a tag', async ({ page }) => { +test.describe('System tags admin settings', () => { + test('Can create a tag', async ({ page, tagName: generateTagName }) => { + const tagName = generateTagName() await page.goto('settings/admin/server') // Scroll the collaborative tags section into view — the admin settings page is long @@ -35,10 +33,12 @@ test.describe('System tags admin settings', () => { // The form resets after creation — verify the tag now appears in the selection dropdown await page.getByRole('combobox', { name: 'Search for a tag to edit' }).click() - await expect(page.getByRole('option', { name: tagName })).toBeVisible() + await expect(page.getByRole('option').filter({ hasText: tagName })).toBeVisible() }) - test('Can update a tag', async ({ page }) => { + test('Can update a tag', async ({ page, tagName: generateTagName }) => { + const tagName = generateTagName() + const updatedTagName = generateTagName() await createTag(tagName) await page.goto('settings/admin/server') @@ -46,7 +46,7 @@ test.describe('System tags admin settings', () => { // Select the tag to edit await page.getByRole('combobox', { name: 'Search for a tag to edit' }).click() - await page.getByRole('option', { name: tagName }).click() + await page.getByRole('option').filter({ hasText: tagName }).click() // Verify the form reflects the selected tag await expect(page.getByLabel('Tag name')).toHaveValue(tagName) @@ -65,13 +65,11 @@ test.describe('System tags admin settings', () => { expect((await updateResponse).status()).toBe(207) await page.getByRole('combobox', { name: 'Search for a tag to edit' }).click() - // NcEllipsisedOption splits names ≥ 10 chars across two spans, breaking the accessible name. - // "bar (invisible)" (15 chars) splits at position 8 → accessible name "bar (inv isible)". - // Use filter({ hasText }) to match on text content instead of the exact accessible name. await expect(page.getByRole('option').filter({ hasText: updatedTagName })).toBeVisible() }) - test('Can delete a tag', async ({ page }) => { + test('Can delete a tag', async ({ page, tagName: generateTagName }) => { + const tagName = generateTagName() await createTag(tagName) await page.goto('settings/admin/server') diff --git a/tests/playwright/e2e/systemtags/files-bulk-action.spec.ts b/tests/playwright/e2e/systemtags/files-bulk-action.spec.ts index 7614d8c2c9dfb..9749bf3610fa2 100644 --- a/tests/playwright/e2e/systemtags/files-bulk-action.spec.ts +++ b/tests/playwright/e2e/systemtags/files-bulk-action.spec.ts @@ -5,7 +5,7 @@ import { test as baseTest } from '../../support/fixtures/systemtags-files-page.ts' import { uploadContent } from '../../support/utils/dav.ts' -import { assignTagsToFile, clearTags, createTag } from '../../support/utils/systemtags.ts' +import { assignTagsToFile, createTag } from '../../support/utils/systemtags.ts' // Extends the base fixture with per-test file IDs so tests in parallel each get // their own isolated file IDs rather than sharing module-level mutable state. @@ -19,11 +19,9 @@ const test = baseTest.extend<{ fileIds: [string, string] }>({ }) test.describe('Systemtags: Files bulk action', () => { - test.afterAll(async () => await clearTags()) - // eslint-disable-next-line @typescript-eslint/no-unused-vars -- Needed to execute the upload by PlayWright - test('Can assign tag to selection', async ({ filesListPage, fileIds }) => { - const tag = crypto.randomUUID() + test('Can assign tag to selection', async ({ filesListPage, fileIds, tagName }) => { + const tag = tagName() await filesListPage.expectInlineTagsForFile('file1.txt', []) await filesListPage.expectInlineTagsForFile('file2.txt', []) @@ -39,9 +37,9 @@ test.describe('Systemtags: Files bulk action', () => { await filesListPage.expectInlineTagsForFile('file2.txt', [tag]) }) - test('Can assign multiple tags to selection', async ({ filesListPage }) => { - const tag1 = crypto.randomUUID() - const tag2 = crypto.randomUUID() + test('Can assign multiple tags to selection', async ({ filesListPage, tagName }) => { + const tag1 = tagName() + const tag2 = tagName() await filesListPage.expectInlineTagsForFile('file1.txt', []) await filesListPage.expectInlineTagsForFile('file2.txt', []) @@ -58,9 +56,9 @@ test.describe('Systemtags: Files bulk action', () => { await filesListPage.expectInlineTagsForFile('file2.txt', [tag1, tag2]) }) - test('Can remove tag from selection', async ({ filesListPage, page, fileIds }) => { - const tag1 = crypto.randomUUID() - const tag2 = crypto.randomUUID() + test('Can remove tag from selection', async ({ filesListPage, page, fileIds, tagName }) => { + const tag1 = tagName() + const tag2 = tagName() await assignTagsToFile(fileIds[0], [tag1, tag2]) await assignTagsToFile(fileIds[1], [tag1, tag2]) await page.reload() @@ -79,10 +77,10 @@ test.describe('Systemtags: Files bulk action', () => { await filesListPage.expectInlineTagsForFile('file2.txt', [tag1]) }) - test('Can remove multiple tags from selection', async ({ filesListPage, page, fileIds }) => { - const tag1 = crypto.randomUUID() - const tag2 = crypto.randomUUID() - const tag3 = crypto.randomUUID() + test('Can remove multiple tags from selection', async ({ filesListPage, page, fileIds, tagName }) => { + const tag1 = tagName() + const tag2 = tagName() + const tag3 = tagName() await assignTagsToFile(fileIds[0], [tag1, tag2, tag3]) await assignTagsToFile(fileIds[1], [tag1, tag2, tag3]) await page.reload() @@ -102,10 +100,10 @@ test.describe('Systemtags: Files bulk action', () => { await filesListPage.expectInlineTagsForFile('file2.txt', [tag1]) }) - test('Can assign and remove multiple tags', async ({ filesListPage, page, fileIds }) => { - const tag1 = crypto.randomUUID() - const tag2 = crypto.randomUUID() - const tag3 = crypto.randomUUID() + test('Can assign and remove multiple tags', async ({ filesListPage, page, fileIds, tagName }) => { + const tag1 = tagName() + const tag2 = tagName() + const tag3 = tagName() await assignTagsToFile(fileIds[0], [tag1, tag2]) await assignTagsToFile(fileIds[1], [tag1, tag2]) await page.reload() @@ -125,8 +123,8 @@ test.describe('Systemtags: Files bulk action', () => { await filesListPage.expectInlineTagsForFile('file2.txt', [tag1, tag3]) }) - test('Can search for tags with insensitive case', async ({ filesListPage }) => { - const tag = crypto.randomUUID().toLowerCase() + test('Can search for tags with insensitive case', async ({ filesListPage, tagName }) => { + const tag = tagName() await createTag(tag, 'public') await filesListPage.expectInlineTagsForFile('file1.txt', []) diff --git a/tests/playwright/e2e/systemtags/files-inline-action.spec.ts b/tests/playwright/e2e/systemtags/files-inline-action.spec.ts index a2383af3f15e7..2bb647e2cbe1a 100644 --- a/tests/playwright/e2e/systemtags/files-inline-action.spec.ts +++ b/tests/playwright/e2e/systemtags/files-inline-action.spec.ts @@ -5,18 +5,15 @@ import { expect, test } from '../../support/fixtures/systemtags-files-page.ts' import { uploadContent } from '../../support/utils/dav.ts' -import { clearTags } from '../../support/utils/systemtags.ts' test.describe('Systemtags: Files integration', () => { - test.afterAll(async () => await clearTags()) - test.beforeEach(async ({ page, user, filesListPage }) => { await uploadContent(page.request, user, Buffer.alloc(0), 'text/plain', '/file.txt') await filesListPage.open() }) - test('See first assigned tag in the file list', async ({ page, filesListPage }) => { - const tag = crypto.randomUUID() + test('See first assigned tag in the file list', async ({ page, filesListPage, tagName }) => { + const tag = tagName() await filesListPage.openTagPickerForFile('file.txt') await filesListPage.createNewTagInPicker(tag) @@ -29,9 +26,9 @@ test.describe('Systemtags: Files integration', () => { await expect(tagList.getByRole('listitem')).toContainText(tag) }) - test('See two assigned tags are also shown in the file list', async ({ page, filesListPage }) => { - const tag1 = crypto.randomUUID() - const tag2 = crypto.randomUUID() + test('See two assigned tags are also shown in the file list', async ({ page, filesListPage, tagName }) => { + const tag1 = tagName() + const tag2 = tagName() await filesListPage.openTagPickerForFile('file.txt') await filesListPage.createNewTagInPicker(tag1) @@ -46,10 +43,10 @@ test.describe('Systemtags: Files integration', () => { await expect(tagList).toContainText(tag2) }) - test('See three assigned tags result in overflow entry', async ({ page, filesListPage }) => { - const tag1 = crypto.randomUUID() - const tag2 = crypto.randomUUID() - const tag3 = crypto.randomUUID() + test('See three assigned tags result in overflow entry', async ({ page, filesListPage, tagName }) => { + const tag1 = tagName() + const tag2 = tagName() + const tag3 = tagName() await filesListPage.openTagPickerForFile('file.txt') await filesListPage.createNewTagInPicker(tag1) diff --git a/tests/playwright/e2e/systemtags/files-sidebar.spec.ts b/tests/playwright/e2e/systemtags/files-sidebar.spec.ts index 3decfaf866d0d..0ae72e4448e4e 100644 --- a/tests/playwright/e2e/systemtags/files-sidebar.spec.ts +++ b/tests/playwright/e2e/systemtags/files-sidebar.spec.ts @@ -5,18 +5,15 @@ import { expect, test } from '../../support/fixtures/systemtags-files-page.ts' import { uploadContent } from '../../support/utils/dav.ts' -import { clearTags } from '../../support/utils/systemtags.ts' test.describe('Systemtags: Files sidebar integration', () => { - test.afterAll(async () => await clearTags()) - test.beforeEach(async ({ page, user, filesListPage }) => { await uploadContent(page.request, user, Buffer.alloc(0), 'text/plain', '/file.txt') await filesListPage.open() }) - test('Can assign tags using the sidebar', async ({ filesListPage, filesSidebar }) => { - const tag = crypto.randomUUID() + test('Can assign tags using the sidebar', async ({ filesListPage, filesSidebar, tagName }) => { + const tag = tagName() await expect(filesListPage.getRowForFile('file.txt')).toBeVisible() diff --git a/tests/playwright/e2e/systemtags/files-view.spec.ts b/tests/playwright/e2e/systemtags/files-view.spec.ts index dc2fe681c8da3..9a686b4517fc5 100644 --- a/tests/playwright/e2e/systemtags/files-view.spec.ts +++ b/tests/playwright/e2e/systemtags/files-view.spec.ts @@ -5,19 +5,16 @@ import { expect, test } from '../../support/fixtures/systemtags-files-page.ts' import { mkdir, uploadContent } from '../../support/utils/dav.ts' -import { clearTags } from '../../support/utils/systemtags.ts' test.describe('Systemtags: Files view', () => { - test.afterAll(async () => await clearTags()) - test.beforeEach(async ({ page, user, filesListPage }) => { await mkdir(page.request, user, '/folder') await uploadContent(page.request, user, Buffer.alloc(0), 'text/plain', '/file.txt') await filesListPage.open() }) - test('See first assigned tag in the file list', async ({ page, filesListPage }) => { - const tag = crypto.randomUUID() + test('See first assigned tag in the file list', async ({ page, filesListPage, tagName }) => { + const tag = tagName() await expect(filesListPage.getRowForFile('folder')).toBeVisible() await expect(filesListPage.getRowForFile('file.txt')).toBeVisible() diff --git a/tests/playwright/support/fixtures/systemtags-files-page.ts b/tests/playwright/support/fixtures/systemtags-files-page.ts index 894ea1c029989..543fe6906d7ff 100644 --- a/tests/playwright/support/fixtures/systemtags-files-page.ts +++ b/tests/playwright/support/fixtures/systemtags-files-page.ts @@ -4,21 +4,25 @@ */ import { SystemTagsFilesListPage } from '../sections/SystemTagsFilesListPage.ts' +import { tagNameFixture } from '../utils/systemtags.ts' import { test as filesTest } from './files-page.ts' type SystemTagsFixtures = { filesListPage: SystemTagsFilesListPage + tagName: () => string } /** * Extends the base files-page fixture by replacing `filesListPage` with a * {@link SystemTagsFilesListPage}, which adds SystemTagPicker actions and - * inline-tags assertion helpers on top of the standard file list interactions. + * inline-tags assertion helpers on top of the standard file list interactions, + * and provides `tagName` to generate tag names that are cleaned up after the test. */ export const test = filesTest.extend({ filesListPage: async ({ page }, use) => { await use(new SystemTagsFilesListPage(page)) }, + tagName: tagNameFixture, }) export { expect } from '../matchers.ts' diff --git a/tests/playwright/support/utils/systemtags.ts b/tests/playwright/support/utils/systemtags.ts index 3c19b8c70bd3a..82233c938554a 100644 --- a/tests/playwright/support/utils/systemtags.ts +++ b/tests/playwright/support/utils/systemtags.ts @@ -42,15 +42,32 @@ export async function listTags(): Promise { } /** - * Delete all existing tags via OCC. + * Delete the tags with the given names via OCC, leaving all other tags untouched. + * + * @param names - The names of the tags to delete */ -export async function clearTags(): Promise { +export async function deleteTagsByName(names: string[]): Promise { const tags = await listTags() - for (const tag of tags) { + for (const tag of tags.filter(({ name }) => names.includes(name))) { await deleteTag(tag.id, true) } } +/** + * Fixture providing a generator for unique tag names. Tags are server-global, + * so every tag created under one of these names is deleted after the test + * instead of wiping all tags, which would break tests running in parallel. + */ +export async function tagNameFixture({}, use: (generate: () => string) => Promise): Promise { + const names: string[] = [] + await use(() => { + const name = crypto.randomUUID() + names.push(name) + return name + }) + await deleteTagsByName(names) +} + /** * Assign tags to a file via OCC. * From 25d65268c7f4e9ed2619174d6b37f4c256e4ec38 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Thu, 29 Oct 2026 23:44:57 +0100 Subject: [PATCH 02/11] test(settings): lift the language enforcement in one worker only `force_language` / `force_locale` were deleted in `beforeAll` and restored in `afterAll` of the whole personal info spec. With `fullyParallel`, every worker that runs a test of the file runs those hooks, so one worker restoring the enforcement made the language and locale section read-only for another worker still testing it. The per-test `beforeEach` only narrowed that window. Toggle the enforcement around the language and locale tests alone and run them serially, so a single worker owns that state. The restored values are the defaults of the test server. Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Ferdinand Thiessen --- .../e2e/settings/personal-info.spec.ts | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/tests/playwright/e2e/settings/personal-info.spec.ts b/tests/playwright/e2e/settings/personal-info.spec.ts index cef090218b291..5cf0e61d2a8dc 100644 --- a/tests/playwright/e2e/settings/personal-info.spec.ts +++ b/tests/playwright/e2e/settings/personal-info.spec.ts @@ -12,16 +12,10 @@ test.describe('Settings: Change personal information', () => { test.beforeAll(async () => { // Prevent the Fediverse section from making outbound HTTP requests await runOcc(['config:system:set', 'has_internet_connection', '--type', 'bool', '--value', 'false']) - // Let each user choose their own language and locale - await runOcc(['config:system:delete', 'force_language']) - await runOcc(['config:system:delete', 'force_locale']) }) test.afterAll(async () => { await runOcc(['config:system:delete', 'has_internet_connection']) - // Restore English defaults so other test suites are unaffected - await runOcc(['config:system:set', 'force_language', '--value', 'en']) - await runOcc(['config:system:set', 'force_locale', '--value', 'en_US']) }) // ── Profile ─────────────────────────────────────────────────────────────── @@ -343,13 +337,22 @@ test.describe('Settings: Change personal information', () => { // ── Language & locale ───────────────────────────────────────────────────── test.describe('Language & locale', () => { - test.beforeEach(async () => { - // The section is read-only while a language or locale is enforced, and a - // parallel worker may have restored the defaults of this suite already + // The section is read-only while a language or locale is enforced. Lifting + // the instance-wide enforcement must happen in a single worker, otherwise + // one worker's cleanup re-enforces it while another is still testing. + test.describe.configure({ mode: 'serial' }) + + test.beforeAll(async () => { await runOcc(['config:system:delete', 'force_language']) await runOcc(['config:system:delete', 'force_locale']) }) + test.afterAll(async () => { + // Restore the test server's English defaults for the other suites + await runOcc(['config:system:set', 'force_language', '--value', 'en']) + await runOcc(['config:system:set', 'force_locale', '--value', 'en_US']) + }) + test('can change language', async ({ page, languageLocalePage }) => { await languageLocalePage.open() From d2232dea1b997fd707b9e6e595ac07f218fad190 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Thu, 29 Oct 2026 23:45:12 +0100 Subject: [PATCH 03/11] test(settings): drop the networkidle waits from the personal info spec `networkidle` is timing-fragile with background polling. After picking a locale it could also resolve on the page that was about to reload, before the reload had even started. Wait for the reload's `load` event instead, and rely on the retrying assertion after opening the profile page. Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Ferdinand Thiessen --- tests/playwright/e2e/settings/personal-info.spec.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/playwright/e2e/settings/personal-info.spec.ts b/tests/playwright/e2e/settings/personal-info.spec.ts index 5cf0e61d2a8dc..96caa264eeded 100644 --- a/tests/playwright/e2e/settings/personal-info.spec.ts +++ b/tests/playwright/e2e/settings/personal-info.spec.ts @@ -30,7 +30,7 @@ test.describe('Settings: Change personal information', () => { await profileContactPage.setProfileEnabled(false) // Profile is disabled: the public profile page shows a "not found" heading - await page.goto(`/u/${user.userId}`, { waitUntil: 'networkidle' }) + await page.goto(`/u/${user.userId}`) await expect(page.getByRole('heading', { name: /Profile not found/i })).toBeVisible() // Re-enable the profile @@ -368,10 +368,10 @@ test.describe('Settings: Change personal information', () => { // The example is rendered in the active locale, US English by default await expect(languageLocalePage.localeExample()).toContainText(/Example: \d{2}\/\d{2}\/\d{4}/) - await languageLocalePage.selectLocale('German (Germany)', 'German') - // Locale change triggers a full page reload - await page.waitForLoadState('networkidle') + const reloaded = page.waitForEvent('load') + await languageLocalePage.selectLocale('German (Germany)', 'German') + await reloaded await languageLocalePage.expectSelected(languageLocalePage.localeSelect(), 'German (Germany)') // The example now uses the German date format await expect(languageLocalePage.localeExample()).toContainText(/Example: \d{2}\.\d{2}\.\d{4}/) From 9f040ee6767497d17f10a125175eb737da8376be Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Thu, 29 Oct 2026 23:45:53 +0100 Subject: [PATCH 04/11] test: retry fixture logins and account creation with a backoff The session and account fixtures retried a failed login or `createRandomUser()` exactly once after a fixed 800 ms. The transient failures are real (logins answered with HTTP 500 twice in a row, user creation exiting non-zero), and a single fixed-delay retry is not enough to ride them out. Share a `retryWithBackoff` helper that tries three times with an exponential delay, and use it in every fixture that had its own copy of the single retry. Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Ferdinand Thiessen --- .../support/fixtures/admin-session.ts | 9 ++---- .../support/fixtures/random-user-session.ts | 9 ++---- .../support/fixtures/random-user.ts | 10 ++----- .../fixtures/transfer-ownership-page.ts | 19 ++----------- tests/playwright/support/utils/retry.ts | 28 +++++++++++++++++++ 5 files changed, 37 insertions(+), 38 deletions(-) create mode 100644 tests/playwright/support/utils/retry.ts diff --git a/tests/playwright/support/fixtures/admin-session.ts b/tests/playwright/support/fixtures/admin-session.ts index 5f3f577c68991..eec169e2cf4b0 100644 --- a/tests/playwright/support/fixtures/admin-session.ts +++ b/tests/playwright/support/fixtures/admin-session.ts @@ -7,18 +7,13 @@ import { User } from '@nextcloud/e2e-test-server' import { login } from '@nextcloud/e2e-test-server/playwright' import { test as baseTest } from '@playwright/test' import { installPasswordConfirmationHandler } from '../utils/password-confirmation.ts' +import { retryWithBackoff } from '../utils/retry.ts' const admin = new User('admin', 'admin') export const test = baseTest.extend({ page: async ({ page, context }, use) => { - try { - await login(context.request, admin) - } catch (error) { - console.info('Failed to authenticate as admin, retrying', error) - await new Promise((resolve) => setTimeout(resolve, 800)) - await login(context.request, admin) - } + await retryWithBackoff('Authenticating as admin', () => login(context.request, admin)) await installPasswordConfirmationHandler(page, admin.password) await use(page) }, diff --git a/tests/playwright/support/fixtures/random-user-session.ts b/tests/playwright/support/fixtures/random-user-session.ts index daa4636def12a..b3718804c0703 100644 --- a/tests/playwright/support/fixtures/random-user-session.ts +++ b/tests/playwright/support/fixtures/random-user-session.ts @@ -5,6 +5,7 @@ import { login } from '@nextcloud/e2e-test-server/playwright' import { installPasswordConfirmationHandler } from '../utils/password-confirmation.ts' +import { retryWithBackoff } from '../utils/retry.ts' import { test as randomUserTest } from './random-user.ts' /** @@ -14,13 +15,7 @@ import { test as randomUserTest } from './random-user.ts' export const test = randomUserTest.extend({ page: async ({ browser, user }, use) => { const page = await browser.newPage() - try { - await login(page.request, user) - } catch (error) { - console.info('Failed to authenticate as random user, retrying', error) - await new Promise((resolve) => setTimeout(resolve, 800)) - await login(page.request, user) - } + await retryWithBackoff('Authenticating as random user', () => login(page.request, user)) await installPasswordConfirmationHandler(page, user.password) await use(page) }, diff --git a/tests/playwright/support/fixtures/random-user.ts b/tests/playwright/support/fixtures/random-user.ts index ebef3792f4934..c0e68c94d7ebc 100644 --- a/tests/playwright/support/fixtures/random-user.ts +++ b/tests/playwright/support/fixtures/random-user.ts @@ -8,6 +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 { test as baseTest } from '@playwright/test' +import { retryWithBackoff } from '../utils/retry.ts' /** * Extends the base test with a freshly-created random `user`. @@ -15,14 +16,7 @@ import { test as baseTest } from '@playwright/test' */ export const test = baseTest.extend<{ user: User }>({ user: async ({}, use) => { - let user: User - try { - user = await createRandomUser() - } catch { - // Retry once on transient failure - await new Promise((resolve) => setTimeout(resolve, 800)) - user = await createRandomUser() - } + const user = await retryWithBackoff('Creating a random user', () => createRandomUser()) await use(user) await runOcc(['user:delete', user.userId], { failOnError: false }) }, diff --git a/tests/playwright/support/fixtures/transfer-ownership-page.ts b/tests/playwright/support/fixtures/transfer-ownership-page.ts index 6db18102daf0d..62efd49b71f4e 100644 --- a/tests/playwright/support/fixtures/transfer-ownership-page.ts +++ b/tests/playwright/support/fixtures/transfer-ownership-page.ts @@ -10,6 +10,7 @@ import { runOcc } from '@nextcloud/e2e-test-server/docker' import { createRandomUser, login } from '@nextcloud/e2e-test-server/playwright' import { FilesListPage } from '../sections/FilesListPage.ts' import { TransferOwnershipPage } from '../sections/TransferOwnershipPage.ts' +import { retryWithBackoff } from '../utils/retry.ts' import { test as filesTest } from './files-page.ts' type TransferOwnershipFixtures = { @@ -36,14 +37,7 @@ type TransferOwnershipFixtures = { */ export const test = filesTest.extend({ recipient: async ({}, use) => { - let recipient: User - try { - recipient = await createRandomUser() - } catch { - // Retry once on transient failure, as the `user` fixture does - await new Promise((resolve) => setTimeout(resolve, 800)) - recipient = await createRandomUser() - } + const recipient = await retryWithBackoff('Creating the recipient', () => createRandomUser()) await use(recipient) await runOcc(['user:delete', recipient.userId], { failOnError: false }) }, @@ -62,14 +56,7 @@ export const test = filesTest.extend({ recipientPage: async ({ browser, recipient }, use) => { const context = await browser.newContext() const recipientPage = await context.newPage() - try { - await login(recipientPage.request, recipient) - } catch (error) { - // Same transient failure the session of `user` is retried for - console.info('Failed to authenticate as recipient, retrying', error) - await new Promise((resolve) => setTimeout(resolve, 800)) - await login(recipientPage.request, recipient) - } + await retryWithBackoff('Authenticating as recipient', () => login(recipientPage.request, recipient)) await use(recipientPage) await context.close() }, diff --git a/tests/playwright/support/utils/retry.ts b/tests/playwright/support/utils/retry.ts new file mode 100644 index 0000000000000..a78274f84562a --- /dev/null +++ b/tests/playwright/support/utils/retry.ts @@ -0,0 +1,28 @@ +/* + * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +/** + * Run `action`, retrying it with an exponential backoff while it throws. + * Meant for fixture setup (logins, account creation) that can hit transient + * server errors on a busy test server. + * + * @param description - What the action does, used to log the retries + * @param action - The action to run + * @param attempts - How often to try in total + */ +export async function retryWithBackoff(description: string, action: () => Promise, attempts = 3): Promise { + for (let attempt = 1; ; attempt++) { + try { + return await action() + } catch (error) { + if (attempt >= attempts) { + throw error + } + const delay = 800 * 2 ** (attempt - 1) + console.info(`${description} failed (attempt ${attempt} of ${attempts}), retrying in ${delay} ms`, error) + await new Promise((resolve) => setTimeout(resolve, delay)) + } + } +} From 20ffa19c1b11bc2996f2dc9f4fd4115d753796ab Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Thu, 29 Oct 2026 23:46:09 +0100 Subject: [PATCH 05/11] test(core): wait for the contacts menu search response `ContactsMenuPage.search()` promised to wait for the server response but only filled the debounced input, so callers asserted against whatever the list showed before the search request was even sent. Wait for the contacts request carrying the search term. Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Ferdinand Thiessen --- tests/playwright/support/sections/ContactsMenuPage.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/playwright/support/sections/ContactsMenuPage.ts b/tests/playwright/support/sections/ContactsMenuPage.ts index f13856636f7ed..8062b77396498 100644 --- a/tests/playwright/support/sections/ContactsMenuPage.ts +++ b/tests/playwright/support/sections/ContactsMenuPage.ts @@ -65,6 +65,10 @@ export class ContactsMenuPage { * a network request always follows a fill(). */ async search(query: string): Promise { + const searched = this.page.waitForResponse((r) => r.url().includes('/contactsmenu/contacts') + && r.request().method() === 'POST' + && r.request().postDataJSON()?.filter === query) await this.page.getByRole('searchbox', { name: 'Search contacts' }).fill(query) + await searched } } From 2f5ee240a519a07309985dd7296096ce8c9cf9de Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Thu, 29 Oct 2026 23:46:33 +0100 Subject: [PATCH 06/11] test(files): treat unrendered rows as outside the viewport `isFullyInViewport()` first asserted that the locator resolves to one element. For a row the virtual list has dropped from the DOM, which is the strongest form of "not in the viewport", that assertion threw, and since `expect.poll()` does not catch errors thrown by its callback, the scrolling specs polling for `false` failed instead of passing. Report an element that is not rendered as not in the viewport. Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Ferdinand Thiessen --- tests/playwright/support/utils/viewport.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/tests/playwright/support/utils/viewport.ts b/tests/playwright/support/utils/viewport.ts index ce61737242b38..fe75b0d7bfffc 100644 --- a/tests/playwright/support/utils/viewport.ts +++ b/tests/playwright/support/utils/viewport.ts @@ -5,8 +5,6 @@ import type { Locator, Page } from '@playwright/test' -import { expect } from '@playwright/test' - /** * The fixed per-item heights the files virtual list renders with, mirroring the * `itemHeight` getter in `apps/files/src/components/VirtualList.vue`. Using the @@ -54,12 +52,15 @@ export async function fitFilesListToRows(page: Page, rows: number, gridMode = fa * Whether the locator's box lies fully within the current viewport (top-to-bottom * and left-to-right). Playwright's `toBeVisible` only checks the DOM/paint state, * not the scroll position, so this is what tells a scrolled-off buffered row from - * a genuinely on-screen one. + * a genuinely on-screen one. An element that is not rendered at all, like a row + * the virtual list dropped, is not in the viewport either. * * @param locator - The element to test */ export async function isFullyInViewport(locator: Locator): Promise { - await expect(locator).toHaveCount(1) // ensure the locator resolves to a single element + if (await locator.count() === 0) { + return false + } const box = await locator.boundingBox() if (!box) { From 9390b253e55cd272f65254fa14fc019414be13e8 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Thu, 29 Oct 2026 23:47:18 +0100 Subject: [PATCH 07/11] test(files): verify the select-all checkbox state after toggling it `selectAll()` / `deselectAll()` force-clicked the master checkbox without checking the outcome. A click landing mid-render could be a no-op, and the caller only failed later on an unrelated assertion. Use `check()` / `uncheck()` instead, which assert the resulting checked state. The data attribute sits on the `` itself. Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Ferdinand Thiessen --- tests/playwright/support/sections/FilesListPage.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/playwright/support/sections/FilesListPage.ts b/tests/playwright/support/sections/FilesListPage.ts index 0e21a744e908d..003869624689e 100644 --- a/tests/playwright/support/sections/FilesListPage.ts +++ b/tests/playwright/support/sections/FilesListPage.ts @@ -377,16 +377,16 @@ export class FilesListPage { } async selectAll(): Promise { - await this.getSelectAllCheckbox().click({ force: true }) + await this.getSelectAllCheckbox().check({ force: true }) } /** * Clear the current selection via the master checkbox. It is a toggle, so it - * clicks the same control as {@link selectAll}; call it while rows are + * uses the same control as {@link selectAll}; call it while all rows are * selected to deselect them all. */ async deselectAll(): Promise { - await this.getSelectAllCheckbox().click({ force: true }) + await this.getSelectAllCheckbox().uncheck({ force: true }) } /** From 73a1b1ca6d18b2343246f7d2978e9cc762698df4 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Thu, 29 Oct 2026 23:47:33 +0100 Subject: [PATCH 08/11] test(appstore): match app rows by their exact app name `appRow()` filtered table rows by a substring of their text and silently took the first match in DOM order, so "Files" also matched "Files sharing". The enable and disable button helpers inherited that. Match the row by the app name its details link starts with. Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Ferdinand Thiessen --- tests/playwright/support/sections/AppstorePage.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/playwright/support/sections/AppstorePage.ts b/tests/playwright/support/sections/AppstorePage.ts index e936cb998ef21..6836a03815236 100644 --- a/tests/playwright/support/sections/AppstorePage.ts +++ b/tests/playwright/support/sections/AppstorePage.ts @@ -56,10 +56,14 @@ export class AppstorePage { } /** - * Gets a specific app row by app name + * Gets a specific app row by app name. The name is matched exactly, so + * "Files" does not also match "Files sharing". */ appRow(appName: string): Locator { - return this.appsTable().locator('tr').filter({ hasText: appName }).first() + // The details link is named " (Show details)" + const escapedName = appName.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') + const detailsLink = this.page.getByRole('link', { name: new RegExp(`^${escapedName} \\(`) }) + return this.appsTable().getByRole('row').filter({ has: detailsLink }) } /** From 2b0346086df020a6d4fc18934785172d4e5df21c Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Thu, 29 Oct 2026 23:47:51 +0100 Subject: [PATCH 09/11] test(appstore): restore the toggled apps after the app management spec The app management tests enable `testing` and disable `updatenotification` instance-wide, but only reset that state before each of their own tests. The last test's changes leaked into every spec running afterwards on the same server. Restore both apps to their default state once the spec is done. Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Ferdinand Thiessen --- tests/playwright/e2e/appstore/admin-settings-apps.spec.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/playwright/e2e/appstore/admin-settings-apps.spec.ts b/tests/playwright/e2e/appstore/admin-settings-apps.spec.ts index e00b5df646f96..9ed2e73c7b4ae 100644 --- a/tests/playwright/e2e/appstore/admin-settings-apps.spec.ts +++ b/tests/playwright/e2e/appstore/admin-settings-apps.spec.ts @@ -22,6 +22,12 @@ test.describe('Settings: App management', () => { await appstorePage.appsTable().waitFor({ state: 'visible', timeout: 10000 }) }) + test.afterAll(async () => { + // Leave the apps the tests toggle in the state the other suites expect + await runOcc(['app:disable', 'testing'], { failOnError: false }) + await runOcc(['app:enable', 'updatenotification'], { failOnError: false }) + }) + test('Can enable an installed app', async ({ page, appstorePage }) => { // Intercept the enable app request const enableRequest = page.waitForResponse((response) => response.url().includes('/ocs/v2.php/apps/appstore/api/v1/apps/enable')) From 15731a5dc1e0530ee275cb580cc2458986f2fd44 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Thu, 29 Oct 2026 23:48:01 +0100 Subject: [PATCH 10/11] test(files_trashbin): wait for the share to reach the sharee The group share test had the sharee delete the file through the shared folder right after creating the share. Shares mount into the recipient's tree asynchronously, so the DAV `DELETE` could hit a 404 before the mount was there. Poll for the shared file with `waitForShare()` first, as the other sharing specs do. Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Ferdinand Thiessen --- tests/playwright/e2e/files_trashbin/files.spec.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/playwright/e2e/files_trashbin/files.spec.ts b/tests/playwright/e2e/files_trashbin/files.spec.ts index 7a21659aa354a..78f0f55d0e9b1 100644 --- a/tests/playwright/e2e/files_trashbin/files.spec.ts +++ b/tests/playwright/e2e/files_trashbin/files.spec.ts @@ -9,7 +9,7 @@ import type { TrashbinListPage } from '../../support/sections/TrashbinListPage.t import { readFile } from 'node:fs/promises' import { expect, test } from '../../support/fixtures/files-trashbin-page.ts' import { mkdir, rm, uploadContent } from '../../support/utils/dav.ts' -import { createShare, ShareType } from '../../support/utils/sharing.ts' +import { createShare, ShareType, waitForShare } from '../../support/utils/sharing.ts' import { setUserDisplayName } from '../../support/utils/users.ts' test.describe('files_trashbin: download files', () => { @@ -71,6 +71,7 @@ test.describe('files_trashbin: file row', () => { const fileId = Number(await uploadContent(aliceRequest, user, '', 'text/plain', '/Shared/test-file.txt')) // Bob (the sharee) deletes the file from his view of the shared folder + await waitForShare(bobRequest, bob, 'Shared', 'test-file.txt') await rm(bobRequest, bob, '/Shared/test-file.txt') await filesListPage.open('trashbin') From 4d2f66c7209d46dccc0c4822fd44d730d0c46981 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Thu, 29 Oct 2026 23:48:19 +0100 Subject: [PATCH 11/11] test(files): poll the rename input selection The rename input selects the basename in a `$nextTick` after it is rendered, but the test read `selectionStart` / `selectionEnd` once as soon as the input was visible, which could be before the selection was applied. Poll the selection range until it matches. Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Ferdinand Thiessen --- tests/playwright/e2e/files/files-renaming.spec.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/playwright/e2e/files/files-renaming.spec.ts b/tests/playwright/e2e/files/files-renaming.spec.ts index e7260381757db..93e8436f23fd9 100644 --- a/tests/playwright/e2e/files/files-renaming.spec.ts +++ b/tests/playwright/e2e/files/files-renaming.spec.ts @@ -40,9 +40,11 @@ test.describe('Files: Rename nodes', () => { const input = filesListPage.getRenameInputForFile('file.txt') await expect(input).toBeVisible() - const { selectionStart, selectionEnd } = await input.evaluate((el) => ({ selectionStart: (el as HTMLInputElement).selectionStart, selectionEnd: (el as HTMLInputElement).selectionEnd })) - expect(selectionStart).toBe(0) - expect(selectionEnd).toBe('file'.length) + // The selection is applied after the input is shown + await expect.poll(() => input.evaluate((el) => { + const { selectionStart, selectionEnd } = el as HTMLInputElement + return { selectionStart, selectionEnd } + })).toEqual({ selectionStart: 0, selectionEnd: 'file'.length }) }) test('shows validation error on invalid filename', async ({ filesListPage }) => {