suggestions for PR32#33
Conversation
|
| #expect(mock.requests(for: tokenUrl) == [request]) | ||
| #expect(try mock.firstRequest(for: tokenUrl) == request) | ||
| #expect(await mock.requests(for: tokenUrl) == [request]) | ||
| #expect(try await mock.firstRequest(for: tokenUrl) == request) |
There was a problem hiding this comment.
Why do these need awaits? They're not async?
There was a problem hiding this comment.
the MockHTTPFetcher is now an actor instead of a class, so all calls into the actor may possibly suspend to deconflict concurrent access to its internal state.
This is to address the @unchecked Sendable. Which might be fine in local code if we know it will only ever be used synchronously. A public mock should probably properly be thread-safe(sendable).
- making the MockHTTPFetcher an actor is the way I chose here. has the risk of introducing suspension points, but if it's being used sequentially without shared access shouldn't need to suspend
- could, alternatively, serialize access to MockHTTPFetcher by requiring it to run on the main thread with
MainActor
| import GermConvenience | ||
| import HTTPTypes | ||
|
|
||
| extension BundledHTTPRequest: Equatable { |
There was a problem hiding this comment.
@germ-mark shouldn't this move up into GermConvenience core?
There was a problem hiding this comment.
- is there a use for comparing BundledHTTPRequest's? I think this was defined for use in test, which is also may be why it performs a limited equality
- should the equality check compare the body?
2 participants
Implemented the GermConvenienceMocks breakout and checked sendable MockHTTPFetcher as I suggested in that PR
Noticed that the package structure has ballooned a bit; collected all the test helpers within the mocks target, and all tests in a single test target