rspec users - Sep 2008 - Should change not comparing arrays how I expected

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.

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/

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.

> 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
>

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?
>
>
>> 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
>>
>

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/

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.

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

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
>

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.

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

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
>

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

Michael Latta <lattam at mac.com> writes:
> 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 this is a leaky abstraction, and letting it leak out like that
is worse than having to do a little framework mojo on our part.

Pat

p.s. does anyone else think that "magic" sounds deragotory while
"mojo"
sounds cool?  I propose we say the rspec internals "got their mojo
workin"
rather than "magic" :)

On Sep 28, 2008, at 5:01 pm, David Chelimsky wrote:
> 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.
Hey,

Thanks for looking at this so quick!  I''m sure I should be paying for  
RSpec :)

I think I follow, except shouldn''t your references to lambdas 1 and 2  
be reversed above?

> 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?
Yes I sees what''s going on now.

But... I''m not using Rails, and in fact I''m not even writing a
web
app.  I''m using DataMapper, but clearly it violates the PoLS like AR.
What I''d give for an ORM that uses plain old Ruby objects!!!

Ashley


-- 
http://www.patchspace.co.uk/
http://aviewfromafar.net/

On Sep 28, 2008, at 7:52 pm, David Chelimsky wrote:
> 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?


On the basis that my code isn''t a Rails app*, but I''d still
like this
fix, I vote +1 for "array_consisting_of" (or something).

How about...
   lambda {...}.should change{...}.to(collection_equivalent_to(...))
?

Bit long winded but I think it expresses the intent?  (Correct me if  
not...)

As for change vs change_contents_of, I vote for keeping the one  
matcher.  I don''t think I should be expected to know when an object  
that replies to ==([]) is not an array.

Or an H R Giger alien for that matter...

Cheers
Ashley



* Ironically in my attempt to escape Rails, I have, in the same day,  
both been bitten by an ORM that behaves like ActiveRecord, and a  
different gem that loads ActiveSupport and borks ''require''
when I run
my specs.  =(

-- 
http://www.patchspace.co.uk/
http://aviewfromafar.net/

I think the difference comes down to whether you are writing specs or  
tests.  It is good enough for RSpec or Cucumber to fake out AR proxies  
if all you are doing is testing something.  If it is a spec, then I  
believe that having the framework fake out the test is very dangerous,  
because a random piece of Ruby code that does the same thing will get  
different results than the spec.  The bottom line is that AR has bad  
behavior in this regard.  But, having a spec that pretends it is  
otherwise only documents a false view of the world, and other  
programmers reading that spec will think that they can count on the  
results to be different than they are.  The result of the first call  
and the second are not different objects, but different contents.  It  
would be much better if AR was consistent in representing the result  
set over time, rather than changing the collection out from under you,  
but it does do that.

Just my $0.02, but I really like specs to be treated as specifications  
for what SHOULD be happening, not pretending other code is different  
than it is.

In this case the argument can be made on whether RSpec is introducing  
the change in behavior with its timing of the evaluations and  
comparisons, so in this case it is not black and white to me, but I  
would lean to more explicit acknowledgment of what AR is rather than  
what AR should be.  David''s suggestion of a constraint that shows the  
reader something odd is going on here with the proxies and give them  
pause before using the proxy in the same way the RSpec code did  
initially.

Michael




On Sep 28, 2008, at 3:09 PM, Ashley Moran wrote:
>
> On Sep 28, 2008, at 7:52 pm, David Chelimsky wrote:
>
>> 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?
>
>
>
> On the basis that my code isn''t a Rails app*, but I''d
still like
> this fix, I vote +1 for "array_consisting_of" (or something).
>
> How about...
>  lambda {...}.should change{...}.to(collection_equivalent_to(...))
> ?
>
> Bit long winded but I think it expresses the intent?  (Correct me if  
> not...)
>
> As for change vs change_contents_of, I vote for keeping the one  
> matcher.  I don''t think I should be expected to know when an
object
> that replies to ==([]) is not an array.
>
> Or an H R Giger alien for that matter...
>
> Cheers
> Ashley
>
>
>
> * Ironically in my attempt to escape Rails, I have, in the same day,  
> both been bitten by an ORM that behaves like ActiveRecord, and a  
> different gem that loads ActiveSupport and borks
''require'' when I
> run my specs.  =(
>
> -- 
> http://www.patchspace.co.uk/
> http://aviewfromafar.net/
>
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users