Fix integration test event races (onReady, onSyncDone) - #159
Merged
Conversation
The method channel reports readiness before the event channel has dispatched onReady, so tests asserting on a listener-backed isReady flag right after InitializeHelper.initialize could race the event. Add an optional isReady getter to InitializeHelper.initialize that polls the caller's flag (within the existing 20s timeout) and asserts it became true, and pass it from every call site.
waitForSync only polled on syncReadyEvent, but callers assert on syncDoneUserId right after it returns. onSyncReady and onSyncDone are dispatched as separate events with no ordering guarantee, so the helper could return while syncDoneUserId was still null, failing assertions such as set_user_with_id_test.dart:282 intermittently on CI. Also bound the wait with a 20s timeout so a lost event ends the test instead of spinning until the suite-level timeout. Paths ending in onSyncError are unaffected: syncDoneUserId legitimately stays null there, and the existing !syncError term still ends the loop.
nicolas-chaix-didomi
changed the title
…eIfNeeded
Removes the duplicated `if (!isReady) { InitializeHelper.initialize(...) }`
block from every integration test in favor of a single helper method, per
review feedback on PR #159.
pmerlet-at-didomi
approved these changes
Aug 5, 2026
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
2 participants
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.
Two related races in the integration test helpers, both causing intermittent CI failures on Android. No production code touched — integration tests only.
1.
onReady—InitializeHelper.initializeInitializeHelper.initializeonly waited onDidomiSdk.isReady(method channel). The event channel dispatchesonReadyslightly later, so tests that assert on their own listener-backedisReadyflag immediately after the helper returns could race the event.InitializeHelper.initializetakes an optionalisReadygetter. After the method-channel check it keeps polling the caller's flag until it flips true (sharing the existing 20s timeout), then asserts it with a clear failure reason.isReady: () => isReadyfrom every call site so the flag is re-read on each poll rather than captured by value.2.
onSyncDone—waitForSyncCI still failed after the above, at
set_user_with_id_test.dart:282(syncDoneUserId == userId). That is a separate race, in a helper the first commit did not touch.waitForSyncpolled only onsyncReadyEvent, but callers assert onsyncDoneUserIdimmediately after it returns.onSyncReadyandonSyncDoneare dispatched as separate events with no ordering guarantee, so the helper could return whilesyncDoneUserIdwas stillnullfrom the precedingresetExpectedSyncValues().set_user_with_id_test.dart,set_user_with_hash_test.dartandset_user_with_encryption_test.dart.syncTimeoutinutil/constants.dart) so a lost event fails the test rather than spinning until the suite-level timeout.assert(syncError == true)are unaffected:syncDoneUserIdlegitimately staysnullthere, and the existing!syncErrorterm still ends the loop.Verification
flutter analyze integration_testis clean. These are timing races needing an emulator, so a single local run would not prove much either way — CI over a couple of runs is the real check.Not addressed
vendor_status_listener_test.dartandinitialize_with_success_test.dartstill callInitializeHelper.initializewithoutisReady:. Both pass today, but they are the remaining files not covered by the first fix.waitForSynccopies remain duplicated across test files. Worth hoisting intoutil/, but that is a wider refactor than this fix.