mocha developer - Oct 2008 - Stubbing objects and calling to_json on them errors (circular reference)

My apologies if there''s a user list I should be posting this to. I  
didn''t find one.

Suppose you have a controller action like this:

def do_something
   @something = Something.find(:first)
   @something.do_whatever
   render :json => @something.to_json
end

A test might be this:

context "when do_something is called" do
   setup do
     @something = Something.new()
     Something.expects(:find).returns(@something)
     @something.stubs(:do_whatever)
     get :do_something
   end
   should_respond_with :success
end

Because I stubbed a method on my @something object (and this also  
happens with expects()), I get this:

1) Error:
test: when do_something is called should respond with success.  
(SomethingControllerTest):
ActiveSupport::JSON::CircularReferenceError: object references itself

If I print out the object @something, I see that it contains an @mocha  
attribute that references the containing object, which is the cause of  
to_json''s heartburn and the error. But that @mocha attribute is not  
present if I never call stubs() or expects() on the object that''s  
going to be converted to JSON, and I do not get the error in that  
case. Can anyone show me what I''m doing wrong, or is this actually a  
known issue?

--
Bill Kocik

On Oct 20, 2008, at 8:14 AM, Bill Kocik wrote:
> Suppose you have a controller action like this:
>
> def do_something
>  @something = Something.find(:first)
>  @something.do_whatever
>  render :json => @something.to_json
> end
>
> A test might be this:
>
> context "when do_something is called" do
>  setup do
>    @something = Something.new()
>    Something.expects(:find).returns(@something)
>    @something.stubs(:do_whatever)
>    get :do_something
>  end
>  should_respond_with :success
> end
>
> Because I stubbed a method on my @something object (and this also  
> happens with expects()), I get this:
>
> 1) Error:
> test: when do_something is called should respond with success.  
> (SomethingControllerTest):
> ActiveSupport::JSON::CircularReferenceError: object references itself
Hi Bill,

I would suggest not using an actual instance of Something at all. You  
are testing SomethingController, not Something?and you are already  
using mocking/stubbing?so why not just have:

     context "when do_something is called" do
       setup do
         something = stub_everything
         Something.stubs(:find).returns(something)
         get :do_something
       end
       should_respond_with :success
     end

-- 
Regards,

John Wilger

On  20 Oct, 2008, at 2:01 PM, John Wilger wrote:
> Hi Bill,
>
> I would suggest not using an actual instance of Something at all.  
> You are testing SomethingController, not Something
I''m right there with ya, John. Sadly the situation I''m in is
that I''ve
just joined this project, and it''s kind of a mess. In some cases I can
mock the object (and I do), but in others there are so many calls to  
the object, and then calls to objects that come back from those calls,  
that stubbing all that stuff out is a little bit prohibitive. It''s in  
these situations that I opt to create a genuine object. Our first step  
in the cleanup effort is to try to increase our test coverage to a  
level we''re comfortable with, so for now I''m stuck with
writing less-
than-ideal tests for less-than-ideal code.

I think a work-around I can use for now (until we can clean all of  
this up) is to stub the to_json method.

--
Bill Kocik

On Oct 20, 2008, at 12:10 PM, Bill Kocik wrote:
>
> I''m right there with ya, John. Sadly the situation I''m in
is that
> I''ve just joined this project, and it''s kind of a mess.
In some
> cases I can mock the object (and I do), but in others there are so  
> many calls to the object, and then calls to objects that come back  
> from those calls, that stubbing all that stuff out is a little bit  
> prohibitive. It''s in these situations that I opt to create a
genuine
> object. Our first step in the cleanup effort is to try to increase  
> our test coverage to a level we''re comfortable with, so for now
I''m
> stuck with writing less-than-ideal tests for less-than-ideal code.
Using `stub_everything` as opposed to just `stub` can alleviate that  
to some degree, however the one thing I don''t like about Mocha is  
`stub_everything` returns nil for all calls. If it instead returned  
`self` by default, then it would work better for this kind of  
situation. (If anyone knows that there /is/ a way to cause Mocha to  
behave this way, please speak up!)

-- 
Regards,

John Wilger

On Mon, Oct 20, 2008 at 3:10 PM, Bill Kocik <bkocik at gmail.com>
wrote:
> Our first step in the cleanup effort is to
> try to increase our test coverage to a level we''re comfortable
with, so for
> now I''m stuck with writing less-than-ideal tests for
less-than-ideal code.
Bill,
I realize it''s been a few days and apologize for the late response,
but I just saw this. I hate to be the guy on the mailing list saying
"you shouldn''t be trying to do what you''re trying to
do," but that''s
sort of where I''m headed. I hope it''s helpful anyway.

When adopting a codebase that wasn''t developed test-driven,
I''d
strongly recommend focusing on Rails-style functional tests rather
than introducing mock-based testing of the current design. The biggest
downside to mocking is that it ties you to certain interactions
between objects. When you''re using it to determine what those
interactions should be, that''s ok. When you already know
you''re not
happy with the way those interactions work and intend to clean them
up, what you really need is tests that give you a safety net to enable
refactoring (a green bar that you can keep green as you improve the
design). Mock-based tests aren''t likely to give you that. Rather
they''re probably going to mean that as you skinny down your
controllers, you''ll have to change everything in at least two places
instead of just one, and meanwhile the suite won''t tell you whether
things are still working.

Mock-based tests are a useful design tool, and can (in some cases) be
useful documentation of design, but they don''t usually make for a good
regression suite. All three of those are important when developing
from scratch. But when taking on legacy code whose design you intend
to change, the first two goals are off the table. The third is better
served with higher-level tests.

It''s definitely worth your time to read the 12-page version of Michael
Feathers'' Working Effectively With Legacy Code.
www.objectmentor.com/resources/articles/WorkingEffectivelyWithLegacyCode.pdf

I hope that wasn''t too obnoxious.
-hume.

-- 
http://elhumidor.blogspot.com/

Please don''t apologize - this is probably the best, most well- 
delivered advice I''ve ever received from a mailing list, and I greatly
appreciate it. I''ve actually sort of thought the same thing; what
I''m
doing now is testing implementation rather than results, and while I  
knew it wasn''t what I should be doing I wasn''t quite sure what
that
thing I should be doing was.

Thanks very much for taking the time to spell some of this out for me.  
You''ve taught me something, which I''m grateful for, and
I''m going to
share your advice with my direct manager and suggest that we consider  
a change in testing focus.

On  24 Oct, 2008, at 8:49 AM, John D. Hume wrote:
> On Mon, Oct 20, 2008 at 3:10 PM, Bill Kocik <bkocik at gmail.com>
wrote:
>> Our first step in the cleanup effort is to
>> try to increase our test coverage to a level we''re comfortable
>> with, so for
>> now I''m stuck with writing less-than-ideal tests for
less-than-
>> ideal code.
>
> Bill,
> I realize it''s been a few days and apologize for the late
response,
> but I just saw this. I hate to be the guy on the mailing list saying
> "you shouldn''t be trying to do what you''re trying to
do," but that''s
> sort of where I''m headed. I hope it''s helpful anyway.
>
> When adopting a codebase that wasn''t developed test-driven,
I''d
> strongly recommend focusing on Rails-style functional tests rather
> than introducing mock-based testing of the current design. The biggest
> downside to mocking is that it ties you to certain interactions
> between objects. When you''re using it to determine what those
> interactions should be, that''s ok. When you already know
you''re not
> happy with the way those interactions work and intend to clean them
> up, what you really need is tests that give you a safety net to enable
> refactoring (a green bar that you can keep green as you improve the
> design). Mock-based tests aren''t likely to give you that. Rather
> they''re probably going to mean that as you skinny down your
> controllers, you''ll have to change everything in at least two
places
> instead of just one, and meanwhile the suite won''t tell you
whether
> things are still working.
>
> Mock-based tests are a useful design tool, and can (in some cases) be
> useful documentation of design, but they don''t usually make for a
good
> regression suite. All three of those are important when developing
> from scratch. But when taking on legacy code whose design you intend
> to change, the first two goals are off the table. The third is better
> served with higher-level tests.
>
> It''s definitely worth your time to read the 12-page version of
Michael
> Feathers'' Working Effectively With Legacy Code.
>
www.objectmentor.com/resources/articles/WorkingEffectivelyWithLegacyCode.pdf
>
> I hope that wasn''t too obnoxious.
> -hume.
>
> -- 
> http://elhumidor.blogspot.com/
> _______________________________________________
> mocha-developer mailing list
> mocha-developer at rubyforge.org
> http://rubyforge.org/mailman/listinfo/mocha-developer


--
Bill Kocik

2008/10/20 Bill Kocik <bkocik at gmail.com>
>
> My apologies if there''s a user list I should be posting this to. I
didn''t
> find one.

No worries. This list serves as both user & developer list.

> Suppose you have a controller action like this:
>
> def do_something
>  @something = Something.find(:first)
>  @something.do_whatever
>  render :json => @something.to_json
> end
>
> A test might be this:
>
> context "when do_something is called" do
>  setup do
>    @something = Something.new()
>    Something.expects(:find).returns(@something)
>    @something.stubs(:do_whatever)
>    get :do_something
>  end
>  should_respond_with :success
> end
>
> Because I stubbed a method on my @something object (and this also happens
> with expects()), I get this:
>
> 1) Error:
> test: when do_something is called should respond with success.
> (SomethingControllerTest):
> ActiveSupport::JSON::CircularReferenceError: object references itself
>
> If I print out the object @something, I see that it contains an @mocha
> attribute that references the containing object, which is the cause of
> to_json''s heartburn and the error. But that @mocha attribute is
not present
> if I never call stubs() or expects() on the object that''s going to
be
> converted to JSON, and I do not get the error in that case. Can anyone show
> me what I''m doing wrong, or is this actually a known issue?
>
> --
> Bill Kocik
>
This sounds like a bug. I''m going to add a bug report to track this
[1].
Thanks for letting us know about it.

-- 
James.
http://blog.floehopper.org

[1] http://rubyforge.org/tracker/index.php?group_id=1917&atid=7477

2008/10/20 John Wilger <johnwilger at gmail.com>
> Using `stub_everything` as opposed to just `stub` can alleviate that to
> some degree, however the one thing I don''t like about Mocha is
> `stub_everything` returns nil for all calls. If it instead returned `self`
> by default, then it would work better for this kind of situation. (If
anyone
> knows that there /is/ a way to cause Mocha to behave this way, please speak
> up!)
>
Hi John,

Thanks for your message. There isn''t currently a way to do this.
I''m not
convinced that always returning "self" would be sensible, but I can
see the
case for having control over the default return value. I''ve added a
feature
request [1] to track this and I''ll have a think about how the API would
work. In the meantime if you have any ideas, please let me know.

Thanks.
-- 
James.
http://blog.floehopper.org

[1]
http://rubyforge.org/tracker/index.php?func=detail&aid=22564&group_id=1917&atid=7480

2008/10/24 John D. Hume <duelin.markers at gmail.com>
> Bill,
> I realize it''s been a few days and apologize for the late
response,
> but I just saw this. I hate to be the guy on the mailing list saying
> "you shouldn''t be trying to do what you''re trying to
do," but that''s
> sort of where I''m headed. I hope it''s helpful anyway.
>
> When adopting a codebase that wasn''t developed test-driven,
I''d
> strongly recommend focusing on Rails-style functional tests rather
> than introducing mock-based testing of the current design. The biggest
> downside to mocking is that it ties you to certain interactions
> between objects. When you''re using it to determine what those
> interactions should be, that''s ok. When you already know
you''re not
> happy with the way those interactions work and intend to clean them
> up, what you really need is tests that give you a safety net to enable
> refactoring (a green bar that you can keep green as you improve the
> design). Mock-based tests aren''t likely to give you that. Rather
> they''re probably going to mean that as you skinny down your
> controllers, you''ll have to change everything in at least two
places
> instead of just one, and meanwhile the suite won''t tell you
whether
> things are still working.
>
> Mock-based tests are a useful design tool, and can (in some cases) be
> useful documentation of design, but they don''t usually make for a
good
> regression suite. All three of those are important when developing
> from scratch. But when taking on legacy code whose design you intend
> to change, the first two goals are off the table. The third is better
> served with higher-level tests.
>
> It''s definitely worth your time to read the 12-page version of
Michael
> Feathers'' Working Effectively With Legacy Code.
>
>
www.objectmentor.com/resources/articles/WorkingEffectivelyWithLegacyCode.pdf
>
Hi John,

Thanks for that excellent constructive response. Sorry I haven''t been
active
on the list for a while - I really appreciate you taking the time to help
Bill.

Thanks also for the pointer to that shortened version of Michael
Feathers''
book.
-- 
James.
http://blog.floehopper.org

I''m not sold on stub_everything returning self. Generally you only  
want that kind of behavior if you violate the law of demeter. There  
are cases where this is desirable, but they are few and far between in  
my experience. Changing mocha to help you violate the law of demeter  
seems like a bad idea. Although, I guess you could argue that people  
should be given all the rope they want.

Cheers, Jay


On Oct 26, 2008, at 2:16 PM, James Mead wrote:
> 2008/10/20 John Wilger <johnwilger at gmail.com>
>
>> Using `stub_everything` as opposed to just `stub` can alleviate  
>> that to
>> some degree, however the one thing I don''t like about Mocha is
>> `stub_everything` returns nil for all calls. If it instead returned  
>> `self`
>> by default, then it would work better for this kind of situation.  
>> (If anyone
>> knows that there /is/ a way to cause Mocha to behave this way,  
>> please speak
>> up!)
>>
>
> Hi John,
>
> Thanks for your message. There isn''t currently a way to do this.
I''m
> not
> convinced that always returning "self" would be sensible, but I
can
> see the
> case for having control over the default return value. I''ve added
a
> feature
> request [1] to track this and I''ll have a think about how the API
> would
> work. In the meantime if you have any ideas, please let me know.
>
> Thanks.
> -- 
> James.
> http://blog.floehopper.org
>
> [1]
>
http://rubyforge.org/tracker/index.php?func=detail&aid=22564&group_id=1917&atid=7480
> _______________________________________________
> mocha-developer mailing list
> mocha-developer at rubyforge.org
> http://rubyforge.org/mailman/listinfo/mocha-developer

Jay,

Where it comes in handy is when you have to work with poorly written  
dependencies that violate LoD and you don''t have a good way to either  
replace them or adapt them. It definitely should /not/ be the default  
behavior to return self, but the option to do so would be helpful in  
some cicumstances.

-- 
Regards,
John Wilger

On Oct 26, 2008, at 2:38 PM, Jay Fields <jay at jayfields.com> wrote:
> I''m not sold on stub_everything returning self. Generally you only
> want that kind of behavior if you violate the law of demeter. There  
> are cases where this is desirable, but they are few and far between  
> in my experience. Changing mocha to help you violate the law of  
> demeter seems like a bad idea. Although, I guess you could argue  
> that people should be given all the rope they want.
>
> Cheers, Jay
>
>
> On Oct 26, 2008, at 2:16 PM, James Mead wrote:
>
>> 2008/10/20 John Wilger <johnwilger at gmail.com>
>>
>>> Using `stub_everything` as opposed to just `stub` can alleviate  
>>> that to
>>> some degree, however the one thing I don''t like about
Mocha is
>>> `stub_everything` returns nil for all calls. If it instead  
>>> returned `self`
>>> by default, then it would work better for this kind of situation.  
>>> (If anyone
>>> knows that there /is/ a way to cause Mocha to behave this way,  
>>> please speak
>>> up!)
>>>
>>
>> Hi John,
>>
>> Thanks for your message. There isn''t currently a way to do
this.
>> I''m not
>> convinced that always returning "self" would be sensible, but
I can
>> see the
>> case for having control over the default return value. I''ve
added a
>> feature
>> request [1] to track this and I''ll have a think about how the
API
>> would
>> work. In the meantime if you have any ideas, please let me know.
>>
>> Thanks.
>> -- 
>> James.
>> http://blog.floehopper.org
>>
>> [1]
>>
http://rubyforge.org/tracker/index.php?func=detail&aid=22564&group_id=1917&atid=7480
>> _______________________________________________
>> mocha-developer mailing list
>> mocha-developer at rubyforge.org
>> http://rubyforge.org/mailman/listinfo/mocha-developer
>
> _______________________________________________
> mocha-developer mailing list
> mocha-developer at rubyforge.org
> http://rubyforge.org/mailman/listinfo/mocha-developer

I''d probably vote against this feature, but if mocha were going to
offer that sort of message eating behavior, I''d prefer that each
message eater return a new message eater that knew what method it was
the return value for, the benefit being that it could report that in
to_s and/or inspect (rather than have you scratching your head at how
such-and-such a piece of code got a hold of the stub you passed to
something-or-other).
-hume.

On Sun, Oct 26, 2008 at 9:22 PM, John Wilger <johnwilger at gmail.com>
wrote:
> Where it comes in handy is when you have to work with poorly written
> dependencies that violate LoD and you don''t have a good way to
either
> replace them or adapt them. It definitely should /not/ be the default
> behavior to return self, but the option to do so would be helpful in some
> cicumstances.
> On Oct 26, 2008, at 2:38 PM, Jay Fields <jay at jayfields.com> wrote:
>> I''m not sold on stub_everything returning self. Generally you
only want
>> that kind of behavior if you violate the law of demeter. There are
cases
>> where this is desirable, but they are few and far between in my
experience.
>> Changing mocha to help you violate the law of demeter seems like a bad
idea.
>> Although, I guess you could argue that people should be given all the
rope
>> they want.
-- 
http://elhumidor.blogspot.com/

2008/11/1 John D. Hume <duelin.markers at gmail.com>
> I''d probably vote against this feature, but if mocha were going to
> offer that sort of message eating behavior, I''d prefer that each
> message eater return a new message eater that knew what method it was
> the return value for, the benefit being that it could report that in
> to_s and/or inspect (rather than have you scratching your head at how
> such-and-such a piece of code got a hold of the stub you passed to
> something-or-other).
> -hume.
>
> On Sun, Oct 26, 2008 at 9:22 PM, John Wilger <johnwilger at
gmail.com> wrote:
> > Where it comes in handy is when you have to work with poorly written
> > dependencies that violate LoD and you don''t have a good way
to either
> > replace them or adapt them. It definitely should /not/ be the default
> > behavior to return self, but the option to do so would be helpful in
some
> > cicumstances.
> > On Oct 26, 2008, at 2:38 PM, Jay Fields <jay at jayfields.com>
wrote:
> >> I''m not sold on stub_everything returning self. Generally
you only want
> >> that kind of behavior if you violate the law of demeter. There are
cases
> >> where this is desirable, but they are few and far between in my
> experience.
> >> Changing mocha to help you violate the law of demeter seems like a
bad
> idea.
> >> Although, I guess you could argue that people should be given all
the
> rope
> >> they want.
>
> --
> http://elhumidor.blogspot.com/
> _______________________________________________
> mocha-developer mailing list
> mocha-developer at rubyforge.org
> http://rubyforge.org/mailman/listinfo/mocha-developer
>
Hi John,

Another belated reply, I''m afraid - I''m doing a bit of
catching up after a
hectic December.

I think your suggestion makes a lot of sense i.e. returning another
"stub_everything" that has knowledge of its provenance.

The ticket [1] has now moved to Lighthouse, but I''ll add a comment to
reflect your suggestion.

Cheers, James.
http://blog.floehopper.org

[1] http://floehopper.lighthouseapp.com/projects/22289/tickets/13