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

[MOB-6632] [MOB-6633] [MOB-6634] [MOB-6636] Add Iterable embedded manager unit tests #670

Merged
merged 3 commits into from
Aug 29, 2023

Conversation

anlinguist
Copy link
Collaborator

@anlinguist anlinguist commented Aug 23, 2023

🔹 Jira Ticket(s)

✏️ Description

This PR implements a number of unit tests for the IterableEmbeddedManager:

  • getMessages
  • syncMessages
  • notify multiple delegates
  • add and remove listeners
  • init/deinit
  • didBecomeActiveNotification

Had to make a couple minor adjustments to the actual manager to add/remove forerground notification observers (they hadn't been implemented yet), and to add onDeinit closure, which we can then observe in tests.

Copy link
Member

@Ayyanchira Ayyanchira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking really great. Added some comments for my own clarity.

manager.syncMessages { }

XCTAssertTrue(delegate1Called, "Delegate 1 should have been notified.")
XCTAssertTrue(delegate2Called, "Delegate 2 should have been notified.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wonderful test case!

])
manager.syncMessages { }

XCTAssertFalse(delegateCalled, "Delegate should not have been notified after being removed.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

}

override func getEmbeddedMessages() -> IterableSDK.Pending<IterableSDK.PlacementsPayload, IterableSDK.SendRequestError> {
if invalidApiKey {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A little confused here. Does the actual getMessages not get called in the tests?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ayyanchira if we didn't override getEmbeddedMessages(), the test would attempt to call the API. As this is only a unit test (and not an endpoint test), we don't want to make actual API calls, as that could introduce flakiness and external dependency.

@anlinguist anlinguist marked this pull request as ready for review August 29, 2023 15:39
@Ayyanchira Ayyanchira self-requested a review August 29, 2023 18:06
@anlinguist anlinguist merged commit 5df8c39 into embedded-messaging Aug 29, 2023
1 of 2 checks passed
@anlinguist anlinguist deleted the anelson/embedded-tests branch August 29, 2023 20:05
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