Skip to content

Conversation

@abdulraqeeb33
Copy link
Contributor

@abdulraqeeb33 abdulraqeeb33 commented Feb 2, 2026

Description

One Line Summary

Adding unit tests and refactoring existing ones
Also updated the production url

Details

Motivation

We added new functionality and want to make sure we have enough coverage.

Scope

Unit tests only

Testing

Unit testing

Added them

Manual testing

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
    • 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 Reviewable

@abdulraqeeb33 abdulraqeeb33 requested a review from Copilot February 2, 2026 16:29
@abdulraqeeb33 abdulraqeeb33 changed the title Cleaned up and Added new tests chore: Cleaned up and Added new tests Feb 2, 2026
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds and refactors unit tests around the OpenTelemetry (OTel) crash reporting/uploader pipeline and related configuration/helpers, while removing an old networked integration-style test.

Changes:

  • Added new unit test suites for OTel factory/open-telemetry wiring, logging helper, uploader behavior, and config builders.
  • Refactored existing crash handler/reporter and attribute-field tests to reduce duplication and improve coverage.
  • Removed the CrashReportUploadTest integration test that performed real network calls.

Reviewed changes

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

Show a summary per file
File Description
OneSignalSDK/onesignal/otel/src/test/java/com/onesignal/otel/crash/OtelCrashUploaderTest.kt New tests for uploader start behavior and constants.
OneSignalSDK/onesignal/otel/src/test/java/com/onesignal/otel/crash/OtelCrashReporterTest.kt Expanded crash reporter coverage (timestamp, error paths, logging).
OneSignalSDK/onesignal/otel/src/test/java/com/onesignal/otel/crash/OtelCrashHandlerTest.kt Refactored handler tests + added isOneSignalAtFault coverage.
OneSignalSDK/onesignal/otel/src/test/java/com/onesignal/otel/config/OtelConfigTest.kt New tests validating config builders (resource, exporters, crash file storage).
OneSignalSDK/onesignal/otel/src/test/java/com/onesignal/otel/attributes/OtelFieldsTopLevelTest.kt Refactored top-level attribute tests and added wrapper-field coverage.
OneSignalSDK/onesignal/otel/src/test/java/com/onesignal/otel/attributes/OtelFieldsPerEventTest.kt Refactored per-event attribute tests and UID uniqueness checks.
OneSignalSDK/onesignal/otel/src/test/java/com/onesignal/otel/OtelLoggingHelperTest.kt New tests for log-level → OTel severity mapping and emission behavior.
OneSignalSDK/onesignal/otel/src/test/java/com/onesignal/otel/OneSignalOpenTelemetryTest.kt New tests for factory output types, extension helpers, and basic behavior.
OneSignalSDK/onesignal/core/src/test/java/com/onesignal/debug/internal/logging/otel/android/AndroidOtelLoggerTest.kt New tests for Android OTel logger adapter basics (no-throw, interface).
OneSignalSDK/onesignal/core/src/test/java/com/onesignal/debug/internal/crash/OneSignalCrashUploaderWrapperTest.kt Updated wrapper tests (prefs setup, multiple-start safety).
OneSignalSDK/onesignal/core/src/test/java/com/onesignal/debug/internal/crash/OneSignalCrashHandlerFactoryTest.kt Updated factory tests with Robolectric config and idempotency checks.
OneSignalSDK/onesignal/core/src/test/java/com/onesignal/debug/internal/crash/CrashReportUploadTest.kt Removed integration/network upload test.

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

Comment on lines 22 to 31
Thread.getDefaultUncaughtExceptionHandler() shouldBe crashHandler
verify { mockLogger.debug("OtelCrashHandler initialized") }
verify { mockLogger.info("OtelCrashHandler: Setting up uncaught exception handler...") }
verify { mockLogger.info("OtelCrashHandler: ✅ Successfully initialized and registered as default uncaught exception handler") }
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

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

These assertions match the exact log strings (including the ✅). Log message text is typically not a stable contract and this makes the test brittle to harmless wording/formatting changes. Prefer verifying via match { it.contains(...) } (or verifying state changes, e.g., default handler set) rather than exact string equality.

Copilot uses AI. Check for mistakes.
Comment on lines 115 to 125
test("setAllAttributes with Map should set all string attributes") {
val mockBuilder = mockk<LogRecordBuilder>(relaxed = true)
val attributes = mapOf(
"key1" to "value1",
"key2" to "value2"
)

mockBuilder.setAllAttributes(attributes)

// Verify the extension function was called (relaxed mock accepts all calls)
}
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

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

This test currently has no assertions/verifications, so it will pass even if setAllAttributes(Map) stops setting attributes. Consider verifying setAttribute is called for each entry (and optionally that the method returns the same builder for chaining).

Copilot uses AI. Check for mistakes.
Comment on lines 127 to 139
test("setAllAttributes with Attributes should handle different types") {
val mockBuilder = mockk<LogRecordBuilder>(relaxed = true)
val attributes = Attributes.builder()
.put("string.key", "string-value")
.put("long.key", 123L)
.put("double.key", 45.67)
.put("boolean.key", true)
.build()

mockBuilder.setAllAttributes(attributes)

// Verify extension function handles all types without throwing
}
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

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

This test doesn’t assert anything; it only calls setAllAttributes(Attributes). To make it meaningful, verify that setAttribute is invoked with the correct key/value types (string/long/double/boolean) and/or that unsupported types are stringified as intended.

Copilot uses AI. Check for mistakes.
Comment on lines +35 to 39
test("getAttributes should include all per-event fields when all values present") {
setupDefaultMocks()

val attributes = fields.getAttributes()

Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

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

OtelFieldsPerEvent.getAttributes() sets the app state under the key "app.state" (see OtelFieldsPerEvent.kt:25), but this test later asserts "android.app.state" (in multiple places). That mismatch will fail and also encodes the wrong attribute name. Update the assertions to use "app.state".

Copilot uses AI. Check for mistakes.
Comment on lines 89 to 91
test("SEND_TIMEOUT_SECONDS should be 30 seconds") {
OtelCrashUploader.SEND_TIMEOUT_SECONDS shouldNotBe 0L
}
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

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

Test name says it validates SEND_TIMEOUT_SECONDS is 30 seconds, but the assertion only checks it is non-zero. This won’t catch regressions (e.g., changing to 10 or 60). Assert equality to 30L (or to the expected constant value) instead.

Copilot uses AI. Check for mistakes.
// ===== OtelConfigRemoteOneSignal Tests =====

test("BASE_URL should point to staging endpoint") {
OtelConfigRemoteOneSignal.SdkLoggerProviderConfig.BASE_URL shouldContain "onesignal.com"
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

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

The test description says the BASE_URL should point to the staging endpoint, but the assertions only check for onesignal.com and /sdk/otel. Since the current value is https://api.staging.onesignal.com/sdk/otel, consider asserting the actual expected host/prefix (e.g., contains api.staging.onesignal.com) or rename the test to match what is being asserted.

Suggested change
OtelConfigRemoteOneSignal.SdkLoggerProviderConfig.BASE_URL shouldContain "onesignal.com"
OtelConfigRemoteOneSignal.SdkLoggerProviderConfig.BASE_URL shouldContain "api.staging.onesignal.com"

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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


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

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

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


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

Comment on lines 22 to 31
fun setupDefaultMocks(
remoteLogLevel: String? = "ERROR",
crashStoragePath: String = "/test/crash/path",
minFileAgeForReadMillis: Long = 100L
) {
every { mockPlatformProvider.remoteLogLevel } returns remoteLogLevel
every { mockPlatformProvider.crashStoragePath } returns crashStoragePath
every { mockPlatformProvider.minFileAgeForReadMillis } returns minFileAgeForReadMillis
every { mockRemoteTelemetry.logExporter } returns mockExporter
every { mockExporter.export(any()) } returns CompletableResultCode.ofSuccess()
Copy link

Copilot AI Feb 2, 2026

Choose a reason for hiding this comment

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

These tests call uploader.start(), which creates a FileLogRecordStorage rooted at "/test/crash/path". On many CI environments this absolute path won't exist or won't be writable, causing test failures/flakiness. Use a per-test temp directory (e.g., under java.io.tmpdir), ensure it exists, and clean it up after the test.

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.


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

@github-actions
Copy link
Contributor

github-actions bot commented Feb 2, 2026

📊 Diff Coverage Report

Diff Coverage Report

Threshold: 80%

Changed Files Coverage

  • ⚠️ MainApplicationKT.kt: Not in coverage report (may not be compiled/tested)
  • JSONUtils.kt: 17/72 lines (23.6%)
    • ⚠️ Below threshold: 55 uncovered lines
  • ⚠️ CoreModule.kt: Not in coverage report (may not be compiled/tested)
  • IParamsBackendService.kt: 0/35 lines (0.0%)
    • ⚠️ Below threshold: 35 uncovered lines
  • ParamsBackendService.kt: 0/95 lines (0.0%)
    • ⚠️ Below threshold: 95 uncovered lines
  • BackgroundManager.kt: 0/89 lines (0.0%)
    • ⚠️ Below threshold: 89 uncovered lines
  • ConfigModel.kt: 46/143 lines (32.2%)
    • ⚠️ Below threshold: 97 uncovered lines
  • ConfigModelStore.kt: 0/2 lines (0.0%)
    • ⚠️ Below threshold: 2 uncovered lines
  • ConfigModelStoreListener.kt: 0/67 lines (0.0%)
    • ⚠️ Below threshold: 67 uncovered lines
  • HttpClient.kt: 131/151 lines (86.8%)
  • OperationRepo.kt: 187/205 lines (91.2%)
  • Time.kt: 2/3 lines (66.7%)
    • ⚠️ Below threshold: 1 uncovered lines
  • OneSignalCrashHandlerFactory.kt: 0/9 lines (0.0%)
    • ⚠️ Below threshold: 9 uncovered lines
  • OneSignalCrashUploaderWrapper.kt: 0/11 lines (0.0%)
    • ⚠️ Below threshold: 11 uncovered lines
  • OtelAnrDetector.kt: 0/99 lines (0.0%)
    • ⚠️ Below threshold: 99 uncovered lines
  • Logging.kt: 48/102 lines (47.1%)
    • ⚠️ Below threshold: 54 uncovered lines
  • AndroidOtelLogger.kt: 9/9 lines (100.0%)
  • OtelIdResolver.kt: 0/95 lines (0.0%)
    • ⚠️ Below threshold: 95 uncovered lines
  • OtelPlatformProvider.kt: 0/76 lines (0.0%)
    • ⚠️ Below threshold: 76 uncovered lines
  • OneSignalCrashLogInit.kt: 0/52 lines (0.0%)
    • ⚠️ Below threshold: 52 uncovered lines
  • OneSignalImp.kt: 60/262 lines (22.9%)
    • ⚠️ Below threshold: 202 uncovered lines
  • OutcomeEventsController.kt: 142/161 lines (88.2%)
  • SessionListener.kt: 12/23 lines (52.2%)
    • ⚠️ Below threshold: 11 uncovered lines
  • LoginHelper.kt: 31/31 lines (100.0%)
  • IdentityModelStore.kt: 1/3 lines (33.3%)
    • ⚠️ Below threshold: 2 uncovered lines
  • InAppBackendService.kt: 125/131 lines (95.4%)
  • InAppDisplayer.kt: 0/67 lines (0.0%)
    • ⚠️ Below threshold: 67 uncovered lines
  • InAppMessageView.kt: 0/354 lines (0.0%)
    • ⚠️ Below threshold: 354 uncovered lines
  • InAppHydrator.kt: 22/25 lines (88.0%)
  • LocationManager.kt: 88/95 lines (92.6%)
  • HmsLocationController.kt: 0/124 lines (0.0%)
    • ⚠️ Below threshold: 124 uncovered lines
  • OneSignalHmsEventBridge.kt: 0/34 lines (0.0%)
    • ⚠️ Below threshold: 34 uncovered lines
  • NotificationChannelManager.kt: 0/126 lines (0.0%)
    • ⚠️ Below threshold: 126 uncovered lines
  • OSWorkManagerHelper.kt: 0/12 lines (0.0%)
    • ⚠️ Below threshold: 12 uncovered lines
  • NotificationRepository.kt: 0/344 lines (0.0%)
    • ⚠️ Below threshold: 344 uncovered lines
  • NotificationGenerationProcessor.kt: 131/158 lines (82.9%)
  • NotificationLifecycleService.kt: 0/134 lines (0.0%)
    • ⚠️ Below threshold: 134 uncovered lines
  • PushTokenManager.kt: 23/25 lines (92.0%)
  • ReceiveReceiptProcessor.kt: 0/9 lines (0.0%)
    • ⚠️ Below threshold: 9 uncovered lines
  • PushRegistratorADM.kt: 0/29 lines (0.0%)
    • ⚠️ Below threshold: 29 uncovered lines
  • PushRegistratorAbstractGoogle.kt: 0/85 lines (0.0%)
    • ⚠️ Below threshold: 85 uncovered lines
  • PushRegistratorHMS.kt: 0/42 lines (0.0%)
    • ⚠️ Below threshold: 42 uncovered lines
  • NotificationRestoreProcessor.kt: 0/34 lines (0.0%)
    • ⚠️ Below threshold: 34 uncovered lines
  • NotificationRestoreWorkManager.kt: 0/31 lines (0.0%)
    • ⚠️ Below threshold: 31 uncovered lines
  • ADMMessageHandler.kt: 0/28 lines (0.0%)
    • ⚠️ Below threshold: 28 uncovered lines
  • ADMMessageHandlerJob.kt: 0/30 lines (0.0%)
    • ⚠️ Below threshold: 30 uncovered lines
  • OneSignalOpenTelemetry.kt: 0/66 lines (0.0%)
    • ⚠️ Below threshold: 66 uncovered lines
  • OtelFactory.kt: 0/23 lines (0.0%)
    • ⚠️ Below threshold: 23 uncovered lines
  • OtelLoggingHelper.kt: 0/31 lines (0.0%)
    • ⚠️ Below threshold: 31 uncovered lines
  • OtelFieldsPerEvent.kt: 0/13 lines (0.0%)
    • ⚠️ Below threshold: 13 uncovered lines
  • OtelFieldsTopLevel.kt: 0/21 lines (0.0%)
    • ⚠️ Below threshold: 21 uncovered lines
  • OtelConfigCrashFile.kt: 0/19 lines (0.0%)
    • ⚠️ Below threshold: 19 uncovered lines
  • OtelConfigRemoteOneSignal.kt: 0/18 lines (0.0%)
    • ⚠️ Below threshold: 18 uncovered lines
  • OtelConfigShared.kt: 0/18 lines (0.0%)
    • ⚠️ Below threshold: 18 uncovered lines
  • OtelCrashHandler.kt: 0/42 lines (0.0%)
    • ⚠️ Below threshold: 42 uncovered lines
  • OtelCrashReporter.kt: 0/33 lines (0.0%)
    • ⚠️ Below threshold: 33 uncovered lines
  • OtelCrashUploader.kt: 0/29 lines (0.0%)
    • ⚠️ Below threshold: 29 uncovered lines

Overall Coverage

1075/4025 lines covered (26.7%)

❌ Coverage Check Failed

Files below 80% threshold:

  • JSONUtils.kt: 23.6% (55 uncovered lines)

  • IParamsBackendService.kt: 0.0% (35 uncovered lines)

  • ParamsBackendService.kt: 0.0% (95 uncovered lines)

  • BackgroundManager.kt: 0.0% (89 uncovered lines)

  • ConfigModel.kt: 32.2% (97 uncovered lines)

  • ConfigModelStore.kt: 0.0% (2 uncovered lines)

  • ConfigModelStoreListener.kt: 0.0% (67 uncovered lines)

  • Time.kt: 66.7% (1 uncovered lines)

  • OneSignalCrashHandlerFactory.kt: 0.0% (9 uncovered lines)

  • OneSignalCrashUploaderWrapper.kt: 0.0% (11 uncovered lines)

  • OtelAnrDetector.kt: 0.0% (99 uncovered lines)

  • Logging.kt: 47.1% (54 uncovered lines)

  • OtelIdResolver.kt: 0.0% (95 uncovered lines)

  • OtelPlatformProvider.kt: 0.0% (76 uncovered lines)

  • OneSignalCrashLogInit.kt: 0.0% (52 uncovered lines)

  • OneSignalImp.kt: 22.9% (202 uncovered lines)

  • SessionListener.kt: 52.2% (11 uncovered lines)

  • IdentityModelStore.kt: 33.3% (2 uncovered lines)

  • InAppDisplayer.kt: 0.0% (67 uncovered lines)

  • InAppMessageView.kt: 0.0% (354 uncovered lines)

  • HmsLocationController.kt: 0.0% (124 uncovered lines)

  • OneSignalHmsEventBridge.kt: 0.0% (34 uncovered lines)

  • NotificationChannelManager.kt: 0.0% (126 uncovered lines)

  • OSWorkManagerHelper.kt: 0.0% (12 uncovered lines)

  • NotificationRepository.kt: 0.0% (344 uncovered lines)

  • NotificationLifecycleService.kt: 0.0% (134 uncovered lines)

  • ReceiveReceiptProcessor.kt: 0.0% (9 uncovered lines)

  • PushRegistratorADM.kt: 0.0% (29 uncovered lines)

  • PushRegistratorAbstractGoogle.kt: 0.0% (85 uncovered lines)

  • PushRegistratorHMS.kt: 0.0% (42 uncovered lines)

  • NotificationRestoreProcessor.kt: 0.0% (34 uncovered lines)

  • NotificationRestoreWorkManager.kt: 0.0% (31 uncovered lines)

  • ADMMessageHandler.kt: 0.0% (28 uncovered lines)

  • ADMMessageHandlerJob.kt: 0.0% (30 uncovered lines)

  • OneSignalOpenTelemetry.kt: 0.0% (66 uncovered lines)

  • OtelFactory.kt: 0.0% (23 uncovered lines)

  • OtelLoggingHelper.kt: 0.0% (31 uncovered lines)

  • OtelFieldsPerEvent.kt: 0.0% (13 uncovered lines)

  • OtelFieldsTopLevel.kt: 0.0% (21 uncovered lines)

  • OtelConfigCrashFile.kt: 0.0% (19 uncovered lines)

  • OtelConfigRemoteOneSignal.kt: 0.0% (18 uncovered lines)

  • OtelConfigShared.kt: 0.0% (18 uncovered lines)

  • OtelCrashHandler.kt: 0.0% (42 uncovered lines)

  • OtelCrashReporter.kt: 0.0% (33 uncovered lines)

  • OtelCrashUploader.kt: 0.0% (29 uncovered lines)

📥 View workflow run

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.

1 participant