Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MTKAssertThrowsError & MTKAssertNoThrow #39

Open
nhgrif opened this issue Nov 14, 2018 · 2 comments
Open

MTKAssertThrowsError & MTKAssertNoThrow #39

nhgrif opened this issue Nov 14, 2018 · 2 comments

Comments

@nhgrif
Copy link
Contributor

nhgrif commented Nov 14, 2018

MetovaTestKit already contains two assertions for dealing with Objective-C level exceptions, as documented here.

Should we include assertions for dealing with Swift level errors? I understand that both XCTAssertThrowsError and XCTAssertNoThrow exist, and I generally prefer to not rewrite Apple code, however I have two issues with both of these methods.

First, the signature for the Xcode Test functions is arguably confusing or misleading. We generally prefer closure arguments to be last so we can leverage Swift's nice trailing closure syntax, as we did with Exception Testing Functions. The Xcode Test default functions have the closure to be tested as the first argument. This is done presumably because all of the following arguments are optional (and only used in very rare occasion... they're for people doing things like what we're doing with MTK) and to be consistent with every other function in the Xcode Test suite, which is fair enough.

Second, and arguably more importantly, the Xcode Test functions do not allow you to have much information about the error that was thrown (other than what was printed), nor does it allow you to make further assertions about the thrown error. The first problem is relatively minor, but the second problem can potentially get in the way of writing a complete test suite, particularly in the case where you want to assert that an error was thrown. You certainly want to be sure the error was of the type you were expecting and it contained all the information you expected to, otherwise you might have false positive results. If we were to implement these assertions in MTK, we would return any thrown error we encountered, just as we do with Objective-C exceptions we catch in the exception testing methods.

However, I believe everything else in MTK is purely supplemental functions. These two additions would mark the first replacements for existing XCT functionality.

@nhgrif
Copy link
Contributor Author

nhgrif commented Nov 14, 2018

@lgauthier @schrismartin @BevTheDev @cdillard thoughts/opinions/concerns/etc?

@schrismartin
Copy link

schrismartin commented Nov 14, 2018

So the suggestions would look like this?

@discardableResult public func MTKAssertThrowsError(message: @autoclosure () -> String? = nil, file: StaticString = #file, line: UInt = #line, testBlock: () throws -> Void) -> Error?
@discardableResult public func MTKAssertNoThrow(message: @autoclosure () -> String? = nil, file: StaticString = #file, line: UInt = #line, testBlock: () throws -> Void) -> Error?

I personally like XCTAssertThrowsError for two reasons:

  • Because it uses an @autoclosure to capture the throwing error, it encourages the writer to limit the scope of the throwing code down to a single throwable command, rather than putting a bunch of code in one place.
  • It has an error handler function that allows you to further inspect the thrown error – and this function only gets called if an error is thrown, rather than having to unwrap the optional error if it does get thrown.
XCTAssertThrowsError(try doAFailableThing(), "doAFailableThing() did not throw an error.") { error in
    XCTAssertEqual(error, BadError.badThingHappened, "I was expecting a bad thing! It wasn't actually that bad!")
}

Though, in the case of XCTAssertNoThrow, I can level with you that it would be nice to handle the error that occurs (and fails the test case) in some way that better helped you extract relevant information on the error. While the above signature for MTKAssertThrowsError doesn't solve this problem, you could potentially solve using something like this...

public func MTKAssertNoThrow<T>(_ expression: @autoclosure () throws -> T, message: (Error) -> String = default, file: StaticString = #file, line: UInt = #line)

You'll notice in this case that message is a function that takes a resulting Error as an input and returns a String as an output. Though I feel this gets messy and doesn't allow for trailing closure syntax. However, moving it to the tail end of the function seems messy in my opinion, as you lose the call-site function label showing that it is indeed a message that's being defined here.

MTKAssertNoThrow(try doAFailableThing()) { error in "Error occurred: \(error.localizedDescription)" }

Any thoughts on how to best clean that up?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants