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

Make all matchers composable for 3.0 #280

Closed
myronmarston opened this issue Jun 30, 2013 · 9 comments · Fixed by #393
Closed

Make all matchers composable for 3.0 #280

myronmarston opened this issue Jun 30, 2013 · 9 comments · Fixed by #393
Assignees
Milestone

Comments

@myronmarston
Copy link
Member

To me, the real value of using matcher objects rather than simple assert_xyz methods is the extra power matcher objects give you that simple methods don't. In particularly, hamcrest, discussed in GOOS, allows matchers to be fully composed so that you can express detailed intent through a combination of matchers. In rspec-expectations, we have some places where composition currently works:

expect {
  record.save
}.to change { record.created_at }.from(nil).to(be_within(1.second).of(Time.now))

expect(list).to include(match(/foo/), match(/bar/))

...but it's not yet supported everywhere. For example, this expression doesn't work yet:

expect { |b|
  foo(&b)
}.to yield_with_args(include(match(/foo/), match(/bar/)))

...which means "I expect foo to yield with a collection that includes an element matching the regex /foo/ and an element matching the regex /bar/". In this case, we're composing 3 different matchers.

I think it would be a great new feature for 3.0 to make all matchers fully composable. Things to consider:

expect {
  record.save
}.to change { record.created_at }.from(nil).to(a_value_within(1.second).of(Time.now))

expect(list).to include(a_string_matching(/foo/), a_string_matching(/bar/))

expect { |b|
  foo(&b)
}.to yield_with_args(a_collection_including(a_string_matching(/foo/), a_string_matching(/bar/)))
  • If we want to provide those aliases, we can make a module like RSpec::Matchers::ComposableAliases that defines aliases for each of the built-ins in this kind of voice, and provide a config API that will include the module. (I think I'd want it opt-in since the aliases will squat on a lot more name real estate that users might otherwise be able to use).
  • We may want to add some additional matchers that help with boolean operations. E.g. a negation matcher (so that you can wrap any matcher in the negation matcher and pass that as an argument), an and matcher and an or matcher. We'd have to figure out the right phrasing for all these.
  • We'd want the failure messages to read well in these cases, and not simply include the inspect output of the composed matchers.

Thoughts?

@JonRowe
Copy link
Member

JonRowe commented Jul 1, 2013

I'm in favour of this, I've often run into situations where I want to use matchers in a more composable partial fashion.

As discussed in #161 and in #277, it would be good to switch to === rather than == for this, as === means "matches" in ruby, where as == means "equals", and === makes much more sense (IMO).

In my experience == is usually used to mean an object comparable to another, where as === is used to mean this is the same object, so I'm unsure wether the semantics work for us to use this, but I'm not averse to it.

If we want to provide those aliases, we can make a module like RSpec::Matchers::ComposableAliases that defines aliases for each of the built-ins in this kind of voice, and provide a config API that will include the module. (I think I'd want it opt-in since the aliases will squat on a lot more name real estate that users might otherwise be able to use).
We may want to add some additional matchers that help with boolean operations. E.g. a negation matcher (so that you can wrap any matcher in the negation matcher and pass that as an argument), an and matcher and an or matcher. We'd have to figure out the right phrasing for all these.

One thing I'm keen to try, is to use constants rather than aliased over methods, altho we could provide those in a configurable fashion on top. I'm not sure this style would work here but would avoid over "squatting".

We'd want the failure messages to read well in these cases, and not simply include the inspect output of the composed matchers.

I agree this is very important.

@cupakromer
Copy link
Member

👍 ❤️ I'm very much in favor for this. I agree with all of @myronmarston suggestions above.

As discussed in 161 and in 277, it would be good to switch to === rather than == for this, as === means "matches" in ruby, where as == means "equals", and === makes much more sense (IMO).

I think this is the proper way to go. Per Object#=== this is the case equality statement, which IMHO makes perfect sense for a matcher:

# Just showing semantic meaning not that we would use matchers like this
case obj
when a_string_matching(/foo/)
when a_value_within(1.second).of(Time.now)
# ...

This is actually a trick commonly used with procs to provide more semantic case statements.

The grammatical "voice" of the existing matchers is good for use in a basic expect().to expression, but poor when composed as an argument to another matcher.

I actually ran into this yesterday with match_array. I ended up adding an alias array_matching. So this is something to put some thought into if readability is a goal. Though, it will add more to the namespace as you pointed out.

We'd want the failure messages to read well in these cases, and not simply include the inspect output of the composed matchers.

I believe we briefly talked about that on IRC. 👍 It is definitely something that makes it hard to debug these types of chains.

One other thing that I think should be given consideration is how this fits in to the matcher DSL and custom matcher classes.

@myronmarston
Copy link
Member Author

In my experience == is usually used to mean an object comparable to another, where as === is used to mean this is the same object, so I'm unsure wether the semantics work for us to use this, but I'm not averse to it.

== is the equality operator. === is the case/match operator. Not sure where you learned that === is used to mean it's the same object...but that's not what that operator is used for at all. In fact, there are core types for which === returns false when passed the receiver:

1.9.3-p327 :001 > String === String
 => false
1.9.3-p327 :002 > String === "a string"
 => true

This is because it's meant to be used in case statements, and Class#=== is meant to match instances.

@JonRowe
Copy link
Member

JonRowe commented Jul 13, 2013

I've always considered == equal and === strictly equal so that behaviour is as I would expect, String is not a String but a class. I would more normally use 'a string' === String. It's possible that's a hangup from other languages though.

@myronmarston
Copy link
Member Author

I've always considered == equal and === strictly equal so that behaviour is as I would expect, String is not a String but a class

You must have a different understanding of "strict" than I do. Consider that there is exactly one object (String) for which String.== will return true, but there are an infinite number of objects (any string) for which String.=== will return true. By my understanding of "strict", String.== is certainly much stricter than String.===. (But maybe I'm not understanding what sense you mean "strict"?)

I would more normally use 'a string' === String

That expression is not equivalent to String === 'a string'. It returns false:

"1.9.3-p327 :001 > "a string" === String
 => false
1.9.3-p327 :002 > String === "a string"
 => true

Note that === is not commutative, because it's not an equality operator...it's a "match" operator, for use in case statements, rescue clauses, etc.

It's possible that's a hangup from other languages though.

I wonder if you're confusing the semantics of Javascript's ==/=== vs Ruby's ==/===? I haven't done much javascript, but as I understand it, Javascript's === is stricter than ==.

@JonRowe
Copy link
Member

JonRowe commented Jul 14, 2013

Who knows, I haven't used it much obviously :) I'll defer to you here as I'm obviously mistaken

@alindeman
Copy link
Contributor

I think @myronmarston is right. In languages like PHP and JavaScript, === is a more strict equality, but in Ruby, it's more of a "match"

@avdi once termed it "magic fuzzy match" :)

@eloyesp
Copy link
Contributor

eloyesp commented Sep 26, 2013

I would really like something like:

# Composed matcher

expect({ foo: 'bar'}).to be_kind_of(Hash).and(have(1).item).
  and(include(:foo)).and_not(include(:bang))

# much better than repeated code.

expect({ foo: 'bar'}).to     be_kind_of(Hash)
expect({ foo: 'bar'}).to     have(1).item
expect({ foo: 'bar'}).to     include(:foo)
expect({ foo: 'bar'}).to_not include(:bang)

This would be awesome for me, as I found myself much times doing something like:

hash_that_interest = { foo: 'bar'} # set up first so I don't repeat it on every expectation.
expect(hash_that_interest).to     be_kind_of(Hash)
expect(hash_that_interest).to     have(1).item
expect(hash_that_interest).to     include(:foo)
expect(hash_that_interest).to_not include(:bang)

@myronmarston
Copy link
Member Author

Closing in favor of #393 since we'll be merging that soon.

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

Successfully merging a pull request may close this issue.

5 participants