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

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

Closed
1 task done
jeslyvarghese opened this issue Feb 26, 2024 · 2 comments
Closed
1 task done

Comments

@jeslyvarghese
Copy link

jeslyvarghese commented Feb 26, 2024

  • I have read CONTRIBUTING and have done my best to follow them.

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:

  • Nimble: 13.2.1
  • Xcode Version: 15.2 (15C500b)
  • Swift Version: 5.9.2

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

  • Swift Package Manager: 5.9.0

Project that demonstrates the issue

NimbleIssueDemo.zip

@jeslyvarghese
Copy link
Author

jeslyvarghese commented Feb 26, 2024

@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.

@jeslyvarghese jeslyvarghese changed the title toNever(equal(<non nil value>)) fails on encountering nil Test withtoNever(equal(<non nil value>)) fails on encountering nil Feb 26, 2024
@jeslyvarghese jeslyvarghese changed the title Test withtoNever(equal(<non nil value>)) fails on encountering nil Test withtoNever(equal(<non nil value>)) fails on encountering nil Feb 26, 2024
@jeslyvarghese jeslyvarghese changed the title Test withtoNever(equal(<non nil value>)) fails on encountering nil Test with toNever(equal(<non nil value>)) fails on encountering nil Feb 26, 2024
@younata
Copy link
Member

younata commented Feb 27, 2024

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")).

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

No branches or pull requests

2 participants