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.

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

[unit tests] Add some infrastructure for OneSignalCore and OneSignalUser #1371

Merged
merged 9 commits into from
Feb 21, 2024

Conversation

nan-li
Copy link
Contributor

@nan-li nan-li commented Feb 14, 2024

Description

One Line Summary

Rebuilding our unit tests by adding basic infrastructure to work with the modules OneSignalCore and OneSignalUser.

Details

  • This PR is best reviewed commit by commit.
  • Overall, to avoid undergoing the work of refactoring the whole SDK to really use dependency injection, we are doing a pseudo-form. The eventual vision is that everything can be injected.

1. (cleanup) Removed some unused things from the OneSignalClient

2. Make OneSignalCore class to set and access module's services

  • Add a protocol for OneSignalClient, called IOneSignalClient (open to more ios-appropriate name for the protocol)
  • In production, still use the original OneSignalClient.sharedClient but if unit tests set their own client, return that one

3. Now, access the client via OneSignalCore

  • Change all OneSignalClient.sharedClient calls to OneSignalCore.sharedClient

4. Extract Operation Repo's poll interval into a constant

  • Store in the variable POLL_INTERVAL_MS so that it can have a different value for testing vs non-testing

5. Add 2 mocking frameworks and 2 testing bundles

  • Add mocking framework called OneSignalCoreMocks to simplify working with OneSignalCore in unit tests
    • Add helper method clearUserDefaults()
    • Create MockOneSignalClient class to replace OneSignalClient in unit tests
  • Add mocking framework called OneSignalUserMocks to simplify working with OneSignalUser in unit tests
    • Add some helper methods to reset state between tests, WIP as we realize more state needs to be cleared
  • Add unit testing bundle OneSignalCoreTests, meant to test OneSignalCore
    • Add one test testNotificationJson, to get started
  • Add unit testing bundle OneSignalUserTests, meant to test OneSignalUser
    • Add one test testLoginSetsExternalId, to get started

6. (cleanup) Delete old "user model" test files

  • Delete old "user model" test files UserModelObjcTests.m and UserModelSwiftTests.swift
    • These weren't real tests

7. Add separate swiftlint file for test modules to allow force casts

Motivation

Rebuilding unit tests

Scope

  • Affects unit testing

Testing

Unit testing

  • There are only 2 tests in the SDK
  • To run them, go to the Test Navigator (diamond icon) and run the two testing bundles.

Screenshot 2024-02-14 at 1 36 03 PM

Manual testing

  • Ran unit tests on emulator and physical iPhone with iOS 17.2
  • Ran example app on physical iPhone with iOS 17.2

Affected code checklist

  • Notifications
    • Display
    • Open
    • Push Processing
    • Confirm Deliveries
  • Outcomes
  • Sessions
  • In-App Messaging
  • REST API requests
  • Public API changes
  • Unit 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

Looks like these methods are no longer used within the SDK, so let's remove them from OneSignalClient:
- executeSimultaneousRequests
- executeSynchronousRequest
- executeDataRequest

Update OneSignalClient.h

And types:
- OSDataRequestSuccessBlock
- OSMultipleCompletionBlock
- OSMultipleFailureBlock
- OSMultipleSuccessBlock

- genericTimedOutError
- Make a OneSignalCore class through which the services of this module can be set and accessed
- To start, make a protocol for OneSignalClient, IOneSignalClient (name of protocol TBD)
- Change all `OneSignalClient.sharedClient` calls to `OneSignalCore.sharedClient`
* Store in the variable `POLL_INTERVAL_MS` so that it can have a different value for testing vs non-testing
@nan-li nan-li changed the title [unit tests] Add some infrastructure for Core and User in tests [unit tests] Add some infrastructure for OneSignalCore and OneSignalUser Feb 14, 2024
* Add mocking framework called `OneSignalCoreMocks` to simplify working with `OneSignalCore` in unit tests
    - Add helper method `clearUserDefaults()`
    - Create `MockOneSignalClient` class to replace `OneSignalClient` in unit tests

* Add mocking framework called `OneSignalUserMocks` to simplify working with `OneSignalUser` in unit tests
    - Add some helper methods to reset state between tests, WIP

* Add unit testing bundle `OneSignalCoreTests`, meant to test `OneSignalCore`
    - Add one test `testNotificationJson`, to get started

* Add unit testing bundle `OneSignalUserTests`, meant to test `OneSignalUser`
    - Add one test `testLoginSetsExternalId`, to get started
* Delete old "user model" test files `UserModelObjcTests.m and `UserModelSwiftTests.swift`
    - These weren't real tests
@nan-li nan-li force-pushed the 5.x.x/unit_tests_infra branch from 2e215b4 to d6102b4 Compare February 14, 2024 20:29
* We want to use force casts in testing
@nan-li nan-li requested review from emawby, jennantilla, jkasten2, jinliu9508 and shepherd-l and removed request for emawby February 14, 2024 22:06
@nan-li
Copy link
Contributor Author

nan-li commented Feb 16, 2024

The CI succeeded at first because it was only testing the tests in UnitTests module (which includes 0 tests).

We are trying to make all testing bundles run, so we added UserTests and CoreTests to the UnitTestApp scheme.

However, after that change, now the CI is failing.
Still investigating.

@nan-li nan-li force-pushed the 5.x.x/unit_tests_infra branch from 2db39b3 to 4f30fe0 Compare February 21, 2024 21:31
@nan-li nan-li force-pushed the 5.x.x/unit_tests_infra branch from 4bfcb1e to a7c1143 Compare February 21, 2024 22:12
- ENABLE_TESTABILITY in OneSignalUserTests and OneSignalCoreTests modules updated to match existing UnitTests
- UnitTestApp scheme updated to include OneSignalUserTests and OneSignalCoreTests test modules
- Now, the CI runs all tests automatically!
@nan-li nan-li force-pushed the 5.x.x/unit_tests_infra branch 2 times, most recently from 78158a4 to 194832e Compare February 21, 2024 22:50
@nan-li
Copy link
Contributor Author

nan-li commented Feb 21, 2024

The CI now passes, and runs all the testing bundles, UnitTests, OneSignalCoreTests.xctest, and OneSignalUserTests.xctest.

@nan-li nan-li merged commit 782ae72 into main Feb 21, 2024
7 checks passed
@nan-li nan-li deleted the 5.x.x/unit_tests_infra branch February 21, 2024 23:01
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.

2 participants