OperationQueue: log debug message when requests are found in cache and skipped
#1926
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
aboedo
approved these changes
Sep 21, 2022
NachoSoto
force-pushed
the
operation-queue-skip-log
branch
from
2ac0a17
to
3a5f1df
Compare
Merged
NachoSoto
force-pushed
the
operation-queue-skip-log
branch
2 times, most recently
from
29d6746
to
794e649
Compare
…and skipped Right now there is no way to know when a request is being reused. If this were to lead to a hard to debug issue, at lease we'll now have evidence for this in the logs.
NachoSoto
force-pushed
the
operation-queue-skip-log
branch
from
794e649
to
f414b32
Compare
NachoSoto
added a commit
that referenced
this pull request
Sep 23, 2022
Turns out we hadn't been running tests on `tvOS` for a while: > [19:08:22]: Couldn't find any matching simulators for '["Apple TV (15.2)"]' - falling back to default simulator > [19:08:22]: Found simulator "iPhone 13 Pro (15.5)" I found this went I went to dig into [this failure](#1926 (comment)). Turns out that wasn't running on `tvOS` anyway. This turns them back on, fixes the compilation issues on `tvOS` tests, and ensures that we don't run into this issue again. ### Changes: - Renamed device in lane `test_tvos` - Enabled `ensure_devices_found` on `iOS` and `tvOS` lanes. This will also ensure that we don't stop running tests on iOS XX because it doesn't find that simulator. - Removed `UnitTestHostApp` storyboards and scene, since those aren't supported in `tvOS`, and aren't needed because there's no UI. - Made `StoreKitUnitTests` and `UnitTestHostApp` targets universal - No longer linking `AdServices` on `tvOS` in test targets - Disabled attribution tests on `tvOS`. Those methods weren't being compiled, but we were testing that a message was being sent, even though it no longer was. - Changed iOS 14 tests to use Xcode 13 due to xcpretty/xcode-install#466 (same as #1918).
NachoSoto
added a commit
that referenced
this pull request
Sep 23, 2022
Turns out we hadn't been running tests on `tvOS` for a while: > [19:08:22]: Couldn't find any matching simulators for '["Apple TV (15.2)"]' - falling back to default simulator > [19:08:22]: Found simulator "iPhone 13 Pro (15.5)" I found this went I went to dig into [this failure](#1926 (comment)). Turns out that wasn't running on `tvOS` anyway. This turns them back on, fixes the compilation issues on `tvOS` tests, and ensures that we don't run into this issue again. ### Changes: - Renamed device in lane `test_tvos` - Enabled `ensure_devices_found` on `iOS` and `tvOS` lanes. This will also ensure that we don't stop running tests on iOS XX because it doesn't find that simulator. - Removed `UnitTestHostApp` storyboards and scene, since those aren't supported in `tvOS`, and aren't needed because there's no UI. - Made `StoreKitUnitTests` and `UnitTestHostApp` targets universal - No longer linking `AdServices` on `tvOS` in test targets - Disabled attribution tests on `tvOS`. Those methods weren't being compiled, but we were testing that a message was being sent, even though it no longer was. - Changed iOS 14 tests to use Xcode 13 due to xcpretty/xcode-install#466 (same as #1918).
NachoSoto
added a commit
that referenced
this pull request
Sep 23, 2022
The test introduced by #1926 passed locally, but sometimes failed in CI due to a race condition. What we're testing is that the second request reuses the callback of the first one, **if called while the first one is still in progress**. However, nothing in the test ensures that's the case. This introduces an optional delay in `MockHTTPClient` so we can ensure that. _Note: because of the nature of these 2 tests, they don't actually wait for the whole response to finish, so they still pass in ~0.006s_
NachoSoto
added a commit
that referenced
this pull request
Sep 23, 2022
The test introduced by #1926 passed locally, but sometimes failed in CI due to a race condition. What we're testing is that the second request reuses the callback of the first one, **if called while the first one is still in progress**. However, nothing in the test ensures that's the case. This introduces an optional delay in `MockHTTPClient` so we can ensure that. Another reason that test was failing randomly is because we had another test modifying global state, which interferes with every other test. We run tests in parallel, so it was making the new test that requires `debug` logs to fail randomly, if this test happened to run before. _Note: because of the nature of these 2 tests, they don't actually wait for the whole response to finish, so they still pass in ~0.006s_
Merged
NachoSoto
added a commit
that referenced
this pull request
Sep 23, 2022
NachoSoto
added a commit
that referenced
this pull request
Sep 27, 2022
**This is an automatic release.** ### New Features * 🚨 `StoreKit 2` is now enabled by default 🚨 (#1922) via NachoSoto (@NachoSoto) * Extracted `PurchasesType` and `PurchasesSwiftType` (#1912) via NachoSoto (@NachoSoto) ### Bugfixes * `StoreKit 1`: changed result of cancelled purchases to be consistent with `StoreKit 2` (#1910) via NachoSoto (@NachoSoto) * `PaymentQueueWrapper`: handle promotional purchase requests from App Store when SK1 is disabled (#1901) via NachoSoto (@NachoSoto) ### Other Changes * Fixed iOS 12 tests (#1936) via NachoSoto (@NachoSoto) * `CacheableNetworkOperation`: fixed race condition in new test (#1932) via NachoSoto (@NachoSoto) * `BasePurchasesTests`: changed default back to SK1 (#1935) via NachoSoto (@NachoSoto) * `Logger`: refactored default `LogLevel` definition (#1934) via NachoSoto (@NachoSoto) * `AppleReceipt`: refactored declarations into nested types (#1933) via NachoSoto (@NachoSoto) * `Integration Tests`: relaunch tests when retrying failures (#1925) via NachoSoto (@NachoSoto) * `CircleCI`: downgraded release jobs to Xcode 13.x (#1927) via NachoSoto (@NachoSoto) * `ErrorUtils`: added test to verify that `PublicError`s can be `catch`'d as `ErrorCode` (#1924) via NachoSoto (@NachoSoto) * `StoreKitIntegrationTests`: print `AppleReceipt` data whenever `verifyEntitlementWentThrough` fails (#1929) via NachoSoto (@NachoSoto) * `OperationQueue`: log debug message when requests are found in cache and skipped (#1926) via NachoSoto (@NachoSoto) * `GetCustomerInfoAPI`: avoid making a request if there's any `PostReceiptDataOperation` in progress (#1911) via NachoSoto (@NachoSoto) * `PurchaseTester`: allow HTTP requests and enable setting `ProxyURL` (#1917) via NachoSoto (@NachoSoto)
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.
Right now there is no way to know when a request is being reused. If this were to lead to a hard to debug issue, at lease we'll now have evidence for this in the logs.