From 771d4a84171a71c02dad2f5c69526de29cbf6a5d Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Fri, 11 Oct 2024 12:48:46 +0100 Subject: [PATCH] Fix flaky crypto playwright tests (#143) * 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: add a comment about a broken helper * playwright: fix more flakiness in the shields test This bit can take a while as well. * Update playwright/pages/client.ts Co-authored-by: R Midhun Suresh * Add a timeout to `awaitRoomMembership` --------- Co-authored-by: R Midhun Suresh --- playwright/e2e/crypto/crypto.spec.ts | 7 ++- playwright/e2e/crypto/event-shields.spec.ts | 9 ++-- playwright/e2e/utils.ts | 8 ++++ playwright/pages/client.ts | 48 +++++++++++++++++++++ 4 files changed, 68 insertions(+), 4 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..0544a7c904 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 ({ @@ -287,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(); 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, diff --git a/playwright/pages/client.ts b/playwright/pages/client.ts index 06e05fdcfa..7d62f42ae0 100644 --- a/playwright/pages/client.ts +++ b/playwright/pages/client.ts @@ -289,6 +289,54 @@ 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. + * + * Times out with an error after 1 second. + * + * @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; + + 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); + resolve(); + } + } + + cli.on(window.matrixcs.ClientEvent.Event, onEvent); + }); + + return Promise.race([timeoutPromise, readyPromise]); + }, + { roomId, membership }, + ); + } + /** * @param {MatrixEvent} event * @param {ReceiptType} receiptType