[WIP] Add DNSSEC for iOS 16#2561
Conversation
|
marking as draft because I only tested in unit tests and against a single device. Also we need to test this against RC Fortress. |
|
I'm really confused as to why tests for iOS 14 are failing compilation, but the ones for iOS 15 aren't, when the only relevant API is iOS 16 only |
|
Because the API isn't visible when compiling with Xcode 13.x |
| } | ||
| } | ||
|
|
||
| static func iOS16APIAvailableOrSkipTest() throws { |
|
|
||
| struct URLSessionConfigurationFactory: URLSessionConfigurationFactoryType { | ||
|
|
||
| func urlSessionConfiguration(requestTimeout: TimeInterval) -> URLSessionConfiguration { |
There was a problem hiding this comment.
This name is very Obj-C. How about just createConfiguration()? The rest is all in the types.
| config.timeoutIntervalForRequest = requestTimeout | ||
| config.timeoutIntervalForResource = requestTimeout | ||
| self.session = URLSession(configuration: config, | ||
| requestTimeout: TimeInterval = Configuration.networkTimeoutDefault, |
There was a problem hiding this comment.
I think this parameter isn't need anymore?
| import Foundation | ||
|
|
||
| protocol URLSessionConfigurationFactoryType { | ||
| func urlSessionConfiguration(requestTimeout: TimeInterval) -> URLSessionConfiguration |
There was a problem hiding this comment.
I don't care that much, but we should decide whether we want to keep the consistency in the style guide of spaces around types.
|
|
||
| class URLSessionConfigurationFactoryTests: TestCase { | ||
|
|
||
| var factory: URLSessionConfigurationFactory! |
There was a problem hiding this comment.
Nit: This can be private
| factory = URLSessionConfigurationFactory() | ||
| } | ||
|
|
||
| override func tearDown() { |
There was a problem hiding this comment.
This isn't really necessary
| } | ||
|
|
||
| @available(iOS 16.0, macOS 13.0, tvOS 16.0, watchOS 9.0, *) | ||
| func testRequiresDNSSECValidation() throws { |
|
|
||
| @testable import RevenueCat | ||
|
|
||
| class URLSessionConfigurationFactoryTests: TestCase { |
There was a problem hiding this comment.
This is awesome. Let's add a test to cover the change in #2668 too.
|
Should we close this? |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2561 +/- ##
==========================================
+ Coverage 87.85% 87.90% +0.04%
==========================================
Files 201 202 +1
Lines 13871 13878 +7
==========================================
+ Hits 12187 12200 +13
+ Misses 1684 1678 -6 ☔ View full report in Codecov by Sentry. |
Addresses #2560.
Sets DNSSEC for URLSession. I need to test more in detail, but so far it seems to work just fine with no surprises.
I also ended up doing a few things for the sake of testing: