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

@@ -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[]) {
+    private async sendMessageAsClient(cli: Client, room: RoomRef, messages: string[]) {
-        const room = await this.findRoomByName(typeof roomName === "string" ? roomName : roomName.name);
+        const roomId = room.roomId;
-        const roomId = await room.evaluate((room) => 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 }) {
+    async goTo(room: RoomRef) {
-        await this.app.viewRoomByName(typeof room === "string" ? room : room.name);
+        await this.app.viewRoomByName(room.name);
     }
 
     /**

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

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) {
+    async jumpTo(roomRef: RoomRef, message: string, includeThreads = false) {
-        const room = await this.helpers.findRoomByName(roomName);
+        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[]) {
+    async sendMessageAsClient(cli: Client, roomRef: RoomRef, messages: Message[]) {
-        const room = await this.findRoomByName(typeof roomName === "string" ? roomName : roomName.name);
+        const roomId = roomRef.roomId;
-        const roomId = await room.evaluate((room) => room.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 }) {
+    getRoomListTile(label: string) {
-        const roomName = typeof room === "string" ? room : room.name;
+        return this.page.getByRole("treeitem", { name: new RegExp("^" + label) });
-        return this.page.getByRole("treeitem", { name: new RegExp("^" + roomName) });
     }
 
     /**
      * 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 }) {
+    async markAsRead(room: RoomRef) {
-        await this.getRoomListTile(room).click({ button: "right" });
+        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 }) {
+    async assertRead(room: RoomRef) {
-        const tile = this.getRoomListTile(room);
+        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 | ".") {
+    async assertUnread(room: RoomRef, count: number | ".") {
-        const tile = this.getRoomListTile(room);
+        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) {
+    async assertUnreadLessThan(room: RoomRef, lessThan: number) {
-        const tile = this.getRoomListTile(room);
+        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) {
+    async assertUnreadGreaterThan(room: RoomRef, greaterThan: number) {
-        const tile = this.getRoomListTile(room);
+        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<JSHandle<Room>> {
+    async findRoomById(roomId: string): Promise<JSHandle<Room>> {
-        return this.app.client.evaluateHandle((cli, roomName) => {
+        return this.app.client.evaluateHandle((cli, roomId) => {
-            return cli.getRooms().find((r) => r.name === roomName);
+            return cli.getRooms().find((r) => r.roomId === roomId);
-        }, roomName);
+        }, 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);
     }
 

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();

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);

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");
 

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");
     });
 

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[]) {
+    async sendMessageAsClient(cli: Client, roomRef: RoomRef, messages: Message[]) {
-        const room = await this.findRoomByName(typeof roomName === "string" ? roomName : roomName.name);
+        const roomId = roomRef.roomId;
-        const roomId = await room.evaluate((room) => room.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<JSHandle<Room>> {
+    async findRoomById(roomId: string): Promise<JSHandle<Room | undefined>> {
-        return this.app.client.evaluateHandle((cli, roomName) => {
+        return this.app.client.evaluateHandle((cli, roomId) => {
-            return cli.getRooms().find((r) => r.name === roomName);
+            return cli.getRooms().find((r) => r.roomId === roomId);
-        }, roomName);
+        }, 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);
     }
 

playwright/pages/client.ts

@@ -175,18 +175,18 @@ export class Client {
     public async createRoom(options: ICreateRoomOpts): Promise<string> {
         const client = await this.prepareClient();
         return await client.evaluate(async (cli, options) => {
-            const resp = await cli.createRoom(options);
+            const roomPromise = new Promise<void>((resolve) => {
-            const roomId = resp.room_id;
+                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<void>((resolve) => {
+                await roomPromise;
-                    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);