fix(tests): stop partial mock.module calls from dropping real exports#13
Merged
Conversation
Bun's mock.module replaces the whole module for the rest of the run and leaks across files, so a factory that returns only the symbol under test silently deletes every other export. Any file that loads later and imports a dropped symbol dies with 'Export named X not found', which is reported as an unhandled error between tests rather than as a failing assertion. This broke three CI suites, each with a different victim: claw-server dropped textResult from browser-mcp's tool framework, agent dropped providerTemplates and providerTypeOptions, and server-api dropped tool from the ai package. The last one looked environmental because the job also logs Chromium dbus noise; it is not. The suites pass on macOS because directory traversal order differs from Linux and happens to load the real module first. Each fix was proven with an explicit two-file ordering that fails before and passes after. Two further latent instances were found by forcing every mock.module file to run first within its CI group: provider-factory-acp dropped most of node:fs/promises and browseros-dir, and metrics blanked INLINED_ENV, which src/config.ts reads. Suites now: claw-server 370, app 292, server-api 154, agent 301, lib 266, all with zero failures; bun run check exits 0.
Section 3 described a workflow that merges upstream every six hours and opens a tracking issue on conflict. That workflow is Upstream Sync (Quarantined): workflow_dispatch only, contents: read, and its single job prints the stop condition. The fork and upstream have no common ancestor, so an ordinary merge cannot synchronize them. fork-lifecycle.md already forbids claiming the upstream relationship is synchronized, so the section contradicted the repository's own rule. It also described opening a conflict issue, which cannot work because this repository has issues disabled.
✅ Tests passed — 1446/1450
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Three CI suites have been failing on
main—claw-server,agent, andserver-api. They share one root cause.Bun's
mock.modulereplaces the whole module for the remainder of abun testrun and leaks across files. A factory that returns only the symbol under test therefore deletes every other export. The next file that loads and imports a dropped symbol dies withExport named 'X' not found, surfaced as an unhandled error between tests rather than a failing assertion.tests/services/tab-group-ops.test.tstextResult(browser-mcp tool framework)modules/llm-providers/oauth-provider-flow.hooks.test.tsproviderTemplates,providerTypeOptionstests/api/services/chat-service.test.tstool(theaipackage)server-apilooked environmental because that job also logs Chromium/dbus noise and an AppImage cache miss. It is not — the browser started fine and the workflow handles the cache miss.The fix is the idiom two sibling tests already used: import the real module and spread it, overriding only what the test needs.
Why it was invisible locally
macOS and Linux walk directories in different orders. On macOS the real module happens to load first, so the suites pass. Each fix was therefore proven with an explicit two-file ordering that fails before and passes after — not by running the suite and hoping.
Also fixed
Forcing every
mock.modulefile to run first within its CI group surfaced two more latent instances:provider-factory-acpdropped most ofnode:fs/promisesandbrowseros-dir, andmetricsblankedINLINED_ENV, whichsrc/config.tsreads.The misleading comments that argued for the partial mocks were corrected too.
Verification
bun run checkNot fixed, reported honestly
tests/lib/mcp-manager/reconcile.test.tsfails under some multi-file orderings. It is an assertion mismatch from shared singleton state, not this export-drop class, and it does not occur in CI order (server-lib is green). It could not be narrowed to a minimal repro, so it was left alone rather than guessed at.Second commit
docs/nddev-fork.md§3 described a six-hourly upstream auto-merge that opens a conflict issue. The workflow isUpstream Sync (Quarantined)—workflow_dispatchonly, printing a stop condition, because the fork and upstream have no common ancestor.fork-lifecycle.mdexplicitly forbids claiming the relationship is synchronized, and issues are disabled on this repository so the conflict-issue path could not work either.