Quick / Nimble

A Matcher Framework for Swift and Objective-C
https://quick.github.io/Nimble/documentation/nimble/
Apache License 2.0
4.81k stars 600 forks source link

Test with toNever(equal(<non nil value>)) fails on encountering nil #1123

Closed jeslyvarghese closed 7 months ago

jeslyvarghese commented 7 months ago

What did you do?

Consider the following unit test

func test_toNever_failingOnNil() async throws {
    var sut: String? = "foo"
    Task {
        sut =  nil
    }
    await expect(sut).toNever(equal("bar"))
}

What did you expect to happen?

The test to pass since the expectation set to never be equal to bar is true. Here my intent is to test for value to never equate to 'bar', it becoming nil is also a valid expectation.

What actually happened instead?

Test fails with

expected to never equal <bar>, got <nil> (use beNil() to match nils)

If I follow the suggestion from error message, I will greatly alter the intent of the test.

Environment

List the software versions you're using:

Please also mention which package manager you used and its version. Delete the other package managers in this list:

Project that demonstrates the issue

NimbleIssueDemo.zip

jeslyvarghese commented 7 months ago

@younata thanks for your efforts maintaining this project.

Noticed this behaviour in 13.2.1. 13.2.0 a test case with above scenario passes. Perhaps the changes in https://github.com/Quick/Nimble/pull/1121/files brought it up.

I am unfamiliar with Nimble's codebase to verify if the issue surfaced is an expected behaviour that was fixed in 13.2.1. If so please let me know, I will close this issue.

younata commented 7 months ago

After thinking on this for a bit, here's my conclusion: toNever in Nimble 13.2.1 is exhibiting the correct behavior. The equal matcher return MatcherStatus.fail when it encounters nil, and toNever should fail/immediately exit when the matcher stops returning MatcherStatus.doesNotMatch.

This issue is actually that you expect equal to return MatcherStatus.doesNotMatch when the given expression is nil. Which is reasonable. After all, nil is not equal to "bar", so why is Nimble marking that as a test failure?

Usually, nil is treated as a special case in most matchers, where the matcher will fail regardless of whether it's a "to" or "toNot" style. For example, expect(nil as Int?).toNot(equal(0)) and expect(nil as Int?).to(equal(0)) will both be marked as failing. This is a baggage from the common Objective-C idiom of returning nil & setting an error pointer on error that was still present in Swift prior to the swift error system (e.g. expect([subject doTheThingWithError:&error]).to(equal(someValue)) should fail when nil is returned, because Objective-C doesn't have the concept of non-optional objects).

Considering that Objective-C is not used nearly as much as Swift is, we should probably special-case on Objective-C for the equal matcher. That is expect(nil).to(equal(someValue)) in Objective-C should return MatcherStatus.fail, but in Swift, it should return MatcherStatus.doesNotMatch (unless someValue is actually nil, in which case it should be the same as beNil().). We should also do the same for a bunch of other matchers in Nimble as well.

Relatedly, I've been meaning to do a bunch of work so that Matchers have to explicitly declare that they take in Optionals to operate on them (and similarly, that expect and require should not even compile if you pass in an expression returning Optionals to a Matcher that doesn't take Optionals).

All of this work is a breaking change, and something that will require a new major version.

I'm going to close this, and add another issue to specifically re-examine how we treat nils in Nimble.

Until then, as a very silly workaround, you could do an inline expression in expect to return a non-nil value:

expect { value ?? "bar" }.toNever(equal("foo")).