Change XCTAssertThrowsError function signature to rethrow when the error handler throws

[csknns]

The error handler for the XCTAssertThrowsError is not allowed to throw errors. This limits the code that we can write in the handler or forces us to use workarounds(i.e. explicit call XCTFail if the code inside the handler throws errors).
This PR creates an overload of the XCTAssertThrowsError to allow to the handler to throw errors.

See more details in https://forums.swift.org/t/change-xctassertthrowserror-function-signature-to-rethrow-when-the-error-handler-throws/45713.

I created an overload of the method because if I declare the original as rethrows then it will throw if the expression argument throws.

I build the framework and run the tests. Everything seems OK. If you would like I could add some unit tests for this.

[stmontgomery]

Thanks @csknns ! I will take a look at this. In the mean time, could you add a unit test for it, alongside the various other assertion tests?

[csknns]

No problem @stmontgomery ! I pushed a commit that adds unit tests for this change.

[stmontgomery]

@swift-ci Please test

[stmontgomery]

Could we update the existing XCTAssertThrowsError overload (the one whose errorHandler is non-throwing) to now call into the new overload, to consolidate the two similar code paths?

(Note that I do think we should keep both overloads just to be safe and preserve source compatibility, so I appreciate you adding a new one instead of modifying the existing one!)

[csknns]

Thanks for your comments @stmontgomery !

After checking your suggestion for consolidating the code paths I think we can have the new throwing XCTAssertThrowsError calling the existing non-throwing like this:

public func XCTAssertThrowsError<T>(_ expression: @autoclosure () throws -> T, _ message: @autoclosure () -> String = "", file: StaticString = #file, line: UInt = #line, _ errorHandler: (_ error: Swift.Error) throws -> Void = { _ in }) rethrows {

    var caughtErrorOptional: Error?
    XCTAssertThrowsError(try expression(), message(), file: file, line: line, { aError in
        caughtErrorOptional = aError
    })

    if let caughtError = caughtErrorOptional {
        try errorHandler(caughtError)
    }
}

The other way(the existing XCTAssertThrowsError calling the new overload) poses problems because if we try and call the new XCTAssertThrowsError overload with a non throwing handler it will call it self and if we wrap the handler in an throwing closure to force it to call the overloaded method then we have to handle a throwing XCTAssertThrowsError method:

public func XCTAssertThrowsError<T>(_ expression: @autoclosure () throws -> T, _ message: @autoclosure () -> String = "", file: StaticString = #file, line: UInt = #line, _ errorHandler: (_ error: Swift.Error) -> Void = { _ in }) {
// If we do not wrap the handler with a throwing closure it will not call the overloaded method
    let handler: (_ error: Swift.Error) throws -> Void = { error in errorHandler(error) }

//And we have the ungly try! here
    try! XCTAssertThrowsError(try expression(), message(), file: file, line: line, handler)
}

Would you like me update the PR with the 1 first solution?

[stmontgomery]

@csknns Ah thanks, I see why that may cause problems. I believe we can make it work the way I described by explicitly capturing a reference to the new overload within the old overload. Something like the following (without the code comments, which I just included for explanation during code review):

// The old overload:
public func XCTAssertThrowsError<T>(_ expression: @autoclosure () throws -> T, _ message: @autoclosure () -> String = "", file: StaticString = #file, line: UInt = #line, _ errorHandler: (_ error: Swift.Error) -> Void = { _ in }) {
    // Store a reference to the new overload whose `errorHandler` is throwing,
    // using an explicit function type to disambiguate:
    let rethrowsOverload: (() throws -> T, () -> String, StaticString, UInt, (Swift.Error) throws -> Void) throws -> Void = XCTAssertThrowsError

    // Call that overload directly, using `try?` to swallow the error. There won't be an error thrown anyway,
    // it's just that the compiler does not allow using `rethrows` in the explicit type above:
    try? rethrowsOverload(expression, message, file, line, errorHandler)
}

I'm not able to run the unit tests right now, but would you like to give this a try on your branch?

[csknns]

@stmontgomery Nice methodology to explicitly reference the method overload we need. I will check it out and if the unit tests are passing, I will push the change.

[csknns]

Unit tests passed so I commited the change and rebased & pushed it.

[stmontgomery]

@swift-ci Please test

[stmontgomery]

Thanks again @csknns !