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 <noreply@anthropic.com>
This commit is contained in:
Vlad MocanuandClaude Opus 4.6 committed 2026-02-26 09:05:39 +02:00
1 parent cff360d00a
commit dab92ef0eb
2 files changed
+90 -88

No files matched your search

+2 -13
View File
@@ -2,16 +2,6 @@ import fs from "fs/promises";
import path from "path"; import path from "path";
import type { Octokits } from "../api/client"; 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 // Build regexes at call time, reading GITHUB_SERVER_URL from process.env
// directly. The config.ts export freezes the value at module load time, // directly. The config.ts export freezes the value at module load time,
// which can be wrong when the Forgejo CI runner injects its own // which can be wrong when the Forgejo CI runner injects its own
@@ -73,12 +63,11 @@ export async function downloadCommentImages(
owner: string, owner: string,
repo: string, repo: string,
comments: CommentWithImages[], comments: CommentWithImages[],
fsOps: FsOps = defaultFsOps,
): Promise<Map<string, string>> { ): Promise<Map<string, string>> {
const urlToPathMap = new Map<string, string>(); const urlToPathMap = new Map<string, string>();
const downloadsDir = "/tmp/github-images"; const downloadsDir = "/tmp/github-images";
await fsOps.mkdir(downloadsDir, { recursive: true }); await fs.mkdir(downloadsDir, { recursive: true });
const { imageRegex, htmlImgRegex } = buildImageRegexes(); const { imageRegex, htmlImgRegex } = buildImageRegexes();
@@ -228,7 +217,7 @@ export async function downloadCommentImages(
const arrayBuffer = await imageResponse.arrayBuffer(); const arrayBuffer = await imageResponse.arrayBuffer();
const buffer = Buffer.from(arrayBuffer); const buffer = Buffer.from(arrayBuffer);
await fsOps.writeFile(localPath, buffer); await fs.writeFile(localPath, buffer);
console.log(`✓ Saved: ${localPath}`); console.log(`✓ Saved: ${localPath}`);
urlToPathMap.set(originalUrl, localPath); urlToPathMap.set(originalUrl, localPath);
+88 -75
View File
@@ -5,13 +5,26 @@ import {
spyOn, spyOn,
beforeEach, beforeEach,
afterEach, afterEach,
jest, mock,
setSystemTime, setSystemTime,
} from "bun:test"; } 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 { import {
downloadCommentImages, downloadCommentImages,
type CommentWithImages, type CommentWithImages,
type FsOps,
} from "../src/github/utils/image-downloader"; } from "../src/github/utils/image-downloader";
import type { Octokits } from "../src/github/api/client"; import type { Octokits } from "../src/github/api/client";
@@ -22,14 +35,6 @@ describe("downloadCommentImages", () => {
let fetchSpy: any; let fetchSpy: any;
let savedServerUrl: string | undefined; 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(() => { beforeEach(() => {
// Pin GITHUB_SERVER_URL so tests pass regardless of runner environment. // Pin GITHUB_SERVER_URL so tests pass regardless of runner environment.
// The image-downloader reads process.env directly at call time. // The image-downloader reads process.env directly at call time.
@@ -67,13 +72,13 @@ describe("downloadCommentImages", () => {
return { return {
rest: { rest: {
issues: { issues: {
getComment: jest.fn(), getComment: mock(() => {}),
get: jest.fn(), get: mock(() => {}),
}, },
pulls: { pulls: {
getReviewComment: jest.fn(), getReviewComment: mock(() => {}),
getReview: jest.fn(), getReview: mock(() => {}),
get: jest.fn(), get: mock(() => {}),
}, },
}, },
} as any as Octokits; } as any as Octokits;
@@ -83,13 +88,7 @@ describe("downloadCommentImages", () => {
const mockOctokit = createMockOctokit(); const mockOctokit = createMockOctokit();
const comments: CommentWithImages[] = []; const comments: CommentWithImages[] = [];
await downloadCommentImages( await downloadCommentImages(mockOctokit, "owner", "repo", comments);
mockOctokit,
"owner",
"repo",
comments,
mockFsOps,
);
expect(mockMkdir).toHaveBeenCalledWith("/tmp/github-images", { expect(mockMkdir).toHaveBeenCalledWith("/tmp/github-images", {
recursive: true, recursive: true,
@@ -111,7 +110,6 @@ describe("downloadCommentImages", () => {
"owner", "owner",
"repo", "repo",
comments, comments,
mockFsOps,
); );
expect(result.size).toBe(0); expect(result.size).toBe(0);
@@ -129,11 +127,13 @@ describe("downloadCommentImages", () => {
// Mock octokit response // Mock octokit response
// @ts-expect-error Mock implementation doesn't match full type signature // @ts-expect-error Mock implementation doesn't match full type signature
mockOctokit.rest.issues.getComment = jest.fn().mockResolvedValue({ mockOctokit.rest.issues.getComment = mock(() =>
Promise.resolve({
data: { data: {
body_html: `<img src="${signedUrl}">`, body_html: `<img src="${signedUrl}">`,
}, },
}); }),
);
// Mock fetch for image download // Mock fetch for image download
const mockArrayBuffer = new ArrayBuffer(8); const mockArrayBuffer = new ArrayBuffer(8);
@@ -155,7 +155,6 @@ describe("downloadCommentImages", () => {
"owner", "owner",
"repo", "repo",
comments, comments,
mockFsOps,
); );
expect(mockOctokit.rest.issues.getComment).toHaveBeenCalledWith({ expect(mockOctokit.rest.issues.getComment).toHaveBeenCalledWith({
@@ -192,11 +191,13 @@ describe("downloadCommentImages", () => {
"https://private-user-images.githubusercontent.com/review.jpg?jwt=token"; "https://private-user-images.githubusercontent.com/review.jpg?jwt=token";
// @ts-expect-error Mock implementation doesn't match full type signature // @ts-expect-error Mock implementation doesn't match full type signature
mockOctokit.rest.pulls.getReviewComment = jest.fn().mockResolvedValue({ mockOctokit.rest.pulls.getReviewComment = mock(() =>
Promise.resolve({
data: { data: {
body_html: `<img src="${signedUrl}">`, body_html: `<img src="${signedUrl}">`,
}, },
}); }),
);
fetchSpy = spyOn(global, "fetch").mockResolvedValue({ fetchSpy = spyOn(global, "fetch").mockResolvedValue({
ok: true, ok: true,
@@ -216,7 +217,6 @@ describe("downloadCommentImages", () => {
"owner", "owner",
"repo", "repo",
comments, comments,
mockFsOps,
); );
expect(mockOctokit.rest.pulls.getReviewComment).toHaveBeenCalledWith({ expect(mockOctokit.rest.pulls.getReviewComment).toHaveBeenCalledWith({
@@ -239,11 +239,13 @@ describe("downloadCommentImages", () => {
"https://private-user-images.githubusercontent.com/body.png?jwt=token"; "https://private-user-images.githubusercontent.com/body.png?jwt=token";
// @ts-expect-error Mock implementation doesn't match full type signature // @ts-expect-error Mock implementation doesn't match full type signature
mockOctokit.rest.pulls.getReview = jest.fn().mockResolvedValue({ mockOctokit.rest.pulls.getReview = mock(() =>
Promise.resolve({
data: { data: {
body_html: `<img src="${signedUrl}">`, body_html: `<img src="${signedUrl}">`,
}, },
}); }),
);
fetchSpy = spyOn(global, "fetch").mockResolvedValue({ fetchSpy = spyOn(global, "fetch").mockResolvedValue({
ok: true, ok: true,
@@ -264,7 +266,6 @@ describe("downloadCommentImages", () => {
"owner", "owner",
"repo", "repo",
comments, comments,
mockFsOps,
); );
expect(mockOctokit.rest.pulls.getReview).toHaveBeenCalledWith({ expect(mockOctokit.rest.pulls.getReview).toHaveBeenCalledWith({
@@ -288,11 +289,13 @@ describe("downloadCommentImages", () => {
"https://private-user-images.githubusercontent.com/issue.gif?jwt=token"; "https://private-user-images.githubusercontent.com/issue.gif?jwt=token";
// @ts-expect-error Mock implementation doesn't match full type signature // @ts-expect-error Mock implementation doesn't match full type signature
mockOctokit.rest.issues.get = jest.fn().mockResolvedValue({ mockOctokit.rest.issues.get = mock(() =>
Promise.resolve({
data: { data: {
body_html: `<img src="${signedUrl}">`, body_html: `<img src="${signedUrl}">`,
}, },
}); }),
);
fetchSpy = spyOn(global, "fetch").mockResolvedValue({ fetchSpy = spyOn(global, "fetch").mockResolvedValue({
ok: true, ok: true,
@@ -312,7 +315,6 @@ describe("downloadCommentImages", () => {
"owner", "owner",
"repo", "repo",
comments, comments,
mockFsOps,
); );
expect(mockOctokit.rest.issues.get).toHaveBeenCalledWith({ expect(mockOctokit.rest.issues.get).toHaveBeenCalledWith({
@@ -337,11 +339,13 @@ describe("downloadCommentImages", () => {
"https://private-user-images.githubusercontent.com/pr.webp?jwt=token"; "https://private-user-images.githubusercontent.com/pr.webp?jwt=token";
// @ts-expect-error Mock implementation doesn't match full type signature // @ts-expect-error Mock implementation doesn't match full type signature
mockOctokit.rest.pulls.get = jest.fn().mockResolvedValue({ mockOctokit.rest.pulls.get = mock(() =>
Promise.resolve({
data: { data: {
body_html: `<img src="${signedUrl}">`, body_html: `<img src="${signedUrl}">`,
}, },
}); }),
);
fetchSpy = spyOn(global, "fetch").mockResolvedValue({ fetchSpy = spyOn(global, "fetch").mockResolvedValue({
ok: true, ok: true,
@@ -361,7 +365,6 @@ describe("downloadCommentImages", () => {
"owner", "owner",
"repo", "repo",
comments, comments,
mockFsOps,
); );
expect(mockOctokit.rest.pulls.get).toHaveBeenCalledWith({ expect(mockOctokit.rest.pulls.get).toHaveBeenCalledWith({
@@ -389,11 +392,13 @@ describe("downloadCommentImages", () => {
"https://private-user-images.githubusercontent.com/2.jpg?jwt=token2"; "https://private-user-images.githubusercontent.com/2.jpg?jwt=token2";
// @ts-expect-error Mock implementation doesn't match full type signature // @ts-expect-error Mock implementation doesn't match full type signature
mockOctokit.rest.issues.getComment = jest.fn().mockResolvedValue({ mockOctokit.rest.issues.getComment = mock(() =>
Promise.resolve({
data: { data: {
body_html: `<img src="${signedUrl1}"><img src="${signedUrl2}">`, body_html: `<img src="${signedUrl1}"><img src="${signedUrl2}">`,
}, },
}); }),
);
fetchSpy = spyOn(global, "fetch").mockResolvedValue({ fetchSpy = spyOn(global, "fetch").mockResolvedValue({
ok: true, ok: true,
@@ -413,7 +418,6 @@ describe("downloadCommentImages", () => {
"owner", "owner",
"repo", "repo",
comments, comments,
mockFsOps,
); );
expect(fetchSpy).toHaveBeenCalledTimes(2); expect(fetchSpy).toHaveBeenCalledTimes(2);
@@ -436,11 +440,13 @@ describe("downloadCommentImages", () => {
"https://private-user-images.githubusercontent.com/dup.png?jwt=token"; "https://private-user-images.githubusercontent.com/dup.png?jwt=token";
// @ts-expect-error Mock implementation doesn't match full type signature // @ts-expect-error Mock implementation doesn't match full type signature
mockOctokit.rest.issues.getComment = jest.fn().mockResolvedValue({ mockOctokit.rest.issues.getComment = mock(() =>
Promise.resolve({
data: { data: {
body_html: `<img src="${signedUrl}">`, body_html: `<img src="${signedUrl}">`,
}, },
}); }),
);
fetchSpy = spyOn(global, "fetch").mockResolvedValue({ fetchSpy = spyOn(global, "fetch").mockResolvedValue({
ok: true, ok: true,
@@ -465,7 +471,6 @@ describe("downloadCommentImages", () => {
"owner", "owner",
"repo", "repo",
comments, comments,
mockFsOps,
); );
expect(fetchSpy).toHaveBeenCalledTimes(1); // Only downloaded once expect(fetchSpy).toHaveBeenCalledTimes(1); // Only downloaded once
@@ -480,11 +485,13 @@ describe("downloadCommentImages", () => {
const imageUrl = "https://github.com/user-attachments/assets/missing.png"; const imageUrl = "https://github.com/user-attachments/assets/missing.png";
// @ts-expect-error Mock implementation doesn't match full type signature // @ts-expect-error Mock implementation doesn't match full type signature
mockOctokit.rest.issues.getComment = jest.fn().mockResolvedValue({ mockOctokit.rest.issues.getComment = mock(() =>
Promise.resolve({
data: { data: {
body_html: null, body_html: null,
}, },
}); }),
);
const comments: CommentWithImages[] = [ const comments: CommentWithImages[] = [
{ {
@@ -499,7 +506,6 @@ describe("downloadCommentImages", () => {
"owner", "owner",
"repo", "repo",
comments, comments,
mockFsOps,
); );
expect(result.size).toBe(0); expect(result.size).toBe(0);
@@ -515,11 +521,13 @@ describe("downloadCommentImages", () => {
"https://private-user-images.githubusercontent.com/error.png?jwt=token"; "https://private-user-images.githubusercontent.com/error.png?jwt=token";
// @ts-expect-error Mock implementation doesn't match full type signature // @ts-expect-error Mock implementation doesn't match full type signature
mockOctokit.rest.issues.getComment = jest.fn().mockResolvedValue({ mockOctokit.rest.issues.getComment = mock(() =>
Promise.resolve({
data: { data: {
body_html: `<img src="${signedUrl}">`, body_html: `<img src="${signedUrl}">`,
}, },
}); }),
);
fetchSpy = spyOn(global, "fetch").mockResolvedValue({ fetchSpy = spyOn(global, "fetch").mockResolvedValue({
ok: false, ok: false,
@@ -540,7 +548,6 @@ describe("downloadCommentImages", () => {
"owner", "owner",
"repo", "repo",
comments, comments,
mockFsOps,
); );
expect(result.size).toBe(0); expect(result.size).toBe(0);
@@ -555,9 +562,9 @@ describe("downloadCommentImages", () => {
const imageUrl = "https://github.com/user-attachments/assets/api-error.png"; const imageUrl = "https://github.com/user-attachments/assets/api-error.png";
// @ts-expect-error Mock implementation doesn't match full type signature // @ts-expect-error Mock implementation doesn't match full type signature
mockOctokit.rest.issues.getComment = jest mockOctokit.rest.issues.getComment = mock(() =>
.fn() Promise.reject(new Error("API rate limit exceeded")),
.mockRejectedValue(new Error("API rate limit exceeded")); );
const comments: CommentWithImages[] = [ const comments: CommentWithImages[] = [
{ {
@@ -572,7 +579,6 @@ describe("downloadCommentImages", () => {
"owner", "owner",
"repo", "repo",
comments, comments,
mockFsOps,
); );
expect(result.size).toBe(0); expect(result.size).toBe(0);
@@ -618,11 +624,13 @@ describe("downloadCommentImages", () => {
let callIndex = 0; let callIndex = 0;
// @ts-expect-error Mock implementation doesn't match full type signature // @ts-expect-error Mock implementation doesn't match full type signature
mockOctokit.rest.issues.getComment = jest.fn().mockResolvedValue({ mockOctokit.rest.issues.getComment = mock(() =>
Promise.resolve({
data: { data: {
body_html: `<img src="https://private-user-images.githubusercontent.com/test?jwt=token">`, body_html: `<img src="https://private-user-images.githubusercontent.com/test?jwt=token">`,
}, },
}); }),
);
fetchSpy = spyOn(global, "fetch").mockResolvedValue({ fetchSpy = spyOn(global, "fetch").mockResolvedValue({
ok: true, ok: true,
@@ -644,7 +652,6 @@ describe("downloadCommentImages", () => {
"owner", "owner",
"repo", "repo",
comments, comments,
mockFsOps,
); );
expect(result.get(url)).toBe( expect(result.get(url)).toBe(
`/tmp/github-images/image-${1704067200000 + callIndex}-0${ext}`, `/tmp/github-images/image-${1704067200000 + callIndex}-0${ext}`,
@@ -665,11 +672,13 @@ describe("downloadCommentImages", () => {
// Only one signed URL for two images // Only one signed URL for two images
// @ts-expect-error Mock implementation doesn't match full type signature // @ts-expect-error Mock implementation doesn't match full type signature
mockOctokit.rest.issues.getComment = jest.fn().mockResolvedValue({ mockOctokit.rest.issues.getComment = mock(() =>
Promise.resolve({
data: { data: {
body_html: `<img src="${signedUrl1}">`, body_html: `<img src="${signedUrl1}">`,
}, },
}); }),
);
fetchSpy = spyOn(global, "fetch").mockResolvedValue({ fetchSpy = spyOn(global, "fetch").mockResolvedValue({
ok: true, ok: true,
@@ -689,7 +698,6 @@ describe("downloadCommentImages", () => {
"owner", "owner",
"repo", "repo",
comments, comments,
mockFsOps,
); );
expect(fetchSpy).toHaveBeenCalledTimes(1); expect(fetchSpy).toHaveBeenCalledTimes(1);
@@ -709,11 +717,13 @@ describe("downloadCommentImages", () => {
// Mock octokit response // Mock octokit response
// @ts-expect-error Mock implementation doesn't match full type signature // @ts-expect-error Mock implementation doesn't match full type signature
mockOctokit.rest.issues.getComment = jest.fn().mockResolvedValue({ mockOctokit.rest.issues.getComment = mock(() =>
Promise.resolve({
data: { data: {
body_html: `<img src="${signedUrl}">`, body_html: `<img src="${signedUrl}">`,
}, },
}); }),
);
// Mock fetch for image download // Mock fetch for image download
const mockArrayBuffer = new ArrayBuffer(8); const mockArrayBuffer = new ArrayBuffer(8);
@@ -735,7 +745,6 @@ describe("downloadCommentImages", () => {
"owner", "owner",
"repo", "repo",
comments, comments,
mockFsOps,
); );
expect(mockOctokit.rest.issues.getComment).toHaveBeenCalledWith({ expect(mockOctokit.rest.issues.getComment).toHaveBeenCalledWith({
@@ -776,11 +785,13 @@ describe("downloadCommentImages", () => {
"https://private-user-images.githubusercontent.com/double.png?jwt=token2"; "https://private-user-images.githubusercontent.com/double.png?jwt=token2";
// @ts-expect-error Mock implementation doesn't match full type signature // @ts-expect-error Mock implementation doesn't match full type signature
mockOctokit.rest.issues.getComment = jest.fn().mockResolvedValue({ mockOctokit.rest.issues.getComment = mock(() =>
Promise.resolve({
data: { data: {
body_html: `<img src="${signedUrl1}"><img src="${signedUrl2}">`, body_html: `<img src="${signedUrl1}"><img src="${signedUrl2}">`,
}, },
}); }),
);
fetchSpy = spyOn(global, "fetch").mockResolvedValue({ fetchSpy = spyOn(global, "fetch").mockResolvedValue({
ok: true, ok: true,
@@ -800,7 +811,6 @@ describe("downloadCommentImages", () => {
"owner", "owner",
"repo", "repo",
comments, comments,
mockFsOps,
); );
expect(fetchSpy).toHaveBeenCalledTimes(2); expect(fetchSpy).toHaveBeenCalledTimes(2);
@@ -827,11 +837,13 @@ describe("downloadCommentImages", () => {
"https://private-user-images.githubusercontent.com/html.jpg?jwt=token2"; "https://private-user-images.githubusercontent.com/html.jpg?jwt=token2";
// @ts-expect-error Mock implementation doesn't match full type signature // @ts-expect-error Mock implementation doesn't match full type signature
mockOctokit.rest.issues.getComment = jest.fn().mockResolvedValue({ mockOctokit.rest.issues.getComment = mock(() =>
Promise.resolve({
data: { data: {
body_html: `<img src="${signedUrl1}"><img src="${signedUrl2}">`, body_html: `<img src="${signedUrl1}"><img src="${signedUrl2}">`,
}, },
}); }),
);
fetchSpy = spyOn(global, "fetch").mockResolvedValue({ fetchSpy = spyOn(global, "fetch").mockResolvedValue({
ok: true, ok: true,
@@ -851,7 +863,6 @@ describe("downloadCommentImages", () => {
"owner", "owner",
"repo", "repo",
comments, comments,
mockFsOps,
); );
expect(fetchSpy).toHaveBeenCalledTimes(2); expect(fetchSpy).toHaveBeenCalledTimes(2);
@@ -874,11 +885,13 @@ describe("downloadCommentImages", () => {
"https://private-user-images.githubusercontent.com/dup.png?jwt=token"; "https://private-user-images.githubusercontent.com/dup.png?jwt=token";
// @ts-expect-error Mock implementation doesn't match full type signature // @ts-expect-error Mock implementation doesn't match full type signature
mockOctokit.rest.issues.getComment = jest.fn().mockResolvedValue({ mockOctokit.rest.issues.getComment = mock(() =>
Promise.resolve({
data: { data: {
body_html: `<img src="${signedUrl}">`, body_html: `<img src="${signedUrl}">`,
}, },
}); }),
);
fetchSpy = spyOn(global, "fetch").mockResolvedValue({ fetchSpy = spyOn(global, "fetch").mockResolvedValue({
ok: true, ok: true,
@@ -898,7 +911,6 @@ describe("downloadCommentImages", () => {
"owner", "owner",
"repo", "repo",
comments, comments,
mockFsOps,
); );
expect(fetchSpy).toHaveBeenCalledTimes(1); // Only downloaded once expect(fetchSpy).toHaveBeenCalledTimes(1); // Only downloaded once
@@ -919,11 +931,13 @@ describe("downloadCommentImages", () => {
"https://private-user-images.githubusercontent.com/complex.webp?jwt=token"; "https://private-user-images.githubusercontent.com/complex.webp?jwt=token";
// @ts-expect-error Mock implementation doesn't match full type signature // @ts-expect-error Mock implementation doesn't match full type signature
mockOctokit.rest.issues.getComment = jest.fn().mockResolvedValue({ mockOctokit.rest.issues.getComment = mock(() =>
Promise.resolve({
data: { data: {
body_html: `<img src="${signedUrl}">`, body_html: `<img src="${signedUrl}">`,
}, },
}); }),
);
fetchSpy = spyOn(global, "fetch").mockResolvedValue({ fetchSpy = spyOn(global, "fetch").mockResolvedValue({
ok: true, ok: true,
@@ -943,7 +957,6 @@ describe("downloadCommentImages", () => {
"owner", "owner",
"repo", "repo",
comments, comments,
mockFsOps,
); );
expect(fetchSpy).toHaveBeenCalledTimes(1); expect(fetchSpy).toHaveBeenCalledTimes(1);