[rspec-users] stub vs stub!
Nick Hoffman
"also alias_method :stub, :stub!, so you can stub with less bang"
which I've been taking advantage of a lot.
However, I just ran into a situation where using #stub caused an error
to occur, and changing to #stub! caused the error the disappear.
Any idea what's going on?
Thanks,
Nick
--
Posted via http://www.ruby-forum.com/.
_______________________________________________
rspec-users mailing list
rspec...@rubyforge.org
http://rubyforge.org/mailman/listinfo/rspec-users
Pat Maddox
Pat
Nick Hoffman
> I've never heard of CurbFu, but according to
> http://github.com/gdi/curb-fu/blob/master/lib/curb-fu.rb#L43 it defines
> a stub method already. So you're hitting that one, which expects two
> arguments. stub! goes to RSpec's mocking framework.
>
> Pat
Good catch! Thanks, Pat.
David Chelimsky
> Pat Maddox wrote:
>> I've never heard of CurbFu, but according to
>> http://github.com/gdi/curb-fu/blob/master/lib/curb-fu.rb#L43 it defines
>> a stub method already. So you're hitting that one, which expects two
>> arguments. stub! goes to RSpec's mocking framework.
>>
>> Pat
>
> Good catch! Thanks, Pat.
This presents an interesting dilemma, in that I (now not so) secretly plan to deprecate and remove stub().
I also plan to offer up a mode in which you don't get stub and should_receive added to every object, but instead have to wrap each object you want to set stubs/message expectations on. i.e.
double(Foo).stub(:bar) { return value }
This is how flexmock works, and it helps to avoid problems exactly like the one you've uncovered.
I guess these two changes will need to ship together :)
Cheers,
David
Patrick J. Collins
I have this in my controller action:
from_address = Setting.find_by_name("shipping_from_address").address
and my spec fails:
NoMethodError in 'Admin::ShippingLabelsController should render a pdf file'
undefined method `address' for nil:NilClass
yet in the console I can do:
>> Setting.find_by_name("shipping_from_address").address
=> #<Address id: 1970, label: .... etc>
and I get the address-- so I don't understand why it's failing in the spec.. ?
...
Also, if I want to test that I am getting a pdf back, how is the best way to do that?
I was doing:
controller:
respond_to do |format|
format.pdf do
render :template => 'admin/contacts/shipping_label'
end
spec:
def do_get
get :create, :format => :pdf
end
it "should render a pdf file" do
do_get
response.should render_template("admin/contacts/shipping_label")
end
-- but that gives me:
'Admin::ShippingLabelsController should render a pdf file' FAILED
expected "admin/contacts/shipping_label", got nil
/Users/patrick/coding/rails/matthew/spec/controllers/admin/shipping_labels_controller_spec.rb:11:
thanks...
Patrick J. Collins
http://collinatorstudios.com
Nick Hoffman
> Hi,
>
> I have this in my controller action:
>
> from_address = Setting.find_by_name("shipping_from_address").address
>
> and my spec fails:
>
> NoMethodError in 'Admin::ShippingLabelsController should render a pdf
> file'
> undefined method `address' for nil:NilClass
>
Patrick, please create a new thread for this, since it doesn't have
anything to do with the current topic.
--
Posted via http://www.ruby-forum.com/.
patrick99e99
> anything to do with the current topic.
Wow that is weird.. I replied to the previous thread in my email
client but removed the subject-- as I assumed that was what kept
things associated with threads; there was no other text in the email
pertaining to this thread, and the address was the generic rspec-
us...@rubyforge.org... So I don't quite understand why my post was
thrown under this topic..
Oh well, sorry about that and I will repost.
-patrick
patrick99e99
I have this in my controller action:
from_address = Setting.find_by_name("shipping_from_address").address
and my spec fails:
NoMethodError in 'Admin::ShippingLabelsController should render a pdf
file'
undefined method `address' for nil:NilClass
yet in the console I can do:
>> Setting.find_by_name("shipping_from_address").address
...
I was doing:
controller:
spec:
thanks...
-patrick
Nick Hoffman
>> Patrick, please create a new thread for this, since it doesn't have
>> anything to do with the current topic.
>
> Wow that is weird.. I replied to the previous thread in my email
> client but removed the subject-- as I assumed that was what kept
> things associated with threads; there was no other text in the email
> pertaining to this thread, and the address was the generic rspec-
> us...@rubyforge.org... So I don't quite understand why my post was
> thrown under this topic..
>
> Oh well, sorry about that and I will repost.
>
> -patrick
No worries, mate.
There's an email header that identifies which thread a given post
belongs to. If you reply to a post, your reply will inherit that header,
and thus be associated with the thread.
Cheers,
Nick
--
Posted via http://www.ruby-forum.com/.
Nick Hoffman
> I have this in my controller action:
>
> from_address = Setting.find_by_name("shipping_from_address").address
>
> and my spec fails:
>
> NoMethodError in 'Admin::ShippingLabelsController should render a pdf
> file'
> undefined method `address' for nil:NilClass
>
> yet in the console I can do:
>
>>> Setting.find_by_name("shipping_from_address").address
>
> => #<Address id: 1970, label: .... etc>
>
> and I get the address-- so I don't understand why it's failing in the
> spec.. ?
Your console was most likely running in the development environment, and
thus using your development database. Specs run in the test environment,
and use your test database. Thus, your specs won't see the data in your
dev DB.
Also, your test DB is wiped clean at the beginning of all of your tests.
That way, existing data can't taint or skew the results of your specs.
You probably want something along the lines of this:
http://pastie.org/878189
However, that line of code of yours should be modified slightly. At the
moment, it'll raise the error that you posted above if the setting isn't
found. This is because Setting.find_by_name returns nil if the setting
isn't found, and nil#address doesn't exist.
You can put Object#try right before the call to #address, like this:
Setting.find_by_name('...').try(:address)
I'd probably break the line into a few: search for the setting, then
check if it's nil, then grab the address or deal with the nil value.
There're many solutions. Try a few and see what feels right to you.
--
Posted via http://www.ruby-forum.com/.
Patrick J. Collins
Thank you very much for your reply. One thing I am finding incredibly
frustrating with Rspec, is that I don't even know what questions to ask--
because I find the whole thing so confusing. So forgive me, but I am going to
break down your code and ask specific questions to hopefully gain
understanding.
1. Ok-- so in your example (which I greatly appreciate), you have this do_action
method.. Is that where I would put something like
get :create, :format => :pdf
?
...
2. Next in the before each block, you set @address..
Now, I have factory girl installed and have used it in cucumber, is that
something that could be used instead of setting @address to "123 some st" ? Is
that common practice and acceptable to do?
3. You set shipping_setting to mock the model Setting with
:address set to @address....
>From my understanding of what I've read-- Mocking a model,
simply means that it's a fake object, that acts like the real
thing------ So, when you do that mock_model, is it virtually
the same to have just done:
@shipping_setting = Setting.create(:address => @address)
? That would be utilizing the test database, correct? By
using mock_model, nothing is being stored, right? It's more
just temporary data in memory... ?
4. You do .stub! ........... This is where I get really
confused. From the peepcode screencast that I have watched
several times, he explained that stubbing is the same as
mocking-- except there is no expectation of what it returns.
That made sense when he said it, but in all the examples I've
seen with code (including yours), there is this .and_return at
the end.. Which as far as I can tell, is an expectation.. I
mean, you are asking it to return a specific thing-- that is
an expectation is it not? Or does expectation really mean
something else in this context?
That aside, if I try to guess what this Setting.stub! this is
doing-- it is pretending that you are actually looking for a
record, and getting it..... but in this case, is that even
worth doing? I mean, the shipping_setting record is really
only important in the sense that the record is needed to
provide a valid "from" address for a label.. But, if we're
dealing with a test database where a from address doesn't
exist-- then it seems kind of pointless.. To clarify, the
importance for me would be to have a test that actually looks
in my real database and makes sure there is a shipping from
address in there... I have no idea if this paragraph will
make sense to you, but hopefully it will.
5. the it "finds the shipping address setting" block--
totally confused by this, since it's doing the same thing as
in the before :each block-- except it's mocking instead of
stubbing..... agh.. totally confused.. don't get it.. don't
even know what question to ask in hopes to get it.
6. same with the "gets the address from the setting" block...
........ I am sorry for being so confused!!!! If you can
break this down, I'd really appreciate it.
Thank you.
Patrick J. Collins
http://collinatorstudios.com
David Chelimsky
> On Mar 19, 2010, at 1:27 PM, Nick Hoffman wrote:
>
>> Pat Maddox wrote:
>>> I've never heard of CurbFu, but according to
>>> http://github.com/gdi/curb-fu/blob/master/lib/curb-fu.rb#L43 it defines
>>> a stub method already. So you're hitting that one, which expects two
>>> arguments. stub! goes to RSpec's mocking framework.
>>>
>>> Pat
>>
>> Good catch! Thanks, Pat.
>
>
> This presents an interesting dilemma, in that I (now not so) secretly plan to deprecate and remove stub().
D'oh - stub!() - that's the one I plan to remove. So we would only
have stub(). There is no need to have both, since they do the same
thing, and there is nothing particularly bangy about the stub method.
David Chelimsky
<pat...@collinatorstudios.com> wrote:
> 4. You do .stub! ........... This is where I get really
> confused. From the peepcode screencast that I have watched
> several times, he explained that stubbing is the same as
> mocking-- except there is no expectation of what it returns.
>
> That made sense when he said it, but in all the examples I've
> seen with code (including yours), there is this .and_return at
> the end.. Which as far as I can tell, is an expectation.. I
> mean, you are asking it to return a specific thing-- that is
> an expectation is it not? Or does expectation really mean
> something else in this context?
.and_xxx is admittedly confusing, as evidenced by your confusion.
These are _not_ expectations, they are merely configuration.
When you say
foo.stub(:bar).and_return('this value')
you're saying: hey, foo, you might receive the bar() message. If you
do, return "this value".
When you say
foo.should_receive(:bar).and_return('this value')
you're saying: hey, foo, you _should_ receive the bar() message. If
you do, return "this value". If you don't, raise an ExpectationNotMet
error.
Neither are saying: hey, foo, you should return "this value", and if
you don't, I'll consider it a failure.
There are also and_raise, and_yield, and and_throw methods that
operate similarly. They are instructions to the object that allow you
to control how it behaves so you can set expectations on other objects
as to how they should respond. Make sense?
There is an alternate syntax that I'm using nearly exclusively at this
point, which is to pass a block along with the stub() and
should_receive() messages:
foo.stub(:bar) { "return value" }
foo.stub(:bar) { raise SomeError }
foo.stub(:bar) { yield "value" }
foo.stub(:bar) { throw :value }
foo.should_receive(:bar) { "return value" }
foo.should_receive(:bar) { raise SomeError }
foo.should_receive(:bar) { yield "value" }
foo.should_receive(:bar) { throw :value }
As you can see, there is much less API to worry about here. You're
just supplying an implementation.
This approach also works with scoped arguments:
input = Input.new
calculator = Calculator.new(input)
calculator.should_receive(:add).with(1,2) { 3 }
input.type("1", "+", "2", "Enter")
I've been thinking of deprecating the and_xxx methods in rspec-2. Does
anybody think that's an awful idea?
Phillip Koebbe
started using it a couple of years ago with models, and understood that
well enough (I think). Then I came to controllers and I just couldn't
wrap my mind around it. I gave up for quite some time. When I came back
to testing, it was on a project that was using test::unit, but at least
I was working with someone who had more experience than I did, and I was
able to gain some understanding that I lacked earlier. But during the
use of test::unit, I realized I missed RSpec, so when I moved on, I put
RSpec back into my workflow. I've been trudging along for a few months
now, and while I still have lots and lots to learn, I think I am
actually using it somewhat correctly and productively. So while I
definitely don't want to try to supersede Nick's or anyone else's
contributions to your questions, I thought I would chime with what I've
learned.
On 2010-03-20 2:28 AM, Patrick J. Collins wrote:
> Hi Nick,
>
> Thank you very much for your reply. One thing I am finding incredibly
> frustrating with Rspec, is that I don't even know what questions to ask--
>
Ask everything. In my life, I've learned that the only stupid question
is the one you didn't ask. Most of the people in the RSpec community are
*very* understanding, patient, and incredibly helpful. No one is going
to bite your head off!
> because I find the whole thing so confusing. So forgive me, but I am going to
> break down your code and ask specific questions to hopefully gain
> understanding.
>
> 1. Ok-- so in your example (which I greatly appreciate), you have this do_action
> method.. Is that where I would put something like
>
> get :create, :format => :pdf
>
> ?
>
> ...
>
> 2. Next in the before each block, you set @address..
>
> Now, I have factory girl installed and have used it in cucumber, is that
> something that could be used instead of setting @address to "123 some st" ? Is
> that common practice and acceptable to do?
>
> 3. You set shipping_setting to mock the model Setting with
> :address set to @address....
>
> > From my understanding of what I've read-- Mocking a model,
> simply means that it's a fake object, that acts like the real
> thing------ So, when you do that mock_model, is it virtually
> the same to have just done:
> @shipping_setting = Setting.create(:address => @address)
>
> ? That would be utilizing the test database, correct? By
> using mock_model, nothing is being stored, right? It's more
> just temporary data in memory... ?
>
>
No, you're not using the test database. That's the point of mocking: you
want to break the dependency on another layer. When you test, you want
to try to isolate what you are testing so you can specify how it behaves
given certain criteria. For example, when you test a model, you don't
want a controller involved. When you test a controller, you don't really
want a model involved. Mocking and stubbing allow you to completely
control all (or most) of the extenuating circumstances that your
controller will find itself in. Then you can properly test something
like "When the controller receives these params, it should do blah blah
blah"
> 4. You do .stub! ........... This is where I get really
> confused. From the peepcode screencast that I have watched
> several times, he explained that stubbing is the same as
> mocking-- except there is no expectation of what it returns.
>
>
Stubbing is creating the "plumbing" that your application expects to
exist to function properly. If you have a person object and you want to
do something with the name attribute, you code will be expecting
"person.name" to work. If you have a fake object @person and call
@person.name, you will get an error unless you stub it like
@person.stub!(:name).and_return('John Doe')
> That made sense when he said it, but in all the examples I've
> seen with code (including yours), there is this .and_return at
> the end.. Which as far as I can tell, is an expectation.. I
> mean, you are asking it to return a specific thing-- that is
> an expectation is it not? Or does expectation really mean
> something else in this context?
>
>
The .and_return() bit just says "for this stubbed method, return this
value when it's called." Keep in mind that these objects you are
creating are going to be interacting with your application code. So if
your application code calls Person.new, it expects to receive back a
person object. So you can do something like this in your specs:
@person = stub_model(Person)
Person.stub!(:new).and_return(@person)
Then you will be bypassing your Person model completely but still
sending something back for your code to play with. Now, I've introduced
another method in this mess, stub_model. This is very similar to stub,
except that it goes ahead and and stubs out the attribute methods that
are on your model. So if you have Person with name, age, and gender, the
single call to stub_model also does this for you:
@person.stub!(:name)
@person.stub!(:age)
@person.stub!(:gender)
By default, the stubs created with stub_model will return nil. If you
need to specify return values, do something like this:
@person = stub_model(Person, :name => 'John Doe', :age => 99, :gender =>
'M')
> That aside, if I try to guess what this Setting.stub! this is
> doing-- it is pretending that you are actually looking for a
> record, and getting it..... but in this case, is that even
> worth doing? I mean, the shipping_setting record is really
> only important in the sense that the record is needed to
> provide a valid "from" address for a label.. But, if we're
> dealing with a test database where a from address doesn't
> exist-- then it seems kind of pointless.. To clarify, the
> importance for me would be to have a test that actually looks
> in my real database and makes sure there is a shipping from
> address in there... I have no idea if this paragraph will
> make sense to you, but hopefully it will.
>
>
Here is where testing in isolation comes back into play. If you are
testing a controller, you want to try to avoid the dependence on the
database. Ideally, you want to use mocking and stubbing to create an
artificial environment, that you completely control, to specify the
behavior of your controller. Think about each situation you expect your
controller to encounter. For each one of those situations, create the
imaginary world that needs to exist, and then test that your controller
behaves correctly.
For example, if your controller needs to do one thing if a particular
record is found and something else if it is not found, you should have
two sets of specifications. I usually do something like this:
context 'when records is found' do
before :each do
@record = stub_model(SomeClass)
SomeClass.should_receive(:find).and_return(@record)
end
it 'should test something' do
blah blah blah
end
end
context 'when record is not found' do
SomeClass.should_receive(:find).and_return(nil)
it 'should test something else' do
end
end
I've thrown in another method, .should_receive. This establishes an
expectation that this method should be called during the execution of
your code. If it does not get called, you will get a failure. Stub!, on
the other hand, just creates the method *in case* it gets called. No
error will be raised if it doesn't. So in the bogus code above, I am
saying that I expect my controller to call SomeClass.find, and I want to
test what it does when the find is successful and when it isn't. When it
is successful, it will be returning something that the application will
think is an instance of the class, and when it is not successful, it
will be returning nil, which is what normally happens in AR. However,
because I'm "mocking and stubbing", I'm bypassing AR completely.
That's all I have time for right now. I hope this has helped a little
bit. I encourage you to take deep breaths often and just hang in there.
Keep asking questions. Soon or later, the light will click and you'll
have an "Aha!" moment.
Peace,
Phillip
Phillip Koebbe
> There is an alternate syntax that I'm using nearly exclusively at this
> point, which is to pass a block along with the stub() and
> should_receive() messages:
>
> foo.stub(:bar) { "return value" }
> foo.stub(:bar) { raise SomeError }
> foo.stub(:bar) { yield "value" }
> foo.stub(:bar) { throw :value }
> foo.should_receive(:bar) { "return value" }
> foo.should_receive(:bar) { raise SomeError }
> foo.should_receive(:bar) { yield "value" }
> foo.should_receive(:bar) { throw :value }
>
> As you can see, there is much less API to worry about here. You're
> just supplying an implementation.
>
> This approach also works with scoped arguments:
>
> input = Input.new
> calculator = Calculator.new(input)
> calculator.should_receive(:add).with(1,2) { 3 }
> input.type("1", "+", "2", "Enter")
>
> I've been thinking of deprecating the and_xxx methods in rspec-2. Does
> anybody think that's an awful idea?
>
It may be because my RSpec-fu is relatively immature, but I like
and_return and friends. It makes sense to my brain. Admittedly, I have
not used the other syntax yet, so I really should give it a go and see
what I think. At the present moment, I would be very disappointed if the
and_ methods went away.
-1
Peace,
Phillip
Patrick J. Collins
> I've been thinking of deprecating the and_xxx methods in rspec-2. Does
> anybody think that's an awful idea?
Well I can tell you that this syntax you demonstrated:
> foo.stub(:bar) { "return value" }
> foo.stub(:bar) { raise SomeError }
> foo.stub(:bar) { yield "value" }
> foo.stub(:bar) { throw :value }
is so much clearer to me, intuitive, and friendly....
Thank you very much for your explanations, they really help-- I finally get the
whole stub / should_receive... The only thing that is still a bit fuzzy to me,
is knowing when to use one over the other.
With the case of should_receive, I am having a hard time imagining a useful
test, because everything I am thinking of-- is something like:
def index
@blogs = Blog.all
...
end
#
Blog.should_receve(:all) { @list_of_blogs }
...
And that to me, seems like a pointless test because it's evident visually that
the code is fine, flawless and will work exactly as expected. But I probably
am missing something...
Now, one other thing I might ask you to explain, since you do such a great job,
is what are the differences between "mock" "mock_model" and "should_receive"
since they all are "mocking"...?
Patrick J. Collins
http://collinatorstudios.com
Tom Stuart
On 20 Mar 2010, at 10:42, David Chelimsky wrote:
> There is an alternate syntax that I'm using nearly exclusively at this
> point, which is to pass a block along with the stub() and
> should_receive() messages:
> [...]
> I've been thinking of deprecating the and_xxx methods in rspec-2. Does
> anybody think that's an awful idea?
I don't think it's an awful idea -- less API is a laudable goal -- but I dislike it for two reasons:
1. I like the symmetry of ".with(...).and_return(...)". The alternative of ".with(...) { ... }" feels weird, lopsided and misleadingly implicit to me; #with is "merely configuration" too, so why does #and_return get the special treatment?
2. I like the semantics of #and_return, i.e. its argument gets eagerly evaluated by Ruby (at "stub time") and the return value sits there waiting to be returned whenever the stubbed message is received. I'm uncomfortable about *always* specifying the return value by providing a block whose evaluation happens later (at "message time") and potentially has side-effects, argument-dependent behaviour etc. I appreciate that #stub and #should_receive already give you the ability to pass a block for this purpose, and that's occasionally useful, but I much prefer being able to explicitly choose that style when it's absolutely necessary (e.g. when I absolutely must have a "dynamic stub" whose return value actually depends upon its arguments) but this is very much the exception rather than the rule. (Quoth rspec.info: "While this is possible, it is generally to be avoided. Calculating a value to return defeats the declarative nature of stubs.") It's nice to be able to see the difference
syntactically rather than having to inspect the behaviour of the block to see whether it's doing anything clever; namely, #and_return serves as a nice annotation that you're doing "the normal thing".
So basically I suspect that your proposal makes it easier for RSpec newbies to learn the API since the block form is potentially less surprising and "more Ruby", but I fear that it also makes things less explicit and therefore harder to understand & maintain further down the road.
Cheers,
-Tom
Patrick J. Collins
> Welcome to RSpec, Patrick. For some of us, it's pretty rocky at first. I
Thank you Phillip for your great explanation...
After reading what you wrote, I have a few questions:
1. From what I understand of what you wrote, stub_model makes a fake object,
with accessor methods for the AR columns, and they return nil.
So what happens behind the scenes with mock_model? What is the difference?
2. I sort of asked this earlier, but didn't really get addressed--- What is the benefit to doing stubbing
vs. using a factory or fixture?
3. In your code, you wrote:
> context 'when record is not found' do
> SomeClass.should_receive(:find).and_return(nil)
I just wondered about this, because-- correct me if I am wrong, but a record not found would not return
nil-- it returns a record not found error, correct? Or does an error actually equate to nil? I know in a
lot of my controllers to avoid record not found errors, I do .find_by_id instead of .find, because that
will return nil if it's not found.
4. should_receive... So, if you have a method that is calling other class and instance methods, isn't it
a given that those calls are important and that they should be expected? In other words, I am finding
myself asking the same question again-- Why use stub instead of should_receive? When would you actually
not care whether a method gets called or not if you are testing something. I mean, I would assume if I am
testing something, I want to test everything that happens-- so any real code in my app that deals with
class/instance methods, those seem like they would be important and therefore those calls should be
expected. ?
5. Since I learn really well from example, I just wrote up some random and strange code, which does a
handful of things, and I would love it if you could go through and spec it-- and explain each step of the
way, what you're doing and why. What is necessary to test, and what is not, and of course also-- why.
def index
type = params[:foo]
value = params[:bar]
@foo = type && value ? Foo.send("find_by_#{type}", value) : Foo.all
@baz = /.*lower/.match(params[:baz]) ? @foo.lowered_name : @foo.upper_name
flash[:notice] = "Something with #{@foo.numbers}" unless params[:bool].to_i.zero?
respond_to do |format|
format.html { render 'foo_listing' }
format.js { render :json => { :foo => @foo }.to_json
format.pdf { render :inline => @foo.pdf }
end
end
#
class Foo < AR::base
def lowered_name
name.downcase
end
def upper_name
name.upcase
end
def numbers
rand(id)
end
def pdf
# return some prawn pdf object
end
...
If you could do that for me, it would be incredibly awesome...
Thank you.
Patrick J. Collins
http://collinatorstudios.com
Phillip Koebbe
> On Sat, 20 Mar 2010, Phillip Koebbe wrote:
>
>
>> Welcome to RSpec, Patrick. For some of us, it's pretty rocky at first. I
>>
> Thank you Phillip for your great explanation...
>
> After reading what you wrote, I have a few questions:
>
> 1. From what I understand of what you wrote, stub_model makes a fake object,
> with accessor methods for the AR columns, and they return nil.
>
> So what happens behind the scenes with mock_model? What is the difference?
>
>
Looking at http://rspec.rubyforge.org/rspec-rails/1.3.2/, I can't see
any difference between mock_model and stub_model. David?
>
> 2. I sort of asked this earlier, but didn't really get addressed--- What is the benefit to doing stubbing
> vs. using a factory or fixture?
>
>
>
I passed on this question since I am not very familiar with projects
like factory_girl or machinist. I'd like to look into them more at some
point, but just don't have the time right now. As for fixtures, I can't
stand them. We used fixtures on that project I mentioned earlier. At
first, they were great. But as the project grew, it was a nightmare
keeping them working right. I *much* prefer mocking and stubbing.
> 3. In your code, you wrote:
>
>
>> context 'when record is not found' do
>> SomeClass.should_receive(:find).and_return(nil)
>>
> I just wondered about this, because-- correct me if I am wrong, but a record not found would not return
> nil-- it returns a record not found error, correct? Or does an error actually equate to nil? I know in a
> lot of my controllers to avoid record not found errors, I do .find_by_id instead of .find, because that
> will return nil if it's not found.
>
Right. I goofed there. It should have been .find_by_id. The point that I
was hoping to make, though, is that you can create the environment
needed to test what your controller does when certain conditions are
met. Consider the example of wanting to redirect to an error page if a
resource is not found:
context 'resource is found' do
before :each do
@some_object = stub_model(SomeClass)
SomeClass.should_receive(:find_by_id).and_return(@some_object)
get :show, :id => 1
end
it 'should render the show page' do
# not sure if my syntax is correct here...i use remarkable which
has a macro for it
response.should render(:show)
end
end
context 'resource is not found' do
before :each do
SomeClass.should_receive(:find_by_id).and_return(nil)
get :show, :id => 1
end
it 'should redirect to error page' do
# not sure if my syntax is correct here...i use remarkable which
has a macro for redirects
response.should redirect_to error_path
end
end
You are validating that the controller code works correctly. It would
look like:
def show
@some_object = SomeClass.find_by_id(params[:id])
redirect_to error_path and return unless @some_object
end
>
>
> 4. should_receive... So, if you have a method that is calling other class and instance methods, isn't it
> a given that those calls are important and that they should be expected? In other words, I am finding
> myself asking the same question again-- Why use stub instead of should_receive? When would you actually
> not care whether a method gets called or not if you are testing something. I mean, I would assume if I am
> testing something, I want to test everything that happens-- so any real code in my app that deals with
> class/instance methods, those seem like they would be important and therefore those calls should be
> expected. ?
>
>
Think about testing in isolation, but now isolating specific pieces of
functionality in your controller. If I have already tested one piece
with expectations, when I move on to another piece, I can just stub them
and focus on the code at hand.
This example isn't necessarily controller-specific, but maybe it will
help see the difference. Suppose you have a method that performs some
calculation. At one point in your testing, you should verify that the
method gets called when you expect it to. That's when you would use
.should_receive. At this point, you don't really care what the result
is, just that your application code is invoking the calculation at the
right time. Now, when you are validating what the application does *in
response* to the result of the calculation, that's when you would just
stub it since you don't really care about the calling of it, but what it
returns. So if you want to do one thing when the result is A and another
when it is B, you would stub it .and_return(A) in one case and
.and_return(B) in the other.
>
> 5. Since I learn really well from example, I just wrote up some random and strange code, which does a
> handful of things, and I would love it if you could go through and spec it-- and explain each step of the
> way, what you're doing and why. What is necessary to test, and what is not, and of course also-- why.
>
> def index
> type = params[:foo]
> value = params[:bar]
>
> @foo = type&& value ? Foo.send("find_by_#{type}", value) : Foo.all
>
> @baz = /.*lower/.match(params[:baz]) ? @foo.lowered_name : @foo.upper_name
>
> flash[:notice] = "Something with #{@foo.numbers}" unless params[:bool].to_i.zero?
>
> respond_to do |format|
> format.html { render 'foo_listing' }
> format.js { render :json => { :foo => @foo }.to_json
> format.pdf { render :inline => @foo.pdf }
> end
> end
>
> #
> class Foo< AR::base
>
> def lowered_name
> name.downcase
> end
>
> def upper_name
> name.upcase
> end
>
> def numbers
> rand(id)
> end
>
> def pdf
> # return some prawn pdf object
> end
>
> ...
>
> If you could do that for me, it would be incredibly awesome...
>
>
I would be glad to help in this way, but it will have to be later today.
And instead of using completely made-up code, I'll use some of my
real-world code and tests as an example. I'll gist them later, unless
one of the gurus comes along and provides enlightenment for you!
Phillip Koebbe
> I would be glad to help in this way, but it will have to be later
> today. And instead of using completely made-up code, I'll use some of
> my real-world code and tests as an example. I'll gist them later,
> unless one of the gurus comes along and provides enlightenment for you!
>
I was able to get to it sooner than I thought.
This is only one action on a controller. Hopefully it will be enough to
help. I have other code, but it may take too much explanation about the
business process for it to make sense, and that would defeat the purpose
of helping you understand.
It may help you if you try to think in terms of writing the specs first,
before you have any code. After all, that's the point of T/BDD: to write
tests/specs first, then the code that satisfies them. The whole
red/green/refactor process. So when you establish an expectation that a
method will be called, that expectation is satisfied when you have that
code in your application. If you are writing specs to cover existing
code, you don't have the luxury of thinking this way, and that makes it
more difficult.
In the gist, notice the helper methods to simulate someone being logged
in. I'm using stub there because I don't want an expectation of the
methods being called to potentially raise an error. I'll do that
elsewhere. In this case, I just want the methods to be available when
they are needed.
David Chelimsky
<philli...@gmail.com> wrote:
>
>> I would be glad to help in this way, but it will have to be later today.
>> And instead of using completely made-up code, I'll use some of my real-world
>> code and tests as an example. I'll gist them later, unless one of the gurus
>> comes along and provides enlightenment for you!
>>
>
> I was able to get to it sooner than I thought.
>
> http://gist.github.com/338707
Hey Phillip,
Thanks for taking time to do this. Your examples are clear, and the
documentation very helpful. They do, however, violate a few guidelines
that I try to follow and recommend.
Note that I am not trying to change your mind, Phillip, about how you
should be writing your specs. In the end, the whole purpose of writing
specs is to help you build a system that you can understand and
maintain, and provide confidence that the system works according to
your assumptions by encoding them. For me, that confidence is tied
directly to my ability to read specs and understand failures quickly,
which is an admittedly subjective thing.
Given that context ...
1. Keep setup local
When an example fails, we want to understand the context as quickly as possible.
The purpose of nested describe and context blocks is to group things
together conceptually. While the before blocks happen to support
cascading (i.e. the ones higher up the nesting chain get run first),
using them that way makes it much harder to understand the context
when an example fails. In order to understand why any of these
examples fail, I'd have to look at the before block in the current
context, then the before block in the describe block, then at the
setup methods at the top. That might not seem so bad in a simple
example like this, but it gets unwieldy quite quickly.
2. Setup context in before blocks, not expectations
We want the example names to express the intent of each example.
The before block in the "with valid id' context sets two expectations,
and then each example sets one of its own. If "should_not_assign_to
:message" fails because Message didn't receive :find_by_id, the
failure has nothing to do with the intent expressed by that example.
Same goes for @message.should_receive(:destroy).
If you want those expectations to be part of the spec, then add
examples for them. If you don't care that they are specified, then
don't. Either way, prefer stub() over should_receive() in before
blocks.
3. Use expressive names for helper methods
When I see "before(:each) { setup_admin }", I don't really know what
that means. In order to figure it out, I have to look up at the
setup_user, setup_admin, and stub_user methods, and I need to
understand them all to really understand the purpose of any of them.
These could (and I'd say should) all be collapsed into this:
def login_as(type)
user.stub(:is_administrator).and_return(type == :admin)
User.stub(:find_by_id).and_return(user)
session[:user_id = user.id]
end
Now the before block can say "before(:each) { login_as :admin}" and a)
I probably don't have to go looking at that method to understand its
intent and b) I definitely don't have to go looking at three different
methods.
4. Clarity trumps DRY
DRY is a very important principle, but it is often misunderstood to
mean "don't type the same thing twice." That is not its intent, and
interpreting it as such often leads to less clarity. The bigger idea
is that we shouldn't express the same concept in two different parts
of a system. i.e. if we have two different parts of a codebase that
validate an email address, for example, and the rules for a valid
email address change (we decide to ping a service to see if the email
address actually exists in addition to validating its format), we run
the risk that we'll forget to change it in both cases. DRY helps us to
avoid that risk.
This is a completely different matter from organizing a spec.
Consider http://gist.github.com/338767. There are 4 permutations of
what is essentially the same spec. I'd argue that the two examples
near the top are far easier to grok than the latter two, even though
the latter two are DRYer. The first example spells out everything in
each example with zero indirection and very little API to grok. The
second one might be a bit DRYer but it requires more API (let).
5. Given/When/Then
This probably shouldn't be last, but ...
Express every example in the form Given/When/Then: given some context,
when some event occurs, then the outcome should be ...
Consider the first example group in http://gist.github.com/338767.
Both examples in that express the givens (person and day) in the first
two lines, the event (person walks on the day) on the third line, and
the outcome on the last line. The symmetry between them makes it very
easy to see the differences between them, even though that is probably
the least DRY example in the gist.
That's probably enough to chew on for now. All comments welcome.
Cheers,
David
Phillip Koebbe
> On Sat, Mar 20, 2010 at 10:19 AM, Phillip Koebbe
> <philli...@gmail.com> wrote:
>
>>
>>> I would be glad to help in this way, but it will have to be later today.
>>> And instead of using completely made-up code, I'll use some of my real-world
>>> code and tests as an example. I'll gist them later, unless one of the gurus
>>> comes along and provides enlightenment for you!
>>>
>>>
>> I was able to get to it sooner than I thought.
>>
>> http://gist.github.com/338707
>>
> Hey Phillip,
>
> Thanks for taking time to do this. Your examples are clear, and the
> documentation very helpful. They do, however, violate a few guidelines
> that I try to follow and recommend.
>
Heh, that doesn't surprise me at all. All of my life, I've needed to do
ABC, so I've figured out a way to do ABC. I frequently learn later,
however, that I chose a hard way to do ABC and then must learn an easier
way. Programming has shown itself to not be an exception to this pattern. :)
> Note that I am not trying to change your mind, Phillip, about how you
> should be writing your specs. In the end, the whole purpose of writing
> specs is to help you build a system that you can understand and
> maintain, and provide confidence that the system works according to
> your assumptions by encoding them. For me, that confidence is tied
> directly to my ability to read specs and understand failures quickly,
> which is an admittedly subjective thing.
>
>
I appreciate your perspective, David. I appreciate all of the
perspectives that I see here. Some are presented in a more palatable
manner than others, but they are all valuable. I don't let much get by
without considering its place in my own workflow.
> Given that context ...
>
> 1. Keep setup local
>
> When an example fails, we want to understand the context as quickly as possible.
>
> The purpose of nested describe and context blocks is to group things
> together conceptually. While the before blocks happen to support
> cascading (i.e. the ones higher up the nesting chain get run first),
> using them that way makes it much harder to understand the context
> when an example fails. In order to understand why any of these
> examples fail, I'd have to look at the before block in the current
> context, then the before block in the describe block, then at the
> setup methods at the top. That might not seem so bad in a simple
> example like this, but it gets unwieldy quite quickly.
>
>
I confess that I have created nestings just to be able to share a before
block. A typical spec file for one of my controllers is a nested mess,
and now I know why. Funny that I wrote the silly things but didn't
realize I was creating such a problem until someone else took a quick
peek. Oh that I was in a position to pair with someone regularly. That
would help tremendously.
> 2. Setup context in before blocks, not expectations
>
> We want the example names to express the intent of each example.
>
> The before block in the "with valid id' context sets two expectations,
> and then each example sets one of its own. If "should_not_assign_to
> :message" fails because Message didn't receive :find_by_id, the
> failure has nothing to do with the intent expressed by that example.
> Same goes for @message.should_receive(:destroy).
>
> If you want those expectations to be part of the spec, then add
> examples for them. If you don't care that they are specified, then
> don't. Either way, prefer stub() over should_receive() in before
> blocks.
>
Good point. By having the expectations in the setup, I also don't get
the documentation when I run the examples. I didn't think of that before.
> 3. Use expressive names for helper methods
>
> When I see "before(:each) { setup_admin }", I don't really know what
> that means. In order to figure it out, I have to look up at the
> setup_user, setup_admin, and stub_user methods, and I need to
> understand them all to really understand the purpose of any of them.
> These could (and I'd say should) all be collapsed into this:
>
> def login_as(type)
> user.stub(:is_administrator).and_return(type == :admin)
> User.stub(:find_by_id).and_return(user)
> session[:user_id = user.id]
> end
>
I like this much better than what I currently have. As a recovering
perfectionist, I suffer from the "it's gotta be named just right"
syndrome. I sometimes spend far too much time trying to figure out the
best name for this or that, and I've taken to grabbing something that is
remotely close and moving on so I don't spend my wheels too long.
> Now the before block can say "before(:each) { login_as :admin}" and a)
> I probably don't have to go looking at that method to understand its
> intent and b) I definitely don't have to go looking at three different
> methods.
>
Of course, *you* might have to wonder what's going on because you're not
familiar with the project. But since *I* wrote it, I immediately
understand what setup_admin means. But I still like login_as better ;)
> 4. Clarity trumps DRY
>
> DRY is a very important principle, but it is often misunderstood to
> mean "don't type the same thing twice." That is not its intent, and
> interpreting it as such often leads to less clarity. The bigger idea
> is that we shouldn't express the same concept in two different parts
> of a system. i.e. if we have two different parts of a codebase that
> validate an email address, for example, and the rules for a valid
> email address change (we decide to ping a service to see if the email
> address actually exists in addition to validating its format), we run
> the risk that we'll forget to change it in both cases. DRY helps us to
> avoid that risk.
>
> This is a completely different matter from organizing a spec.
>
>
This is the rub, I think. Much of the "incorrectness" of my specs can be
traced back to my effort to stay DRY. I truly appreciate your comment
about people often misunderstanding DRY to mean "don't retype what you
don't have to." I'm guilty of that. Admittedly, I have not done
extensive reading about the topic, but most of what I catch when people
mention it in forums is just that: keep something DRY by refactoring it.
I can see that retyping is obviously part of it, but it is really much
more than that. As with just about everything else in life, one must
find the balance.
> Consider http://gist.github.com/338767. There are 4 permutations of
> what is essentially the same spec. I'd argue that the two examples
> near the top are far easier to grok than the latter two, even though
> the latter two are DRYer. The first example spells out everything in
> each example with zero indirection and very little API to grok. The
> second one might be a bit DRYer but it requires more API (let).
>
>
The first two are most certainly easier on my eyes. That in and of
itself would be worth changing how I do these things.
[As an aside, I believe you have a copy/paste oops in your "without an
umbrella" or "gets wet" contexts. Shouldn't it be person.should_not
be_dry ?]
> 5. Given/When/Then
>
> This probably shouldn't be last, but ...
>
> Express every example in the form Given/When/Then: given some context,
> when some event occurs, then the outcome should be ...
>
> Consider the first example group in http://gist.github.com/338767.
> Both examples in that express the givens (person and day) in the first
> two lines, the event (person walks on the day) on the third line, and
> the outcome on the last line. The symmetry between them makes it very
> easy to see the differences between them, even though that is probably
> the least DRY example in the gist.
>
>
I should probably try to use that terminology more in my specs. It's
proven valuable in the cucumber features, so it should be likewise so in
rspec.
> That's probably enough to chew on for now. All comments welcome.
>
>
That's a mouthful, for sure!
Thank you, David, for taking the time to generate such a thoughtful
critique of my work. Not only do I get the benefit of your experience,
knowledge and insight, but so does everyone following at home.
Patrick, don't listen to me :) Listen to David.
> Cheers,
> David
David Chelimsky
> Patrick, don't listen to me :) Listen to David.
That's just silly. Patrick, please listen to both of us (and everyone else who contributes to this list). What Phillip wrote about the difference between mocks and stubs was dead on. I was commenting on the context.
Cheers,
David
Phillip Koebbe
>
> We want the example names to express the intent of each example.
>
> The before block in the "with valid id' context sets two expectations,
> and then each example sets one of its own. If "should_not_assign_to
> :message" fails because Message didn't receive :find_by_id, the
> failure has nothing to do with the intent expressed by that example.
> Same goes for @message.should_receive(:destroy).
>
> If you want those expectations to be part of the spec, then add
> examples for them. If you don't care that they are specified, then
> don't. Either way, prefer stub() over should_receive() in before
> blocks.
>
I just remembered why I started putting .should_receive in the setup. It
was because I use Remarkable and I had the action (post :create, get
:new, whatever) in the before block. Here's an example from what I'm
working on today:
Before I started using Remarkable, I think I would have put the
expectation in an example (I can't remember how much I used
should_receive at that point, so I don't know for sure). When I started
using Remarkable, I ran into the issue of needing to set the expectation
before the action was performed, so I moved it into the setup. I had
forgotten that until today when I was merrily moving the expectations
back to examples and only having the stub in the setup.
So now my dilemma is to
1) continue doing it the way I have been even though it's not "proper"
2) wrap the Remarkable macros in a context so I can use a before block
to execute the action
3) stop using Remarkable
4) some other solution I'm not currently aware of
Funny enough, I'm actually leaning toward #3.
Thoughts?