chore: improve testing with injected coroutine dispatchers #2524

jinliu9508 · 2026-01-20T16:22:59Z

Description

One Line Summary

Inject coroutine dispatcher provider into startup/async services to improve test determinism and reduce flaky coroutine timing issues.

Details

Motivation

Several SDK components launch background work using global/default dispatchers, which makes unit/integration tests timing-dependent and occasionally flaky (especially under CI load). This PR introduces dependency injection for coroutine dispatchers (via a CoroutineDispatcherProvider with a default implementation), allowing tests to supply a controlled dispatcher (e.g., unconfined/test dispatcher) while keeping production behavior unchanged. The goal is to make startup and async flows deterministic, easier to test, and less prone to race conditions.

Scope

Services/classes that previously used global dispatchers (e.g., StartupService) now accept an injected CoroutineDispatcherProvider (defaulting to DefaultDispatcherProvider()), and use the injected dispatchers for coroutine launches.

OPTIONAL - Other

This pattern centralizes dispatcher usage, makes it easier to add structured concurrency in the future, and encourages consistent coroutine threading across modules. It also reduces the need for sleeps/polling in tests by allowing controlled scheduling.

Testing

Unit testing

Updated existing tests to inject a test dispatcher provider where startup/initialization work is involved.
Verified async startup flows complete deterministically without relying on timing-based polling or delays.

Manual testing

Not needed

Affected code checklist

Checklist

Overview

I have filled out all REQUIRED sections above
PR does one thing
- If it is hard to explain how any codes changes are related to each other then it most likely needs to be more than one PR
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.
- Simplify with less code, followed by splitting up code into well named functions and variables, followed by adding comments to the code.
I have reviewed this PR myself, ensuring it meets each checklist item
- WIP (Work In Progress) is ok, but explain what is still in progress and what you would like feedback on. Start the PR title with "WIP" to indicate this.

This change is

Copilot

Pull request overview

This PR introduces dependency injection for coroutine dispatchers to improve test determinism and reduce flaky tests. A new CoroutineDispatcherProvider interface with DefaultDispatcherProvider (production) and TestDispatcherProvider (testing) implementations allows controlled execution of coroutines in tests.

Changes:

Added CoroutineDispatcherProvider interface and implementations for production (DefaultDispatcherProvider) and testing (TestDispatcherProvider)
Injected dispatcher provider into OneSignalImp, StartupService, NotificationRepository, and OutcomeEventsRepository with default values for backward compatibility
Updated tests to use TestDispatcherProvider for deterministic coroutine execution

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
CoroutineDispatcherProvider.kt	Interface defining dispatcher provider contract with IO and Default dispatchers plus launch methods
DefaultDispatcherProvider.kt	Production implementation delegating to existing OneSignalDispatchers
TestDispatcherProvider.kt	Test implementation using StandardTestDispatcher with comprehensive usage documentation
StartupService.kt	Accepts injected dispatcher provider, uses it for launching startable services
OneSignalImp.kt	Accepts injected dispatcher provider, uses it for all withContext and runBlocking calls
OutcomeEventsRepository.kt	Accepts injected dispatcher provider, uses it for all database operations
NotificationRepository.kt	Accepts injected dispatcher provider, uses it for all database operations
StartupServiceTests.kt	Updated to use TestDispatcherProvider with proper runTest wrapper and advanceUntilIdle
OneSignalImpTests.kt	Updated to create TestDispatcherProvider instances (tests work without runTest as they test non-suspend paths)
OutcomeEventsRepositoryTests.kt	Updated to create TestDispatcherProvider instances but missing critical runTest wrappers

Comments suppressed due to low confidence (2)

OneSignalSDK/onesignal/core/src/test/java/com/onesignal/session/internal/outcomes/OutcomeEventsRepositoryTests.kt:52

Tests using TestDispatcherProvider with StandardTestDispatcher must be wrapped in runTest() to work correctly. Without runTest(), StandardTestDispatcher won't execute any coroutines, and the test assertions will run before the repository methods complete. The test should follow the pattern shown in TestDispatcherProvider's documentation. Wrap the test body in runTest(testDispatcher.scheduler) and call advanceUntilIdle() before assertions.

    test("delete outcome event should use the timestamp to delete row from database") {
        // Given
        val testDispatcher = StandardTestDispatcher()
        val dispatcherProvider = TestDispatcherProvider(testDispatcher)
        val mockDatabasePair = DatabaseMockHelper.databaseProvider(OutcomeEventsTable.TABLE_NAME)
        val outcomeEventsRepository = OutcomeEventsRepository(mockDatabasePair.first, dispatcherProvider)

        // When
        outcomeEventsRepository.deleteOldOutcomeEvent(OutcomeEventParams("outcomeId", null, 0f, 0, 1111))

        // Then
        verify(exactly = 1) {
            mockDatabasePair.second.delete(
                OutcomeEventsTable.TABLE_NAME,
                withArg {
                    it.contains(OutcomeEventsTable.COLUMN_NAME_TIMESTAMP)
                },
                withArg { it.contains("1111") },
            )
        }
    }

OneSignalSDK/onesignal/core/src/test/java/com/onesignal/session/internal/outcomes/OutcomeEventsRepositoryTests.kt:155

Tests using TestDispatcherProvider with StandardTestDispatcher must be wrapped in runTest() to work correctly. Without runTest(), StandardTestDispatcher won't execute any coroutines, and the test assertions will run before the repository methods complete. The test should follow the pattern shown in TestDispatcherProvider's documentation. Wrap the test body in runTest(testDispatcher.scheduler) and call advanceUntilIdle() before assertions.

    test("save outcome event should insert row into database") {
        // Given
        val mockDatabasePair = DatabaseMockHelper.databaseProvider(OutcomeEventsTable.TABLE_NAME)
        val testDispatcher = StandardTestDispatcher()
        val dispatcherProvider = TestDispatcherProvider(testDispatcher)
        val outcomeEventsRepository = OutcomeEventsRepository(mockDatabasePair.first, dispatcherProvider)

        // When
        outcomeEventsRepository.saveOutcomeEvent(OutcomeEventParams("outcomeId1", null, 0f, 0, 1111))
        outcomeEventsRepository.saveOutcomeEvent(
            OutcomeEventParams(
                "outcomeId2",
                OutcomeSource(
                    OutcomeSourceBody(JSONArray().put("notificationId1")),
                    OutcomeSourceBody(null, JSONArray().put("iamId1").put("iamId2")),
                ),
                .2f,
                0,
                2222,
            ),
        )
        outcomeEventsRepository.saveOutcomeEvent(
            OutcomeEventParams(
                "outcomeId3",
                OutcomeSource(
                    OutcomeSourceBody(JSONArray().put("notificationId1"), JSONArray().put("iamId1")),
                    null,
                ),
                .4f,
                0,
                3333,
            ),
        )
        outcomeEventsRepository.saveOutcomeEvent(
            OutcomeEventParams(
                "outcomeId4",
                OutcomeSource(
                    null,
                    OutcomeSourceBody(JSONArray().put("notificationId1"), JSONArray().put("iamId1").put("iamId2")),
                ),
                .6f,
                0,
                4444,
            ),
        )

        // Then
        verifySequence {
            mockDatabasePair.second.insert(
                OutcomeEventsTable.TABLE_NAME,
                null,
                withArg {
                    it[OutcomeEventsTable.COLUMN_NAME_NAME] shouldBe "outcomeId1"
                    it[OutcomeEventsTable.COLUMN_NAME_WEIGHT] shouldBe 0f
                    it[OutcomeEventsTable.COLUMN_NAME_TIMESTAMP] shouldBe 1111L
                    it[OutcomeEventsTable.COLUMN_NAME_NOTIFICATION_INFLUENCE_TYPE] shouldBe "unattributed"
                    it[OutcomeEventsTable.COLUMN_NAME_NOTIFICATION_IDS] shouldBe JSONArray().toString()
                    it[OutcomeEventsTable.COLUMN_NAME_IAM_INFLUENCE_TYPE] shouldBe "unattributed"
                    it[OutcomeEventsTable.COLUMN_NAME_IAM_IDS] shouldBe JSONArray().toString()
                },
            )
            mockDatabasePair.second.insert(
                OutcomeEventsTable.TABLE_NAME,
                null,
                withArg {
                    it[OutcomeEventsTable.COLUMN_NAME_NAME] shouldBe "outcomeId2"
                    it[OutcomeEventsTable.COLUMN_NAME_WEIGHT] shouldBe .2f
                    it[OutcomeEventsTable.COLUMN_NAME_TIMESTAMP] shouldBe 2222L
                    it[OutcomeEventsTable.COLUMN_NAME_NOTIFICATION_INFLUENCE_TYPE] shouldBe "direct"
                    it[OutcomeEventsTable.COLUMN_NAME_NOTIFICATION_IDS] shouldBe JSONArray("[\"notificationId1\"]").toString()
                    it[OutcomeEventsTable.COLUMN_NAME_IAM_INFLUENCE_TYPE] shouldBe "indirect"
                    it[OutcomeEventsTable.COLUMN_NAME_IAM_IDS] shouldBe JSONArray("[\"iamId1\", \"iamId2\"]").toString()
                },
            )
            mockDatabasePair.second.insert(
                OutcomeEventsTable.TABLE_NAME,
                null,
                withArg {
                    it[OutcomeEventsTable.COLUMN_NAME_NAME] shouldBe "outcomeId3"
                    it[OutcomeEventsTable.COLUMN_NAME_WEIGHT] shouldBe .4f
                    it[OutcomeEventsTable.COLUMN_NAME_TIMESTAMP] shouldBe 3333L
                    it[OutcomeEventsTable.COLUMN_NAME_NOTIFICATION_INFLUENCE_TYPE] shouldBe "direct"
                    it[OutcomeEventsTable.COLUMN_NAME_NOTIFICATION_IDS] shouldBe JSONArray("[\"notificationId1\"]").toString()
                    it[OutcomeEventsTable.COLUMN_NAME_IAM_INFLUENCE_TYPE] shouldBe "direct"
                    it[OutcomeEventsTable.COLUMN_NAME_IAM_IDS] shouldBe JSONArray("[\"iamId1\"]").toString()
                },
            )
            mockDatabasePair.second.insert(
                OutcomeEventsTable.TABLE_NAME,
                null,
                withArg {
                    it[OutcomeEventsTable.COLUMN_NAME_NAME] shouldBe "outcomeId4"
                    it[OutcomeEventsTable.COLUMN_NAME_WEIGHT] shouldBe .6f
                    it[OutcomeEventsTable.COLUMN_NAME_TIMESTAMP] shouldBe 4444L
                    it[OutcomeEventsTable.COLUMN_NAME_NOTIFICATION_INFLUENCE_TYPE] shouldBe "indirect"
                    it[OutcomeEventsTable.COLUMN_NAME_NOTIFICATION_IDS] shouldBe JSONArray("[\"notificationId1\"]").toString()
                    it[OutcomeEventsTable.COLUMN_NAME_IAM_INFLUENCE_TYPE] shouldBe "indirect"
                    it[OutcomeEventsTable.COLUMN_NAME_IAM_IDS] shouldBe JSONArray("[\"iamId1\", \"iamId2\"]").toString()
                },
            )
        }
    }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

...l/core/src/test/java/com/onesignal/session/internal/outcomes/OutcomeEventsRepositoryTests.kt

github-actions · 2026-01-23T16:31:26Z

📊 Diff Coverage Report

Diff Coverage Report

Threshold: 80%

Changed Files Coverage

❌ DefaultDispatcherProvider.kt: 0/5 lines (0.0%)
- ⚠️ Below threshold: 5 uncovered lines
✅ StartupService.kt: 9/10 lines (90.0%)
❌ OutcomeEventsRepository.kt: 0/208 lines (0.0%)
- ⚠️ Below threshold: 208 uncovered lines
❌ NotificationRepository.kt: 0/346 lines (0.0%)
- ⚠️ Below threshold: 346 uncovered lines
⚠️ TestDispatcherProvider.kt: Not in coverage report (may not be compiled/tested)

Overall Coverage

9/569 lines covered (1.6%)

❌ Coverage Check Failed

Files below 80% threshold:

DefaultDispatcherProvider.kt: 0.0% (5 uncovered lines)
OutcomeEventsRepository.kt: 0.0% (208 uncovered lines)
NotificationRepository.kt: 0.0% (346 uncovered lines)

📥 View workflow run

abdulraqeeb33

looks good to me

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-23T16:39:30Z

OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpTests.kt

-        val os = OneSignalImp()
+        runTest(testDispatcher.scheduler) {
+            // Given
+            val os = OneSignalImp()


This test case creates OneSignalImp() without passing the test dispatcher, while all other test cases in this file consistently pass dispatcherProvider.io. For consistency and proper test isolation, this should also use the test dispatcher: OneSignalImp(dispatcherProvider.io).

Copilot · 2026-01-23T16:39:31Z

...l/core/src/test/java/com/onesignal/session/internal/outcomes/OutcomeEventsRepositoryTests.kt

+        }
    }

    test("save unique outcome should insert 1 row for each unique influence when direct notification and indiract iam") {


Typo in test name: "indiract" should be "indirect".

Suggested change

test("save unique outcome should insert 1 row for each unique influence when direct notification and indiract iam") {

test("save unique outcome should insert 1 row for each unique influence when direct notification and indirect iam") {

Copilot · 2026-01-23T16:39:31Z

...l/core/src/test/java/com/onesignal/session/internal/outcomes/OutcomeEventsRepositoryTests.kt

        }
    }

    test("save unique outcome should insert 1 row for each unique influence when direct iam and indiract notifications") {


Typo in test name: "indiract" should be "indirect".

Copilot · 2026-01-23T16:39:31Z

...tions/src/main/java/com/onesignal/notifications/internal/data/impl/NotificationRepository.kt

    private val _databaseProvider: IDatabaseProvider,
    private val _time: ITime,
    private val _badgeCountUpdater: IBadgeCountUpdater,
+    private val dispatchers: CoroutineDispatcherProvider = DefaultDispatcherProvider(),


The NotificationRepository accepts a CoroutineDispatcherProvider but only uses the io dispatcher (dispatchers.io). For consistency with OutcomeEventsRepository and to avoid over-injection, consider changing the parameter type to CoroutineDispatcher and accepting only the IO dispatcher directly. This would make the API clearer about what's actually needed.

Copilot · 2026-01-23T16:39:32Z

OneSignalSDK/onesignal/core/src/test/java/com/onesignal/internal/OneSignalImpTests.kt

-        val os = OneSignalImp()
+        runTest(testDispatcher.scheduler) {
+            // Given
+            val os = OneSignalImp()


These test cases create OneSignalImp() without passing the test dispatcher, while all other test cases in this file consistently pass dispatcherProvider.io. For consistency and proper test isolation, these should also use the test dispatcher: OneSignalImp(dispatcherProvider.io).

Standard test pattern

4a1740f

jinliu9508 requested a review from abdulraqeeb33 January 20, 2026 16:22

jinliu9508 assigned abdulraqeeb33 Jan 20, 2026

abdulraqeeb33 requested a review from Copilot January 20, 2026 20:30

Copilot started reviewing on behalf of abdulraqeeb33 January 20, 2026 20:31 View session

Copilot AI reviewed Jan 20, 2026

View reviewed changes

improvement: update more classes with the dispatcher injection pattern

2d6ca0c

jinliu9508 force-pushed the chore/standard-test-pattern branch from dc6b67c to 2d6ca0c Compare January 23, 2026 15:40

abdulraqeeb33 requested a review from Copilot January 23, 2026 16:35

abdulraqeeb33 approved these changes Jan 23, 2026

View reviewed changes

Copilot started reviewing on behalf of abdulraqeeb33 January 23, 2026 16:35 View session

Copilot AI reviewed Jan 23, 2026

View reviewed changes

	test("save unique outcome should insert 1 row for each unique influence when direct notification and indiract iam") {
	test("save unique outcome should insert 1 row for each unique influence when direct notification and indirect iam") {

chore: improve testing with injected coroutine dispatchers #2524

Are you sure you want to change the base?

chore: improve testing with injected coroutine dispatchers #2524

Uh oh!

Conversation

jinliu9508 commented Jan 20, 2026 • edited by niels-s Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

One Line Summary

Details

Motivation

Scope

OPTIONAL - Other

Testing

Unit testing

Manual testing

Affected code checklist

Checklist

Overview

Testing

Final pass

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Jan 23, 2026

📊 Diff Coverage Report

Diff Coverage Report

Changed Files Coverage

Overall Coverage

❌ Coverage Check Failed

Uh oh!

abdulraqeeb33 left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 23, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jinliu9508 commented Jan 20, 2026 •

edited by niels-s

Loading