fix: remove all image-downloader mocking from forgejo-data-fetcher test

Both mock.module and spyOn cause cross-file test pollution in Bun's
single-process runner — mock.module poisons the module registry permanently,
and spyOn doesn't correctly restore ESM live bindings across files on Linux.

The real downloadCommentImages is safe to call because test comment bodies
contain no GitHub image URLs, so the function finds zero images and returns
an empty Map naturally. The only side effect is mkdir("/tmp/github-images")
which is harmless on CI runners.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Vlad MocanuandClaude Opus 4.6 committed 2026-02-26 10:45:09 +02:00
1 parent ede18897b0
commit 2f44e40942
1 file changed
+18 -16
+18 -16
View File
@@ -1,4 +1,4 @@
import { beforeEach, describe, expect, it, jest, spyOn } from "bun:test"; import { describe, expect, it, jest } from "bun:test";
import { import {
fetchForgejoData, fetchForgejoData,
mapFileStatus, mapFileStatus,
@@ -8,12 +8,23 @@ import {
} from "../src/forgejo/data/fetcher"; } from "../src/forgejo/data/fetcher";
import type { Octokits } from "../src/github/api/client"; import type { Octokits } from "../src/github/api/client";
import type { GitHubReviewComment } from "../src/github/types"; import type { GitHubReviewComment } from "../src/github/types";
// Import the module namespace so spyOn can patch the export directly.
// This avoids mock.module() which replaces the module in Bun's registry // NOTE: We intentionally do NOT mock downloadCommentImages here.
// globally for the entire process — poisoning other test files //
// (like image-downloader.test.ts) that import the same module. // Previous attempts to mock it (mock.module and spyOn) both caused
// spyOn is auto-restored after each test, so no cross-file pollution. // cross-file test pollution in Bun's single-process runner:
import * as imageDownloaderModule from "../src/github/utils/image-downloader"; // - mock.module() permanently replaces the module in Bun's registry,
// poisoning image-downloader.test.ts when it runs afterward.
// - spyOn() on ESM module namespace exports doesn't correctly restore
// live bindings across test files on Linux Bun.
//
// The real downloadCommentImages is safe to call here because:
// 1. Test comment bodies contain no GitHub image URLs
// (github.com/user-attachments/assets/...), so the function
// finds zero images and returns an empty Map.
// 2. The only side effect is mkdir("/tmp/github-images") which is
// harmless on CI runners.
// This avoids all module-level mocking issues entirely.
// Helper to build a mock Octokits with REST methods // Helper to build a mock Octokits with REST methods
function createMockOctokits({ function createMockOctokits({
@@ -126,15 +137,6 @@ const mockIssueRestResponse = {
}; };
describe("fetchForgejoData", () => { describe("fetchForgejoData", () => {
// Stub downloadCommentImages per-test — returns empty Map (no-op on Forgejo).
// spyOn patches the live module export and is auto-restored after each test,
// unlike mock.module which persists for the entire Bun process.
beforeEach(() => {
spyOn(imageDownloaderModule, "downloadCommentImages").mockResolvedValue(
new Map(),
);
});
describe("PR data fetching", () => { describe("PR data fetching", () => {
it("should fetch PR details, files, commits, comments, and reviews in parallel then per-review comments", async () => { it("should fetch PR details, files, commits, comments, and reviews in parallel then per-review comments", async () => {
const mockOctokits = createMockOctokits({ const mockOctokits = createMockOctokits({