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

In-App Message Unit Test Migration #2103

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

Conversation

jennantilla
Copy link
Contributor

@jennantilla jennantilla commented May 30, 2024

Description

One Line Summary

In-App Message Unit Test Migration

Details

Since its release, the Android User Model SDK has largely included only new tests and is lacking may important Player Model unit tests. Due to the significant updates to User Model, the existing unit tests on Player Model require significant refactoring.

Motivation

Provide a more comprehensive test suite to Android SDK.

Scope

This PR encompasses In-App Message unit tests only.

OPTIONAL - Other

Where possible, I've kept the original test names of the junit tests from Player Model for easier tracking.

Some considerations I'm still working through:

  • reconciling conflicting types between between old unit tests and new unit tests
  • enabling mockk for more tests
  • "weeding out" unnecessary old tests where appropriate

Testing

Unit testing

All tests in InApp MessagesTests.kt pass. Commented tests with "TODO" markers still need to be migrated/implemented.

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

@jennantilla jennantilla marked this pull request as draft May 30, 2024 17:56
@jennantilla jennantilla marked this pull request as ready for review July 30, 2024 18:33
@jennantilla jennantilla changed the title WIP In-App Message Unit Test Migration In-App Message Unit Test Migration Jul 30, 2024
@nan-li nan-li self-requested a review July 30, 2024 18:47
- Add Robolectric
@jennantilla jennantilla force-pushed the iam_unit_test_migration branch from c19e4f0 to 495013b Compare July 30, 2024 23:41
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