From dad1bd68343bb5bcd6e7f9d4c26ac0ff089f472d Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Tue, 9 Sep 2025 20:30:48 +0100 Subject: [PATCH] Fix flaky shields playwright test (#30731) * Playwright: split `logIntoElement` into two Split up this helper function, so that rather than being a single function with an optional argument, it is two separate functions. * Playwright: fix flaky shields test Wait for the application to redirect to `/#/home` after completing security, so that we don't end up racing with it. Fixes https://github.com/element-hq/element-web/issues/28836 --- playwright/e2e/crypto/event-shields.spec.ts | 4 +- playwright/e2e/crypto/toasts.spec.ts | 6 +-- playwright/e2e/crypto/utils.ts | 44 +++++++++++++-------- 3 files changed, 32 insertions(+), 22 deletions(-) diff --git a/playwright/e2e/crypto/event-shields.spec.ts b/playwright/e2e/crypto/event-shields.spec.ts index 326ee2521f..722a30a0d2 100644 --- a/playwright/e2e/crypto/event-shields.spec.ts +++ b/playwright/e2e/crypto/event-shields.spec.ts @@ -14,7 +14,7 @@ import { createSecondBotDevice, createSharedRoomWithUser, enableKeyBackup, - logIntoElement, + logIntoElementAndVerify, logOutOfElement, verify, waitForDevices, @@ -195,7 +195,7 @@ test.describe("Cryptography", function () { window.localStorage.clear(); }); await page.reload(); - await logIntoElement(page, aliceCredentials, securityKey); + await logIntoElementAndVerify(page, aliceCredentials, securityKey); /* go back to the test room and find Bob's message again */ await app.viewRoomById(testRoomId); diff --git a/playwright/e2e/crypto/toasts.spec.ts b/playwright/e2e/crypto/toasts.spec.ts index 592076afa0..c96615d7eb 100644 --- a/playwright/e2e/crypto/toasts.spec.ts +++ b/playwright/e2e/crypto/toasts.spec.ts @@ -8,7 +8,7 @@ import { type GeneratedSecretStorageKey } from "matrix-js-sdk/src/crypto-api"; import { test, expect } from "../../element-web-test"; -import { createBot, deleteCachedSecrets, disableKeyBackup, logIntoElement } from "./utils"; +import { createBot, deleteCachedSecrets, disableKeyBackup, logIntoElementAndVerify } from "./utils"; import { type Bot } from "../../pages/bot"; test.describe("Key storage out of sync toast", () => { @@ -18,7 +18,7 @@ test.describe("Key storage out of sync toast", () => { const res = await createBot(page, homeserver, credentials); recoveryKey = res.recoveryKey; - await logIntoElement(page, credentials, recoveryKey.encodedPrivateKey); + await logIntoElementAndVerify(page, credentials, recoveryKey.encodedPrivateKey); await deleteCachedSecrets(page); @@ -65,7 +65,7 @@ test.describe("'Turn on key storage' toast", () => { const recoveryKey = res.recoveryKey; botClient = res.botClient; - await logIntoElement(page, credentials, recoveryKey.encodedPrivateKey); + await logIntoElementAndVerify(page, credentials, recoveryKey.encodedPrivateKey); // We won't be prompted for crypto setup unless we have an e2e room, so make one await page.getByRole("button", { name: "Add room" }).click(); diff --git a/playwright/e2e/crypto/utils.ts b/playwright/e2e/crypto/utils.ts index 42bc4d643e..f3660ae747 100644 --- a/playwright/e2e/crypto/utils.ts +++ b/playwright/e2e/crypto/utils.ts @@ -206,32 +206,42 @@ export async function checkDeviceIsConnectedKeyBackup( /** * Fill in the login form in element with the given creds. - * - * If a `securityKey` is given, verifies the new device using the key. */ -export async function logIntoElement(page: Page, credentials: Credentials, securityKey?: string) { +export async function logIntoElement(page: Page, credentials: Credentials) { await page.goto("/#/login"); await page.getByRole("textbox", { name: "Username" }).fill(credentials.userId); await page.getByPlaceholder("Password").fill(credentials.password); await page.getByRole("button", { name: "Sign in" }).click(); +} - // if a securityKey was given, verify the new device - if (securityKey !== undefined) { - await page.locator(".mx_AuthPage").getByRole("button", { name: "Verify with Recovery Key" }).click(); +/** + * Fill in the login form in Element with the given creds, and then complete the `CompleteSecurity` step, using the + * given recovery key. (Normally this will verify the new device using the secrets from 4S.) + * + * Afterwards, waits for the application to redirect to the home page. + */ +export async function logIntoElementAndVerify(page: Page, credentials: Credentials, recoveryKey: string) { + await logIntoElement(page, credentials); - const useSecurityKey = page.locator(".mx_Dialog").getByRole("button", { name: "use your Recovery Key" }); - // If the user has set a recovery *passphrase*, they'll be prompted for that first and have to click - // through to enter the recovery key which is what we have here. If they haven't, they'll be prompted - // for a recovery key straight away. We click the button if it's there so this works in both cases. - if (await useSecurityKey.isVisible()) { - await useSecurityKey.click(); - } - // Fill in the recovery key - await page.locator(".mx_Dialog").getByTitle("Recovery key").fill(securityKey); - await page.getByRole("button", { name: "Continue", disabled: false }).click(); - await page.getByRole("button", { name: "Done" }).click(); + await page.locator(".mx_AuthPage").getByRole("button", { name: "Verify with Recovery Key" }).click(); + + const useSecurityKey = page.locator(".mx_Dialog").getByRole("button", { name: "use your Recovery Key" }); + // If the user has set a recovery *passphrase*, they'll be prompted for that first and have to click + // through to enter the recovery key which is what we have here. If they haven't, they'll be prompted + // for a recovery key straight away. We click the button if it's there so this works in both cases. + if (await useSecurityKey.isVisible()) { + await useSecurityKey.click(); } + + // Fill in the recovery key + await page.locator(".mx_Dialog").getByTitle("Recovery key").fill(recoveryKey); + await page.getByRole("button", { name: "Continue", disabled: false }).click(); + await page.getByRole("button", { name: "Done" }).click(); + + // The application should now redirect to `/#/home`. Wait for that to happen, otherwise if a test immediately does + // a `viewRoomById` or similar, it could race. + await page.waitForURL("/#/home"); } /**