Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BasePurchasesTests: fixed leak detection #2534

Merged
merged 2 commits into from
May 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions Sources/Logging/Strings/PurchaseStrings.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ enum PurchaseStrings {
case storekit1_wrapper_deinit(StoreKit1Wrapper)
case device_cache_init(DeviceCache)
case device_cache_deinit(DeviceCache)
case purchases_orchestrator_init(PurchasesOrchestrator)
case purchases_orchestrator_deinit(PurchasesOrchestrator)
case updating_all_caches
case cannot_purchase_product_appstore_configuration_error
case entitlements_revoked_syncing_purchases(productIdentifiers: [String])
Expand Down Expand Up @@ -92,6 +94,12 @@ extension PurchaseStrings: CustomStringConvertible {
case let .device_cache_deinit(instance):
return "DeviceCache.deinit: \(Strings.objectDescription(instance))"

case let .purchases_orchestrator_init(instance):
return "PurchasesOrchestrator.init: \(Strings.objectDescription(instance))"

case let .purchases_orchestrator_deinit(instance):
return "PurchasesOrchestrator.deinit: \(Strings.objectDescription(instance))"

case .updating_all_caches:
return "Updating all caches"

Expand Down
6 changes: 6 additions & 0 deletions Sources/Purchasing/Purchases/PurchasesOrchestrator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,12 @@ final class PurchasesOrchestrator {
self.offeringsManager = offeringsManager
self.manageSubscriptionsHelper = manageSubscriptionsHelper
self.beginRefundRequestHelper = beginRefundRequestHelper

Logger.verbose(Strings.purchase.purchases_orchestrator_init(self))
}

deinit {
Logger.verbose(Strings.purchase.purchases_orchestrator_deinit(self))
}

func restorePurchases(completion: (@Sendable (Result<CustomerInfo, PurchasesError>) -> Void)?) {
Expand Down
90 changes: 46 additions & 44 deletions Tests/UnitTests/Purchasing/Purchases/BasePurchasesTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -101,54 +101,20 @@ class BasePurchasesTests: TestCase {
// this level it should be moved to `StoreKitUnitTests`, which runs serially.
Purchases.logLevel = .verbose

// See `addTeardownBlock` docs:
// - These run *before* `tearDown`.
// - They run in LIFO order.
self.addTeardownBlock {
expect { [weak purchases = self.purchases] in purchases }
.toEventually(beNil(), description: "Purchases has leaked")
}
self.addTeardownBlock {
expect { [weak orchestrator = self.purchasesOrchestrator] in orchestrator }
.toEventually(beNil(), description: "PurchasesOrchestrator has leaked")
}
self.addTeardownBlock {
expect { [weak deviceCache = self.deviceCache] in deviceCache }
.toEventually(beNil(), description: "DeviceCache has leaked: \(self)")
}
weak var purchases = self.purchases
weak var orchestrator = self.purchasesOrchestrator
weak var deviceCache = self.deviceCache
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we rename these 3 so there is less confusion with the global variables?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You'd rename them to weakPurchases, etc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They're limited to the scope of this method so I think it's clear that they're shadowing those variables. But let me know if you disagree.


self.addTeardownBlock {
Purchases.clearSingleton()
self.clearReferences()

self.mockOperationDispatcher = nil
self.paymentQueueWrapper = nil
self.requestFetcher = nil
self.receiptFetcher = nil
self.mockProductsManager = nil
self.mockIntroEligibilityCalculator = nil
self.mockTransactionsManager = nil
self.backend = nil
self.attributionFetcher = nil
self.purchasesDelegate.makeDeferredPurchase = nil
self.purchasesDelegate = nil
self.storeKit1Wrapper.delegate = nil
self.storeKit1Wrapper = nil
self.systemInfo = nil
self.notificationCenter = nil
self.subscriberAttributesManager = nil
self.trialOrIntroPriceEligibilityChecker = nil
self.attributionPoster = nil
self.attribution = nil
self.customerInfoManager = nil
self.identityManager = nil
self.mockOfferingsManager = nil
self.mockOfflineEntitlementsManager = nil
self.mockPurchasedProductsFetcher = nil
self.mockManageSubsHelper = nil
self.mockBeginRefundRequestHelper = nil
self.purchasesOrchestrator = nil
self.deviceCache = nil
self.purchases = nil
expect(purchases)
.toEventually(beNil(), description: "Purchases has leaked")
expect(orchestrator)
.toEventually(beNil(), description: "PurchasesOrchestrator has leaked")
expect(deviceCache)
.toEventually(beNil(), description: "DeviceCache has leaked: \(self)")
}
}

Expand Down Expand Up @@ -498,3 +464,39 @@ extension OfferingsResponse {
)

}

private extension BasePurchasesTests {

func clearReferences() {
self.mockOperationDispatcher = nil
self.paymentQueueWrapper = nil
self.requestFetcher = nil
self.receiptFetcher = nil
self.mockProductsManager = nil
self.mockIntroEligibilityCalculator = nil
self.mockTransactionsManager = nil
self.backend = nil
self.attributionFetcher = nil
self.purchasesDelegate.makeDeferredPurchase = nil
self.purchasesDelegate = nil
self.storeKit1Wrapper.delegate = nil
self.storeKit1Wrapper = nil
self.systemInfo = nil
self.notificationCenter = nil
self.subscriberAttributesManager = nil
self.trialOrIntroPriceEligibilityChecker = nil
self.attributionPoster = nil
self.attribution = nil
self.customerInfoManager = nil
self.identityManager = nil
self.mockOfferingsManager = nil
self.mockOfflineEntitlementsManager = nil
self.mockPurchasedProductsFetcher = nil
self.mockManageSubsHelper = nil
self.mockBeginRefundRequestHelper = nil
self.purchasesOrchestrator = nil
self.deviceCache = nil
self.purchases = nil
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ class PurchasesConfiguringTests: BasePurchasesTests {
let purchases = Purchases.configure(
with: .init(withAPIKey: "")
// This test requires no previously stored user
.with(userDefaults: .init(suiteName: UUID().uuidString)!)
.with(userDefaults: .emptyNewUserDefaults())
.with(appUserID: "")
)
expect(purchases.appUserID).toNot(beEmpty())
Expand All @@ -120,7 +120,7 @@ class PurchasesConfiguringTests: BasePurchasesTests {

func testUserIdOverridesPreviouslyConfiguredUser() {
// This test requires no previously stored user
let userDefaults: UserDefaults = .init(suiteName: UUID().uuidString)!
let userDefaults: UserDefaults = .emptyNewUserDefaults()

let newUserID = Self.appUserID + "_new"

Expand All @@ -141,7 +141,7 @@ class PurchasesConfiguringTests: BasePurchasesTests {

func testNilUserIdIsIgnoredIfPreviousUserExists() {
// This test requires no previously stored user
let userDefaults: UserDefaults = .init(suiteName: UUID().uuidString)!
let userDefaults: UserDefaults = .emptyNewUserDefaults()

_ = Purchases.configure(
with: .init(withAPIKey: "")
Expand Down Expand Up @@ -408,13 +408,15 @@ class PurchasesConfiguringTests: BasePurchasesTests {
}

func testConfigureWithCustomEntitlementComputationFatalErrorIfNoAppUserID() throws {
self.systemInfo = MockSystemInfo(finishTransactions: true,
customEntitlementsComputation: true)

let expectedMessage = Strings.configure.custom_entitlements_computation_enabled_but_no_app_user_id.description

expectFatalError(expectedMessage: expectedMessage) {
self.setupAnonPurchases()
_ = Purchases(apiKey: "",
appUserID: nil,
userDefaults: .emptyNewUserDefaults(),
observerMode: false,
responseVerificationMode: .default,
dangerousSettings: .init(customEntitlementComputation: true))
}
}

Expand Down Expand Up @@ -498,3 +500,11 @@ class PurchasesConfiguringTests: BasePurchasesTests {
private static let customUserDefaults: UserDefaults = .init(suiteName: "com.revenuecat.testing_user_defaults")!

}

private extension UserDefaults {

static func emptyNewUserDefaults() -> Self {
return .init(suiteName: UUID().uuidString)!
}

}