Skip to content

Populate firebaseInstallationId field in session #4801

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

Merged
merged 1 commit into from
Mar 21, 2023

Conversation

mrober
Copy link
Contributor

@mrober mrober commented Mar 20, 2023

Populate firebaseInstallationId field in session data.
Add SessionCoordinator class and plumb in a CoroutineDispatcher.
Setup coroutines including testing.

val sessionCoordinator =
SessionCoordinator(
firebaseInstallations = FakeFirebaseInstallations("FaKeFiD"),
backgroundDispatcher = StandardTestDispatcher(testScheduler),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

:integ-testing doesn't have any test CoroutineDispatchers to work well with runTesting.

Copy link
Contributor

Choose a reason for hiding this comment

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

How does that impact us?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

:integ-testing is to provide executors for testing that mimic the executors provided by firebase-common with policies e.g. don't do blocking network requests on Background. See https://firebase.github.io/firebase-android-sdk/components/executors/#testing

So these tests don't mimic the Background policy. I tried to use TestOnlyExecutors.background().asCoroutineDispatcher() when I first wrote this test, but then I couldn't call runCurrent() to wait for the suspend functions to finish before doing the assert.

@google-oss-bot
Copy link
Contributor

Coverage Report 1

Affected Products

  • firebase-sessions

    Overall coverage changed from ? (ac3d738) to 31.39% (0d90f8b) by ?.

    FilenameBase (ac3d738)Merge (0d90f8b)Diff
    FirebaseSessions.kt?0.00%?
    FirebaseSessionsRegistrar.kt?0.00%?
    SessionCoordinator.kt?83.33%?
    SessionEvent.kt?100.00%?
    SessionEvents.kt?75.00%?
    SessionGenerator.kt?0.00%?
    SessionInitiator.kt?0.00%?
    WallClock.kt?0.00%?

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/t7Mi68embk.html

@github-actions
Copy link
Contributor

Unit Test Results

10 files  10 suites   17s ⏱️
14 tests 14 ✔️ 0 💤 0
28 runs  28 ✔️ 0 💤 0

Results for commit e104917.

@google-oss-bot
Copy link
Contributor

Size Report 1

Affected Products

  • base

    TypeBase (ac3d738)Merge (0d90f8b)Diff
    apk (aggressive)?8.39 kB? (?)
    apk (release)?8.65 kB? (?)
  • firebase-encoders-json

    TypeBase (ac3d738)Merge (0d90f8b)Diff
    aar?11.3 kB? (?)
    apk (aggressive)?24.0 kB? (?)
    apk (release)?596 kB? (?)
  • firebase-sessions

    TypeBase (ac3d738)Merge (0d90f8b)Diff
    aar?29.2 kB? (?)
    apk (aggressive)?202 kB? (?)
    apk (release)?1.68 MB? (?)

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/HDF9tuPywU.html

Copy link
Contributor

@samedson samedson left a comment

Choose a reason for hiding this comment

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

This looks good!

@mrober mrober merged commit 09d35f6 into firebase-sessions Mar 21, 2023
@mrober mrober deleted the sessions-fid branch March 21, 2023 12:55
@firebase firebase locked and limited conversation to collaborators Apr 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants