Add support for concurrency mocking #277
Conversation
There was a problem hiding this comment.
Thanks for making this PR—looks really good. It seems that Swift is choosing the incorrect overload because the constraint system heavily penalizes mismatched calling contexts (coderef). In the case of XCTests, we’re calling a synchronous method overload from an async context.
I’d like to avoid additional flavors of given and verify to keep the testing API surface as small as possible. After experimenting a bit, I think the cleanest API that also makes sense semantically is something like this:
given(await myMock.doSomethingAsync()).willReturn(someValue)It does require marking the Mockable<...> framework declarations as async. Let me know what you think.
Edit: Looks like there’s some failing tests due to missing availability annotations.
| if method.attributes.contains(.async) { | ||
| attributes += " async" | ||
| } |
There was a problem hiding this comment.
nit: single line for consistency
| func XCTAssertThrowsAsyncError<T: Sendable>( | ||
| _ expression: @autoclosure () async throws -> T, | ||
| _ message: @autoclosure () -> String = "XCTAssertThrowsAsyncError failed: did not throw an error", | ||
| file: StaticString = #filePath, | ||
| line: UInt = #line, | ||
| _ errorHandler: (_ error: Error) -> Void = { _ in } | ||
| ) async { | ||
| do { | ||
| _ = try await expression() | ||
| XCTFail(message(), file: file, line: line) | ||
| } catch { | ||
| errorHandler(error) | ||
| } | ||
| } |
| if method.attributes.contains(.rethrows) { attributes += " rethrows" } | ||
| if method.attributes.contains(.throws) { attributes += " throws" } |
There was a problem hiding this comment.
We should preserve the previous semantics as methods can be marked as either rethrows or throws but not both; a rethrowing method implies a throwing method.
|
Nice, you're right. That's a shame, unfortunately, because the method has the Wow, your idea sounds great. There is just one more thing to considering. Do you think this "syntax" should be extended to throwable methods? given(try await myMock.doSomethingAsyncAndThrowable()).willReturn(someValue)Even for non async methods? given(try myMock.doSomethingThrowable()).willReturn(someValue)Or just for
|
|
Workaround implemented 🎉 given(await myMock.doSomethingAsync()).willReturn(someValue)And verify like before as well. verify(await myMock.doSomethingAsync()).wasCalled() |
There was a problem hiding this comment.
Nice work! The changes look good to me.
Do you think this "syntax" should be extended to throwable methods?
If Swift’s overload handling changes we might be able to remove await as a requirement when stubbing/verifying, so let’s keep it simple and hold off on adding additional keywords for now.
This PR implements support for parsing async methods and closures as discussed in #229