How baked are some of Xcode 15.3's concurrency warnings? One notable issue is that XCTestCase can no longer be annotated @MainActor:
@MainActor
class MyTests: XCTestCase { ⚠️
Main actor-isolated class 'MyTests' has different actor isolation from nonisolated superclass 'XCTestCase'; this is an error in Swift 6
While it seems like the migration path is to move the actor annotation to the individual test case, that breaks quickly if you need to use a method like fulfillment(of:):
Passing argument of non-sendable type 'XCTestCase' outside of main actor-isolated context may introduce data races
And some XCTest APIs require@MainActor, like UI tests, which touch XCUIApplication, so these are just incompatible with fulfillment(of:) at the moment if you want to avoid introducing warnings, though we can workaround the problem with a synchronously call to the old API, which seems like a hack:
_ = { wait(for: [something]) }()
So I guess my question is, what is the correct migration path here? Just wait for the XCTest framework to address these issues?
Xcode 15.3 shipped without doing anything about this, so I guess we just have to turn Sendability checking off for test suites? I haven't been able to figure out any way to use fulfillment(of:) that doesn't produce warnings and it's so many warnings that it makes any other sendability warnings impossible to spot anyway.
A pattern that has sometimes worked for me is to remove the MainActor isolation entirely.
func mainActorTest() async {
let exp = expectation(description: "happened")
Task {
// access MainActor-isolated stuff here, including using the expectation
}
await fulfillment(of: [exp], timeout: 1.0)
}
This has limited utility, because if you cannot make use of local state across calls to fulfillment. You may be able to use test instance variables to do it. But, I've found that incredibly annoying and not particularly scalable.
I just fixed a whole bunch of those in my project.
In this case I simply moved the @MainActor annotation from the entire XCTestCase subclass definition to each individual method (that required it). Apparently that also works when you overridesetUpWithError(), etc. so while it was a little bothersome to change so many test functions it now runs without issues (note: I am not yet using -strict-concurrency=complete, but will look at this in the near future, I hope).
What's mostly affected so far seem to be the tests for UIViewController subclasses, as we're still using a lot of UIKit, but to me it looks like this is the way to properly go: Leave the test class un-annotated, and annotate the test methods instead.
Btw, I don't entirely get what the problem with
is? That in itself is not really an issue with @MainActor, but rather just a consequence of the test method not being async. Just using wait(for: [something]) is sufficient in this case, whether the (synchronous) test method runs on the main actor or not. And while wait is "old" it's not deprecated and I doubt it's going anywhere anytime soon. It's simply the synchronous counterpart to fulfillment(of:).
Hm, okay, I was considering you might have left that out, but then that code does not produce any error, I believe? At least when I tried it out it does not?
Admittedly I could have also noticed that the error message you gave does not at all match what you'd get when simply not making the function async... that's my bad. That would be the much more obvious 'async' call in a function that does not support concurrency...
Your example obviously leaves out the part where the expectation is fulfilled, so I quickly spun up this:
That compiles and runs without any problems in my freshly updated Xcode, even with -strict-concurrency=complete. Perhaps that was a last little bug that had been fixed in the final release?
The only other reason I could see is in how you use whatever instance of the type to test you have to fulfill the expectation... I find myself often using an [unowned self] capture list in some closures I use to do so, as I tend to have an underTest property in my test's instance that I use to, well, test...
If the warning in this case is still there, can you maybe give a more concrete example that I could try to reproduce it with?
As said I was able to simply make all my functions @MainActor where I had previously used test cases annotated that way, but none of those were actually async...
Thanks for starting a discussion about this. This warning
Is pointing out a real potential for data races and it's not in the camp of "some more compiler analysis could prove this code safe". I do think there are ways that the compiler could lift the restriction at the cost of some different restrictions, e.g. we could make is so that you can add isolation to a subclass of a non-Sendable, non-isolated superclass, but the subclass cannot be made Sendable and overrides still must be nonisolated. Other folks have brought up the idea of somehow making isolated subclasses retroactively make everything from the superclass isolated to the main actor, but that would come with other restrictions too like no upcasting, and no access to nonisolated async methods which wouldn't solve the problem here with the fulfillment method.
The reason why this is unsafe is because XCTestCase is not Sendable since it is a reference type with mutable state. For non-Sendable types, you get a guarantee that only one isolation domain has access to the value at any given time. XCTestCase.fulfillment is a nonisolated async method, which means it's evaluated off of the main actor, so calling it from the main actor will pass self over an isolation boundary. While your test case is suspended waiting for the result of fulfillment, other work can happen on the main actor that accesses self, e.g. if you've done any concurrent work in your test method. My understanding is that XCTest will create fresh instances of your test case type for parallel tests, so I think the only time you can get concurrent access to self is if you've done any concurrent work yourself in your tests.
If your test isn't actually doing anything concurrently, you can use nonisolated(unsafe) to suppress the warning:
@MainActor
func testSomething() async {
let something = expectation(description: "something")
// This is okay because this test isn't doing any concurrent work, and any tests
// that run in parallel will have a completely separate instance of the test case type.
nonisolated(unsafe) let `self` = self
await self.fulfillment(of: [something])
}
Do you know if we should we be filing Feedbacks for APIs which aren't playing nicely with Swift Concurrency at this stage?
For example, in SwiftUI:
import StoreKit
/* @MainActor */ // occurs with or without annotation
struct MyView: View {
// Cannot form key path to main actor-isolated property 'requestReview'; this is an error in Swift 6
@Environment(\.requestReview) private var requestReview
var body: some View {
Text("Cannot form key path to main actor-isolated property")
}
}
Or os:
/* @preconcurrency doesn't help */ import os
enum Log {
// Static property 'shared' is not concurrency-safe because it is not either conforming to 'Sendable' or isolated to a global actor; this is an error in Swift 6
static let shared = Logger(subsystem: "logging", category: "warnings")
}
Or Foundation:
import Foundation
public final class UnitWarning: Dimension {
public override class func baseUnit() -> UnitWarning { .warnings }
// Static property 'warnings' is not concurrency-safe because it is not either conforming to 'Sendable' or isolated to a global actor; this is an error in Swift 6
public static let warnings = UnitWarning(
symbol: "WARN",
converter: UnitConverterLinear(coefficient: 1)
)
}
(This can be 'fixed' by turning it into a computed property but that would be pretty terrible for performance.)
Not all of your examples are SDK issues, but yes, you should absolutely file feedback reports if you believe an API is mis-annotated.
I think using a main-actor isolated key-path in a property wrapper initializer should be fine as long as your enclosing view is @MainActor, per SE-0411: Isolated default values. That makes this just a compiler bug.
It's a compiler bug that the @preconcurrency import doesn't suppress the warning.
Dimension is has an explicit, unavailable Sendable conformance, which means it's definitely not a thread-safe type, so this is not an issue for Foundation to fix. If you want to use a non-Sendable type as a global/static variable in your code, you will need to isolated it to a global actor for safety to be proven statically, or wrap it in some kind of Sendable container type that protects access to it. If you don't want static data-race safety, you can mark it as nonisolated(unsafe) and take care at each use site to make sure you are synchronizing access to it, e.g. by using a lock or a dispatch queue.