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 more flaky playwright tests #29007

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from
Open
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
21 changes: 11 additions & 10 deletions playwright/e2e/audio-player/audio-player.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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");
Expand Down
1 change: 1 addition & 0 deletions playwright/e2e/knock/create-knock-room.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
});
});
1 change: 1 addition & 0 deletions playwright/e2e/knock/knock-into-room.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
66 changes: 44 additions & 22 deletions playwright/e2e/messages/messages.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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" },
Expand All @@ -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 }) => {
Expand All @@ -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 }) => {
Expand All @@ -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 }) => {
Expand All @@ -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 }) => {
Expand All @@ -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),
);
});
});
});
Expand Down
8 changes: 4 additions & 4 deletions playwright/e2e/pinned-messages/pinned-messages.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
`,
});
},
);
Expand Down
3 changes: 1 addition & 2 deletions playwright/e2e/settings/encryption-user-tab/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Webkit doesn't support clipboard permissions under Playwright

Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();

Expand Down
4 changes: 2 additions & 2 deletions playwright/e2e/spaces/spaces.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();

Expand Down
2 changes: 2 additions & 0 deletions playwright/e2e/spaces/threads-activity-centre/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
1 change: 1 addition & 0 deletions playwright/e2e/timeline/timeline.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
30 changes: 26 additions & 4 deletions playwright/flaky-reporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down
4 changes: 0 additions & 4 deletions playwright/pages/ElementAppPage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
16 changes: 3 additions & 13 deletions playwright/pages/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import type {
ICreateRoomOpts,
ISendEventResponse,
MatrixClient,
Room,
MatrixEvent,
ReceiptType,
IRoomDirectoryOptions,
Expand Down Expand Up @@ -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;
}

/**
Expand Down
8 changes: 6 additions & 2 deletions playwright/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Loading