Skip to content

[SDK-406] Sdk fix flaky tests#1007

Open
franco-zalamena-iterable wants to merge 18 commits intomasterfrom
SDK-fix-flaky-tests
Open

[SDK-406] Sdk fix flaky tests#1007
franco-zalamena-iterable wants to merge 18 commits intomasterfrom
SDK-fix-flaky-tests

Conversation

@franco-zalamena-iterable
Copy link
Copy Markdown
Contributor

@franco-zalamena-iterable franco-zalamena-iterable commented Mar 24, 2026 •
edited by atlassian bot
Loading

🔹 Jira Ticket(s) if any

✏️ Description

Removing timeouts in some tests that can be causing flakiness. Moved some unit tests that were running as instrumental tests, added better description into ignored tests

franco-zalamena-iterable and others added 13 commits April 1, 2026 17:35
@franco-zalamena-iterable @claude
Move 13 criteria tests from androidTest to unit tests
46e736d 
These tests use @RunWith(JUnit4) with only org.json imports and have
no Android framework dependencies. Running them as JVM unit tests
removes the emulator requirement and speeds up CI.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@franco-zalamena-iterable @claude
Replace non-deterministic test synchronization with looper control
2426fb7 
- Replace Mockito.timeout() with shadowOf(getMainLooper()).idle() +
  verify() in IterablePushRegistrationTaskTest and IterableInAppManagerTest
- Replace Thread.sleep(1000) with shadowOf(getMainLooper()).idle() in
  IterableNotificationTest

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@franco-zalamena-iterable @claude
Document root causes in @ignore annotations
69acb4a 
Update 23 @ignore annotations with actionable descriptions:
- JWT tests: blocked on IterableAuthManager.executor not being injectable
- Universal link test: needs MockWebServer to stub HTTP redirect
- Database logout test: IterableTaskStorage singleton state leakage
- In-app stalling tests: Robolectric incompatible, need Espresso

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@franco-zalamena-iterable
Fix for deprecated test sleep
0a823b6
@franco-zalamena-iterable @claude
Fix flaky testSyncOnLogin and testTrackPushOpenWithCustomAction
e3c36f0 
testSyncOnLogin: initialize() can trigger syncInApp via background init,
causing a double call. Reset the mock after initialize so the verify
only checks the setEmail-triggered sync.

testTrackPushOpenWithCustomAction: cross-test state leakage causes a
spurious inApp/getMessages request that arrives at the mock server before
trackPushOpen. Skip non-trackPushOpen requests when verifying.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@franco-zalamena-iterable @claude
Fix flaky testCriteriaMetEmailMergeTrue in IterableApiMergeUserEmailT…
9abed24 
…ests

Cross-test state leakage causes spurious inApp/getMessages requests to
arrive at the mock server before the expected requests. Added a helper
takeRequestWithPath() that skips unexpected in-app sync requests when
verifying request order.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@franco-zalamena-iterable @claude
Fix flaky push registration and InAppManager read state tests
a218b09 
testSetEmailWithAutomaticPushRegistration / testSetUserIdWithAutomaticPushRegistration:
initialize() can trigger executePushRegistrationTask via background init.
Reset the mock after initialize so verify only captures the setEmail/setUserId call.

testMessagePersistentReadStateFromServer: Flush pending looper callbacks
from setUp's createIterableApiNew before the test starts. Stale
onPostExecute callbacks can corrupt InlineExecutorService state under
CI resource pressure.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@franco-zalamena-iterable @claude
Fix flaky InAppManager tests: listener, action link, and custom actio…
d97ce0c 
…n delete

The createIterableApiNew() call triggers syncInApp() via setEmail(), which
consumes the single enqueued MockWebServer response meant for the test.
Fix by enqueuing a dummy response for the init-triggered sync, plus flushing
the looper before test assertions.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@franco-zalamena-iterable @claude
Fix flaky InAppManager tests by draining setUp HTTP requests
93cdb30 
setUp's createIterableApiNew() fires async HTTP requests (InAppManager
constructor sync + setEmail sync) on background threads. On CI, these
can arrive at MockWebServer after a test enqueues its responses,
consuming them. Fix by:

1. Draining all setUp HTTP requests via server.takeRequest() in setUp
2. Flushing their looper callbacks with idle()
3. Adding idle() to testJsonOnlyInAppMessageDelegateCallbacks to flush
   the constructor sync before foreground transition

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@franco-zalamena-iterable @claude
Fix flaky PII masking tests with deterministic init state
567e738 
The tests raced initializeInBackground() against setEmail/setUserId —
if background init completed before the API calls, operations executed
immediately instead of being queued, failing the size assertion.

Fix: use simulateInitializingState() to hold init in progress, then
simulateInitializationComplete() after assertions.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@franco-zalamena-iterable @claude
Fix flaky push registration test and increase async init timeouts
a217a65 
- testSetEmailWithAutomaticPushRegistration: flush looper before resetting
  the mock so background init callbacks don't trigger extra push registration
- IterableAsyncInitializationTest: increase all waitForAsyncInitialization
  timeouts from 3s to 5s for CI reliability

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@franco-zalamena-iterable @claude
Fix executor shutdown race in PII masking tests
933171f 
Remove simulateInitializationComplete() from PII masking tests that
only verify queue contents. The call triggers processAll() ->
shutdownBackgroundExecutorAsync() which can race with the next test's
executor setup, causing testInitializeInBackground_WithConfig to fail.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@franco-zalamena-iterable @claude
Fix executor shutdown race between tests
86f560a 
shutdownBackgroundExecutorAsync() now captures the executor reference
before spawning the shutdown thread, so it only shuts down the intended
executor — not a replacement created by resetBackgroundInitializationState().

resetBackgroundInitializationState() now always creates a fresh executor
instead of conditionally reusing one that may have a pending async shutdown.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@franco-zalamena-iterable franco-zalamena-iterable changed the title Sdk fix flaky tests [SDK-406] Sdk fix flaky tests Apr 1, 2026
sumeruchat
sumeruchat requested changes Apr 6, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@sumeruchat sumeruchat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@franco-zalamena-iterable Can you please explain the changes in background executor.

@franco-zalamena-iterable
Copy link
Copy Markdown
Contributor Author

franco-zalamena-iterable commented Apr 7, 2026

The changes on the Background executor were made to prevent the main cause of flakiness on our tests. We were having a synchronized read and write with a lock, so even if it was a different executor it was waiting for the other test to finish to be able to shut it down, meanwhile the other test was doing the same thing. We now have a synchronized read but no need to wait for the other test to be able to shut it down. this prevents the deadlock we were seeing before

@sumeruchat
Copy link
Copy Markdown
Contributor

sumeruchat commented Apr 8, 2026

The suggested fix is to stop shutdownBackgroundExecutorAsync() from looking up the global backgroundExecutor at shutdown time.

Right now it does this in iterableapi/src/main/java/com/iterable/iterableapi/IterableBackgroundInitializer.java:340:

  • grab the current static backgroundExecutor
  • spawn a thread
  • shut that executor down later

That is the race. If test reset swaps in a new executor before a straggler calls this helper, the helper can shut down the new executor instead of the old
one.

The concrete change is:

  1. Change shutdownBackgroundExecutorAsync() to accept an ExecutorService argument.
  2. Pass the executor instance that actually ran the work.
  3. In reset, keep a local reference to the old executor, install the new one, then shut down only the old reference.

@franco-zalamena-iterable
Passing ExecutorService as parameter instead of lock read
8ebbb16
@franco-zalamena-iterable
Drains the initialize requests before tests
9a74970
@franco-zalamena-iterable
Fix processAll executor race and restore PII test lifecycle
1192074 
- Fix race condition in OperationQueue.processAll() where
  shutdownBackgroundExecutorAsync read the static backgroundExecutor
  field instead of using the captured executor parameter. If
  resetBackgroundInitializationState() ran between task submission
  and shutdown, it would shut down the newly created executor.
- Restore simulateInitializationComplete() calls in PII masking tests
  so queued operations are properly processed, matching production
  behavior instead of relying solely on tearDown to reset state.

Made-with: Cursor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@joaodordio joaodordio Awaiting requested review from joaodordio

@sumeruchat sumeruchat Awaiting requested review from sumeruchat

Requested changes must be addressed to merge this pull request.

Assignees

@franco-zalamena-iterable franco-zalamena-iterable

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@franco-zalamena-iterable @sumeruchat @joaodordio