fix(test): resolve vi.mock hoisting failures on macOS runners#21500
fix(test): resolve vi.mock hoisting failures on macOS runners#21500irchelper wants to merge 2 commits intoopenclaw:mainfrom
Conversation
openclaw-barnacle
bot
added
channel: bluebubbles
irchelper
force-pushed
the
fix/macos-vi-mock-hoisting
branch
from
bcba112 to
6e70be6
Compare
irchelper
force-pushed
the
fix/macos-vi-mock-hoisting
branch
from
6e70be6 to
d5021d5
Compare
irchelper
force-pushed
the
fix/macos-vi-mock-hoisting
branch
2 times, most recently
from
a269c87 to
5df98a6
Compare
|
This pull request has been automatically marked as stale due to inactivity. |
|
Still relevant — CI green, ready for review. |
irchelper
force-pushed
the
fix/macos-vi-mock-hoisting
branch
from
5df98a6 to
c8821c7
Compare
irchelper
force-pushed
the
fix/macos-vi-mock-hoisting
branch
from
c8821c7 to
8d435b9
Compare
irchelper
force-pushed
the
fix/macos-vi-mock-hoisting
branch
from
8d435b9 to
c8821c7
Compare
irchelper
force-pushed
the
fix/macos-vi-mock-hoisting
branch
from
c8821c7 to
8e070ad
Compare
openclaw-barnacle
bot
added
size: S
and removed
size: M
stale
fabianwilliams
left a comment
fabianwilliams
left a comment
There was a problem hiding this comment.
Thanks for the fix — the vi.hoisted() wraps in actions.test.ts and message-action-runner.test.ts look clean and match the existing codebase pattern.
Two questions on the other files:
extensions/bluebubbles/src/test-mocks.ts — The old code imported from the test harness (createBlueBubblesAccountsMockModule, createBlueBubblesProbeMockModule). The new code inlines the mock implementations directly. Was there a reason you couldn't wrap the existing harness imports with vi.hoisted() instead? Inlining means these mocks won't pick up future changes to the harness.
src/discord/monitor/exec-approvals.test.ts — Same pattern: the old mock used importOriginal to spread the real module and only override createDiscordClient. The new version replaces the entire module and manually reimplements stripUndefinedFields. If that module adds exports later, this mock would silently hide them. Could vi.hoisted() be applied to just the overridden functions while keeping importOriginal?
Happy to re-review once addressed — or if there's a reason the inlining is intentional, that context would be helpful. Thanks!
|
Please don't make PRs for test failures on main. The team is aware of those and will handle them directly on the codebase, not only fixing the tests but also investigating what the root cause is. Having to sift through test-fix-PRs (including some that have been out of date for weeks...) on top of that doesn't help. There are already way too many PRs for humans to manage; please don't make the flood worse. Thank you. |
3 participants
Fix macOS-specific vi.mock hoisting failures (pre-existing, unrelated to any open feature PR).
On macOS Vitest runners, module-scope vi.fn() in vi.mock factories can be undefined after hoisting. Wrapping with vi.hoisted() ensures correct initialization order.
Files changed: actions.test.ts, media-send.test.ts, test-mocks.ts, feishu/media.test.ts, exec-approvals.test.ts, message-action-runner.test.ts
Greptile Summary
Wraps module-scope
vi.fn()calls invi.hoisted()to fix macOS-specific Vitest hoisting failures where mocks could be undefined after hoisting. The changes update 4 test files to use the correct initialization order for mock factories.Confidence Score: 5/5
vi.hoisted). The fixes address a real macOS-specific testing issue without touching production code.Last reviewed commit: 8b6c3b6