Fix e2e test flakiness (#9249)

* update readme

* update CI.yml

* collect traces on error always

* Try bumping global assertion timeout to 20s

* Add a visibility check for mod action and retry if failed

* re-enabling sidebar close e2e spec

* fixing flaky liveEditing test

* wip

* adjust timeouts to use global val

* config back to normal

* debug flakiness

* try removing await for request, and improve extension console opening logic

* fix assignment

* pr feedback

* config back to normal

* fix welcome starter bricks flakiness

* more flakiness fixes

* fix setup flakiness

* try to fix setup flakiness

* sidebar open flakiness

.github/workflows/ci.yml

@@ -3,13 +3,16 @@ name: CI
 on:
   pull_request:
     branches: [main]
-    paths-ignore:
-      - "**.md"
-      - .gitignore
-      - .editorconfig
-      - LICENSE
-      - "**.iml"
-      - .idea/**
+  ## Ideally we could include the following `paths-ignore` option to avoid unnecessary runs, but
+  ## it conflicts with branch protection rules that always require successful CI job status checks
+  ## See: https://github.com/orgs/community/discussions/13690
+  #    paths-ignore:
+  #      - "**.md"
+  #      - .gitignore
+  #      - .editorconfig
+  #      - LICENSE
+  #      - "**.iml"
+  #      - .idea/**
   push:
     branches:
       - release/**

.secrets.baseline

@@ -149,7 +149,7 @@
         "filename": ".github/workflows/ci.yml",
         "hashed_secret": "3e26d6750975d678acb8fa35a0f69237881576b0",
         "is_verified": false,
-        "line_number": 193
+        "line_number": 196
       }
     ],
     ".github/workflows/e2e-test-pre-release-browsers.yml": [
@@ -273,5 +273,5 @@
       }
     ]
   },
-  "generated_at": "2024-09-13T21:02:21Z"
+  "generated_at": "2024-10-07T19:58:30Z"
 }

end-to-end-tests/README.md

@@ -65,7 +65,8 @@ in the cli.
 
 Focus on testing high-level user behavior and integration points, avoiding duplication of unit test coverage. Each
 test should represent one full feature flow, which may include multiple steps and assertions. Avoid splitting
-a single feature flow across multiple tests, preferring longer tests if necessary.
+a single feature flow across multiple tests, preferring longer tests if necessary. Ensure that new tests are not flaky by running them
+multiple times locally (you can use the `--repeat-each` option for convenience: `npm run test:e2e -- <name-of-test --repeat-each 3`)
 
 ## Debugging Tests
 
@@ -124,6 +125,12 @@ view the report by running
 ./scripts/show-pr-e2e-report.sh -p <pull-request-number>
 ```
 
+You can also pass in the run id instead if you want to view a specific run's report.
+
+```bash
+./scripts/show-pr-e2e-report.sh -r <run-id>
+```
+
 You will need to have `7zz` and `gh` installed to run this script (install using brew).
 
 ```bash

end-to-end-tests/fixtures/authentication.ts

@@ -85,7 +85,7 @@ export const test = mergeTests(
       );
 
       // The admin console automatically opens a new tab to log in and link the newly installed extension to the user's account.
-      const page = await context.waitForEvent("page", { timeout: 10_000 });
+      const page = await context.waitForEvent("page");
 
       await use({ context, page });
 

end-to-end-tests/fixtures/utils.ts

@@ -48,9 +48,7 @@ export const launchPersistentContextWithExtension = async (
 export const getExtensionId = async (context: BrowserContext) => {
   const background =
     context.serviceWorkers()[0] ||
-    (await context.waitForEvent("serviceworker", {
-      timeout: 3000,
-    }));
+    (await context.waitForEvent("serviceworker"));
 
   const extensionId = background.url().split("/")[2];
 

end-to-end-tests/pageObjects/extensionConsole/modsPage.ts

@@ -21,25 +21,37 @@ import { BasePageObject } from "../basePageObject";
 import { ensureVisibility } from "../../utils";
 import { validateRegistryId } from "@/types/helpers";
 import { API_PATHS, UI_PATHS } from "@/data/service/urlPaths";
+import { DEFAULT_TIMEOUT } from "../../../playwright.config";
 
 export class ModTableItem extends BasePageObject {
   dropdownButton = this.getByTestId("ellipsis-menu-button");
   dropdownMenu = this.getByLabel("Menu");
   statusCell = this.getByTestId("status-cell");
 
   async clickAction(actionName: string) {
-    // Wrapped in `toPass` due to flakiness with dropdown visibility
-    // TODO: https://github.com/pixiebrix/pixiebrix-extension/issues/8458
+    // Wrapped in `toPass` due to flakiness with dropdown visibility due to component remounting
     await expect(async () => {
       if (!(await this.dropdownMenu.isVisible())) {
+        await this.dropdownButton.click({
+          timeout: 5000,
+        });
+      }
+
+      try {
+        await this.getByRole("menuitem", { name: actionName }).waitFor({
+          timeout: 5000,
+        });
+      } catch (error) {
+        // Sometimes the action is not visible because the permissions network request has not completed.
+        // Close the dropdown menu if the action is not visible, and try opening again.
         await this.dropdownButton.click();
+        throw error;
       }
 
       await this.getByRole("menuitem", { name: actionName }).click({
-        // Short timeout in order to handle retrying in the `toPass` block.
-        timeout: 500,
+        timeout: 5000,
       });
-    }).toPass({ timeout: 5000 });
+    }).toPass({ timeout: DEFAULT_TIMEOUT });
   }
 }
 
@@ -74,7 +86,7 @@ export class ModsPage extends BasePageObject {
     const contentLoadedLocator = this.getByText("Welcome to PixieBrix!").or(
       this.modTableItems.nth(0),
     );
-    await expect(contentLoadedLocator).toBeVisible({ timeout: 15_000 });
+    await expect(contentLoadedLocator).toBeVisible();
   }
 
   async viewAllMods() {
@@ -162,9 +174,7 @@ export class ActivateModPage extends BasePageObject {
       this.getByRole("heading", { name: "Activate " }),
     ).toBeVisible();
     // Loading the mod details may take a long time. Using ensureVisibility because the modId may be attached and hidden
-    await ensureVisibility(this.getByText(this.modId), {
-      timeout: 10_000,
-    });
+    await ensureVisibility(this.getByText(this.modId));
   }
 
   async getIntegrationConfigField(index: number) {
@@ -190,9 +200,7 @@ export class ActivateModPage extends BasePageObject {
     const modsPage = new ModsPage(this.page, this.extensionId);
     await modsPage.viewActiveMods();
     // Loading mods sometimes takes upwards of 10s
-    await expect(modsPage.modTableItems.getByText(this.modId)).toBeVisible({
-      timeout: 10_000,
-    });
+    await expect(modsPage.modTableItems.getByText(this.modId)).toBeVisible();
     return modsPage;
   }
 

end-to-end-tests/pageObjects/extensionConsole/workshop/workshopPage.ts

@@ -42,6 +42,7 @@ export class WorkshopPage extends BasePageObject {
 
   async findAndSelectMod(modId: string) {
     await this.getByPlaceholder("Start typing to find results").fill(modId);
+    await this.getByRole("cell", { name: modId }).waitFor();
     await this.getByRole("cell", { name: modId }).click();
 
     const editPage = new EditWorkshopModPage(this.page);
@@ -56,9 +57,7 @@ export class WorkshopPage extends BasePageObject {
     const modMedata =
       await createPage.editor.replaceWithModDefinition(modDefinitionName);
     await createPage.createBrickButton.click();
-    await expect(this.getByRole("status").getByText("Created ")).toBeVisible({
-      timeout: 8000,
-    });
+    await expect(this.getByRole("status").getByText("Created ")).toBeVisible();
     return modMedata;
   }
 

end-to-end-tests/pageObjects/pageEditor/pageEditorPage.ts

@@ -30,6 +30,7 @@ import { ModifiesModFormState } from "./utils";
 import { CreateModModal } from "./createModModal";
 import { DeactivateModModal } from "end-to-end-tests/pageObjects/pageEditor/deactivateModModal";
 import { uuidv4 } from "@/types/helpers";
+import { DEFAULT_TIMEOUT } from "../../../playwright.config";
 
 class EditorPane extends BasePageObject {
   editTab = this.getByRole("tab", { name: "Edit" });
@@ -160,15 +161,16 @@ export class PageEditorPage extends BasePageObject {
         "Save the mod to assign a Mod ID",
       ),
     ).toBeVisible();
-    // eslint-disable-next-line playwright/no-wait-for-timeout -- The save button re-mounts several times so we need a slight delay here before playwright clicks
-    await this.page.waitForTimeout(2000);
-    await modListItem.saveButton.click();
 
-    // Handle the "Save new mod" modal
     const saveNewModModal = this.page.locator(".modal-content");
-    await expect(saveNewModModal).toBeVisible();
-    await expect(saveNewModModal.getByText("Save new mod")).toBeVisible();
+    // The save button re-mounts several times so we need to retry clicking the saveButton until the modal is visible
+    // See: https://github.com/pixiebrix/pixiebrix-extension/issues/9266
+    await expect(async () => {
+      await modListItem.saveButton.click();
+      await expect(saveNewModModal).toBeVisible({ timeout: 5000 });
+    }).toPass({ timeout: DEFAULT_TIMEOUT });
 
+    await expect(saveNewModModal.getByText("Save new mod")).toBeVisible();
     // // Can't use getByLabel to target because the field is composed of multiple widgets
     const registryIdInput = saveNewModModal.getByTestId("registryId-id-id");
     const currentId = await registryIdInput.inputValue();

end-to-end-tests/setup/affiliated.setup.ts

@@ -45,7 +45,6 @@ test("authenticate with affiliated user", async ({
       page.getByText(
         "Successfully linked the Browser Extension to your PixieBrix account",
       ),
-      { timeout: 12_000 },
     );
     await expect(page.getByText(E2E_TEST_USER_EMAIL_AFFILIATED)).toBeVisible();
     await expect(page.getByText("Admin Console")).toBeVisible();
@@ -54,7 +53,7 @@ test("authenticate with affiliated user", async ({
   // We need to wait for a couple of seconds to ensure that the deployment is activated in the bg script to avoid flakiness
   // when loading the extension console. If the deployment is not activated, then a modal will pop up prompting for activation.
   // eslint-disable-next-line playwright/no-wait-for-timeout -- no easy way to detect when the bg script is done activating the deployment
-  await page.waitForTimeout(3000);
+  await page.waitForTimeout(5000);
 
   let extensionConsolePage: Page;
   await test.step("Open Extension Console", async () => {

end-to-end-tests/setup/unaffiliated.setup.ts

@@ -47,7 +47,6 @@ test("authenticate with unaffiliated user", async ({
       page.getByText(
         "Successfully linked the Browser Extension to your PixieBrix account",
       ),
-      { timeout: 12_000 },
     );
     await expect(
       page.getByText(E2E_TEST_USER_EMAIL_UNAFFILIATED),
@@ -74,7 +73,7 @@ test("authenticate with unaffiliated user", async ({
       );
       await localIntegrationsPage.goto();
 
-      const popupPromise = context.waitForEvent("page", { timeout: 5000 });
+      const popupPromise = context.waitForEvent("page");
       await localIntegrationsPage.createNewIntegration("Google Drive");
 
       const googleAuthPopup = await popupPromise;

end-to-end-tests/setup/utils.ts

@@ -31,7 +31,7 @@ export const openExtensionConsoleFromAdmin = async (
     await adminPage
       .locator("button")
       .filter({ hasText: "Open Extension Console" })
-      .click();
+      .click({ timeout: 5000 });
 
     extensionConsolePage = context
       .pages()
@@ -40,12 +40,14 @@ export const openExtensionConsoleFromAdmin = async (
     if (!extensionConsolePage) {
       throw new Error("Extension console page not found");
     }
+  }).toPass({ timeout: 20_000 });
 
-    await expect(extensionConsolePage.locator("#container")).toContainText(
-      "Extension Console",
-    );
-    await expect(extensionConsolePage.getByText(userName)).toBeVisible();
-  }).toPass({ timeout: 15_000 });
+  // eslint-disable-next-line @typescript-eslint/no-non-null-assertion -- extensionConsolePage is defined
+  await expect(extensionConsolePage!.locator("#container")).toContainText(
+    "Extension Console",
+  );
+  // eslint-disable-next-line @typescript-eslint/no-non-null-assertion -- extensionConsolePage is defined
+  await expect(extensionConsolePage!.getByText(userName)).toBeVisible();
 
   // eslint-disable-next-line @typescript-eslint/no-non-null-assertion -- extensionConsolePage is defined
   return extensionConsolePage!;

end-to-end-tests/tests/bricks/sidebarEffects.spec.ts

@@ -20,6 +20,7 @@ import { ActivateModPage } from "../../pageObjects/extensionConsole/modsPage";
 // @ts-expect-error -- https://youtrack.jetbrains.com/issue/AQUA-711/Provide-a-run-configuration-for-Playwright-tests-in-specs-with-fixture-imports-only
 import { type Page, test as base } from "@playwright/test";
 import { runModViaQuickBar, getSidebarPage, isSidebarOpen } from "../../utils";
+import { DEFAULT_TIMEOUT } from "../../../playwright.config";
 
 test.describe("sidebar effect bricks", () => {
   test("toggle sidebar brick", async ({ page, extensionId }) => {
@@ -46,6 +47,6 @@ test.describe("sidebar effect bricks", () => {
 
     await expect(() => {
       expect(isSidebarOpen(page, extensionId)).toBe(false);
-    }).toPass({ timeout: 5000 });
+    }).toPass({ timeout: DEFAULT_TIMEOUT });
   });
 });

end-to-end-tests/tests/extensionConsole/activation.spec.ts

@@ -32,6 +32,7 @@ import { type Serializable } from "playwright-core/types/structs";
 import { SERVICE_URL } from "../../env";
 import { ExtensionsShortcutsPage } from "../../pageObjects/extensionsShortcutsPage";
 import { FloatingActionButton } from "../../pageObjects/floatingActionButton";
+import { DEFAULT_TIMEOUT } from "../../../playwright.config";
 
 test("can activate a mod with no config options", async ({
   page,
@@ -121,7 +122,6 @@ test("can activate a mod with built-in integration", async ({
 test("validates activating a mod with required integrations", async ({
   page,
   extensionId,
-  context,
 }) => {
   const modId = "@e2e-testing/summarize-text-open-ai";
   const modActivationPage = new ActivateModPage(page, extensionId, modId);
@@ -160,13 +160,18 @@ test("can activate a mod with a database", async ({ page, extensionId }) => {
 
   await expect(sideBarPage.getByTestId("card").getByText(note)).toBeVisible();
 
-  // Get the correct container element, as the note text and delete button are wrapped in a div
-  await sideBarPage
-    .getByText(`${note} Delete Note`)
-    .getByRole("button", { name: "Delete Note" })
-    .click();
-
-  await expect(sideBarPage.getByTestId("card").getByText(note)).toBeHidden();
+  // Wrapped in a toPass block in case the delete button isn't clicked successfully due to page shifting
+  await expect(async () => {
+    // Get the correct container element, as the note text and delete button are wrapped in a div
+    await sideBarPage
+      .getByText(`${note} Delete Note`)
+      .getByRole("button", { name: "Delete Note" })
+      .click();
+
+    await expect(sideBarPage.getByTestId("card").getByText(note)).toBeHidden({
+      timeout: 5000,
+    });
+  }).toPass({ timeout: DEFAULT_TIMEOUT });
 });
 
 test("activating a mod when the quickbar shortcut is not configured", async ({
@@ -240,14 +245,10 @@ test("can activate a mod via url", async ({ page, extensionId }) => {
 
   await page.goto(activationLink);
 
-  await expect(async () => {
-    await expect(page).toHaveURL(
-      `chrome-extension://${extensionId}/options.html#/marketplace/activate/${modIdUrlEncoded}`,
-    );
-  }).toPass({ timeout: 5000 });
-  await expect(page.getByRole("code")).toContainText(modId, {
-    timeout: 10_000,
-  });
+  await page.waitForURL(
+    `chrome-extension://${extensionId}/options.html#/marketplace/activate/${modIdUrlEncoded}`,
+  );
+  await expect(page.getByRole("code")).toContainText(modId);
 
   const modActivationPage = new ActivateModPage(page, extensionId, modId);
   await modActivationPage.clickActivateAndWaitForModsPageRedirect();

end-to-end-tests/tests/extensionConsole/modsPage.spec.ts

@@ -19,6 +19,7 @@ import { expect, test } from "../../fixtures/testBase";
 import { ModsPage } from "../../pageObjects/extensionConsole/modsPage";
 // @ts-expect-error -- https://youtrack.jetbrains.com/issue/AQUA-711/Provide-a-run-configuration-for-Playwright-tests-in-specs-with-fixture-imports-only
 import { test as base } from "@playwright/test";
+import { DEFAULT_TIMEOUT } from "../../../playwright.config";
 
 test("can open mod in the workshop", async ({ page, extensionId }) => {
   const modId = "@e2e-testing/shared-notes-sidebar";
@@ -32,6 +33,6 @@ test("can open mod in the workshop", async ({ page, extensionId }) => {
     expect(pageTitle).toBe("Edit [Testing] St... | PixieBrix");
   }).toPass({
     // Clicking Edit in Workshop fetches editable packages to determine the surrogate package id
-    timeout: 10_000,
+    timeout: DEFAULT_TIMEOUT,
   });
 });

end-to-end-tests/tests/pageEditor/addStarterBrick.spec.ts

@@ -157,6 +157,11 @@ test("Add starter brick to mod", async ({
   verifyModDefinitionSnapshot,
   chromiumChannel,
 }) => {
+  test.slow(
+    true,
+    "Longer test due to verifying each starter brick definition in one user flow",
+  );
+
   await page.goto("/");
   const pageEditorPage = await newPageEditorPage(page.url());
   const brickPipeline = pageEditorPage.brickActionsPanel.bricks;