Skip to content

Fix an issue where sandbox detector would return false in simulator builds.#2275

Closed
clindsay3 wants to merge 2 commits into
RevenueCat:mainfrom
clindsay3:simulator-receipt
Closed

Fix an issue where sandbox detector would return false in simulator builds.#2275
clindsay3 wants to merge 2 commits into
RevenueCat:mainfrom
clindsay3:simulator-receipt

Conversation

@clindsay3

Copy link
Copy Markdown
Collaborator

Checklist

  • If applicable, unit tests
  • If applicable, create follow-up issues for purchases-android and hybrids

Motivation

I have extensive UI tests checking various upgrade and conversion scenarios in my app. I recently transitioned from on-device receipt checking using StoreKit 1 to RevenueCat, and my tests were failing because the simulator was being treated as a non-sandbox environment.

Description

When running in the simulator, the receipt does not seem to be named sandboxReceipt, even though purchases are always made in the sandbox. This change handles this case.

I have tried to update the unit tests where appropriate, although currently there are some SystemInfo tests that are failing. I wasn't sure exactly the best approach to updating those tests, and would welcome suggestions on how to update them.

…uilds.

When running in the simulator, the receipt does not seem to be named sandboxReceipt, even though purchases are always made in the sandbox.
@joshdholtz

Copy link
Copy Markdown
Member

@chrisvasselli So excited that you made a PR already 🥳

My gut says that SystemInfo should maybe allow a SandboxEnvironmentDetector to be passed in through the constructor but @aboedo or @NachoSoto might have a more correct thought on this 😇

@NachoSoto NachoSoto left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks like we ran into the same thing and solved it in a different way: #1730
But if this isn't isolated I agree checking the target environment would be appropriate.

Thanks for sending a suggested PR! Let me expand on this and remove the custom detector for our integration tests since that wouldn't be necessary anymore.

@NachoSoto

Copy link
Copy Markdown
Contributor

Thanks! Closing this in favor of #2276.

@NachoSoto NachoSoto closed this Feb 9, 2023
@NachoSoto

Copy link
Copy Markdown
Contributor

Actually I like @joshdholtz idea to keep those tests and improve SystemInfo, let me do that.

@clindsay3

Copy link
Copy Markdown
Collaborator Author

@NachoSoto @joshdholtz great, glad I could help! 👍

@clindsay3 clindsay3 deleted the simulator-receipt branch February 10, 2023 02:17
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