Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[tests] In App Messaging tests basic infrastructure #1538

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

[tests] In App Messaging tests basic infrastructure #1538

wants to merge 9 commits into from

Conversation

nan-li
Copy link
Contributor

@nan-li nan-li commented Jan 28, 2025 •
edited
Loading

Description

One Line Summary

Add basic infrastructure to test the OneSignalInAppMessages module.

Details

  • We want to test the Objective-C framework OneSignalInAppMessages using Swift language. Previously we could access all APIs by importing directly in Objective-C. However, there are limitations to accessing internal Objective-C APIs from Swift - that is, they must all be public.
  • This is not an issue if Swift test code was accessing Swift internal methods, as it is easy to annotate @testable import in that scenario.
  • Considered making some internal APIs public, but as I added more necessary steps to tests, I found I had to expose an increasing number of internal APIs. This is not preferable.
  • While not ideal to add more Objective-C code in tests, I resorted to keeping internal APIs as is, and adding Objective-C helpers. Eventually, I expect the SDK to migrate to Swift and this would not be needed then.
  • I investigated other options but eventually hit dead ends with each.

Motivation

We need tests!

Scope

Testing only, not changing the shipped SDK, or making any internal APIs public.

Testing

Unit testing

  • Add Swift lang test file IAMIntegrationTests and write one integration test
    • In attempt to migrate existing testDisablingIAMs_stillCreatesMessageQueue_butPreventsMessageDisplay to new tests, but behavior has changed. It becomes testPausingIAMs_doesNotCreateMessageQueue.
  • Add new module OneSignalInAppMessagesMocks for re-usable test helpers and mock functionality
  • 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.
  • Add ConsistencyManagerTestHelpers with methods for repeating functionality

Manual testing

  • Just the app builds and runs still.

Affected code checklist

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

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

This change is Reviewable

nan-li added 4 commits January 27, 2025 20:01
@nan-li
Add Swift IAMIntegrationTests test file
c47b9e1 
Add Swift `IAMIntegrationTests` test file and check for accessibility of `OSMessagingController` from the test file
@nan-li
nit: remove unused methods getTriggerValueForKey
22ee265 
* no longer used, dead code
@nan-li
nit: remove methods from OSMessagingController header
0cdd45a 
* Aren't called by anything so no need to be on the interface, these are called by the class itself.
@nan-li
Add description to OSRequestGetInAppMessages
b3cd370 
* 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_iam_module_v5 branch 2 times, most recently from 13233a7 to 8482fcb Compare January 28, 2025 04:51
@nan-li nan-li changed the title [tests] In App Messaging tests basic infrastructure foo Jan 28, 2025
@nan-li nan-li closed this Jan 28, 2025
@nan-li nan-li reopened this Jan 28, 2025
@nan-li nan-li force-pushed the tests_iam_module_v5 branch 3 times, most recently from 53c0f79 to a3cd2c9 Compare January 28, 2025 16:42
@nan-li
Add framework for OneSignalInAppMessagesMocks
0768f8d 
* 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
@nan-li nan-li force-pushed the tests_iam_module_v5 branch from a3cd2c9 to 0768f8d Compare January 28, 2025 17:00
@nan-li
Make a MockMessagingController
ecdd49a 
* 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.
@nan-li nan-li force-pushed the tests_iam_module_v5 branch from 9777cbc to ecdd49a Compare January 28, 2025 17:33
nan-li added 3 commits January 28, 2025 09:54
@nan-li
Move UIApplication+OneSignal from OneSIgnal > OneSignalInAppMessages
64c25b6 
* 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).
@nan-li
Add ConsistencyManagerTestHelpers to tests
dfbd6d4 
* Has helper methods for repeating functionality
@nan-li
Add / migrate one existing IAM integration test
9de20c7 
* Attempt to migrate `testDisablingIAMs_stillCreatesMessageQueue_butPreventsMessageDisplay` to new tests, but behavior has changed.
* It becomes `testPausingIAMs_doesNotCreateMessageQueue`
@nan-li nan-li changed the title foo [tests] In App Messaging tests basic infrastructure Jan 28, 2025
@nan-li nan-li requested review from jkasten2 and jinliu9508 January 28, 2025 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jkasten2 jkasten2 Awaiting requested review from jkasten2

@jinliu9508 jinliu9508 Awaiting requested review from jinliu9508

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@nan-li