From d8f6c12c3dd07d116f964124aa0e2fe242c0d1f6 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Mon, 6 Jan 2025 13:31:33 +0000 Subject: [PATCH 1/2] Fix some flaky playwright tests (#28855) * Fix some flaky playwright tests Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Fix more flaky tests Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Iterate Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Reuse existing assertion Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --------- Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- playwright/e2e/audio-player/audio-player.spec.ts | 1 - playwright/e2e/composer/RTE.spec.ts | 5 ++--- playwright/e2e/login/soft_logout.spec.ts | 4 +++- playwright/e2e/read-receipts/high-level.spec.ts | 2 ++ .../settings/roles-permissions-room-settings-tab.spec.ts | 2 ++ playwright/e2e/spotlight/spotlight.spec.ts | 8 ++++---- 6 files changed, 13 insertions(+), 9 deletions(-) diff --git a/playwright/e2e/audio-player/audio-player.spec.ts b/playwright/e2e/audio-player/audio-player.spec.ts index 89d2002ed6..a6d920dcb8 100644 --- a/playwright/e2e/audio-player/audio-player.spec.ts +++ b/playwright/e2e/audio-player/audio-player.spec.ts @@ -253,7 +253,6 @@ test.describe("Audio player", { tag: ["@no-firefox", "@no-webkit"] }, () => { // Find and click "Reply" button const clickButtonReply = async () => { - await tile.scrollIntoViewIfNeeded(); await tile.hover(); await tile.getByRole("button", { name: "Reply", exact: true }).click(); }; diff --git a/playwright/e2e/composer/RTE.spec.ts b/playwright/e2e/composer/RTE.spec.ts index 418f28e126..3b750000c5 100644 --- a/playwright/e2e/composer/RTE.spec.ts +++ b/playwright/e2e/composer/RTE.spec.ts @@ -79,9 +79,8 @@ test.describe("Composer", () => { // Enter some more text, then send the message await page.getByRole("textbox").pressSequentially("this is the spoiler text "); await page.getByRole("button", { name: "Send message" }).click(); - // Check that a spoiler item has appeared in the timeline and locator the spoiler command text - await expect(page.locator("button.mx_EventTile_spoiler")).toBeVisible(); - await expect(page.getByText("this is the spoiler text")).toBeVisible(); + // Check that a spoiler item has appeared in the timeline and contains the spoiler text + await expect(page.locator("button.mx_EventTile_spoiler")).toHaveText("this is the spoiler text"); }); }); }); diff --git a/playwright/e2e/login/soft_logout.spec.ts b/playwright/e2e/login/soft_logout.spec.ts index cb8f832f9d..884dd3fee6 100644 --- a/playwright/e2e/login/soft_logout.spec.ts +++ b/playwright/e2e/login/soft_logout.spec.ts @@ -32,7 +32,9 @@ test.describe("Soft logout", () => { // back to the welcome page await expect(page).toHaveURL(/\/#\/home/); - await expect(page.getByRole("heading", { name: `Welcome ${user.userId}`, exact: true })).toBeVisible(); + await expect( + page.getByRole("heading", { name: "Now, let's help you get started", exact: true }), + ).toBeVisible(); }); test("still shows the soft-logout page when the page is reloaded after a soft-logout", async ({ diff --git a/playwright/e2e/read-receipts/high-level.spec.ts b/playwright/e2e/read-receipts/high-level.spec.ts index afd790fdb8..627b2d348d 100644 --- a/playwright/e2e/read-receipts/high-level.spec.ts +++ b/playwright/e2e/read-receipts/high-level.spec.ts @@ -11,6 +11,8 @@ Please see LICENSE files in the repository root for full details. import { customEvent, many, test } from "."; test.describe("Read receipts", { tag: "@mergequeue" }, () => { + test.slow(); + test.describe("Ignored events", () => { test("If all events after receipt are unimportant, the room is read", async ({ roomAlpha: room1, diff --git a/playwright/e2e/settings/roles-permissions-room-settings-tab.spec.ts b/playwright/e2e/settings/roles-permissions-room-settings-tab.spec.ts index 856546e91a..1193afe135 100644 --- a/playwright/e2e/settings/roles-permissions-room-settings-tab.spec.ts +++ b/playwright/e2e/settings/roles-permissions-room-settings-tab.spec.ts @@ -37,7 +37,9 @@ test.describe("Roles & Permissions room settings tab", () => { // Change the role of Alice to Moderator (50) await combobox.selectOption("Moderator"); await expect(combobox).toHaveValue("50"); + const respPromise = page.waitForRequest("**/state/**"); await applyButton.click(); + await respPromise; // Reload and check Alice is still Moderator (50) await page.reload(); diff --git a/playwright/e2e/spotlight/spotlight.spec.ts b/playwright/e2e/spotlight/spotlight.spec.ts index f72d88d8cc..d1bb3dec25 100644 --- a/playwright/e2e/spotlight/spotlight.spec.ts +++ b/playwright/e2e/spotlight/spotlight.spec.ts @@ -125,7 +125,7 @@ test.describe("Spotlight", () => { await expect(resultLocator).toHaveCount(1); await expect(resultLocator.first()).toContainText(room1Name); await resultLocator.first().click(); - expect(page.url()).toContain(room1Id); + await expect(page).toHaveURL(new RegExp(`#/room/${room1Id}`)); await expect(roomHeaderName(page)).toContainText(room1Name); }); @@ -139,7 +139,7 @@ test.describe("Spotlight", () => { await expect(resultLocator.first()).toContainText(room1Name); await expect(resultLocator.first()).toContainText("View"); await resultLocator.first().click(); - expect(page.url()).toContain(room1Id); + await expect(page).toHaveURL(new RegExp(`#/room/${room1Id}`)); await expect(roomHeaderName(page)).toContainText(room1Name); }); @@ -153,7 +153,7 @@ test.describe("Spotlight", () => { await expect(resultLocator.first()).toContainText(room2Name); await expect(resultLocator.first()).toContainText("Join"); await resultLocator.first().click(); - expect(page.url()).toContain(room2Id); + await expect(page).toHaveURL(new RegExp(`#/room/${room2Id}`)); await expect(page.locator(".mx_RoomView_MessageList")).toHaveCount(1); await expect(roomHeaderName(page)).toContainText(room2Name); }); @@ -168,7 +168,7 @@ test.describe("Spotlight", () => { await expect(resultLocator.first()).toContainText(room3Name); await expect(resultLocator.first()).toContainText("View"); await resultLocator.first().click(); - expect(page.url()).toContain(room3Id); + await expect(page).toHaveURL(new RegExp(`#/room/${room3Id}`)); await page.getByRole("button", { name: "Join the discussion" }).click(); await expect(roomHeaderName(page)).toHaveText(room3Name); }); From 29624f7bcbfb85dd5f79f84009e960e3ee7c2e65 Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 6 Jan 2025 16:32:10 +0000 Subject: [PATCH 2/2] Add a playwright test for backup reset / deleted (#28858) * Add a playwright test for backup reset / deleted A slightly tricky one to test but an important case that people can hit, and one that otherwise wouldn't get hit a lot during normal usage, so I think probably quite a useful test to have. Mostly though, I'm about to change this to a toast, so I'd like a test to assert that it still works. * Return directly Co-authored-by: Florian Duros * Fix tsdco --------- Co-authored-by: Florian Duros --- playwright/e2e/crypto/backups.spec.ts | 57 +++++++++++++++++++++++++++ playwright/e2e/csAPI.ts | 48 ++++++++++++++++++++++ 2 files changed, 105 insertions(+) create mode 100644 playwright/e2e/csAPI.ts diff --git a/playwright/e2e/crypto/backups.spec.ts b/playwright/e2e/crypto/backups.spec.ts index 5936c2ede5..de40741c04 100644 --- a/playwright/e2e/crypto/backups.spec.ts +++ b/playwright/e2e/crypto/backups.spec.ts @@ -11,6 +11,7 @@ import { type Page } from "@playwright/test"; import { test, expect } from "../../element-web-test"; import { test as masTest, registerAccountMas } from "../oidc"; import { isDendrite } from "../../plugins/homeserver/dendrite"; +import { TestClientServerAPI } from "../csAPI"; async function expectBackupVersionToBe(page: Page, version: string) { await expect(page.locator(".mx_SecureBackupPanel_statusList tr:nth-child(5) td")).toHaveText( @@ -20,6 +21,9 @@ async function expectBackupVersionToBe(page: Page, version: string) { await expect(page.locator(".mx_SecureBackupPanel_statusList tr:nth-child(6) td")).toHaveText(version); } +// These tests register an account with MAS because then we go through the "normal" registration flow +// and crypto gets set up. Using the 'user' fixture create a a user an synthesizes an existing login, +// which is faster but leaves us without crypto set up. masTest.describe("Encryption state after registration", () => { masTest.skip(isDendrite, "does not yet support MAS"); @@ -46,6 +50,59 @@ masTest.describe("Encryption state after registration", () => { }); }); +masTest.describe("Key backup reset from elsewhere", () => { + masTest.skip(isDendrite, "does not yet support MAS"); + + masTest( + "Key backup is disabled when reset from elsewhere", + async ({ page, mailhog, request, masPrepare, homeserver }) => { + const testUsername = "alice"; + const testPassword = "Pa$sW0rD!"; + + // there's a delay before keys are uploaded so the error doesn't appear immediately: use a fake + // clock so we can skip the delay + await page.clock.install(); + + await page.goto("/#/login"); + await page.getByRole("button", { name: "Continue" }).click(); + await registerAccountMas(page, mailhog.api, testUsername, "alice@email.com", testPassword); + + await page.getByRole("button", { name: "Add room" }).click(); + await page.getByRole("menuitem", { name: "New room" }).click(); + await page.getByRole("textbox", { name: "Name" }).fill("test room"); + await page.getByRole("button", { name: "Create room" }).click(); + + // @ts-ignore - this runs in the browser scope where mxMatrixClientPeg is a thing. Here, it is not. + const accessToken = await page.evaluate(() => mxMatrixClientPeg.get().getAccessToken()); + + const csAPI = new TestClientServerAPI(request, homeserver, accessToken); + + const backupInfo = await csAPI.getCurrentBackupInfo(); + + await csAPI.deleteBackupVersion(backupInfo.version); + + await page.getByRole("textbox", { name: "Send an encrypted message…" }).fill("/discardsession"); + await page.getByRole("button", { name: "Send message" }).click(); + + await page + .getByRole("textbox", { name: "Send an encrypted message…" }) + .fill("Message with broken key backup"); + await page.getByRole("button", { name: "Send message" }).click(); + + // Should be the message we sent plus the room creation event + await expect(page.locator(".mx_EventTile")).toHaveCount(2); + await expect( + page.locator(".mx_RoomView_MessageList > .mx_EventTile_last .mx_EventTile_receiptSent"), + ).toBeVisible(); + + // Wait for it to try uploading the key + await page.clock.fastForward(20000); + + await expect(page.getByRole("heading", { level: 1, name: "New Recovery Method" })).toBeVisible(); + }, + ); +}); + test.describe("Backups", () => { test.use({ displayName: "Hanako", diff --git a/playwright/e2e/csAPI.ts b/playwright/e2e/csAPI.ts new file mode 100644 index 0000000000..7fb7bece8d --- /dev/null +++ b/playwright/e2e/csAPI.ts @@ -0,0 +1,48 @@ +/* +Copyright 2024 New Vector Ltd. + +SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only +Please see LICENSE files in the repository root for full details. +*/ + +import { APIRequestContext } from "playwright-core"; +import { KeyBackupInfo } from "matrix-js-sdk/src/crypto-api"; + +import { HomeserverInstance } from "../plugins/homeserver"; + +/** + * A small subset of the Client-Server API used to manipulate the state of the + * account on the homeserver independently of the client under test. + */ +export class TestClientServerAPI { + public constructor( + private request: APIRequestContext, + private homeserver: HomeserverInstance, + private accessToken: string, + ) {} + + public async getCurrentBackupInfo(): Promise { + const res = await this.request.get(`${this.homeserver.config.baseUrl}/_matrix/client/v3/room_keys/version`, { + headers: { Authorization: `Bearer ${this.accessToken}` }, + }); + + return await res.json(); + } + + /** + * Calls the API directly to delete the given backup version + * @param version The version to delete + */ + public async deleteBackupVersion(version: string): Promise { + const res = await this.request.delete( + `${this.homeserver.config.baseUrl}/_matrix/client/v3/room_keys/version/${version}`, + { + headers: { Authorization: `Bearer ${this.accessToken}` }, + }, + ); + + if (!res.ok) { + throw new Error(`Failed to delete backup version: ${res.status}`); + } + } +}