Skip to content

Loading…

Recommend wrapping or aliasing matchers #1

Open
dchelimsky opened this Issue · 7 comments

2 participants

@dchelimsky

This is from the README:

it { should assign :blah => be_a(String) }

Using the infinitive form of the verb reads funny in this context, so I'd recommend aliasing that with a present tense form or an adjective:

it { should assign :blah => is_a(String) }
it { should assign :blah => instance_of(String) }
@dchelimsky

Similarly, the description should be "should assign a String to @blah" rather than "should assign @blah to be a kind of String". Might want to consider a set of custom matchers for this.

@sj26
Owner

Agreed, I was not aware of alternate forms of these matchers, or are you suggesting adding aliases? This was the primary reason I was using assign(:blah).to be_a String.

I'd be hesitant to alias every core rspec matcher to cater for this particular matcher.

@dchelimsky

I appreciate the hesitation, but I wonder how far this really needs to go. Besides literal values and is_a, what other matchers do you see being used?

@dchelimsky

BTW - rspec-mocks has a bunch of these (see ArgumentMatchers on http://rubydoc.info/github/rspec/rspec-mocks/master/file/README.md). Maybe in rspec-3 we should normalize the matchers and pull them out to a separate library like Hamcrest.

@sj26
Owner

True, I was mainly thinking of be_<predicate> matchers, like be_empty, be_admin, etc., which can be useful when testing properties of an exposed ActiveRecord object for instance. This could be achieved easily by adding a method_missing catch for is_<predicate> aliased to BePredicate.

Unfortunately most of my company's projects use RR. :-( I've found it quite restrictive lately, mainly in testing blocks, and am trying to spearhead a change to rspec mocks.

It would be nice to have a ubiquitous set of matchers. A lot of matcher functionality was reimplemented inside AssignMatcher because I couldn't find a nice way to reuse it. Hamcrest looks neat!

I think an ideal implementation of this matcher would actually be a subject modifier, not a matcher at all. Something in the vein of it_assigns(:blah) { should be_present }.

@dchelimsky

I'd avoid adding new syntax. The current form is consistent with the it { should matcher } form. I also think it reads better as/is. The spec is that the controller assigns something. That the something should have some quality is a secondary aspect. Saying it_assigns(:var) { should match(value) } reverses those roles and doesn't read as well IMO.

@sj26
Owner

Again agreed, which is why I chose the matcher route. I think form-changing matcher aliases sate my desire for readability. :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.