Offline Entitlements: integration tests

[NachoSoto]

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]

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]

Changed this from DangerousSettings.InternalSettings to DangerousSettings.Internal.

[winstondu]

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.

[NachoSoto]

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]

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]

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]

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.

[NachoSoto]

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]

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

[NachoSoto]

That's done in testPurchaseWhileServerIsDownPostsReceiptAfterServerComesBack.