`Offline Entitlements`: integration tests by NachoSoto · Pull Request #2501 · RevenueCat/purchases-ios

NachoSoto · 2023-05-17T19:00:25Z

Example tests:

func testOfferingsAreCachedInMemory() async throws {
    let onlineOfferings = try await Purchases.shared.offerings()
    expect(onlineOfferings.all).toNot(beEmpty())

    self.serverDown()

    let offlineOfferings = try await Purchases.shared.offerings()
    expect(offlineOfferings) === onlineOfferings
}

func testOfferingsAreCachedOnDisk() async throws {
    let onlineOfferings = try await Purchases.shared.offerings()
    expect(onlineOfferings.all).toNot(beEmpty())

    self.serverDown()
    await self.resetPurchases()

    let offlineOfferings = try await Purchases.shared.offerings()
    expect(offlineOfferings.response) == onlineOfferings.response

    let offering = try XCTUnwrap(offlineOfferings.current)
    expect(offering.availablePackages.count) == onlineOfferings.current?.availablePackages.count
    expect(offering.monthly?.storeProduct.productIdentifier) == "com.revenuecat.monthly_4.99.1_week_intro"
}

NachoSoto · 2023-05-17T23:36:26Z

This allows having a custom type in integration tests so we can swap the return value, without having to make the actual DangerousSettings.Internal struct mutable.

NachoSoto · 2023-05-17T23:36:45Z

Changed this from DangerousSettings.InternalSettings to DangerousSettings.Internal.

winstondu · 2023-05-18T04:48:31Z

I think composition over inheritance is the way to go here.

It feels unnatural for the test class itself, which is fairly large and complex, to conform to this protocol and being what gets "dependency-injected" into the DangerousSettings initializer.

Why not have BaseBackendIntegrationTests instances hold a member variable of a new type, say a MutableInternalDangerousSettings class, that conforms to this protocol?

The serverUp() and serverDown() methods can then just mutate forceServerErrors that variable.

I think composition over inheritance is the way to go

Normally I agree, but I fail to see how that would improve this particular case. That would semantically be the same.
This set up is much simpler this way. The way these tests are set up is such that the tests themselves become the configuration, which allows us to override flags like this.

What benefit does your suggestion provide?

tonidero

Really love these tests! I think I should be able to copy part of it for the Android tests 👍. Though we don't support coroutines there yet so they won't look nearly as nice 😬

NachoSoto · 2023-05-18T17:30:24Z

Though we don't support coroutines there yet so they won't look nearly as nice

@tonidero migrating tests to async (#1332) was huge, it allowed us to iterate, maintain, and write new integration tests much faster. Before it was callback hell.
Might be worth investing on that in Android.

codecov · 2023-05-18T23:39:53Z

Codecov Report

Merging #2501 (d76b395) into nacho/sdk-2991-write-flow-to-use-locally-computed (8459cb7) will increase coverage by 0.05%.
The diff coverage is 63.15%.

❗ Current head d76b395 differs from pull request most recent head 09ad41b. Consider uploading reports for the commit 09ad41b to get more accurate results

@@                                  Coverage Diff                                  @@
##           nacho/sdk-2991-write-flow-to-use-locally-computed    #2501      +/-   ##
=====================================================================================
+ Coverage                                              87.83%   87.89%   +0.05%     
=====================================================================================
  Files                                                    198      198              
  Lines                                                  13583    13617      +34     
=====================================================================================
+ Hits                                                   11931    11968      +37     
+ Misses                                                  1652     1649       -3

Impacted Files	Coverage Δ
Sources/Misc/SystemInfo.swift	`98.27% <ø> (-0.02%)`	⬇️
...ources/Purchasing/StoreKit1/StoreKit1Wrapper.swift	`84.21% <51.72%> (-7.66%)`	⬇️
Sources/Logging/Strings/NetworkStrings.swift	`100.00% <100.00%> (ø)`
Sources/Misc/DangerousSettings.swift	`100.00% <100.00%> (ø)`
Sources/Networking/HTTPClient/HTTPClient.swift	`98.33% <100.00%> (+<0.01%)`	⬆️

... and 6 files with indirect coverage changes

tonidero

Looks great! This brought a great point in favor of supporting coroutines in Android 🙏

tonidero · 2023-05-19T09:16:43Z

I believe apple/Google shouldn't allow you to purchase the same subscription a second time right? So not sure if this is a valid scenario.

You can call the method multiple times, it will just reuse the transaction if it's not finished. It's not very important, but I was thinking "what if for some reason the purchase fails the first time while the server is down, will the SDK get in a weird state"?
So that's why I wrote this.

tonidero · 2023-05-19T13:20:27Z

Should we add a test case for foregrounding the app sends unsynced purchases?

That's done in testPurchaseWhileServerIsDownPostsReceiptAfterServerComesBack.

…er is down (#2368) ### Changes: - Created `PurchasedProductsFetcherType` to be able to mock and hold fetchers, even on older devices - Created `ProductEntitlementMappingFetcher` to abstract injecting `ProductEntitlementMapping` - Added logic to `CustomerInfoResponseHandler` to compute offline `CustomerInfo` after a server error (see #2367). **This is used by all `CustomerInfo` requests as well as `PostReceiptDataOperation`**. - Wrote comprehensive `CustomerInfoResponseHandler` tests ### TODO: - [x] Don't cache offline computed `CustomerInfo`: #2378 - [x] Integration tests: #2501.

@NachoSoto

**This is an automatic release.** ### New Features * `Offline Entitlements`: use offline-computed `CustomerInfo` when server is down (#2368) via NachoSoto (@NachoSoto) ### Bugfixes * `AppleReceipt.debugDescription`: don't pretty-print JSON (#2564) via NachoSoto (@NachoSoto) * `SK2StoreProduct`: fix crash on iOS 12 (#2565) via NachoSoto (@NachoSoto) * `GetCustomerInfo` posts receipts if there are pending transactions (#2533) via NachoSoto (@NachoSoto) ### Performance Improvements * `PurchasedProductsFetcher`: cache current entitlements (#2507) via NachoSoto (@NachoSoto) * Performance: new check to ensure serialization / deserialization is done from background thread (#2496) via NachoSoto (@NachoSoto) ### Dependency Updates * Bump fastlane from 2.212.2 to 2.213.0 (#2544) via dependabot[bot] (@dependabot[bot]) ### Other Changes * `CustomerInfoManager`: post all unfinished transactions (#2563) via NachoSoto (@NachoSoto) * `PostReceiptOperation`: added ability to also post `AdServices` token (#2566) via NachoSoto (@NachoSoto) * `Offline Entitlements`: improved computation log (#2562) via NachoSoto (@NachoSoto) * Added `TransactionPoster` tests (#2557) via NachoSoto (@NachoSoto) * Refactored `TransactionPoster`: removed 2 dependencies and abstracted parameters (#2542) via NachoSoto (@NachoSoto) * `CustomerInfoManagerTests`: wait for `getAndCacheCustomerInfo` to finish (#2555) via NachoSoto (@NachoSoto) * `StoreTransaction`: implemented `description` (#2556) via NachoSoto (@NachoSoto) * `Backend.ResponseHandler` is now `@Sendable` (#2541) via NachoSoto (@NachoSoto) * Extracted `TransactionPoster` from `PurchasesOrchestrator` (#2540) via NachoSoto (@NachoSoto) * `enableAdServicesAttributionTokenCollection`: added integration test (#2551) via NachoSoto (@NachoSoto) * `AttributionPoster`: replaced hardcoded strings with constants (#2548) via NachoSoto (@NachoSoto) * `DefaultDecodable`: moved to `Misc/Codable/DefaultDecodable.swift` (#2528) via NachoSoto (@NachoSoto) * `CircleCI`: specify device to run `backend_integration_tests` (#2547) via NachoSoto (@NachoSoto) * Created `StoreKit2TransactionFetcher` (#2539) via NachoSoto (@NachoSoto) * Fix load shedder integration tests (#2546) via Josh Holtz (@joshdholtz) * Fix doc on `Offering.getMetadataValue` (#2545) via Josh Holtz (@joshdholtz) * Extracted and tested `AsyncSequence.extractValues` (#2538) via NachoSoto (@NachoSoto) * `Offline Entitlements`: don't compute offline `CustomerInfo` when purchasing a consumable products (#2522) via NachoSoto (@NachoSoto) * `OfflineEntitlementsManager`: disable offline `CustomerInfo` in observer mode (#2520) via NachoSoto (@NachoSoto) * `BasePurchasesTests`: fixed leak detection (#2534) via NachoSoto (@NachoSoto) * `PurchaseTesterSwiftUI`: added `ProxyView` to `iOS` (#2531) via NachoSoto (@NachoSoto) * `PurchasedProductsFetcher`: removed `AppStore.sync` call (#2521) via NachoSoto (@NachoSoto) * `PurchaseTesterSwiftUI`: added new window on Mac to manage proxy (#2518) via NachoSoto (@NachoSoto) * `PurchasedProductsFetcher`: added log if fetching purchased products is slow (#2515) via NachoSoto (@NachoSoto) * `Offline Entitlements`: disable for custom entitlements mode (#2509) via NachoSoto (@NachoSoto) * `Offline Entitlements`: fixed iOS 12 tests (#2514) via NachoSoto (@NachoSoto) * `PurchasedProductsFetcher`: don't throw errors if purchased products were found (#2506) via NachoSoto (@NachoSoto) * `Offline Entitlements`: allow creating offline `CustomerInfo` with empty `ProductEntitlementMapping` (#2504) via NachoSoto (@NachoSoto) * `Offline Entitlements`: integration tests (#2501) via NachoSoto (@NachoSoto) * `CustomerInfoManager`: don't cache offline `CustomerInfo` (#2378) via NachoSoto (@NachoSoto) * `DangerousSettings`: debug-only `forceServerErrors` (#2486) via NachoSoto (@NachoSoto) * `CocoapodsInstallation`: fixed `Xcode 14.3.0` issue (#2489) via NachoSoto (@NachoSoto) * `CarthageInstallation`: removed workaround (#2488) via NachoSoto (@NachoSoto) --------- Co-authored-by: NachoSoto <ignaciosoto90@gmail.com>

NachoSoto added the test label May 17, 2023

NachoSoto requested a review from a team May 17, 2023 19:00

NachoSoto force-pushed the nacho/sdk-2991-write-flow-to-use-locally-computed branch 2 times, most recently from 8e823a7 to 860d17b Compare May 17, 2023 19:32

NachoSoto force-pushed the offline-entitlements-integration-tests branch from 8417156 to c83d3d0 Compare May 17, 2023 19:33

NachoSoto force-pushed the nacho/sdk-2991-write-flow-to-use-locally-computed branch from 860d17b to 6cf0d99 Compare May 17, 2023 20:43

NachoSoto force-pushed the offline-entitlements-integration-tests branch 5 times, most recently from 31dd833 to 0f54435 Compare May 17, 2023 22:55

NachoSoto force-pushed the nacho/sdk-2991-write-flow-to-use-locally-computed branch from 6cf0d99 to 6cd570e Compare May 17, 2023 23:04

NachoSoto force-pushed the offline-entitlements-integration-tests branch from 0f54435 to f052941 Compare May 17, 2023 23:17

NachoSoto force-pushed the nacho/sdk-2991-write-flow-to-use-locally-computed branch from 0edce74 to 3719956 Compare May 17, 2023 23:27

NachoSoto force-pushed the offline-entitlements-integration-tests branch from f052941 to 58f528c Compare May 17, 2023 23:28

NachoSoto mentioned this pull request May 17, 2023

Offline Entitlements: use offline-computed CustomerInfo when server is down #2368

Merged

2 tasks

NachoSoto marked this pull request as ready for review May 17, 2023 23:35

NachoSoto commented May 17, 2023

View reviewed changes

Comment thread Sources/Networking/Operations/Handling/CustomerInfoResponseHandler.swift Outdated

NachoSoto force-pushed the offline-entitlements-integration-tests branch from 58f528c to 6fcca74 Compare May 17, 2023 23:41

NachoSoto commented May 17, 2023

View reviewed changes

Comment thread Tests/BackendIntegrationTests/OfflineStoreKitIntegrationTests.swift Outdated

winstondu reviewed May 18, 2023

View reviewed changes

Comment thread Tests/BackendIntegrationTests/BaseBackendIntegrationTests.swift Outdated

winstondu reviewed May 18, 2023

View reviewed changes

tonidero reviewed May 18, 2023

View reviewed changes

Comment thread Tests/BackendIntegrationTests/OfflineStoreKitIntegrationTests.swift Outdated

Comment thread Tests/BackendIntegrationTests/OfflineStoreKitIntegrationTests.swift Outdated

Comment thread Tests/BackendIntegrationTests/OfflineStoreKitIntegrationTests.swift Outdated

NachoSoto force-pushed the offline-entitlements-integration-tests branch from ee1d6bc to 82bda68 Compare May 18, 2023 17:19

NachoSoto force-pushed the nacho/sdk-2991-write-flow-to-use-locally-computed branch from 14d252b to 777318f Compare May 18, 2023 17:40

NachoSoto force-pushed the offline-entitlements-integration-tests branch 2 times, most recently from d6d52c7 to a44e3d1 Compare May 18, 2023 17:55

NachoSoto force-pushed the offline-entitlements-integration-tests branch from 3bd13cb to f72a183 Compare May 18, 2023 23:50

NachoSoto force-pushed the nacho/sdk-2991-write-flow-to-use-locally-computed branch from 6115e26 to 8459cb7 Compare May 18, 2023 23:52

NachoSoto force-pushed the offline-entitlements-integration-tests branch from f72a183 to d76b395 Compare May 18, 2023 23:52

tonidero approved these changes May 19, 2023

View reviewed changes

tonidero reviewed May 19, 2023

View reviewed changes

Comment thread Tests/BackendIntegrationTests/OfflineStoreKitIntegrationTests.swift Outdated

Base automatically changed from nacho/sdk-2991-write-flow-to-use-locally-computed to main May 19, 2023 17:27

NachoSoto added 12 commits May 19, 2023 10:29

Offline integration tests

51d25ff

Fixed crash in failing tests

2698b67

Improved name

a3f1b32

Added tests with no entitlement mapping

3c4762f

Fixed SK1 test

977b8e7

Added test for multiple purchases

7999f80

Added test verifying cache

599a020

Added more tests

993a396

Fixed test

2a4b921

Removed duplicate test

04f3ae8

Better method name

2bc158f

Fixed typo

fd03123

NachoSoto force-pushed the offline-entitlements-integration-tests branch from 09ad41b to fd03123 Compare May 19, 2023 17:29

NachoSoto enabled auto-merge (squash) May 19, 2023 17:31

NachoSoto merged commit e9e6d88 into main May 19, 2023

NachoSoto deleted the offline-entitlements-integration-tests branch May 19, 2023 17:54

RCGitBot mentioned this pull request May 24, 2023

[AUTOMATIC] Release/4.20.0 #2537

Closed

This was referenced May 31, 2023

[AUTOMATIC] Release/4.21.0 #2569

Closed

[AUTOMATIC] Release/4.21.0 #2571

Merged

NachoSoto mentioned this pull request May 31, 2023

Release/4.20.1 #2572

Closed

Conversation

NachoSoto commented May 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Example tests:

Uh oh!

NachoSoto May 17, 2023

Choose a reason for hiding this comment

Uh oh!

NachoSoto May 17, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

winstondu May 18, 2023

Choose a reason for hiding this comment

Uh oh!

NachoSoto May 18, 2023

Choose a reason for hiding this comment

Uh oh!

tonidero left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

NachoSoto commented May 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented May 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

tonidero left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tonidero May 19, 2023

Choose a reason for hiding this comment

Uh oh!

NachoSoto May 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tonidero May 19, 2023

Choose a reason for hiding this comment

Uh oh!

NachoSoto May 19, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

NachoSoto commented May 17, 2023 •

edited

Loading

NachoSoto commented May 18, 2023 •

edited

Loading

codecov Bot commented May 18, 2023 •

edited

Loading

NachoSoto May 19, 2023 •

edited

Loading