fix: use dependency injection for image-downloader fs mocks

mock.module("fs/promises") on Linux Bun poisons the entire mock
tracking system — not just fs mocks, but ALL mock/spy assertions
(octokit mocks, spyOn(global, "fetch"), etc.) stop recording calls.

Replace with dependency injection: downloadCommentImages() now accepts
an optional fsDeps parameter. Tests pass mock mkdir/writeFile directly.
Production callers omit it and get real fs. No mock.module anywhere.

Fixes 18 test failures blocking CI on Forgejo Linux 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:08 +02:00
1 parent 6113a4ef2d
commit a6b3c94004
3 files changed
+54 -16

No files matched your search

+13
View File
@@ -31,6 +31,18 @@ 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
- [x] **M1.5: Fix image-downloader test failures blocking CI**
- **Problem**: All 18 `downloadCommentImages` tests failed on Forgejo CI (Linux Bun 1.2.21) while passing locally on macOS. This blocked all PRs.
- **Root cause**: `mock.module("fs/promises")` on Linux Bun poisons the entire mock tracking system — not just fs mocks, but ALL mock/spy assertions in the file (octokit parameter mocks, `spyOn(global, "fetch")`, etc.) stopped recording calls. The only passing test used a **negative** assertion (`not.toHaveBeenCalledWith`) which trivially passes when tracking is broken.
- **History of failed attempts**:
1. Upstream uses `spyOn(fs, "mkdir")` — works on GitHub CI (Bun 1.2.12/ubuntu) but fails on Forgejo Linux (different `fs/promises` object references between test and source modules)
2. `96ec04e` tried `_fsForTest` mutable export — abandoned
3. `cff360d` tried dependency injection — replaced
4. `dab92ef` tried `mock.module("fs/promises")` — works on macOS, poisons mock tracking on Linux
- **Fix**: Dependency injection via optional `fsDeps` parameter on `downloadCommentImages()`. Tests pass mock `mkdir`/`writeFile` directly — no module-level mocking at all. Production callers omit the parameter and get real `fs`. Zero `mock.module` usage in the test file.
- **Validation**: 797/797 tests pass locally, typecheck clean. Awaiting CI confirmation.
- [ ] **M2: Test PR review comment trigger**
- Create a test PR on the repo
@@ -89,6 +101,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 |
## Success Criteria