refactor(test): dedupe repeated fixture setup helpers

2026-03-08 06:54:24 +00:00 · 2026-02-22 20:04:02 +00:00
parent 6ef4eda1f0
commit 8af6d1a186
5 changed files with 131 additions and 156 deletions
--- a/src/browser/server-context.remote-tab-ops.test.ts
+++ b/src/browser/server-context.remote-tab-ops.test.ts
@@ -64,6 +64,16 @@ function createRemoteRouteHarness(fetchMock?: ReturnType<typeof vi.fn>) {
  return { state, remote: ctx.forProfile("remote"), fetchMock: activeFetchMock };
 }

+function createSequentialPageLister<T>(responses: T[]) {
+  return vi.fn(async () => {
+    const next = responses.shift();
+    if (!next) {
+      throw new Error("no more responses");
+    }
+    return next;
+  });
+}
+
 describe("browser server-context remote profile tab operations", () => {
  it("uses Playwright tab operations when available", async () => {
    const listPagesViaPlaywright = vi.fn(async () => [
@@ -158,13 +168,7 @@ describe("browser server-context remote profile tab operations", () => {
      [{ targetId: "T1", title: "Tab 1", url: "https://example.com", type: "page" }],
      [{ targetId: "T1", title: "Tab 1", url: "https://example.com", type: "page" }],
    ];
-    const listPagesViaPlaywright = vi.fn(async () => {
-      const next = responses.shift();
-      if (!next) {
-        throw new Error("no more responses");
-      }
-      return next;
-    });
+    const listPagesViaPlaywright = createSequentialPageLister(responses);

    vi.spyOn(pwAiModule, "getPwAiModule").mockResolvedValue({
      listPagesViaPlaywright,
@@ -186,13 +190,7 @@ describe("browser server-context remote profile tab operations", () => {
        { targetId: "B", title: "B", url: "https://b.example", type: "page" },
      ],
    ];
-    const listPagesViaPlaywright = vi.fn(async () => {
-      const next = responses.shift();
-      if (!next) {
-        throw new Error("no more responses");
-      }
-      return next;
-    });
+    const listPagesViaPlaywright = createSequentialPageLister(responses);

    vi.spyOn(pwAiModule, "getPwAiModule").mockResolvedValue({
      listPagesViaPlaywright,
--- a/src/plugins/install.test.ts
+++ b/src/plugins/install.test.ts
@@ -167,6 +167,26 @@ function setupPluginInstallDirs() {
  return { tmpDir, pluginDir, extensionsDir };
 }

+function setupInstallPluginFromDirFixture(params?: { devDependencies?: Record<string, string> }) {
+  const workDir = makeTempDir();
+  const stateDir = makeTempDir();
+  const pluginDir = path.join(workDir, "plugin");
+  fs.mkdirSync(path.join(pluginDir, "dist"), { recursive: true });
+  fs.writeFileSync(
+    path.join(pluginDir, "package.json"),
+    JSON.stringify({
+      name: "@openclaw/test-plugin",
+      version: "0.0.1",
+      openclaw: { extensions: ["./dist/index.js"] },
+      dependencies: { "left-pad": "1.3.0" },
+      ...(params?.devDependencies ? { devDependencies: params.devDependencies } : {}),
+    }),
+    "utf-8",
+  );
+  fs.writeFileSync(path.join(pluginDir, "dist", "index.js"), "export {};", "utf-8");
+  return { pluginDir, extensionsDir: path.join(stateDir, "extensions") };
+}
+
 async function installFromDirWithWarnings(params: { pluginDir: string; extensionsDir: string }) {
  const warnings: string[] = [];
  const result = await installPluginFromDir({
@@ -445,21 +465,7 @@ describe("installPluginFromArchive", () => {

 describe("installPluginFromDir", () => {
  it("uses --ignore-scripts for dependency install", async () => {
-    const workDir = makeTempDir();
-    const stateDir = makeTempDir();
-    const pluginDir = path.join(workDir, "plugin");
-    fs.mkdirSync(path.join(pluginDir, "dist"), { recursive: true });
-    fs.writeFileSync(
-      path.join(pluginDir, "package.json"),
-      JSON.stringify({
-        name: "@openclaw/test-plugin",
-        version: "0.0.1",
-        openclaw: { extensions: ["./dist/index.js"] },
-        dependencies: { "left-pad": "1.3.0" },
-      }),
-      "utf-8",
-    );
-    fs.writeFileSync(path.join(pluginDir, "dist", "index.js"), "export {};", "utf-8");
+    const { pluginDir, extensionsDir } = setupInstallPluginFromDirFixture();

    const run = vi.mocked(runCommandWithTimeout);
    await expectInstallUsesIgnoreScripts({
@@ -467,31 +473,18 @@ describe("installPluginFromDir", () => {
      install: async () =>
        await installPluginFromDir({
          dirPath: pluginDir,
-          extensionsDir: path.join(stateDir, "extensions"),
+          extensionsDir,
        }),
    });
  });

  it("strips workspace devDependencies before npm install", async () => {
-    const workDir = makeTempDir();
-    const stateDir = makeTempDir();
-    const pluginDir = path.join(workDir, "plugin");
-    fs.mkdirSync(path.join(pluginDir, "dist"), { recursive: true });
-    fs.writeFileSync(
-      path.join(pluginDir, "package.json"),
-      JSON.stringify({
-        name: "@openclaw/test-plugin",
-        version: "0.0.1",
-        openclaw: { extensions: ["./dist/index.js"] },
-        dependencies: { "left-pad": "1.3.0" },
-        devDependencies: {
-          openclaw: "workspace:*",
-          vitest: "^3.0.0",
-        },
-      }),
-      "utf-8",
-    );
-    fs.writeFileSync(path.join(pluginDir, "dist", "index.js"), "export {};", "utf-8");
+    const { pluginDir, extensionsDir } = setupInstallPluginFromDirFixture({
+      devDependencies: {
+        openclaw: "workspace:*",
+        vitest: "^3.0.0",
+      },
+    });

    const run = vi.mocked(runCommandWithTimeout);
    run.mockResolvedValue({
@@ -505,7 +498,7 @@ describe("installPluginFromDir", () => {

    const res = await installPluginFromDir({
      dirPath: pluginDir,
-      extensionsDir: path.join(stateDir, "extensions"),
+      extensionsDir,
    });
    expect(res.ok).toBe(true);
    if (!res.ok) {
--- a/src/plugins/uninstall.test.ts
+++ b/src/plugins/uninstall.test.ts
@@ -46,6 +46,24 @@ async function createInstalledNpmPluginFixture(params: {
  };
 }

+type UninstallResult = Awaited<ReturnType<typeof uninstallPlugin>>;
+
+async function runDeleteInstalledNpmPluginFixture(baseDir: string): Promise<{
+  pluginDir: string;
+  result: UninstallResult;
+}> {
+  const { pluginId, extensionsDir, pluginDir, config } = await createInstalledNpmPluginFixture({
+    baseDir,
+  });
+  const result = await uninstallPlugin({
+    config,
+    pluginId,
+    deleteFiles: true,
+    extensionsDir,
+  });
+  return { pluginDir, result };
+}
+
 function createSinglePluginEntries(pluginId = "my-plugin") {
  return {
    [pluginId]: { enabled: true },
@@ -61,6 +79,21 @@ function createSinglePluginWithEmptySlotsConfig(): OpenClawConfig {
  };
 }

+function createSingleNpmInstallConfig(installPath: string): OpenClawConfig {
+  return {
+    plugins: {
+      entries: createSinglePluginEntries(),
+      installs: {
+        "my-plugin": {
+          source: "npm",
+          spec: "my-plugin@1.0.0",
+          installPath,
+        },
+      },
+    },
+  };
+}
+
 async function createPluginDirFixture(baseDir: string, pluginId = "my-plugin") {
  const pluginDir = path.join(baseDir, pluginId);
  await fs.mkdir(pluginDir, { recursive: true });
@@ -330,18 +363,9 @@ describe("uninstallPlugin", () => {
  });

  it("deletes directory when deleteFiles is true", async () => {
-    const { pluginId, extensionsDir, pluginDir, config } = await createInstalledNpmPluginFixture({
-      baseDir: tempDir,
-    });
+    const { pluginDir, result } = await runDeleteInstalledNpmPluginFixture(tempDir);

    try {
-      const result = await uninstallPlugin({
-        config,
-        pluginId,
-        deleteFiles: true,
-        extensionsDir,
-      });
-
      expect(result.ok).toBe(true);
      if (result.ok) {
        expect(result.actions.directory).toBe(true);
@@ -389,18 +413,7 @@ describe("uninstallPlugin", () => {
  it("does not delete directory when deleteFiles is false", async () => {
    const pluginDir = await createPluginDirFixture(tempDir);

-    const config: OpenClawConfig = {
-      plugins: {
-        entries: createSinglePluginEntries(),
-        installs: {
-          "my-plugin": {
-            source: "npm",
-            spec: "my-plugin@1.0.0",
-            installPath: pluginDir,
-          },
-        },
-      },
-    };
+    const config = createSingleNpmInstallConfig(pluginDir);

    const result = await uninstallPlugin({
      config,
@@ -417,20 +430,7 @@ describe("uninstallPlugin", () => {
  });

  it("succeeds even if directory does not exist", async () => {
-    const config: OpenClawConfig = {
-      plugins: {
-        entries: {
-          "my-plugin": { enabled: true },
-        },
-        installs: {
-          "my-plugin": {
-            source: "npm",
-            spec: "my-plugin@1.0.0",
-            installPath: "/nonexistent/path",
-          },
-        },
-      },
-    };
+    const config = createSingleNpmInstallConfig("/nonexistent/path");

    const result = await uninstallPlugin({
      config,
@@ -447,18 +447,9 @@ describe("uninstallPlugin", () => {
  });

  it("returns a warning when directory deletion fails unexpectedly", async () => {
-    const { pluginId, extensionsDir, config } = await createInstalledNpmPluginFixture({
-      baseDir: tempDir,
-    });
-
    const rmSpy = vi.spyOn(fs, "rm").mockRejectedValueOnce(new Error("permission denied"));
    try {
-      const result = await uninstallPlugin({
-        config,
-        pluginId,
-        deleteFiles: true,
-        extensionsDir,
-      });
+      const { result } = await runDeleteInstalledNpmPluginFixture(tempDir);

      expect(result.ok).toBe(true);
      if (result.ok) {
@@ -477,20 +468,7 @@ describe("uninstallPlugin", () => {
    await fs.mkdir(outsideDir, { recursive: true });
    await fs.writeFile(path.join(outsideDir, "index.js"), "// keep me");

-    const config: OpenClawConfig = {
-      plugins: {
-        entries: {
-          "my-plugin": { enabled: true },
-        },
-        installs: {
-          "my-plugin": {
-            source: "npm",
-            spec: "my-plugin@1.0.0",
-            installPath: outsideDir,
-          },
-        },
-      },
-    };
+    const config = createSingleNpmInstallConfig(outsideDir);

    const result = await uninstallPlugin({
      config,
--- a/src/security/temp-path-guard.test.ts
+++ b/src/security/temp-path-guard.test.ts
@@ -17,6 +17,13 @@ const SKIP_PATTERNS = [
  /[\\/][^\\/]*test-harness(?:\.[^\\/]+)?\.ts$/,
 ];

+type QuoteChar = "'" | '"' | "`";
+
+type QuoteScanState = {
+  quote: QuoteChar | null;
+  escaped: boolean;
+};
+
 function shouldSkip(relativePath: string): boolean {
  return SKIP_PATTERNS.some((pattern) => pattern.test(relativePath));
 }
@@ -27,26 +34,13 @@ function stripCommentsForScan(input: string): string {

 function findMatchingParen(source: string, openIndex: number): number {
  let depth = 1;
-  let quote: "'" | '"' | "`" | null = null;
-  let escaped = false;
+  const quoteState: QuoteScanState = { quote: null, escaped: false };
  for (let i = openIndex + 1; i < source.length; i += 1) {
    const ch = source[i];
-    if (quote) {
-      if (escaped) {
-        escaped = false;
-        continue;
-      }
-      if (ch === "\\") {
-        escaped = true;
-        continue;
-      }
-      if (ch === quote) {
-        quote = null;
-      }
+    if (consumeQuotedChar(quoteState, ch)) {
      continue;
    }
-    if (ch === "'" || ch === '"' || ch === "`") {
-      quote = ch;
+    if (beginQuotedSection(quoteState, ch)) {
      continue;
    }
    if (ch === "(") {
@@ -69,27 +63,15 @@ function splitTopLevelArguments(source: string): string[] {
  let parenDepth = 0;
  let bracketDepth = 0;
  let braceDepth = 0;
-  let quote: "'" | '"' | "`" | null = null;
-  let escaped = false;
+  const quoteState: QuoteScanState = { quote: null, escaped: false };
  for (let i = 0; i < source.length; i += 1) {
    const ch = source[i];
-    if (quote) {
+    if (quoteState.quote) {
      current += ch;
-      if (escaped) {
-        escaped = false;
-        continue;
-      }
-      if (ch === "\\") {
-        escaped = true;
-        continue;
-      }
-      if (ch === quote) {
-        quote = null;
-      }
+      consumeQuotedChar(quoteState, ch);
      continue;
    }
-    if (ch === "'" || ch === '"' || ch === "`") {
-      quote = ch;
+    if (beginQuotedSection(quoteState, ch)) {
      current += ch;
      continue;
    }
@@ -142,6 +124,32 @@ function splitTopLevelArguments(source: string): string[] {
  return out;
 }

+function beginQuotedSection(state: QuoteScanState, ch: string): boolean {
+  if (ch !== "'" && ch !== '"' && ch !== "`") {
+    return false;
+  }
+  state.quote = ch;
+  return true;
+}
+
+function consumeQuotedChar(state: QuoteScanState, ch: string): boolean {
+  if (!state.quote) {
+    return false;
+  }
+  if (state.escaped) {
+    state.escaped = false;
+    return true;
+  }
+  if (ch === "\\") {
+    state.escaped = true;
+    return true;
+  }
+  if (ch === state.quote) {
+    state.quote = null;
+  }
+  return true;
+}
+
 function isOsTmpdirExpression(argument: string): boolean {
  return /^os\s*\.\s*tmpdir\s*\(\s*\)$/u.test(argument.trim());
 }
--- a/src/slack/monitor/monitor.test.ts
+++ b/src/slack/monitor/monitor.test.ts
@@ -113,6 +113,16 @@ function createThreadStarterRepliesClient(
  return { replies, client };
 }

+function createListedChannelsContext(groupPolicy: "open" | "allowlist") {
+  return createSlackMonitorContext({
+    ...baseParams(),
+    groupPolicy,
+    channelsConfig: {
+      C_LISTED: { requireMention: true },
+    },
+  });
+}
+
 describe("normalizeSlackChannelType", () => {
  it("infers channel types from ids when missing", () => {
    expect(normalizeSlackChannelType(undefined, "C123")).toBe("channel");
@@ -138,13 +148,7 @@ describe("isChannelAllowed with groupPolicy and channelsConfig", () => {
  it("allows unlisted channels when groupPolicy is open even with channelsConfig entries", () => {
    // Bug fix: when groupPolicy="open" and channels has some entries,
    // unlisted channels should still be allowed (not blocked)
-    const ctx = createSlackMonitorContext({
-      ...baseParams(),
-      groupPolicy: "open",
-      channelsConfig: {
-        C_LISTED: { requireMention: true },
-      },
-    });
+    const ctx = createListedChannelsContext("open");
    // Listed channel should be allowed
    expect(ctx.isChannelAllowed({ channelId: "C_LISTED", channelType: "channel" })).toBe(true);
    // Unlisted channel should ALSO be allowed when policy is "open"
@@ -152,13 +156,7 @@ describe("isChannelAllowed with groupPolicy and channelsConfig", () => {
  });

  it("blocks unlisted channels when groupPolicy is allowlist", () => {
-    const ctx = createSlackMonitorContext({
-      ...baseParams(),
-      groupPolicy: "allowlist",
-      channelsConfig: {
-        C_LISTED: { requireMention: true },
-      },
-    });
+    const ctx = createListedChannelsContext("allowlist");
    // Listed channel should be allowed
    expect(ctx.isChannelAllowed({ channelId: "C_LISTED", channelType: "channel" })).toBe(true);
    // Unlisted channel should be blocked when policy is "allowlist"