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

Michael Latta lattam at mac.com
Sun Sep 28 16:25:06 EDT 2008


The constraint looks good.  ar_proxy_of(...) for this case?  Or is  
your constraint specified as making a copy?

Your patch seems to be narrow enough that that is also workable.  As  
you say, it is Rails that is causing the surprise.

Michael



On Sep 28, 2008, at 11:52 AM, David Chelimsky wrote:

> 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
>>
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users



More information about the rspec-users mailing list