[tests] In App Messaging tests basic infrastructure #1538

nan-li · 2025-01-28T04:05:32Z

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

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

Add Swift `IAMIntegrationTests` test file and check for accessibility of `OSMessagingController` from the test file

* no longer used, dead code

* 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.

* 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.

* 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`

jkasten2 · 2025-03-04T18:37:41Z

iOS_SDK/OneSignalSDK/OneSignalInAppMessages/Controller/OSMessagingController.m

@@ -165,6 +165,7 @@ + (OSMessagingController *)sharedInstance {
    return sharedInstance;
 }

+// TODO: Used by tests only? Come back and see if this is needed.


Can this TODO be addressed in this PR?

This method is used by tests only, I left this TODO as I am unsure if we will need to rely on this implementation as we migrate tests in the future, or if we can reset OSMessagingController state differently. I can clarify this TODO.

jkasten2 · 2025-03-04T18:40:10Z

iOS_SDK/OneSignalSDK/OneSignalInAppMessagesMocks/MockMessagingController.m

+
+@implementation MockMessagingController
+ (void)resetState {
+    // TODO: reseting OSMessagingController state between tests


Do we need to address this now?

jkasten2 · 2025-03-04T18:41:20Z

iOS_SDK/OneSignalSDK/UnitTests/Shadows/OSMessagingControllerOverrider.m

@@ -112,7 +112,7 @@ - (void)overrideShowMessage:(OSInAppMessageInternal *)message {

 - (void)overrideWebViewContentFinishedLoading:(OSInAppMessageInternal *)message {
    if (message) {
-        [OSMessagingController.sharedInstance messageViewImpressionRequest:message];
+        // [OSMessagingController.sharedInstance messageViewImpressionRequest:message];


Can we remove this code instead of commenting it out?

nan-li added 4 commits January 27, 2025 20:01

Add Swift IAMIntegrationTests test file

c47b9e1

Add Swift `IAMIntegrationTests` test file and check for accessibility of `OSMessagingController` from the test file

nit: remove unused methods getTriggerValueForKey

22ee265

* no longer used, dead code

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.

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 force-pushed the tests_iam_module_v5 branch 2 times, most recently from 13233a7 to 8482fcb Compare January 28, 2025 04:51

nan-li changed the title ~~[tests] In App Messaging tests basic infrastructure~~ foo Jan 28, 2025

nan-li closed this Jan 28, 2025

nan-li reopened this Jan 28, 2025

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 force-pushed the tests_iam_module_v5 branch from a3cd2c9 to 0768f8d Compare January 28, 2025 17:00

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

Add ConsistencyManagerTestHelpers to tests

dfbd6d4

* Has helper methods for repeating functionality

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 changed the title ~~foo~~ [tests] In App Messaging tests basic infrastructure Jan 28, 2025

nan-li requested review from jkasten2 and jinliu9508 January 28, 2025 18:35

jkasten2 requested changes Mar 4, 2025

View reviewed changes

wip

1df9783

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[tests] In App Messaging tests basic infrastructure #1538

[tests] In App Messaging tests basic infrastructure #1538

nan-li commented Jan 28, 2025 •

edited

Loading

jkasten2 Mar 4, 2025

nan-li Mar 6, 2025

jkasten2 Mar 4, 2025

jkasten2 Mar 4, 2025

[tests] In App Messaging tests basic infrastructure #1538

Are you sure you want to change the base?

[tests] In App Messaging tests basic infrastructure #1538

Conversation

nan-li commented Jan 28, 2025 • edited Loading

Description

One Line Summary

Details

Motivation

Scope

Testing

Unit testing

Manual testing

Affected code checklist

Checklist

Overview

Testing

Final pass

jkasten2 Mar 4, 2025

Choose a reason for hiding this comment

nan-li Mar 6, 2025

Choose a reason for hiding this comment

jkasten2 Mar 4, 2025

Choose a reason for hiding this comment

jkasten2 Mar 4, 2025

Choose a reason for hiding this comment

nan-li commented Jan 28, 2025 •

edited

Loading