Skip to content

Conversation

@koke
Copy link
Member

@koke koke commented Nov 9, 2022

Closes: #8057

Description

This adds some unit tests for the InAppPurchaseStore. I haven't tested every single aspect since it seems like MockNetwork can only simulate responses and not validate that an endpoint was called. So I'm not sure how to test the transaction listener, but I feel like this tests already touch the most critical points and it's an improvement over no tests 🤷🏽

Note that I had to bring back the local StoreKit configuration file for this to work. After testing different variations, I realized that there's no need to set the Xcode scheme to use it. As long as the file is included in the test bundle, when you initialize the SKTestSession, it will switch to the local configuration. The only tricky aspect was that I had to add the tests in the WooCommerce project, since Yosemite tests run without a host application and it doesn't seem like StoreKitTest works that way.

Testing instructions

Unit tests should pass


  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@koke koke added type: task An internally driven task. category: unit tests Related to unit testing. feature: in-app purchases Related to In-app purchases and subscriptions labels Nov 9, 2022
@koke koke added this to the 11.2 milestone Nov 9, 2022
@koke koke requested a review from toupper November 9, 2022 12:29
@peril-woocommerce
Copy link

Warnings
⚠️ This PR is assigned to a milestone which is closing in less than 2 days Please, make sure to get it merged by then or assign it to a later expiring milestone

Generated by 🚫 dangerJS

Comment on lines 10 to 12
"_applicationInternalID" : "1389130815",
"_developerTeamID" : "PZYM8XX95Q",
"_lastSynchronizedDate" : 689685986.85126996
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm having second thoughts about this. I added the StoreKit configuration with sync enabled, but if we're only going to use this for testing, maybe I should convert to local and ensure it only has the debug product? I'm not sure we want the whole product catalog synced to a public repo.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, if it's only for unit testing I would convert it to local and only add the debug product, no need to have the whole product catalog even if I don't see huge security issues with that.

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Nov 9, 2022

You can test the changes from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr8067-7407d46 on your iPhone

If you need access to App Center, please ask a maintainer to add you.

Comment on lines 10 to 12
"_applicationInternalID" : "1389130815",
"_developerTeamID" : "PZYM8XX95Q",
"_lastSynchronizedDate" : 689685986.85126996
Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, if it's only for unit testing I would convert it to local and only add the debug product, no need to have the whole product catalog even if I don't see huge security issues with that.

@toupper
Copy link
Contributor

toupper commented Nov 9, 2022

Great job! ILGTM, now we have vital parts tested 👍

I haven't tested every single aspect since it seems like MockNetwork can only simulate responses and not validate that an endpoint was called.

That's a good point. It seems to me like MockNetwork is easy to extend to validate that an endpoint was called; do you think it is worth doing that (In other PR/issue)? I see value in that, not only for this task, but for other cases when we want to test that an endpoint was called.

@koke
Copy link
Member Author

koke commented Nov 10, 2022

👍🏽 I'll take a look at extending MockNetwork in a separate PR

@koke koke enabled auto-merge November 10, 2022 09:27
@koke koke merged commit 7bbef09 into trunk Nov 10, 2022
@koke koke deleted the issue/8057-iap-store-tests branch November 10, 2022 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category: unit tests Related to unit testing. feature: in-app purchases Related to In-app purchases and subscriptions type: task An internally driven task.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[In-app Purchases] Adding unit tests to InAppPurchaseStore

4 participants