Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix and stabilise sliding sync playwright tests #28809

Merged
merged 8 commits into from
Jan 2, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions playwright/e2e/crypto/event-shields.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 ({
Expand Down
3 changes: 2 additions & 1 deletion playwright/e2e/lazy-loading/lazy-loading.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
119 changes: 75 additions & 44 deletions playwright/e2e/sliding-sync/sliding-sync.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,57 @@

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;
testRoom: { roomId: string; name: string };
joinedBot: Bot;
}>({
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", () => {
let roomId: string;

test.beforeEach(async ({ slidingSyncProxy, 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);
};
Expand All @@ -32,30 +72,21 @@
});
};

const createAndJoinBot = async (app: ElementAppPage, bot: Bot): Promise<Bot> => {
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.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"]) {
await app.client.createRoom({ name: fruit });
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);

Expand All @@ -71,7 +102,7 @@
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"]) {
Expand All @@ -94,7 +125,7 @@
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"]) {
Expand Down Expand Up @@ -122,11 +153,9 @@
await checkOrder(["Apple", "Orange", "Pineapple", "Test Room"], page);
});

test.skip("should show the right unread notifications", async ({ page, app, user, bot }) => {
const bob = await createAndJoinBot(app, bot);

test.skip("should show the right unread notifications", async ({ page, 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");
Expand All @@ -136,7 +165,7 @@
);

// 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.",
});
Expand All @@ -150,9 +179,8 @@
).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" });
Expand All @@ -165,7 +193,7 @@

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);
Expand All @@ -178,15 +206,18 @@
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();
expect(locator.locator(".mx_ToggleSwitch_on")).not.toBeAttached();
await expect(locator).toBeVisible();
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 }) => {
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:
Expand Down Expand Up @@ -262,10 +293,10 @@

// 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();
Expand Down Expand Up @@ -294,11 +325,11 @@
});

// 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(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();
Expand All @@ -322,7 +353,7 @@
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"]) {
Expand All @@ -339,7 +370,7 @@
return;
}
expect(body.unsubscribe_rooms).toEqual([unsubRoomId]);
expect(body.room_subscriptions).not.toHaveProperty(unsubRoomId);

Check failure on line 373 in playwright/e2e/sliding-sync/sliding-sync.spec.ts

View workflow job for this annotation

GitHub Actions / Run Tests [Chrome] 5/6

[Chrome] › sliding-sync/sliding-sync.spec.ts:356:5 › Sliding Sync › should send unsubscribe_rooms for every room switch

1) [Chrome] › sliding-sync/sliding-sync.spec.ts:356:5 › Sliding Sync › should send unsubscribe_rooms for every room switch Error: expect(received).not.toHaveProperty(path) Matcher error: received value must not be null nor undefined Received has value: undefined 371 | } 372 | expect(body.unsubscribe_rooms).toEqual([unsubRoomId]); > 373 | expect(body.room_subscriptions).not.toHaveProperty(unsubRoomId); | ^ 374 | expect(body.room_subscriptions).toHaveProperty(subRoomId); 375 | }; 376 | at assertUnsubExists (/home/runner/work/element-web/element-web/playwright/e2e/sliding-sync/sliding-sync.spec.ts:373:49) at /home/runner/work/element-web/element-web/playwright/e2e/sliding-sync/sliding-sync.spec.ts:394:9
expect(body.room_subscriptions).toHaveProperty(subRoomId);
};

Expand Down
22 changes: 1 addition & 21 deletions playwright/element-web-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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));
Expand All @@ -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();
Expand Down
4 changes: 4 additions & 0 deletions playwright/pages/ElementAppPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down
4 changes: 4 additions & 0 deletions playwright/pages/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading
Loading