From dab92ef0ebd89e32e47f5baa5e13db7ba3cf91ed Mon Sep 17 00:00:00 2001 From: Vlad Mocanu Date: Thu, 26 Feb 2026 09:05:39 +0200 Subject: [PATCH] fix: use mock.module for image-downloader tests Replace the _fsForTest/DI patterns with Bun's native mock.module() API. This mocks fs/promises at the module loader level, which works reliably on Linux where jest.fn() call tracking fails cross-module. The source code is now clean with no testing artifacts. Also switches from jest.fn() to mock() (Bun's native API) for Octokit mocks in these tests. Co-Authored-By: Claude Opus 4.6 --- src/github/utils/image-downloader.ts | 15 +- test/image-downloader.test.ts | 259 ++++++++++++++------------- 2 files changed, 138 insertions(+), 136 deletions(-) diff --git a/src/github/utils/image-downloader.ts b/src/github/utils/image-downloader.ts index 924b009..0b2c0c7 100644 --- a/src/github/utils/image-downloader.ts +++ b/src/github/utils/image-downloader.ts @@ -2,16 +2,6 @@ import fs from "fs/promises"; import path from "path"; import type { Octokits } from "../api/client"; -export type FsOps = { - mkdir: typeof fs.mkdir; - writeFile: typeof fs.writeFile; -}; - -const defaultFsOps: FsOps = { - mkdir: fs.mkdir.bind(fs), - writeFile: fs.writeFile.bind(fs), -}; - // Build regexes at call time, reading GITHUB_SERVER_URL from process.env // directly. The config.ts export freezes the value at module load time, // which can be wrong when the Forgejo CI runner injects its own @@ -73,12 +63,11 @@ export async function downloadCommentImages( owner: string, repo: string, comments: CommentWithImages[], - fsOps: FsOps = defaultFsOps, ): Promise> { const urlToPathMap = new Map(); const downloadsDir = "/tmp/github-images"; - await fsOps.mkdir(downloadsDir, { recursive: true }); + await fs.mkdir(downloadsDir, { recursive: true }); const { imageRegex, htmlImgRegex } = buildImageRegexes(); @@ -228,7 +217,7 @@ export async function downloadCommentImages( const arrayBuffer = await imageResponse.arrayBuffer(); const buffer = Buffer.from(arrayBuffer); - await fsOps.writeFile(localPath, buffer); + await fs.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 36656df..7c453b9 100644 --- a/test/image-downloader.test.ts +++ b/test/image-downloader.test.ts @@ -5,13 +5,26 @@ import { spyOn, beforeEach, afterEach, - jest, + mock, setSystemTime, } from "bun:test"; + +// Mock fs/promises at the module loader level. Bun hoists mock.module calls +// before imports, so the source module's `import fs from "fs/promises"` gets +// these mocks. This avoids cross-module jest.fn() tracking bugs on Linux Bun. +const mockMkdir = mock(() => Promise.resolve(undefined as any)); +const mockWriteFile = mock(() => Promise.resolve(undefined as any)); + +mock.module("fs/promises", () => ({ + default: { + mkdir: mockMkdir, + writeFile: mockWriteFile, + }, +})); + import { downloadCommentImages, type CommentWithImages, - type FsOps, } from "../src/github/utils/image-downloader"; import type { Octokits } from "../src/github/api/client"; @@ -22,14 +35,6 @@ describe("downloadCommentImages", () => { let fetchSpy: any; let savedServerUrl: string | undefined; - // 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. // The image-downloader reads process.env directly at call time. @@ -67,13 +72,13 @@ describe("downloadCommentImages", () => { return { rest: { issues: { - getComment: jest.fn(), - get: jest.fn(), + getComment: mock(() => {}), + get: mock(() => {}), }, pulls: { - getReviewComment: jest.fn(), - getReview: jest.fn(), - get: jest.fn(), + getReviewComment: mock(() => {}), + getReview: mock(() => {}), + get: mock(() => {}), }, }, } as any as Octokits; @@ -83,13 +88,7 @@ describe("downloadCommentImages", () => { const mockOctokit = createMockOctokit(); const comments: CommentWithImages[] = []; - await downloadCommentImages( - mockOctokit, - "owner", - "repo", - comments, - mockFsOps, - ); + await downloadCommentImages(mockOctokit, "owner", "repo", comments); expect(mockMkdir).toHaveBeenCalledWith("/tmp/github-images", { recursive: true, @@ -111,7 +110,6 @@ describe("downloadCommentImages", () => { "owner", "repo", comments, - mockFsOps, ); expect(result.size).toBe(0); @@ -129,11 +127,13 @@ describe("downloadCommentImages", () => { // Mock octokit response // @ts-expect-error Mock implementation doesn't match full type signature - mockOctokit.rest.issues.getComment = jest.fn().mockResolvedValue({ - data: { - body_html: ``, - }, - }); + mockOctokit.rest.issues.getComment = mock(() => + Promise.resolve({ + data: { + body_html: ``, + }, + }), + ); // Mock fetch for image download const mockArrayBuffer = new ArrayBuffer(8); @@ -155,7 +155,6 @@ describe("downloadCommentImages", () => { "owner", "repo", comments, - mockFsOps, ); expect(mockOctokit.rest.issues.getComment).toHaveBeenCalledWith({ @@ -192,11 +191,13 @@ describe("downloadCommentImages", () => { "https://private-user-images.githubusercontent.com/review.jpg?jwt=token"; // @ts-expect-error Mock implementation doesn't match full type signature - mockOctokit.rest.pulls.getReviewComment = jest.fn().mockResolvedValue({ - data: { - body_html: ``, - }, - }); + mockOctokit.rest.pulls.getReviewComment = mock(() => + Promise.resolve({ + data: { + body_html: ``, + }, + }), + ); fetchSpy = spyOn(global, "fetch").mockResolvedValue({ ok: true, @@ -216,7 +217,6 @@ describe("downloadCommentImages", () => { "owner", "repo", comments, - mockFsOps, ); expect(mockOctokit.rest.pulls.getReviewComment).toHaveBeenCalledWith({ @@ -239,11 +239,13 @@ describe("downloadCommentImages", () => { "https://private-user-images.githubusercontent.com/body.png?jwt=token"; // @ts-expect-error Mock implementation doesn't match full type signature - mockOctokit.rest.pulls.getReview = jest.fn().mockResolvedValue({ - data: { - body_html: ``, - }, - }); + mockOctokit.rest.pulls.getReview = mock(() => + Promise.resolve({ + data: { + body_html: ``, + }, + }), + ); fetchSpy = spyOn(global, "fetch").mockResolvedValue({ ok: true, @@ -264,7 +266,6 @@ describe("downloadCommentImages", () => { "owner", "repo", comments, - mockFsOps, ); expect(mockOctokit.rest.pulls.getReview).toHaveBeenCalledWith({ @@ -288,11 +289,13 @@ describe("downloadCommentImages", () => { "https://private-user-images.githubusercontent.com/issue.gif?jwt=token"; // @ts-expect-error Mock implementation doesn't match full type signature - mockOctokit.rest.issues.get = jest.fn().mockResolvedValue({ - data: { - body_html: ``, - }, - }); + mockOctokit.rest.issues.get = mock(() => + Promise.resolve({ + data: { + body_html: ``, + }, + }), + ); fetchSpy = spyOn(global, "fetch").mockResolvedValue({ ok: true, @@ -312,7 +315,6 @@ describe("downloadCommentImages", () => { "owner", "repo", comments, - mockFsOps, ); expect(mockOctokit.rest.issues.get).toHaveBeenCalledWith({ @@ -337,11 +339,13 @@ describe("downloadCommentImages", () => { "https://private-user-images.githubusercontent.com/pr.webp?jwt=token"; // @ts-expect-error Mock implementation doesn't match full type signature - mockOctokit.rest.pulls.get = jest.fn().mockResolvedValue({ - data: { - body_html: ``, - }, - }); + mockOctokit.rest.pulls.get = mock(() => + Promise.resolve({ + data: { + body_html: ``, + }, + }), + ); fetchSpy = spyOn(global, "fetch").mockResolvedValue({ ok: true, @@ -361,7 +365,6 @@ describe("downloadCommentImages", () => { "owner", "repo", comments, - mockFsOps, ); expect(mockOctokit.rest.pulls.get).toHaveBeenCalledWith({ @@ -389,11 +392,13 @@ describe("downloadCommentImages", () => { "https://private-user-images.githubusercontent.com/2.jpg?jwt=token2"; // @ts-expect-error Mock implementation doesn't match full type signature - mockOctokit.rest.issues.getComment = jest.fn().mockResolvedValue({ - data: { - body_html: ``, - }, - }); + mockOctokit.rest.issues.getComment = mock(() => + Promise.resolve({ + data: { + body_html: ``, + }, + }), + ); fetchSpy = spyOn(global, "fetch").mockResolvedValue({ ok: true, @@ -413,7 +418,6 @@ describe("downloadCommentImages", () => { "owner", "repo", comments, - mockFsOps, ); expect(fetchSpy).toHaveBeenCalledTimes(2); @@ -436,11 +440,13 @@ describe("downloadCommentImages", () => { "https://private-user-images.githubusercontent.com/dup.png?jwt=token"; // @ts-expect-error Mock implementation doesn't match full type signature - mockOctokit.rest.issues.getComment = jest.fn().mockResolvedValue({ - data: { - body_html: ``, - }, - }); + mockOctokit.rest.issues.getComment = mock(() => + Promise.resolve({ + data: { + body_html: ``, + }, + }), + ); fetchSpy = spyOn(global, "fetch").mockResolvedValue({ ok: true, @@ -465,7 +471,6 @@ describe("downloadCommentImages", () => { "owner", "repo", comments, - mockFsOps, ); expect(fetchSpy).toHaveBeenCalledTimes(1); // Only downloaded once @@ -480,11 +485,13 @@ describe("downloadCommentImages", () => { const imageUrl = "https://github.com/user-attachments/assets/missing.png"; // @ts-expect-error Mock implementation doesn't match full type signature - mockOctokit.rest.issues.getComment = jest.fn().mockResolvedValue({ - data: { - body_html: null, - }, - }); + mockOctokit.rest.issues.getComment = mock(() => + Promise.resolve({ + data: { + body_html: null, + }, + }), + ); const comments: CommentWithImages[] = [ { @@ -499,7 +506,6 @@ describe("downloadCommentImages", () => { "owner", "repo", comments, - mockFsOps, ); expect(result.size).toBe(0); @@ -515,11 +521,13 @@ describe("downloadCommentImages", () => { "https://private-user-images.githubusercontent.com/error.png?jwt=token"; // @ts-expect-error Mock implementation doesn't match full type signature - mockOctokit.rest.issues.getComment = jest.fn().mockResolvedValue({ - data: { - body_html: ``, - }, - }); + mockOctokit.rest.issues.getComment = mock(() => + Promise.resolve({ + data: { + body_html: ``, + }, + }), + ); fetchSpy = spyOn(global, "fetch").mockResolvedValue({ ok: false, @@ -540,7 +548,6 @@ describe("downloadCommentImages", () => { "owner", "repo", comments, - mockFsOps, ); expect(result.size).toBe(0); @@ -555,9 +562,9 @@ describe("downloadCommentImages", () => { const imageUrl = "https://github.com/user-attachments/assets/api-error.png"; // @ts-expect-error Mock implementation doesn't match full type signature - mockOctokit.rest.issues.getComment = jest - .fn() - .mockRejectedValue(new Error("API rate limit exceeded")); + mockOctokit.rest.issues.getComment = mock(() => + Promise.reject(new Error("API rate limit exceeded")), + ); const comments: CommentWithImages[] = [ { @@ -572,7 +579,6 @@ describe("downloadCommentImages", () => { "owner", "repo", comments, - mockFsOps, ); expect(result.size).toBe(0); @@ -618,11 +624,13 @@ describe("downloadCommentImages", () => { let callIndex = 0; // @ts-expect-error Mock implementation doesn't match full type signature - mockOctokit.rest.issues.getComment = jest.fn().mockResolvedValue({ - data: { - body_html: ``, - }, - }); + mockOctokit.rest.issues.getComment = mock(() => + Promise.resolve({ + data: { + body_html: ``, + }, + }), + ); fetchSpy = spyOn(global, "fetch").mockResolvedValue({ ok: true, @@ -644,7 +652,6 @@ describe("downloadCommentImages", () => { "owner", "repo", comments, - mockFsOps, ); expect(result.get(url)).toBe( `/tmp/github-images/image-${1704067200000 + callIndex}-0${ext}`, @@ -665,11 +672,13 @@ describe("downloadCommentImages", () => { // Only one signed URL for two images // @ts-expect-error Mock implementation doesn't match full type signature - mockOctokit.rest.issues.getComment = jest.fn().mockResolvedValue({ - data: { - body_html: ``, - }, - }); + mockOctokit.rest.issues.getComment = mock(() => + Promise.resolve({ + data: { + body_html: ``, + }, + }), + ); fetchSpy = spyOn(global, "fetch").mockResolvedValue({ ok: true, @@ -689,7 +698,6 @@ describe("downloadCommentImages", () => { "owner", "repo", comments, - mockFsOps, ); expect(fetchSpy).toHaveBeenCalledTimes(1); @@ -709,11 +717,13 @@ describe("downloadCommentImages", () => { // Mock octokit response // @ts-expect-error Mock implementation doesn't match full type signature - mockOctokit.rest.issues.getComment = jest.fn().mockResolvedValue({ - data: { - body_html: ``, - }, - }); + mockOctokit.rest.issues.getComment = mock(() => + Promise.resolve({ + data: { + body_html: ``, + }, + }), + ); // Mock fetch for image download const mockArrayBuffer = new ArrayBuffer(8); @@ -735,7 +745,6 @@ describe("downloadCommentImages", () => { "owner", "repo", comments, - mockFsOps, ); expect(mockOctokit.rest.issues.getComment).toHaveBeenCalledWith({ @@ -776,11 +785,13 @@ describe("downloadCommentImages", () => { "https://private-user-images.githubusercontent.com/double.png?jwt=token2"; // @ts-expect-error Mock implementation doesn't match full type signature - mockOctokit.rest.issues.getComment = jest.fn().mockResolvedValue({ - data: { - body_html: ``, - }, - }); + mockOctokit.rest.issues.getComment = mock(() => + Promise.resolve({ + data: { + body_html: ``, + }, + }), + ); fetchSpy = spyOn(global, "fetch").mockResolvedValue({ ok: true, @@ -800,7 +811,6 @@ describe("downloadCommentImages", () => { "owner", "repo", comments, - mockFsOps, ); expect(fetchSpy).toHaveBeenCalledTimes(2); @@ -827,11 +837,13 @@ describe("downloadCommentImages", () => { "https://private-user-images.githubusercontent.com/html.jpg?jwt=token2"; // @ts-expect-error Mock implementation doesn't match full type signature - mockOctokit.rest.issues.getComment = jest.fn().mockResolvedValue({ - data: { - body_html: ``, - }, - }); + mockOctokit.rest.issues.getComment = mock(() => + Promise.resolve({ + data: { + body_html: ``, + }, + }), + ); fetchSpy = spyOn(global, "fetch").mockResolvedValue({ ok: true, @@ -851,7 +863,6 @@ describe("downloadCommentImages", () => { "owner", "repo", comments, - mockFsOps, ); expect(fetchSpy).toHaveBeenCalledTimes(2); @@ -874,11 +885,13 @@ describe("downloadCommentImages", () => { "https://private-user-images.githubusercontent.com/dup.png?jwt=token"; // @ts-expect-error Mock implementation doesn't match full type signature - mockOctokit.rest.issues.getComment = jest.fn().mockResolvedValue({ - data: { - body_html: ``, - }, - }); + mockOctokit.rest.issues.getComment = mock(() => + Promise.resolve({ + data: { + body_html: ``, + }, + }), + ); fetchSpy = spyOn(global, "fetch").mockResolvedValue({ ok: true, @@ -898,7 +911,6 @@ describe("downloadCommentImages", () => { "owner", "repo", comments, - mockFsOps, ); expect(fetchSpy).toHaveBeenCalledTimes(1); // Only downloaded once @@ -919,11 +931,13 @@ describe("downloadCommentImages", () => { "https://private-user-images.githubusercontent.com/complex.webp?jwt=token"; // @ts-expect-error Mock implementation doesn't match full type signature - mockOctokit.rest.issues.getComment = jest.fn().mockResolvedValue({ - data: { - body_html: ``, - }, - }); + mockOctokit.rest.issues.getComment = mock(() => + Promise.resolve({ + data: { + body_html: ``, + }, + }), + ); fetchSpy = spyOn(global, "fetch").mockResolvedValue({ ok: true, @@ -943,7 +957,6 @@ describe("downloadCommentImages", () => { "owner", "repo", comments, - mockFsOps, ); expect(fetchSpy).toHaveBeenCalledTimes(1);