All pass matcher

[rbeeger]

This implements #52

[jeffh]

Hey @rbeeger, I like the overloaded allPass matcher that also accepts a nimble matcher.

For the objc part, is there something that prevents the use of NSFastEnumeration/NSEnumeration? It would help generalize it for more than NSArray and NSSet.

[rbeeger]

NSFastEnumeration doesn't work in swift's for in. It only works in the ObjC for in. But here http://stackoverflow.com/questions/25788290/nsfastenumeration-in-swift someone mentioned NSFastGenerator which allows to handle all NSFastEnumerations quite well. It works for the tests I have written for it. I've added a new commit with this change to this pull request.

[rbeeger]

Oops, while changing to NSFastEnumeration, I removed rejecting objects it cannot handle. And the error message also looked wrong.

[jeffh]

Should this if statement be inlined to where collectionIsUsable = false? I think we can remove that variable entirely.

[rbeeger]

Unfortunately there are two situations at which the collection becomes unusable:

• it contains at least one object that isn't a NSObject (line 58)
• the collection isn't a NSFastEnumeration

Inlining would double the same code.
It could be moved into a closure that would be called at both places, but that would add a variable for the closure and would make the whole thing more confusing.

[jeffh]

Oops, my bad, I didn't see that, thanks for the pointing that out.

[jeffh]

After looking at it again, there seems to be some more complex details with nested matchers.

Should matching nils in collections be allowed?

expect([nil, nil] as [Int?]).to(allPass(beNil()))

What's the error message for matching nils against non-nil accepting matchers?

expect([nil, nil] as [Int?]).to(allPass(beLessThan(5)))

Thoughts @Quick/contributors & @rbeeger?

@rbeeger, a minor quibble: allPass fails to explain that nils should explicitly use beNil():

    func testAllPassFailsAgainstNils() {
        // should it be "pass" instead of "match"?
        // expected error message: "expected to all match (use beNil() to match nils)"
        failsWithErrorMessageForNil("expected to all match") {
            expect(nil as [Int]?).to(allPass(beLessThan(5)))
        }
    }

Also, I put one other comment in the commit.

[rbeeger]

I fixed the handling of nil epected values.

The other one is harder. Each attempt at allowing matching nils in collections I made failed. We have two layers of generics here. One is in the allPass matcher and the other in the used matcher.

Currently the passed collection cannot have an optional generator element type. Passing '[1,2,3,4] as [Int?]' in any of the tests of the allPass matcher breaks it. I haven't found the correct generic types constellation that would make it work.

The problem is in getting the optional from the test through allPass into the used matcher without having it wrapped in yet another optional.

[rbeeger]

I investigated sequences of optional values as expected values a bit further and still haven't found a solution that works well.

I can change the element type of the sequence to T? like in

public func allPass<T,U where U: SequenceType, U.Generator.Element == T?>
    (matcher: NonNilMatcherFunc<T>) -> NonNilMatcherFunc<U> {
        return createAllPassMatcher() {matcher.matches($0, failureMessage: $1)}
}

Of course I need to change createAllPassMatcherso that it accepts T?. No problem here. But then I can still not check that all elements of a sequence are nil, So I need to duplicate the allPass function to work for MatcherFunc. Also no problem.

Error messages now contain Optional<whatever> in the listing of the sequence contents. That can be handled by stringifying all elements locally.

But now we get to the part that doesn't work anymore

expect(Set([1,2,3,4])).to(allPass(beLessThan(5)))

That's because we now need a sequence of Int? as the actual value. Int? is not Hashable and you can only create Sets of Hashables.

I cannot duplicate the functions with Tand T? as sequence element types, because that crashes the Swift compiler. I could provide specific functions for Sets, but that would still break with other sequence types that have similar requirements.

BTW: If we were to have those sequences of optional values, we would need to wrap the usage of NonNilMatcherFunclike this

public func allPass<T,U where U: SequenceType, U.Generator.Element == T>
    (matcher: NonNilMatcherFunc<T>) -> NonNilMatcherFunc<U> {
        let wrapper = NonNilMatcherWrapper(NonNilBasicMatcherWrapper(matcher))
        return createAllPassMatcher() {wrapper.matches($0, failureMessage: $1)}
}

because some of them like beLessThanwill always tell you that nil is less than anything else. Only the wrapper rejects nils. That's a nonissue now as there cannot be any nil elements in the actual value.

[jeffh]

NonNilMatcherFunc and MatcherFunc are API-conveniences (since defining functions using generic protocols is verbose), it might be better if allPass utilizes the underlying protocols instead: Matcher, BasicMatcher, NonNilBasicMatcher. Nimble currently doesn't provide a good abstraction to using these protocols since only Expectation needed to use those protocols.

That compiler crashing seems like a radar to file 🐛. Although I think having a more restrictive matcher is better than nothing. So if non-nil sequence types can only be matched, I think that's an acceptable compromise until Apple addresses that compiler bug.

@rbeeger, thanks again for the exploring the problem space.

[rbeeger]

The biggest problem is that NonNilBasicMatcher and BasicMatcher don't share a common super protocol. So you have to implement anything for both if you want to support both. In Swift 1.2 as now also works for protocols. So maybe there would be some way to provide such a common protocol and factor the specific handling with wrappers out of Expectation. I'll probably give it a try one of these days.

Anyways, always interesting to work on Nimble.

[rbeeger]

Unfortunately replacing the usage of NonNilMatcherwith NonNilBasicMatcher currently doesn't work. In AllPass.swift I changed the function dealing with the sub matcher like this

public func allPass<T,U,V where U: SequenceType, V: NonNilBasicMatcher, U.Generator.Element == T, V.ValueType == T>
    (matcher: V) -> NonNilMatcherFunc<U> {
        let wrapper = NonNilMatcherWrapper(NonNilBasicMatcherWrapper(matcher))
        return createAllPassMatcher() {wrapper.matches($0, failureMessage: $1)}
}

The static code analyzer seems to like it and compiling Nimble also seems to work, but the compiler crashes with a segmentation fault while compiling AllPassTest.swift for testing.

Hopefully XCode 6.3 beta 3 will be better at it.

[rbeeger]

OK, I found the problem. Tin my last comment wasn't used in the method itself. It was only needed to connect the sequence and the matcher. Removing Tand establishing a direct connetion U.Generator.Element == V.ValueTypefixed the problem.

[rbeeger]

I added support for Matcherand BasicMatchersub matchers, but that still doesn't enable you to check whether all elements of a collection are nil.

At first sight allPass behaves strangely when used with collections of optionals like [Int?]

The tests testAllPassCollectionsWithOptionalsDontWorkand testAllPassCollectionsWithOptionalsUnwrappingOneOptionalLayer show this strange behavior:
https://github.com/rbeeger/Nimble/blob/allPassMatcher/NimbleTests/Matchers/AllPassTest.swift#L38-L55

The problem is that we loose the information about having a collection of optionals here https://github.com/rbeeger/Nimble/blob/allPassMatcher/Nimble/Matchers/AllPass.swift#L34 and end up with optionals that are again wrapped. So instead of nilwe get `Òptional(nil)`` which isn't nil.

As the second of the two tests shows, this special Optional layer can be removed in the variant that takes a simple function by force unwrapping the element to be tested.

[jeffh]

Hey @rbeeger, I ran this against my Xcode 6.3 beta 2 and it seemed to run just fine (test passing and all). Maybe this is just another stale derived data issue that you've been experiencing?

[rbeeger]

@jeffh All the checked in tests also pass here. I wouldn't commit red tests 😀 .

In the first one two statements are wrapped in failsWithErrorMessage. If it worked the way I would like it to, those statements should run without failing, without the wrapper.

The second test shows how you can work around the current strange behavior.

The current situation is the best I can come up with for this matcher at this time. The two tests are just a warning to those using it. It shows that I am aware of this not fully satisfying situation with collections of optionals.

[rbeeger]

@jeffh I'm wondering if there is some serious problem with this pull request that keeps it from being merged and if there is something I could do to fix it.

It's not perfect, but the best I could come up with. And its little quirks don't look overly problematic to me.

[modocache]

This is tangential to this pull request, but I recently created a matcher similar to this one: https://github.com/modocache/Guanaco/blob/18126c0c8ba9a7cb3cc5d43aec83c81047410ab6/Guanaco/HaveSucceeded.swift#L21-L49

I also had to write several matcher functions in order to handle Matcher, BasicMatcher, and NonNilBasicMatcher. Architecturally, this seems like a shortcoming. Any thoughts on improvements here, @jeffh?

[jeffh]

Yeah, there needs to be some changes to simplify the internals. I think the removal a BasicMatcher and NonNilBasicMatcher would be preferred in some way. There also needs a reinvestigation if the swift compiler still gives an obscure message when a generic does match optionals. having the matcher be able to specify that while Nimble's Expectation and matcher protocols be ignorant of that detail would be best.

All that stuff are just floating in my head right now. For various personal reasons, I don't have the available free time yet.

[modocache]

I think this looks great. I'll merge tonight unless any @Quick/contributors want to chime in. 🌸

[ashfurrow]

Looks 🎉

[modocache]

Thanks again, @rbeeger! 👍