Pitch: XCTAssertNoThrow with result validation

hybridcattt · January 20, 2019, 4:40pm

Hi everyone,

This is my first post here and my first pitch for Swift. Looking forward to the feedback

Introduction

Swift 2 has introduced error handling mechanism, which has been widely adopted since. Shortly after that, XCTest added error catching support to its assert functions. Latest change on this front was introduced in Xcode 8.3 with addition of XCTAssertNoThrow.

Today there are still use-cases that require boilerplate or custom helpers to test throwing functions, which I believe are fairly common. We should fill the holes in XCTest so tests of code that interacts with Swift error handling is concise and clean.

The main limitation right now appears to be that XCTAssertNoThrow does not provide access to a produced value.

Motivation

There is a major use case that is currently not covered by any of the available XCTAssert* functions. If a developer wants to test a complex return value of a function that may throw an error, they have limited options, all of which result in awkward code.

If the return value has a type that conforms to Equatable, a simple XCTAssertEqual call would do the trick.
However, there are multiple cases when that is not possible or preferred:

if the return type does not conform to Equatable
the type conforms to Equatable, but is so complex that failure message of a single XCTAssertEqual call produces unreadable results
the initializer of the type is not accessible

In all these cases the user might want to test the value by spelling out multiple separate assert statements over some of the properties of the return value.

Some common real-life use-cases include:

model deserialization
functions returning large collections
any functions that return types with inaccessible initializers

Currently, a developer has following options to test the return value of a throwing function or an initializer:

Use provided XCTAssertNoThrow, ignoring the value

In case all necessary initializers are available, use a single XCTAssertEqual assert

 XCTAssertEqual(try BookModel(from: dictionary), 
                BookObject(id: "...", title: "...", authors: [Author(id: "...", name: "..."), Author(id: "...", name: "...")], ...))

Use XCTAssertNoThrow, followed by a try statement and optional handling.
Something like the following:

 XCTAssertNoThrow(try BookModel(dictionary: sampleDictionary))
 let book = try? BookModel(dictionary: sampleDictionary)
 XCTAssertEqual(book?.name, "...")
 XCTAssertEqual(book?.description, "...")
 XCTAssertEqual(book?.rating, 5)
 ...

Write multiple XCTAssertEqual calls, which implicitly already have error catching capabilities:

 XCTAssertNoThrow(try BookModel(dictionary: sampleDictionary))
 XCTAssertEqual(try BookModel(dictionary: sampleDictionary).name, "...")
 XCTAssertEqual(try BookModel(dictionary: sampleDictionary).description, "...")
 XCTAssertEqual(try BookModel(dictionary: sampleDictionary).rating, 5)
 ...

Don’t use either of the above, and use do/catch mechanism directly

 do {
     let book = try BookModel(dictionary: sampleDictionary)
     XCTAssertEqual(book.name, "...")
     XCTAssertEqual(book.description, "...")
     XCTAssertEqual(book.rating, 5)
 }
 catch {
     XCTFail(...)
 }

Option 1 does not really solve the problem, as it doesn’t actually test the value.

Option 2, if available at all, can produce failure messages that are hard to interpret and that make it extremely hard to find what is actually wrong

Screenshot here

Options 3 and 4 have multiple downsides, such as: the function in test is executed multiple times, on each assert; copy-paste overhead to write and maintain such asserts; in case an error is thrown, multiple failures are produced:

Screenshot here

Option 5 makes the user write boilerplate code, avoiding asserts included in XCTest framework. The boilerplate has to include not only do/catch, but also to correctly record failures.

Additional motivation point is inconsistency with XCTAssertThrowsError, which already does provide access to the caught error for additional evaluation. Its signature looks like this:
func XCTAssertThrowsError<T>(_ expression: @autoclosure () throws -> T, _ message: @autoclosure () -> String = default, file: StaticString = #file, line: UInt = #line, _ errorHandler: (Error) -> Void = default)

Proposed solution

Add an extra trailing argument to XCTAssertNoThrow similar to XCTAssertThrowsError - validation closure that’s executed on a value, in case the function behaves as expected and does not throw. If a functions does throw an error, the validation closure won’t be executed.

The proposed function signature:
func XCTAssertNoThrow<T> (_ expression: @autoclosure () throws -> T, _ message: String = "", file: StaticString = #file, line: UInt = #line, also resultHandler: (T) -> Void)

Having such function available, a return value can be tested as such:

XCTAssertNoThrow(try BookModel(from: testDictionary)) { book in
    XCTAssertEqual(book.id, "123")
    XCTAssertEqual(model.authors.count, 66)
    XCTAssertNil(model.optionalProperty)
    …
}

In case a throw happens, a single failure would be recorded on the first line. In case one of the asserts from the validation closure fails, it would be correctly displayed.

Screenshots here

Implementation details

I have a working version of this that uses existing XCTAssertNoThrow under the hood. We’ve had this used in our test suite for the past 1.5 years, and it can be considered a prototype implementation of the proposed feature.

When it comes to an actual PR phase to swift-corelibs-xctest the implementation can be revised and adapted.

public func XCTAssertNoThrow<T> (_ expression: @autoclosure () throws -> T, _ message: String = "", file: StaticString = #file, line: UInt = #line, _ resultHandler: (T) -> Void) {

    func executeAndAssignResult (_ expression: @autoclosure () throws -> T, to: inout T?) rethrows {
        to = try expression()
    }

    var result: T?
    XCTAssertNoThrow(try executeAndAssignResult(expression, to: &result), message, file: file, line: line)
    if let r = result {
        resultHandler(r)
    }
}

Impact on existing code

The change is purely additive.

Impact on ABI stability / resilience

... tbd
(The change would be (probably?) additive if we were adding an overload, but I'm not entirely sure in case of adding a parameter with a default value. Help here is very welcome)

Alternatives considered

It is an option to do nothing in XCTest and leave it up to individual developers to add this convenience assert function to their test suites.

Note: I've written an article about this and other similar assert functions we use at Storytel. It can be found here on medium. It's not at all necessary to read the article in order to review this pitch.

/ Marina

benrimmington · January 20, 2019, 10:16pm

Option 6: Add throws to your test method.

func testBookModel() throws {
    let book = try BookModel(dictionary: sampleDictionary)
    XCTAssertEqual(book.name, "...")
    XCTAssertEqual(book.description, "...")
    XCTAssertEqual(book.rating, 5)
}

If an error is thrown, the test will fail (with a description of the error).

Jon_Shier · January 20, 2019, 10:31pm

I think this feature could be generalized to more types of assertions. For example, if I accumulate test results into an array, I may initially test that the array has a certain number of items. But them I'm left with the question of how to test values from certain indices safely while still being able to produce assertion failures. Having a similar closure for XCTAssertEqual would work well.

XCTAssertEqual(results.count, 3) {
    XCTAssertEqual(results[0], "0")
    XCTAssertEqual(results[1], "1")
}

Bonus points for being able to fail assertions in the closure when they can't run.

omochimetaru · January 21, 2019, 2:18am

Yes, but this behavior is insufficient.
XCTest runtime converts our Swift Error object into NSError.
When this happen, error message information is lost even if error conform CustomStringConvertible.
I am in trouble about this everyday.

For example, this code

    func testBrokenJSON() throws {
        let json = """
{
  "a": "aaa"
  "b": 1
}
"""
        let data = json.data(using: .utf8)!
        let decoder = FineJSONDecoder()
        
        _ = try decoder.decode(B.self, from: data)
    }

prints

Test Case '-[FineJSONTests.DecodeTests testBrokenJSON]' started.
<unknown>:0: error: -[FineJSONTests.DecodeTests testBrokenJSON] : failed: caught error: The operation couldn’t be completed. (RichJSONParser.JSONParser.Error error 2.)

But this code

    func testBrokenJSON() throws {
        let json = """
{
  "a": "aaa"
  "b": 1
}
"""
        let data = json.data(using: .utf8)!
        let decoder = FineJSONDecoder()
        
        do {
            _ = try decoder.decode(B.self, from: data)
        } catch {
            XCTFail("\(error)")
        }
    }

prints

Test Case '-[FineJSONTests.DecodeTests testBrokenJSON]' started.
/Users/omochi/github/omochi/FineJSON/Tests/FineJSONTests/DecodeTests.swift:69: error: -[FineJSONTests.DecodeTests testBrokenJSON] : failed - unexcepted token (JSONToken(string, 3 bytes, at 3:3(17))), expected (, or })

So I think that idea about improve error handling for XCTest is great.

benrimmington · January 21, 2019, 4:32am

@omochimetaru Have you tried the LocalizedError or CustomNSError protocols?

omochimetaru · January 21, 2019, 10:27am

Oh I didn't know them. It works good when I tried.
LocalizedError is perfect solution I was looking for. Thank you!!

hybridcattt · January 21, 2019, 7:42pm

Option 6: Add throws to your test method.
func testBookModel() throws {
   let book = try BookModel(dictionary: sampleDictionary)
   XCTAssertEqual(book.name, "...")
   XCTAssertEqual(book.description, "...")
   XCTAssertEqual(book.rating, 5)
}
If an error is thrown, the test will fail (with a description of the error).

Thanks for mentioning it, it indeed should be Option 6!

This option also has some drawbacks that make it barely usable in real life:

the failure is not shown in the source editor, as other assert failures are
the failure is recorded on test method level, not line level. The whole test is marked as failed, not individual assert / line. So if there are two try calls, it's not possible to know which one threw.
error description is not included in the failure.

This behaviour should definitely be also improved, but I believe even in it's ideal form it's orthogonal to assert functions.

Here are some screenshots of how failures look

source editor view:

test summary view:

Would you say I should update the first post to include this?

hybridcattt · January 21, 2019, 8:02pm

I think this feature could be generalized to more types of assertions. For example, if I accumulate test results into an array, I may initially test that the array has a certain number of items. But them I'm left with the question of how to test values from certain indices safely while still being able to produce assertion failures. Having a similar closure for XCTAssertEqual would work well.

This is a great idea to explore! I have definitely written test cases like that many many times. We would often have to resort to only verifying first and last elements in the collection. So cumbersome and limiting.

I've now thought about this example more and there are a few questions that come up in this particular example:

How do we make a jump from comparing integers (results.count and 3) to evaluating a closure on result itself?
Brainstorming a bit, what comes to mind is maybe we could have a special assert that would verify a condition on a value, followed by an additional validation closure. So this would be a separate assert function that can be used in combination with others, instead of adding validation to each existing assert function
Or maybe there's something to be done in the more narrow area of asserts for collections, specifically addressing this problem of safe access?
XCTAssertEqual takes two arguments, but a validating closure would normally take one.
How do we know which value, first or second, to validate? Equality doesn't necessarily mean same identity, so first and second value can't be assumed to be identical.
To solve this, we would need to explicitly mark first and second arguments as actual and expected, making it clear that actual is passed to the closure. Now XCTAssertEqual is agnostic to order and treats both arguments equally (pun intended).
I'm not sure we would want to open the pandora's box of having to decide whether it should be assert(actual, expected) or assert(expected, actual). Having it be agnostic to order seems to be advantageous because people can be a bit free to choose how to write their tests.

I'm gonna keep thinking about this idea and how to approach it in a way that would resolve mentioned pitfalls, and in the meantime I'm looking forward to hear what you think about these questions.

rnantes · August 12, 2019, 2:08am

+1 to this Pitch. When I am trying to test code that throws I want to see the error as to why, if it doesn't throw I want to assert values in the object that was successfully created. This should be a 1 liner

let myFetchedObject = XCTAssertNoThrow(try MyAPIFetcher.fetchAnObject())
XCTAssertEqual(myFetchedObject.name, "bob")

rnantes · October 11, 2019, 4:48pm

So I was testing the XCTUnwrap() function (added in Swift 5.1) and it looks like it provides the same functionality of the proposed XCTAssertNoThrow. For example

enum SomeError: Error {
    case someThrowingErrorCase
}

func somePassingThrowingFunc() throws -> String {
    if 0 != 0 {
        throw SomeError.someThrowingErrorCase
    }
    return "abc123"
}

func someFailingThrowingFunc() throws -> String {
    if 0 == 0 {
        throw SomeError.someThrowingErrorCase
    }
    return "abc123"
}


func testThrowingFunction() throws {
    let firstString = try XCTUnwrap(somePassingThrowingFunc()) // this line will not throw and will set firstString
    print(firstString); // prints 'abc123'
    let secondString = try XCTUnwrap(someFailingThrowingFunc()) // this line will throw and fail the test 
    print(secondString) // this line will never be reached
}

John_McCall · October 11, 2019, 5:11pm

Note that the throwing-ness pattern used by XCTAssertNoThrow — a non-throws, non-rethrows function that takes a throwing autoclosure — is something that should arguably be illegal in the language and certainly should not be used in new API. XCTUnwrap does not have this problem.

rnantes · October 11, 2019, 5:20pm

So essentially if you have a throwing function that you are testing and you either want to

Not throw, set a variable, and continue the test
OR
Throw and fail the test

XCTUnwrap() is the best way to accomplish this?

John_McCall · October 11, 2019, 5:33pm

I think so, yes. Arguably XCTUnwrap should require its argument to already be optional, rather than allowing promotion to an optional, but in fact it's useful that it doesn't work that way. (Also, unfortunately, XCTUnwrap has no way to require an optional like this in the language today; the ?? operator does this, but its behavior is hard-coded.)

Nevin · October 11, 2019, 5:46pm

For what reason?

Any function that catches errors thrown within it can take a throwing parameter and not itself throw. I don’t see how autoclosures change anything there.

John_McCall · October 11, 2019, 7:37pm

Autoclosures work like closures on a basic level in the callee, but at the call site they look like they're part of the surrounding code, and readers should be able to expect that they behave as if they're evaluated more-or-less normally. It is arguably inappropriate to do something like evaluate an autoclosure twice or, if it throws an error, to catch or substantially change that error value. You should not make something an autoclosure just to allow callers to elide braces; it is a semantically loaded decision that needs to be made carefully.

hybridcattt · November 15, 2019, 11:04pm

This issue applies to all XCTAssert* functions though. I agree with the point, but I feel this shouldn't hinder improvements to existing asserts, unless it's the intention to deprecate the existing way of doing it. Do you see such deprecation as a real possibility in the future?

John_McCall · November 15, 2019, 11:07pm

It was never intended to work. It is already informally "deprecated". I don't know when we'll have time to formally declare that.

hybridcattt · November 15, 2019, 11:11pm

Indeed the addition of XCTUnwrap() makes life easier in some cases, and Xcode 11 handles throwing test functions much better.

Though I still see it having limitations same way using a simple try does - it stops execution of the rest of the test function immediately, and effectively forces the developer to structure their test functions in a certain way that wasn't enforced before.
Such behaviour also conflicts with XCTestCase.continueAfterFailure - which, even though true by default, is effectively ignored in this case.

Given the above I still think it's worth looking at improving the existing assert functions, but I would appreciate other opinions in regards to whether it's worth to continue in this direction.

hybridcattt · November 15, 2019, 11:12pm

I'm quite surprised to be honest, could you please elaborate more?

liambutler-lawrence · July 9, 2021, 4:37pm

I've run into this issue with a slightly different use case (details below) and would like to put a word in for fully supporting this feature (i.e. within closure params of rethrows functions) rather than removing/deprecating it. I think the readability issue is more appropriate for a linter to handle, and we should have the choice whether to use it or not

Details:
I have these 2 small helpers for handling “programmer errors” (e.g. decoding a known file):

public func shouldBeImpossible(_ description: String) -> Never {
  fatalError("\(description) should be impossible")
}
public func tryAnd<T>(
  fail getErrorDescription: @autoclosure () -> String,
  succeed block: @autoclosure () throws -> T
) -> T {
  do {
    return try block()
  } catch {
    shouldBeImpossible(getErrorDescription() + ": \(error)")
  }
}

In normal usage, the compiler allows me to use tryAnd in a non-throwing function without any errors:
No error

    func foo(indexFileLocation: URL) {
        let indexFileData = tryAnd(
            fail: "Could not decode index file at \(indexFileLocation)",
            succeed: try Data(contentsOf: indexFileLocation)
        )
        ...
    }

However, when I use tryAnd within a closure passed to a rethrows function, such as map, it doesn’t work:
Error : ‘map’ call can throw, but it is not marked with ‘try’ and the error is not handled

let instructions = fileIndex.instructions.map { instructionFileLocation in
    let data = tryAnd(
        fail: "Could not get data of instruction file at \(instructionFileLocation)",
        succeed: try Data(contentsOf: instructionFileLocation)
    )
    ...
}

Topic		Replies	Views
Proposal: XCTest Support for Swift Error Handling Discussion	25	3471	January 14, 2016
Change XCTAssertThrowsError function signature to rethrow when the error handler throws Pitches testing , xctest	4	1739	July 18, 2021
XCTAssert for enum cases? Using Swift xctest	2	7672	February 2, 2020
[Pitch] Prettify XCTAssertEqual Pitches xctest	8	2526	August 23, 2021
Testing Assertions Discussion	2	1958	January 5, 2016

Pitch: XCTAssertNoThrow with result validation

Related Topics