[rspec-users] lots of nil problems!

David Chelimsky dchelimsky at gmail.com
Sat Mar 20 13:17:58 EDT 2010


On Sat, Mar 20, 2010 at 10:19 AM, Phillip Koebbe
<phillipkoebbe at 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

>
> 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
> _______________________________________________
> rspec-users mailing list
> rspec-users at rubyforge.org
> http://rubyforge.org/mailman/listinfo/rspec-users
>


More information about the rspec-users mailing list