`MTKWaitThenContinueTest(after:)` should be updated so that it doesn't use a closure

[lgauthier]

@nhgrif I kind of forgot about this for a while. Basically, the day after releasing version 1.1.0 which included the MTKWaitThenContinueTest(after:) function, @schrismartin made a keen observation which makes me want to deprecate this original version and add a new version that takes no closure.

Usage for the original version looks like this:

// Perform some async action
MTKWaitThenContinueTest(after: 1) {
    // Perform assertions
}

What Chris pointed out is that this works just the same because it's a blocking function:

// Perform some async action
MTKWaitThenContinueTest(after: 1) {}
// Perform assertions

I think the benefit of removing the closure is that it simplifies the interface and we'd be able to remove these warnings in the function's docs:

    /// - Warning: This method works by creating an expectation and calling `waitForExpectations(timeout:handler:)`. Because of this, the following actions will result in a failure:
    ///   - Creating nested calls to `MTKWaitThenContinueTest(after:on:testAction:)`.
    ///   - Creating expectations within `testAction` that are not fulfilled within `testAction`
    ///   - Calling `waitForExpectations(timeout:handler:)` within `testAction`.

Technically, we'd still want to include a warning to explain that you won't be able to create an expectation that isn't fulfilled prior to calling the method, but we'll be able to remove that warning for iOS 11 since the expectation API has been updated to allow us to wait only for a specific expectation.

Of course, because we've already released version 1.1.0, we can't just remove the closure argument because it would break backwards compatibility. One backwards compatible option would be to make the closure optional and add a default value of nil. However, this strategy won't allow us to add a deprecation annotation on the function. So, I'm proposing that we leave the original function as is and add a deprecation annotation. Then add a new function that's identical but doesn't have the closure argument or the DispatchQueue argument.

Thoughts?

[nhgrif]

If we change the method to warn against using the main queue to dispatch your async code to, we could simple take out the the expectation stuff altogether and just sleep the current thread... but then, this really isn't much different from a sleep.

It might honestly be best if we just re-implement an expectations alternative so we can remove that altogether. But that's really only necessary for the case where we're passing a closure. I'm a little concerned if we get to a waitForExpectations and start waiting, that the async code will only run if it was dispatched to a queue other than the one this logic was called on. Did you test that?

[lgauthier]

Sleeping the current thread is different than how waitForExpectations works. I'm not familiar with what exactly it's doing behind the scenes (I assume modifying run loop modes), but it appears as though is blocks the current operation (the test) but still allows other work that is queued up on that queue to execute. If it didn't work this way, then our MTKWaitThenContinueTest(after:on:testAction:) method wouldn't actually work because, in most cases, it is called on the main queue and the main queue is the queue that it dispatches the block that fulfills the expectation.

I would be interested in looking into rolling our own mechanism for this using DispatchQueue, but I'm not sure if it would be worth it at this point since func wait(for expectations: [XCTestExpectation], timeout seconds: TimeInterval) has been introduced. I was thinking it wasn't going to be available until iOS 11, but it looks like it was actually included with the 10.3 SDK. https://developer.apple.com/documentation/xctest/xctestcase/2806856-wait?changes=latest_major

Either way, do you have any thoughts on the proposed deprecation for the closure-based version of MTKWaitThenContinueTest(after:on:testAction:)?