thr3ads.net - mocha developer - [mocha-developer] Organizing tests and mocha expectations [Oct 2006]

If this information is useful, please help other people find it:
Share via:

Bryan Helmkamp

2006-Oct-08 23:12 UTC

[mocha-developer] Organizing tests and mocha expectations

I''m simultaneously getting into using Mocha and RSpec-style tests
(courtesy of the simply_bdd plugin) and I''m struggling with some
issues while trying to organize my specs/test.  Here''s a code example
illustrating the problem:

context "update cliient invalid data" do
  include ClientsControllerSpecHelper

  specify "should render edit form" do
    setup_controller

    @client = mock
    Client.expects(:find).with("1").returns(@client)

    @client.expects(:attributes=).with("attributes")
    @client.expects(:save!).raises(ActiveRecord::RecordInvalid.new(stub(:errors
=> stub(:full_messages => []))))

    @controller.expects(:render).with()
    @controller.expects(:render).with(:action => "edit")\

    put :update, :id => 1, :client => "attributes"
  end
end

Now, to fit well with RSpec''s methodology, I''d like to have a
setup
method which gets the context ready, and then follow that up with some
specifications.  Essentially here''s the logic:

Update client with invalid data
 - Should find client by id
 - Should attempt to save client with new attributes
 - Should render edit form

The problem is that I can''t break the expectations up into separate
test methods like that because all the tests depend on those in order
to work.  It''s as if each of those expectations is both setup code and
a test, and each test relies on all the setup code for the whole test
case.

Is  there anyway to reorganize this code to still be DRY but yet also
achieve more granular tests, despite their interdependencies?

Thanks,

-Bryan


-- 
http://www.MyCongress.org/ -- coming soon

James Mead

2006-Oct-09 08:45 UTC

head link

[mocha-developer] Organizing tests and mocha expectations

On 09/10/06, Bryan Helmkamp <bhelmkamp at gmail.com>
wrote:>
> I''m simultaneously getting into using Mocha and RSpec-style tests
> (courtesy of the simply_bdd plugin) and I''m struggling with some
> issues while trying to organize my specs/test.  Here''s a code
example
> illustrating the problem:
>
> context "update cliient invalid data" do
>   include ClientsControllerSpecHelper
>
>   specify "should render edit form" do
>     setup_controller
>
>     @client = mock
>     Client.expects(:find).with("1").returns(@client)
>
>     @client.expects(:attributes=).with("attributes")
>     @client.expects(:save!).raises(ActiveRecord::RecordInvalid.new
> (stub(:errors
> => stub(:full_messages => []))))
>
>     @controller.expects(:render).with()
>     @controller.expects(:render).with(:action => "edit")\
>
>     put :update, :id => 1, :client => "attributes"
>   end
> end
>
> Now, to fit well with RSpec''s methodology, I''d like to
have a setup
> method which gets the context ready, and then follow that up with some
> specifications.  Essentially here''s the logic:
>
> Update client with invalid data
> - Should find client by id
> - Should attempt to save client with new attributes
> - Should render edit form
>
> The problem is that I can''t break the expectations up into
separate
> test methods like that because all the tests depend on those in order
> to work.  It''s as if each of those expectations is both setup code
and
> a test, and each test relies on all the setup code for the whole test
> case.
>
> Is  there anyway to reorganize this code to still be DRY but yet also
> achieve more granular tests, despite their interdependencies?
>
> Thanks,
>
> -Bryan

If I understand correctly, I think your problem boils down to the fact that
the mock is providing two pieces of functionality for each expectation - (1)
asserts that a particular method has been called with particular parameters;
(2) returns a specified result (stubbing). I''m not overly familiar with
the
BDD style, but the only way I can see you could do this at the moment is...

context "update cliient invalid data" do
  include ClientsControllerSpecHelper

  specify "should find client by id" do
    setup_controller

    @client = mock
    Client.expects(:find).with("1").returns(@client)

    @client.stubs(:attributes=)
    @client.stubs(:save!).raises(ActiveRecord::RecordInvalid.new(stub(:errors
=> stub(:full_messages => []))))

    @controller.stubs(:render)

    put :update, :id => 1, :client => "attributes"
  end

  specify "should attempt to save client with new attributes" do
    setup_controller

    @client = mock
    Client.stubs(:find).returns(@client)

    @client.expects(:attributes=).with("attributes")
    @client.expects(:save!).raises(ActiveRecord::RecordInvalid.new(stub(:errors
=> stub(:full_messages => []))))

    @controller.stubs(:render)

    put :update, :id => 1, :client => "attributes"
  end

  specify "should render edit form" do
    setup_controller

    @client = mock
    Client.stubs(:find).returns(@client)

    @client.stubs(:attributes=)
    @client.stubs(:save!).raises(ActiveRecord::RecordInvalid.new(stub(:errors
=> stub(:full_messages => []))))

    @controller.expects(:render).with()
    @controller.expects(:render).with(:action => "edit")

    put :update, :id => 1, :client => "attributes"
  end

end

...admittedly not very DRY, but I don''t think DRY-ness is as important
as
readability in tests.

However this reminds me of a conversation I had a while ago with my
colleague Chris Roos. We were discussing the possibility of replacing one
expectation with another. If we could do this then it might be possible to
do something like this...

context "update cliient invalid data" do
  include ClientsControllerSpecHelper

  setup do
    setup_controller

    @client = mock
    Client.stubs(:find).returns(@client)

    @client.stubs(:attributes=)
    @client.stubs(:save!).raises(ActiveRecord::RecordInvalid.new(stub(:errors
=> stub(:full_messages => []))))

    @controller.stubs(:render)
  end

  specify "should find client by id" do
    Client.expects(:find).with("1").returns(@client)

    put :update, :id => 1, :client => "attributes"
  end

  specify "should attempt to save client with new attributes" do
    @client.expects(:attributes=).with("attributes")
    @client.expects(:save!).raises(ActiveRecord::RecordInvalid.new(stub(:errors
=> stub(:full_messages => []))))

    put :update, :id => 1, :client => "attributes"
  end

  specify "should render edit form" do
    @controller.expects(:render).with()
    @controller.expects(:render).with(:action => "edit")

    put :update, :id => 1, :client => "attributes"
  end

end


...I''m guessing this is more like what you''re after. If yes,
let me know and
we''ll look into making the change.

-- 
James.
http://blog.floehopper.org
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
http://rubyforge.org/pipermail/mocha-developer/attachments/20061009/655df30c/attachment.html

Bryan Helmkamp

2006-Oct-09 13:10 UTC

head link

[mocha-developer] Organizing tests and mocha expectations

James,

Yes, that''s exactly what I need!  I need to stub a method in the
setup, and then replace the stub with expectations in specific
tests/specs.  Either that, or stub the method and then be able to
provide expectations for the stub.  There''s a slight difference
between the two, but, at least initially, the second way sounds better
because I wouldn''t have to duplicate the stubbed return values in the
expectation.

How hard would this be to get into Mocha?  Would it be likely to break
anyone''s tests?

Thanks,

-Bryan

On 10/9/06, James Mead <jamesmead44 at gmail.com>
wrote:> On 09/10/06, Bryan Helmkamp <bhelmkamp at gmail.com> wrote:
>
> > I''m simultaneously getting into using Mocha and RSpec-style
tests
> > (courtesy of the simply_bdd plugin) and I''m struggling with
some
> > issues while trying to organize my specs/test.  Here''s a code
example
> > illustrating the problem:
> >
> > context "update cliient invalid data" do
> >   include ClientsControllerSpecHelper
> >
> >   specify "should render edit form" do
> >     setup_controller
> >
> >     @client = mock
> >     Client.expects (:find).with("1").returns(@client)
> >
> >     @client.expects(:attributes=).with("attributes")
> >
> @client.expects(:save!).raises(ActiveRecord::RecordInvalid.new(stub(:errors
> > => stub(:full_messages => []))))
> >
> >     @controller.expects(:render).with()
> >     @controller.expects(:render).with(:action => "edit")\
> >
> >     put :update, :id => 1, :client => "attributes"
> >   end
> > end
> >
> > Now, to fit well with RSpec''s methodology, I''d like
to have a setup
> > method which gets the context ready, and then follow that up with some
> > specifications.  Essentially here''s the logic:
> >
> > Update client with invalid data
> > - Should find client by id
> > - Should attempt to save client with new attributes
> > - Should render edit form
> >
> > The problem is that I can''t break the expectations up into
separate
> > test methods like that because all the tests depend on those in order
> > to work.  It''s as if each of those expectations is both setup
code and
> > a test, and each test relies on all the setup code for the whole test
> > case.
> >
> > Is  there anyway to reorganize this code to still be DRY but yet also
> > achieve more granular tests, despite their interdependencies?
> >
> > Thanks,
> >
> > -Bryan
>
> If I understand correctly, I think your problem boils down to the fact that
> the mock is providing two pieces of functionality for each expectation -
(1)
> asserts that a particular method has been called with particular
parameters;
> (2) returns a specified result (stubbing). I''m not overly familiar
with the
> BDD style, but the only way I can see you could do this at the moment is...
>
> context "update cliient invalid data" do
>   include ClientsControllerSpecHelper
>
>   specify "should find client by id" do
>     setup_controller
>
>     @client = mock
>     Client.expects(:find).with("1").returns(@client)
>
>     @client.stubs(:attributes=)
>
> @client.stubs(:save!).raises(ActiveRecord::RecordInvalid.new(stub(:errors
=>
> stub(:full_messages => []))))
>
>     @controller.stubs(:render)
>
>     put :update, :id => 1, :client => "attributes"
>   end
>
>   specify "should attempt to save client with new attributes" do
>     setup_controller
>
>     @client = mock
>     Client.stubs(:find).returns(@client)
>
>     @client.expects(:attributes=).with("attributes")
>
> @client.expects(:save!).raises(ActiveRecord::RecordInvalid.new(stub(:errors
> => stub(:full_messages => []))))
>
>     @controller.stubs(:render)
>
>     put :update, :id => 1, :client => "attributes"
>   end
>
>   specify "should render edit form" do
>     setup_controller
>
>     @client = mock
>     Client.stubs(:find).returns(@client)
>
>     @client.stubs(:attributes=)
>
> @client.stubs(:save!).raises(ActiveRecord::RecordInvalid.new(stub(:errors
=>
> stub(:full_messages => []))))
>
>     @controller.expects (:render).with()
>     @controller.expects(:render).with(:action => "edit")
>
>     put :update, :id => 1, :client => "attributes"
>   end
>
> end
>
> ...admittedly not very DRY, but I don''t think DRY-ness is as
important as
> readability in tests.
>
> However this reminds me of a conversation I had a while ago with my
> colleague Chris Roos. We were discussing the possibility of replacing one
> expectation with another. If we could do this then it might be possible to
> do something like this...
>
> context "update cliient invalid data" do
>   include ClientsControllerSpecHelper
>
>   setup do
>     setup_controller
>
>      @client = mock
>     Client.stubs(:find).returns(@client)
>
>     @client.stubs(:attributes=)
>
> @client.stubs(:save!).raises(ActiveRecord::RecordInvalid.new(stub(:errors
=>
> stub(:full_messages => []))))
>
>      @controller.stubs(:render)
>   end
>
>    specify "should find client by id" do
>     Client.expects (:find).with("1").returns(@client)
>
>     put :update, :id => 1, :client => "attributes"
>   end
>
>    specify "should attempt to save client with new attributes" do
>     @client.expects(:attributes=).with("attributes")
>
> @client.expects(:save!).raises(ActiveRecord::RecordInvalid.new(stub(:errors
> => stub(:full_messages => []))))
>
>     put :update, :id => 1, :client => "attributes"
>   end
>
>    specify "should render edit form" do
>     @controller.expects(:render).with()
>      @controller.expects(:render).with(:action => "edit")
>
>      put :update, :id => 1, :client => "attributes"
>    end
>
> end
>
>
> ...I''m guessing this is more like what you''re after. If
yes, let me know and
> we''ll look into making the change.
>
> --
> James.
> http://blog.floehopper.org
> _______________________________________________
> mocha-developer mailing list
> mocha-developer at rubyforge.org
> http://rubyforge.org/mailman/listinfo/mocha-developer
>
>

-- 
http://www.MyCongress.org/ -- coming soon

James Mead

2006-Oct-09 16:06 UTC

head link

[mocha-developer] Organizing tests and mocha expectations

On 09/10/06, Bryan Helmkamp <bhelmkamp at gmail.com>
wrote:>
> Yes, that''s exactly what I need!  I need to stub a method in the
> setup, and then replace the stub with expectations in specific
> tests/specs.  Either that, or stub the method and then be able to
> provide expectations for the stub.  There''s a slight difference
> between the two, but, at least initially, the second way sounds better
> because I wouldn''t have to duplicate the stubbed return values in
the
> expectation.
>
> How hard would this be to get into Mocha?  Would it be likely to break
> anyone''s tests?

I don''t think it would be too hard to make the necessary changes
(although
there''s a bit of a backlog of changes at the moment), but I''d
like to spend
a bit of time thinking about the best syntax for it. I''ve been chatting
to
my colleague Chris Roos and we''ve come up with a few different
alternatives...

# suggestion 1 (pretty much what I suggested to you before)
object.stubs(:expected_method).with(:parameter1,
:parameter2).returns(:result)
object.expects(:expected_method).with(:parameter1,
:parameter2).returns(:result) # replaces stubbing expectation

note: in order to preserve the ability to set multiple expectations for the
same method with different parameters, the stubs() call will need to have
exactly the same with() call, so that the specific expectation can be
identified and replaced.

# suggestion 2
object.stubs(:expected_method).returns(:result)
object.expects(:expected_method, :replace => true).with(:parameter1,
:parameter2).returns(:result) # replaces stubbing expectation

note: setting :replace => true causes expectation to replace all existing
stubbing or asserting expectations for the given method name (whatever their
expected parameters).

# suggestion 3
call_to_expected_method = object.stubs(:expected_method).returns(:result)
expect(call_to_expected_method).with(:parameter1,
:parameter2).returns(:result) # replaces stubbing expectation

So far, I''m leaning towards suggestion 3 or something similar. stubs()
and
expects() and their fellow expectation builder methods already return the
expectation object, so it wouldn''t be hard to implement. I''m
planning on
taking a look at JMock to see whether they handle this kind of idea.

Thoughts anyone?

-- 
James.
http://blog.floehopper.org
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
http://rubyforge.org/pipermail/mocha-developer/attachments/20061009/43c4eeed/attachment.html

David Chelimsky

2006-Oct-09 21:06 UTC

head link

[mocha-developer] Organizing tests and mocha expectations

On 10/9/06, James Mead <jamesmead44 at gmail.com>
wrote:> On 09/10/06, Bryan Helmkamp <bhelmkamp at gmail.com> wrote:
> > Yes, that''s exactly what I need!  I need to stub a method in
the
> > setup, and then replace the stub with expectations in specific
> > tests/specs.  Either that, or stub the method and then be able to
> > provide expectations for the stub.  There''s a slight
difference
> > between the two, but, at least initially, the second way sounds better
> > because I wouldn''t have to duplicate the stubbed return
values in the
> > expectation.
> >
> > How hard would this be to get into Mocha?  Would it be likely to break
> > anyone''s tests?
>
>  I don''t think it would be too hard to make the necessary changes
(although
> there''s a bit of a backlog of changes at the moment), but
I''d like to spend
> a bit of time thinking about the best syntax for it. I''ve been
chatting to
> my colleague Chris Roos and we''ve come up with a few different
> alternatives...
>
> # suggestion 1 (pretty much what I suggested to you before)
>  object.stubs(:expected_method).with(:parameter1,
> :parameter2).returns(:result)
> object.expects(:expected_method).with(:parameter1,
> :parameter2).returns(:result) # replaces stubbing expectation
-1
This makes sense while the two are right next to each other, but if
the first was in the setup and the second in a spec it might be
confusing - especially when you''ve got other specs that have different
params.
> note: in order to preserve the ability to set multiple expectations for the
> same method with different parameters, the stubs() call will need to have
> exactly the same with() call, so that the specific expectation can be
> identified and replaced.
>
> # suggestion 2
>  object.stubs(:expected_method).returns(:result)
>  object.expects(:expected_method, :replace => true).with(:parameter1,
> :parameter2).returns(:result) # replaces stubbing expectation
+1
This is nice because the stub syntax doesn''t change and the fact that
the expectation is replacing a stub becomes explicit. Much less
confusion.
>
> note: setting :replace => true causes expectation to replace all
existing
> stubbing or asserting expectations for the given method name (whatever
their
> expected parameters).
>
>  # suggestion 3
> call_to_expected_method >
object.stubs(:expected_method).returns(:result)
>  expect(call_to_expected_method).with(:parameter1,
> :parameter2).returns(:result) # replaces stubbing expectation
-1
I think this one leads to more confusion in the end.

Just my 3 cents (I guess -1 cent if you add them up).

Cheers,
David

>
> So far, I''m leaning towards suggestion 3 or something similar.
stubs() and
> expects() and their fellow expectation builder methods already return the
> expectation object, so it wouldn''t be hard to implement.
I''m planning on
> taking a look at JMock to see whether they handle this kind of idea.
>
> Thoughts anyone?
>
> --
>
> James.
> http://blog.floehopper.org
> _______________________________________________
> mocha-developer mailing list
> mocha-developer at rubyforge.org
> http://rubyforge.org/mailman/listinfo/mocha-developer
>
>

Bryan Helmkamp

2006-Oct-09 21:16 UTC

head link

[mocha-developer] Organizing tests and mocha expectations

I''m very new to Mocha and mocking in general, so I''m not ready
to
offer up suggestions for syntax just yet.  I''ll ponder these a bit
more when I have a chance.

One consideration I''d like to throw out there is the duplication of
the returned value (or, equivalently, the raised exception).  For my
original example, I don''t see a need to re-specify the results for the
save!() and find() calls when I add my expectations.

What do you guys think?  I''ll try to chime in with some more input
about the syntax soon.

-Bryan

On 10/9/06, James Mead <jamesmead44 at gmail.com>
wrote:> On 09/10/06, Bryan Helmkamp <bhelmkamp at gmail.com> wrote:
> > Yes, that''s exactly what I need!  I need to stub a method in
the
> > setup, and then replace the stub with expectations in specific
> > tests/specs.  Either that, or stub the method and then be able to
> > provide expectations for the stub.  There''s a slight
difference
> > between the two, but, at least initially, the second way sounds better
> > because I wouldn''t have to duplicate the stubbed return
values in the
> > expectation.
> >
> > How hard would this be to get into Mocha?  Would it be likely to break
> > anyone''s tests?
>
>  I don''t think it would be too hard to make the necessary changes
(although
> there''s a bit of a backlog of changes at the moment), but
I''d like to spend
> a bit of time thinking about the best syntax for it. I''ve been
chatting to
> my colleague Chris Roos and we''ve come up with a few different
> alternatives...
>
> # suggestion 1 (pretty much what I suggested to you before)
>  object.stubs(:expected_method).with(:parameter1,
> :parameter2).returns(:result)
> object.expects(:expected_method).with(:parameter1,
> :parameter2).returns(:result) # replaces stubbing expectation
>
> note: in order to preserve the ability to set multiple expectations for the
> same method with different parameters, the stubs() call will need to have
> exactly the same with() call, so that the specific expectation can be
> identified and replaced.
>
> # suggestion 2
>  object.stubs(:expected_method).returns(:result)
>  object.expects(:expected_method, :replace => true).with(:parameter1,
> :parameter2).returns(:result) # replaces stubbing expectation
>
> note: setting :replace => true causes expectation to replace all
existing
> stubbing or asserting expectations for the given method name (whatever
their
> expected parameters).
>
>  # suggestion 3
> call_to_expected_method >
object.stubs(:expected_method).returns(:result)
>  expect(call_to_expected_method).with(:parameter1,
> :parameter2).returns(:result) # replaces stubbing expectation
>
> So far, I''m leaning towards suggestion 3 or something similar.
stubs() and
> expects() and their fellow expectation builder methods already return the
> expectation object, so it wouldn''t be hard to implement.
I''m planning on
> taking a look at JMock to see whether they handle this kind of idea.
>
> Thoughts anyone?
>
> --
>
> James.
> http://blog.floehopper.org
> _______________________________________________
> mocha-developer mailing list
> mocha-developer at rubyforge.org
> http://rubyforge.org/mailman/listinfo/mocha-developer
>
>

-- 
http://www.MyCongress.org/ -- coming soon

James Mead

2006-Oct-10 08:30 UTC

head link

[mocha-developer] Organizing tests and mocha expectations

On 09/10/06, Bryan Helmkamp <bhelmkamp at gmail.com>
wrote:>
> One consideration I''d like to throw out there is the duplication
of
> the returned value (or, equivalently, the raised exception).  For my
> original example, I don''t see a need to re-specify the results for
the
> save!() and find() calls when I add my expectations.
>
Good point. Actually going with suggestion 3 would mean you didn''t need
to
repeat anything - basically the key difference for this case is that you
have a means of modifying an existing expectation - in particular the
ability to make it assert (and perhaps to stop it asserting).

-- 
James.
http://blog.floehopper.org
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
http://rubyforge.org/pipermail/mocha-developer/attachments/20061010/3bbe1a47/attachment.html

David Chelimsky

2006-Oct-10 11:59 UTC

head link

[mocha-developer] Organizing tests and mocha expectations

On 10/10/06, James Mead <jamesmead44 at gmail.com>
wrote:> On 09/10/06, Bryan Helmkamp <bhelmkamp at gmail.com> wrote:
> > One consideration I''d like to throw out there is the
duplication of
> > the returned value (or, equivalently, the raised exception).  For my
> > original example, I don''t see a need to re-specify the
results for the
> > save!() and find() calls when I add my expectations.
> >
>
> Good point. Actually going with suggestion 3 would mean you didn''t
need to
> repeat anything - basically the key difference for this case is that you
> have a means of modifying an existing expectation - in particular the
> ability to make it assert (and perhaps to stop it asserting).
For what it''s worth - feel free to disagree - but I really feel
strongly that suggestion #3 is the wrong direction:

# suggestion 3
call_to_expected_method = object.stubs(:expected_method).returns(:result)
expect(call_to_expected_method).with(:parameter1,
:parameter2).returns(:result) # replaces stubbing expectation

Slinging the expectation around in the specs is exposing mocha''s
implementation to the spec-writer. This is going to lead to specs that
are much more difficult to understand when looking back at them. Also,
it''s going to bind mocha to this particular implementation. Should you
guys to decide to return some different object, a wrapper of some
kind, or whatever, you''d be screwed.

Bryan''s concern about duplicating the return value makes sense only
when you want the same return value. What if you want a different one
coming from the stub than from the subsequent expectation? The
duplication of the return value in this case actually helps the reader
of the spec to understand that "in this instance, we''re expecting
something slightly different, but we''re going to return the same
thing".

The second suggestion you had makes the binding between the stub and
the expectation clear to the reader and leaves you the flexibility to
return the same values or different values:

# suggestion 2
object.stubs(:expected_method).returns(:result)
object.expects(:expected_method, :replace => true).with(:parameter1,
:parameter2).returns(:result)

Bryan - in your original example you stated that following rspec''s
methodology makes you want to have the spec broken into three separate
specs. I don''t think that''s really necessary in this case. In
fact,
binding specs together in any way is the antithesis of rspec''s
methodology. Every spec should run completely independently from the
others and should never depend on state left behind by another spec.

The spec name is "update client invalid data should render edit form",
so (if I understand correctly - maybe I don''t) the fact that the
client data is invalid is not really the issue. The goal is that when
AR raises RecordInvalid you should render the edit form. The spec you
suggested has some information in it that is not relevant to the
meaning of the spec - specifically the stuff about attributes. To that
end, a single spec w/ less information would be just fine:

context "update client invalid data" do
  include ClientsControllerSpecHelper

  specify "should render edit form" do
    setup_controller

    @client = mock
    Client.expects(:find).with("1").returns(@client)

    @client.expects(:save!).raises(
      ActiveRecord::RecordInvalid.new(
        stub(:errors => stub(:full_messages => []))
      )
    )

    @controller.expects(:render).with(:action => "edit")

    put :update, :id => 1
  end
end

Does that all make sense? Apologies for the soap-boxy nature of all of
this, but a primary goal of rspec is to bring us back to specs (tests,
whatever) that are readable, understandable and easy to maintain. DRY
is a very valuable principle, but it is not the only principle. In
specs, DRY and clarity are often at odds and need to be balanced. When
only one or the other can be served, for my money, clarity wins.

Thanks for listening.

Cheers,
David

>
> --
>
> James.
> http://blog.floehopper.org
> _______________________________________________
> mocha-developer mailing list
> mocha-developer at rubyforge.org
> http://rubyforge.org/mailman/listinfo/mocha-developer
>
>

Bryan Helmkamp

2006-Oct-10 13:54 UTC

head link

[mocha-developer] Organizing tests and mocha expectations

On 10/10/06, David Chelimsky <dchelimsky at gmail.com>
wrote:> The second suggestion you had makes the binding between the stub and
> the expectation clear to the reader and leaves you the flexibility to
> return the same values or different values:
>
> # suggestion 2
> object.stubs(:expected_method).returns(:result)
> object.expects(:expected_method, :replace => true).with(:parameter1,
> :parameter2).returns(:result)
I tend to agree with your reasoning, David.  Suggestion #3 does not
feel like the best option to me.  You''re right that, in this case,
repeating the result of the method call is probably more clear than a
completely DRY solution.
> Bryan - in your original example you stated that following rspec''s
> methodology makes you want to have the spec broken into three separate
> specs. I don''t think that''s really necessary in this
case. In fact,
> binding specs together in any way is the antithesis of rspec''s
> methodology. Every spec should run completely independently from the
> others and should never depend on state left behind by another spec.
I see where you are coming from here.  Obviously, removing one spec
should never cause another spec to fail.  On the other hand, I don''t
think that what I was considering would necessarily introduce a
dependency between specs.  The only dependency would be between the
specs and the context, which I think is okay.
> The spec name is "update client invalid data should render edit
form",
> so (if I understand correctly - maybe I don''t) the fact that the
> client data is invalid is not really the issue. The goal is that when
> AR raises RecordInvalid you should render the edit form. The spec you
> suggested has some information in it that is not relevant to the
> meaning of the spec - specifically the stuff about attributes. To that
> end, a single spec w/ less information would be just fine:
Yes, you''re spot on.  This is probably heading towards off-topic, but
here''s the best I could come up with (that still passes):

context "update client" do
  include ClientsControllerSpecHelper

  specify "should use client param" do
    setup_controller

    @client = mock
    Client.expects(:find).with("1").returns(@client)
    @client.expects(:attributes=).with("attributes")

    @client.stubs(:save!)
    @controller.stubs(:render)

    put :update, :id => 1, :client => "attributes"
  end
end

context "update client valid data" do
  include ClientsControllerSpecHelper

  def setup
    setup_controller

    @client = stub_everything
    Client.expects(:find).with("1").returns(@client)
    @client.expects(:save!).returns(true)

    put :update, :id => 1
  end

  specify "should redirect" do
    assert_response :redirect
  end
end

context "update client invalid data" do
  include ClientsControllerSpecHelper

  specify "should render edit form" do
    setup_controller

    @client = stub_everything
    Client.expects(:find).with("1").returns(@client)

    @client.expects(:save!).raises(
      ActiveRecord::RecordInvalid.new(
        stub(:errors => stub(:full_messages => []))
      )
    )

    @controller.expects(:render).with()
    @controller.expects(:render).with(:action => "edit")

    put :update, :id => 1
  end
end

I was able to remove the references to the attributes= method in the
valid and invalid data specs by using stubs_everything.  I''m not very
fond of having to repeat the setup for the Client#find call
everywhere.

Also, it''s a bit disheartening to have to resort to 43 lines of specs
to test three relatively simple aspects of a 7 line controller method.
 Are there any ways to reduce the volume of this code that I might be
missing?  or is this 6:1 code to test ratio a necessary consequence of
the system?

Thanks for the help, David.

-Bryan

David Chelimsky

2006-Oct-10 21:15 UTC

head link

[mocha-developer] Organizing tests and mocha expectations

On 10/10/06, Bryan Helmkamp <bhelmkamp at gmail.com>
wrote:> On 10/10/06, David Chelimsky <dchelimsky at gmail.com> wrote:
> > The second suggestion you had makes the binding between the stub and
> > the expectation clear to the reader and leaves you the flexibility to
> > return the same values or different values:
> >
> > # suggestion 2
> > object.stubs(:expected_method).returns(:result)
> > object.expects(:expected_method, :replace =>
true).with(:parameter1,
> > :parameter2).returns(:result)
>
> I tend to agree with your reasoning, David.  Suggestion #3 does not
> feel like the best option to me.  You''re right that, in this case,
> repeating the result of the method call is probably more clear than a
> completely DRY solution.
>
> > Bryan - in your original example you stated that following
rspec''s
> > methodology makes you want to have the spec broken into three separate
> > specs. I don''t think that''s really necessary in this
case. In fact,
> > binding specs together in any way is the antithesis of
rspec''s
> > methodology. Every spec should run completely independently from the
> > others and should never depend on state left behind by another spec.
>
> I see where you are coming from here.  Obviously, removing one spec
> should never cause another spec to fail.  On the other hand, I
don''t
> think that what I was considering would necessarily introduce a
> dependency between specs.  The only dependency would be between the
> specs and the context, which I think is okay.
>
> > The spec name is "update client invalid data should render edit
form",
> > so (if I understand correctly - maybe I don''t) the fact that
the
> > client data is invalid is not really the issue. The goal is that when
> > AR raises RecordInvalid you should render the edit form. The spec you
> > suggested has some information in it that is not relevant to the
> > meaning of the spec - specifically the stuff about attributes. To that
> > end, a single spec w/ less information would be just fine:
>
> Yes, you''re spot on.  This is probably heading towards off-topic,
but
> here''s the best I could come up with (that still passes):
>
> context "update client" do
>   include ClientsControllerSpecHelper
>
>   specify "should use client param" do
>     setup_controller
>
>     @client = mock
>     Client.expects(:find).with("1").returns(@client)
>     @client.expects(:attributes=).with("attributes")
>
>     @client.stubs(:save!)
>     @controller.stubs(:render)
>
>     put :update, :id => 1, :client => "attributes"
>   end
> end
>
> context "update client valid data" do
>   include ClientsControllerSpecHelper
>
>   def setup
>     setup_controller
>
>     @client = stub_everything
>     Client.expects(:find).with("1").returns(@client)
>     @client.expects(:save!).returns(true)
>
>     put :update, :id => 1
>   end
>
>   specify "should redirect" do
>     assert_response :redirect
>   end
> end
>
> context "update client invalid data" do
>   include ClientsControllerSpecHelper
>
>   specify "should render edit form" do
>     setup_controller
>
>     @client = stub_everything
>     Client.expects(:find).with("1").returns(@client)
>
>     @client.expects(:save!).raises(
>       ActiveRecord::RecordInvalid.new(
>         stub(:errors => stub(:full_messages => []))
>       )
>     )
>
>     @controller.expects(:render).with()
>     @controller.expects(:render).with(:action => "edit")
>
>     put :update, :id => 1
>   end
> end
>
> I was able to remove the references to the attributes= method in the
> valid and invalid data specs by using stubs_everything.  I''m not
very
> fond of having to repeat the setup for the Client#find call
> everywhere.
>
> Also, it''s a bit disheartening to have to resort to 43 lines of
specs
> to test three relatively simple aspects of a 7 line controller method.
>  Are there any ways to reduce the volume of this code that I might be
> missing?  or is this 6:1 code to test ratio a necessary consequence of
> the system?
I think we should move the rspec conversation over to the rspec list
to give it visibility there and to get out of the way of the mocha
list members who aren''t interested. Fair? If you agree, go ahead and
start a conversation there.

Thanks,
David

>
> Thanks for the help, David.
>
> -Bryan
> _______________________________________________
> mocha-developer mailing list
> mocha-developer at rubyforge.org
> http://rubyforge.org/mailman/listinfo/mocha-developer
>

Possibly Parallel Threads

Search for more seemingly similar threads

mocha developer - Oct 2006 - Organizing tests and mocha expectations

[mocha-developer] Organizing tests and mocha expectations

[mocha-developer] Organizing tests and mocha expectations

[mocha-developer] Organizing tests and mocha expectations

[mocha-developer] Organizing tests and mocha expectations

[mocha-developer] Organizing tests and mocha expectations

[mocha-developer] Organizing tests and mocha expectations

[mocha-developer] Organizing tests and mocha expectations

[mocha-developer] Organizing tests and mocha expectations

[mocha-developer] Organizing tests and mocha expectations

[mocha-developer] Organizing tests and mocha expectations

Possibly Parallel Threads