

RSpec Best Practices - andreareginato
http://betterspecs.org/

======
molf
I can't believe this line is considered good practice:

    
    
        collection.should have(2).items
    

It illustrates what I believe is wrong with RSpec: the assertion style is
completely disconnected from the API that is being tested. There's no way you
can tell what methods are being called on the `collection` object without
being intimately familiar with _this specific_ RSpec matcher.

So what does that line actually do? Apart from calling strange methods like
`items` on strange objects like `have(2)` in the end the test passes iff
`collection.length` equals 2. Compare RSpec with classic test/unit assertions:

    
    
        collection.should have(2).items
    
        assert_equal 2, collection.length
    

The second line is just so much more obvious to me.

On the other hand I really like the RSpec way of grouping tests and
structuring test cases. My favourite test cases are written with minitest/spec
(a lightweight test/unit-compatible clone of RSpec) and test/unit-style
assertions.

~~~
reyan
In fact RSpec team is in the process of changing the default to the new expect
matcher syntax ([http://myronmars.to/n/dev-blog/2012/06/rspecs-new-
expectatio...](http://myronmars.to/n/dev-blog/2012/06/rspecs-new-expectation-
syntax)).

------
latortuga
This would've been good when I was just starting with RSpec. A couple things
I'd like to add:

    
    
        Mock or not to Mock
    

Mock and stub relentlessly. Ensure that the object you're testing the behavior
of sends the right messages to its collaborators but put the tests for those
messages in the unit test for the collaborator. This keeps your tests isolated
and fast. As long as you adhere to "Tell, Don't Ask" you should simply be able
to assert that your method is sending messages to other objects without
relying on the behavior that those methods perform.

    
    
        Create only the data you need
    
        describe "User"
          describe ".top" do
            before { 3.times { Factory(:user) } }
            it { User.top(2).should have(2).item }
          end
        end
    
    

The author advocates not creating dozens of objects but why make any at all?
This seems like a place where setting expectations is a better course of
action. There's no reason to test ActiveRecord query methods like this - just
assert that the filters that you want are being set by the method and call it.
I usually do something like this (using rspec-mocks):

    
    
        subject.should_receive(:popular).and_return(subject)
        subject.should_receive(:by_create_date).and_return(subject)
        ...
    

This lets you know that the class method is performing the filtering you want
without actually fetching data from the db. Much like validations, this is the
kind of thing you can rely on either the Rails team or the db adapter writers
to get the actual filtering right without having to test this over and over in
your app. Having your unit tests (business logic!) actually hit your
persistence layer like this is the kind of thing that makes test suites slow!

~~~
chromatic
_This keeps your tests isolated and fast._

No, _writing good tests_ keeps your test isolated and fast. Creating lots of
mock objects distracts you from writing good tests, because what you're
testing is your ability to 1) write mock objects that conform to the ad hoc
interfaces you've produced and 2) maintain those mock objects when those ad
hoc interfaces change.

If you want to know if your code works, you still have to test if it all works
together.

~~~
urbanautomaton
But if you're having to create lots of mock objects, that's a huge signal that
your objects are tightly coupled, and that not only are you writing a bad
test, you've written a bad subject-under-test.

Yes, you still have to write integration tests. But the discipline of testing
your objects in isolation and outlining the objects they collaborate with is a
major benefit of using test doubles, not a downside. If you can't reason about
your objects in isolation, how can you expect to reason about them in
aggregate?

~~~
chromatic
_If you can't reason about your objects in isolation, how can you expect to
reason about them in aggregate?_

I don't _use_ my objects in isolation.

Maybe I can mock up some factory which creates model objects outside of my
persistence mechanism, but what value is that? I care that when I create an
object in response to an action from a real user the persistence mechanism
produces the correct object.

I happily bulk load testing data into my persistence mechanism for testing
purposes, but I see almost no reason to mock it for tests.

------
oesmith
I _really_ don't like the idea of "no controller tests at all" as a best
practice.

If you're having to mixin Rack::Test::Methods to write your "full-coverage"
integration spec, then you're really writing a controller test. It should go
in a controller spec without all the horrid mixin hacks.

~~~
josephlord
I test at controller level that things aren't accessible (security) and at
integration level that the appropriate things are accessible and error
messages are correct when not (functionality).

~~~
oesmith
Yup, the _real_ best practice is to use your judgement and choose the right
test for the job.

If it's something that you can see in a browser (and the results are visible
too), then it's a candidate for an integration spec.

You really shouldn't be integration testing stuff like security. For example,
testing a user can't submit a form they can't see (which is where horrible
Rake::Test::Methods mixin hacks start appearing).

You also shouldn't (imho) be integration testing side effects that aren't
visible to the user. For example, if an action ends up in an audit log that
the user is never going to see, there's no point firing up an integration test
to do that. A light-weight controller test can do that just fine.

------
ryth
I'm not a ruby dev, but that first example seems to fly in the face of the
whole semantic DSL that RSpec is attempting to accomplish.

~~~
gamache
RSpec isn't aiming to be used as plain English; that's more of a Cucumber
thing. I appreciate RSpec for that.

~~~
viseztrance
Personally I don't like the first example. I also don't believe it resembles
cucumber at all.

It's certainly more closer to a documentation string (the ones you see in
python or common lisp).

In any case given the following:

    
    
        describe 'if the user is an admin' do
    

versus

    
    
        describe '#admin?' do
    

Ignoring the fact that the author is ignoring BDD here, I certainly like the
first one more, even if the user part is a bit redundant.

Nevertheless, what the author has done is commendable but there are few cases
there that I just can't agree with. I don't see anything really wrong about
them, it's just that in real life things can get complicated.

~~~
arthurschreiber
To be honest, I think both styles are good practice, but it totally depends on
the context. For example, if I want to describe how the "#admin?" method is
working, I'll use

    
    
        describe "#admin?" do ...
    

but if I'm describing a different method, that does different things depending
on whether the user is an admin or not, I'll do the following:

    
    
        describe "#some_other_method" do
          describe "if the user is an admin" do
            ...
    

So you can mix and match both styles, depending on what exactly you're
describing (and it's context).

If you're looking at the OP more closely, it even says this is only
recommended for "describing methods".

------
awj
I typically go one step further in the "mocking http requests" and write a
simple class that does nothing but make the http call and translate
input/output. From there you can stub/mock that object as appropriate. That
way you have a _very_ simple interface to mock/stub against and don't have to
worry about things like webmock. Well, at least until you start testing your
http service interaction classes, but at least those are simpler and have
relatively clearly defined uses.

