Amiruddin Nagri
2010-Jun-10 03:31 UTC
[rspec-users] Custom Matchers : Shouldn''t they follow DRY principle ?
I am trying to write custom matchers for RESTful behavior of my controllers. I have created two matchers *be_created *( http://github.com/anagri/rspec_matchers/blob/80d0d021a7ad1e9af2ff18e79b80c004324abdc7/spec/support/http_status_matcher.rb) *have_created_resource *( http://github.com/anagri/rspec_matchers/blob/80d0d021a7ad1e9af2ff18e79b80c004324abdc7/spec/support/restful_resource_matcher.rb) In be_created, I check for the status code to be 201 and a location of the new resource. In have_created_resource, i also pass in the resource that was created and verify it exists (non nil check) as well as that it passes the be_created criteria. Github commit => http://github.com/anagri/rspec_matchers/commit/80d0d021a7ad1e9af2ff18e79b80c004324abdc7 I am getting an error saying be_created not found. Obviously, be_created is not in scope but if I fix it and include the required module it fails for be_nil not found. I am finding this way of reusing the matchers not correct. Is there a standard way of re-using matchers inside matchers ? Have anybody tried it out ? Regards, Amiruddin Nagri, Bangalore, 560008, KA India Y! IM : amir_nagri at yahoo.com GTalk : amir.nagri at gmail.com -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://rubyforge.org/pipermail/rspec-users/attachments/20100610/9387f682/attachment.html>
David Chelimsky
2010-Jun-10 06:08 UTC
[rspec-users] Custom Matchers : Shouldn''t they follow DRY principle ?
On Wed, Jun 9, 2010 at 11:31 PM, Amiruddin Nagri <amir.nagri at gmail.com> wrote:> > I am trying to write custom matchers for RESTful behavior of my controllers. > > I have created two matchers > be_created ( > http://github.com/anagri/rspec_matchers/blob/80d0d021a7ad1e9af2ff18e79b80c004324abdc7/spec/support/http_status_matcher.rb > ) > have_created_resource ( > http://github.com/anagri/rspec_matchers/blob/80d0d021a7ad1e9af2ff18e79b80c004324abdc7/spec/support/restful_resource_matcher.rb > ) > > In be_created, I check for the status code to be 201 and a location of the > new resource. In have_created_resource, i also pass in the resource that was > created and verify it exists (non nil check) as well as that it passes the > be_created criteria. > > Github commit => > http://github.com/anagri/rspec_matchers/commit/80d0d021a7ad1e9af2ff18e79b80c004324abdc7 > > I am getting an error saying be_created not found. Obviously, be_created is > not in scope but if I fix it and include the required module it fails for > be_nil not found. > I am finding this way of reusing the matchers not correct. > > Is there a standard way of re-using matchers inside matchers ? Have anybody > tried it out ?You''ll have access to the other matchers in rspec-2 if you use the matcher DSL (see the rdoc on http://github.com/rspec/rspec-expectations/blob/master/lib/rspec/matchers.rb). Regarding DRY: while it is a very important principle, it is not the only principle. The matches? method in have_created_resource exhibits the Long Method and Feature Envy code smells, and violates the Tell, Don''t Ask principle. Using be_created the way it''s being used is operating with lower level details of the matcher framework, letting those abstractions leak up to the matcher. I''d just write it like this (using the matcher DSL): RSpec::Matchers.define :have_created_resource do |resource, location| match do |response| response.code == "201" && resource end failure_message_for_should do |response| <<-MESSAGE expected to have created resource, but got: response code: #{response.code} resource: #{resource} MESSAGE end end Now each method does one thing. The match method does what it''s supposed to: deciding whether or not to match. The failure_message_for_should does what it''s supposed to: create a failure message. The failure message is better aligned with the matcher, and it''s always the same format regardless of why it failed. There''s also no dependency between the matchers now, so they are free to change independently. HTH, David