Fix a few flaky tests #3833
Fix a few flaky tests #3833
Conversation
aboedo
force-pushed
the
andy/fix_some_flaky_tests
branch
from
a0782cd
to
0f01ba2
Compare
Generated by 🚫 Danger |
…anual waits with accelerated times
joshdholtz
force-pushed
the
andy/fix_some_flaky_tests
branch
from
705a78c
to
9614a4d
Compare
Tests/BackendIntegrationTests/BaseStoreKitIntegrationTests.swift
Outdated
Show resolved
Hide resolved
|
|
||
| try self.testSession.forceRenewalOfSubscription(productIdentifier: productIdentifier) | ||
| // swiftlint:disable:next force_try | ||
| try! await Task.sleep(nanoseconds: 3 * 1_000_000_000) |
There was a problem hiding this comment.
I never thought I'd say this, but sleeping in this test actually makes more sense than relying on the old flaky StoreKit behavior 😂
| @@ -241,16 +243,23 @@ class OfflineStoreKit1IntegrationTests: BaseOfflineStoreKitIntegrationTests { | |||
|
|
|||
| @available(iOS 15.0, tvOS 15.0, watchOS 8.0, macOS 12.0, *) | |||
| func testCallToGetCustomerInfoWithPendingTransactionsPostsReceiptOnlyOnce() async throws { | |||
| // forceRenewalOfSubscription doesn't work well, so we use this instead | |||
| if #available(iOS 16.4, *) { | |||
There was a problem hiding this comment.
It looks like we have this code duplicated in several tests, could it be abstracted out to a common function in BaseStoreKitIntegrationTests?
| @@ -641,21 +683,21 @@ class StoreKit1IntegrationTests: BaseStoreKitIntegrationTests { | |||
| await waitForNewPurchaseDate() | |||
|
|
|||
| // 4. Expire subscription | |||
| try await self.expireSubscription(entitlement) | |||
| // try await self.expireSubscription(entitlement) | |||
There was a problem hiding this comment.
Is commenting out the latter part of this test intentional?
There was a problem hiding this comment.
Nope! I noticed this a bit ago and am fixing 😛
Tests/StoreKitUnitTests/StoreKit2/StoreKit2TransactionListenerTests.swift
Outdated
Show resolved
Hide resolved
| if #available(iOS 16.4, *) { | ||
| self.testSession.timeRate = .oneRenewalEveryTwoSeconds | ||
| } else { | ||
| self.testSession.timeRate = SKTestSession.TimeRate.monthlyRenewalEveryThirtySeconds |
There was a problem hiding this comment.
Will this fail if we use SKTestSession.TimeRate.monthlyRenewalEveryThirtySeconds on iOS <16.4 and then sleep for 3 seconds later in the test?
There was a problem hiding this comment.
@fire-at-will We only run backend tests on iOS 17 right now so... odds are this will never happen 😛
|
@joshdholtz Looks good! I had a few suggestions/questions and then I think it'll be good to go! |
| try await self.purchaseShortestDuration(allowOfflineEntitlements: true) | ||
|
|
||
| // swiftlint:disable:next force_try | ||
| try! await Task.sleep(nanoseconds: 3 * 1_000_000_000) |
There was a problem hiding this comment.
The 3 * 1_000_000_000 seems to be used in a few tests. Could we extract it somewhere?
| RETRY_SCAN_SAVE_FAILED_TESTS_CUSTOM_VALUE = :RETRY_SCAN_SAVE_FAILED_TESTS_CUSTOM_VALUE | ||
| end | ||
|
|
||
| class RetryScanSaveFailedTestsAction < Action |
There was a problem hiding this comment.
Wonder if we should move this to the plugin, so it can also be tested... But since I think it will only be used here for now, I think this is ok 👍
This is "ready for review" but it requires #3833 to be merged into it first to fully pass ### Motivation Prevent the need to fully rerun all backend tests job when only a few flaky tests fail ### Description 1. Runs backend integration tests - Parses junit.xml for failed tests - Writes them to a file 2. Runs new step in CircleCI - Looks at file for tests to re-run and only runs those tests - Merges new junit.xml into original junit.xml The retry happens 4 times right now and will fail (raise an error) if there are any left over tests that never ended up passing --------- Co-authored-by: Will Taylor <wtaylor151@gmail.com> Co-authored-by: Andy Boedo <andresboedo@gmail.com>
Looking into our flaky tests, I noticed that calls to forceRenewalOfSubscription don't always trigger calls from
StoreKit.product.updates, it's flaky as hell.I tried instead using an accelerated timeRate value and adding explicit waits, and that seems to consistently work in local testing. Opening this up to see if this helps with the flakiness of these particular tests.
Notes from Josh
swiftlintfix from Fix lint and CI with latestswiftlintversion (0.55.1) #3909