refactor(diffs): share artifact detail and screenshot test helpers

2026-03-12 07:20:45 +00:00 · 2026-03-02 12:13:21 +00:00
parent 7533015532
commit d977af5853
3 changed files with 159 additions and 209 deletions
--- a/extensions/diffs/src/browser.test.ts
+++ b/extensions/diffs/src/browser.test.ts
@@ -35,19 +35,7 @@ describe("PlaywrightDiffScreenshotter", () => {
  });

  it("reuses the same browser across renders and closes it after the idle window", async () => {
-    const pages: Array<{
-      close: ReturnType<typeof vi.fn>;
-      screenshot: ReturnType<typeof vi.fn>;
-      pdf: ReturnType<typeof vi.fn>;
-    }> = [];
-    const browser = createMockBrowser(pages);
-    launchMock.mockResolvedValue(browser);
-    const { PlaywrightDiffScreenshotter } = await import("./browser.js");
-
-    const screenshotter = new PlaywrightDiffScreenshotter({
-      config: createConfig(),
-      browserIdleMs: 1_000,
-    });
+    const { pages, browser, screenshotter } = await createScreenshotterHarness();

    await screenshotter.screenshotHtml({
      html: '<html><head></head><body><main class="oc-frame"></main></body></html>',
@@ -106,19 +94,7 @@ describe("PlaywrightDiffScreenshotter", () => {
  });

  it("renders PDF output when format is pdf", async () => {
-    const pages: Array<{
-      close: ReturnType<typeof vi.fn>;
-      screenshot: ReturnType<typeof vi.fn>;
-      pdf: ReturnType<typeof vi.fn>;
-    }> = [];
-    const browser = createMockBrowser(pages);
-    launchMock.mockResolvedValue(browser);
-    const { PlaywrightDiffScreenshotter } = await import("./browser.js");
-
-    const screenshotter = new PlaywrightDiffScreenshotter({
-      config: createConfig(),
-      browserIdleMs: 1_000,
-    });
+    const { pages, browser, screenshotter } = await createScreenshotterHarness();
    const pdfPath = path.join(rootDir, "preview.pdf");

    await screenshotter.screenshotHtml({
@@ -184,19 +160,7 @@ describe("PlaywrightDiffScreenshotter", () => {
  });

  it("fails fast when maxPixels is still exceeded at scale 1", async () => {
-    const pages: Array<{
-      close: ReturnType<typeof vi.fn>;
-      screenshot: ReturnType<typeof vi.fn>;
-      pdf: ReturnType<typeof vi.fn>;
-    }> = [];
-    const browser = createMockBrowser(pages);
-    launchMock.mockResolvedValue(browser);
-    const { PlaywrightDiffScreenshotter } = await import("./browser.js");
-
-    const screenshotter = new PlaywrightDiffScreenshotter({
-      config: createConfig(),
-      browserIdleMs: 1_000,
-    });
+    const { pages, screenshotter } = await createScreenshotterHarness();

    await expect(
      screenshotter.screenshotHtml({
@@ -225,6 +189,24 @@ function createConfig(): OpenClawConfig {
  } as OpenClawConfig;
 }

+async function createScreenshotterHarness(options?: {
+  boundingBox?: { x: number; y: number; width: number; height: number };
+}) {
+  const pages: Array<{
+    close: ReturnType<typeof vi.fn>;
+    screenshot: ReturnType<typeof vi.fn>;
+    pdf: ReturnType<typeof vi.fn>;
+  }> = [];
+  const browser = createMockBrowser(pages, options);
+  launchMock.mockResolvedValue(browser);
+  const { PlaywrightDiffScreenshotter } = await import("./browser.js");
+  const screenshotter = new PlaywrightDiffScreenshotter({
+    config: createConfig(),
+    browserIdleMs: 1_000,
+  });
+  return { pages, browser, screenshotter };
+}
+
 function createMockBrowser(
  pages: Array<{
    close: ReturnType<typeof vi.fn>;
--- a/extensions/diffs/src/tool.test.ts
+++ b/extensions/diffs/src/tool.test.ts
@@ -3,9 +3,11 @@ import os from "node:os";
 import path from "node:path";
 import type { OpenClawPluginApi } from "openclaw/plugin-sdk";
 import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import type { DiffScreenshotter } from "./browser.js";
 import { DEFAULT_DIFFS_TOOL_DEFAULTS } from "./config.js";
 import { DiffArtifactStore } from "./store.js";
 import { createDiffsTool } from "./tool.js";
+import type { DiffRenderOptions } from "./types.js";

 describe("diffs tool", () => {
  let rootDir: string;
@@ -53,38 +55,22 @@ describe("diffs tool", () => {

  it("returns an image artifact in image mode", async () => {
    const cleanupSpy = vi.spyOn(store, "scheduleCleanup");
-    const screenshotter = {
-      screenshotHtml: vi.fn(
-        async ({
-          html,
-          outputPath,
-          image,
-        }: {
-          html: string;
-          outputPath: string;
-          image: { format: string; qualityPreset: string; scale: number; maxWidth: number };
-        }) => {
-          expect(html).not.toContain("/plugins/diffs/assets/viewer.js");
-          expect(image).toMatchObject({
-            format: "png",
-            qualityPreset: "standard",
-            scale: 2,
-            maxWidth: 960,
-          });
-          await fs.mkdir(path.dirname(outputPath), { recursive: true });
-          await fs.writeFile(outputPath, Buffer.from("png"));
-          return outputPath;
-        },
-      ),
-    };
-
-    const tool = createDiffsTool({
-      api: createApi(),
-      store,
-      defaults: DEFAULT_DIFFS_TOOL_DEFAULTS,
-      screenshotter,
+    const screenshotter = createPngScreenshotter({
+      assertHtml: (html) => {
+        expect(html).not.toContain("/plugins/diffs/assets/viewer.js");
+      },
+      assertImage: (image) => {
+        expect(image).toMatchObject({
+          format: "png",
+          qualityPreset: "standard",
+          scale: 2,
+          maxWidth: 960,
+        });
+      },
    });

+    const tool = createToolWithScreenshotter(store, screenshotter);
+
    const result = await tool.execute?.("tool-2", {
      before: "one\n",
      after: "two\n",
@@ -148,22 +134,14 @@ describe("diffs tool", () => {
  });

  it("accepts mode=file as an alias for file artifact rendering", async () => {
-    const screenshotter = {
-      screenshotHtml: vi.fn(async ({ outputPath }: { outputPath: string }) => {
+    const screenshotter = createPngScreenshotter({
+      assertOutputPath: (outputPath) => {
        expect(outputPath).toMatch(/preview\.png$/);
-        await fs.mkdir(path.dirname(outputPath), { recursive: true });
-        await fs.writeFile(outputPath, Buffer.from("png"));
-        return outputPath;
-      }),
-    };
-
-    const tool = createDiffsTool({
-      api: createApi(),
-      store,
-      defaults: DEFAULT_DIFFS_TOOL_DEFAULTS,
-      screenshotter,
+      },
    });

+    const tool = createToolWithScreenshotter(store, screenshotter);
+
    const result = await tool.execute?.("tool-2c", {
      before: "one\n",
      after: "two\n",
@@ -180,20 +158,8 @@ describe("diffs tool", () => {
    const now = new Date("2026-02-27T16:00:00Z");
    vi.setSystemTime(now);
    try {
-      const screenshotter = {
-        screenshotHtml: vi.fn(async ({ outputPath }: { outputPath: string }) => {
-          await fs.mkdir(path.dirname(outputPath), { recursive: true });
-          await fs.writeFile(outputPath, Buffer.from("png"));
-          return outputPath;
-        }),
-      };
-
-      const tool = createDiffsTool({
-        api: createApi(),
-        store,
-        defaults: DEFAULT_DIFFS_TOOL_DEFAULTS,
-        screenshotter,
-      });
+      const screenshotter = createPngScreenshotter();
+      const tool = createToolWithScreenshotter(store, screenshotter);

      const result = await tool.execute?.("tool-2c-ttl", {
        before: "one\n",
@@ -215,34 +181,18 @@ describe("diffs tool", () => {
  });

  it("accepts image* tool options for backward compatibility", async () => {
-    const screenshotter = {
-      screenshotHtml: vi.fn(
-        async ({
-          outputPath,
-          image,
-        }: {
-          outputPath: string;
-          image: { qualityPreset: string; scale: number; maxWidth: number };
-        }) => {
-          expect(image).toMatchObject({
-            qualityPreset: "hq",
-            scale: 2.4,
-            maxWidth: 1100,
-          });
-          await fs.mkdir(path.dirname(outputPath), { recursive: true });
-          await fs.writeFile(outputPath, Buffer.from("png"));
-          return outputPath;
-        },
-      ),
-    };
-
-    const tool = createDiffsTool({
-      api: createApi(),
-      store,
-      defaults: DEFAULT_DIFFS_TOOL_DEFAULTS,
-      screenshotter,
+    const screenshotter = createPngScreenshotter({
+      assertImage: (image) => {
+        expect(image).toMatchObject({
+          qualityPreset: "hq",
+          scale: 2.4,
+          maxWidth: 1100,
+        });
+      },
    });

+    const tool = createToolWithScreenshotter(store, screenshotter);
+
    const result = await tool.execute?.("tool-2legacy", {
      before: "one\n",
      after: "two\n",
@@ -294,22 +244,10 @@ describe("diffs tool", () => {
  });

  it("honors defaults.mode=file when mode is omitted", async () => {
-    const screenshotter = {
-      screenshotHtml: vi.fn(async ({ outputPath }: { outputPath: string }) => {
-        await fs.mkdir(path.dirname(outputPath), { recursive: true });
-        await fs.writeFile(outputPath, Buffer.from("png"));
-        return outputPath;
-      }),
-    };
-
-    const tool = createDiffsTool({
-      api: createApi(),
-      store,
-      defaults: {
-        ...DEFAULT_DIFFS_TOOL_DEFAULTS,
-        mode: "file",
-      },
-      screenshotter,
+    const screenshotter = createPngScreenshotter();
+    const tool = createToolWithScreenshotter(store, screenshotter, {
+      ...DEFAULT_DIFFS_TOOL_DEFAULTS,
+      mode: "file",
    });

    const result = await tool.execute?.("tool-2d", {
@@ -429,43 +367,27 @@ describe("diffs tool", () => {
  });

  it("prefers explicit tool params over configured defaults", async () => {
-    const screenshotter = {
-      screenshotHtml: vi.fn(
-        async ({
-          html,
-          outputPath,
-          image,
-        }: {
-          html: string;
-          outputPath: string;
-          image: { format: string; qualityPreset: string; scale: number; maxWidth: number };
-        }) => {
-          expect(html).not.toContain("/plugins/diffs/assets/viewer.js");
-          expect(image).toMatchObject({
-            format: "png",
-            qualityPreset: "print",
-            scale: 2.75,
-            maxWidth: 1320,
-          });
-          await fs.mkdir(path.dirname(outputPath), { recursive: true });
-          await fs.writeFile(outputPath, Buffer.from("png"));
-          return outputPath;
-        },
-      ),
-    };
-    const tool = createDiffsTool({
-      api: createApi(),
-      store,
-      defaults: {
-        ...DEFAULT_DIFFS_TOOL_DEFAULTS,
-        mode: "view",
-        theme: "light",
-        layout: "split",
-        fileQuality: "hq",
-        fileScale: 2.2,
-        fileMaxWidth: 1180,
+    const screenshotter = createPngScreenshotter({
+      assertHtml: (html) => {
+        expect(html).not.toContain("/plugins/diffs/assets/viewer.js");
      },
-      screenshotter,
+      assertImage: (image) => {
+        expect(image).toMatchObject({
+          format: "png",
+          qualityPreset: "print",
+          scale: 2.75,
+          maxWidth: 1320,
+        });
+      },
+    });
+    const tool = createToolWithScreenshotter(store, screenshotter, {
+      ...DEFAULT_DIFFS_TOOL_DEFAULTS,
+      mode: "view",
+      theme: "light",
+      layout: "split",
+      fileQuality: "hq",
+      fileScale: 2.2,
+      fileMaxWidth: 1180,
    });

    const result = await tool.execute?.("tool-6", {
@@ -527,6 +449,49 @@ function createApi(): OpenClawPluginApi {
  };
 }

+function createToolWithScreenshotter(
+  store: DiffArtifactStore,
+  screenshotter: DiffScreenshotter,
+  defaults = DEFAULT_DIFFS_TOOL_DEFAULTS,
+) {
+  return createDiffsTool({
+    api: createApi(),
+    store,
+    defaults,
+    screenshotter,
+  });
+}
+
+function createPngScreenshotter(
+  params: {
+    assertHtml?: (html: string) => void;
+    assertImage?: (image: DiffRenderOptions["image"]) => void;
+    assertOutputPath?: (outputPath: string) => void;
+  } = {},
+): DiffScreenshotter {
+  const screenshotHtml: DiffScreenshotter["screenshotHtml"] = vi.fn(
+    async ({
+      html,
+      outputPath,
+      image,
+    }: {
+      html: string;
+      outputPath: string;
+      image: DiffRenderOptions["image"];
+    }) => {
+      params.assertHtml?.(html);
+      params.assertImage?.(image);
+      params.assertOutputPath?.(outputPath);
+      await fs.mkdir(path.dirname(outputPath), { recursive: true });
+      await fs.writeFile(outputPath, Buffer.from("png"));
+      return outputPath;
+    },
+  );
+  return {
+    screenshotHtml,
+  };
+}
+
 function readTextContent(result: unknown, index: number): string {
  const content = (result as { content?: Array<{ type?: string; text?: string }> } | undefined)
    ?.content;
--- a/extensions/diffs/src/tool.ts
+++ b/extensions/diffs/src/tool.ts
@@ -192,25 +192,16 @@ export function createDiffsTool(params: {
                "Use the `message` tool with `path` or `filePath` to send this file.",
            },
          ],
-          details: {
-            title: rendered.title,
-            inputKind: rendered.inputKind,
-            fileCount: rendered.fileCount,
-            mode,
-            filePath: artifactFile.path,
-            imagePath: artifactFile.path,
-            path: artifactFile.path,
-            fileBytes: artifactFile.bytes,
-            imageBytes: artifactFile.bytes,
-            format: image.format,
-            fileFormat: image.format,
-            fileQuality: image.qualityPreset,
-            imageQuality: image.qualityPreset,
-            fileScale: image.scale,
-            imageScale: image.scale,
-            fileMaxWidth: image.maxWidth,
-            imageMaxWidth: image.maxWidth,
-          },
+          details: buildArtifactDetails({
+            baseDetails: {
+              title: rendered.title,
+              inputKind: rendered.inputKind,
+              fileCount: rendered.fileCount,
+              mode,
+            },
+            artifactFile,
+            image,
+          }),
        };
      }

@@ -272,22 +263,11 @@ export function createDiffsTool(params: {
                "Use the `message` tool with `path` or `filePath` to send this file.",
            },
          ],
-          details: {
-            ...baseDetails,
-            filePath: artifactFile.path,
-            imagePath: artifactFile.path,
-            path: artifactFile.path,
-            fileBytes: artifactFile.bytes,
-            imageBytes: artifactFile.bytes,
-            format: image.format,
-            fileFormat: image.format,
-            fileQuality: image.qualityPreset,
-            imageQuality: image.qualityPreset,
-            fileScale: image.scale,
-            imageScale: image.scale,
-            fileMaxWidth: image.maxWidth,
-            imageMaxWidth: image.maxWidth,
-          },
+          details: buildArtifactDetails({
+            baseDetails,
+            artifactFile,
+            image,
+          }),
        };
      } catch (error) {
        if (mode === "both") {
@@ -327,6 +307,29 @@ function isArtifactOnlyMode(mode: DiffMode): mode is "image" | "file" {
  return mode === "image" || mode === "file";
 }

+function buildArtifactDetails(params: {
+  baseDetails: Record<string, unknown>;
+  artifactFile: { path: string; bytes: number };
+  image: DiffRenderOptions["image"];
+}) {
+  return {
+    ...params.baseDetails,
+    filePath: params.artifactFile.path,
+    imagePath: params.artifactFile.path,
+    path: params.artifactFile.path,
+    fileBytes: params.artifactFile.bytes,
+    imageBytes: params.artifactFile.bytes,
+    format: params.image.format,
+    fileFormat: params.image.format,
+    fileQuality: params.image.qualityPreset,
+    imageQuality: params.image.qualityPreset,
+    fileScale: params.image.scale,
+    imageScale: params.image.scale,
+    fileMaxWidth: params.image.maxWidth,
+    imageMaxWidth: params.image.maxWidth,
+  };
+}
+
 async function renderDiffArtifactFile(params: {
  screenshotter: DiffScreenshotter;
  store: DiffArtifactStore;