1

Title says it all, really.

if someFatalConditionThatShouldNeverOccur {
    preconditionFailure("Uh oh, the dev team made a huge mistake!!")
}

The above branch cannot be unit tested, because doing so would cause the test process to crash. And, after all, the whole point of preconditionFailure is that the code execution should never get there.

How feasible would it be to just make preconditionFailure branches get ignored by the coverage checks?

Bonus points if we could also exclude the message parameter from the precondition() function so that this wouldn't set off the coverage checker:

precondition(somePrereq, "Coverage check complains unless you leave this out")
1 Like
2

@hamishknight any thoughts on this?

3

Rather than just considering just fatalError/preconditionFailure and other similar stdlib functions, to achieve the desired effect I think we'd need to exclude any regions that definitively end in a call to a (non-throwing) Never-returning function. Otherwise, you'd fail to identify situations like this:

if someFatalConditionThatShouldNeverOccur {
  logSomething()  // line still marked as uncovered
  fatalError("There can be no recovery from this")
}

or basically any situation where your failure case goes into another function that unconditionally ends in something like fatalError:

if someFatalConditionThatShouldNeverOccur {
  // No statements in this block can be covered
  logSomething()
  myCustomFatalErrorFunction()
}

// This function can never be covered
func myCustomFatalErrorFunction() -> Never {
  doSomethingElse()
  fatalError()
}

Things get a little trickier with branch coverage (does Swift support this? I don't remember), since you'd also want to exclude the branches that would allow that non-coverable code to be entered.

2 Likes
4

I'm somewhat loath to expand the problem such that it makes it difficult to implement, because that reduces the chances that it ever will be implemented. Something simple that I can find ways to work around is better than nothing.

Heck, it'd be an improvement over the status quo even if we just had an annotation we could drop in:

if someFatalConditionThatShouldNeverOccur {
    // code-coverage: ignore-block (or something like that)
    logSomething("Hold my beer!")
    fatalError("Aieee!!! *BONK*")
}
5

Wouldn’t it be better if we could test these code paths?

11 Likes
6

If the code path ends in fatalError because it's unreachable but it can't be proved to the front-end, it might still be impossible to test.

7

Sure, I can buy that argument where one is asserting that a code path is unreachable.

I don’t think I’d agree with OP that such cases are the “whole point” of fatalError, though, and write them off across-the-board as untestable.

Indeed, the Swift standard library has internal testing facilities for expected crashes, and I’ve had many occasions where I wished I could have done the same with my own code just as the StackOverflow conversation participants have found.

2 Likes
8

From that answer:

// backup of the original Swift `fatalError`
private static let defaultFatalErrorClosure = { Swift.fatalError($0, file: $1, line: $2) }

This line right here is going to set off the coverage checker, because the contents of that closure are not going to be covered. So this doesn't really solve the problem, it just moves it.

9

I don’t think I’d agree with OP that such cases are the “whole point” of fatalError, though, and write them off across-the-board as untestable.

What else do you use fatalError for? To me, if I have an error condition that I expect to be reachable, rather than an indication of a programming error, I'm going to throw an error or otherwise find a way to handle it more gracefully than that.

Compare to the ! postfix operator, which says, "I am promising, based on some knowledge I have that the compiler is not privy to, that this variable will never be nil, and if it does, that is a programming error and therefore should crash." In this way, these two expressions could be equivalent in meaning:

let nonNil = typedAsOptionalButIKnowItsNotNil!

and

guard let nonNil = typedAsOptionalButIKnowItsNotNil else {
    fatalError("Programming Error: this thing should never be nil")
}

The main difference being that the less expressive version doesn't set off the coverage check, whereas the more expressive one does. :confused:

1 Like
10

What if fatal error indicates that some application state is corrupt (e.g. invalid indices to an array), and these indices are shared by many tests. After we “recover” from fatal error, do we restart the entire test execution skipping the crashing test, or do we just avoid running any further tests from the same test suite? Maybe neither is required if sharing state between tests turns out to be uncommon in the real world.

11

I would love to see fatalError and friends becoming testable. While they are sometimes used in paths that are truly never reachable. They are also often used to catch developer errors or enforce API semantics. Making this code testable would be amazing!

8 Likes
12

Matt Gallagher came up with an excellent workaround for testing these code paths:

2 Likes
13

Right, so adapt this solution in the test runner itself, moving this line or something equivalent into there. Your coverage checker isn’t coverage checking XCTest.

A reachable but unrecoverable condition. This is a pervasive pattern and philosophy in the Swift standard library, though more so with preconditions which I’ve noticed you’ve lumped together with fatalError in your pitch.

Nit: in this specific example, narrating “this thing should never be nil” instead of writing ! isn’t more expressive—indeed, as you say, it expresses exactly the same thing, but with more ceremony, demonstrating that it’s using less expressive language features.

14

Good point. My guess would be that it’d be rare to share mutable state with a test that’s expected to fail and corrupt that shared state while doing so. I’d expect that with our concurrency features now in place it would be possible to have a design for this feature that requires any state to be Sendable and fences off the entire region of logic that deliberately corrupts state in its own async context.

1 Like
15

Aside from the cases where fatalError might be used as a reachable but otherwise unrecoverable condition as mentioned by @xwu, it should alse be noted that there are some Never returning functions that gracefully exit e.g exit, which we probably wouldn't want to apply this to.

If by branch coverage you're referring to branch regions in LLVM coverage maps, then no, the compiler does not currently support them. We currently only ever emit regular code regions. But even with just code regions, we'd need to do dominance checking to properly eliminate all regions that would unconditionally end in something unreachable. This isn't something that's straightforward to do currently since we emit coverage information while we do SILGen, which is pretty error prone as we're essentially doing ad-hoc control flow analysis. However I want to investigate pushing more of the coverage logic into the SILOptimizer pipeline, which would make this much easier and more robust to do.

Indeed, this is a somewhat commonly requested feature and is internally tracked by rdar://74839955.

1 Like
16

But XCTest doesn't do this natively, so this will have to be in your own code—which won't be covered.

If the team feels strongly about one or the other, I'd be willing to limit my use to one or the other of these functions, if one of them could do it. Right now, it's impossible to do this with any sort of precondition/fatal error mechanism of which I am aware.

Not if the message contains information about why this should be unreachable, which a real-world example would.

For a more fleshed-out example of a code path that should never be reachable, imagine we have a type that wraps several more primitive interfaces, providing a common interface to each. The backing of this may be implemented something like:

public class MyWrapperThingy {
    private enum Backing {
        case bleep(bleep_t)
        case bloop(bloop_t)
    }

    private let backing: Backing

    public init(bleep: bleep_t) {
        self.backing = .bleep(bleep)
    }

    public init(bloop: bloop_t) {
        self.backing = .bloop(bloop)
    }

    public func doSomething() {
        switch self.backing {
        case .bleep(let bleep):
            // do something with bleep
        case .bloop(let bloop):
            // do something with bloop
        }
    }
}

Now, suppose we want to add support for a backing type blorp_t that was introduced in a recent version of the OS. We will need to use availability markers:

public class MyWrapperThingy {
    private enum Backing {
        ...
        case blorp(Any) // Any because otherwise we will not be able to compile for earlier OS versions
    }

    ...

    @available(macOS whatever, *)
    public init(blorp: blorp_t) {
        self.backing = .blorp(blorp)
    }

And then in doSomething:

    public func doSomething() {
        switch self.backing {
        ...
        case blorp(let blorp):
            guard #available(macOS whatever, *) else {
                fatalError("Should be impossible to reach this point on old macOS")
            }

            ... do stuff with blorp ...
        }
    }
}

In the above case, it should be literally impossible to reach that fatalError, but we have to have it to satisfy the compiler. Even if there were a good way to test fatalError, in order to even manipulate things into a state where that path would even be reachable would require polluting the code with artificial spaghetti just to cause what should just be a programmer error.

17

Right, hence my first post—wouldn't it be better if Swift did offer this natively?

Why wouldn't one write:

case blorp(let blorp):
if #available(macOS whatever, *) {
  // do stuff with blorp
}

In other words, why include an unreachable, untestable fatalError? Why's it "satisfying the compiler"?

18

Well, in the specific case I'm dealing with, a lot of these functions need to return a value. So in that case, in order to avoid a compiler error about a missing return, it's necessary either to add an unreachable fatalError or an unreachable dummy return value. Of the two, the fatalError is clearly preferable.

From a secondary, purely stylistic point of view, I prefer #guard available over #if available because it more cleanly isolates the "obsolete" code paths from the modern, preferred path. Once you can move the minimum target up such that you no longer need the availability checks, you can just delete the offending code rather than having to adjust the indentation levels of everything.

19

In the case you’re actually dealing with (not the example you gave above), you’ve identified an example where you’re using fatalError to satisfy the compiler when some code path is unreachable.

As I said in my second post in this thread, I totally acknowledge that this is one use case for fatalError. I’d have no such quibble with, say, proposing a Swift counterpart to an unreachable annotation, particularly if paired with proposing a way to test reachable fatal errors and preconditions.

However, that you’ve described one (current) use case of fatalError doesn’t mean that it’s the “whole point” of fatalError. And if one use case demands special treatment, then you’ve found some argument in favor of inventing some new way of expressing that use case, not for a change in behavior for all uses of the current feature.

20

Well, it's not the only use case that results in unreachable code; there's quite a few of them that I've seen. Off the top of my head, I can also think of:

  • switch statements where, for some reason, I happen to know that one of the cases will never be reached
  • switch statements that are on something other than an enum, like a series of integer ranges, that can't be formally exhaustive but which, for some reason, we happen to know what possible values the input will take
  • required init(coder:) methods on NSView subclasses that are only to be initialized via init(frame:)
  • force-unwrapped optionals in cases where we need to clarify to the reader of the code why we know this value won't be nil, or where we want to make any programmer error that results in this crash easier to debug

What I haven't seen is a convincing use case for fatalError in an expected code path that is not the result of a programmer error and that warrants testing. However, giving the benefit of the doubt that this does exist, let me amend my position to be that we need to have some way to assert that a path is unreachable in a way that satisfies the coverage checker. Whether we call that mechanism fatalError, preconditionFailure, florglequat, or something else is simply a matter of bikeshedding.