[rspec-users] Should change not comparing arrays how I expected

David Chelimsky dchelimsky at gmail.com
Sun Sep 28 14:52:07 EDT 2008


On Sun, Sep 28, 2008 at 1:44 PM, Michael Latta <lattam at mac.com> wrote:
> Is your patch AR proxy specific?  If it is for any collection, it prevents
> two collections from being compared for equality.  I have had many examples
> of collections that are not simple containers, and only comparing the
> contents would be equally invalid as the simple equality on AR proxies.  If
> you added an AR specific method / test that would have less impact on other
> cases, but of course be less clean.  That is why I suggested separate
> matchers, since I know of cases where your patch would be incorrect.

The patch is only in the rspec-rails plugin, and checks to see if it's
a proxy before doing anything, so it won't affect any other sort of
collection.

As for matching the collection, the comparison is done using == so it
passes or fails if the collections are considered equal according to
ruby's semantics for ==.

What you're proposing could be resolved with an argument constraint
that's been discussed in some other threads on this list - something
like:

  lambda {...}.should change{...}.to(array_consisting_of(...))

I'd prefer this as it lets us keep the single matcher, but allows us
to make it more flexible by adding different constraints. We already
have hash_including. This would expand that sort of capability.

WDYT?

>
> Michael
>
>
> On Sep 28, 2008, at 11:35 AM, David Chelimsky wrote:
>
>> On Sun, Sep 28, 2008 at 1:18 PM, Michael Latta <lattam at mac.com> wrote:
>>>
>>> David,
>>>
>>> It seems to me that the root of the problem is that the specification is
>>> incorrect.  Since Rails returns association proxies the specification
>>> fails
>>> because it does not specify what the behavior should be.  I would suggest
>>> that instead of patching the change matcher, that you should add a
>>> change_contents matcher that matches the contents of a collection vs. the
>>> contents of a collection.  That way the framework is not guessing what
>>> was
>>> meant, but relying on the specification to be correct.  Since that is
>>> really
>>> what you want to specify (the contents have changed).  I think this is
>>> cleaner.
>>
>> I think what you propose would be objectively more explicit, but
>> "cleanliness" is a very subjective thing.
>>
>> This is tricky because AR is simultaneously violating the principle of
>> least surprise by giving you a proxy instead of the real collection
>> AND adhering to principles of encapsulation and duck-typing. The fact
>> that team.players is a proxy should not matter to its consumer one way
>> or the other as long as it can treat it just like the collection of
>> players.
>>
>> Now, in the case of what rspec is doing in the change matcher, I would
>> say that rspec is getting bitten by the violation of the principle of
>> least surprise because the matcher is doing something that AR is not
>> expecting - further delaying evaluation of AR::A::AP's delayed
>> evaluation. But I think rspec can deal w/ that and alleviate the
>> burden on the rspec-rails user to know two different matchers and when
>> to use them.
>>
>> I've already implemented this so it's all transparent for the user:
>> http://rspec.lighthouseapp.com/projects/5645-rspec/tickets/545.
>>
>> I'm open to re-opening this but I'd like some feedback from other
>> people before I do. Anybody else have any opinions?
>>
>> David
>>
>>
>>
>>
>>> Michael
>>>
>>>
>>> On Sep 28, 2008, at 11:13 AM, David Chelimsky wrote:
>>>
>>>> On Sun, Sep 28, 2008 at 11:01 AM, David Chelimsky <dchelimsky at gmail.com>
>>>> wrote:
>>>>>
>>>>> On Sun, Sep 28, 2008 at 10:43 AM, David Chelimsky
>>>>> <dchelimsky at gmail.com>
>>>>> wrote:
>>>>>>
>>>>>> On Sun, Sep 28, 2008 at 9:47 AM, Ashley Moran
>>>>>> <ashley.moran at patchspace.co.uk> wrote:
>>>>>>>
>>>>>>> Hi
>>>>>>>
>>>>>>> Just had a surprising result:
>>>>>>>
>>>>>>> it "should not appear in the Story.unposted list" do
>>>>>>> @story.save
>>>>>>> lambda {
>>>>>>>  @story.post_to_twitter(@twitter_client)
>>>>>>> }.should change { Story.unposted }.from([@story]).to([])
>>>>>>> end
>>>>>>>
>>>>>>> 'Story#post_to_twitter should not appear in the Story.unposted list'
>>>>>>> FAILED
>>>>>>> result should have been changed to [], but is now []
>>>>>>>
>>>>>>> Anyone know why this fails?  I've looked in change.rb but I can't
>>>>>>> figure it
>>>>>>> out.
>>>>>>
>>>>>> Whenever I've seen output like "should have been foo, but was foo" it
>>>>>> has boiled down to AR Assocation Proxies, which don't align in their
>>>>>> response to == and inspect.
>>>>>>
>>>>>> I'm looking at seeing if there's a way we can make "should change"
>>>>>> work in spite.
>>>>>
>>>>> Wow.
>>>>>
>>>>> OK - here's what I figured out. Talk about insidious bugs! This is
>>>>> actually quite a bit different from what I thought.
>>>>>
>>>>> There are two lambdas involved here:
>>>>>
>>>>> lambda {
>>>>> 1st lambda: expression that should cause the change
>>>>> }.should change{
>>>>> 2nd lambda: expression that returns the object that should change
>>>>> }.to(expected value)
>>>>>
>>>>> The matcher executes the 1st lambda and stores the result as @before.
>>>>> In your example this is a Rails association proxy for the
>>>>> Story.unposted collection.
>>>>>
>>>>> The matcher then executes the 2nd lambda.
>>>>>
>>>>> The matcher then executes the 1st lambda again and stores the result
>>>>> as @after. In your example, this is, again, a Rails association proxy
>>>>> for the Story.unposted collection.
>>>>>
>>>>> At this point, @before and @after point to the same object - the same
>>>>> Rails association proxy!!!!!!
>>>>>
>>>>> The matcher passes if @before != @after and fails if @before ==
>>>>> @after. Because they are actually the same association proxy, the
>>>>> example fails.
>>>>>
>>>>> Now rspec asks the matcher to print out the reason why it failed,
>>>>> which does this:
>>>>>
>>>>> "#{result} should have been changed to #{@to.inspect}, but is now
>>>>> #{@after.inspect}"
>>>>>
>>>>> @to is the expected value []
>>>>> @after is the association proxy, which proxies to an empty collection.
>>>>> Now, when the matcher calls @after.inspect, is the first time that the
>>>>> proxy is actually evaluated!!!!
>>>>>
>>>>> Does this make sense?
>>>>>
>>>>> I was able to get a similar example to pass by doing this immediately
>>>>> after storing the proxy in the @before variable:
>>>>>
>>>>> @before = @before.collect{|item|item} if @before.respond_to?(:collect)
>>>>>
>>>>> Ugly, ugly, ugly. But perhaps necessary to deal w/ this problem.
>>>>>
>>>>> I think I'll restructure things so the the change matcher handles this
>>>>> in rails, but not in core rspec.
>>>>>
>>>>> Thoughts?
>>>>
>>>> FYI - ticket added and problem resolved:
>>>> http://rspec.lighthouseapp.com/projects/5645-rspec/tickets/545
>>>>
>>>>>
>>>>>>
>>>>>>
>>>>>>> I can make it work with:
>>>>>>> should change { Story.unposted.length }.from(1).to(0)
>>>>>>>
>>>>>>> But that's a weaker test.
>>>>>>>
>>>>>>> Thanks
>>>>>>> Ashley
>>>>>>>
>>>>>>> --
>>>>>>> http://www.patchspace.co.uk/
>>>>>>> http://aviewfromafar.net/
>>>>
>>>> _______________________________________________
>>>> rspec-users mailing list
>>>> rspec-users at rubyforge.org
>>>> http://rubyforge.org/mailman/listinfo/rspec-users
>>>
>>> _______________________________________________
>>> rspec-users mailing list
>>> rspec-users at rubyforge.org
>>> http://rubyforge.org/mailman/listinfo/rspec-users
>>>
>> _______________________________________________
>> rspec-users mailing list
>> rspec-users at rubyforge.org
>> http://rubyforge.org/mailman/listinfo/rspec-users
>
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>


More information about the rspec-users mailing list