From 84dfc5d22d84efcee2d118f2ae6448bb77b0b319 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 13 Jan 2025 10:43:56 +0000 Subject: [PATCH 1/4] Fix flaky tests Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- .../e2e/right-panel/right-panel.spec.ts | 1 + .../general-room-settings-tab.spec.ts | 1 + playwright/pages/client.ts | 19 +++++++++---------- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/playwright/e2e/right-panel/right-panel.spec.ts b/playwright/e2e/right-panel/right-panel.spec.ts index cb2a11ac002..0bdd0a283a9 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/settings/general-room-settings-tab.spec.ts b/playwright/e2e/settings/general-room-settings-tab.spec.ts index eec32f7af5e..5e29f802c2f 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 c2586f1b5ef..362915ce71f 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); From d7a1c307da7c334e6540025b62b08394ad4552ec Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 13 Jan 2025 11:10:59 +0000 Subject: [PATCH 2/4] Fix flaky tests Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- playwright/e2e/crypto/backups-mas.spec.ts | 3 +-- playwright/e2e/login/login-consent.spec.ts | 5 +++++ playwright/services.ts | 3 ++- playwright/stale-screenshot-reporter.ts | 6 +++++- playwright/testcontainers/mas.ts | 8 ++++++++ playwright/testcontainers/synapse.ts | 7 ++++++- 6 files changed, 27 insertions(+), 5 deletions(-) diff --git a/playwright/e2e/crypto/backups-mas.spec.ts b/playwright/e2e/crypto/backups-mas.spec.ts index 614bde50646..03c83efb1af 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 ab70e1d1869..a87b5019df6 100644 --- a/playwright/e2e/login/login-consent.spec.ts +++ b/playwright/e2e/login/login-consent.spec.ts @@ -88,6 +88,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/services.ts b/playwright/services.ts index b480cbc4054..38c4e6c6664 100644 --- a/playwright/services.ts +++ b/playwright/services.ts @@ -131,10 +131,11 @@ export const test = base.extend<{}, Services>({ { scope: "worker" }, ], - context: async ({ logger, context, request, homeserver }, use, testInfo) => { + context: async ({ logger, context, request, homeserver, mailhogClient }, use, testInfo) => { homeserver.setRequest(request); await logger.testStarted(testInfo); await use(context); await logger.testFinished(testInfo); + await mailhogClient.deleteAll(); }, }); diff --git a/playwright/stale-screenshot-reporter.ts b/playwright/stale-screenshot-reporter.ts index dc934827c1d..36aba56a071 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 d15f619dbc6..ba95129b66a 100644 --- a/playwright/testcontainers/mas.ts +++ b/playwright/testcontainers/mas.ts @@ -168,6 +168,14 @@ const DEFAULT_CONFIG = { access_token_ttl: 300, compat_token_ttl: 300, }, + rate_limiting: { + login: { + burst: 1000, + }, + registration: { + burst: 1000, + }, + }, }; export class MatrixAuthenticationServiceContainer extends GenericContainer { diff --git a/playwright/testcontainers/synapse.ts b/playwright/testcontainers/synapse.ts index 5111a6f0a66..51658153610 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; } From 7fbf9ef2699027c6c420f622533347ac0824e758 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 13 Jan 2025 11:47:32 +0000 Subject: [PATCH 3/4] Fix mas config Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- playwright/testcontainers/mas.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/playwright/testcontainers/mas.ts b/playwright/testcontainers/mas.ts index ba95129b66a..2c795c2c47d 100644 --- a/playwright/testcontainers/mas.ts +++ b/playwright/testcontainers/mas.ts @@ -170,10 +170,12 @@ const DEFAULT_CONFIG = { }, rate_limiting: { login: { - burst: 1000, + burst: 10, + per_second: 1, }, registration: { - burst: 1000, + burst: 10, + per_second: 1, }, }, }; From d27b55841f9ae7445b894f14733002297b267521 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 13 Jan 2025 12:02:00 +0000 Subject: [PATCH 4/4] Fix another flaky test Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- playwright/e2e/room-directory/room-directory.spec.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/playwright/e2e/room-directory/room-directory.spec.ts b/playwright/e2e/room-directory/room-directory.spec.ts index 34004c90d27..a38cc7d3950 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");