Encodable.jsonEncodedData: fixed tests on iOS 17 due to inconsistent key ordering
#2607
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
Merged
Codecov Report
@@ Coverage Diff @@
## main #2607 +/- ##
==========================================
- Coverage 86.29% 86.26% -0.03%
==========================================
Files 205 205
Lines 14334 14328 -6
==========================================
- Hits 12369 12360 -9
- Misses 1965 1968 +3 |
tonidero
approved these changes
Jun 9, 2023
NachoSoto
changed the title
JSONEncoding: add tests to cover Xcode 15 regressionJSONEncoding: sort keys to ensure data consistency
|
That issue was closed, which confirms that we do indeed need to sort keys to ensure consistent results. |
These and a bunch of other tests are failing on Xcode 15 due to [a regression](swiftlang/swift#66466). To make the other failures more explicit, I've added new comparisons to ensure that both the `CustomerInfoResponse` and underlying `Data` are encoded equally. This breaks tests because we relying on being able to verify that the encoded `Data` saved to caches is consistent. A potential workaround (if they don't fix that regression) will be to force key ordering: ```diff diff --git i/Sources/LocalReceiptParsing/DataConverters/Codable+Extensions.swift w/Sources/LocalReceiptParsing/DataConverters/Codable+Extensions.swift index 025419d6..2d43b7a5 100644 --- i/Sources/LocalReceiptParsing/DataConverters/Codable+Extensions.swift +++ w/Sources/LocalReceiptParsing/DataConverters/Codable+Extensions.swift @@ -53,6 +53,7 @@ extension JSONEncoder { let encoder = JSONEncoder() encoder.keyEncodingStrategy = .convertToSnakeCase encoder.dateEncodingStrategy = .iso8601 + encoder.outputFormatting = .sortedKeys return encoder }() @@ -61,7 +62,7 @@ extension JSONEncoder { let encoder = JSONEncoder() encoder.keyEncodingStrategy = .convertToSnakeCase encoder.dateEncodingStrategy = .iso8601 - encoder.outputFormatting = .prettyPrinted + encoder.outputFormatting = [.sortedKeys, .prettyPrinted] return encoder }() ```
NachoSoto
force-pushed
the
json-encoding-test-regression
branch
from
ee29e79 to
07f1342
Compare
NachoSoto
changed the title
JSONEncoding: sort keys to ensure data consistencyEncodable.jsonEncodedData: fixed tests on iOS 17 due to inconsistent key ordering
NachoSoto
force-pushed
the
json-encoding-test-regression
branch
from
07f1342 to
d82ca8e
Compare
|
@tonidero mind taking another look? I reworked this PR to actually fix the issue, now that I got a response and we know it's not really a regression. |
This was referenced Jun 13, 2023
NachoSoto
pushed a commit
that referenced
this pull request
Jun 13, 2023
**This is an automatic release.** ### New Features * New `DebugViewController`: UIKit counterpart for SwiftUI's `debugRevenueCatOverlay` (#2631) via NachoSoto (@NachoSoto) * Created `PaywallExtensions`: `StoreView` and `SubscriptionStoreView` overloads for `Offering` (#2593) via NachoSoto (@NachoSoto) * Introduced `debugRevenueCatOverlay()`: new SwiftUI debug overlay (#2567) via NachoSoto (@NachoSoto) ### Bugfixes * Removed `preventPurchasePopupCallFromTriggeringCacheRefresh`, update caches on `willEnterForeground` (#2623) via NachoSoto (@NachoSoto) * Fixed `Catalyst` build with `Xcode 15 beta 1` (#2586) via NachoSoto (@NachoSoto) ### Dependency Updates * Bump danger from 9.3.0 to 9.3.1 (#2592) via dependabot[bot] (@dependabot[bot]) ### Other Changes * `StoreTransaction`: added new `Storefront` to API testers (#2634) via NachoSoto (@NachoSoto) * `DebugView`: added snapshot tests (#2630) via NachoSoto (@NachoSoto) * `verifyNoUnfinishedTransactions`/`verifyUnfinishedTransaction`: added missing `#file` parameter (#2625) via NachoSoto (@NachoSoto) * `PostReceiptDataOperation`: clean up cache key (#2628) via NachoSoto (@NachoSoto) * `PurchasesOrchestrator`: also get `Storefront` from SK1 (#2629) via NachoSoto (@NachoSoto) * `CI`: disable iOS 17 for now (#2627) via NachoSoto (@NachoSoto) * `Tests`: fixed crash on iOS 13 (#2624) via NachoSoto (@NachoSoto) * `StoreTransaction`: read `Storefront` from `StoreKit.Transaction` (#2611) via NachoSoto (@NachoSoto) * `StoreKitConfigTestCase`/`BaseStoreKitIntegrationTests`: also clear transactions after every test (#2616) via NachoSoto (@NachoSoto) * `ErrorCode.networkError`: improved description (#2610) via NachoSoto (@NachoSoto) * `PurchaseTester`: make CI job always point to current version (#2622) via NachoSoto (@NachoSoto) * Improved `finishAllUnfinishedTransactions` (#2615) via NachoSoto (@NachoSoto) * `StoreKitConfigTestCase`: improved `waitForStoreKitTestIfNeeded` (#2614) via NachoSoto (@NachoSoto) * `StoreKitConfigTestCase`: set `continueAfterFailure` to `false` (#2617) via NachoSoto (@NachoSoto) * `PaywallExtensions`: fixed compilation (#2613) via NachoSoto (@NachoSoto) * `CI`: added `iOS 17` job (#2591) via NachoSoto (@NachoSoto) * `Encodable.jsonEncodedData`: fixed tests on iOS 17 due to inconsistent key ordering (#2607) via NachoSoto (@NachoSoto) * `debugRevenueCatOverlay`: added ability to display new `SubscriptionStoreView` (#2595) via NachoSoto (@NachoSoto) * Refactor: extracted all log strings (#2600) via NachoSoto (@NachoSoto) * Changed tests to work around `URL` decoding differences in `iOS 17` (#2605) via NachoSoto (@NachoSoto) * Removed unnecessary `Strings.trimmedOrError` (#2601) via NachoSoto (@NachoSoto) * Fixed test compilation with `Xcode 15` (#2602) via NachoSoto (@NachoSoto) * Tests: added `iOS 17` snapshots (#2603) via NachoSoto (@NachoSoto) * `StoreProductDiscount`: added `description` (#2604) via NachoSoto (@NachoSoto) * `debugRevenueCatOverlay` improvements (#2594) via NachoSoto (@NachoSoto) * `Xcode 15`: fixed all documentation warnings (#2596) via NachoSoto (@NachoSoto) * `StoreKitObserverModeIntegrationTests`: fixed and disabled SK2 `testPurchaseInDevicePostsReceipt` (#2589) via NachoSoto (@NachoSoto) * `StoreKit2TransactionListener`: added log when receiving `Transactions.Updates` (#2588) via NachoSoto (@NachoSoto) * `Dictionary.MergeStrategy`: simplify implementation (#2587) via NachoSoto (@NachoSoto) * `Configuration.Builder`: fixed doc reference (#2583) via NachoSoto (@NachoSoto) * `APITesters`: available since iOS 11 (#2581) 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.
A bunch of tests were failing on Xcode 15 due to a change in Swift, because we relying on being able to verify that the encoded
Datasaved to caches is consistent.Starting on Swift 5.9
JSONEncoderhas optimizations that mean we can't rely on it.To fix this, this PR does 2 things:
JSONEncoder.prettyPrintedto have.sortedKeysstruct, not the raw encodedDataNote that I didn't make
JSONEncoder.defaultuse.sortedKeysbecause of the potential performance impact.Encodable.prettyPrintedDatais used in tests likeReceiptFetcherTests(which this PR fixes) and for debug purposes, butEncodable.encodedJSONis used everywhere.