Mock URLSession Behavior for Testing #11
Merged
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
mliberatore
commented
May 9, 2023
| let behaviors = defaultRequestBehaviors + requestBehaviors | ||
|
|
||
| let urlRequest = makeFinalizedRequest(fromOriginalRequest: request.urlRequest, behaviors: behaviors) | ||
| let dataTask = makeDataTask(forURLRequest: urlRequest, successHTTPStatusCodes: request.successHTTPStatusCodes, completion: completion) | ||
| let dataTask = makeDataTask(forURLRequest: urlRequest, behaviors: behaviors, successHTTPStatusCodes: request.successHTTPStatusCodes, completion: completion) |
There was a problem hiding this comment.
Our test caught this bug that must've been around for a while. This change ensures that the behaviors’ requestDidFinish will get called, whereas before, it was not 😱
mliberatore
commented
May 9, 2023
| @@ -101,7 +101,7 @@ extension NetworkController: NetworkRequestPerformer { | |||
|
|
|||
| public func send(_ request: any NetworkRequest, requestBehaviors: [RequestBehavior]) async throws -> NetworkResponse { | |||
| try await withCheckedThrowingContinuation { continuation in | |||
| send(request) { result in | |||
| send(request, requestBehaviors: requestBehaviors) { result in | |||
There was a problem hiding this comment.
Similarly, for our new async/await API, we weren’t passing along behaviors.
bcapps
approved these changes
May 10, 2023
Co-authored-by: Brian Capps <[email protected]>
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.
Co-authored by @ashlirankin18
What it Does
Follow-up to #6
In #6 it was proposed that we’d test out the functionality, but timing-wise it didn’t make sense, so we’ve added some basic tests today.
And good news, the tests caught two bugs!
We decided to minimally mock
URLSessionandURLSessionDataTaskwith protocol conformances. This unfortunately changes some types in public APIs, but hopefully that's deemed worth it for the future of test writing in these libraries.Additionally, strictly for time constraints, we only mocked the non-
CombineURLSessionmethod as we only intended to test the new async/await public API during this working session.How I Tested
Notes
See GitHub comments in the code below on the bug fixes caught by tests.
Screenshot
N/A