fix(test): make pairing + network-ingest suites order-independent (config.json + network-ingest cache leaks)#154
Merged
Conversation
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.
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
Follow-up to #153. After that PR fixed the
MOCK_MODEleak (6 of the original 21 CI failures), 15 tests still failed deterministically in theBE unit (bun) + exec-guard + biomejob. This PR fixes the two remaining, independent cross-file test-isolation leaks:apps/backend/src/tests/pairing-platform-claim.test.ts— readconfig.jsonunconditionally inbeforeEach.apps/backend/src/tests/network-ingest-control.test.ts— wrote the module-level network-ingest cache via the real RPC but never reset it in teardown.13 insertions, 4 deletions, no production code.
Why
bun testruns every file in one shared process, so any global state a file leaves dirty is inherited by whatever file runs next — and the full-suite file order differs between a fresh CI runner and a local box, so both leaks reproduce deterministically in CI but not always locally.Leak 1 — config.json (14 failures).
config.jsonis gitignored, so it is absent on a fresh CI checkout.pairing-platform-claim.test.ts'sbeforeEachdidreadFileSync("config.json")unconditionally; when absent that throwsENOENT, failing every test in the file — including the pureplatformClaimErrorForStatusandgetPlatformUrlcases, which is exactly the 14-failure set. The cross-file trigger isremote-repair-migration.test.ts, which correctly deletes theconfig.jsonit created when the file was absent at its start (restoring prior state) — its sibling comment even notes "Reading it unconditionally throws on a fresh CI checkout." The bug is the unconditional read, not the cleanup. Fix: snapshot only whenexistsSync, and inafterEachremove it when it was absent at start, else restore the bytes.Leak 2 — network-ingest cache (1 C7 failure).
source-lost-rejection.test.ts's "available:false network source (gateway down) → source_unavailable" reads the module-levelcachedsnapshot innetwork-ingest.ts.network-ingest-control.test.ts's "mocks" block drives the realnetwork.setIngestEnabledRPC (which writes that cache) but never calledresetNetworkIngestState()in teardown — the only one of the five cache-writers that didn't. Its last test leaves rtmp enabled+active, so a later file sees the rtmp source asavailable:true, and the C7 start returnsnetwork_ingest_gateway_inactiveinstead ofsource_unavailable.streaming-gateway-gate.test.tsalready documents this exact "stale ambient cache leaves service_active:true … under CI test ordering" hazard. Fix: reset the cache in the mocks block'safterEach+afterAll.How to verify
pairing-platform-claim.test.tsalone withconfig.jsonremoved: 14 pass / 0 fail (was 0/14).remote-repair-migration→pairing-platform-claimwith config.json removed: 30 pass / 0 fail.network-ingest-control→source-lost-rejection(C7): 27 pass / 0 fail; a cache probe after the fix reportsrtmp=null(wasservice_active:true).config.jsonpresent and absent: 1910 pass / 0 fail both ways.bun tsc --noEmit,bunx biome check: clean. The only other full-suite fails are the pre-existingqw-e-audio-hotplug.test.tsfs.watchflakes (fail identically on the pristine tree; out of scope).Risks
Minimal — test-only changes that restore/reset global state the suites should never have leaked. No production code, no test skipped or weakened (Rule E). With this merged, the four previously-blocked PRs should have a fully green
BE unitjob.