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

@@ -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);

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();

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) {
+ * Fill in the login form in Element with the given creds, and then complete the `CompleteSecurity` step, using the
-        await page.locator(".mx_AuthPage").getByRole("button", { name: "Verify with Recovery Key" }).click();
+ * 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" });
+    await page.locator(".mx_AuthPage").getByRole("button", { name: "Verify with Recovery Key" }).click();
-        // 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
+    const useSecurityKey = page.locator(".mx_Dialog").getByRole("button", { name: "use your Recovery Key" });
-        // for a recovery key straight away. We click the button if it's there so this works in both cases.
+    // If the user has set a recovery *passphrase*, they'll be prompted for that first and have to click
-        if (await useSecurityKey.isVisible()) {
+    // through to enter the recovery key which is what we have here. If they haven't, they'll be prompted
-            await useSecurityKey.click();
+    // for a recovery key straight away. We click the button if it's there so this works in both cases.
-        }
+    if (await useSecurityKey.isVisible()) {
-        // Fill in the recovery key
+        await useSecurityKey.click();
-        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();
     }
+
+    // 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");
 }
 
 /**