Skip to content

Added integration and unit tests to verify observer mode behavior#2069

Merged
NachoSoto merged 4 commits into
mainfrom
observer-mode-tests
Dec 15, 2022
Merged

Added integration and unit tests to verify observer mode behavior#2069
NachoSoto merged 4 commits into
mainfrom
observer-mode-tests

Conversation

@NachoSoto

@NachoSoto NachoSoto commented Nov 17, 2022

Copy link
Copy Markdown
Contributor

See TRIAGE-179.

I wrote all these to cover the change in #2063. I wrote a failing test that only passed with the change in #2063. However, when running that in SK1, that would fail.
With the change as suggested in #2063, the SK2 behavior would differ from SK1.

From the Transaction.all docs:

This sequence returns the user’s transaction history current to the moment you access the sequence. The sequence emits a finite number of transactions. If the App Store processes new transactions for the user while you’re accessing this sequence, the new transactions appear in the transaction listener, updates.

These new tests verify the behavior of the SDK when encountering transactions created prior to initializing the SDK, and some made after initialization.

Integration tests now are as follows:

  • BaseStoreKitIntegrationTests
    • StoreKit1IntegrationTests
    • StoreKit2IntegrationTests
    • BaseStoreKitObserverModeIntegrationTests:
      • StoreKit1ObserverModeIntegrationTests
      • StoreKit2ObserverModeIntegrationTests
      • StoreKit1ObserverModeWithExistingPurchasesTests
      • StoreKit2ObserverModeWithExistingPurchasesTests

Depends on #2064, #2066, and #2134.

Comment on lines +22 to +23
private var _invokedTransactionUpdated: Atomic<Bool> = false
private var _updatedTransactions: Atomic<[StoreTransactionType]> = .init([])

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Made this thread safe so that MockStoreKit2TransactionListenerDelegate is Sendable

@NachoSoto

Copy link
Copy Markdown
Contributor Author

This is ready for review.

Comment thread Tests/BackendIntegrationTests/StoreKitIntegrationTests.swift

try await self.purchaseProductFromStoreKit()

let info = try await Purchases.shared.restorePurchases()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm confused about this - when we do restore we're forcing a receipt post, so this effectively is just testing that if the purchase there it gets posted, right?
like, shouldn't this test just be "purchase from storekit, then verify entitlement went through"? Same question for the other one

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

To be clear, my concern is that the minute that we call restorePurchases, we're not really testing observer mode behavior

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Honestly I don't remember cause it's been 20 days :/ but I'll take a look and see if I can figure out what case this was.
The TLDR was that some of these started failing in SK1 / SK2 if we changed the behavior, so it's documenting existing behavior.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Okay I see what you're saying. I guess it's a very basic test. What else would you do if we don't restore purchases to verify what the SDK does in this case of purchasing from "outside" the SDK?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

after a purchase was made from "outside", the listeners should get notified and we should:

  • get a notification from the customerInfoChanged delegate
  • if you call getCustomerInfo, it should have the entitlements
    If those are not happening, then observer mode isn't working.
    It could be that it just doesn't work well with StoreKitTest or something, but if we call restorePurchases we do a force sync to backend, so we're not really doing what someone in observer mode would be doing

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh cool, let me try changing the test to that then.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I updated this using #2134.

The good news: this works great! It waits for the transaction to be detected by the listener, which posts it to the backend, and then we see the entitlement.
The bad news: for some reason the transaction listener doesn't get the update until like 20-30 seconds later, which makes this test extremely slow (therefore why I added such a high timeout).

I think it's a very super useful test, but it being so slow is annoying. What do you think?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@aboedo this is the new test.

NachoSoto added a commit that referenced this pull request Dec 13, 2022
This is necessary because we can't use `Quick` v6.x yet.

Abstracting this out to use it in #2069.
Comment on lines 561 to 566

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this test case is interesting. ideally purchases don't go through before the SDK gets configured, otherwise, this is what happens - we miss the purchase.

in real life the most common case is a renewal which happens with the SDK being closed, and in that case, it will still show up through the queue. Same for purchases on other devices.

@NachoSoto NachoSoto changed the base branch from main to async-await December 13, 2022 20:25
Base automatically changed from async-await to main December 15, 2022 16:51
NachoSoto added a commit that referenced this pull request Dec 15, 2022
This is necessary because we can't use `Quick` v6.x yet.

Abstracting this out to use it in #2069.
I wrote all these to cover the change in #2063. I wrote a failing test that only passed with the change in #2063. However, when running that in SK1, that would fail.
With the change as suggested in #2063, the SK2 behavior would differ from SK1.

From the `Transaction.all` docs:
> This sequence returns the user’s transaction history current to the moment you access the sequence. The sequence emits a finite number of transactions. If the App Store processes new transactions for the user while you’re accessing this sequence, the new transactions appear in the transaction listener, updates.

So all these new tests verify the behavior of the SDK when encountering transactions created prior to initializing the SDK.

Depends on #2064 and #2066.
@NachoSoto NachoSoto removed the blocked label Dec 15, 2022
@NachoSoto NachoSoto enabled auto-merge (squash) December 15, 2022 17:39
@NachoSoto NachoSoto merged commit 6716c45 into main Dec 15, 2022
@NachoSoto NachoSoto deleted the observer-mode-tests branch December 15, 2022 17:49
NachoSoto added a commit that referenced this pull request Dec 15, 2022
With #2069 this file grew up a bit too much.
This splits up `BaseStoreKitIntegrationTests` and `StoreKitObserverModeIntegrationTests` to make them more maintanable.
NachoSoto pushed a commit that referenced this pull request Dec 15, 2022
**This is an automatic release.**

### Bugfixes
* Fix sending presentedOfferingIdentifier in StoreKit2 (#2156) via Toni
Rico (@tonidero)
* `ReceiptFetcher`: throttle receipt refreshing to avoid `StoreKit`
throttle errors (#2146) via NachoSoto (@NachoSoto)
### Other Changes
* Added integration and unit tests to verify observer mode behavior
(#2069) via NachoSoto (@NachoSoto)
* Created `ClockType` and `TestClock` to be able to mock time (#2145)
via NachoSoto (@NachoSoto)
* Extracted `asyncWait` to poll `async` conditions in tests (#2134) via
NachoSoto (@NachoSoto)
* `StoreKitRequestFetcher`: added log when starting/ending requests
(#2151) via NachoSoto (@NachoSoto)
* `CI`: fixed `PurchaseTester` deployment (#2147) via NachoSoto
(@NachoSoto)
NachoSoto added a commit that referenced this pull request Dec 20, 2022
With #2069 this file grew up a bit too much.
This splits up `BaseStoreKitIntegrationTests` and `StoreKitObserverModeIntegrationTests` to make them more maintanable.
NachoSoto added a commit that referenced this pull request Dec 21, 2022
With #2069 this file grew up a bit too much.

This splits up `BaseStoreKitIntegrationTests` and
`StoreKitObserverModeIntegrationTests` to make them more maintainable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants