From b2935069be441dd8b15ef665b34b8f293e5ebf90 Mon Sep 17 00:00:00 2001 From: Cole Murray Date: Wed, 24 Jun 2026 06:01:36 +0000 Subject: [PATCH 1/2] refactor: replace vi.mock with injected dependency in resolve-users --- .../shared/src/slack/resolve-users.test.ts | 73 +++++++++---------- packages/shared/src/slack/resolve-users.ts | 11 ++- 2 files changed, 42 insertions(+), 42 deletions(-) diff --git a/packages/shared/src/slack/resolve-users.test.ts b/packages/shared/src/slack/resolve-users.test.ts index ae9ab05ae..f9033e552 100644 --- a/packages/shared/src/slack/resolve-users.test.ts +++ b/packages/shared/src/slack/resolve-users.test.ts @@ -1,78 +1,65 @@ -import { beforeEach, describe, expect, it, vi } from "vitest"; -import type * as ClientModule from "./client"; - -const { mockGetUserInfo } = vi.hoisted(() => ({ - mockGetUserInfo: vi.fn(), -})); - -vi.mock("./client", async () => { - const actual = await vi.importActual("./client"); - return { - ...actual, - getUserInfo: mockGetUserInfo, - }; -}); - +import { describe, expect, it } from "vitest"; +import type { SlackEnvelope, SlackUser } from "./client"; import { resolveUserNames } from "./resolve-users"; +type GetUserInfo = (token: string, userId: string) => Promise>; + describe("resolveUserNames", () => { - beforeEach(() => { - mockGetUserInfo.mockReset(); - }); it("resolves display_name when available", async () => { - mockGetUserInfo.mockResolvedValue({ + const fakeGetUserInfo: GetUserInfo = async (_token, _userId) => ({ ok: true, user: { id: "U1", name: "alice", profile: { display_name: "Alice S" } }, }); - const result = await resolveUserNames("token", ["U1"]); + const result = await resolveUserNames("token", ["U1"], { getUserInfo: fakeGetUserInfo }); expect(result.get("U1")).toBe("Alice S"); }); it("falls back to name when display_name is empty", async () => { - mockGetUserInfo.mockResolvedValue({ + const fakeGetUserInfo: GetUserInfo = async (_token, _userId) => ({ ok: true, user: { id: "U2", name: "bob.jones", profile: { display_name: "" } }, }); - const result = await resolveUserNames("token", ["U2"]); + const result = await resolveUserNames("token", ["U2"], { getUserInfo: fakeGetUserInfo }); expect(result.get("U2")).toBe("bob.jones"); }); it("falls back to user ID when API fails", async () => { - mockGetUserInfo.mockRejectedValue(new Error("network error")); + const fakeGetUserInfo: GetUserInfo = async (_token, _userId) => { + throw new Error("network error"); + }; - const result = await resolveUserNames("token", ["U3"]); + const result = await resolveUserNames("token", ["U3"], { getUserInfo: fakeGetUserInfo }); // Promise.allSettled catches the rejection — ID is not in the map expect(result.has("U3")).toBe(false); }); it("falls back to user ID when user info is missing", async () => { - mockGetUserInfo.mockResolvedValue({ ok: false, error: "user_not_found" }); + const fakeGetUserInfo: GetUserInfo = async (_token, _userId) => ({ + ok: false, + error: "user_not_found", + }); - const result = await resolveUserNames("token", ["U4"]); + const result = await resolveUserNames("token", ["U4"], { getUserInfo: fakeGetUserInfo }); expect(result.get("U4")).toBe("U4"); }); it("resolves multiple users in parallel", async () => { - mockGetUserInfo - .mockResolvedValueOnce({ - ok: true, - user: { id: "U1", name: "alice", profile: { display_name: "Alice" } }, - }) - .mockResolvedValueOnce({ - ok: true, - user: { id: "U2", name: "bob", profile: { display_name: "Bob" } }, - }); + const responses: Record> = { + U1: { ok: true, user: { id: "U1", name: "alice", profile: { display_name: "Alice" } } }, + U2: { ok: true, user: { id: "U2", name: "bob", profile: { display_name: "Bob" } } }, + }; + const fakeGetUserInfo: GetUserInfo = async (_token, userId) => responses[userId]!; - const result = await resolveUserNames("token", ["U1", "U2"]); + const result = await resolveUserNames("token", ["U1", "U2"], { getUserInfo: fakeGetUserInfo }); expect(result.get("U1")).toBe("Alice"); expect(result.get("U2")).toBe("Bob"); expect(result.size).toBe(2); }); it("does not include real_name in fallback chain", async () => { - mockGetUserInfo.mockResolvedValue({ + const fakeGetUserInfo: GetUserInfo = async (_token, _userId) => ({ ok: true, user: { id: "U5", @@ -82,14 +69,20 @@ describe("resolveUserNames", () => { }, }); - const result = await resolveUserNames("token", ["U5"]); + const result = await resolveUserNames("token", ["U5"], { getUserInfo: fakeGetUserInfo }); // Should use name (jdoe), not real_name (John Doe) expect(result.get("U5")).toBe("jdoe"); }); it("returns empty map for empty input", async () => { - const result = await resolveUserNames("token", []); + let callCount = 0; + const fakeGetUserInfo: GetUserInfo = async (_token, _userId) => { + callCount++; + return { ok: true, user: { id: "X", name: "x" } }; + }; + + const result = await resolveUserNames("token", [], { getUserInfo: fakeGetUserInfo }); expect(result.size).toBe(0); - expect(mockGetUserInfo).not.toHaveBeenCalled(); + expect(callCount).toBe(0); }); }); diff --git a/packages/shared/src/slack/resolve-users.ts b/packages/shared/src/slack/resolve-users.ts index ae5890571..fcc9d2232 100644 --- a/packages/shared/src/slack/resolve-users.ts +++ b/packages/shared/src/slack/resolve-users.ts @@ -1,4 +1,6 @@ -import { getUserInfo } from "./client"; +import { getUserInfo as defaultGetUserInfo } from "./client"; + +type GetUserInfo = (token: string, userId: string) => ReturnType; /** * Resolve Slack user IDs to display names. @@ -7,11 +9,16 @@ import { getUserInfo } from "./client"; * error envelope or thrown exception via `Promise.allSettled`), the entry is * either populated with the raw user ID (envelope failure) or omitted entirely * (thrown exception). Callers should treat a missing entry as "unknown user". + * + * The optional `deps` parameter allows injecting a custom `getUserInfo` + * implementation for testing without module-level mocking. */ export async function resolveUserNames( token: string, - userIds: string[] + userIds: string[], + deps: { getUserInfo?: GetUserInfo } = {} ): Promise> { + const getUserInfo = deps.getUserInfo ?? defaultGetUserInfo; const names = new Map(); const results = await Promise.allSettled( userIds.map(async (id) => { From 31a2e233a3c038d85d93543b690ab6fa6cb66291 Mon Sep 17 00:00:00 2001 From: Cole Murray Date: Wed, 24 Jun 2026 06:08:30 +0000 Subject: [PATCH 2/2] refactor: inject getUserInfo directly as positional param with default --- .../shared/src/slack/resolve-users.test.ts | 18 ++++++++---------- packages/shared/src/slack/resolve-users.ts | 8 ++------ 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/packages/shared/src/slack/resolve-users.test.ts b/packages/shared/src/slack/resolve-users.test.ts index f9033e552..c7d79b0af 100644 --- a/packages/shared/src/slack/resolve-users.test.ts +++ b/packages/shared/src/slack/resolve-users.test.ts @@ -1,8 +1,6 @@ import { describe, expect, it } from "vitest"; import type { SlackEnvelope, SlackUser } from "./client"; -import { resolveUserNames } from "./resolve-users"; - -type GetUserInfo = (token: string, userId: string) => Promise>; +import { resolveUserNames, type GetUserInfo } from "./resolve-users"; describe("resolveUserNames", () => { it("resolves display_name when available", async () => { @@ -11,7 +9,7 @@ describe("resolveUserNames", () => { user: { id: "U1", name: "alice", profile: { display_name: "Alice S" } }, }); - const result = await resolveUserNames("token", ["U1"], { getUserInfo: fakeGetUserInfo }); + const result = await resolveUserNames("token", ["U1"], fakeGetUserInfo); expect(result.get("U1")).toBe("Alice S"); }); @@ -21,7 +19,7 @@ describe("resolveUserNames", () => { user: { id: "U2", name: "bob.jones", profile: { display_name: "" } }, }); - const result = await resolveUserNames("token", ["U2"], { getUserInfo: fakeGetUserInfo }); + const result = await resolveUserNames("token", ["U2"], fakeGetUserInfo); expect(result.get("U2")).toBe("bob.jones"); }); @@ -30,7 +28,7 @@ describe("resolveUserNames", () => { throw new Error("network error"); }; - const result = await resolveUserNames("token", ["U3"], { getUserInfo: fakeGetUserInfo }); + const result = await resolveUserNames("token", ["U3"], fakeGetUserInfo); // Promise.allSettled catches the rejection — ID is not in the map expect(result.has("U3")).toBe(false); }); @@ -41,7 +39,7 @@ describe("resolveUserNames", () => { error: "user_not_found", }); - const result = await resolveUserNames("token", ["U4"], { getUserInfo: fakeGetUserInfo }); + const result = await resolveUserNames("token", ["U4"], fakeGetUserInfo); expect(result.get("U4")).toBe("U4"); }); @@ -52,7 +50,7 @@ describe("resolveUserNames", () => { }; const fakeGetUserInfo: GetUserInfo = async (_token, userId) => responses[userId]!; - const result = await resolveUserNames("token", ["U1", "U2"], { getUserInfo: fakeGetUserInfo }); + const result = await resolveUserNames("token", ["U1", "U2"], fakeGetUserInfo); expect(result.get("U1")).toBe("Alice"); expect(result.get("U2")).toBe("Bob"); expect(result.size).toBe(2); @@ -69,7 +67,7 @@ describe("resolveUserNames", () => { }, }); - const result = await resolveUserNames("token", ["U5"], { getUserInfo: fakeGetUserInfo }); + const result = await resolveUserNames("token", ["U5"], fakeGetUserInfo); // Should use name (jdoe), not real_name (John Doe) expect(result.get("U5")).toBe("jdoe"); }); @@ -81,7 +79,7 @@ describe("resolveUserNames", () => { return { ok: true, user: { id: "X", name: "x" } }; }; - const result = await resolveUserNames("token", [], { getUserInfo: fakeGetUserInfo }); + const result = await resolveUserNames("token", [], fakeGetUserInfo); expect(result.size).toBe(0); expect(callCount).toBe(0); }); diff --git a/packages/shared/src/slack/resolve-users.ts b/packages/shared/src/slack/resolve-users.ts index fcc9d2232..da8be05a7 100644 --- a/packages/shared/src/slack/resolve-users.ts +++ b/packages/shared/src/slack/resolve-users.ts @@ -1,6 +1,6 @@ import { getUserInfo as defaultGetUserInfo } from "./client"; -type GetUserInfo = (token: string, userId: string) => ReturnType; +export type GetUserInfo = (token: string, userId: string) => ReturnType; /** * Resolve Slack user IDs to display names. @@ -9,16 +9,12 @@ type GetUserInfo = (token: string, userId: string) => ReturnType> { - const getUserInfo = deps.getUserInfo ?? defaultGetUserInfo; const names = new Map(); const results = await Promise.allSettled( userIds.map(async (id) => {