Merge pull request #62688 from nextcloud/chore/stableize-tests

test: resolve potential flaky Playwright tests

Author: susnux

playwright.config.ts

@@ -11,6 +11,7 @@ export default defineConfig({
 	forbidOnly: !!process.env.CI,
 	retries: process.env.CI ? 1 : 0,
 	workers: process.env.CI ? 1 : undefined,
+	timeout: process.env.CI ? 45_000 : undefined, // on CI allow 1.5x the default timeout to compensate for shared server resources
 	reporter: process.env.CI ? [['blob'], ['dot'], ['github']] : 'html',
 	use: {
 		baseURL: 'http://localhost:8042/index.php/',
@@ -57,10 +58,10 @@ export default defineConfig({
 		stdout: 'pipe',
 		gracefulShutdown: {
 			signal: 'SIGTERM',
-			timeout: 10000,
+			timeout: 10_000,
 		},
 		reuseExistingServer: !process.env.CI,
-		timeout: 5 * 60 * 1000,
+		timeout: 300_000,
 		wait: {
 			stdout: /Nextcloud container ready to run Playwright tests/,
 		},

tests/playwright/e2e/core/header-unified-search.spec.ts

@@ -48,6 +48,9 @@ test.describe('Header: unified search keyboard navigation', () => {
 	test('typing auto-selects the first result', async ({ page }) => {
 		const search = new UnifiedSearchPage(page)
 		await search.input().fill(TOKEN)
+		// A row rendering is not enough: a provider settling later re-renders the
+		// list and re-establishes the selection, so read it only once it is final.
+		await search.waitForSettledResults()
 
 		const firstOption = search.options().first()
 		await expect(firstOption).toBeVisible()
@@ -60,37 +63,43 @@ test.describe('Header: unified search keyboard navigation', () => {
 	test('arrow keys move the selection while focus stays in the input', async ({ page }) => {
 		const search = new UnifiedSearchPage(page)
 		await search.input().fill(TOKEN)
+		await search.waitForSettledResults()
 		// Need at least two rows to have somewhere to move to.
 		await expect(search.options().nth(1)).toBeVisible()
-		await expect(search.input()).toHaveAttribute('aria-activedescendant', /\w/)
 
-		const firstId = await search.input().getAttribute('aria-activedescendant')
+		const firstId = await search.activeOptionId()
 
 		await page.keyboard.press('ArrowDown')
 		// Selection advanced to another row and the input never lost focus.
-		await expect(search.input()).not.toHaveAttribute('aria-activedescendant', firstId!)
+		await expect(search.input()).not.toHaveAttribute('aria-activedescendant', firstId)
 		await expect(search.input()).toBeFocused()
 
 		await page.keyboard.press('ArrowUp')
-		await expect(search.input()).toHaveAttribute('aria-activedescendant', firstId!)
+		await expect(search.input()).toHaveAttribute('aria-activedescendant', firstId)
 		await expect(search.input()).toBeFocused()
 	})
 
 	test('Enter opens the selected result', async ({ page }) => {
 		const search = new UnifiedSearchPage(page)
 		await search.input().fill(TOKEN)
-		await expect(search.options().first()).toBeVisible()
+		// Enter activates whatever is selected *at that moment* and does nothing at
+		// all when the list is mid-flight (no selection), so the search has to have
+		// settled before the key is pressed.
+		await search.waitForSettledResults()
 
-		const activeId = await search.input().getAttribute('aria-activedescendant')
+		const activeId = await search.activeOptionId()
 		// The row is an NcListItem link to the file's short URL (/f/<id>), which the
 		// server redirects into the Files viewer. Grab the id and assert we land on it.
-		const href = await search.option(activeId!).getByRole('link').getAttribute('href')
+		const href = await search.option(activeId).getByRole('link').getAttribute('href')
 		const fileId = href?.match(/\/f\/(\d+)/)?.[1]
 		expect(fileId).toBeTruthy()
 
+		// Arm the wait before the key press: the URL is only reached after a
+		// server-side redirect, which outlives the default assertion timeout on a
+		// loaded CI machine.
+		const opened = page.waitForURL(new RegExp(`/files/${fileId}(?:[/?]|$)`), { timeout: 15_000 })
 		await page.keyboard.press('Enter')
-
-		await expect(page).toHaveURL(new RegExp(`/files/${fileId}(?:[/?]|$)`))
+		await opened
 	})
 
 	test('Escape closes the open results popover', async ({ page }) => {

tests/playwright/e2e/files_sharing/admin-settings-limit-to-same-group.spec.ts

@@ -25,6 +25,11 @@ test.describe('files_sharing: Sharing limited to members of the same group', ()
 
 	test.beforeAll(async () => {
 		await runOcc(['config:app:set', '--value', 'yes', 'core', 'shareapi_only_share_with_group_members'])
+		// The groups are shared by both tests, so create them once here rather than
+		// spending two container round-trips inside every test's own timeout.
+		for (const group of groups) {
+			await runOcc(['group:add', group], { failOnError: false })
+		}
 	})
 
 	test.afterAll(async () => {
@@ -34,36 +39,6 @@ test.describe('files_sharing: Sharing limited to members of the same group', ()
 		}
 	})
 
-	/**
-	 * Put both accounts in two shared groups, then share a file each way.
-	 *
-	 * @returns the file names, `fromUser` shared by `user`, `fromRecipient` by `recipient`
-	 */
-	async function seedMutualShares(
-		user: User,
-		recipient: User,
-		userRequest: Parameters<typeof createShare>[0],
-		recipientRequest: Parameters<typeof createShare>[0],
-	): Promise<{ fromUser: string, fromRecipient: string }> {
-		for (const group of groups) {
-			await runOcc(['group:add', group], { failOnError: false })
-			await runOcc(['group:adduser', group, user.userId])
-			await runOcc(['group:adduser', group, recipient.userId])
-		}
-
-		const fromUser = 'shared-by-user.txt'
-		const fromRecipient = 'shared-by-recipient.txt'
-		await uploadContent(userRequest, user, 'share to recipient', 'text/plain', `/${fromUser}`)
-		await uploadContent(recipientRequest, recipient, 'share to user', 'text/plain', `/${fromRecipient}`)
-		await createShare(userRequest, `/${fromUser}`, recipient.userId)
-		await createShare(recipientRequest, `/${fromRecipient}`, user.userId)
-
-		await waitForShare(recipientRequest, recipient, '', fromUser)
-		await waitForShare(userRequest, user, '', fromRecipient)
-
-		return { fromUser, fromRecipient }
-	}
-
 	test('keeps the shares while one common group is left', async ({ page, user, recipient, recipientRequest, filesListPage }) => {
 		const { fromUser, fromRecipient } = await seedMutualShares(user, recipient, page.request, recipientRequest)
 
@@ -92,4 +67,33 @@ test.describe('files_sharing: Sharing limited to members of the same group', ()
 		await filesListPage.open()
 		await expect(filesListPage.getRowForFile(fromUser)).toHaveCount(0)
 	})
+
+	/**
+	 * Put both accounts in two shared groups, then share a file each way.
+	 *
+	 * @returns the file names, `fromUser` shared by `user`, `fromRecipient` by `recipient`
+	 */
+	async function seedMutualShares(
+		user: User,
+		recipient: User,
+		userRequest: Parameters<typeof createShare>[0],
+		recipientRequest: Parameters<typeof createShare>[0],
+	): Promise<{ fromUser: string, fromRecipient: string }> {
+		for (const group of groups) {
+			await runOcc(['group:adduser', group, user.userId])
+			await runOcc(['group:adduser', group, recipient.userId])
+		}
+
+		const fromUser = 'shared-by-user.txt'
+		const fromRecipient = 'shared-by-recipient.txt'
+		await uploadContent(userRequest, user, 'share to recipient', 'text/plain', `/${fromUser}`)
+		await uploadContent(recipientRequest, recipient, 'share to user', 'text/plain', `/${fromRecipient}`)
+		await createShare(userRequest, `/${fromUser}`, recipient.userId)
+		await createShare(recipientRequest, `/${fromRecipient}`, user.userId)
+
+		await waitForShare(recipientRequest, recipient, '', fromUser)
+		await waitForShare(userRequest, user, '', fromRecipient)
+
+		return { fromUser, fromRecipient }
+	}
 })

tests/playwright/support/sections/FilesListPage.ts

@@ -25,9 +25,11 @@ export class FilesListPage {
 		return this.page.locator('[data-cy-files-list]')
 	}
 
-	/** Wait for the file list container to be rendered (e.g. after a direct goto). */
+	/**
+	 * Wait for the file list container to be rendered (e.g. after a direct goto).
+	 */
 	async waitForList(): Promise<void> {
-		await this.getFilesList().waitFor({ state: 'visible' })
+		await this.getFilesList().waitFor({ state: 'visible', timeout: 20_000 })
 	}
 
 	/**
@@ -176,6 +178,8 @@ export class FilesListPage {
 	 * row Locator so it serves both name- and fileid-addressed rows.
 	 */
 	private async openActionsMenuForRow(row: Locator): Promise<Locator> {
+		await expect(row).toBeVisible({ timeout: 15_000 })
+
 		await row.hover()
 
 		const actionsButton = row.getByRole('button', { name: 'Actions' })
@@ -431,7 +435,9 @@ export class FilesListPage {
 			// the row's buttons by the folder name is ambiguous for shared folders,
 			// whose row also carries a "Shared by …" action button that can contain
 			// the same text.
-			await this.getRowNameLinkForFile(directory).click()
+			const link = this.getRowNameLinkForFile(directory)
+			await expect(link).toBeVisible()
+			await link.click()
 
 			// Assert the deepest segment of the `dir` query param matches the folder
 			// we just opened. Comparing the decoded value (URLSearchParams decodes

tests/playwright/support/sections/UnifiedSearchPage.ts

@@ -5,6 +5,8 @@
 
 import type { Locator, Page } from '@playwright/test'
 
+import { expect } from '@playwright/test'
+
 /**
  * The unified search in the top header: the combobox input and the results
  * popover it controls. Desktop layout only (the mobile header collapses to a
@@ -63,6 +65,46 @@ export class UnifiedSearchPage {
 		return this.panel().getByRole('option')
 	}
 
+	/**
+	 * The popover's polite status region (WCAG 4.1.3). It reports the search
+	 * progress: "Searching …" while any provider is still in flight, then the
+	 * settled outcome — "No matching results" or the result count.
+	 */
+	status(): Locator {
+		return this.panel().getByRole('status')
+	}
+
+	/**
+	 * Wait for the result set to stop changing, i.e. for every provider to have
+	 * answered.
+	 *
+	 * Results stream in per provider and the rows re-render on every batch, which
+	 * clears and re-establishes the selection in between — so a visible first row
+	 * does not mean the selection is settled. Anything that reads
+	 * `aria-activedescendant`, or activates the selected row with Enter, has to
+	 * wait for this first: on a mid-flight list the id is momentarily absent and
+	 * Enter is a no-op.
+	 *
+	 * The status region is the app's own settled signal, so this needs no
+	 * arbitrary delay.
+	 */
+	async waitForSettledResults(): Promise<void> {
+		// The count, not just "results": "No matching results" is a settled state too,
+		// but one no selection assertion can work with. Anchored (with the template's
+		// surrounding whitespace, which a regex match does not normalize away) so a
+		// still-running search cannot slip through.
+		await expect(this.status()).toHaveText(/^\s*\d+ results?\s*$/)
+	}
+
+	/**
+	 * The id of the currently selected row, once the input names one. Waits for the
+	 * attribute so a still-settling list cannot yield `null`.
+	 */
+	async activeOptionId(): Promise<string> {
+		await expect(this.input()).toHaveAttribute('aria-activedescendant', /\S/)
+		return (await this.input().getAttribute('aria-activedescendant'))!
+	}
+
 	/**
 	 * A single result row addressed by its DOM id (the value the input carries in
 	 * aria-activedescendant). Attribute selector so provider ids with dots or colons