Allow extracting recorded requests #29
Conversation
pwalters
force-pushed
the
feature/allow-extracting-saved-requests
branch
from
ba54a60
to
c32e492
Compare
pwalters
force-pushed
the
feature/allow-extracting-saved-requests
branch
from
c32e492
to
ccbf277
Compare
| /// | ||
| /// - Parameter targetURL: URL that should be loaded from file | ||
| /// - Returns: MockRequestResponse if the request exists at that URL | ||
| func loadResponseFile(relativePath: String, baseURL: URL?) -> MockRequestResponse? { |
There was a problem hiding this comment.
Looks like we could mark this private
| func loadResponseFile(relativePath: String, baseURL: URL?) -> MockRequestResponse? { | ||
| guard let baseURL = baseURL else { return nil } | ||
| let targetURL = baseURL.appendingPathComponent(relativePath) | ||
| guard FileManager.default.fileExists(atPath: targetURL.path) else { return nil} |
There was a problem hiding this comment.
A single guard here might make it clearer.
guard
let baseURL = baseURL,
case let targetURL = baseURL.appendingPathComponent(relativePath),
FileManager.default.fileExists(atPath: targetURL.path)
else { return nil }
| guard | ||
| host == url.host, | ||
| scheme == url.scheme | ||
| else { return nil } |
| @@ -107,6 +107,10 @@ final class MockRequestResponse: Codable { | |||
| hashData.append(body) | |||
| } | |||
|
|
|||
| if let bodyData = normalizedRequest.httpBodyStreamData { | |||
There was a problem hiding this comment.
Will this break any existing recordings?
There was a problem hiding this comment.
This will change the hash for the requests that have something in the .httpBodyStream and will break the recordings. We can tag and release a version for users that have some recordings already and then merge this and release a 2.0 version.
There was a problem hiding this comment.
The better approach in my opinion is adding a delegate method that lets user compose the hash for each request based on their needs. We can keep the current hashing method as default to prevent broken recordings and we can expose a delegate so that people can compose the hash for each request based on their project needs. What do you think @scelis? If you agree with this approach we can merge this PR without the hashing change and I can open another PR to expose that delegate method.
There was a problem hiding this comment.
Instead of bumping the version, what do you think about calculating multiple hashes for backwards compatibility? When loading, if the new/updated hash doesn't match anything on disk calculate the backwards-compatible hash and check for that file, too.
There was a problem hiding this comment.
My suggestion, as I explained in another comment, is keeping the hashing code as is, so don’t breaking the previous versions, and expose an optional delegate method for people to calculate hash based on their needs because I think each project has different requirements here. For one project the headers are not important for the other one the body of the request. Some project need to remove the headers or the body from the hash to get same response on a set of request. It completely depends on the project needs.
There was a problem hiding this comment.
I think that is a decent feature request, but that MockDuck should still provide basic hash-computation functionality that should work for the 95% use case.
While not the main objective of MockDuck, it would be really useful as part of UI testing if MockDuck could be used to audit network activity.
This PR would allow an app to record requests with MockDuck, then extract the recordings for a host as
MockRequestResponseobjects. These objects can then be verified by the test suite to ensure network requests meet necessary requirements (parameters, post data, etc)