diff --git a/src/github/utils/image-downloader.ts b/src/github/utils/image-downloader.ts index 63922f9..924b009 100644 --- a/src/github/utils/image-downloader.ts +++ b/src/github/utils/image-downloader.ts @@ -2,11 +2,12 @@ import fs from "fs/promises"; import path from "path"; import type { Octokits } from "../api/client"; -// Mutable fs reference for cross-platform test compatibility. -// On Linux, Bun gives test and source files different fs/promises objects, -// so spyOn(fs, "mkdir") in a test file won't intercept calls here. -// Tests override these methods directly instead. -export const _fsForTest = { +export type FsOps = { + mkdir: typeof fs.mkdir; + writeFile: typeof fs.writeFile; +}; + +const defaultFsOps: FsOps = { mkdir: fs.mkdir.bind(fs), writeFile: fs.writeFile.bind(fs), }; @@ -72,11 +73,12 @@ export async function downloadCommentImages( owner: string, repo: string, comments: CommentWithImages[], + fsOps: FsOps = defaultFsOps, ): Promise> { const urlToPathMap = new Map(); const downloadsDir = "/tmp/github-images"; - await _fsForTest.mkdir(downloadsDir, { recursive: true }); + await fsOps.mkdir(downloadsDir, { recursive: true }); const { imageRegex, htmlImgRegex } = buildImageRegexes(); @@ -226,7 +228,7 @@ export async function downloadCommentImages( const arrayBuffer = await imageResponse.arrayBuffer(); const buffer = Buffer.from(arrayBuffer); - await _fsForTest.writeFile(localPath, buffer); + await fsOps.writeFile(localPath, buffer); console.log(`✓ Saved: ${localPath}`); urlToPathMap.set(originalUrl, localPath); diff --git a/test/image-downloader.test.ts b/test/image-downloader.test.ts index d68b63f..36656df 100644 --- a/test/image-downloader.test.ts +++ b/test/image-downloader.test.ts @@ -10,8 +10,8 @@ import { } from "bun:test"; import { downloadCommentImages, - _fsForTest, type CommentWithImages, + type FsOps, } from "../src/github/utils/image-downloader"; import type { Octokits } from "../src/github/api/client"; @@ -22,13 +22,13 @@ describe("downloadCommentImages", () => { let fetchSpy: any; let savedServerUrl: string | undefined; - // Store originals for restoration - const origMkdir = _fsForTest.mkdir; - const origWriteFile = _fsForTest.writeFile; - - // Mock functions for fs operations + // Mock functions for fs operations — injected via parameter, no module mutation needed const mockMkdir = jest.fn().mockResolvedValue(undefined); const mockWriteFile = jest.fn().mockResolvedValue(undefined); + const mockFsOps: FsOps = { + mkdir: mockMkdir as any, + writeFile: mockWriteFile as any, + }; beforeEach(() => { // Pin GITHUB_SERVER_URL so tests pass regardless of runner environment. @@ -41,11 +41,8 @@ describe("downloadCommentImages", () => { consoleWarnSpy = spyOn(console, "warn").mockImplementation(() => {}); consoleErrorSpy = spyOn(console, "error").mockImplementation(() => {}); - // Override _fsForTest methods directly — works cross-platform mockMkdir.mockClear(); mockWriteFile.mockClear(); - _fsForTest.mkdir = mockMkdir as any; - _fsForTest.writeFile = mockWriteFile as any; // Set fake system time for consistent filenames setSystemTime(new Date("2024-01-01T00:00:00.000Z")); // 1704067200000 @@ -59,10 +56,6 @@ describe("downloadCommentImages", () => { delete process.env.GITHUB_SERVER_URL; } - // Restore original fs methods - _fsForTest.mkdir = origMkdir; - _fsForTest.writeFile = origWriteFile; - consoleLogSpy.mockRestore(); consoleWarnSpy.mockRestore(); consoleErrorSpy.mockRestore(); @@ -90,7 +83,13 @@ describe("downloadCommentImages", () => { const mockOctokit = createMockOctokit(); const comments: CommentWithImages[] = []; - await downloadCommentImages(mockOctokit, "owner", "repo", comments); + await downloadCommentImages( + mockOctokit, + "owner", + "repo", + comments, + mockFsOps, + ); expect(mockMkdir).toHaveBeenCalledWith("/tmp/github-images", { recursive: true, @@ -112,6 +111,7 @@ describe("downloadCommentImages", () => { "owner", "repo", comments, + mockFsOps, ); expect(result.size).toBe(0); @@ -155,6 +155,7 @@ describe("downloadCommentImages", () => { "owner", "repo", comments, + mockFsOps, ); expect(mockOctokit.rest.issues.getComment).toHaveBeenCalledWith({ @@ -215,6 +216,7 @@ describe("downloadCommentImages", () => { "owner", "repo", comments, + mockFsOps, ); expect(mockOctokit.rest.pulls.getReviewComment).toHaveBeenCalledWith({ @@ -262,6 +264,7 @@ describe("downloadCommentImages", () => { "owner", "repo", comments, + mockFsOps, ); expect(mockOctokit.rest.pulls.getReview).toHaveBeenCalledWith({ @@ -309,6 +312,7 @@ describe("downloadCommentImages", () => { "owner", "repo", comments, + mockFsOps, ); expect(mockOctokit.rest.issues.get).toHaveBeenCalledWith({ @@ -357,6 +361,7 @@ describe("downloadCommentImages", () => { "owner", "repo", comments, + mockFsOps, ); expect(mockOctokit.rest.pulls.get).toHaveBeenCalledWith({ @@ -408,6 +413,7 @@ describe("downloadCommentImages", () => { "owner", "repo", comments, + mockFsOps, ); expect(fetchSpy).toHaveBeenCalledTimes(2); @@ -459,6 +465,7 @@ describe("downloadCommentImages", () => { "owner", "repo", comments, + mockFsOps, ); expect(fetchSpy).toHaveBeenCalledTimes(1); // Only downloaded once @@ -492,6 +499,7 @@ describe("downloadCommentImages", () => { "owner", "repo", comments, + mockFsOps, ); expect(result.size).toBe(0); @@ -532,6 +540,7 @@ describe("downloadCommentImages", () => { "owner", "repo", comments, + mockFsOps, ); expect(result.size).toBe(0); @@ -563,6 +572,7 @@ describe("downloadCommentImages", () => { "owner", "repo", comments, + mockFsOps, ); expect(result.size).toBe(0); @@ -634,6 +644,7 @@ describe("downloadCommentImages", () => { "owner", "repo", comments, + mockFsOps, ); expect(result.get(url)).toBe( `/tmp/github-images/image-${1704067200000 + callIndex}-0${ext}`, @@ -678,6 +689,7 @@ describe("downloadCommentImages", () => { "owner", "repo", comments, + mockFsOps, ); expect(fetchSpy).toHaveBeenCalledTimes(1); @@ -723,6 +735,7 @@ describe("downloadCommentImages", () => { "owner", "repo", comments, + mockFsOps, ); expect(mockOctokit.rest.issues.getComment).toHaveBeenCalledWith({ @@ -787,6 +800,7 @@ describe("downloadCommentImages", () => { "owner", "repo", comments, + mockFsOps, ); expect(fetchSpy).toHaveBeenCalledTimes(2); @@ -837,6 +851,7 @@ describe("downloadCommentImages", () => { "owner", "repo", comments, + mockFsOps, ); expect(fetchSpy).toHaveBeenCalledTimes(2); @@ -883,6 +898,7 @@ describe("downloadCommentImages", () => { "owner", "repo", comments, + mockFsOps, ); expect(fetchSpy).toHaveBeenCalledTimes(1); // Only downloaded once @@ -927,6 +943,7 @@ describe("downloadCommentImages", () => { "owner", "repo", comments, + mockFsOps, ); expect(fetchSpy).toHaveBeenCalledTimes(1);