Lots of nil problems!

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

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

Hey Patrick.

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.

On Sat, Mar 20, 2010 at 2:28 AM, Patrick J. Collins
[email protected] wrote:

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?

Welcome to RSpec, Patrick. For some of us, it’s pretty rocky at first. I
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!

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”

  1. 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 D.’)

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 D.’, :age => 99, :gender =>
‘M’)

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

foo.should_receive(:bar) { yield “value” }
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

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

?

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

  1. 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… ?

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

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

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

Hi David,

On 20 Mar 2010, at 10:42, David C. 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

On Sat, 20 Mar 2010, Phillip K. 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?

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

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

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

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

On 2010-03-20 7:02 AM, Patrick J. Collins wrote:

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?

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

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

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.

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!

Peace,
Phillip

Hi David,

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

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.

Peace,
Phillip

On Sat, Mar 20, 2010 at 10:19 AM, Phillip K.
[email protected] 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.

gist:338707 · GitHub

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.

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

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

  1. 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 clear_vs_dry.rb · GitHub. 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).

  1. 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 clear_vs_dry.rb · GitHub.
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

On Mar 20, 2010, at 1:22 PM, Phillip K. wrote:

Patrick, don’t listen to me :slight_smile: 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

On 2010-03-20 12:17 PM, David C. wrote:

I was able to get to it sooner than I thought.

gist:338707 · GitHub

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. :slight_smile:

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.

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.

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.

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

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 clear_vs_dry.rb · GitHub. 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 ?]

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 :slight_smile: Listen to David.

Cheers,
David

Peace,
Phillip

On 2010-03-20 12:17 PM, David C. wrote:

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?

Peace,
Phillip