-
Notifications
You must be signed in to change notification settings - Fork 117
Description
Description
#expect
works differently when await
keyword is inside its body. await #expect(expr)
shows a breakdown of the actual value, while #expect(await expr)
does not. I would like to know why the expectation failed even if the result is computed from an async expression.
Expected behavior
#expect(await Array(stream) == [1,2])
throws the error testAll(): Expectation failed: (Array(stream) → [1, 2, 3]) == ([1,2] → [1, 2])
Actual behavior
#expect(await Array(stream) == [1,2])
throws the error testAll(): Expectation failed: await Array(stream) == [1,2]
Steps to reproduce
No response
swift-testing version/commit hash
Swift & OS version (output of swift --version && uname -a
)
swift-driver version: 1.87.3 Apple Swift version 5.9.2 (swiftlang-5.9.2.2.56 clang-1500.1.0.2.5)
Target: arm64-apple-macosx14.0
Darwin MBP 23.1.0 Darwin Kernel Version 23.1.0: Mon Oct 9 21:27:24 PDT 2023; root:xnu-10002.41.9~6/RELEASE_ARM64_T6000 arm64
Activity
grynspan commentedon Dec 15, 2023
This is expected and is a constraint of macros in Swift. The presence of the await keyword signals to the compiler that some subexpression of the expectation is asynchronous, but it is impossible to know exactly which subexpression from the AST alone. This means it is not possible to correctly decompose the expression as we can with synchronous, non-throwing ones.
tevelee commentedon Dec 21, 2023
@grynspan Thanks! If I understand the issue correctly, for a test case like
the
expect
macro is going to resolve to a__checkValue
(generic boolean expression) and not a__checkBinaryOperation
(where we would be able to decompose actual and expected values on lhs and rhs respectively).Can't we work around this in private func _parseCondition by not just looking for
InfixOperatorExprSyntax
but anything embedded in anAwaitExprSyntax
? In this case:Also, how is this issue different from matching a
try
expression?grynspan commentedon Dec 21, 2023
The same issue exists for a
try
expression.Imagine we add support for an
AwaitExprSyntax
path. What would the implementation look like? It would need to preserveawait
semantics on all parts of the expanded expression, but it is impossible to know which parts need to be awaited (and which are synchronous) just from the syntax tree alone.This means we'd need additional overloads of every
__check
function that areasync
,throws
, andasync throws
. This quadruples the number of overloads that need to be resolved (which has a worse-than-linear impact on compile times), and it is semantically incorrect because we'd have to introduce multiple fake suspension points that could affect the correctness of a test.The expanded form of
#expect(await x == y)
would look something like:And would have at least 4 and at most 6 suspension points when the original expression had as few as 1.
grynspan commentedon Dec 21, 2023
Now, it may be possible to simplify that expansion a bit given that the outermost call to
#expect()
would need anawait
keyword applied to it, but the effect keywords on a macro are not visible to the macro during expansion, so we can't know if the developer typed it or not. We could place our ownawait
keyword on the call to__check()
, and that could let us simplify part of (but not all of) the macro expansion, but we're still left with four times as many overloads as before.rethrows
(and a hypotheticalreasync
) doesn't help us because we must express the infix operator (among other possible subexpressions) as a closure, and that requires us to explicitly writetry
and/orawait
within the closure body, and that defeats the purpose ofrethrows
/reasync
.tevelee commentedon Dec 21, 2023
Thank you for the detailed explanation. I understand the tradeoffs now.
This seems to be the only viable solution to the issue.
grynspan commentedon Dec 21, 2023
The issue can be avoided by extracting the async subexpression out into a separate expression:
So I'd steer developers toward that solution as preferable.
tevelee commentedon Mar 21, 2024
I appreciate the writeup and the warning in #302. Thank you @grynspan
grynspan commentedon Oct 9, 2024
Reopening. Now that we have
#isolation
, it may be possible to avoid the unnecessary hops.26 remaining items