Skip to content
This repository was archived by the owner on Oct 22, 2024. It is now read-only.

Commit 771d4a8

Browse files
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 <[email protected]> * Add a timeout to `awaitRoomMembership` --------- Co-authored-by: R Midhun Suresh <[email protected]>
1 parent c71dc6b commit 771d4a8

File tree

4 files changed

+68
-4
lines changed

4 files changed

+68
-4
lines changed

Diff for: playwright/e2e/crypto/crypto.spec.ts

+6-1
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ const testMessages = async (page: Page, bob: Bot, bobRoomId: string) => {
4343
};
4444

4545
const bobJoin = async (page: Page, bob: Bot) => {
46+
// Wait for Bob to get the invite
4647
await bob.evaluate(async (cli) => {
4748
const bobRooms = cli.getRooms();
4849
if (!bobRooms.length) {
@@ -55,9 +56,13 @@ const bobJoin = async (page: Page, bob: Bot) => {
5556
});
5657
}
5758
});
58-
const roomId = await bob.joinRoomByName("Alice");
5959

60+
const roomId = await bob.joinRoomByName("Alice");
6061
await expect(page.getByText("Bob joined the room")).toBeVisible();
62+
63+
// Even though Alice has seen Bob's join event, Bob may not have done so yet. Wait for the sync to arrive.
64+
await bob.awaitRoomMembership(roomId);
65+
6166
return roomId;
6267
};
6368

Diff for: playwright/e2e/crypto/event-shields.spec.ts

+6-3
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ test.describe("Cryptography", function () {
3333
await app.client.bootstrapCrossSigning(aliceCredentials);
3434
await autoJoin(bob);
3535

36-
// create an encrypted room
36+
// create an encrypted room, and wait for Bob to join it.
3737
testRoomId = await createSharedRoomWithUser(app, bob.credentials.userId, {
3838
name: "TestRoom",
3939
initial_state: [
@@ -46,6 +46,9 @@ test.describe("Cryptography", function () {
4646
},
4747
],
4848
});
49+
50+
// Even though Alice has seen Bob's join event, Bob may not have done so yet. Wait for the sync to arrive.
51+
await bob.awaitRoomMembership(testRoomId);
4952
});
5053

5154
test("should show the correct shield on e2e events", async ({
@@ -287,9 +290,9 @@ test.describe("Cryptography", function () {
287290
// Let our app start syncing again
288291
await app.client.network.goOnline();
289292

290-
// Wait for the messages to arrive
293+
// Wait for the messages to arrive. It can take quite a while for the sync to wake up.
291294
const last = page.locator(".mx_EventTile_last");
292-
await expect(last).toContainText("test encrypted from unverified");
295+
await expect(last).toContainText("test encrypted from unverified", { timeout: 20000 });
293296
const lastE2eIcon = last.locator(".mx_EventTile_e2eIcon");
294297
await expect(lastE2eIcon).toHaveClass(/mx_EventTile_e2eIcon_warning/);
295298
await lastE2eIcon.focus();

Diff for: playwright/e2e/utils.ts

+8
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,14 @@ import { Client } from "../pages/client";
2020
* @param client Client instance that can be user or bot
2121
* @param roomId room id to find room and check
2222
* @param predicate defines condition that is used to check the room state
23+
*
24+
* FIXME this does not do what it is supposed to do, and I think it is unfixable.
25+
* `page.exposeFunction` adds a function which returns a Promise. `window[predicateId](room)` therefore
26+
* always returns a truthy value (a Promise). But even if you fix that: as far as I can tell, the Room is
27+
* just passed to the callback function as a JSON blob: you cannot actually call any methods on it, so the
28+
* callback is useless.
29+
*
30+
* @deprecated This function is broken.
2331
*/
2432
export async function waitForRoom(
2533
page: Page,

Diff for: playwright/pages/client.ts

+48
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,54 @@ export class Client {
289289
await client.evaluate((client, { roomId, userId }) => client.unban(roomId, userId), { roomId, userId });
290290
}
291291

292+
/**
293+
* Wait for the client to have specific membership of a given room
294+
*
295+
* This is often useful after joining a room, when we need to wait for the sync loop to catch up.
296+
*
297+
* Times out with an error after 1 second.
298+
*
299+
* @param roomId - ID of the room to check
300+
* @param membership - required membership.
301+
*/
302+
public async awaitRoomMembership(roomId: string, membership: string = "join") {
303+
await this.evaluate(
304+
(cli: MatrixClient, { roomId, membership }) => {
305+
const isReady = () => {
306+
// Fetch the room on each check, because we get a different instance before and after the join arrives.
307+
const room = cli.getRoom(roomId);
308+
const myMembership = room?.getMyMembership();
309+
// @ts-ignore access to private field "logger"
310+
cli.logger.info(`waiting for room ${roomId}: membership now ${myMembership}`);
311+
return myMembership === membership;
312+
};
313+
if (isReady()) return;
314+
315+
const timeoutPromise = new Promise((resolve) => setTimeout(resolve, 1000)).then(() => {
316+
const room = cli.getRoom(roomId);
317+
const myMembership = room?.getMyMembership();
318+
throw new Error(
319+
`Timeout waiting for room ${roomId} membership (now '${myMembership}', wanted '${membership}')`,
320+
);
321+
});
322+
323+
const readyPromise = new Promise<void>((resolve) => {
324+
async function onEvent() {
325+
if (isReady()) {
326+
cli.removeListener(window.matrixcs.ClientEvent.Event, onEvent);
327+
resolve();
328+
}
329+
}
330+
331+
cli.on(window.matrixcs.ClientEvent.Event, onEvent);
332+
});
333+
334+
return Promise.race([timeoutPromise, readyPromise]);
335+
},
336+
{ roomId, membership },
337+
);
338+
}
339+
292340
/**
293341
* @param {MatrixEvent} event
294342
* @param {ReceiptType} receiptType

0 commit comments

Comments
 (0)