From ede18897b01777e9317ac60922aee6974d0700af Mon Sep 17 00:00:00 2001 From: Vlad Mocanu Date: Thu, 26 Feb 2026 09:33:50 +0200 Subject: [PATCH] docs(prd-6): mark M1.5 resolved, document root cause and fix Root cause was mock.module cross-file pollution in Bun's test runner, not Linux-specific mock tracking bugs as previously theorized. Co-Authored-By: Claude Opus 4.6 --- prds/6-e2e-test-forgejo-triggers.md | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/prds/6-e2e-test-forgejo-triggers.md b/prds/6-e2e-test-forgejo-triggers.md index c72564a..dbb72ca 100644 --- a/prds/6-e2e-test-forgejo-triggers.md +++ b/prds/6-e2e-test-forgejo-triggers.md @@ -31,18 +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** +- [ ] **M1.5: Fix image-downloader test failures blocking CI** _(pending CI verification)_ - - **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 blocks all PRs. - - **What we know**: On this Linux Docker image, `mock()` and `spyOn()` call tracking from `bun:test` is broken for this file. Even with DI (no module-level mocking at all), mock functions report "not called" despite being invoked. The ONLY passing test uses a negative assertion. This is NOT a `mock.module` issue — it affects plain `mock()` functions passed as parameters too. - - **History of failed attempts** (5 so far): - 1. Upstream uses `spyOn(fs, "mkdir")` — works on GitHub CI (Bun 1.2.12/ubuntu) but fails on Forgejo Linux - 2. `96ec04e` tried `_fsForTest` mutable export — abandoned - 3. `cff360d` tried dependency injection for fs mocks — still failed on CI - 4. `dab92ef` tried `mock.module("fs/promises")` — works on macOS, fails on Linux - 5. `c6ca8a7` removed `mock.module` entirely, used DI with `fsDeps` param — **still fails on CI** (mock tracking broken at a deeper level) - - **Next step**: Set up local Docker reproduction using `grunt-runner:v1.9.0` to debug interactively - - **Validation**: Pending — need local Docker repro first + - **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 - [ ] **M2: Test PR review comment trigger** @@ -102,7 +99,7 @@ Systematically exercise each untested scenario on the live Forgejo instance (git | Forgejo webhook payloads differ from expected | Log raw payloads, compare with GitHub format, adapt parsing | | Trigger type not supported by Forgejo Actions | Document as known limitation, skip gracefully | | Action creates unwanted branches/comments during testing | Use a dedicated test repo or clean up after each test | -| Bun mock.module breaks mock tracking on Linux | Use dependency injection for fs mocks, never mock.module | +| Bun mock.module breaks mock tracking on Linux | RESOLVED: use spyOn on module namespace, never mock.module | ## Success Criteria