fix(test): stop mock.module() leaks from poisoning the unit test suite - #21
Merged
Conversation
Three test files replaced the entire Provider/Auth/fs-promises/child_process modules via mock.module() instead of preserving their real exports. Since ESM namespace imports are live bindings, this had two compounding effects for the rest of the bun test process: - Provider/Auth lost their .Service Effect tag (not just .list/.all), and Config.layer/ShareNext.layer memoize their build process-wide (a shared MemoMap in src/effect/run-service.ts) — so the first construction that ran while the mock was live got permanently cached as broken, cascading into 600+ unrelated failures in later files. - The fs/promises readFile mock ignored its path argument, so any unrelated file read elsewhere in the process during the mock window got this fixture's content back (observed corrupting test/session/llm.test.ts's JSON fixture loader). - resetMocks() itself restored from the same live bindings it was trying to undo, which could re-register already-mocked state instead of the original. Fixed by capturing real exports as frozen plain-object snapshots before any mocking happens, and scoping the fs/promises override to the exact path it simulates instead of intercepting every read. Also: bump storybook build's V8 heap (ubuntu-latest has less RAM than the Blacksmith runner this workflow used to run on) and add missing summary/description to 3 Team run-control OpenAPI operations that an unrelated compat test flags as undocumented (SDK regenerated to match).
|
This PR doesn't fully meet our contributing guidelines and PR template. What needs to be fixed:
Please edit this PR description to address the above within 2 hours, or it will be automatically closed. If you believe this was flagged incorrectly, please let a maintainer know. |
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
added 2 commits
July 30, 2026 12:16
…ers mock Masked until now by the storybook build OOMing before it got this far: the mock only re-implemented useProviders(), not the standalone popularProviders array that src/hooks/use-providers.ts also exports and that team-model-selector.tsx imports directly.
"C:/nonexistent/..." is a Windows-style path, not absolute on POSIX, so on Linux CI it hit PATH_NOT_ABSOLUTE (a different, already-covered validation path) instead of the intended PATH_NOT_DIRECTORY/GIT_COMMAND_FAILED. Use tmpdir(), which is absolute on every platform.
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.
Issue for this PR
Closes #
Type of change
What does this PR do?
Root-causes and fixes the
unit (linux)/unit (windows)full-suite cascade failure (600+ unrelated tests failing withTypeError: undefined is not an object (evaluating 'Auth.Service')/Provider.Service).Three test files (
provider-discovery.regression.test.ts,provider-discovery.integration.test.ts,provider-discovery.bench.test.ts) usedmock.module()to replace the entireProvider/Auth/node:fs/promises/node:child_processmodules instead of preserving their real exports. Two compounding effects:.ServiceEffect tag (not just.list/.all).Config.layer/ShareNext.layermemoize their construction process-wide via a sharedMemoMap(src/effect/run-service.ts). If that first construction ran while the mock was live, the resultingundefinedgot permanently cached — cascading into hundreds of unrelated failures in later files for the rest of the process.node:fs/promisesmock'sreadFileignored its path argument, so any unrelated file read elsewhere in the process during the mock window got this fixture's fake content back (observed corruptingtest/session/llm.test.ts's JSON fixture loader).resetMocks()itself restored from the same live ESM namespace bindings it was trying to undo — since those bindings reflect whatever mock.module() currently has active, this could re-register already-mocked state instead of the pristine original.Fixed by capturing the real exports as frozen plain-object snapshots before any test in each file mocks anything, and scoping the
fs/promisesoverride to the exact credential-file path it simulates (~/.claude/.credentials.json) instead of intercepting every read.Also included (found while verifying CI end-to-end):
storybookbuild was OOMing onubuntu-latest(less RAM than the Blacksmith runner it used to run on) — bumpedNODE_OPTIONS=--max-old-space-size.team.pauseRun/resumeRun/cancelRun) were missing adescription, flagged byopenapi-compat.test.ts. Added descriptions and regenerated the SDK.How did you verify your code works?
bun testinpackages/opencode, full suite), bisected to the exact test files and lines.Auth.Service/Provider.Service/leaked-fixture errors across 3 consecutive full runs).unit (linux)GitHub Actions log: both pass in 14-17ms there — they're local-machine artifacts, not regressions).bun turbo typecheck: 15/15 green.packages/sdk/openapi.json+sdk.gen.ts, diff is exactly the 3 added descriptions.Screenshots / recordings
N/A (test infra + CI workflow fix, no UI change).
Checklist