I have a test function implemented, basically an extension to Swift Testing's await confirmation {…} function. In order to get good error messages on the call site, I pass a SourceLocation to test assertions inside. In order to make things work from @MainActor and other tests, I need to pass an isolation argument isolated (any Actor)?.
The code works and everything is fine, but I'm wondering where to put these arguments in the function's signature. Obviously, in my test code I'll never use these directly, only through their default arguments. But I'm wondering about the best practices.
Say my confirmation(…) clone is called myConfirm(…) and I need the following arguments:
expectedCount: the (optional) count of confirmation calls
body: the closure during which the calls should happen
sourceLocation: the source location so I can raise test issues at the call site
isolation: the actor context for the function to work in any case
I was looking for references, and here is what I found:
Putting all this together, that would bring me to this signature:
func myConfirm<T>(
isolation: isolated (any Actor)? = #isolation,
expectedCount: Int = 1,
sourceLocation: SourceLocation = #_sourceLocation,
_ body: () async throws -> T
) async rethrows -> T
What I don't like about this, is that these macros-arguments are "scattered" in-between the user arguments. I've tried other orders, but found them not satisfying for other reasons. E.g. having the body not last feels weird as well.
To make things worse, if I now wanted to also allow comments (like all assertions in Swift Testing do), I'd run into an even wilder situation, as comment arguments are usually unnamed. So extending the above example, comment would need to go second – before expectedCount, mimicking confirmation, but after isolation since that's always first. Rendering:
With a quick search, I haven't been able to find any standard library function that has unnamed arguments in the middle. They only appear as prefix (require(_:_:sourceLocation:)), suffix ( measure(isolation:_:)) or both ( confirmation(_:expectedCount:sourceLocation:_:)), but I've never seen them inline.
Making isolation unnamed as well feels wrong and also makes argument matching very difficult for the compiler. Not sure what's correct here…
Such that the defaulted/compiler-supplied arguments come last. To me, sourceLocationfeels like it should be the last non-closure argument, but that's personal preference only and isn't required by any technical constraint.
Edit: This is also consistent with measure(isolation:_:), which places isolation as the last argument before the trailing closure.
Yes, I was asking about coding style advice. Sorry that was not entirely clear.
And thank you for your reply, that totally makes sense. I think putting sourceLocation last can be argued for that it's the less functionally-relevant of the two compiler-provided arguments. Whereas isolation is required for the code to work in all contexts.
Related, it seems Swift Testing's confirmation() does currently not work from @MainActor test cases in the Swift 6 language mode (also made an issue on Github):
@MainActor @Test func example() async {
await confirmation { _ in } // Error: Sending main actor-isolated value of type '(Confirmation) async -> ()' with later accesses to nonisolated context risks causing data races
}
Thanks for filing the issue! This diagnostic arrived relatively late in the Swift 6 development cycle and we haven't had a chance to update the function to resolve it yet.
I tried that, but it seems to just change the error message to the changed closure type:
@MainActor @Test func example() async {
await confirmation { @MainActor _ in } // Error: Sending main actor-isolated value of type '@MainActor @Sendable (Confirmation) async -> ()' with later accesses to nonisolated context risks causing data races
}
@Sendable does silence the error in the above example, but it imposes the requirement of everything being used in the closure also being sendable.
The main reason to make tests @MainActor (for me at least), is testing of something that's inherently main-actor constrained – like views. But these are never sendable.
Here's a minimal example where the @Sendable closure breaks:
class Value {}
@MainActor @Test func example() async {
var value = Value()
await confirmation { @Sendable _ in
_ = value // Error: Capture of 'value' with non-sendable type 'Value' in a `@Sendable` closure
}
}
My workaround for the time being is to have an alias for confirmation() with an additional isolation parameter – but defined in a Swift 5 module.
This way, I can use confirmations from Swift 6 tests, as the isolation requirement is shifted to the module. But since the module is using the Swift 5 language mode, there is no such requirement and just ignoring the isolation is not even a warning. I know it's not nice, but it seems to work just fine
I've sort of been defaulting to putting #isolation first. Putting it anywhere other than first or last just feels weird, and last is already claimed by trailing closures.
I've also been leaning towards naming it _isolation most of the time because manually passing any value other than explicitly specifying what the default value is expanding to anyway would be an error (and unlike #sourceLocation you don't need to manually forward it), but I'm unsure if that's a universal rule or just a common property to where I've been using it.
That's a really interesting approach indeed. I played around with it and it seems the sending is not needed for the closure (I guess the compiler just knows?), but for the return value only (because of the double actor context hop on the way back).
The more mundane alternative to @isolated(any) would be to make it @MainActor, which in this case does the same:
import Testing
class Value {
}
@MainActor @Test func example() async {
var value = Value()
await confirmation { @MainActor _ in
_ = value
}
}
func confirmation<R>(
_ body: @MainActor (Confirmation) async throws -> sending R
) async rethrows -> sending R {
try await Testing.confirmation { conf in
try await body(conf)
}
}
For a workaround it's great, with the only downside being the verbosity of explicitly mark every confirmation @MainActor. Thanks, also good thought experiment
(There's already a pull request to add the isolation parameter to the library, making workarounds unnecessary).
