fix: use dependency injection for image-downloader fs mocks

The _fsForTest mutable object pattern fails on Linux Bun because
test and source files get different module instances. Switch to
passing fs operations as an optional parameter with defaults.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Vlad MocanuandClaude Opus 4.6 committed 2026-02-26 08:54:58 +02:00
1 parent ac8e22e2f8
commit cff360d00a
2 files changed
+40 -21

No files matched your search

+31 -14
View File
@@ -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);