From 2cddb16a9fdbf9796546681839d580ddea7fe890 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 13 Jan 2025 17:26:01 +0000 Subject: [PATCH] Fix flaky playwright tests (#28975) * Fix flaky tests Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Fix flaky tests Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Fix mas config Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Fix another flaky test Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --------- Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- playwright/e2e/crypto/backups-mas.spec.ts | 3 +-- playwright/e2e/login/login-consent.spec.ts | 5 +++++ .../e2e/right-panel/right-panel.spec.ts | 1 + .../e2e/room-directory/room-directory.spec.ts | 1 + .../general-room-settings-tab.spec.ts | 1 + playwright/pages/client.ts | 19 +++++++++---------- playwright/services.ts | 3 ++- playwright/stale-screenshot-reporter.ts | 6 +++++- playwright/testcontainers/mas.ts | 10 ++++++++++ playwright/testcontainers/synapse.ts | 7 ++++++- 10 files changed, 41 insertions(+), 15 deletions(-) diff --git a/playwright/e2e/crypto/backups-mas.spec.ts b/playwright/e2e/crypto/backups-mas.spec.ts index 614bde5064..03c83efb1a 100644 --- a/playwright/e2e/crypto/backups-mas.spec.ts +++ b/playwright/e2e/crypto/backups-mas.spec.ts @@ -62,8 +62,7 @@ test.describe("Key backup reset from elsewhere", () => { await page.getByRole("textbox", { name: "Name" }).fill("test room"); await page.getByRole("button", { name: "Create room" }).click(); - // @ts-ignore - this runs in the browser scope where mxMatrixClientPeg is a thing. Here, it is not. - const accessToken = await page.evaluate(() => mxMatrixClientPeg.get().getAccessToken()); + const accessToken = await page.evaluate(() => window.mxMatrixClientPeg.get().getAccessToken()); const csAPI = new TestClientServerAPI(request, homeserver, accessToken); diff --git a/playwright/e2e/login/login-consent.spec.ts b/playwright/e2e/login/login-consent.spec.ts index d7d5861a02..33e1e21d7e 100644 --- a/playwright/e2e/login/login-consent.spec.ts +++ b/playwright/e2e/login/login-consent.spec.ts @@ -91,6 +91,11 @@ test.use({ }, }, }, + context: async ({ context, homeserver }, use) => { + // Restart the homeserver to wipe its in-memory db so we can reuse the same user ID without cross-signing prompts + await homeserver.restart(); + await use(context); + }, credentials: async ({ context, homeserver }, use) => { const displayName = "Dave"; const credentials = await homeserver.registerUser(username, password, displayName); diff --git a/playwright/e2e/right-panel/right-panel.spec.ts b/playwright/e2e/right-panel/right-panel.spec.ts index cb2a11ac00..0bdd0a283a 100644 --- a/playwright/e2e/right-panel/right-panel.spec.ts +++ b/playwright/e2e/right-panel/right-panel.spec.ts @@ -47,6 +47,7 @@ test.describe("RightPanel", () => { // Set a local room address const localAddresses = page.locator(".mx_SettingsFieldset", { hasText: "Local Addresses" }); await localAddresses.getByRole("textbox").fill(ROOM_ADDRESS_LONG); + await expect(page.getByText("This address is available to use")).toBeVisible(); await localAddresses.getByRole("button", { name: "Add" }).click(); await expect(localAddresses.getByText(`#${ROOM_ADDRESS_LONG}:localhost`)).toHaveClass( "mx_EditableItem_item", diff --git a/playwright/e2e/room-directory/room-directory.spec.ts b/playwright/e2e/room-directory/room-directory.spec.ts index 34004c90d2..a38cc7d395 100644 --- a/playwright/e2e/room-directory/room-directory.spec.ts +++ b/playwright/e2e/room-directory/room-directory.spec.ts @@ -30,6 +30,7 @@ test.describe("Room Directory", () => { // First add a local address `gaming` const localAddresses = page.locator(".mx_SettingsFieldset", { hasText: "Local Addresses" }); await localAddresses.getByRole("textbox").fill("gaming"); + await expect(page.getByText("This address is available to use")).toBeVisible(); await localAddresses.getByRole("button", { name: "Add" }).click(); await expect(localAddresses.getByText("#gaming:localhost")).toHaveClass("mx_EditableItem_item"); diff --git a/playwright/e2e/settings/general-room-settings-tab.spec.ts b/playwright/e2e/settings/general-room-settings-tab.spec.ts index eec32f7af5..5e29f802c2 100644 --- a/playwright/e2e/settings/general-room-settings-tab.spec.ts +++ b/playwright/e2e/settings/general-room-settings-tab.spec.ts @@ -41,6 +41,7 @@ test.describe("General room settings tab", () => { // 1. Set the room-address to be a really long string const longString = "abcasdhjasjhdaj1jh1asdhasjdhajsdhjavhjksd".repeat(4); await settings.locator("#roomAliases input[label='Room address']").fill(longString); + await expect(page.getByText("This address is available to use")).toBeVisible(); await settings.locator("#roomAliases").getByText("Add", { exact: true }).click(); // 2. wait for the new setting to apply ... diff --git a/playwright/pages/client.ts b/playwright/pages/client.ts index c2586f1b5e..362915ce71 100644 --- a/playwright/pages/client.ts +++ b/playwright/pages/client.ts @@ -179,18 +179,17 @@ export class Client { public async createRoom(options: ICreateRoomOpts): Promise { const client = await this.prepareClient(); return await client.evaluate(async (cli, options) => { - const roomPromise = new Promise((resolve) => { - const onRoom = (room: Room) => { - if (room.roomId === roomId) { - cli.off(window.matrixcs.ClientEvent.Room, onRoom); - resolve(); - } - }; - cli.on(window.matrixcs.ClientEvent.Room, onRoom); - }); const { room_id: roomId } = await cli.createRoom(options); if (!cli.getRoom(roomId)) { - await roomPromise; + await new Promise((resolve) => { + const onRoom = (room: Room) => { + if (room.roomId === roomId) { + cli.off(window.matrixcs.ClientEvent.Room, onRoom); + resolve(); + } + }; + cli.on(window.matrixcs.ClientEvent.Room, onRoom); + }); } return roomId; }, options); diff --git a/playwright/services.ts b/playwright/services.ts index 213af7df42..2444adf3a0 100644 --- a/playwright/services.ts +++ b/playwright/services.ts @@ -130,10 +130,11 @@ export const test = base.extend({ { scope: "worker" }, ], - context: async ({ logger, context, request, homeserver }, use, testInfo) => { + context: async ({ logger, context, request, homeserver, mailhogClient }, use, testInfo) => { homeserver.setRequest(request); await logger.onTestStarted(context); await use(context); await logger.onTestFinished(testInfo); + await mailhogClient.deleteAll(); }, }); diff --git a/playwright/stale-screenshot-reporter.ts b/playwright/stale-screenshot-reporter.ts index dc934827c1..36aba56a07 100644 --- a/playwright/stale-screenshot-reporter.ts +++ b/playwright/stale-screenshot-reporter.ts @@ -20,10 +20,13 @@ const snapshotRoot = path.join(__dirname, "snapshots"); class StaleScreenshotReporter implements Reporter { private screenshots = new Set(); + private failing = false; private success = true; public onTestEnd(test: TestCase): void { - if (!test.ok()) return; + if (!test.ok()) { + this.failing = true; + } for (const annotation of test.annotations) { if (annotation.type === "_screenshot") { this.screenshots.add(annotation.description); @@ -40,6 +43,7 @@ class StaleScreenshotReporter implements Reporter { } public async onExit(): Promise { + if (this.failing) return; const screenshotFiles = new Set(await glob(`**/*.png`, { cwd: snapshotRoot })); for (const screenshot of screenshotFiles) { if (screenshot.split("-").at(-1) !== "linux.png") { diff --git a/playwright/testcontainers/mas.ts b/playwright/testcontainers/mas.ts index d15f619dbc..2c795c2c47 100644 --- a/playwright/testcontainers/mas.ts +++ b/playwright/testcontainers/mas.ts @@ -168,6 +168,16 @@ const DEFAULT_CONFIG = { access_token_ttl: 300, compat_token_ttl: 300, }, + rate_limiting: { + login: { + burst: 10, + per_second: 1, + }, + registration: { + burst: 10, + per_second: 1, + }, + }, }; export class MatrixAuthenticationServiceContainer extends GenericContainer { diff --git a/playwright/testcontainers/synapse.ts b/playwright/testcontainers/synapse.ts index 5111a6f0a6..5165815361 100644 --- a/playwright/testcontainers/synapse.ts +++ b/playwright/testcontainers/synapse.ts @@ -5,7 +5,7 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com Please see LICENSE files in the repository root for full details. */ -import { AbstractStartedContainer, GenericContainer, StartedTestContainer, Wait } from "testcontainers"; +import { AbstractStartedContainer, GenericContainer, RestartOptions, StartedTestContainer, Wait } from "testcontainers"; import { APIRequestContext } from "@playwright/test"; import crypto from "node:crypto"; import * as YAML from "yaml"; @@ -239,6 +239,11 @@ export class StartedSynapseContainer extends AbstractStartedContainer implements super(container); } + public restart(options?: Partial): Promise { + this.adminToken = undefined; + return super.restart(options); + } + public setRequest(request: APIRequestContext): void { this.request = request; }