Skip to content

IntegrationTests: replaced Purchases.shared with a throwing property#2867

Merged
NachoSoto merged 1 commit into
mainfrom
integration-test-shared-crash
Jul 24, 2023
Merged

IntegrationTests: replaced Purchases.shared with a throwing property#2867
NachoSoto merged 1 commit into
mainfrom
integration-test-shared-crash

Conversation

@NachoSoto

@NachoSoto NachoSoto commented Jul 24, 2023

Copy link
Copy Markdown
Contributor

This is a similar change to #2532.

I noticed yet another source of crashes in our integration tests.
This is what the result was:

failed - Message 'Finishing transaction' should not have been logged
expected to not find object in collection that satisfies predicate

That came from this code:

self.logger.verifyMessageWasNotLogged("Finishing transaction")

let info1 = try await Purchases.shared.customerInfo()

After that assertion failure, XCTest continued execution, while it had already tear down Purchases, which lead to this crash:

Thread 0 Crashed:
0   libswiftCore.dylib            	       0x103b84e81 _assertionFailure(_:_:file:line:flags:) + 353
1   RevenueCat                    	       0x1332516d0 static Purchases.shared.getter + 352 (Purchases.swift:65)
2   BackendIntegrationTests       	       0x130f7048a OfflineStoreKit1IntegrationTests.testPurchaseWhileServerIsDownPostsReceiptWhenForegroundingApp() + 362 (OfflineStoreKitIntegrationTests.swift:289)
3   BackendIntegrationTests       	       0x130f75ee1 @objc closure #1 in OfflineStoreKit1IntegrationTests.testPurchaseWhileServerIsDownPostsReceiptWhenForegroundingApp() + 1
4   BackendIntegrationTests       	       0x130f77d91 partial apply for @objc closure #1 in OfflineStoreKit1IntegrationTests.testPurchaseWhileServerIsDownPostsReceiptWhenForegroundingApp() + 1

To prevent this, instead of using Purchases.shared this replaces that with a throwing self.purchases.
Just like we don't use force-unwraps in tests and instead use XCTUwnrap, this now will fail the test instead of crashing it.

@NachoSoto NachoSoto added the test label Jul 24, 2023
@NachoSoto NachoSoto requested a review from a team July 24, 2023 10:26
Comment on lines 18 to 22

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.

This was missing too.

…perty

This is a similar change to #2532.

I noticed yet another source of crashes in our integration tests.
This is what the result was:
```
failed - Message 'Finishing transaction' should not have been logged
expected to not find object in collection that satisfies predicate
```

That came from this code:
```swift
self.logger.verifyMessageWasNotLogged("Finishing transaction")

let info1 = try await Purchases.shared.customerInfo()
```

After that assertion failure, `XCTest` continued execusion, while it had already tear down `Purchases`, which lead to this crash:
```
Thread 0 Crashed:
0   libswiftCore.dylib            	       0x103b84e81 _assertionFailure(_:_:file:line:flags:) + 353
1   RevenueCat                    	       0x1332516d0 static Purchases.shared.getter + 352 (Purchases.swift:65)
2   BackendIntegrationTests       	       0x130f7048a OfflineStoreKit1IntegrationTests.testPurchaseWhileServerIsDownPostsReceiptWhenForegroundingApp() + 362 (OfflineStoreKitIntegrationTests.swift:289)
3   BackendIntegrationTests       	       0x130f75ee1 @objc closure #1 in OfflineStoreKit1IntegrationTests.testPurchaseWhileServerIsDownPostsReceiptWhenForegroundingApp() + 1
4   BackendIntegrationTests       	       0x130f77d91 partial apply for @objc closure #1 in OfflineStoreKit1IntegrationTests.testPurchaseWhileServerIsDownPostsReceiptWhenForegroundingApp() + 1
```

To prevent this, instead of using `Purchases.shared` this replaces that with a throwing `self.purchases`.
Just like we don't use force-unwraps in tests and instead use `XCTUwnrap`, this now will fail the test instead of crashing it.
@NachoSoto NachoSoto force-pushed the integration-test-shared-crash branch from a513e03 to f662094 Compare July 24, 2023 10:28
@NachoSoto NachoSoto enabled auto-merge (squash) July 24, 2023 10:59
@NachoSoto NachoSoto merged commit e182a6a into main Jul 24, 2023
@NachoSoto NachoSoto deleted the integration-test-shared-crash branch July 24, 2023 11:20
NachoSoto added a commit that referenced this pull request Jul 26, 2023
**This is an automatic release.**

_This release is compatible with Xcode 15 beta 5 and visionOS beta 2_

### Bugfixes
* `xrOS`: fixed `SubscriptionStoreView` for visionOS beta 2 (#2884) via
Josh Holtz (@joshdholtz)
### Performance Improvements
* `Perf`: update `CustomerInfo` cache before anything else (#2865) via
NachoSoto (@NachoSoto)
### Other Changes
* `SimpleApp`: added support for localization (#2880) via NachoSoto
(@NachoSoto)
* `TestStoreProduct`: made available on release builds (#2861) via
NachoSoto (@NachoSoto)
* `Tests`: increased default logger capacity (#2870) via NachoSoto
(@NachoSoto)
* `CustomEntitlementComputation`: removed `invalidateCustomerInfoCache`
(#2866) via NachoSoto (@NachoSoto)
* `SimpleApp`: updates for TestFlight compatibility (#2862) via
NachoSoto (@NachoSoto)
* `BasePurchasesTests`: consolidate to only initialize one `DeviceCache`
(#2863) via NachoSoto (@NachoSoto)
* `Codable`: debug log entire JSON when decoding fails (#2864) via
NachoSoto (@NachoSoto)
* `IntegrationTests`: replaced `Purchases.shared` with a `throw`ing
property (#2867) via NachoSoto (@NachoSoto)
* `NetworkError`: 2 new tests to ensure underlying error is included in
description (#2843) via NachoSoto (@NachoSoto)
* Add SPM `Package.resolved` for Xcode Cloud (#2844) via NachoSoto
(@NachoSoto)
* `CustomEntitlementComputation`: added integration test for
cancellations (#2849) via NachoSoto (@NachoSoto)
* `CustomEntitlementComputation`: removed
`syncPurchases`/`restorePurchases` (#2854) via NachoSoto (@NachoSoto)

---------

Co-authored-by: NachoSoto <ignaciosoto90@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants