-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add async variants of XPCClient.send/sendMessage(...)
#59
Conversation
Fixes #19 Adds new tests for the async variants. Updates documentation to match.
@amomchilov for your review |
/// - route: The server route which will handle this. | ||
@available(macOS 10.15.0, *) | ||
public func send(route: XPCRouteWithoutMessageWithoutReply) async throws { | ||
try await withCheckedThrowingContinuation { continuation in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why checked
? This looks safe for withUnsafeThrowingContinuation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, yeah - my bad. I had withChecked
when I was making sure everything worked...then forgot that I should actually swap it out to the "production" version. Will switch them all over before merging.
@@ -20,7 +20,7 @@ class RoundTripIntegrationTest: XCTestCase { | |||
anonymousServer.start() | |||
} | |||
|
|||
func testSendWithMessageWithReply() throws { | |||
func testSendWithMessageWithReply_Sync() throws { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's reasonable to just completely replace these closure-based tests.
In my own work, I've pretty much completely abandoned the XCTest continuation APIs in favour of async/await, even for old projects that don't use it in the production code (I just add wrappers in my test bundles, which makes test writing easier and more correct).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to leave them for now as the behavior differs for two of them depending on whether they have a closure provided or not, but point taken.
Fixes #19
Adds new tests for the async variants. Updates documentation to match.