test: dedupe channel helper suites

2026-03-29 10:50:58 +00:00 · 2026-03-28 07:35:54 +00:00
parent 2181909f9a
commit f4fb45f1ee
5 changed files with 175 additions and 139 deletions
--- a/src/channels/channel-config.test.ts
+++ b/src/channels/channel-config.test.ts
@@ -81,17 +81,15 @@ describe("resolveChannelEntryMatchWithFallback", () => {
    },
  ]);

-  for (const testCase of fallbackCases) {
-    it(testCase.name, () => {
-      const match = resolveChannelEntryMatchWithFallback({
-        entries: testCase.entries,
-        ...testCase.args,
-      });
-      expect(match.entry).toBe(testCase.entries[testCase.expectedEntryKey]);
-      expect(match.matchSource).toBe(testCase.expectedSource);
-      expect(match.matchKey).toBe(testCase.expectedMatchKey);
+  it.each(fallbackCases)("$name", (testCase) => {
+    const match = resolveChannelEntryMatchWithFallback({
+      entries: testCase.entries,
+      ...testCase.args,
    });
-  }
+    expect(match.entry).toBe(testCase.entries[testCase.expectedEntryKey]);
+    expect(match.matchSource).toBe(testCase.expectedSource);
+    expect(match.matchKey).toBe(testCase.expectedMatchKey);
+  });

  it("matches normalized keys when normalizeKey is provided", () => {
    const entries = { "My Team": { allow: true } };
--- a/src/channels/config-presence.test.ts
+++ b/src/channels/config-presence.test.ts
@@ -16,9 +16,22 @@ function makeTempStateDir() {
  return dir;
 }

+function expectPotentialConfiguredChannelCase(params: {
+  cfg: unknown;
+  env: NodeJS.ProcessEnv;
+  expectedIds: string[];
+  expectedConfigured: boolean;
+}) {
+  expect(listPotentialConfiguredChannelIds(params.cfg, params.env)).toEqual(params.expectedIds);
+  expect(hasPotentialConfiguredChannels(params.cfg, params.env)).toBe(params.expectedConfigured);
+}
+
 afterEach(() => {
-  for (const dir of tempDirs.splice(0)) {
-    fs.rmSync(dir, { recursive: true, force: true });
+  while (tempDirs.length > 0) {
+    const dir = tempDirs.pop();
+    if (dir) {
+      fs.rmSync(dir, { recursive: true, force: true });
+    }
  }
 });

@@ -35,7 +48,11 @@ describe("config presence", () => {
    const env = { OPENCLAW_STATE_DIR: stateDir } as NodeJS.ProcessEnv;
    const cfg = { channels: { matrix: { enabled: false } } };

-    expect(listPotentialConfiguredChannelIds(cfg, env)).toEqual([]);
-    expect(hasPotentialConfiguredChannels(cfg, env)).toBe(false);
+    expectPotentialConfiguredChannelCase({
+      cfg,
+      env,
+      expectedIds: [],
+      expectedConfigured: false,
+    });
  });
 });
--- a/src/channels/plugins/account-helpers.test.ts
+++ b/src/channels/plugins/account-helpers.test.ts
@@ -34,6 +34,18 @@ function cfg(accounts?: Record<string, unknown> | null, defaultAccount?: string)
  } as unknown as OpenClawConfig;
 }

+function expectResolvedAccountIdsCase(params: {
+  resolve: (cfg: OpenClawConfig) => string[];
+  input: OpenClawConfig;
+  expected: string[];
+}) {
+  expect(params.resolve(params.input)).toEqual(params.expected);
+}
+
+function expectResolvedDefaultAccountCase(input: OpenClawConfig, expected: string) {
+  expect(resolveDefaultAccountId(input)).toBe(expected);
+}
+
 describe("createAccountListHelpers", () => {
  describe("listConfiguredAccountIds", () => {
    it.each([
@@ -50,7 +62,11 @@ describe("createAccountListHelpers", () => {
        input: cfg({}),
      },
    ])("$name", ({ input }) => {
-      expect(listConfiguredAccountIds(input)).toEqual([]);
+      expectResolvedAccountIdsCase({
+        resolve: listConfiguredAccountIds,
+        input,
+        expected: [],
+      });
    });

    it("filters out empty keys", () => {
@@ -99,7 +115,11 @@ describe("createAccountListHelpers", () => {
        expected: ["a", "m", "z"],
      },
    ])("$name", ({ input, expected }) => {
-      expect(listAccountIds(input)).toEqual(expected);
+      expectResolvedAccountIdsCase({
+        resolve: listAccountIds,
+        input,
+        expected,
+      });
    });
  });

@@ -136,7 +156,7 @@ describe("createAccountListHelpers", () => {
        expected: "default",
      },
    ])("$name", ({ input, expected }) => {
-      expect(resolveDefaultAccountId(input)).toBe(expected);
+      expectResolvedDefaultAccountCase(input, expected);
    });

    it("can preserve configured defaults that are not present in accounts", () => {
@@ -257,74 +277,66 @@ describe("describeAccountSnapshot", () => {
 });

 describe("mergeAccountConfig", () => {
-  it("drops accounts from the base config before merging", () => {
-    const merged = mergeAccountConfig<{
-      enabled?: boolean;
-      name?: string;
-      accounts?: Record<string, { name?: string }>;
-    }>({
-      channelConfig: {
-        enabled: true,
-        accounts: {
-          work: { name: "Work" },
+  it.each([
+    {
+      name: "drops accounts from the base config before merging",
+      input: {
+        channelConfig: {
+          enabled: true,
+          accounts: {
+            work: { name: "Work" },
+          },
+        },
+        accountConfig: {
+          name: "Work",
        },
      },
-      accountConfig: {
-        name: "Work",
-      },
-    });
-
-    expect(merged).toEqual({
-      enabled: true,
-      name: "Work",
-    });
-  });
-
-  it("drops caller-specified keys from the base config before merging", () => {
-    const merged = mergeAccountConfig<{
-      enabled?: boolean;
-      defaultAccount?: string;
-      name?: string;
-    }>({
-      channelConfig: {
+      expected: {
        enabled: true,
-        defaultAccount: "work",
-      },
-      accountConfig: {
        name: "Work",
      },
-      omitKeys: ["defaultAccount"],
-    });
-
-    expect(merged).toEqual({
-      enabled: true,
-      name: "Work",
-    });
-  });
-
-  it("deep-merges selected nested object keys", () => {
-    const merged = mergeAccountConfig<{
-      commands?: { native?: boolean; callbackPath?: string };
-    }>({
-      channelConfig: {
+    },
+    {
+      name: "drops caller-specified keys from the base config before merging",
+      input: {
+        channelConfig: {
+          enabled: true,
+          defaultAccount: "work",
+        },
+        accountConfig: {
+          name: "Work",
+        },
+        omitKeys: ["defaultAccount"],
+      },
+      expected: {
+        enabled: true,
+        name: "Work",
+      },
+    },
+    {
+      name: "deep-merges selected nested object keys",
+      input: {
+        channelConfig: {
+          commands: {
+            native: true,
+          },
+        },
+        accountConfig: {
+          commands: {
+            callbackPath: "/work",
+          },
+        },
+        nestedObjectKeys: ["commands"],
+      },
+      expected: {
        commands: {
          native: true,
-        },
-      },
-      accountConfig: {
-        commands: {
          callbackPath: "/work",
        },
      },
-      nestedObjectKeys: ["commands"],
-    });
-
-    expect(merged).toEqual({
-      commands: {
-        native: true,
-        callbackPath: "/work",
-      },
-    });
+    },
+  ] as const)("$name", ({ input, expected }) => {
+    expect(mergeAccountConfig(input)).toEqual(expected);
  });
 });

--- a/src/channels/plugins/threading-helpers.test.ts
+++ b/src/channels/plugins/threading-helpers.test.ts
@@ -32,8 +32,8 @@ describe("createTopLevelChannelReplyToModeResolver", () => {
 });

 describe("createScopedAccountReplyToModeResolver", () => {
-  it("reads the scoped account reply mode", () => {
-    const resolver = createScopedAccountReplyToModeResolver({
+  function createScopedResolver() {
+    return createScopedAccountReplyToModeResolver({
      resolveAccount: (cfg, accountId) =>
        ((
          cfg.channels as {
@@ -44,7 +44,13 @@ describe("createScopedAccountReplyToModeResolver", () => {
        },
      resolveReplyToMode: (account) => account.replyToMode,
    });
+  }

+  it.each([
+    { accountId: "assistant", expected: "all" },
+    { accountId: "default", expected: "off" },
+  ] as const)("resolves scoped reply mode for $accountId", ({ accountId, expected }) => {
+    const resolver = createScopedResolver();
    const cfg = {
      channels: {
        demo: {
@@ -55,8 +61,7 @@ describe("createScopedAccountReplyToModeResolver", () => {
      },
    } as OpenClawConfig;

-    expect(resolver({ cfg, accountId: "assistant" })).toBe("all");
-    expect(resolver({ cfg, accountId: "default" })).toBe("off");
+    expect(resolver({ cfg, accountId })).toBe(expected);
  });

  it("passes chatType through", () => {
--- a/src/channels/status-reactions.test.ts
+++ b/src/channels/status-reactions.test.ts
@@ -56,6 +56,22 @@ const createSetOnlyController = () => {
  return { calls, controller };
 };

+function expectSetEmojiCall(calls: Array<{ method: string; emoji: string }>, emoji: string) {
+  expect(calls).toContainEqual({ method: "set", emoji });
+}
+
+function expectArrayContainsAll(values: readonly string[], expected: readonly string[]) {
+  expected.forEach((value) => {
+    expect(values).toContain(value);
+  });
+}
+
+function expectObjectHasKeys(value: Record<string, unknown>, keys: readonly string[]) {
+  keys.forEach((key) => {
+    expect(value).toHaveProperty(key);
+  });
+}
+
 // ─────────────────────────────────────────────────────────────────────────────
 // Tests
 // ─────────────────────────────────────────────────────────────────────────────
@@ -126,7 +142,7 @@ describe("createStatusReactionController", () => {
    void controller.setQueued();
    await vi.runAllTimersAsync();

-    expect(calls).toContainEqual({ method: "set", emoji: "👀" });
+    expectSetEmojiCall(calls, "👀");
  });

  it("should debounce setThinking and eventually call adapter", async () => {
@@ -140,7 +156,7 @@ describe("createStatusReactionController", () => {

    // After debounce period
    await vi.advanceTimersByTimeAsync(300);
-    expect(calls).toContainEqual({ method: "set", emoji: DEFAULT_EMOJIS.thinking });
+    expectSetEmojiCall(calls, DEFAULT_EMOJIS.thinking);
  });

  it("should debounce setCompacting and eventually call adapter", async () => {
@@ -149,7 +165,7 @@ describe("createStatusReactionController", () => {
    void controller.setCompacting();
    await vi.advanceTimersByTimeAsync(DEFAULT_TIMING.debounceMs);

-    expect(calls).toContainEqual({ method: "set", emoji: DEFAULT_EMOJIS.compacting });
+    expectSetEmojiCall(calls, DEFAULT_EMOJIS.compacting);
  });

  it("should classify tool name and debounce", async () => {
@@ -158,7 +174,7 @@ describe("createStatusReactionController", () => {
    void controller.setTool("exec");
    await vi.advanceTimersByTimeAsync(DEFAULT_TIMING.debounceMs);

-    expect(calls).toContainEqual({ method: "set", emoji: DEFAULT_EMOJIS.coding });
+    expectSetEmojiCall(calls, DEFAULT_EMOJIS.coding);
  });

  const immediateTerminalCases = [
@@ -174,16 +190,17 @@ describe("createStatusReactionController", () => {
    },
  ] as const;

-  for (const testCase of immediateTerminalCases) {
-    it(`should execute ${testCase.name} immediately without debounce`, async () => {
+  it.each(immediateTerminalCases)(
+    "should execute $name immediately without debounce",
+    async ({ run, expected }) => {
      const { calls, controller } = createEnabledController();

-      await testCase.run(controller);
+      await run(controller);
      await vi.runAllTimersAsync();

-      expect(calls).toContainEqual({ method: "set", emoji: testCase.expected });
-    });
-  }
+      expectSetEmojiCall(calls, expected);
+    },
+  );

  const terminalIgnoreCases = [
    {
@@ -204,18 +221,16 @@ describe("createStatusReactionController", () => {
    },
  ] as const;

-  for (const testCase of terminalIgnoreCases) {
-    it(`should ${testCase.name}`, async () => {
-      const { calls, controller } = createEnabledController();
+  it.each(terminalIgnoreCases)("should $name", async ({ terminal, followup }) => {
+    const { calls, controller } = createEnabledController();

-      await testCase.terminal(controller);
-      const callsAfterTerminal = calls.length;
-      testCase.followup(controller);
-      await vi.advanceTimersByTimeAsync(1000);
+    await terminal(controller);
+    const callsAfterTerminal = calls.length;
+    followup(controller);
+    await vi.advanceTimersByTimeAsync(1000);

-      expect(calls.length).toBe(callsAfterTerminal);
-    });
-  }
+    expect(calls.length).toBe(callsAfterTerminal);
+  });

  it("should only fire last state when rapidly changing (debounce)", async () => {
    const { calls, controller } = createEnabledController();
@@ -272,7 +287,7 @@ describe("createStatusReactionController", () => {
    await vi.advanceTimersByTimeAsync(DEFAULT_TIMING.debounceMs);

    // Should set thinking, then remove queued
-    expect(calls).toContainEqual({ method: "set", emoji: DEFAULT_EMOJIS.thinking });
+    expectSetEmojiCall(calls, DEFAULT_EMOJIS.thinking);
    expect(calls).toContainEqual({ method: "remove", emoji: "👀" });
  });

@@ -322,7 +337,7 @@ describe("createStatusReactionController", () => {

    await controller.restoreInitial();

-    expect(calls).toContainEqual({ method: "set", emoji: "👀" });
+    expectSetEmojiCall(calls, "👀");
  });

  it("should use custom emojis when provided", async () => {
@@ -336,11 +351,11 @@ describe("createStatusReactionController", () => {
    void controller.setThinking();
    await vi.advanceTimersByTimeAsync(DEFAULT_TIMING.debounceMs);

-    expect(calls).toContainEqual({ method: "set", emoji: "🤔" });
+    expectSetEmojiCall(calls, "🤔");

    await controller.setDone();
    await vi.runAllTimersAsync();
-    expect(calls).toContainEqual({ method: "set", emoji: "🎉" });
+    expectSetEmojiCall(calls, "🎉");
  });

  it("should use custom timing when provided", async () => {
@@ -358,7 +373,7 @@ describe("createStatusReactionController", () => {

    // Should fire at 100ms
    await vi.advanceTimersByTimeAsync(60);
-    expect(calls).toContainEqual({ method: "set", emoji: DEFAULT_EMOJIS.thinking });
+    expectSetEmojiCall(calls, DEFAULT_EMOJIS.thinking);
  });

  const stallCases = [
@@ -381,14 +396,12 @@ describe("createStatusReactionController", () => {
    return state;
  };

-  for (const testCase of stallCases) {
-    it(`should trigger ${testCase.name}`, async () => {
-      const { calls } = await createControllerAfterThinking();
-      await vi.advanceTimersByTimeAsync(testCase.delayMs);
+  it.each(stallCases)("should trigger $name", async ({ delayMs, expected }) => {
+    const { calls } = await createControllerAfterThinking();
+    await vi.advanceTimersByTimeAsync(delayMs);

-      expect(calls).toContainEqual({ method: "set", emoji: testCase.expected });
-    });
-  }
+    expectSetEmojiCall(calls, expected);
+  });

  const stallResetCases = [
    {
@@ -407,22 +420,16 @@ describe("createStatusReactionController", () => {
    },
  ] as const;

-  for (const testCase of stallResetCases) {
-    it(`should reset stall timers on ${testCase.name}`, async () => {
-      const { calls, controller } = await createControllerAfterThinking();
+  it.each(stallResetCases)("should reset stall timers on $name", async ({ runUpdate }) => {
+    const { calls, controller } = await createControllerAfterThinking();

-      // Advance halfway to soft stall.
-      await vi.advanceTimersByTimeAsync(DEFAULT_TIMING.stallSoftMs / 2);
+    await vi.advanceTimersByTimeAsync(DEFAULT_TIMING.stallSoftMs / 2);
+    await runUpdate(controller);
+    await vi.advanceTimersByTimeAsync(DEFAULT_TIMING.stallSoftMs / 2);

-      await testCase.runUpdate(controller);
-
-      // Advance another halfway - should not trigger stall yet.
-      await vi.advanceTimersByTimeAsync(DEFAULT_TIMING.stallSoftMs / 2);
-
-      const stallCalls = calls.filter((c) => c.emoji === DEFAULT_EMOJIS.stallSoft);
-      expect(stallCalls).toHaveLength(0);
-    });
-  }
+    const stallCalls = calls.filter((c) => c.emoji === DEFAULT_EMOJIS.stallSoft);
+    expect(stallCalls).toHaveLength(0);
+  });

  it("should call onError callback when adapter throws", async () => {
    const onError = vi.fn();
@@ -448,19 +455,15 @@ describe("createStatusReactionController", () => {

 describe("constants", () => {
  it("should export CODING_TOOL_TOKENS", () => {
-    for (const token of ["exec", "read", "write"]) {
-      expect(CODING_TOOL_TOKENS).toContain(token);
-    }
+    expectArrayContainsAll(CODING_TOOL_TOKENS, ["exec", "read", "write"]);
  });

  it("should export WEB_TOOL_TOKENS", () => {
-    for (const token of ["web_search", "browser"]) {
-      expect(WEB_TOOL_TOKENS).toContain(token);
-    }
+    expectArrayContainsAll(WEB_TOOL_TOKENS, ["web_search", "browser"]);
  });

  it("should export DEFAULT_EMOJIS with all required keys", () => {
-    const emojiKeys = [
+    expectObjectHasKeys(DEFAULT_EMOJIS, [
      "queued",
      "thinking",
      "compacting",
@@ -471,15 +474,16 @@ describe("constants", () => {
      "error",
      "stallSoft",
      "stallHard",
-    ] as const;
-    for (const key of emojiKeys) {
-      expect(DEFAULT_EMOJIS).toHaveProperty(key);
-    }
+    ]);
  });

  it("should export DEFAULT_TIMING with all required keys", () => {
-    for (const key of ["debounceMs", "stallSoftMs", "stallHardMs", "doneHoldMs", "errorHoldMs"]) {
-      expect(DEFAULT_TIMING).toHaveProperty(key);
-    }
+    expectObjectHasKeys(DEFAULT_TIMING, [
+      "debounceMs",
+      "stallSoftMs",
+      "stallHardMs",
+      "doneHoldMs",
+      "errorHoldMs",
+    ]);
  });
 });