From ab37e1809dcf26471ff1bf82e5b5616d701dd3f0 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 11 Oct 2024 00:01:07 +0100 Subject: [PATCH 1/5] Playwright: wait for sync to arrive after joining rooms Fix a couple of flaky tests which were not waiting for the /sync to complete after joining a room. --- playwright/e2e/crypto/crypto.spec.ts | 7 +++- playwright/e2e/crypto/event-shields.spec.ts | 5 ++- playwright/pages/client.ts | 36 +++++++++++++++++++++ 3 files changed, 46 insertions(+), 2 deletions(-) diff --git a/playwright/e2e/crypto/crypto.spec.ts b/playwright/e2e/crypto/crypto.spec.ts index f3a4820ebe..e520d971ea 100644 --- a/playwright/e2e/crypto/crypto.spec.ts +++ b/playwright/e2e/crypto/crypto.spec.ts @@ -43,6 +43,7 @@ const testMessages = async (page: Page, bob: Bot, bobRoomId: string) => { }; const bobJoin = async (page: Page, bob: Bot) => { + // Wait for Bob to get the invite await bob.evaluate(async (cli) => { const bobRooms = cli.getRooms(); if (!bobRooms.length) { @@ -55,9 +56,13 @@ const bobJoin = async (page: Page, bob: Bot) => { }); } }); - const roomId = await bob.joinRoomByName("Alice"); + const roomId = await bob.joinRoomByName("Alice"); await expect(page.getByText("Bob joined the room")).toBeVisible(); + + // Even though Alice has seen Bob's join event, Bob may not have done so yet. Wait for the sync to arrive. + await bob.awaitRoomMembership(roomId); + return roomId; }; diff --git a/playwright/e2e/crypto/event-shields.spec.ts b/playwright/e2e/crypto/event-shields.spec.ts index fa9d1959da..18b4b14c99 100644 --- a/playwright/e2e/crypto/event-shields.spec.ts +++ b/playwright/e2e/crypto/event-shields.spec.ts @@ -33,7 +33,7 @@ test.describe("Cryptography", function () { await app.client.bootstrapCrossSigning(aliceCredentials); await autoJoin(bob); - // create an encrypted room + // create an encrypted room, and wait for Bob to join it. testRoomId = await createSharedRoomWithUser(app, bob.credentials.userId, { name: "TestRoom", initial_state: [ @@ -46,6 +46,9 @@ test.describe("Cryptography", function () { }, ], }); + + // Even though Alice has seen Bob's join event, Bob may not have done so yet. Wait for the sync to arrive. + await bob.awaitRoomMembership(testRoomId); }); test("should show the correct shield on e2e events", async ({ diff --git a/playwright/pages/client.ts b/playwright/pages/client.ts index 06e05fdcfa..7352880752 100644 --- a/playwright/pages/client.ts +++ b/playwright/pages/client.ts @@ -289,6 +289,42 @@ export class Client { await client.evaluate((client, { roomId, userId }) => client.unban(roomId, userId), { roomId, userId }); } + /** + * Wait for the client to have specific membership of a given room + * + * This is often useful after joining a room, when we need to wait for the sync loop to catch up. + * + * @param roomId - ID of the room to check + * @param membership - required membership. + */ + public async awaitRoomMembership(roomId: string, membership: string = "join") { + await this.evaluate( + (cli: MatrixClient, { roomId, membership }) => { + const isReady = () => { + // Fetch the room on each check, because we get a different instance before and after the join arrives. + const room = cli.getRoom(roomId); + const myMembership = room?.getMyMembership(); + // @ts-ignore access to private field "logger" + cli.logger.info(`waiting for room ${roomId}: membership now ${myMembership}`); + return myMembership == membership; + }; + if (isReady()) return; + + return new Promise((resolve) => { + async function onEvent() { + if (isReady()) { + cli.removeListener(window.matrixcs.ClientEvent.Event, onEvent); + resolve(); + } + } + + cli.on(window.matrixcs.ClientEvent.Event, onEvent); + }); + }, + { roomId, membership }, + ); + } + /** * @param {MatrixEvent} event * @param {ReceiptType} receiptType From 4e7daa13fa6da37c5e333291bd11b6b69a424c84 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 11 Oct 2024 00:56:12 +0100 Subject: [PATCH 2/5] Playwright: add a comment about a broken helper --- playwright/e2e/utils.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/playwright/e2e/utils.ts b/playwright/e2e/utils.ts index b357b5ca99..a2bcc0f29a 100644 --- a/playwright/e2e/utils.ts +++ b/playwright/e2e/utils.ts @@ -20,6 +20,14 @@ import { Client } from "../pages/client"; * @param client Client instance that can be user or bot * @param roomId room id to find room and check * @param predicate defines condition that is used to check the room state + * + * FIXME this does not do what it is supposed to do, and I think it is unfixable. + * `page.exposeFunction` adds a function which returns a Promise. `window[predicateId](room)` therefore + * always returns a truthy value (a Promise). But even if you fix that: as far as I can tell, the Room is + * just passed to the callback function as a JSON blob: you cannot actually call any methods on it, so the + * callback is useless. + * + * @deprecated This function is broken. */ export async function waitForRoom( page: Page, From 4a72e6ebe194d92906d835a650458be34ed797eb Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 11 Oct 2024 00:59:30 +0100 Subject: [PATCH 3/5] playwright: fix more flakiness in the shields test This bit can take a while as well. --- playwright/e2e/crypto/event-shields.spec.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/playwright/e2e/crypto/event-shields.spec.ts b/playwright/e2e/crypto/event-shields.spec.ts index 18b4b14c99..0544a7c904 100644 --- a/playwright/e2e/crypto/event-shields.spec.ts +++ b/playwright/e2e/crypto/event-shields.spec.ts @@ -290,9 +290,9 @@ test.describe("Cryptography", function () { // Let our app start syncing again await app.client.network.goOnline(); - // Wait for the messages to arrive + // Wait for the messages to arrive. It can take quite a while for the sync to wake up. const last = page.locator(".mx_EventTile_last"); - await expect(last).toContainText("test encrypted from unverified"); + await expect(last).toContainText("test encrypted from unverified", { timeout: 20000 }); const lastE2eIcon = last.locator(".mx_EventTile_e2eIcon"); await expect(lastE2eIcon).toHaveClass(/mx_EventTile_e2eIcon_warning/); await lastE2eIcon.focus(); From 78e81cb164bbbb765a7f2dc5ecb4587ecc5fa571 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Fri, 11 Oct 2024 10:32:32 +0100 Subject: [PATCH 4/5] Update playwright/pages/client.ts Co-authored-by: R Midhun Suresh --- playwright/pages/client.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/playwright/pages/client.ts b/playwright/pages/client.ts index 7352880752..abd487988b 100644 --- a/playwright/pages/client.ts +++ b/playwright/pages/client.ts @@ -306,7 +306,7 @@ export class Client { const myMembership = room?.getMyMembership(); // @ts-ignore access to private field "logger" cli.logger.info(`waiting for room ${roomId}: membership now ${myMembership}`); - return myMembership == membership; + return myMembership === membership; }; if (isReady()) return; From 01dfc7b6746b28901951a8986b2f77f22a913f30 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 11 Oct 2024 11:02:36 +0100 Subject: [PATCH 5/5] Add a timeout to `awaitRoomMembership` --- playwright/pages/client.ts | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/playwright/pages/client.ts b/playwright/pages/client.ts index abd487988b..7d62f42ae0 100644 --- a/playwright/pages/client.ts +++ b/playwright/pages/client.ts @@ -294,6 +294,8 @@ export class Client { * * This is often useful after joining a room, when we need to wait for the sync loop to catch up. * + * Times out with an error after 1 second. + * * @param roomId - ID of the room to check * @param membership - required membership. */ @@ -310,7 +312,15 @@ export class Client { }; if (isReady()) return; - return new Promise((resolve) => { + const timeoutPromise = new Promise((resolve) => setTimeout(resolve, 1000)).then(() => { + const room = cli.getRoom(roomId); + const myMembership = room?.getMyMembership(); + throw new Error( + `Timeout waiting for room ${roomId} membership (now '${myMembership}', wanted '${membership}')`, + ); + }); + + const readyPromise = new Promise((resolve) => { async function onEvent() { if (isReady()) { cli.removeListener(window.matrixcs.ClientEvent.Event, onEvent); @@ -320,6 +330,8 @@ export class Client { cli.on(window.matrixcs.ClientEvent.Event, onEvent); }); + + return Promise.race([timeoutPromise, readyPromise]); }, { roomId, membership }, );