test: fix flaky fdv2_rapidStateChangesCoalesceIntoOneRebuild#365
Open
abelonogov-ld wants to merge 2 commits into
Open
test: fix flaky fdv2_rapidStateChangesCoalesceIntoOneRebuild#365abelonogov-ld wants to merge 2 commits into
abelonogov-ld wants to merge 2 commits into
Conversation
This ConnectivityManager test intermittently failed CI with "timed out waiting for stopping of data source" (ConnectivityManagerTest.java:916). Two latent issues combined to make it flaky: 1. Strict-mock call ordering. After resetAll(), the test records setOffline(any).anyTimes() then setInBackground(any).anyTimes() on a STRICT eventProcessor mock. Each updateEventProcessor() call repeats the (setOffline, setInBackground) pair, but a strict mock enforces call order, so the second change's setOffline is rejected as "unexpected" and throws on the listener thread — aborting it before setNetworkAvailable() runs. Fixed by disabling order checking for this phase via checkOrder(false); anyTimes() only relaxes the count, not the order. 2. Notification reordering. setAndNotifyConnectivityChangeListeners spawns a new thread per call, so three rapid changes (false, true, false) could be delivered out of order. If true (the baseline) is applied last, the debouncer's A->B->C->A dedup suppresses the rebuild and no data source is ever stopped. Added MockPlatformState.setAndNotifyConnectivityChangeListenersInSequence to deliver a burst deterministically on one thread, ending on false. Verified deterministic across repeated runs of the test, the full ConnectivityManagerTest class, and the full unit test suite. Co-authored-by: Cursor <cursoragent@cursor.com>
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.
Summary
ConnectivityManagerTest.fdv2_rapidStateChangesCoalesceIntoOneRebuildintermittently failed CI (e.g. theBuild and Testrun onmain) with:Two latent issues combined to make it flaky:
1. Strict-mock call ordering. After
resetAll(), the test recordssetOffline(any).anyTimes()thensetInBackground(any).anyTimes()on aMockType.STRICTeventProcessormock. EachupdateEventProcessor()call repeats the(setOffline, setInBackground)pair, but a strict mock enforces call order — so the second connectivity change'ssetOfflineis rejected as an unexpected call and throws on the listener thread, aborting it beforesetNetworkAvailable()runs. The capturedsystem-errshowed exactly this:anyTimes()only relaxes the count, not the order. Fixed by disabling order checking for this phase withcheckOrder(eventProcessor, false).2. Notification reordering.
MockPlatformState.setAndNotifyConnectivityChangeListenersspawns a new thread per call, so three rapid changes (false,true,false) can be delivered out of order. Iftrue(the baseline) lands last, the debouncer's A→B→C→A dedup suppresses the rebuild and no data source is ever stopped. AddedsetAndNotifyConnectivityChangeListenersInSequence(...)which delivers a burst deterministically on a single thread, ending onfalse, while preserving the "rapid changes" intent.The racing version passed locally only by luck; both issues are real races independent of CI load.
Test plan
fdv2_rapidStateChangesCoalesceIntoOneRebuildpasses across repeated--rerun-tasksruns (6/6)ConnectivityManagerTestclass passes across repeated runs (3/3):launchdarkly-android-client-sdk:testDebugUnitTestsuite passes (BUILD SUCCESSFUL)MockPlatformStatehelper is additive (other call sites unchanged)Made with Cursor
Note
Low Risk
Test and shared test-helper changes only; production SDK behavior is unchanged.
Overview
Stabilizes
fdv2_rapidStateChangesCoalesceIntoOneRebuildby fixing two test-only races that caused intermittent CI timeouts waiting for a data source stop.In
ConnectivityManagerTest, afterresetAll()the phase that expects repeatedEventProcessorupdates now callscheckOrder(eventProcessor, false)so EasyMock strict ordering no longer rejects interleavedsetOffline/setInBackgroundpairs when multiple connectivity updates run in parallel.In
MockPlatformState, addssetAndNotifyConnectivityChangeListenersInSequence(boolean...)to fire a burst of connectivity notifications in order on one thread. The debounce test switches from three separatesetAndNotifyConnectivityChangeListenerscalls to this helper so the sequence ends onfalsedeterministically instead of sometimes finishing ontrueand suppressing the offline rebuild.Reviewed by Cursor Bugbot for commit 5e9e9ad. Bugbot is set up for automated code reviews on this repo. Configure here.