Quick / Nimble

A Matcher Framework for Swift and Objective-C
https://quick.github.io/Nimble/documentation/nimble/
Apache License 2.0
4.81k stars 598 forks source link

Mark the sync versions of toEventually as nonasync #1010

Closed younata closed 1 year ago

younata commented 1 year ago

I wish we could actually make this an error, but a warning will have to suffice.

This fixes the developer experience bug where the following will compile perfectly fine, but the test will fail with a timeout error:

class MyXCTestCase: XCTestcase {
    @MainActor func test_myExample() async {
        expect(1).toEventually(equal(1))
    }
}

Which is the exact same issue I tried to avoid in https://github.com/Quick/Nimble/pull/1007.

Swift offers no other way to mark a function as unavailable in async contexts except through the @available(*, noasync) mechanism. Which, in Swift 5.7, emits a warning, not an error. But a warning is better than nothing, so we're doing this.

Note for historical purposes: We shipped Nimble 11 without this because I thought it was more important to have swift version parity with Quick. That was a mistake.

NachoSoto commented 1 year ago

Unfortunately the same thing happens in a function like this:

class MyXCTestCase: XCTestcase {
    @MainActor func test_myExample() {
        expect(1).toEventually(equal(1))
    }
}

Even though it's not marked as async, I think because of the @MainActor annotation, the test also times out. My solution has been to update all these to async and use await expect(x).toEventually.

younata commented 1 year ago

😭

I would expect the "MainActor w/o marking test as async" to be a fairly rare occurrence. Thankfully, this bug doesn't occur if you mark the entire test class as running on the main actor.