fix(backend): stop cross-file test-isolation leaks (MOCK_MODE + wifi timer)#157
Merged
Merged
Conversation
…timer)
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.
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 |
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
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
Fixes two pre-existing, latent test-isolation leaks that only surface in a
full
bun run --filter backend testrun under a local file-execution orderthat differs from CI. On the affected ordering the suite reports 1921 pass /
9 fail; CI (a different readdir order) is green on the same commit. With this
change the full suite is 1930 pass / 0 fail in every order.
The 9 failures were: 8 in
kiosk-rpc.test.tsand 1 inwifi.procedure.test.ts. They trace to two independent leaked-state sources.Why
bun testruns every file in ONE shared global object, so any file that leavesprocess.env, a module singleton, or a pending timer dirty poisons every filethat runs after it.
Leak 1 —
mock-feedback-broadcast.test.tsleaksMOCK_MODE(8 kiosk failures).Two
beforeAllblocks setprocess.env.MOCK_MODE = "true"and never restoredit.
MOCK_MODE=truemakesisDevelopment()true; combined with a leakedmockState.initialized,shouldUseMocks()becomes true for later files. Thekiosk RPC procedures (
system.procedure.ts) then take the mock branch —resolveActiveKioskDeps()returns the in-memory mock harness instead of eachtest's injected spy deps, and the
!shouldUseMocks() && !isRealDevice()gate isbypassed. So the "emulated" tests wrongly succeed, the "real" tests run against
the wrong deps (empty spies), and the "waits for cog-display" test times out.
Leak 2 —
rpc-network.procedure.test.tsleaks a wifi-scan timer (1 wifi failure).Its
network.configureblock callsupdateNetif()in mock mode. The mockifconfiglistswlan0, soprocessIfconfigOutputschedules a detachedsetTimeout(wifiUpdateDevices, 1000). That timer fires ~1s later — inside afollowing mock-mode suite — and registers the mock adapter
dc:a6:32:12:34:57(id 0) into the process-wide
wifiInterfacesByMacAddressregistry. Inwifi.procedure.test.ts,macForDeviceId("0")then resolves to that leakedadapter instead of the test's own seeded interface, so the held device lock is
missed and the handler returns success instead of
DEVICE_BUSY.How
mock-feedback-broadcast.test.ts: snapshotMOCK_MODEonce and restore it inboth teardowns — the same save/restore idiom already used by PR fix(test): restore MOCK_MODE so mock suites do not leak dev mode across the bun suite #153 for the
mmcli-mode-validation/modem-migrationleaks.rpc-network.procedure.test.ts: in thenetwork.configureafterAll, drainthe pending
wifiUpdateDevicestimer (while mocks are still on) and evict theregistered adapter before restoring the environment, so nothing leaks past the
file.
No production code changed. No test weakened, skipped, or deleted.
How to verify
Run it a few times — the failures were deterministic per file order, so a green
result across runs is the proof. The 8 kiosk + 1 wifi cases all pass; nothing
else changes.
Risks
Low — test-only. The wifi fix adds a bounded (~1s) drain to one
afterAll; itpolls for the adapter and exits as soon as it appears, then removes it. If the
timer never fires the loop is capped at 2s and the subsequent
removeWifiInterfaceis a harmless no-op.