refactor: dedupe synology chat tests

2026-05-06 11:20:43 +00:00 · 2026-04-20 23:52:29 +01:00
parent 28d6aa5514
commit 8e681123d8
4 changed files with 157 additions and 156 deletions
--- a/extensions/synology-chat/src/channel.integration.test.ts
+++ b/extensions/synology-chat/src/channel.integration.test.ts
@@ -15,6 +15,16 @@ type _RegisteredRoute = {
 };

 let createSynologyChatPlugin: typeof import("./channel.js").createSynologyChatPlugin;
+
+function makeStartContext<T>(cfg: T, accountId: string, abortSignal: AbortSignal) {
+  return {
+    cfg,
+    accountId,
+    log: { info: vi.fn(), warn: vi.fn(), error: vi.fn() },
+    abortSignal,
+  };
+}
+
 describe("Synology channel wiring integration", () => {
  beforeAll(async () => {
    ({ createSynologyChatPlugin } = await import("./channel.js"));
@@ -30,30 +40,27 @@ describe("Synology channel wiring integration", () => {
  it("registers real webhook handler with resolved account config and enforces allowlist", async () => {
    const plugin = createSynologyChatPlugin();
    const abortController = new AbortController();
-    const ctx = {
-      cfg: {
-        channels: {
-          "synology-chat": {
-            enabled: true,
-            accounts: {
-              alerts: {
-                enabled: true,
-                token: "valid-token",
-                incomingUrl: "https://nas.example.com/incoming",
-                webhookPath: "/webhook/synology-alerts",
-                dmPolicy: "allowlist",
-                allowedUserIds: ["456"],
-              },
+    const cfg = {
+      channels: {
+        "synology-chat": {
+          enabled: true,
+          accounts: {
+            alerts: {
+              enabled: true,
+              token: "valid-token",
+              incomingUrl: "https://nas.example.com/incoming",
+              webhookPath: "/webhook/synology-alerts",
+              dmPolicy: "allowlist",
+              allowedUserIds: ["456"],
            },
          },
        },
      },
-      accountId: "alerts",
-      log: { info: vi.fn(), warn: vi.fn(), error: vi.fn() },
-      abortSignal: abortController.signal,
    };

-    const started = plugin.gateway.startAccount(ctx);
+    const started = plugin.gateway.startAccount(
+      makeStartContext(cfg, "alerts", abortController.signal),
+    );
    expect(registerPluginHttpRouteMock).toHaveBeenCalledTimes(1);

    const firstCall = registerPluginHttpRouteMock.mock.calls[0];
@@ -115,18 +122,12 @@ describe("Synology channel wiring integration", () => {
      },
    };

-    const alphaStarted = plugin.gateway.startAccount({
-      cfg,
-      accountId: "alpha",
-      log: { info: vi.fn(), warn: vi.fn(), error: vi.fn() },
-      abortSignal: alphaAbortController.signal,
-    });
-    const betaStarted = plugin.gateway.startAccount({
-      cfg,
-      accountId: "beta",
-      log: { info: vi.fn(), warn: vi.fn(), error: vi.fn() },
-      abortSignal: betaAbortController.signal,
-    });
+    const alphaStarted = plugin.gateway.startAccount(
+      makeStartContext(cfg, "alpha", alphaAbortController.signal),
+    );
+    const betaStarted = plugin.gateway.startAccount(
+      makeStartContext(cfg, "beta", betaAbortController.signal),
+    );

    expect(registerPluginHttpRouteMock).toHaveBeenCalledTimes(2);
    const alphaRoute = registerPluginHttpRouteMock.mock.calls[0]?.[0];
--- a/extensions/synology-chat/src/channel.test.ts
+++ b/extensions/synology-chat/src/channel.test.ts
@@ -2,26 +2,27 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import { createPluginSetupWizardStatus } from "../../../test/helpers/plugins/setup-wizard.js";
 import type { ResolvedSynologyChatAccount } from "./types.js";

+const securityAccountDefaults: ResolvedSynologyChatAccount = {
+  accountId: "default",
+  enabled: true,
+  token: "t",
+  incomingUrl: "https://nas/incoming",
+  nasHost: "h",
+  webhookPath: "/w",
+  webhookPathSource: "default" as const,
+  dangerouslyAllowNameMatching: false,
+  dangerouslyAllowInheritedWebhookPath: false,
+  dmPolicy: "allowlist" as const,
+  allowedUserIds: [],
+  rateLimitPerMinute: 30,
+  botName: "Bot",
+  allowInsecureSsl: false,
+};
+
 function makeSecurityAccount(
  overrides: Partial<ResolvedSynologyChatAccount> = {},
 ): ResolvedSynologyChatAccount {
-  return {
-    accountId: "default",
-    enabled: true,
-    token: "t",
-    incomingUrl: "https://nas/incoming",
-    nasHost: "h",
-    webhookPath: "/w",
-    webhookPathSource: "default" as const,
-    dangerouslyAllowNameMatching: false,
-    dangerouslyAllowInheritedWebhookPath: false,
-    dmPolicy: "allowlist" as const,
-    allowedUserIds: [],
-    rateLimitPerMinute: 30,
-    botName: "Bot",
-    allowInsecureSsl: false,
-    ...overrides,
-  };
+  return { ...securityAccountDefaults, ...overrides };
 }

 const clientModule = await import("./client.js");
@@ -227,6 +228,26 @@ describe("createSynologyChatPlugin", () => {
  });

  describe("security.collectWarnings", () => {
+    function makeSharedWebhookConfig(alertsOverrides: Record<string, unknown> = {}) {
+      return {
+        channels: {
+          "synology-chat": {
+            token: "base-token",
+            webhookPath: "/webhook/shared",
+            accounts: {
+              alerts: {
+                token: "alerts-token",
+                incomingUrl: "https://nas/alerts",
+                dmPolicy: "allowlist",
+                allowedUserIds: ["123"],
+                ...alertsOverrides,
+              },
+            },
+          },
+        },
+      };
+    }
+
    it("warns when token is missing", () => {
      const plugin = createSynologyChatPlugin();
      const account = makeSecurityAccount({ token: "" });
@@ -277,22 +298,7 @@ describe("createSynologyChatPlugin", () => {

    it("warns when named multi-account routes inherit a shared webhookPath", () => {
      const plugin = createSynologyChatPlugin();
-      const cfg = {
-        channels: {
-          "synology-chat": {
-            token: "base-token",
-            webhookPath: "/webhook/shared",
-            accounts: {
-              alerts: {
-                token: "alerts-token",
-                incomingUrl: "https://nas/alerts",
-                dmPolicy: "allowlist",
-                allowedUserIds: ["123"],
-              },
-            },
-          },
-        },
-      };
+      const cfg = makeSharedWebhookConfig();
      const account = plugin.config.resolveAccount(cfg, "alerts");
      const warnings = plugin.security.collectWarnings({ cfg, account });
      expect(warnings.some((w: string) => w.includes("must set an explicit webhookPath"))).toBe(
@@ -302,23 +308,16 @@ describe("createSynologyChatPlugin", () => {

    it("warns when enabled accounts share the same exact webhookPath", () => {
      const plugin = createSynologyChatPlugin();
+      const base = makeSharedWebhookConfig({ webhookPath: "/webhook/shared" }).channels[
+        "synology-chat"
+      ];
      const cfg = {
        channels: {
          "synology-chat": {
-            token: "base-token",
+            ...base,
            incomingUrl: "https://nas/default",
-            webhookPath: "/webhook/shared",
            dmPolicy: "allowlist",
            allowedUserIds: ["123"],
-            accounts: {
-              alerts: {
-                token: "alerts-token",
-                incomingUrl: "https://nas/alerts",
-                webhookPath: "/webhook/shared",
-                dmPolicy: "allowlist",
-                allowedUserIds: ["123"],
-              },
-            },
          },
        },
      };
--- a/extensions/synology-chat/src/client.test.ts
+++ b/extensions/synology-chat/src/client.test.ts
@@ -4,15 +4,16 @@ import { describe, it, expect, vi, beforeAll, beforeEach, afterEach } from "vite

 // Mock http and https modules before importing the client
 vi.mock("node:https", () => {
-  const mockRequest = vi.fn();
-  const mockGet = vi.fn();
-  return { default: { request: mockRequest, get: mockGet }, request: mockRequest, get: mockGet };
+  const httpsRequest = vi.fn();
+  const httpsGet = vi.fn();
+  const httpsModule = { request: httpsRequest, get: httpsGet };
+  return { default: httpsModule, request: httpsRequest, get: httpsGet };
 });

 vi.mock("node:http", () => {
-  const mockRequest = vi.fn();
-  const mockGet = vi.fn();
-  return { default: { request: mockRequest, get: mockGet }, request: mockRequest, get: mockGet };
+  const httpRequest = vi.fn();
+  const httpGet = vi.fn();
+  return { default: { request: httpRequest, get: httpGet }, request: httpRequest, get: httpGet };
 });

 const https = await import("node:https");
--- a/extensions/synology-chat/src/webhook-handler.test.ts
+++ b/extensions/synology-chat/src/webhook-handler.test.ts
@@ -115,6 +115,69 @@ describe("createWebhookHandler", () => {
    expect(deliver).not.toHaveBeenCalled();
  }

+  function makeTestHandler(params: {
+    accountIdSuffix: string;
+    deliver?: WebhookHandlerDeps["deliver"];
+    account?: Partial<ResolvedSynologyChatAccount>;
+  }) {
+    const deliver = params.deliver ?? vi.fn().mockResolvedValue(null);
+    return {
+      deliver,
+      handler: createWebhookHandler({
+        account: makeAccount({
+          accountId: `${params.accountIdSuffix}-${Date.now()}`,
+          ...params.account,
+        }),
+        deliver,
+        log,
+      }),
+    };
+  }
+
+  async function postToWebhook(
+    handler: ReturnType<typeof createWebhookHandler>,
+    body = validBody,
+    options?: Parameters<typeof makeReq>[2],
+  ) {
+    const req = makeReq("POST", body, options);
+    const res = makeRes();
+    await handler(req, res);
+    return res;
+  }
+
+  async function expectTokenlessBodyAccepted(params: {
+    accountIdSuffix: string;
+    options: Parameters<typeof makeReq>[2];
+  }) {
+    const { deliver, handler } = makeTestHandler({ accountIdSuffix: params.accountIdSuffix });
+    const res = await postToWebhook(
+      handler,
+      makeFormBody({ user_id: "123", username: "testuser", text: "hello" }),
+      params.options,
+    );
+    expect(res._status).toBe(204);
+    expect(deliver).toHaveBeenCalled();
+  }
+
+  async function runValidReply(params: { accountIdSuffix: string; reply?: string }) {
+    const { deliver, handler } = makeTestHandler({
+      accountIdSuffix: params.accountIdSuffix,
+      deliver: vi.fn().mockResolvedValue(params.reply ?? "Bot reply"),
+    });
+    const res = await postToWebhook(handler);
+    expect(res._status).toBe(204);
+    return { deliver, res };
+  }
+
+  function expectBotReplySentTo(chatUserId: string) {
+    expect(sendMessage).toHaveBeenCalledWith(
+      "https://nas.example.com/incoming",
+      "Bot reply",
+      chatUserId,
+      true,
+    );
+  }
+
  it("rejects non-POST methods with 405", async () => {
    const handler = createWebhookHandler({
      account: makeAccount(),
@@ -358,51 +421,25 @@ describe("createWebhookHandler", () => {
  });

  it("accepts token from query when body token is absent", async () => {
-    const deliver = vi.fn().mockResolvedValue(null);
-    const handler = createWebhookHandler({
-      account: makeAccount({ accountId: "query-token-test-" + Date.now() }),
-      deliver,
-      log,
-    });
-
-    const req = makeReq(
-      "POST",
-      makeFormBody({ user_id: "123", username: "testuser", text: "hello" }),
-      {
+    await expectTokenlessBodyAccepted({
+      accountIdSuffix: "query-token-test",
+      options: {
        headers: { "content-type": "application/x-www-form-urlencoded" },
        url: "/webhook/synology?token=valid-token",
      },
-    );
-    const res = makeRes();
-    await handler(req, res);
-
-    expect(res._status).toBe(204);
-    expect(deliver).toHaveBeenCalled();
+    });
  });

  it("accepts token from authorization header when body token is absent", async () => {
-    const deliver = vi.fn().mockResolvedValue(null);
-    const handler = createWebhookHandler({
-      account: makeAccount({ accountId: "header-token-test-" + Date.now() }),
-      deliver,
-      log,
-    });
-
-    const req = makeReq(
-      "POST",
-      makeFormBody({ user_id: "123", username: "testuser", text: "hello" }),
-      {
+    await expectTokenlessBodyAccepted({
+      accountIdSuffix: "header-token-test",
+      options: {
        headers: {
          "content-type": "application/x-www-form-urlencoded",
          authorization: "Bearer valid-token",
        },
      },
-    );
-    const res = makeRes();
-    await handler(req, res);
-
-    expect(res._status).toBe(204);
-    expect(deliver).toHaveBeenCalled();
+    });
  });

  it("returns 403 for unauthorized user with allowlist policy", async () => {
@@ -484,18 +521,7 @@ describe("createWebhookHandler", () => {
  });

  it("responds 204 immediately and delivers async", async () => {
-    const deliver = vi.fn().mockResolvedValue("Bot reply");
-    const handler = createWebhookHandler({
-      account: makeAccount({ accountId: "async-test-" + Date.now() }),
-      deliver,
-      log,
-    });
-
-    const req = makeReq("POST", validBody);
-    const res = makeRes();
-    await handler(req, res);
-
-    expect(res._status).toBe(204);
+    const { deliver, res } = await runValidReply({ accountIdSuffix: "async-test" });
    expect(res._body).toBe("");
    expect(deliver).toHaveBeenCalledWith(
      expect.objectContaining({
@@ -510,18 +536,7 @@ describe("createWebhookHandler", () => {
  });

  it("keeps replies bound to payload.user_id by default", async () => {
-    const deliver = vi.fn().mockResolvedValue("Bot reply");
-    const handler = createWebhookHandler({
-      account: makeAccount({ accountId: "stable-id-test-" + Date.now() }),
-      deliver,
-      log,
-    });
-
-    const req = makeReq("POST", validBody);
-    const res = makeRes();
-    await handler(req, res);
-
-    expect(res._status).toBe(204);
+    const { deliver } = await runValidReply({ accountIdSuffix: "stable-id-test" });
    expect(resolveLegacyWebhookNameToChatUserId).not.toHaveBeenCalled();
    expect(deliver).toHaveBeenCalledWith(
      expect.objectContaining({
@@ -529,12 +544,7 @@ describe("createWebhookHandler", () => {
        chatUserId: "123",
      }),
    );
-    expect(sendMessage).toHaveBeenCalledWith(
-      "https://nas.example.com/incoming",
-      "Bot reply",
-      "123",
-      true,
-    );
+    expectBotReplySentTo("123");
  });

  it("only resolves reply recipient by username when break-glass mode is enabled", async () => {
@@ -548,12 +558,7 @@ describe("createWebhookHandler", () => {
        chatUserId: "456",
      }),
    );
-    expect(sendMessage).toHaveBeenCalledWith(
-      "https://nas.example.com/incoming",
-      "Bot reply",
-      "456",
-      true,
-    );
+    expectBotReplySentTo("456");
  });

  it("falls back to payload.user_id when break-glass resolution does not find a match", async () => {
@@ -569,12 +574,7 @@ describe("createWebhookHandler", () => {
        chatUserId: "123",
      }),
    );
-    expect(sendMessage).toHaveBeenCalledWith(
-      "https://nas.example.com/incoming",
-      "Bot reply",
-      "123",
-      true,
-    );
+    expectBotReplySentTo("123");
  });

  it("sanitizes input before delivery", async () => {