Fix and stabilise sliding sync playwright tests (#28809)

* Fix and stabilise sliding sync playwright tests

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Iterate

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Revert test enablement

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Iterate

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Debug

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Iterate

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Unskip now fixed tests

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

---------

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

playwright/e2e/crypto/event-shields.spec.ts

@@ -53,6 +53,8 @@ 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);
+
+            await app.client.network.setupRoute();
         });
 
         test("should show the correct shield on e2e events", async ({

playwright/e2e/lazy-loading/lazy-loading.spec.ts

@@ -25,12 +25,13 @@ test.describe("Lazy Loading", () => {
         });
     });
 
-    test.beforeEach(async ({ page, homeserver, user, bot }) => {
+    test.beforeEach(async ({ page, homeserver, user, bot, app }) => {
         for (let i = 1; i <= 10; i++) {
             const displayName = `Charly #${i}`;
             const bot = new Bot(page, homeserver, { displayName, startClient: false, autoAcceptInvites: false });
             charlies.push(bot);
         }
+        await app.client.network.setupRoute();
     });
 
     const name = "Lazy Loading Test";

playwright/e2e/sliding-sync/sliding-sync.spec.ts

@@ -8,17 +8,57 @@ Please see LICENSE files in the repository root for full details.
 
 import { Page, Request } from "@playwright/test";
 
-import { test, expect } from "../../element-web-test";
+import { test as base, expect } from "../../element-web-test";
 import type { ElementAppPage } from "../../pages/ElementAppPage";
 import type { Bot } from "../../pages/bot";
+import { ProxyInstance, SlidingSyncProxy } from "../../plugins/sliding-sync-proxy";
 
-test.describe("Sliding Sync", () => {
+const test = base.extend<{
-    let roomId: string;
+    slidingSyncProxy: ProxyInstance;
-
+    testRoom: { roomId: string; name: string };
-    test.beforeEach(async ({ slidingSyncProxy, page, user, app }) => {
+    joinedBot: Bot;
-        roomId = await app.client.createRoom({ name: "Test Room" });
+}>({
+    slidingSyncProxy: async ({ context, page, homeserver }, use) => {
+        const proxy = new SlidingSyncProxy(homeserver.config.dockerUrl, context);
+        const proxyInstance = await proxy.start();
+        const proxyAddress = `http://localhost:${proxyInstance.port}`;
+        await page.addInitScript((proxyAddress) => {
+            window.localStorage.setItem(
+                "mx_local_settings",
+                JSON.stringify({
+                    feature_sliding_sync_proxy_url: proxyAddress,
+                }),
+            );
+            window.localStorage.setItem("mx_labs_feature_feature_sliding_sync", "true");
+        }, proxyAddress);
+        await use(proxyInstance);
+        await proxy.stop();
+    },
+    // Ensure slidingSyncProxy is set up before the user fixture as it relies on an init script
+    credentials: async ({ slidingSyncProxy, credentials }, use) => {
+        await use(credentials);
+    },
+    testRoom: async ({ user, app }, use) => {
+        const name = "Test Room";
+        const roomId = await app.client.createRoom({ name });
+        await use({ roomId, name });
+    },
+    joinedBot: async ({ app, bot, testRoom }, use) => {
+        const roomId = testRoom.roomId;
+        await bot.prepareClient();
+        const bobUserId = await bot.evaluate((client) => client.getUserId());
+        await app.client.evaluate(
+            async (client, { bobUserId, roomId }) => {
+                await client.invite(roomId, bobUserId);
+            },
+            { bobUserId, roomId },
+        );
+        await bot.joinRoom(roomId);
+        await use(bot);
+    },
 });
 
+test.describe("Sliding Sync", () => {
     const checkOrder = async (wantOrder: string[], page: Page) => {
         await expect(page.getByRole("group", { name: "Rooms" }).locator(".mx_RoomTile_title")).toHaveText(wantOrder);
     };
@@ -32,22 +72,13 @@ test.describe("Sliding Sync", () => {
         });
     };
 
-    const createAndJoinBot = async (app: ElementAppPage, bot: Bot): Promise<Bot> => {
+    // Load the user fixture for all tests
-        await bot.prepareClient();
+    test.beforeEach(({ user }) => {});
-        const bobUserId = await bot.evaluate((client) => client.getUserId());
-        await app.client.evaluate(
-            async (client, { bobUserId, roomId }) => {
-                await client.invite(roomId, bobUserId);
-            },
-            { bobUserId, roomId },
-        );
-        await bot.joinRoom(roomId);
-        return bot;
-    };
 
-    test.skip("should render the Rooms list in reverse chronological order by default and allowing sorting A-Z", async ({
+    test("should render the Rooms list in reverse chronological order by default and allowing sorting A-Z", async ({
         page,
         app,
+        testRoom,
     }) => {
         // create rooms and check room names are correct
         for (const fruit of ["Apple", "Pineapple", "Orange"]) {
@@ -55,7 +86,7 @@ test.describe("Sliding Sync", () => {
             await expect(page.getByRole("treeitem", { name: fruit })).toBeVisible();
         }
 
-        // Check count, 3 fruits + 1 room created in beforeEach = 4
+        // Check count, 3 fruits + 1 testRoom = 4
         await expect(page.locator(".mx_RoomSublist_tiles").getByRole("treeitem")).toHaveCount(4);
         await checkOrder(["Orange", "Pineapple", "Apple", "Test Room"], page);
 
@@ -71,7 +102,7 @@ test.describe("Sliding Sync", () => {
         await checkOrder(["Apple", "Orange", "Pineapple", "Test Room"], page);
     });
 
-    test.skip("should move rooms around as new events arrive", async ({ page, app }) => {
+    test("should move rooms around as new events arrive", async ({ page, app, testRoom }) => {
         // create rooms and check room names are correct
         const roomIds: string[] = [];
         for (const fruit of ["Apple", "Pineapple", "Orange"]) {
@@ -94,7 +125,7 @@ test.describe("Sliding Sync", () => {
         await checkOrder(["Pineapple", "Orange", "Apple", "Test Room"], page);
     });
 
-    test.skip("should not move the selected room: it should be sticky", async ({ page, app }) => {
+    test("should not move the selected room: it should be sticky", async ({ page, app, testRoom }) => {
         // create rooms and check room names are correct
         const roomIds: string[] = [];
         for (const fruit of ["Apple", "Pineapple", "Orange"]) {
@@ -122,11 +153,9 @@ test.describe("Sliding Sync", () => {
         await checkOrder(["Apple", "Orange", "Pineapple", "Test Room"], page);
     });
 
-    test.skip("should show the right unread notifications", async ({ page, app, user, bot }) => {
+    test.skip("should show the right unread notifications", async ({ page, user, joinedBot: bob, testRoom }) => {
-        const bob = await createAndJoinBot(app, bot);
-
         // send a message in the test room: unread notification count should increment
-        await bob.sendMessage(roomId, "Hello World");
+        await bob.sendMessage(testRoom.roomId, "Hello World");
 
         const treeItemLocator1 = page.getByRole("treeitem", { name: "Test Room 1 unread message." });
         await expect(treeItemLocator1.locator(".mx_NotificationBadge_count")).toHaveText("1");
@@ -136,7 +165,7 @@ test.describe("Sliding Sync", () => {
         );
 
         // send an @mention: highlight count (red) should be 2.
-        await bob.sendMessage(roomId, `Hello ${user.displayName}`);
+        await bob.sendMessage(testRoom.roomId, `Hello ${user.displayName}`);
         const treeItemLocator2 = page.getByRole("treeitem", {
             name: "Test Room 2 unread messages including mentions.",
         });
@@ -150,9 +179,8 @@ test.describe("Sliding Sync", () => {
         ).not.toBeAttached();
     });
 
-    test.skip("should not show unread indicators", async ({ page, app, bot }) => {
+    test("should not show unread indicators", async ({ page, app, joinedBot: bot, testRoom }) => {
         // TODO: for now. Later we should.
-        await createAndJoinBot(app, bot);
 
         // disable notifs in this room (TODO: CS API call?)
         const locator = page.getByRole("treeitem", { name: "Test Room" });
@@ -165,7 +193,7 @@ test.describe("Sliding Sync", () => {
 
         await checkOrder(["Dummy", "Test Room"], page);
 
-        await bot.sendMessage(roomId, "Do you read me?");
+        await bot.sendMessage(testRoom.roomId, "Do you read me?");
 
         // wait for this message to arrive, tell by the room list resorting
         await checkOrder(["Test Room", "Dummy"], page);
@@ -178,15 +206,18 @@ test.describe("Sliding Sync", () => {
     test("should update user settings promptly", async ({ page, app }) => {
         await app.settings.openUserSettings("Preferences");
         const locator = page.locator(".mx_SettingsFlag").filter({ hasText: "Show timestamps in 12 hour format" });
-        expect(locator).toBeVisible();
+        await expect(locator).toBeVisible();
-        expect(locator.locator(".mx_ToggleSwitch_on")).not.toBeAttached();
+        await expect(locator.locator(".mx_ToggleSwitch_on")).not.toBeAttached();
         await locator.locator(".mx_ToggleSwitch_ball").click();
-        expect(locator.locator(".mx_ToggleSwitch_on")).toBeAttached();
+        await expect(locator.locator(".mx_ToggleSwitch_on")).toBeAttached();
     });
 
-    test.skip("should show and be able to accept/reject/rescind invites", async ({ page, app, bot }) => {
+    test("should show and be able to accept/reject/rescind invites", async ({
-        await createAndJoinBot(app, bot);
+        page,
-
+        app,
+        joinedBot: bot,
+        testRoom,
+    }) => {
         const clientUserId = await app.client.evaluate((client) => client.getUserId());
 
         // invite bot into 3 rooms:
@@ -262,10 +293,10 @@ test.describe("Sliding Sync", () => {
 
     // Regression test for a bug in SS mode, but would be useful to have in non-SS mode too.
     // This ensures we are setting RoomViewStore state correctly.
-    test.skip("should clear the reply to field when swapping rooms", async ({ page, app }) => {
+    test("should clear the reply to field when swapping rooms", async ({ page, app, testRoom }) => {
         await app.client.createRoom({ name: "Other Room" });
         await expect(page.getByRole("treeitem", { name: "Other Room" })).toBeVisible();
-        await app.client.sendMessage(roomId, "Hello world");
+        await app.client.sendMessage(testRoom.roomId, "Hello world");
 
         // select the room
         await page.getByRole("treeitem", { name: "Test Room" }).click();
@@ -294,11 +325,11 @@ test.describe("Sliding Sync", () => {
     });
 
     // Regression test for https://github.com/vector-im/element-web/issues/21462
-    test.skip("should not cancel replies when permalinks are clicked", async ({ page, app }) => {
+    test("should not cancel replies when permalinks are clicked", async ({ page, app, testRoom }) => {
         // we require a first message as you cannot click the permalink text with the avatar in the way
-        await app.client.sendMessage(roomId, "First message");
+        await app.client.sendMessage(testRoom.roomId, "First message");
-        await app.client.sendMessage(roomId, "Permalink me");
+        await app.client.sendMessage(testRoom.roomId, "Permalink me");
-        await app.client.sendMessage(roomId, "Reply to me");
+        await app.client.sendMessage(testRoom.roomId, "Reply to me");
 
         // select the room
         await page.getByRole("treeitem", { name: "Test Room" }).click();
@@ -322,7 +353,7 @@ test.describe("Sliding Sync", () => {
         await expect(page.locator(".mx_ReplyPreview")).toBeVisible();
     });
 
-    test.skip("should send unsubscribe_rooms for every room switch", async ({ page, app }) => {
+    test("should send unsubscribe_rooms for every room switch", async ({ page, app }) => {
         // create rooms and check room names are correct
         const roomIds: string[] = [];
         for (const fruit of ["Apple", "Pineapple", "Orange"]) {

playwright/element-web-test.ts

@@ -23,7 +23,6 @@ import { OAuthServer } from "./plugins/oauth_server";
 import { Crypto } from "./pages/crypto";
 import { Toasts } from "./pages/toasts";
 import { Bot, CreateBotOpts } from "./pages/bot";
-import { ProxyInstance, SlidingSyncProxy } from "./plugins/sliding-sync-proxy";
 import { Webserver } from "./plugins/webserver";
 
 // Enable experimental service worker support
@@ -121,7 +120,6 @@ export interface Fixtures {
     uut?: Locator; // Unit Under Test, useful place to refer a prepared locator
     botCreateOpts: CreateBotOpts;
     bot: Bot;
-    slidingSyncProxy: ProxyInstance;
     labsFlags: string[];
     webserver: Webserver;
 }
@@ -251,6 +249,7 @@ export const test = base.extend<Fixtures>({
     app: async ({ page }, use) => {
         const app = new ElementAppPage(page);
         await use(app);
+        await app.cleanup();
     },
     crypto: async ({ page, homeserver, request }, use) => {
         await use(new Crypto(page, homeserver, request));
@@ -274,25 +273,6 @@ export const test = base.extend<Fixtures>({
         await mailhog.stop();
     },
 
-    slidingSyncProxy: async ({ page, user, homeserver }, use) => {
-        const proxy = new SlidingSyncProxy(homeserver.config.dockerUrl);
-        const proxyInstance = await proxy.start();
-        const proxyAddress = `http://localhost:${proxyInstance.port}`;
-        await page.addInitScript((proxyAddress) => {
-            window.localStorage.setItem(
-                "mx_local_settings",
-                JSON.stringify({
-                    feature_sliding_sync_proxy_url: proxyAddress,
-                }),
-            );
-            window.localStorage.setItem("mx_labs_feature_feature_sliding_sync", "true");
-        }, proxyAddress);
-        await page.goto("/");
-        await page.waitForSelector(".mx_MatrixChat", { timeout: 30000 });
-        await use(proxyInstance);
-        await proxy.stop();
-    },
-
     // eslint-disable-next-line no-empty-pattern
     webserver: async ({}, use) => {
         const webserver = new Webserver();

playwright/pages/ElementAppPage.ts

@@ -37,6 +37,10 @@ export class ElementAppPage {
         return this._timeline;
     }
 
+    public async cleanup() {
+        await this._client?.cleanup();
+    }
+
     /**
      * Open the top left user menu, returning a Locator to the resulting context menu.
      */

playwright/pages/client.ts

@@ -52,6 +52,10 @@ export class Client {
         this.network = new Network(page, this);
     }
 
+    public async cleanup() {
+        await this.network.destroyRoute();
+    }
+
     public evaluate<R, Arg, O extends MatrixClient = MatrixClient>(
         pageFunction: PageFunctionOn<O, Arg, R>,
         arg: Arg,

playwright/pages/network.ts

@@ -6,19 +6,22 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only
 Please see LICENSE files in the repository root for full details.
 */
 
-import type { Page, Request } from "@playwright/test";
+import type { Page, Request, Route } from "@playwright/test";
 import type { Client } from "./client";
 
+/**
+ * Utility class to simulate offline mode by blocking all requests to the homeserver.
+ * Will not affect any requests before `setupRoute` is called,
+ * which happens implicitly using the goOffline/goOnline methods.
+ */
 export class Network {
     private isOffline = false;
-    private readonly setupPromise: Promise<void>;
+    private setupPromise?: Promise<void>;
 
     constructor(
         private page: Page,
         private client: Client,
-    ) {
+    ) {}
-        this.setupPromise = this.setupRoute();
-    }
 
     /**
      * Checks if the request is from the client associated with this network object.
@@ -30,25 +33,47 @@ export class Network {
         return authHeader === `Bearer ${accessToken}`;
     }
 
-    private async setupRoute() {
+    private handler = async (route: Route) => {
-        await this.page.route("**/_matrix/**", async (route) => {
         if (this.isOffline && (await this.isRequestFromOurClient(route.request()))) {
-                route.abort();
+            await route.abort();
         } else {
-                route.continue();
+            await route.continue();
         }
-        });
+    };
+
+    /**
+     * Intercept all /_matrix/ networking requests for client ready to continue/abort them based on offline status
+     * which is set by the goOffline/goOnline methods
+     */
+    public async setupRoute() {
+        if (!this.setupPromise) {
+            this.setupPromise = this.page.route("**/_matrix/**", this.handler);
+        }
+        await this.setupPromise;
     }
 
-    // Intercept all /_matrix/ networking requests for client and fail them
+    /**
+     * Cease intercepting all /_matrix/ networking requests for client
+     */
+    public async destroyRoute() {
+        if (!this.setupPromise) return;
+        await this.page.unroute("**/_matrix/**", this.handler);
+        this.setupPromise = undefined;
+    }
+
+    /**
+     * Reject all /_matrix/ networking requests for client
+     */
     async goOffline(): Promise<void> {
-        await this.setupPromise;
+        await this.setupRoute();
         this.isOffline = true;
     }
 
-    // Remove intercept on all /_matrix/ networking requests for this client
+    /**
+     * Continue all /_matrix/ networking requests for this client
+     */
     async goOnline(): Promise<void> {
-        await this.setupPromise;
+        await this.setupRoute();
         this.isOffline = false;
     }
 }

playwright/plugins/sliding-sync-proxy/index.ts

@@ -6,6 +6,7 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only
 Please see LICENSE files in the repository root for full details.
 */
 
+import type { BrowserContext, Route } from "@playwright/test";
 import { getFreePort } from "../utils/port";
 import { Docker } from "../docker";
 import { PG_PASSWORD, PostgresDocker } from "../postgres";
@@ -24,7 +25,19 @@ export class SlidingSyncProxy {
     private readonly postgresDocker = new PostgresDocker("sliding-sync");
     private instance: ProxyInstance;
 
-    constructor(private synapseIp: string) {}
+    constructor(
+        private synapseIp: string,
+        private context: BrowserContext,
+    ) {}
+
+    private syncHandler = async (route: Route) => {
+        if (!this.instance) return route.abort("blockedbyclient");
+
+        const baseUrl = `http://localhost:${this.instance.port}`;
+        await route.continue({
+            url: new URL(route.request().url().split("/").slice(3).join("/"), baseUrl).href,
+        });
+    };
 
     async start(): Promise<ProxyInstance> {
         console.log(new Date(), "Starting sliding sync proxy...");
@@ -50,10 +63,13 @@ export class SlidingSyncProxy {
         console.log(new Date(), "started!");
 
         this.instance = { containerId, postgresId, port };
+        await this.context.route("**/_matrix/client/unstable/org.matrix.msc3575/sync*", this.syncHandler);
         return this.instance;
     }
 
     async stop(): Promise<void> {
+        await this.context.unroute("**/_matrix/client/unstable/org.matrix.msc3575/sync*", this.syncHandler);
+
         await this.postgresDocker.stop();
         await this.proxyDocker.stop();
         console.log(new Date(), "Stopped sliding sync proxy.");