rspec / rspec-expectations

Provides a readable API to express expected outcomes of a code example
https://rspec.info
MIT License
1.26k stars 395 forks source link

Idea: `each_satisfy` matcher #75

Closed alindeman closed 12 years ago

alindeman commented 13 years ago

Recently ran across some coworker's code who had written:

results.each { |r| r.cost.should be <= 5 }

While this is pretty readable, I wondered if it would make sense to write an rspec matcher that used #each (Enumerable) so the syntax could be even clearer:

results.should each_satisfy { |r| r.cost <= 5 }

The second syntax would also be easier to make diffable.

Let me know thoughts. If agreed, I can make it happen.

alindeman commented 13 years ago

This could also be written as results.should be_all { |r| r.cost <= 5 } but we still lose out on diffability ("expected all? to return true, got false") .. and it still reads a little bit oddly IMHO.

justinko commented 13 years ago

I like it.

rentalcustard commented 13 years ago

I think "all_satisfy" would read a bit clearer.

justinko commented 13 years ago

@mortice - the problem with that is it gives off the impression that the expectation is on the collection, and not on each collection item. Basically, people with associate it with the Ruby all? method.

rentalcustard commented 13 years ago

@justinko fair point, but I don't see the problem with the association with 'all?'. all_satisfy would match whenever all? came back true for the collection.

justinko commented 13 years ago

@mortice - please read the first comment. This matcher aims to set an expectation on each item in the collection, not on the collection as a whole.

@alindeman - with that said, I'm not sure how you're going to "diff" this. Since the blocks return true or false, and don't contain should, I don't think it is possible. I'm sure there is a reason this type of matcher hasn't been added yet, and I suspect this is why.

alindeman commented 13 years ago

@mortice, @justinko My idea for each_satisfy is very similar to all? from Enumerable. I am not tied to the name and welcome more debate/suggestions on it.

@justinko By diffable, I meant we could print out the items that did not satisfy the block (e.g., "expected each item to satisfy the block, but elements ['foo', 'bar', 'baz'] did not")

Great feedback so far. Will wait for a bit more before running with it.

dchelimsky commented 13 years ago

There was a lighthouse ticket for this a few years back and it never got resolved because there were a few tricky parts. We're opening up a bit of a can of worms here, because there are lots of iterators and a single API can't cover them all elegantly. Consider list.any? {|i| i > 3}.should be_true. each_satisfy or all_satisfy wouldn't cover this, and list.should any_satisfy {...} doesn't read all that well.

I'm open to this, but not 1/2 way.

Thoughts?

dchelimsky commented 13 years ago

Just throwing out an idea here to see if it sticks:

list.all.should satisfy { ... }
list.any.should satisfy { ... }
list.none.should satisfy { ... }
wallace commented 13 years ago

+1 on this idea!

dchelimsky commented 13 years ago

Of course the problem with that ^^ is that it adds methods to collections. How about this:

all_in(list).should satisfy { ... }
any_in(list).should satisfy { ... }
none_in(list).should satisfy { ... }
justinko commented 13 years ago
list.all.should satisfy { ... }

Yeah, the above would conflict with Rails. You'd have to do users.all.all.should ...

I agree with David, this is opening a can of worms, and is best to leave up to Ruby:

list.all? {|item| item < 5 }.should be_true

One can of worms (rspec-mocks's any_instance) is enough :)