[rspec-devel] [ rspec-Patches-6417 ] [PATCH] Support @object.some_method_should_be_changed_by {block}

noreply at rubyforge.org noreply at rubyforge.org
Wed Nov 15 11:08:19 EST 2006


Patches item #6417, was opened at 2006-10-31 16:36
You can respond by visiting: 
http://rubyforge.org/tracker/?func=detail&atid=3151&aid=6417&group_id=797

Category: None
Group: None
Status: Open
Resolution: None
Priority: 3
Submitted By: Wilson Bilkovich (wilson)
Assigned to: Nobody (None)
Summary: [PATCH] Support @object.some_method_should_be_changed_by {block}

Initial Comment:
I was toying with how to implement the RSpec equivalent of "assert_difference" from test/unit.
In case you haven't used it, it looks something like this:
assert_difference some_object, :some_method { some code here }

It calls some_object.some_method before and after the block, and fails if there is no difference.

This patch implements something similarly generic:

@some_object.some_method_should_be_changed_by {|obj| code here}
..and its inverse:
@some_object.some_method_should_not_be_changed_by {|obj| code here}

some_method is stripped from the beginning of the message, and passed forward for use inside the Should module.

Has specs, and all other existing specs still pass.

A real 'use case' example:

User.count_should_be_changed_by do
  User.create :login => 'quentin', :email => 'blah at example.com'...
end

User.count will be different before and after, so this expectation will be met.

----------------------------------------------------------------------

>Comment By: Wilson Bilkovich (wilson)
Date: 2006-11-15 11:08

Message:
That's precisely why I implemented it this way. I would much
prefer to be able to write:
User.count.should_be_changed..etc.etc..
..but without a way to get a handle on the "method that was
called to return the object that should was called on", I
couldn't think of another way.

Also, re: sugar.rb
Here's a line of code from trunk:
return original_method_missing.bind(self).call(sym, *args,
&block) unless sym.to_s =~ /^should_/
As you can see, sugar assumes that the message being
intercepted begins with 'should'. Because this patch
requires a different behavior, I will still need to
monkey-patch sugar.rb.

I'm happy to do this as a plugin, it just looks like I would
need to release a new version almost every time the trunk
changed.

The other way to implement this would be something like:
User.should_return_different_results_for(:count).after {
block here}

..but that seems clunky.  I like the way this version reads,
myself.

----------------------------------------------------------------------

Comment By: Brandon Keepers (brandon)
Date: 2006-11-15 09:26

Message:
Ok, looking into this a little, I see why it is implemented how it is.  The idea is that you want to check that the result of a method call is changed by a block, not necessarily the referenced object itself.  It happens to work if the referenced object is being modified, but what about when it's just being re-assigned?

For example, this would work because :count is being incremented:

  User.count.should_be_changed_by(1) { User.create(...) }

But, this would fail:

  @user.name.should_be_changed_to("Jim") { @user.name = "Jim" }

should_be_changed_to is looking that the referenced variable changes, but in the block, I'm just reassigning it, so the original reference to name is not modified.

Thoughts?

----------------------------------------------------------------------

Comment By: Brandon Keepers (brandon)
Date: 2006-11-15 09:06

Message:
So, the main issue is just the implementation, and not the feature itself?  I agree with you about it being too non-deterministic.

I like the examples that you gave, it feels more like original the assert_difference.

I'd also like to see "should_be_changed_to(newval)"

Maybe I'll get myself familiar with the rspec internals and actually contribute instead of standing on the sidelines. ;)

----------------------------------------------------------------------

Comment By: David Chelimsky (dchelimsky)
Date: 2006-11-15 06:30

Message:
Brandon - re: inclusion as a core feature:

I think it is too non-deterministic as/is. In the example, one could override (accidentally, of course) User.create to delete something and the spec would pass.

We *could* make it more deterministic:

User.count.should_be_changed_by('+ 1') do {...}

or

User.count.should_be_changed_from(3).to(4) do {...}

One thing that concerns me is that we lose the separation between context and outcome that is essential to BDD. 

Although it is very behavioral in nature. I'm on the fence.

Thoughts?

----------------------------------------------------------------------

Comment By: David Chelimsky (dchelimsky)
Date: 2006-11-15 06:22

Message:
Wilson - re: your question about plugins and sugar:

sugar.rb now only intercepts the :should_ part of a :should_do_something message. What you want to extend is the Should class itself (in this case with a 'changed_by' method that takes a block), and the Not class if you want a negative counterpart.

Be forewarned that we're not committing to the current structure at this point, so you'd be incurring some risk that a future version of RSpec might break your plugin because you're patching something internal. We will, however, at some point soon (within a month or two I think), publish (and thereby commit to) an entry point to extending expectations.

----------------------------------------------------------------------

Comment By: Brandon Keepers (brandon)
Date: 2006-11-14 20:00

Message:
What is the reasoning for not including this as a core feature?  This is a very helpful expectation.

----------------------------------------------------------------------

Comment By: Aslak Hellesøy (aslak_hellesoy)
Date: 2006-11-06 13:11

Message:
I don't think anyone has thought much about how a plugin mechanism will actually be implemented. We'll try as hard as we can to make something forward compatible and easy to use.

Your patch here would be a good case to verify a plugin mechanism against. 

I actually implemented this very feature a couple of months ago (rel 0.5.14). We decided to remove the day after. The thread about it starts here:

http://rubyforge.org/pipermail/rspec-devel/2006-July/000324.html


----------------------------------------------------------------------

Comment By: Wilson Bilkovich (wilson)
Date: 2006-11-06 12:51

Message:
Hrm. Are plugins going to be able to modify sugar.rb in a
forward-compatible way?
I'm not sure how to implement this without stepping in that
early in the chain.

----------------------------------------------------------------------

Comment By: Aslak Hellesøy (aslak_hellesoy)
Date: 2006-11-06 12:45

Message:
I think this is the kind of stuff we'd encourage people to bundle as an RSpec plugin. We don't have a plugin mechanism yet, but we'll hopefully have one soon.

----------------------------------------------------------------------

You can respond by visiting: 
http://rubyforge.org/tracker/?func=detail&atid=3151&aid=6417&group_id=797


More information about the rspec-devel mailing list