From 821cc86c6e23c3d8669c55991130aca83bab7cc6 Mon Sep 17 00:00:00 2001 From: Vlad Mocanu Date: Thu, 26 Feb 2026 09:38:13 +0200 Subject: [PATCH] =?UTF-8?q?docs(prd-6):=20mark=20M1.5=20resolved=20?= =?UTF-8?q?=E2=80=94=20CI=20confirmed=20green?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.6 --- prds/6-e2e-test-forgejo-triggers.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/prds/6-e2e-test-forgejo-triggers.md b/prds/6-e2e-test-forgejo-triggers.md index dbb72ca..a390675 100644 --- a/prds/6-e2e-test-forgejo-triggers.md +++ b/prds/6-e2e-test-forgejo-triggers.md @@ -31,15 +31,15 @@ Systematically exercise each untested scenario on the live Forgejo instance (git - Ensure the workflow has the correct permissions for each trigger type - **Validation**: Workflow YAML includes all required `on:` triggers -- [ ] **M1.5: Fix image-downloader test failures blocking CI** _(pending CI verification)_ +- [x] **M1.5: Fix image-downloader test failures blocking CI** - **Problem**: All 18 `downloadCommentImages` tests failed on Forgejo CI (Linux Bun 1.2.21 in `harbor.wxs.ro/wxs/grunt-runner:v1.9.0`) while passing locally on macOS. This blocked all PRs. - **Root cause**: `mock.module("../src/github/utils/image-downloader")` in `test/forgejo-data-fetcher.test.ts` replaced the image-downloader module **globally** in Bun's module registry for the entire process. When Bun's non-deterministic test file ordering ran forgejo-data-fetcher.test.ts BEFORE image-downloader.test.ts, the latter imported the stubbed module instead of the real one, breaking all mock tracking. The failure was intermittent because file ordering changed when other files were added/removed. - **Fix (2 parts)**: 1. `c6ca8a7`: Added DI (`FsDeps` interface + optional `fsDeps` param) to `downloadCommentImages()` — removed `mock.module("fs/promises")` from the image-downloader test itself - 2. `7c4191a`: Replaced `mock.module("image-downloader")` in forgejo-data-fetcher.test.ts with `spyOn(module, "fn")` — spyOn patches the live export per-test and is auto-restored, no cross-file pollution - - **Key learning**: Never use `mock.module()` in Bun test files — it poisons the module registry for all files in the process. Always use `spyOn` on the module namespace import instead. - - **Validation**: 797 tests pass locally, typecheck clean, critical file ordering (forgejo first → image-downloader second) verified passing + 2. `49eedd5`: Removed all image-downloader mocking from forgejo-data-fetcher.test.ts entirely — the real function is safe to call because test comments have no GitHub image URLs, so it returns an empty Map naturally. Both `mock.module` and `spyOn` caused cross-file pollution in Bun's single-process runner. + - **Key learning**: Never use `mock.module()` in Bun test files — it poisons the module registry for all files in the process. `spyOn` on ESM namespace exports also fails to restore correctly across files on Linux Bun. Best approach: avoid mocking entirely when the real function is safe to call. + - **Validation**: CI passed (all 3 jobs: test, prettier, typecheck) on commit `49eedd5` - [ ] **M2: Test PR review comment trigger**