From 0555701829b11133982fffb7d441373a90e8c491 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 2 Jan 2025 15:24:09 +0000 Subject: [PATCH] Stabilise playwright tests using createRoom util (#28802) * Stabilise playwright tests using createRoom util Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Pass around RoomRefs to avoid needing to cross the bridge to resolve a room to its ID Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --------- Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- playwright/e2e/pinned-messages/index.ts | 24 +++----- .../e2e/read-receipts/high-level.spec.ts | 2 +- playwright/e2e/read-receipts/index.ts | 59 ++++++++++--------- .../new-messages-in-threads.spec.ts | 2 +- .../new-messages-main-timeline.spec.ts | 2 +- .../new-messages-thread-roots.spec.ts | 2 +- .../e2e/read-receipts/read-receipts.spec.ts | 6 +- .../spaces/threads-activity-centre/index.ts | 21 ++++--- playwright/pages/client.ts | 22 +++---- 9 files changed, 68 insertions(+), 72 deletions(-) diff --git a/playwright/e2e/pinned-messages/index.ts b/playwright/e2e/pinned-messages/index.ts index 545d0e34389..511b39f6dfb 100644 --- a/playwright/e2e/pinned-messages/index.ts +++ b/playwright/e2e/pinned-messages/index.ts @@ -13,6 +13,8 @@ import { Client } from "../../pages/client"; import { ElementAppPage } from "../../pages/ElementAppPage"; import { Bot } from "../../pages/bot"; +type RoomRef = { name: string; roomId: string }; + /** * Set up for pinned message tests. */ @@ -47,7 +49,7 @@ export class Helpers { * @param room - the name of the room to send messages into * @param messages - the list of messages to send, these can be strings or implementations of MessageSpec like `editOf` */ - async receiveMessages(room: string | { name: string }, messages: string[]) { + async receiveMessages(room: RoomRef, messages: string[]) { await this.sendMessageAsClient(this.bot, room, messages); } @@ -55,9 +57,8 @@ export class Helpers { * Use the supplied client to send messages or perform actions as specified by * the supplied {@link Message} items. */ - private async sendMessageAsClient(cli: Client, roomName: string | { name: string }, messages: string[]) { - const room = await this.findRoomByName(typeof roomName === "string" ? roomName : roomName.name); - const roomId = await room.evaluate((room) => room.roomId); + private async sendMessageAsClient(cli: Client, room: RoomRef, messages: string[]) { + const roomId = room.roomId; for (const message of messages) { await cli.sendMessage(roomId, { body: message, msgtype: "m.text" }); @@ -73,22 +74,11 @@ export class Helpers { } } - /** - * Find a room by its name - * @param roomName - * @private - */ - private async findRoomByName(roomName: string) { - return this.app.client.evaluateHandle((cli, roomName) => { - return cli.getRooms().find((r) => r.name === roomName); - }, roomName); - } - /** * Open the room with the supplied name. */ - async goTo(room: string | { name: string }) { - await this.app.viewRoomByName(typeof room === "string" ? room : room.name); + async goTo(room: RoomRef) { + await this.app.viewRoomByName(room.name); } /** diff --git a/playwright/e2e/read-receipts/high-level.spec.ts b/playwright/e2e/read-receipts/high-level.spec.ts index 457cf994814..2f3221d5390 100644 --- a/playwright/e2e/read-receipts/high-level.spec.ts +++ b/playwright/e2e/read-receipts/high-level.spec.ts @@ -120,7 +120,7 @@ test.describe("Read receipts", { tag: "@mergequeue" }, () => { await util.assertUnread(room2, 40); // When I jump to a message in the middle and page up - await msg.jumpTo(room2.name, "x\ny\nz\nMsg0020"); + await msg.jumpTo(room2, "x\ny\nz\nMsg0020"); await util.pageUp(); // Then the room is still unread diff --git a/playwright/e2e/read-receipts/index.ts b/playwright/e2e/read-receipts/index.ts index 49a0195fd72..62fe5c367d0 100644 --- a/playwright/e2e/read-receipts/index.ts +++ b/playwright/e2e/read-receipts/index.ts @@ -13,6 +13,8 @@ import { Bot } from "../../pages/bot"; import { Client } from "../../pages/client"; import { ElementAppPage } from "../../pages/ElementAppPage"; +type RoomRef = { name: string; roomId: string }; + /** * Set up for a read receipt test: * - Create a user with the supplied name @@ -22,9 +24,9 @@ import { ElementAppPage } from "../../pages/ElementAppPage"; */ export const test = base.extend<{ roomAlphaName?: string; - roomAlpha: { name: string; roomId: string }; + roomAlpha: RoomRef; roomBetaName?: string; - roomBeta: { name: string; roomId: string }; + roomBeta: RoomRef; msg: MessageBuilder; util: Helpers; }>({ @@ -248,12 +250,13 @@ export class MessageBuilder { /** * Find and display a message. * - * @param roomName the name of the room to look inside + * @param roomRef the ref of the room to look inside * @param message the content of the message to fine * @param includeThreads look for messages inside threads, not just the main timeline */ - async jumpTo(roomName: string, message: string, includeThreads = false) { - const room = await this.helpers.findRoomByName(roomName); + async jumpTo(roomRef: RoomRef, message: string, includeThreads = false) { + const room = await this.helpers.findRoomById(roomRef.roomId); + expect(room).toBeTruthy(); const foundMessage = await this.getMessage(room, message, includeThreads); const roomId = await room.evaluate((room) => room.roomId); const foundMessageId = await foundMessage.evaluate((ev) => ev.getId()); @@ -333,9 +336,10 @@ class Helpers { * Use the supplied client to send messages or perform actions as specified by * the supplied {@link Message} items. */ - async sendMessageAsClient(cli: Client, roomName: string | { name: string }, messages: Message[]) { - const room = await this.findRoomByName(typeof roomName === "string" ? roomName : roomName.name); - const roomId = await room.evaluate((room) => room.roomId); + async sendMessageAsClient(cli: Client, roomRef: RoomRef, messages: Message[]) { + const roomId = roomRef.roomId; + const room = await this.findRoomById(roomId); + expect(room).toBeTruthy(); for (const message of messages) { if (typeof message === "string") { @@ -359,7 +363,7 @@ class Helpers { /** * Open the room with the supplied name. */ - async goTo(room: string | { name: string }) { + async goTo(room: RoomRef) { await this.app.viewRoomByName(typeof room === "string" ? room : room.name); } @@ -423,17 +427,16 @@ class Helpers { }); } - getRoomListTile(room: string | { name: string }) { - const roomName = typeof room === "string" ? room : room.name; - return this.page.getByRole("treeitem", { name: new RegExp("^" + roomName) }); + getRoomListTile(label: string) { + return this.page.getByRole("treeitem", { name: new RegExp("^" + label) }); } /** * Click the "Mark as Read" context menu item on the room with the supplied name * in the room list. */ - async markAsRead(room: string | { name: string }) { - await this.getRoomListTile(room).click({ button: "right" }); + async markAsRead(room: RoomRef) { + await this.getRoomListTile(room.name).click({ button: "right" }); await this.page.getByText("Mark as read").click(); } @@ -441,8 +444,8 @@ class Helpers { * Assert that the room with the supplied name is "read" in the room list - i.g. * has not dot or count of unread messages. */ - async assertRead(room: string | { name: string }) { - const tile = this.getRoomListTile(room); + async assertRead(room: RoomRef) { + const tile = this.getRoomListTile(room.name); await expect(tile.locator(".mx_NotificationBadge_dot")).not.toBeVisible(); await expect(tile.locator(".mx_NotificationBadge_count")).not.toBeVisible(); } @@ -452,7 +455,7 @@ class Helpers { * (In practice, this just waits a short while to allow any unread marker to * appear, and then asserts that the room is read.) */ - async assertStillRead(room: string | { name: string }) { + async assertStillRead(room: RoomRef) { await this.page.waitForTimeout(200); await this.assertRead(room); } @@ -462,8 +465,8 @@ class Helpers { * @param room - the name of the room to check * @param count - the numeric count to assert, or if "." specified then a bold/dot (no count) state is asserted */ - async assertUnread(room: string | { name: string }, count: number | ".") { - const tile = this.getRoomListTile(room); + async assertUnread(room: RoomRef, count: number | ".") { + const tile = this.getRoomListTile(room.name); if (count === ".") { await expect(tile.locator(".mx_NotificationBadge_dot")).toBeVisible(); } else { @@ -478,8 +481,8 @@ class Helpers { * @param room - the name of the room to check * @param lessThan - the number of unread messages that is too many */ - async assertUnreadLessThan(room: string | { name: string }, lessThan: number) { - const tile = this.getRoomListTile(room); + async assertUnreadLessThan(room: RoomRef, lessThan: number) { + const tile = this.getRoomListTile(room.name); // https://playwright.dev/docs/test-assertions#expectpoll // .toBeLessThan doesn't have a retry mechanism, so we use .poll await expect @@ -496,8 +499,8 @@ class Helpers { * @param room - the name of the room to check * @param greaterThan - the number of unread messages that is too few */ - async assertUnreadGreaterThan(room: string | { name: string }, greaterThan: number) { - const tile = this.getRoomListTile(room); + async assertUnreadGreaterThan(room: RoomRef, greaterThan: number) { + const tile = this.getRoomListTile(room.name); // https://playwright.dev/docs/test-assertions#expectpoll // .toBeGreaterThan doesn't have a retry mechanism, so we use .poll await expect @@ -531,10 +534,10 @@ class Helpers { }); } - async findRoomByName(roomName: string): Promise> { - return this.app.client.evaluateHandle((cli, roomName) => { - return cli.getRooms().find((r) => r.name === roomName); - }, roomName); + async findRoomById(roomId: string): Promise> { + return this.app.client.evaluateHandle((cli, roomId) => { + return cli.getRooms().find((r) => r.roomId === roomId); + }, roomId); } private async getThreadListTile(rootMessage: string) { @@ -578,7 +581,7 @@ class Helpers { * @param room - the name of the room to send messages into * @param messages - the list of messages to send, these can be strings or implementations of MessageSpec like `editOf` */ - async receiveMessages(room: string | { name: string }, messages: Message[]) { + async receiveMessages(room: RoomRef, messages: Message[]) { await this.sendMessageAsClient(this.bot, room, messages); } diff --git a/playwright/e2e/read-receipts/new-messages-in-threads.spec.ts b/playwright/e2e/read-receipts/new-messages-in-threads.spec.ts index 5407f3cb44a..5eb3da5e5e4 100644 --- a/playwright/e2e/read-receipts/new-messages-in-threads.spec.ts +++ b/playwright/e2e/read-receipts/new-messages-in-threads.spec.ts @@ -101,7 +101,7 @@ test.describe("Read receipts", { tag: "@mergequeue" }, () => { await util.goTo(room1); // When I read an older message in the thread - await msg.jumpTo(room2.name, "InThread0000", true); + await msg.jumpTo(room2, "InThread0000", true); // Then the thread is still marked as unread await util.backToThreadsList(); diff --git a/playwright/e2e/read-receipts/new-messages-main-timeline.spec.ts b/playwright/e2e/read-receipts/new-messages-main-timeline.spec.ts index 92f7b10cdd7..fcc8dbd9deb 100644 --- a/playwright/e2e/read-receipts/new-messages-main-timeline.spec.ts +++ b/playwright/e2e/read-receipts/new-messages-main-timeline.spec.ts @@ -59,7 +59,7 @@ test.describe("Read receipts", { tag: "@mergequeue" }, () => { await util.assertUnread(room2, 30); // When I jump to one of the older messages - await msg.jumpTo(room2.name, "Msg0001"); + await msg.jumpTo(room2, "Msg0001"); // Then the room is still unread, but some messages were read await util.assertUnreadLessThan(room2, 30); diff --git a/playwright/e2e/read-receipts/new-messages-thread-roots.spec.ts b/playwright/e2e/read-receipts/new-messages-thread-roots.spec.ts index 3c8ed7849f4..41efa78a1f9 100644 --- a/playwright/e2e/read-receipts/new-messages-thread-roots.spec.ts +++ b/playwright/e2e/read-receipts/new-messages-thread-roots.spec.ts @@ -49,7 +49,7 @@ test.describe("Read receipts", { tag: "@mergequeue" }, () => { await util.assertUnread(room2, 61); // Sanity // When I jump to an old message and read the thread - await msg.jumpTo(room2.name, "beforeThread0000"); + await msg.jumpTo(room2, "beforeThread0000"); // When the thread is opened, the timeline is scrolled until the thread root reached the center await util.openThread("ThreadRoot"); diff --git a/playwright/e2e/read-receipts/read-receipts.spec.ts b/playwright/e2e/read-receipts/read-receipts.spec.ts index f6515361f23..95ddee7c60e 100644 --- a/playwright/e2e/read-receipts/read-receipts.spec.ts +++ b/playwright/e2e/read-receipts/read-receipts.spec.ts @@ -196,7 +196,7 @@ test.describe("Read receipts", { tag: "@mergequeue" }, () => { await sendThreadedReadReceipt(app, thread1a, main1); // Then the room has only one unread - the one in the thread - await util.goTo(otherRoomName); + await util.goTo({ name: otherRoomName, roomId: otherRoomId }); await util.assertUnreadThread("Message 1"); }); @@ -214,7 +214,7 @@ test.describe("Read receipts", { tag: "@mergequeue" }, () => { // Then the room has no unreads await expect(page.getByLabel(`${otherRoomName}`)).toBeVisible(); - await util.goTo(otherRoomName); + await util.goTo({ name: otherRoomName, roomId: otherRoomId }); await util.assertReadThread("Message 1"); }); @@ -239,7 +239,7 @@ test.describe("Read receipts", { tag: "@mergequeue" }, () => { // receipt is for a later event. The room should therefore be // read, and the thread unread. await expect(page.getByLabel(`${otherRoomName}`)).toBeVisible(); - await util.goTo(otherRoomName); + await util.goTo({ name: otherRoomName, roomId: otherRoomId }); await util.assertUnreadThread("Message 1"); }); diff --git a/playwright/e2e/spaces/threads-activity-centre/index.ts b/playwright/e2e/spaces/threads-activity-centre/index.ts index b2b8473640e..68a58d70945 100644 --- a/playwright/e2e/spaces/threads-activity-centre/index.ts +++ b/playwright/e2e/spaces/threads-activity-centre/index.ts @@ -14,6 +14,8 @@ import { Bot } from "../../../pages/bot"; import { Client } from "../../../pages/client"; import { ElementAppPage } from "../../../pages/ElementAppPage"; +type RoomRef = { name: string; roomId: string }; + /** * Set up for a read receipt test: * - Create a user with the supplied name @@ -181,9 +183,10 @@ export class Helpers { * Use the supplied client to send messages or perform actions as specified by * the supplied {@link Message} items. */ - async sendMessageAsClient(cli: Client, roomName: string | { name: string }, messages: Message[]) { - const room = await this.findRoomByName(typeof roomName === "string" ? roomName : roomName.name); - const roomId = await room.evaluate((room) => room.roomId); + async sendMessageAsClient(cli: Client, roomRef: RoomRef, messages: Message[]) { + const roomId = roomRef.roomId; + const room = await this.findRoomById(roomId); + expect(room).toBeTruthy(); for (const message of messages) { if (typeof message === "string") { @@ -205,7 +208,7 @@ export class Helpers { /** * Open the room with the supplied name. */ - async goTo(room: string | { name: string }) { + async goTo(room: RoomRef) { await this.app.viewRoomByName(typeof room === "string" ? room : room.name); } @@ -220,10 +223,10 @@ export class Helpers { await expect(this.page.locator(".mx_ThreadView_timelinePanelWrapper")).toBeVisible(); } - async findRoomByName(roomName: string): Promise> { - return this.app.client.evaluateHandle((cli, roomName) => { - return cli.getRooms().find((r) => r.name === roomName); - }, roomName); + async findRoomById(roomId: string): Promise> { + return this.app.client.evaluateHandle((cli, roomId) => { + return cli.getRooms().find((r) => r.roomId === roomId); + }, roomId); } /** @@ -231,7 +234,7 @@ export class Helpers { * @param room - the name of the room to send messages into * @param messages - the list of messages to send, these can be strings or implementations of MessageSpec like `editOf` */ - async receiveMessages(room: string | { name: string }, messages: Message[]) { + async receiveMessages(room: RoomRef, messages: Message[]) { await this.sendMessageAsClient(this.bot, room, messages); } diff --git a/playwright/pages/client.ts b/playwright/pages/client.ts index 23b48602e98..ef74a1590ad 100644 --- a/playwright/pages/client.ts +++ b/playwright/pages/client.ts @@ -175,18 +175,18 @@ export class Client { public async createRoom(options: ICreateRoomOpts): Promise { const client = await this.prepareClient(); return await client.evaluate(async (cli, options) => { - const resp = await cli.createRoom(options); - const roomId = resp.room_id; + 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 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); - }); + await roomPromise; } return roomId; }, options);