fix(test): restore MOCK_MODE so mock suites do not leak dev mode across the bun suite#153
Merged
Merged
Conversation
bun test runs every file in one shared global, so a stray process.env write is visible to every later suite. mmcli-mode-validation and modem-migration set MOCK_MODE=true without restoring it, leaving isDevelopment() permanently true for the rest of the run. Later real-path suites (checkConnectivity, platform-claim HTTPS gating, C7 source rejection, isRealDevice-gated add-on ops) then took the wrong mock/dev branch depending on file order, failing only in the full suite. Snapshot and restore MOCK_MODE in teardown, matching the save/restore convention already used by mock-device-attach.test.ts. No production code changes; no test skipped or weakened.
Contributor
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
andrescera
added a commit
that referenced
this pull request
Jul 16, 2026
Two more cross-file test-isolation leaks surfaced on top of the MOCK_MODE fix (#153); both fail deterministically only in the full CI suite. 1. pairing-platform-claim.test.ts read config.json unconditionally in beforeEach. config.json is gitignored, so on a fresh CI checkout (or after remote-repair-migration restores its absent prior state) the read throws ENOENT and fails every test in the file — including the pure status-code and getPlatformUrl cases. Snapshot it only when it exists and restore-or-remove in afterEach, matching the pattern remote-repair-migration.test.ts already documents. 2. network-ingest-control.test.ts drove the real setIngestEnabled RPC, which writes the module-level network-ingest cache, but never called resetNetworkIngestState in teardown (the only cache-writer that didn't). A leaked rtmp-active snapshot made source-lost-rejection's C7 test see the rtmp source as available, so it returned network_ingest_gateway_inactive instead of source_unavailable. Reset the cache in the mocks block afterEach + afterAll. No production code changes; no test skipped or weakened.
andrescera
added a commit
that referenced
this pull request
Jul 17, 2026
…timer) (#157) Two pre-existing latent leaks surfaced only in a full backend-suite run under a local file ordering that differs from CI: - mock-feedback-broadcast.test.ts set process.env.MOCK_MODE="true" in two beforeAll blocks and never restored it. bun runs every test file in one shared global, so the leaked MOCK_MODE flipped isDevelopment() true for later files; combined with a leaked mockState.initialized that made shouldUseMocks() true, the kiosk RPC procedures ran against the mock harness instead of each test's injected deps (8 kiosk-rpc failures). - rpc-network.procedure.test.ts calls updateNetif() in mock mode; the mock ifconfig lists wlan0, so it schedules a detached setTimeout( wifiUpdateDevices, 1000) that fired ~1s later inside a following mock suite and registered the mock adapter dc:a6:32:12:34:57 (id 0) into the process-wide wifi-interface registry. macForDeviceId("0") then resolved to that leaked adapter instead of the seeded one, so the held device lock was missed and DEVICE_BUSY was not returned (1 wifi.procedure failure). Restore MOCK_MODE in both teardowns (the #153 save/restore idiom) and drain + evict the pending wifi timer in the network.configure afterAll. Full backend suite: 1930 pass / 0 fail, matching CI.
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
Two backend test files set
process.env.MOCK_MODE = "true"and never restored it, leaking dev-mock mode into every later test suite in the samebun testrun:apps/backend/src/tests/mmcli-mode-validation.test.ts— set in themmList()guard test;afterEachdid not restore it.apps/backend/src/tests/modem-migration.test.ts— set inbeforeAll;afterAlldid not restore it.This PR snapshots
MOCK_MODEonce and restores it in each file's teardown, matching the save/restore convention already used bymock-device-attach.test.ts. 10 insertions, no production code touched.Why
bun test(no--isolate/--parallel, exactly what CI runs) executes all 205 files in one shared global object, sequentially.shouldUseMocks()isisDevelopment() && mockState.initialized, andisDevelopment()isNODE_ENV === "development" || MOCK_MODE === "true".Because nothing ever clears
MOCK_MODE, once either file ran,isDevelopment()stayedtruefor the rest of the process. Later real-path suites — which deliberately never init mocks and inherit whatever global state the run left behind — then took the wrong dev/mock branch:checkConnectivity()short-circuits toreturn trueundershouldUseMocks()→internet-migration.test.tssawtruewhere it expectsfalse.shouldUseMocks()-gated.isRealDevice()-gated, andisRealDevice()short-circuits onisDevelopment().Whether a given victim failed depended on file execution order (Bun uses filesystem readdir order for the full suite), so it reproduced deterministically on the CI runner but not on a typical local box — and it blocked the
BE unit (bun) + exec-guard + biomejob on every open PR regardless of that PR's contents.How to verify
MOCK_MODE=truebefore this change; zero after.bun test <both leakers> internet-migration pairing-platform-claim source-lost-rejection(leakers first): 40 pass, 0 fail (the victims previously inherited the leak).bun tsc --noEmit: clean.bunx biome checkon both files: clean.qw-e-audio-hotplug.test.tsfs.watch-timing flakes that fail identically in isolation on the pristine pre-fix tree (pre-existing, environment-specific, not in the reported failing set).Risks
Minimal — test-only change that restores an env var the files should never have leaked. No production code, no test skipped or weakened (Rule E). Once merged, the four currently-blocked PRs (#149–#152) should go green on their next re-run with no change to them, since the leak lives in files none of them touch.