From 63ce69891c2e78db24d4dd724e4f5445432a7761 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 23 Dec 2024 17:50:36 +0000 Subject: [PATCH 1/2] Fix and stabilise sliding sync playwright tests Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- .../e2e/sliding-sync/sliding-sync.spec.ts | 48 ++++++++++++++----- playwright/element-web-test.ts | 20 -------- .../plugins/sliding-sync-proxy/index.ts | 13 ++++- 3 files changed, 49 insertions(+), 32 deletions(-) diff --git a/playwright/e2e/sliding-sync/sliding-sync.spec.ts b/playwright/e2e/sliding-sync/sliding-sync.spec.ts index 885948980e7..e26525f278b 100644 --- a/playwright/e2e/sliding-sync/sliding-sync.spec.ts +++ b/playwright/e2e/sliding-sync/sliding-sync.spec.ts @@ -8,14 +8,40 @@ 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"; + +const test = base.extend<{ + slidingSyncProxy: ProxyInstance; +}>({ + 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); + }, +}); test.describe("Sliding Sync", () => { let roomId: string; - test.beforeEach(async ({ slidingSyncProxy, page, user, app }) => { + test.beforeEach(async ({ page, user, app }) => { roomId = await app.client.createRoom({ name: "Test Room" }); }); @@ -45,7 +71,7 @@ test.describe("Sliding Sync", () => { 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, }) => { @@ -71,7 +97,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 }) => { // create rooms and check room names are correct const roomIds: string[] = []; for (const fruit of ["Apple", "Pineapple", "Orange"]) { @@ -94,7 +120,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 }) => { // create rooms and check room names are correct const roomIds: string[] = []; for (const fruit of ["Apple", "Pineapple", "Orange"]) { @@ -122,7 +148,7 @@ 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("should show the right unread notifications", async ({ page, app, user, bot }) => { const bob = await createAndJoinBot(app, bot); // send a message in the test room: unread notification count should increment @@ -150,7 +176,7 @@ 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, bot }) => { // TODO: for now. Later we should. await createAndJoinBot(app, bot); @@ -184,7 +210,7 @@ test.describe("Sliding Sync", () => { 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 ({ page, app, bot }) => { await createAndJoinBot(app, bot); const clientUserId = await app.client.evaluate((client) => client.getUserId()); @@ -262,7 +288,7 @@ 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 }) => { await app.client.createRoom({ name: "Other Room" }); await expect(page.getByRole("treeitem", { name: "Other Room" })).toBeVisible(); await app.client.sendMessage(roomId, "Hello world"); @@ -294,7 +320,7 @@ 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 }) => { // 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(roomId, "Permalink me"); @@ -322,7 +348,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"]) { diff --git a/playwright/element-web-test.ts b/playwright/element-web-test.ts index 4fe51d0a8a0..edb862500c0 100644 --- a/playwright/element-web-test.ts +++ b/playwright/element-web-test.ts @@ -121,7 +121,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; } @@ -274,25 +273,6 @@ export const test = base.extend({ 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(); diff --git a/playwright/plugins/sliding-sync-proxy/index.ts b/playwright/plugins/sliding-sync-proxy/index.ts index 3a1075339d0..583d9b8dbf8 100644 --- a/playwright/plugins/sliding-sync-proxy/index.ts +++ b/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 } from "@playwright/test"; import { getFreePort } from "../utils/port"; import { Docker } from "../docker"; import { PG_PASSWORD, PostgresDocker } from "../postgres"; @@ -24,7 +25,10 @@ export class SlidingSyncProxy { private readonly postgresDocker = new PostgresDocker("sliding-sync"); private instance: ProxyInstance; - constructor(private synapseIp: string) {} + constructor( + private synapseIp: string, + private context: BrowserContext, + ) {} async start(): Promise { console.log(new Date(), "Starting sliding sync proxy..."); @@ -49,6 +53,13 @@ export class SlidingSyncProxy { }); console.log(new Date(), "started!"); + const baseUrl = `http://localhost:${port}`; + await this.context.route("**/_matrix/client/unstable/org.matrix.msc3575/sync**", async (route) => { + await route.continue({ + url: new URL(route.request().url().split("/").slice(3).join("/"), baseUrl).href, + }); + }); + this.instance = { containerId, postgresId, port }; return this.instance; } From 7c9984c256d9476ba14d7d34228c36581f882d66 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 23 Dec 2024 18:37:22 +0000 Subject: [PATCH 2/2] Iterate Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- .../e2e/sliding-sync/sliding-sync.spec.ts | 74 ++++++++++--------- playwright/element-web-test.ts | 1 - 2 files changed, 39 insertions(+), 36 deletions(-) diff --git a/playwright/e2e/sliding-sync/sliding-sync.spec.ts b/playwright/e2e/sliding-sync/sliding-sync.spec.ts index e26525f278b..89d7bfc5784 100644 --- a/playwright/e2e/sliding-sync/sliding-sync.spec.ts +++ b/playwright/e2e/sliding-sync/sliding-sync.spec.ts @@ -15,6 +15,8 @@ import { ProxyInstance, SlidingSyncProxy } from "../../plugins/sliding-sync-prox const test = base.extend<{ slidingSyncProxy: ProxyInstance; + testRoom: { roomId: string; name: string }; + joinedBot: Bot; }>({ slidingSyncProxy: async ({ context, page, homeserver }, use) => { const proxy = new SlidingSyncProxy(homeserver.config.dockerUrl, context); @@ -36,15 +38,27 @@ const test = base.extend<{ 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", () => { - let roomId: string; - - test.beforeEach(async ({ page, user, app }) => { - roomId = await app.client.createRoom({ name: "Test Room" }); - }); - const checkOrder = async (wantOrder: string[], page: Page) => { await expect(page.getByRole("group", { name: "Rooms" }).locator(".mx_RoomTile_title")).toHaveText(wantOrder); }; @@ -58,18 +72,8 @@ test.describe("Sliding Sync", () => { }); }; - const createAndJoinBot = async (app: ElementAppPage, bot: Bot): Promise => { - 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); - return bot; - }; + // Load the user fixture for all tests + test.beforeEach(({ user }) => {}); test("should render the Rooms list in reverse chronological order by default and allowing sorting A-Z", async ({ page, @@ -148,11 +152,9 @@ test.describe("Sliding Sync", () => { await checkOrder(["Apple", "Orange", "Pineapple", "Test Room"], page); }); - test("should show the right unread notifications", async ({ page, app, user, bot }) => { - const bob = await createAndJoinBot(app, bot); - + test("should show the right unread notifications", async ({ page, app, user, joinedBot: bob, testRoom }) => { // 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"); @@ -162,7 +164,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.", }); @@ -176,9 +178,8 @@ test.describe("Sliding Sync", () => { ).not.toBeAttached(); }); - test("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" }); @@ -191,7 +192,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); @@ -210,9 +211,12 @@ test.describe("Sliding Sync", () => { expect(locator.locator(".mx_ToggleSwitch_on")).toBeAttached(); }); - test("should show and be able to accept/reject/rescind invites", async ({ page, app, bot }) => { - await createAndJoinBot(app, bot); - + test("should show and be able to accept/reject/rescind invites", async ({ + page, + app, + joinedBot: bot, + testRoom, + }) => { const clientUserId = await app.client.evaluate((client) => client.getUserId()); // invite bot into 3 rooms: @@ -288,10 +292,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("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(); @@ -320,11 +324,11 @@ test.describe("Sliding Sync", () => { }); // Regression test for https://github.com/vector-im/element-web/issues/21462 - test("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(roomId, "Permalink me"); - await app.client.sendMessage(roomId, "Reply to me"); + await app.client.sendMessage(testRoom.roomId, "First message"); + await app.client.sendMessage(testRoom.roomId, "Permalink me"); + await app.client.sendMessage(testRoom.roomId, "Reply to me"); // select the room await page.getByRole("treeitem", { name: "Test Room" }).click(); diff --git a/playwright/element-web-test.ts b/playwright/element-web-test.ts index edb862500c0..49f61b7e7b5 100644 --- a/playwright/element-web-test.ts +++ b/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