Fix more flaky playwright tests (#29007)

* Group systemic playwright flakes Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Fix more flaky tests Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Fix another flake Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Iterate Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Iterate Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * delint Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Fix more flakes Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Iterate Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Fix skip tests being wrong Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --------- Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
element-hq · Jan 17, 2025 · e42ee72 · e42ee72
1 parent 7d30413
commit e42ee72
Show file tree

Hide file tree

Showing 14 changed files with 104 additions and 65 deletions.
diff --git a/playwright/e2e/audio-player/audio-player.spec.ts b/playwright/e2e/audio-player/audio-player.spec.ts
@@ -13,6 +13,14 @@ import { SettingLevel } from "../../../src/settings/SettingLevel";
 import { Layout } from "../../../src/settings/enums/Layout";
 import { ElementAppPage } from "../../pages/ElementAppPage";
 
+// Find and click "Reply" button
+const clickButtonReply = async (tile: Locator) => {
+    await expect(async () => {
+        await tile.hover();
+        await tile.getByRole("button", { name: "Reply", exact: true }).click();
+    }).toPass();
+};
+
 test.describe("Audio player", { tag: ["@no-firefox", "@no-webkit"] }, () => {
     test.use({
         displayName: "Hanako",
@@ -222,8 +230,7 @@ test.describe("Audio player", { tag: ["@no-firefox", "@no-webkit"] }, () => {
 
             // Find and click "Reply" button on MessageActionBar
             const tile = page.locator(".mx_EventTile_last");
-            await tile.hover();
-            await tile.getByRole("button", { name: "Reply", exact: true }).click();
+            await clickButtonReply(tile);
 
             // Reply to the player with another audio file
             await uploadFile(page, "playwright/sample-files/1sec.ogg");
@@ -251,26 +258,20 @@ test.describe("Audio player", { tag: ["@no-firefox", "@no-webkit"] }, () => {
 
             const tile = page.locator(".mx_EventTile_last");
 
-            // Find and click "Reply" button
-            const clickButtonReply = async () => {
-                await tile.hover();
-                await tile.getByRole("button", { name: "Reply", exact: true }).click();
-            };
-
             await uploadFile(page, "playwright/sample-files/upload-first.ogg");
 
             // Assert that the audio player is rendered
             await expect(page.locator(".mx_EventTile_last .mx_AudioPlayer_container")).toBeVisible();
 
-            await clickButtonReply();
+            await clickButtonReply(tile);
 
             // Reply to the player with another audio file
             await uploadFile(page, "playwright/sample-files/upload-second.ogg");
 
             // Assert that the audio player is rendered
             await expect(page.locator(".mx_EventTile_last .mx_AudioPlayer_container")).toBeVisible();
 
-            await clickButtonReply();
+            await clickButtonReply(tile);
 
             // Reply to the player with yet another audio file to create a reply chain
             await uploadFile(page, "playwright/sample-files/upload-third.ogg");

diff --git a/playwright/e2e/knock/create-knock-room.spec.ts b/playwright/e2e/knock/create-knock-room.spec.ts
@@ -81,6 +81,7 @@ test.describe("Create Knock Room", () => {
 
         const spotlightDialog = await app.openSpotlight();
         await spotlightDialog.filter(Filter.PublicRooms);
+        await spotlightDialog.search("Cyber");
         await expect(spotlightDialog.results.nth(0)).toContainText("Cybersecurity");
     });
 });
diff --git a/playwright/e2e/knock/knock-into-room.spec.ts b/playwright/e2e/knock/knock-into-room.spec.ts
@@ -284,6 +284,7 @@ test.describe("Knock Into Room", () => {
 
         const spotlightDialog = await app.openSpotlight();
         await spotlightDialog.filter(Filter.PublicRooms);
+        await spotlightDialog.search("Cyber");
         await expect(spotlightDialog.results.nth(0)).toContainText("Cybersecurity");
         await spotlightDialog.results.nth(0).click();
 

diff --git a/playwright/e2e/messages/messages.spec.ts b/playwright/e2e/messages/messages.spec.ts
@@ -58,6 +58,16 @@ async function editMessage(page: Page, message: Locator, newMsg: string): Promis
     await editComposer.press("Enter");
 }
 
+const screenshotOptions = (page?: Page) => ({
+    mask: page ? [page.locator(".mx_MessageTimestamp")] : undefined,
+    // Hide the jump to bottom button in the timeline to avoid flakiness
+    css: `
+        .mx_JumpToBottomButton {
+            display: none !important;
+        }
+    `,
+});
+
 test.describe("Message rendering", () => {
     [
         { direction: "ltr", displayName: "Quentin" },
@@ -79,24 +89,28 @@ test.describe("Message rendering", () => {
                     await page.goto(`#/room/${room.roomId}`);
 
                     const msgTile = await sendMessage(page, "Hello, world!");
-                    await expect(msgTile).toMatchScreenshot(`basic-message-ltr-${direction}displayname.png`, {
-                        mask: [page.locator(".mx_MessageTimestamp")],
-                    });
+                    await expect(msgTile).toMatchScreenshot(
+                        `basic-message-ltr-${direction}displayname.png`,
+                        screenshotOptions(page),
+                    );
                 },
             );
 
             test("should render an LTR emote", async ({ page, user, app, room }) => {
                 await page.goto(`#/room/${room.roomId}`);
 
                 const msgTile = await sendMessage(page, "/me lays an egg");
-                await expect(msgTile).toMatchScreenshot(`emote-ltr-${direction}displayname.png`);
+                await expect(msgTile).toMatchScreenshot(`emote-ltr-${direction}displayname.png`, screenshotOptions());
             });
 
             test("should render an LTR rich text emote", async ({ page, user, app, room }) => {
                 await page.goto(`#/room/${room.roomId}`);
 
                 const msgTile = await sendMessage(page, "/me lays a *free range* egg");
-                await expect(msgTile).toMatchScreenshot(`emote-rich-ltr-${direction}displayname.png`);
+                await expect(msgTile).toMatchScreenshot(
+                    `emote-rich-ltr-${direction}displayname.png`,
+                    screenshotOptions(),
+                );
             });
 
             test("should render an edited LTR message", async ({ page, user, app, room }) => {
@@ -106,9 +120,10 @@ test.describe("Message rendering", () => {
 
                 await editMessage(page, msgTile, "Hello, universe!");
 
-                await expect(msgTile).toMatchScreenshot(`edited-message-ltr-${direction}displayname.png`, {
-                    mask: [page.locator(".mx_MessageTimestamp")],
-                });
+                await expect(msgTile).toMatchScreenshot(
+                    `edited-message-ltr-${direction}displayname.png`,
+                    screenshotOptions(page),
+                );
             });
 
             test("should render a reply of a LTR message", async ({ page, user, app, room }) => {
@@ -122,32 +137,37 @@ test.describe("Message rendering", () => {
                 ]);
 
                 await replyMessage(page, msgTile, "response to multiline message");
-                await expect(msgTile).toMatchScreenshot(`reply-message-ltr-${direction}displayname.png`, {
-                    mask: [page.locator(".mx_MessageTimestamp")],
-                });
+                await expect(msgTile).toMatchScreenshot(
+                    `reply-message-ltr-${direction}displayname.png`,
+                    screenshotOptions(page),
+                );
             });
 
             test("should render a basic RTL text message", async ({ page, user, app, room }) => {
                 await page.goto(`#/room/${room.roomId}`);
 
                 const msgTile = await sendMessage(page, "مرحبا بالعالم!");
-                await expect(msgTile).toMatchScreenshot(`basic-message-rtl-${direction}displayname.png`, {
-                    mask: [page.locator(".mx_MessageTimestamp")],
-                });
+                await expect(msgTile).toMatchScreenshot(
+                    `basic-message-rtl-${direction}displayname.png`,
+                    screenshotOptions(page),
+                );
             });
 
             test("should render an RTL emote", async ({ page, user, app, room }) => {
                 await page.goto(`#/room/${room.roomId}`);
 
                 const msgTile = await sendMessage(page, "/me يضع بيضة");
-                await expect(msgTile).toMatchScreenshot(`emote-rtl-${direction}displayname.png`);
+                await expect(msgTile).toMatchScreenshot(`emote-rtl-${direction}displayname.png`, screenshotOptions());
             });
 
             test("should render a richtext RTL emote", async ({ page, user, app, room }) => {
                 await page.goto(`#/room/${room.roomId}`);
 
                 const msgTile = await sendMessage(page, "/me أضع بيضة *حرة النطاق*");
-                await expect(msgTile).toMatchScreenshot(`emote-rich-rtl-${direction}displayname.png`);
+                await expect(msgTile).toMatchScreenshot(
+                    `emote-rich-rtl-${direction}displayname.png`,
+                    screenshotOptions(),
+                );
             });
 
             test("should render an edited RTL message", async ({ page, user, app, room }) => {
@@ -157,9 +177,10 @@ test.describe("Message rendering", () => {
 
                 await editMessage(page, msgTile, "مرحبا بالكون!");
 
-                await expect(msgTile).toMatchScreenshot(`edited-message-rtl-${direction}displayname.png`, {
-                    mask: [page.locator(".mx_MessageTimestamp")],
-                });
+                await expect(msgTile).toMatchScreenshot(
+                    `edited-message-rtl-${direction}displayname.png`,
+                    screenshotOptions(page),
+                );
             });
 
             test("should render a reply of a RTL message", async ({ page, user, app, room }) => {
@@ -173,9 +194,10 @@ test.describe("Message rendering", () => {
                 ]);
 
                 await replyMessage(page, msgTile, "مرحبا بالعالم!");
-                await expect(msgTile).toMatchScreenshot(`reply-message-trl-${direction}displayname.png`, {
-                    mask: [page.locator(".mx_MessageTimestamp")],
-                });
+                await expect(msgTile).toMatchScreenshot(
+                    `reply-message-trl-${direction}displayname.png`,
+                    screenshotOptions(page),
+                );
             });
         });
     });

diff --git a/playwright/e2e/pinned-messages/pinned-messages.spec.ts b/playwright/e2e/pinned-messages/pinned-messages.spec.ts
@@ -35,10 +35,10 @@ test.describe("Pinned messages", () => {
                 mask: [tile.locator(".mx_MessageTimestamp")],
                 // Hide the jump to bottom button in the timeline to avoid flakiness
                 css: `
-                .mx_JumpToBottomButton {
-                    display: none !important;
-                }
-            `,
+                    .mx_JumpToBottomButton {
+                        display: none !important;
+                    }
+                `,
             });
         },
     );

diff --git a/playwright/e2e/settings/encryption-user-tab/index.ts b/playwright/e2e/settings/encryption-user-tab/index.ts
@@ -89,8 +89,7 @@ class Helpers {
         await expect(dialog.getByText(title, { exact: true })).toBeVisible();
         await expect(dialog).toMatchScreenshot(screenshot);
 
-        const handle = await this.page.evaluateHandle(() => navigator.clipboard.readText());
-        const clipboardContent = await handle.jsonValue();
+        const clipboardContent = await this.app.getClipboard();
         await dialog.getByRole("textbox").fill(clipboardContent);
         await dialog.getByRole("button", { name: confirmButtonLabel }).click();
         await expect(dialog).toMatchScreenshot("default-recovery.png");

diff --git a/playwright/e2e/settings/encryption-user-tab/recovery.spec.ts b/playwright/e2e/settings/encryption-user-tab/recovery.spec.ts
@@ -53,7 +53,7 @@ test.describe("Recovery section in Encryption tab", () => {
 
     test(
         "should change the recovery key",
-        { tag: "@screenshot" },
+        { tag: ["@screenshot", "@no-webkit"] },
         async ({ page, app, homeserver, credentials, util, context }) => {
             await verifySession(app, "new passphrase");
             const dialog = await util.openEncryptionTab();
@@ -81,7 +81,7 @@ test.describe("Recovery section in Encryption tab", () => {
         },
     );
 
-    test("should setup the recovery key", { tag: "@screenshot" }, async ({ page, app, util }) => {
+    test("should setup the recovery key", { tag: ["@screenshot", "@no-webkit"] }, async ({ page, app, util }) => {
         await verifySession(app, "new passphrase");
         await util.removeSecretStorageDefaultKeyId();
 

diff --git a/playwright/e2e/spaces/spaces.spec.ts b/playwright/e2e/spaces/spaces.spec.ts
@@ -84,7 +84,7 @@ test.describe("Spaces", () => {
 
             // Copy matrix.to link
             await page.getByRole("button", { name: "Share invite link" }).click();
-            expect(await app.getClipboardText()).toEqual(`https://matrix.to/#/#lets-have-a-riot:${user.homeServer}`);
+            expect(await app.getClipboard()).toEqual(`https://matrix.to/#/#lets-have-a-riot:${user.homeServer}`);
 
             // Go to space home
             await page.getByRole("button", { name: "Go to my first room" }).click();
@@ -177,7 +177,7 @@ test.describe("Spaces", () => {
         const shareDialog = page.locator(".mx_SpacePublicShare");
         // Copy link first
         await shareDialog.getByRole("button", { name: "Share invite link" }).click();
-        expect(await app.getClipboardText()).toEqual(`https://matrix.to/#/#space:${user.homeServer}`);
+        expect(await app.getClipboard()).toEqual(`https://matrix.to/#/#space:${user.homeServer}`);
         // Start Matrix invite flow
         await shareDialog.getByRole("button", { name: "Invite people" }).click();
 

diff --git a/playwright/e2e/spaces/threads-activity-centre/index.ts b/playwright/e2e/spaces/threads-activity-centre/index.ts
@@ -38,11 +38,13 @@ export const test = base.extend<{
     room1Name: "Room 1",
     room1: async ({ room1Name: name, app, user, bot }, use) => {
         const roomId = await app.client.createRoom({ name, invite: [bot.credentials.userId] });
+        await bot.awaitRoomMembership(roomId);
         await use({ name, roomId });
     },
     room2Name: "Room 2",
     room2: async ({ room2Name: name, app, user, bot }, use) => {
         const roomId = await app.client.createRoom({ name, invite: [bot.credentials.userId] });
+        await bot.awaitRoomMembership(roomId);
         await use({ name, roomId });
     },
     msg: async ({ page, app, util }, use) => {

diff --git a/playwright/e2e/timeline/timeline.spec.ts b/playwright/e2e/timeline/timeline.spec.ts
@@ -1195,6 +1195,7 @@ test.describe("Timeline", () => {
             });
 
             await sendImage(app.client, room.roomId, NEW_AVATAR);
+            await app.timeline.scrollToBottom();
             await expect(page.locator(".mx_MImageBody").first()).toBeVisible();
 
             // Exclude timestamp and read marker from snapshot

diff --git a/playwright/flaky-reporter.ts b/playwright/flaky-reporter.ts
@@ -24,18 +24,40 @@ type PaginationLinks = {
     first?: string;
 };
 
+// We see quite a few test flakes which are caused by the app exploding
+// so we have some magic strings we check the logs for to better track the flake with its cause
+const SPECIAL_CASES = {
+    "ChunkLoadError": "ChunkLoadError",
+    "Unreachable code should not be executed": "Rust crypto panic",
+    "Out of bounds memory access": "Rust crypto memory error",
+};
+
 class FlakyReporter implements Reporter {
     private flakes = new Map<string, TestCase[]>();
 
     public onTestEnd(test: TestCase): void {
         // Ignores flakes on Dendrite and Pinecone as they have their own flakes we do not track
         if (["Dendrite", "Pinecone"].includes(test.parent.project()?.name)) return;
-        const title = `${test.location.file.split("playwright/e2e/")[1]}: ${test.title}`;
+        let failures = [`${test.location.file.split("playwright/e2e/")[1]}: ${test.title}`];
         if (test.outcome() === "flaky") {
-            if (!this.flakes.has(title)) {
-                this.flakes.set(title, []);
+            const timedOutRuns = test.results.filter((result) => result.status === "timedOut");
+            const pageLogs = timedOutRuns.flatMap((result) =>
+                result.attachments.filter((attachment) => attachment.name.startsWith("page-")),
+            );
+            // If a test failed due to a systemic fault then the test is not flaky, the app is, record it as such.
+            const specialCases = Object.keys(SPECIAL_CASES).filter((log) =>
+                pageLogs.some((attachment) => attachment.name.startsWith("page-") && attachment.body.includes(log)),
+            );
+            if (specialCases.length > 0) {
+                failures = specialCases.map((specialCase) => SPECIAL_CASES[specialCase]);
+            }
+
+            for (const title of failures) {
+                if (!this.flakes.has(title)) {
+                    this.flakes.set(title, []);
+                }
+                this.flakes.get(title).push(test);
             }
-            this.flakes.get(title).push(test);
         }
     }
 

diff --git a/playwright/pages/ElementAppPage.ts b/playwright/pages/ElementAppPage.ts
@@ -158,10 +158,6 @@ export class ElementAppPage {
         return button.click();
     }
 
-    public async getClipboardText(): Promise<string> {
-        return this.page.evaluate("navigator.clipboard.readText()");
-    }
-
     public async openSpotlight(): Promise<Spotlight> {
         const spotlight = new Spotlight(this.page);
         await spotlight.open();

diff --git a/playwright/pages/client.ts b/playwright/pages/client.ts
@@ -15,7 +15,6 @@ import type {
     ICreateRoomOpts,
     ISendEventResponse,
     MatrixClient,
-    Room,
     MatrixEvent,
     ReceiptType,
     IRoomDirectoryOptions,
@@ -178,21 +177,12 @@ export class Client {
      */
     public async createRoom(options: ICreateRoomOpts): Promise<string> {
         const client = await this.prepareClient();
-        return await client.evaluate(async (cli, options) => {
+        const roomId = await client.evaluate(async (cli, options) => {
             const { room_id: roomId } = await cli.createRoom(options);
-            if (!cli.getRoom(roomId)) {
-                await new Promise<void>((resolve) => {
-                    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);
+        await this.awaitRoomMembership(roomId);
+        return roomId;
     }
 
     /**

diff --git a/playwright/services.ts b/playwright/services.ts
@@ -155,9 +155,13 @@ export const test = base.extend<TestFixtures, Services & Options>({
         { scope: "worker" },
     ],
 
-    context: async ({ homeserverType, synapseConfig, logger, context, request, homeserver }, use, testInfo) => {
+    context: async (
+        { homeserverType, synapseConfig, logger, context, request, _homeserver, homeserver },
+        use,
+        testInfo,
+    ) => {
         testInfo.skip(
-            !(homeserver instanceof SynapseContainer) && Object.keys(synapseConfig).length > 0,
+            !(_homeserver instanceof SynapseContainer) && Object.keys(synapseConfig).length > 0,
             `Test specifies Synapse config options so is unsupported with ${homeserverType}`,
         );
         homeserver.setRequest(request);