fix e2e test flakiness due to workshop page fixture

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

@@ -37,13 +37,19 @@ function normalizeUUIDs(string: string) {
 }
 
 export const test = pageContextFixture.extend<{
+  /**
+   * Returns a function that accepts a callback which is passed in a new workshopPage.
+   * The page is closed after the callback is executed.
+   * @param callback The callback to execute with the workshopPage.
+   */
+  withWorkshopPage: (
+    callback: (workshopPage: WorkshopPage) => Promise<void>,
+  ) => Promise<void>;
   /**
    * Names of the mod definitions to create and track in the test. These should correspond
    * 1-1 with the mod definition file names in the fixtures/modDefinitions directory.
    */
   modDefinitionNames: string[];
-  // Used for verifying mod definition snapshots in a separate tab.
-  _workshopPage: WorkshopPage;
   /**
    * A map of mod names to their test metadata. This is used to track the mod definitions for
    * snapshot verifying them. These are updated each time a mod definition snapshot is verified.
@@ -66,45 +72,49 @@ export const test = pageContextFixture.extend<{
     prevModId?: string;
   }) => Promise<void>;
 }>({
-  modDefinitionNames: [],
-  async _workshopPage({ context, extensionId }, use) {
-    const newPage = await context.newPage();
-    const workshopPage = new WorkshopPage(newPage, extensionId);
-    await workshopPage.goto();
-    await use(workshopPage);
-    await newPage.close();
+  async withWorkshopPage({ extensionId, context }, use) {
+    await use(
+      async (callback: (workshopPage: WorkshopPage) => Promise<void>) => {
+        const newPage = await context.newPage();
+        const workshopPage = new WorkshopPage(newPage, extensionId);
+        await workshopPage.goto();
+        await callback(workshopPage);
+        await newPage.close();
+      },
+    );
   },
+  modDefinitionNames: [],
   modDefinitionsMap: [
-    async ({ modDefinitionNames, page, extensionId }, use) => {
+    async ({ modDefinitionNames, withWorkshopPage }, use) => {
       const createdModDefinitions: ModDefinitions = {};
       if (modDefinitionNames.length > 0) {
-        const workshopPage = new WorkshopPage(page, extensionId);
         for (const name of modDefinitionNames) {
-          await workshopPage.goto();
-          const modMetadata =
-            await workshopPage.createNewModFromDefinition(name);
-          createdModDefinitions[name] = { ...modMetadata, autoCleanup: true };
+          await withWorkshopPage(async (workshopPage) => {
+            const modMetadata =
+              await workshopPage.createNewModFromDefinition(name);
+            createdModDefinitions[name] = { ...modMetadata, autoCleanup: true };
+          });
         }
       }
 
       await use(createdModDefinitions);
 
       if (Object.keys(createdModDefinitions).length > 0) {
-        const workshopPage = new WorkshopPage(page, extensionId);
         for (const { id, autoCleanup } of Object.values(
           createdModDefinitions,
         )) {
           if (autoCleanup) {
-            await workshopPage.goto();
-            await workshopPage.deleteModByModId(id);
+            await withWorkshopPage(async (workshopPage) => {
+              await workshopPage.deleteModByModId(id);
+            });
           }
         }
       }
     },
     { auto: true },
   ],
   async verifyModDefinitionSnapshot(
-    { _workshopPage: workshopPage, modDefinitionsMap },
+    { modDefinitionsMap, withWorkshopPage },
     use,
     testInfo,
   ) {
@@ -121,83 +131,83 @@ export const test = pageContextFixture.extend<{
       mode?: "diff" | "current";
       prevModId?: string;
     }) => {
-      await workshopPage.goto();
-      const editPage = await workshopPage.findAndSelectMod(modId);
+      await withWorkshopPage(async (workshopPage) => {
+        const editPage = await workshopPage.findAndSelectMod(modId);
+        const currentModDefinitionYaml = await editPage.editor.getValue();
+        // See if this mod is being tracked in modDefinitions.
+        const lastModDefinitionEntry = Object.entries(modDefinitionsMap).find(
+          ([_name, { id }]) => {
+            if (prevModId) {
+              return id === prevModId;
+            }
 
-      const currentModDefinitionYaml = await editPage.editor.getValue();
-      // See if this mod is being tracked in modDefinitions.
-      const lastModDefinitionEntry = Object.entries(modDefinitionsMap).find(
-        ([_name, { id }]) => {
-          if (prevModId) {
-            return id === prevModId;
+            return id === modId;
+          },
+        );
+
+        if (mode === "diff") {
+          if (!lastModDefinitionEntry) {
+            throw new Error(
+              `Mod definition for ${
+                prevModId ?? modId
+              } not found in modDefinitions. Cannot verify a diff. Use mode 'current' to get the baseline snapshot.`,
+            );
           }
 
-          return id === modId;
-        },
-      );
+          const [
+            modDefinitionName,
+            { definition: lastModDefinition, autoCleanup },
+          ] = lastModDefinitionEntry;
 
-      if (mode === "diff") {
-        if (!lastModDefinitionEntry) {
-          throw new Error(
-            `Mod definition for ${
-              prevModId ?? modId
-            } not found in modDefinitions. Cannot verify a diff. Use mode 'current' to get the baseline snapshot.`,
+          const parsedCurrentModDefinitionYaml = loadBrickYaml(
+            currentModDefinitionYaml,
+          );
+          const parsedLastModDefinitionYaml = loadBrickYaml(lastModDefinition);
+          const yamlDiff = createPatch(
+            snapshotName,
+            normalizeUUIDs(
+              dumpBrickYaml(parsedLastModDefinitionYaml, {
+                indent: 2,
+                sortKeys: true,
+              }),
+            ),
+            normalizeUUIDs(
+              dumpBrickYaml(parsedCurrentModDefinitionYaml, {
+                indent: 2,
+                sortKeys: true,
+              }),
+            ),
+            undefined,
+            undefined,
+            { context: 40 },
           );
-        }
-
-        const [
-          modDefinitionName,
-          { definition: lastModDefinition, autoCleanup },
-        ] = lastModDefinitionEntry;
-
-        const parsedCurrentModDefinitionYaml = loadBrickYaml(
-          currentModDefinitionYaml,
-        );
-        const parsedLastModDefinitionYaml = loadBrickYaml(lastModDefinition);
-        const yamlDiff = createPatch(
-          snapshotName,
-          normalizeUUIDs(
-            dumpBrickYaml(parsedLastModDefinitionYaml, {
-              indent: 2,
-              sortKeys: true,
-            }),
-          ),
-          normalizeUUIDs(
-            dumpBrickYaml(parsedCurrentModDefinitionYaml, {
-              indent: 2,
-              sortKeys: true,
-            }),
-          ),
-          undefined,
-          undefined,
-          { context: 40 },
-        );
 
-        expect(yamlDiff).toMatchSnapshot(snapshotName + ".diff");
+          expect(yamlDiff).toMatchSnapshot(snapshotName + ".diff");
 
-        // Update the mod definition to the last known state
-        modDefinitionsMap[modDefinitionName] = {
-          id: modId,
-          definition: currentModDefinitionYaml,
-          autoCleanup,
-        };
-      } else {
-        const normalizedModDefinitionYaml = normalizeUUIDs(
-          currentModDefinitionYaml,
-        );
-        expect(normalizedModDefinitionYaml).toMatchSnapshot(
-          snapshotName + ".yaml",
-        );
+          // Update the mod definition to the last known state
+          modDefinitionsMap[modDefinitionName] = {
+            id: modId,
+            definition: currentModDefinitionYaml,
+            autoCleanup,
+          };
+        } else {
+          const normalizedModDefinitionYaml = normalizeUUIDs(
+            currentModDefinitionYaml,
+          );
+          expect(normalizedModDefinitionYaml).toMatchSnapshot(
+            snapshotName + ".yaml",
+          );
 
-        // Use the mod definition name to update the mod definition if it exists, otherwise fallback to the modId
-        const name = lastModDefinitionEntry?.[0] ?? modId;
-        const autoCleanup = Boolean(modDefinitionsMap[name]?.autoCleanup);
-        modDefinitionsMap[name] = {
-          id: modId,
-          definition: currentModDefinitionYaml,
-          autoCleanup,
-        };
-      }
+          // Use the mod definition name to update the mod definition if it exists, otherwise fallback to the modId
+          const name = lastModDefinitionEntry?.[0] ?? modId;
+          const autoCleanup = Boolean(modDefinitionsMap[name]?.autoCleanup);
+          modDefinitionsMap[name] = {
+            id: modId,
+            definition: currentModDefinitionYaml,
+            autoCleanup,
+          };
+        }
+      });
     };
 
     await use(_verifyModDefinitionSnapshot);

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

@@ -18,7 +18,6 @@
 import { expect, type Page } from "@playwright/test";
 import { getBaseExtensionConsoleUrl } from "../constants";
 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";
@@ -169,12 +168,19 @@ export class ActivateModPage extends BasePageObject {
 
   async goto() {
     await this.page.goto(this.activateModUrl);
-
-    await expect(
-      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));
+    // Wrapped in toPass due to flakiness with the page not loading ex:
+    // https://github.com/pixiebrix/pixiebrix-extension/actions/runs/11373118427?pr=9286
+    await expect(async () => {
+      await this.getByRole("heading", { name: "Activate " }).waitFor({
+        timeout: 10_000,
+      });
+      try {
+        await this.getByText(this.modId).waitFor({ timeout: 10_000 });
+      } catch (error) {
+        await this.page.reload();
+        throw error;
+      }
+    }).toPass({ timeout: 30_000 });
   }
 
   async getIntegrationConfigField(index: number) {

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

@@ -38,6 +38,8 @@ export class WorkshopPage extends BasePageObject {
     await this.getByRole("link", {
       name: "Workshop",
     }).click();
+    await this.getByRole("heading", { name: "Workshop" }).waitFor();
+    await this.getByRole("cell").first().waitFor();
   }
 
   async findAndSelectMod(modId: string) {

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

@@ -17,7 +17,6 @@
 
 import { getBasePageEditorUrl } from "../constants";
 import { type Page, expect, type Locator } from "@playwright/test";
-import { ModsPage } from "../extensionConsole/modsPage";
 import { WorkshopPage } from "../extensionConsole/workshop/workshopPage";
 import { type UUID } from "@/types/stringTypes";
 import { BasePageObject } from "../basePageObject";
@@ -300,8 +299,9 @@ export class PageEditorPage extends BasePageObject {
    * @see newPageEditorPage in fixtures/testBase.ts
    */
   async cleanup() {
-    const modsPage = new ModsPage(this.page, this.extensionId);
-    await modsPage.goto();
+    if (this.savedModIds.length === 0) {
+      return;
+    }
 
     const workshopPage = new WorkshopPage(this.page, this.extensionId);
     await workshopPage.goto();