From 2f44e40942a074d11d0ebb6964c0349fd479f9b8 Mon Sep 17 00:00:00 2001
From: Vlad Mocanu <vtmocanu@gmail.com>
Date: Thu, 26 Feb 2026 09:37:28 +0200
Subject: [PATCH] fix: remove all image-downloader mocking from
 forgejo-data-fetcher test
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 test/forgejo-data-fetcher.test.ts | 34 ++++++++++++++++---------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/test/forgejo-data-fetcher.test.ts b/test/forgejo-data-fetcher.test.ts
index dd658c3..2f7649f 100644
--- a/test/forgejo-data-fetcher.test.ts
+++ b/test/forgejo-data-fetcher.test.ts
@@ -1,4 +1,4 @@
-import { beforeEach, describe, expect, it, jest, spyOn } from "bun:test";
+import { describe, expect, it, jest } from "bun:test";
 import {
   fetchForgejoData,
   mapFileStatus,
@@ -8,12 +8,23 @@ import {
 } from "../src/forgejo/data/fetcher";
 import type { Octokits } from "../src/github/api/client";
 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
-// globally for the entire process — poisoning other test files
-// (like image-downloader.test.ts) that import the same module.
-// spyOn is auto-restored after each test, so no cross-file pollution.
-import * as imageDownloaderModule from "../src/github/utils/image-downloader";
+
+// NOTE: We intentionally do NOT mock downloadCommentImages here.
+//
+// Previous attempts to mock it (mock.module and spyOn) both caused
+// cross-file test pollution in Bun's single-process runner:
+//   - 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
 function createMockOctokits({
@@ -126,15 +137,6 @@ const mockIssueRestResponse = {
 };
 
 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", () => {
     it("should fetch PR details, files, commits, comments, and reviews in parallel then per-review comments", async () => {
       const mockOctokits = createMockOctokits({