Integration Tests: workaround for XCTest crash after a test failure
#2532
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
See https://app.circleci.com/jobs/github/RevenueCat/purchases-ios/69881 Assertion failures in that method were leading to _crashes_ in `Purchases.shared`, instead of a simple test failure that could be retried. For some reason, `XCTest` is continuing execution after an assertion failure in an `async` method, despite having `self.continueAfterFailure = false`. This might even be a Swift mis-compilation. This PR is a workaround: instead of relying on `XCTest` stopping execution of the test, we instead throw an `Error` to force that at compile time. Note that this still produces nice errors when an assertion fails inside of `verifyEntitlementWentThrough`:
Codecov Report
@@ Coverage Diff @@
## main #2532 +/- ##
==========================================
- Coverage 87.86% 87.80% -0.06%
==========================================
Files 199 199
Lines 13699 13699
==========================================
- Hits 12036 12029 -7
- Misses 1663 1670 +7 |
tonidero
approved these changes
May 24, 2023
There was a problem hiding this comment.
Just a question but looks good! Seems weird that it continues executing after an assertion but 🤷♂️. Considering you said this happens in async/await functions, I wonder if this is a threading issue, like maybe the fail function changes threads at some point before failing?
| // Swift stops executing code when an assertion has failed, | ||
| // and therefore avoid code running after the test has already failed. | ||
| // This prevents test crashes from code calling `Purchases.shared` after the test has ended. | ||
| func failTest(_ message: String) async throws { |
There was a problem hiding this comment.
Is this happening in other tests?
There was a problem hiding this comment.
Not that I know. This particular case was happening a fair amount in CI though, because we use this method in pretty much all integration tests.
These tests are marked as |
NachoSoto
added a commit
that referenced
this pull request
May 25, 2023
…re (#2532) See https://app.circleci.com/jobs/github/RevenueCat/purchases-ios/69881 Assertion failures in that method were leading to _crashes_ in `Purchases.shared`, instead of a simple test failure that could be retried:  For some reason, `XCTest` is continuing execution after an assertion failure in an `async` method, despite having `self.continueAfterFailure = false`. This might even be a Swift mis-compilation. This PR is a workaround: instead of relying on `XCTest` stopping execution of the test, we instead throw an `Error` to force that at compile time. Note that this still produces nice errors when an assertion fails inside of `verifyEntitlementWentThrough`: 
NachoSoto
added a commit
that referenced
this pull request
May 31, 2023
Fixes https://app.circleci.com/pipelines/github/RevenueCat/purchases-ios/11501/workflows/b978f26f-ce2d-4040-ac40-96e121c4407f/jobs/72997/steps Same as #2532, this prevents test crashes when this function is called after `Purchases` has been reset. Test crashes are inconvenient because it makes the entire test run fail, instead of allowing retries.
NachoSoto
added a commit
that referenced
this pull request
Jun 1, 2023
Fixes https://app.circleci.com/pipelines/github/RevenueCat/purchases-ios/11501/workflows/b978f26f-ce2d-4040-ac40-96e121c4407f/jobs/72997/steps Same as #2532, this prevents test crashes when this function is called after `Purchases` has been reset. Test crashes are inconvenient because it makes the entire test run fail, instead of allowing retries.
NachoSoto
added a commit
that referenced
this pull request
Jul 24, 2023
…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
added a commit
that referenced
this pull request
Jul 24, 2023
…perty (#2867) This is a similar change to #2532. I noticed yet another source of crashes in our integration tests. This is what [the result](https://app.circleci.com/pipelines/github/RevenueCat/purchases-ios/13247/workflows/52fd2014-7a19-442f-a572-64116393abec/jobs/94728) 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 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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See https://app.circleci.com/jobs/github/RevenueCat/purchases-ios/69881
Assertion failures in that method were leading to crashes in
Purchases.shared, instead of a simple test failure that could be retried:For some reason,
XCTestis continuing execution after an assertion failure in anasyncmethod, despite havingself.continueAfterFailure = false.This might even be a Swift mis-compilation. This PR is a workaround: instead of relying on
XCTeststopping execution of the test, we instead throw anErrorto force that at compile time.Note that this still produces nice errors when an assertion fails inside of
verifyEntitlementWentThrough: