Skip to content

test: [SDK-5340] cover FID exception-to-status mapping - #2763

Merged
fadi-george merged 2 commits into
firebase-installation-idsfrom
fadi/sdk-5340
Sep 23, 2026
Merged

fadi-george merged 2 commits into
firebase-installation-idsfrom
fadi/sdk-5340

Conversation

@fadi-george

Copy link
Copy Markdown
Contributor

Description

One Line Summary

Add unit tests proving the FID registration path keeps the existing IOException to subscription status mapping.

Details

Motivation

Replaces an unreliable manual bug-bash case with direct unit coverage. When Firebase Installation ID registration fails with an IOException, PushRegistratorAbstractGoogle should still map it to -9, -29, or -11 and not collapse it to -34.

Scope

Tests only, no production behavior change.

  • New PushRegistratorFCMTests cases drive registerForPush() with a failing FID register() task:
    • IOException("SERVICE_NOT_AVAILABLE") maps to -9 after 3 attempts
    • IOException("AUTHENTICATION_FAILED") maps to -29 after 3 attempts
    • Any other IOException maps to -11 after 1 attempt
    • A non-IOException maps to -34
  • An IOException from the installation ID lookup maps to -11, not -34.
  • The tests use runTest so the retry backoff delays run in virtual time, and add kotlinx-coroutines-test to the notifications test dependencies (same version as core).

Testing

Unit testing

  • ./gradlew :OneSignal:notifications:testReleaseUnitTest, spotlessCheck, and detekt all pass.
  • Confirmed the new tests fail with -34 when the IOException rethrow in FCMTokenProvider.retrieveInstallationId is removed.

Manual testing

Not needed, tests only.

Affected code checklist

  • Notifications
    • Display
    • Open
    • Push Processing
    • Confirm Deliveries
  • Outcomes
  • Sessions
  • In-App Messaging
  • REST API requests
  • Public API changes

Checklist

Overview

  • I have filled out all REQUIRED sections above
  • PR does one thing
  • Any Public API changes are explained in the PR details and conform to existing APIs

Testing

  • I have included test coverage for these changes, or explained why they are not needed
  • All automated tests pass, or I explained why that is not possible
  • I have personally tested this on my device, or explained why that is not possible

Final pass

  • Code is as readable as possible.
  • I have reviewed this PR myself, ensuring it meets each checklist item

fadi-george and others added 2 commits September 23, 2026 12:04
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@fadi-george
fadi-george requested a review from a team as a code owner September 23, 2026 19:10
@github-actions

Copy link
Copy Markdown
Contributor

📊 Diff Coverage Report

Diff Coverage Report (Changed Lines Only)

Gate: aggregate coverage on changed executable lines must be ≥ 80% (JaCoCo line data for lines touched in the diff).

Changed Files Coverage

✅ No main-source changes to gate

No Kotlin/Java changes in the diff.

📥 View workflow run

@fadi-george
fadi-george merged commit ae5a60c into firebase-installation-ids Sep 23, 2026
6 checks passed
@fadi-george
fadi-george deleted the fadi/sdk-5340 branch September 23, 2026 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants