#8596: improving mod definition snapshot capabilities - diffing

.eslintrc.js

@@ -149,6 +149,7 @@ module.exports = {
         "unicorn/prefer-dom-node-dataset": "off",
         "unicorn/prefer-module": "off", // `import.meta.dirname` throws "cannot use 'import meta' outside a module"
         "no-await-in-loop": "off",
+        "security/detect-object-injection": "off",
         "playwright/no-skipped-test": [
           "error",
           {

.prettierignore

@@ -5,3 +5,6 @@ coverage
 # ignore libraries
 node_modules
 public/mockServiceWorker.js
+
+# ignore snapshots
+**/*-snapshots/**

end-to-end-tests/env.ts

@@ -52,14 +52,12 @@ type OptionalEnvVariables = Record<
 
 export const assertRequiredEnvVariables = () => {
   for (const key of requiredEnvVariables) {
-    // eslint-disable-next-line security/detect-object-injection -- key is a constant
     if (process.env[key] === undefined) {
       throw new Error(
         `Required environment variable is not configured: ${key}`,
       );
     }
 
-    // eslint-disable-next-line security/detect-object-injection -- key is a constant
     if (typeof process.env[key] !== "string") {
       // For the time being we expect all of our requiredEnvVariables to be strings
       throw new TypeError(

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

@@ -28,7 +28,6 @@ export const test = base.extend<{
       assertRequiredEnvVariables();
 
       for (const key of additionalRequiredEnvVariables) {
-        // eslint-disable-next-line security/detect-object-injection -- internally controlled
         if (process.env[key] === undefined) {
           throw new Error(
             `This test requires additional environment variable ${key} to be configured. Configure it in your .env.development file and re-build the extension.`,

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

@@ -18,9 +18,17 @@
 import { expect } from "@playwright/test";
 import { test as pageContextFixture } from "./pageContext";
 import { WorkshopPage } from "../pageObjects/extensionConsole/workshop/workshopPage";
+import diff from "deep-diff";
+import { loadBrickYaml } from "@/runtime/brickYaml";
+
+// The mod definitions are a map of mod names to their test metadata
+type ModDefinitions = Record<
+  string,
+  { id: string; definition: string; autoCleanup: boolean }
+>;
 
 // Replaces any uuids in the text with a fixed value to make snapshots more stable
-function normalizeUUids(string: string) {
+function normalizeUUIDs(string: string) {
   return string.replaceAll(
     // eslint-disable-next-line unicorn/better-regex -- more clear this way
     /[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}/g,
@@ -29,58 +37,141 @@ function normalizeUUids(string: string) {
 }
 
 export const test = pageContextFixture.extend<{
-  // These should correspond 1-1 with the mod definition file names in the fixtures/modDefinitions directory
+  /**
+   * 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[];
-  createdModIds: 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.
+   */
+  modDefinitionsMap: ModDefinitions;
+  /**
+   * Verifies the current definition state of a mod. Each time this is called, the mod definition is updated
+   * in the modDefinitions map fixture.
+   * @param options.modId The mod id to verify the snapshot for.
+   * @param options.snapshotName The name of the snapshot to verify against.
+   * @param options.mode The mode to use for verifying the snapshot. `diff` will compare the current mod definition
+   * to the last known state, while `current` will compare the whole current mod definition to the snapshot.
+   */
   verifyModDefinitionSnapshot: (options: {
     modId: string;
     snapshotName: string;
+    mode?: "diff" | "current";
   }) => Promise<void>;
 }>({
   modDefinitionNames: [],
-  createdModIds: [
+  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();
+  },
+  modDefinitionsMap: [
     async ({ modDefinitionNames, page, extensionId }, use) => {
-      const createdIds: string[] = [];
+      const createdModDefinitions: ModDefinitions = {};
       if (modDefinitionNames.length > 0) {
         const workshopPage = new WorkshopPage(page, extensionId);
-        for (const definition of modDefinitionNames) {
+        for (const name of modDefinitionNames) {
           await workshopPage.goto();
-          const createdModId =
-            await workshopPage.createNewModFromDefinition(definition);
-          createdIds.push(createdModId);
+          const modMetadata =
+            await workshopPage.createNewModFromDefinition(name);
+          createdModDefinitions[name] = { ...modMetadata, autoCleanup: true };
         }
       }
 
-      await use(createdIds);
+      await use(createdModDefinitions);
 
-      if (createdIds.length > 0) {
+      if (Object.keys(createdModDefinitions).length > 0) {
         const workshopPage = new WorkshopPage(page, extensionId);
-        for (const id of createdIds) {
-          await workshopPage.goto();
-          await workshopPage.deletePackagedModByModId(id);
+        for (const { id, autoCleanup } of Object.values(
+          createdModDefinitions,
+        )) {
+          if (autoCleanup) {
+            await workshopPage.goto();
+            await workshopPage.deletePackagedModByModId(id);
+          }
         }
       }
     },
     { auto: true },
   ],
-  async verifyModDefinitionSnapshot({ page, extensionId }, use, testInfo) {
+  async verifyModDefinitionSnapshot(
+    { _workshopPage: workshopPage, modDefinitionsMap },
+    use,
+    testInfo,
+  ) {
     // Overriding the snapshot suffix to avoid including the os name.
     testInfo.snapshotSuffix = `${testInfo.title}`;
     const _verifyModDefinitionSnapshot = async ({
       modId,
       snapshotName,
+      mode = "diff",
     }: {
       modId: string;
       snapshotName: string;
+      mode?: "diff" | "current";
     }) => {
-      const workshopPage = new WorkshopPage(page, extensionId);
       await workshopPage.goto();
       const editPage = await workshopPage.findAndSelectMod(modId);
 
-      const normalizedModDefinitionYaml = normalizeUUids(
-        await editPage.editor.getValue(),
+      const currentModDefinitionYaml = await editPage.editor.getValue();
+      // See if this mod is being tracked in modDefinitions.
+      const lastModDefinitionEntry = Object.entries(modDefinitionsMap).find(
+        ([_name, { id }]) => id === modId,
       );
-      expect(normalizedModDefinitionYaml).toMatchSnapshot(snapshotName);
+
+      if (mode === "diff") {
+        if (!lastModDefinitionEntry) {
+          throw new Error(
+            `Mod definition for ${modId} not found in modDefinitions. Cannot verify a diff.`,
+          );
+        }
+
+        const [
+          modDefinitionName,
+          { definition: lastModDefinition, autoCleanup },
+        ] = lastModDefinitionEntry;
+
+        const parsedCurrentModDefinitionYaml = loadBrickYaml(
+          currentModDefinitionYaml,
+        );
+        const parsedLastModDefinitionYaml = loadBrickYaml(lastModDefinition);
+        const yamlDiff =
+          diff(parsedLastModDefinitionYaml, parsedCurrentModDefinitionYaml) ||
+          [];
+
+        expect(JSON.stringify(yamlDiff, undefined, 2) + "\n").toMatchSnapshot(
+          snapshotName + ".json",
+        );
+
+        // 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,
+        };
+      }
     };
 
     await use(_verifyModDefinitionSnapshot);

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

@@ -17,7 +17,6 @@
 
 import { expect, type Page } from "@playwright/test";
 import { getBaseExtensionConsoleUrl } from "../constants";
-import { ensureVisibility } from "../../utils";
 import { BasePageObject } from "../basePageObject";
 
 export class ModsPage extends BasePageObject {
@@ -46,15 +45,11 @@ export class ModsPage extends BasePageObject {
     await expect(this.getByText("Extension Console")).toBeVisible();
     await registryPromise;
 
-    // Check that the page is stable, and that the content has finished loading
-    const activeModsHeading = this.getByRole("heading", {
-      name: "Active Mods",
-    });
-    await ensureVisibility(activeModsHeading, { timeout: 10_000 });
+    // Check that the content has finished loading
     const contentLoadedLocator = this.getByText("Welcome to PixieBrix!").or(
       this.modTableItems.nth(0),
     );
-    await expect(contentLoadedLocator).toBeVisible();
+    await expect(contentLoadedLocator).toBeVisible({ timeout: 10_000 });
   }
 
   async viewAllMods() {
@@ -88,7 +83,6 @@ export class ModsPage extends BasePageObject {
     // Open the dropdown action menu for the specified mod in the table
     await modSearchResult.locator(".dropdown").click();
 
-    // Click the delete button in the delete confirmation modal
     await this.getByRole("button", { name: actionName }).click();
   }
 

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

@@ -17,18 +17,23 @@
 
 import { WorkshopModEditor } from "./modEditor";
 import { BasePageObject } from "end-to-end-tests/pageObjects/basePageObject";
+import { expect } from "@playwright/test";
 
 export class EditWorkshopModPage extends BasePageObject {
   editor = new WorkshopModEditor(this.getByLabel("Editor"));
 
   async updateBrick() {
     await this.getByRole("button", { name: "Update Brick" }).click();
+    await expect(
+      this.page.getByRole("status").filter({ hasText: "Updated " }),
+    ).toBeVisible();
   }
 
   async deleteBrick() {
     await this.getByRole("button", { name: "Delete Brick" }).click();
     await this.getByRole("button", { name: "Permanently Delete" }).click();
-    // eslint-disable-next-line playwright/no-networkidle -- for some reason, can't assert on the "Brick deleted" notice
-    await this.page.waitForLoadState("networkidle");
+    await expect(
+      this.page.getByRole("status").filter({ hasText: "Deleted " }),
+    ).toBeVisible();
   }
 }

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

@@ -68,6 +68,6 @@ export class WorkshopModEditor extends BasePageObject {
     const replacedDefinition = modDefinition.replace("{{ modId }}", modId);
 
     await this.textArea.fill(replacedDefinition);
-    return modId;
+    return { id: modId, definition: replacedDefinition };
   }
 }

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

@@ -53,13 +53,13 @@ export class WorkshopPage extends BasePageObject {
     await this.createNewBrickButton.click();
     const createPage = new CreateWorkshopModPage(this.page);
     await createPage.editor.waitForLoad();
-    const modId =
+    const modMedata =
       await createPage.editor.replaceWithModDefinition(modDefinitionName);
     await createPage.createBrickButton.click();
     await expect(this.getByRole("status").getByText("Created ")).toBeVisible({
       timeout: 8000,
     });
-    return modId;
+    return modMedata;
   }
 
   async deletePackagedModByModId(modId: string) {

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

@@ -129,6 +129,7 @@ export class PageEditorPage extends BasePageObject {
     this.savedStandaloneModNames.push(modName);
   }
 
+  @ModifiesModState
   async createModFromModComponent({
     modNameRoot,
     modComponentName,

end-to-end-tests/tests/modLifecycle.spec.ts

@@ -18,7 +18,10 @@
 import { expect, test } from "../fixtures/testBase";
 // @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 { ModsPage } from "../pageObjects/extensionConsole/modsPage";
+import {
+  ActivateModPage,
+  ModsPage,
+} from "../pageObjects/extensionConsole/modsPage";
 import { clickAndWaitForNewPage } from "end-to-end-tests/utils";
 import { WorkshopPage } from "end-to-end-tests/pageObjects/extensionConsole/workshop/workshopPage";
 
@@ -115,22 +118,15 @@ test("create, run, package, and update mod", async ({
     await modsPage.goto();
 
     await modsPage.viewActiveMods();
-    const modListing = modsPage.modTableItemById(modId);
+    await expect(modsPage.modTableItemById(modId)).toContainText(
+      "version 1.0.1",
+    );
+    await modsPage.actionForModByName(modId, "Reactivate");
 
-    await expect(
-      modListing.getByRole("button", { name: "Update" }),
-    ).toBeVisible();
-    await modListing.getByRole("button", { name: "Update" }).click();
+    const modActivatePage = new ActivateModPage(newPage, extensionId, modId);
 
-    await expect(modsPage.locator("form")).toContainText(
+    await expect(modActivatePage.locator("form")).toContainText(
       "Created through Playwright Automation",
     );
-
-    await expect(
-      modsPage.getByRole("button", { name: "Reactivate" }),
-    ).toBeVisible();
-    await modsPage.getByRole("button", { name: "Reactivate" }).click();
-
-    await expect(modListing).toContainText("version 1.0.1");
   });
 });