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

Add UnitTests for PayPalNativeCheckout Module #1144

Merged
merged 10 commits into from
Dec 7, 2023
Merged

Conversation

jaxdesmarais
Copy link
Contributor

Summary of changes

  • Test coverage is now at 84% for this module 🥳
  • Address issues with test coverage in the MXO module by adding mocking
  • This PR takes the same approach as PPCP to mocking as this will make it easier to jump between the 2 SDKs and grock what is happening
    • Add BTPayPalNativeCheckoutStartable (PPCP version)
    • Add BTPayPalNativeCheckoutProvider (PPCP version)
    • Add BTPayPalNativeCheckoutProtocol (PPCP version)
    • Names for these classes/protocols were kept as consistent as possible across the SDKs
  • Add new unit test file BTPayPalNativeCheckoutProvider_Tests and add additional unit tests to BTPayPalNativeCheckoutClient_Tests - many of the added tests are 1:1 with the tests for this module on PPCP
  • Update logic for new mocking protocol/classes

Checklist

  • [ ] Added a changelog entry

Authors

@jaxdesmarais jaxdesmarais requested a review from a team as a code owner December 5, 2023 15:45
Comment on lines 148 to 151
XCTAssertEqual(error.domain, BTPayPalNativeCheckoutError.errorDomain)
XCTAssertEqual(error.code, BTPayPalNativeCheckoutError.orderCreationFailed(BTPayPalNativeCheckoutError.invalidJSONResponse).errorCode)
XCTAssertEqual(error.localizedDescription, BTPayPalNativeCheckoutError.orderCreationFailed(BTPayPalNativeCheckoutError.invalidJSONResponse).errorDescription)
expectation.fulfill()
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick - I think ideally the tests should assert on the raw string values & raw int codes expected, versus referring to constants in the SDK. Otherwise we don't catch spelling errors or typos.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done: 2325569

Do we think it's worth documenting this somewhere? Not sure if the style guide or elsewhere is best.

Copy link
Contributor

@scannillo scannillo left a comment

Choose a reason for hiding this comment

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

Left a small comment re: asserting on raw strings vs internal constants, but LGTM!

Thanks for doing this really tricky/tedious work 👏

@jaxdesmarais jaxdesmarais merged commit 441f6d9 into main Dec 7, 2023
6 checks passed
@jaxdesmarais jaxdesmarais deleted the mxo-test-coverage branch December 7, 2023 16:10
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.

3 participants