PurchasesOrchestrator: don't listen for StoreKit 2 transactions if it's disabled#1593
Conversation
|
|
||
| if #available(iOS 15.0, macOS 12.0, tvOS 15.0, watchOS 8.0, *) { | ||
| if #available(iOS 15.0, macOS 12.0, tvOS 15.0, watchOS 8.0, *), | ||
| systemInfo.storeKit2Setting == .enabledForCompatibleDevices { |
There was a problem hiding this comment.
feels like we should have an accompanying test for this, so we don't inadvertently break it in the future. It can be done as a separate PR
aboedo
left a comment
There was a problem hiding this comment.
approved, but could you add a test for it?
|
|
||
| if #available(iOS 15.0, macOS 12.0, tvOS 15.0, watchOS 8.0, *) { | ||
| if #available(iOS 15.0, macOS 12.0, tvOS 15.0, watchOS 8.0, *), | ||
| systemInfo.storeKit2Setting == .enabledForCompatibleDevices { |
There was a problem hiding this comment.
feels like we should have an accompanying test for this, so we don't inadvertently break it in the future. It can be done as a separate PR
|
See the description:
I couldn't think of a way to add a test for it. |
|
@NachoSoto I see, good catch. Good to go then! |
…it's disabled I tried adding tests for this, but because the listener is created internally, there is no time to override it with the mock before verifying if `listenForTransactions` is called.
b5af2c1 to
dbf8a67
Compare
The rabbit hole goes deep and deep:
Here's a proposed solution: #1618 It adds a fair amount of complexity, but I guess it's important to test this. |
|
I'll merge this for now, maybe we can have it out in the release. |
I tried adding tests for this, but because the listener is created internally, there is no time to override it with the mock before verifying if
listenForTransactionsis called.Will at least prevent #1528 if SK2 is disabled.