From 95e235f24f0b98fd455b656ee20f759200c1289a Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 20 Jun 2024 12:40:25 +0100 Subject: [PATCH 1/5] Disable "legacy crypto" playwright project --- playwright.config.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/playwright.config.ts b/playwright.config.ts index 96a8dd95ec5..017381adcce 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -43,10 +43,6 @@ export default defineConfig({ retries: process.env.CI ? 2 : 0, reporter: process.env.CI ? [["blob"], ["github"]] : [["html", { outputFolder: "playwright/html-report" }]], projects: [ - { - name: "Legacy Crypto", - use: { cryptoBackend: "legacy" }, - }, { name: "Rust Crypto", use: { cryptoBackend: "rust" }, From 3dc418b1a9130f9baef1ab4ced80f2814e28bacc Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 20 Jun 2024 12:45:31 +0100 Subject: [PATCH 2/5] playwight: Remove crypto stack conditions We only ever run under the rust stack now, so these conditions are redundant. --- playwright/e2e/crypto/crypto.spec.ts | 28 ++----------------- playwright/e2e/crypto/verification.spec.ts | 3 -- .../e2e/read-receipts/high-level.spec.ts | 2 -- playwright/e2e/timeline/timeline.spec.ts | 4 +-- 4 files changed, 4 insertions(+), 33 deletions(-) diff --git a/playwright/e2e/crypto/crypto.spec.ts b/playwright/e2e/crypto/crypto.spec.ts index 8cac8abcb8d..d29c9d041c4 100644 --- a/playwright/e2e/crypto/crypto.spec.ts +++ b/playwright/e2e/crypto/crypto.spec.ts @@ -322,13 +322,7 @@ test.describe("Cryptography", function () { }); }); - test("should show the correct shield on e2e events", async ({ - page, - app, - bot: bob, - homeserver, - cryptoBackend, - }) => { + test("should show the correct shield on e2e events", async ({ page, app, bot: bob, homeserver }) => { // Bob has a second, not cross-signed, device const bobSecondDevice = new Bot(page, homeserver, { bootstrapSecretStorage: false, @@ -432,14 +426,8 @@ test.describe("Cryptography", function () { await app.viewRoomByName("Bob"); await app.viewRoomByName("TestRoom"); - // some debate over whether this should have a red or a grey shield. Legacy crypto shows a grey shield, - // Rust crypto a red one. await expect(last).toContainText("test encrypted from unverified"); - if (cryptoBackend === "rust") { - await expect(lastE2eIcon).toHaveClass(/mx_EventTile_e2eIcon_warning/); - } else { - await expect(lastE2eIcon).toHaveClass(/mx_EventTile_e2eIcon_normal/); - } + await expect(lastE2eIcon).toHaveClass(/mx_EventTile_e2eIcon_warning/); await lastE2eIcon.focus(); await expect(page.getByRole("tooltip")).toContainText("Encrypted by an unknown or deleted device."); }); @@ -549,9 +537,7 @@ test.describe("Cryptography", function () { app, credentials, user, - cryptoBackend, }) => { - test.skip(cryptoBackend === "legacy", "Not implemented for legacy crypto"); test.setTimeout(60000); // Start with a logged-in session, without key backup, and send a message. @@ -615,11 +601,8 @@ test.describe("Cryptography", function () { app, credentials: aliceCredentials, user: alice, - cryptoBackend, bot: bob, }) => { - test.skip(cryptoBackend === "legacy", "Not implemented for legacy crypto"); - // Bob creates an encrypted room and sends a message to it. He then invites Alice const roomId = await bob.evaluate( async (client, { alice }) => { @@ -723,15 +706,8 @@ test.describe("Cryptography", function () { app, credentials: aliceCredentials, user: alice, - cryptoBackend, bot: bob, }) => { - // The old pre-join UTD hiding code would hide events sent - // before our latest join event, even if the event that we're - // jumping to was decryptable. We test that this no longer happens. - - test.skip(cryptoBackend === "legacy", "Not implemented for legacy crypto"); - // Bob: // - creates an encrypted room, // - invites Alice, diff --git a/playwright/e2e/crypto/verification.spec.ts b/playwright/e2e/crypto/verification.spec.ts index af30a78b016..826d01e638e 100644 --- a/playwright/e2e/crypto/verification.spec.ts +++ b/playwright/e2e/crypto/verification.spec.ts @@ -303,10 +303,7 @@ test.describe("User verification", () => { user: aliceCredentials, toasts, room: { roomId: dmRoomId }, - cryptoBackend, }) => { - test.skip(cryptoBackend === "legacy", "Not implemented for legacy crypto"); - // once Alice has joined, Bob starts the verification const bobVerificationRequest = await bob.evaluateHandle( async (client, { dmRoomId, aliceCredentials }) => { diff --git a/playwright/e2e/read-receipts/high-level.spec.ts b/playwright/e2e/read-receipts/high-level.spec.ts index e237afd64a8..30a3788e3ec 100644 --- a/playwright/e2e/read-receipts/high-level.spec.ts +++ b/playwright/e2e/read-receipts/high-level.spec.ts @@ -249,7 +249,6 @@ test.describe("Read receipts", () => { }); test("Paging up to find old threads that were never read keeps the room unread", async ({ - cryptoBackend, roomAlpha: room1, roomBeta: room2, util, @@ -338,7 +337,6 @@ test.describe("Read receipts", () => { }); test("After marking room as read, paging up to find old threads that were never read leaves the room read", async ({ - cryptoBackend, roomAlpha: room1, roomBeta: room2, util, diff --git a/playwright/e2e/timeline/timeline.spec.ts b/playwright/e2e/timeline/timeline.spec.ts index a011ecdead7..de4df133d0b 100644 --- a/playwright/e2e/timeline/timeline.spec.ts +++ b/playwright/e2e/timeline/timeline.spec.ts @@ -568,9 +568,9 @@ test.describe("Timeline", () => { ); }); - test("should set inline start padding to a hidden event line", async ({ page, app, room, cryptoBackend }) => { + test("should set inline start padding to a hidden event line", async ({ page, app, room }) => { test.skip( - cryptoBackend === "rust", + true, "Disabled due to screenshot test being flaky - https://github.com/element-hq/element-web/issues/26890", ); await sendEvent(app.client, room.roomId); From 4a8c219777c4d7b10e83ca3f3e441258f9d394d5 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 20 Jun 2024 12:52:23 +0100 Subject: [PATCH 3/5] playwright: remove `cryptoBackend` test option --- playwright.config.ts | 1 - playwright/element-web-test.ts | 11 ++--------- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/playwright.config.ts b/playwright.config.ts index 017381adcce..15d5683e96c 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -45,7 +45,6 @@ export default defineConfig({ projects: [ { name: "Rust Crypto", - use: { cryptoBackend: "rust" }, }, ], snapshotDir: "playwright/snapshots", diff --git a/playwright/element-web-test.ts b/playwright/element-web-test.ts index 66b36836766..672f158b9eb 100644 --- a/playwright/element-web-test.ts +++ b/playwright/element-web-test.ts @@ -63,9 +63,7 @@ const CONFIG_JSON: Partial = { }, }; -export type TestOptions = { - cryptoBackend: "legacy" | "rust"; -}; +export type TestOptions = {}; interface CredentialsWithDisplayName extends Credentials { displayName: string; @@ -138,9 +136,8 @@ export const test = base.extend< webserver: Webserver; } >({ - cryptoBackend: ["legacy", { option: true }], config: CONFIG_JSON, - page: async ({ context, page, config, cryptoBackend, labsFlags }, use) => { + page: async ({ context, page, config, labsFlags }, use) => { await context.route(`http://localhost:8080/config.json*`, async (route) => { const json = { ...CONFIG_JSON, ...config }; json["features"] = { @@ -151,10 +148,6 @@ export const test = base.extend< return obj; }, {}), }; - // the default is to use rust now, so set to `false` if on legacy backend - if (cryptoBackend === "legacy") { - json.features.feature_rust_crypto = false; - } await route.fulfill({ json }); }); await use(page); From 32fa0597972bf14d91f450efcfa5bc8783f8b6f3 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 20 Jun 2024 12:58:09 +0100 Subject: [PATCH 4/5] playwright: remove redundant `projects` We don't need this any more --- playwright.config.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/playwright.config.ts b/playwright.config.ts index 15d5683e96c..3f9da597ec1 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -42,11 +42,6 @@ export default defineConfig({ workers: 1, retries: process.env.CI ? 2 : 0, reporter: process.env.CI ? [["blob"], ["github"]] : [["html", { outputFolder: "playwright/html-report" }]], - projects: [ - { - name: "Rust Crypto", - }, - ], snapshotDir: "playwright/snapshots", snapshotPathTemplate: "{snapshotDir}/{testFilePath}/{arg}-{platform}{ext}", }); From 6cb70ff3086de684d52b0a0ec356febdfd5dc30c Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 20 Jun 2024 13:00:07 +0100 Subject: [PATCH 5/5] playwright: remove redundant `TestOptions` --- playwright.config.ts | 4 +- playwright/element-web-test.ts | 134 ++++++++++++++++----------------- 2 files changed, 66 insertions(+), 72 deletions(-) diff --git a/playwright.config.ts b/playwright.config.ts index 3f9da597ec1..458cf98daf0 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -16,11 +16,9 @@ limitations under the License. import { defineConfig } from "@playwright/test"; -import { TestOptions } from "./playwright/element-web-test"; - const baseURL = process.env["BASE_URL"] ?? "http://localhost:8080"; -export default defineConfig({ +export default defineConfig({ use: { viewport: { width: 1280, height: 720 }, ignoreHTTPSErrors: true, diff --git a/playwright/element-web-test.ts b/playwright/element-web-test.ts index 672f158b9eb..75235fe03d0 100644 --- a/playwright/element-web-test.ts +++ b/playwright/element-web-test.ts @@ -63,79 +63,75 @@ const CONFIG_JSON: Partial = { }, }; -export type TestOptions = {}; - interface CredentialsWithDisplayName extends Credentials { displayName: string; } -export const test = base.extend< - TestOptions & { - axe: AxeBuilder; - checkA11y: () => Promise; - - /** - * The contents of the config.json to send when the client requests it. - */ - config: typeof CONFIG_JSON; - - /** - * The options with which to run the {@link #homeserver} fixture. - */ - startHomeserverOpts: StartHomeserverOpts | string; - - homeserver: HomeserverInstance; - oAuthServer: { port: number }; - - /** - * The displayname to use for the user registered in {@link #credentials}. - * - * To set it, call `test.use({ displayName: "myDisplayName" })` in the test file or `describe` block. - * See {@link https://playwright.dev/docs/api/class-test#test-use}. - */ - displayName?: string; - - /** - * A test fixture which registers a test user on the {@link #homeserver} and supplies the details - * of the registered user. - */ - credentials: CredentialsWithDisplayName; - - /** - * The same as {@link https://playwright.dev/docs/api/class-fixtures#fixtures-page|`page`}, - * but adds an initScript which will populate localStorage with the user's details from - * {@link #credentials} and {@link #homeserver}. - * - * Similar to {@link #user}, but doesn't load the app. - */ - pageWithCredentials: Page; - - /** - * A (rather poorly-named) test fixture which registers a user per {@link #credentials}, stores - * the credentials into localStorage per {@link #homeserver}, and then loads the front page of the - * app. - */ - user: CredentialsWithDisplayName; - - /** - * The same as {@link https://playwright.dev/docs/api/class-fixtures#fixtures-page|`page`}, - * but wraps the returned `Page` in a class of utilities for interacting with the Element-Web UI, - * {@link ElementAppPage}. - */ - app: ElementAppPage; - - mailhog: { api: mailhog.API; instance: Instance }; - crypto: Crypto; - room?: { roomId: string }; - toasts: Toasts; - uut?: Locator; // Unit Under Test, useful place to refer a prepared locator - botCreateOpts: CreateBotOpts; - bot: Bot; - slidingSyncProxy: ProxyInstance; - labsFlags: string[]; - webserver: Webserver; - } ->({ +export const test = base.extend<{ + axe: AxeBuilder; + checkA11y: () => Promise; + + /** + * The contents of the config.json to send when the client requests it. + */ + config: typeof CONFIG_JSON; + + /** + * The options with which to run the {@link #homeserver} fixture. + */ + startHomeserverOpts: StartHomeserverOpts | string; + + homeserver: HomeserverInstance; + oAuthServer: { port: number }; + + /** + * The displayname to use for the user registered in {@link #credentials}. + * + * To set it, call `test.use({ displayName: "myDisplayName" })` in the test file or `describe` block. + * See {@link https://playwright.dev/docs/api/class-test#test-use}. + */ + displayName?: string; + + /** + * A test fixture which registers a test user on the {@link #homeserver} and supplies the details + * of the registered user. + */ + credentials: CredentialsWithDisplayName; + + /** + * The same as {@link https://playwright.dev/docs/api/class-fixtures#fixtures-page|`page`}, + * but adds an initScript which will populate localStorage with the user's details from + * {@link #credentials} and {@link #homeserver}. + * + * Similar to {@link #user}, but doesn't load the app. + */ + pageWithCredentials: Page; + + /** + * A (rather poorly-named) test fixture which registers a user per {@link #credentials}, stores + * the credentials into localStorage per {@link #homeserver}, and then loads the front page of the + * app. + */ + user: CredentialsWithDisplayName; + + /** + * The same as {@link https://playwright.dev/docs/api/class-fixtures#fixtures-page|`page`}, + * but wraps the returned `Page` in a class of utilities for interacting with the Element-Web UI, + * {@link ElementAppPage}. + */ + app: ElementAppPage; + + mailhog: { api: mailhog.API; instance: Instance }; + crypto: Crypto; + room?: { roomId: string }; + toasts: Toasts; + uut?: Locator; // Unit Under Test, useful place to refer a prepared locator + botCreateOpts: CreateBotOpts; + bot: Bot; + slidingSyncProxy: ProxyInstance; + labsFlags: string[]; + webserver: Webserver; +}>({ config: CONFIG_JSON, page: async ({ context, page, config, labsFlags }, use) => { await context.route(`http://localhost:8080/config.json*`, async (route) => {