Skip to content

Commit

Permalink
Fix flaky playwright tests (#28975)
Browse files Browse the repository at this point in the history
* Fix flaky tests

Signed-off-by: Michael Telatynski <[email protected]>

* Fix flaky tests

Signed-off-by: Michael Telatynski <[email protected]>

* Fix mas config

Signed-off-by: Michael Telatynski <[email protected]>

* Fix another flaky test

Signed-off-by: Michael Telatynski <[email protected]>

---------

Signed-off-by: Michael Telatynski <[email protected]>
  • Loading branch information
t3chguy authored Jan 13, 2025
1 parent 671d6de commit 2cddb16
Show file tree
Hide file tree
Showing 10 changed files with 41 additions and 15 deletions.
3 changes: 1 addition & 2 deletions playwright/e2e/crypto/backups-mas.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
5 changes: 5 additions & 0 deletions playwright/e2e/login/login-consent.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions playwright/e2e/right-panel/right-panel.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
1 change: 1 addition & 0 deletions playwright/e2e/room-directory/room-directory.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down
1 change: 1 addition & 0 deletions playwright/e2e/settings/general-room-settings-tab.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 ...
Expand Down
19 changes: 9 additions & 10 deletions playwright/pages/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,18 +179,17 @@ export class Client {
public async createRoom(options: ICreateRoomOpts): Promise<string> {
const client = await this.prepareClient();
return await client.evaluate(async (cli, options) => {
const roomPromise = new Promise<void>((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<void>((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);
Expand Down
3 changes: 2 additions & 1 deletion playwright/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,11 @@ export const test = base.extend<TestFixtures, 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.onTestStarted(context);
await use(context);
await logger.onTestFinished(testInfo);
await mailhogClient.deleteAll();
},
});
6 changes: 5 additions & 1 deletion playwright/stale-screenshot-reporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,13 @@ const snapshotRoot = path.join(__dirname, "snapshots");

class StaleScreenshotReporter implements Reporter {
private screenshots = new Set<string>();
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);
Expand All @@ -40,6 +43,7 @@ class StaleScreenshotReporter implements Reporter {
}

public async onExit(): Promise<void> {
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") {
Expand Down
10 changes: 10 additions & 0 deletions playwright/testcontainers/mas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
7 changes: 6 additions & 1 deletion playwright/testcontainers/synapse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -239,6 +239,11 @@ export class StartedSynapseContainer extends AbstractStartedContainer implements
super(container);
}

public restart(options?: Partial<RestartOptions>): Promise<void> {
this.adminToken = undefined;
return super.restart(options);
}

public setRequest(request: APIRequestContext): void {
this.request = request;
}
Expand Down

0 comments on commit 2cddb16

Please sign in to comment.