Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dataconnect: TestFirebaseAppFactory.kt: work around IllegalStateException in tests by adding a delay before calling FirebaseApp.delete() #6447

Conversation

dconeybe
Copy link
Contributor

@dconeybe dconeybe commented Nov 8, 2024

This PR fixes a race condition in the Data Connect tests where if a FirebaseApp is deleted very shortly after creating it then an IllegalStateException can be thrown, crashing the test app entirely, resulting in a flaky test.

The "fix" is really a "workaround", in that it waits for 1 second before calling FirebaseApp.delete() to minimize the chances that the FirebaseAuth worker thread will try to access the deleted FirebaseApp instance. Googlers see b/378116261 for details.

The crash fixed by this PR looks like this:

FATAL EXCEPTION: TokenRefresher
Process: com.google.firebase.dataconnect.connectors.test, PID: 7058
java.lang.IllegalStateException: FirebaseApp with name test-app-nwdpj3757g doesn't exist. Available app names: [DEFAULT]
  at com.google.firebase.FirebaseApp.getInstance(FirebaseApp.java:212)
  at com.google.firebase.auth.internal.zzat.run(com.google.firebase:firebase-auth@@22.3.1:4)                                
  at android.os.Handler.handleCallback(Handler.java:958)                                                         
  at android.os.Handler.dispatchMessage(Handler.java:99)                                         
  at android.os.Looper.loopOnce(Looper.java:205)                                                                             
  at android.os.Looper.loop(Looper.java:294)                                                                        
  at android.os.HandlerThread.run(HandlerThread.java:67)    

… by adding a delay before calling FirebaseApp.delete()
Copy link
Contributor

github-actions bot commented Nov 8, 2024

📝 PRs merging into main branch

Our main branch should always be in a releasable state. If you are working on a larger change, or if you don't want this change to see the light of the day just yet, consider using a feature branch first, and only merge into the main branch when the code complete and ready to be released.

1 similar comment
Copy link
Contributor

github-actions bot commented Nov 8, 2024

📝 PRs merging into main branch

Our main branch should always be in a releasable state. If you are working on a larger change, or if you don't want this change to see the light of the day just yet, consider using a feature branch first, and only merge into the main branch when the code complete and ready to be released.

Copy link
Contributor

github-actions bot commented Nov 8, 2024

Vertex AI Mock Responses Check ⚠️

A newer major version of the mock responses for Vertex AI unit tests is available. update_responses.sh should be updated to clone the latest version of the responses: v5.2

@dconeybe dconeybe changed the title TestFirebaseAppFactory.kt: work around IllegalStateException in tests by adding a delay before calling FirebaseApp.delete() dataconnect: TestFirebaseAppFactory.kt: work around IllegalStateException in tests by adding a delay before calling FirebaseApp.delete() Nov 8, 2024
Copy link
Contributor

github-actions bot commented Nov 8, 2024

Test Results

   54 files  ±0     54 suites  ±0   2m 12s ⏱️ -2s
  515 tests  - 2    514 ✅  - 2  1 💤 ±0  0 ❌ ±0 
1 030 runs   - 4  1 028 ✅  - 4  2 💤 ±0  0 ❌ ±0 

Results for commit d1ba01b. ± Comparison against base commit 5f6bc63.

This pull request removes 2 tests.
com.google.firebase.dataconnect.core.DataConnectAuthUnitTest ‑ addIdTokenListener() throwing IllegalStateException due to FirebaseApp deleted should be ignored
com.google.firebase.dataconnect.core.DataConnectAuthUnitTest ‑ removeIdTokenListener() throwing IllegalStateException due to FirebaseApp deleted should be ignored

@dconeybe dconeybe merged commit f20340a into main Nov 8, 2024
41 checks passed
@dconeybe dconeybe deleted the dconeybe/dataconnect/FirebaseAppDeleteIllegalStateExceptionFix branch November 8, 2024 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants