Skip to content

Conversation

@nan-li
Copy link
Contributor

@nan-li nan-li commented Nov 20, 2025

Description

One Line Summary

REQUIRED - Very short description that summaries the changes in this PR.

Details

Motivation

REQUIRED - Why is this code change being made? Or what is the goal of this PR? Examples: Fixes a specific bug, provides additional logging to debug future issues, feature to allow X.

Scope

RECOMMEND - OPTIONAL - What is intended to be effected. What is known not to change. Example: Notifications are grouped when parameter X is set, not enabled by default.

OPTIONAL - Other

OPTIONAL - Feel free to add any other sections or sub-sections that can explain your PR better.

Testing

Unit testing

OPTIONAL - Explain unit tests added, if not clear in the code.

Manual testing

RECOMMEND - OPTIONAL - Explain what scenarios were tested and the environment.
Example: Tested opening a notification while the app was foregrounded, app build with Android Studio 2020.3 with a fresh install of the OneSignal example app on a Pixel 6 with Android 12.

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

@nan-li nan-li added the WIP Work In Progress label Nov 20, 2025
@nan-li nan-li force-pushed the ci/add_code_coverage branch from a69c00c to f828623 Compare November 20, 2025 20:50
Add Swift `IAMIntegrationTests` test file and check for accessibility of `OSMessagingController` from the test file
* Aren't called by anything so no need to be on the interface, these are called by the class itself.
* Without a description, the object will return something like `<OSRequestGetInAppMessages: 0x600001798bc0>`. By adding a predictable description, we can operate on the object.
@nan-li nan-li force-pushed the tests/add_iam_tests branch from f620371 to d38abc1 Compare November 20, 2025 20:51
* Re-usable test helpers and mock functionality
* Include own swiftlint file
* ❗️ After `OneSignalInAppMessagesMocks` was created, I had to then manually "convert to group" or else the CI had build errors about the following:

xcodebuild: error: Unable to read project 'OneSignal.xcodeproj'
Reason: The project ‘OneSignal’ is damaged and cannot be opened. Examine the project file for invalid edits or unresolved source control conflicts.
Exception: didn't find classname for 'isa' key
* Make a `MockMessagingController` in Objective C to allow us to access internal APIs on the OSMessagingController without needing to make them public. This is needed because Swift code cannot access Objective-C APIs unless they are public. This is a workaround for this limitation.
…alInAppMessages

* This extension on `UIApplication` is only used in the `OneSignalInAppMessages` framework, so there is no need to keep it in the umbrella `OneSignalFramework` framework, which causes errors if only `OneSignalInAppMessages` is tested (without also importing OneSignalFramework).
* Has helper methods for repeating functionality
* Attempt to migrate `testDisablingIAMs_stillCreatesMessageQueue_butPreventsMessageDisplay` to new tests, but behavior has changed.
* It becomes `testPausingIAMs_doesNotCreateMessageQueue`
@nan-li nan-li force-pushed the tests/add_iam_tests branch from d38abc1 to 72510a5 Compare November 20, 2025 20:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

WIP Work In Progress

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants